-
Notifications
You must be signed in to change notification settings - Fork 740
chore: Adds build file to get new codebuild project running in CI #5476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does duplicates a lot from https://github.com/aws/s2n-tls/blob/main/codebuild/spec/buildspec_ktls.yml
- Is https://github.com/aws/s2n-tls/blob/main/codebuild/spec/buildspec_ktls.yml#L34-L44 actually necessary? Could both use the simpler spec?
- If the old spec still needs all the extra stuff, can we make that conditional? Either probe for info about the environment, or control it with an environment variable set in the Codebuild job itself?
Idk this just seems like very complicated build logic to duplicate :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're able to remove those lines because Doug now has a script that will run those lines before creating the AMI snapshot. So my guess is that we would need to recreate the existing KTLS fleet in order to also use this newer slimmer buildspec (@dougch should probably weigh in on this).
I dunno, adding a branch to the build logic would be 🤮 . We totally could do it, we'd just have to branch for like, either ubuntu25 or al2023.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we recreate the existing ktls fleet and delete the existing ktls buildspec as a follow-up to this, then I think I'm happy with that plan. Otherwise, I'd prefer the explicit branch on al2023 to gate the ugly setup logic. It should just be one if, and it'd make the behavior between the two builds really clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've signed up to rebuild the existing AL2023 fleets, then we can def. consolidate on a single kTLS buildspec in a future PR. Points for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay so it seems like Doug was already planning on recreating the existing ktls fleet. So we should commit this PR and delete the existing buildspec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we open and link an issue to track that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened an issue: #5483
Release Summary:
Resolved issues:
Description of changes:
Adds a new buildspec that builds s2n-tls with aws-lc and runs the unit tests. We will be using this buildspec in our CI on our new Ubuntu25 codebuild project. With this newest build we will be able to do KTLS key updates. This buildspec is roughly the same as the existing ktls buildspec, but with several steps removed. Those steps are not necessary for the fleet we're using for ktls_keyupdates, because we now know how to create an AMI where those steps have already been performed. You can see a successful run of this buildspec here.
Call-outs:
I need this buildspec in our CI before I start opening ktls keyupdate PRs. Once this PR is merged I can change the build to use this buildspec.
Testing:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.