Create pre-production stage for CI pipeline #2282

nhatnghiho · 2025-03-20T15:59:58Z

Issues:

Resolves #CryptoAlg-2983 and #CryptoAlg-2984

Description of changes:

This PR implements steps to deploy to pre-production in the CI pipeline, which will act as template for production deployment later on. The PR also sets up a dev deployment environment for testing purposes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

codecov-commenter · 2025-03-20T16:23:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.75%. Comparing base (652f7a9) to head (cf7cdeb).
Report is 38 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2282      +/-   ##
==========================================
- Coverage   79.02%   78.75%   -0.27%     
==========================================
  Files         614      620       +6     
  Lines      106934   107954    +1020     
  Branches    15145    15335     +190     
==========================================
+ Hits        84504    85023     +519     
- Misses      21777    22277     +500     
- Partials      653      654       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tests/ci/cdk/cdk/codebuild/ec2_test_framework_omnibus.yaml

skmcgrail · 2025-04-02T18:32:24Z

tests/ci/cdk/cdk/aws_lc_ec2_test_framework_ci_stack.py

+        if env.account == PRE_PROD_ACCOUNT:
+            github_repo_owner = STAGING_GITHUB_REPO_OWNER
+            github_repo_name = STAGING_GITHUB_REPO_NAME


How does this work for deploying to your own personal account if you want to go through setting up the pipeline and all that?

I guess these are correctly set to the values from the pipeline cdk_env in AwsLcCiPipeline?

Yes that's right, whenever you deploy the pipeline (using run-cdk.sh or manually with cdk deploy), the environment variables from your local terminal will passed to and retained in AwsLcCiPipeline through cdk_env

tests/ci/cdk/pipeline/scripts/build_target.sh

tests/ci/cdk/pipeline/scripts/check_trigger_conditions.sh

andrewhop · 2025-04-08T21:41:59Z

tests/ci/cdk/README.md

+### CI Commands
+Use these commands if you wish to deploy individual stacks instead of the entire pipeline.
+
+These commands are run from `aws-lc/tests/ci/cdk`.
+
+If not done previously, bootstrap cdk before running the commands below. Make sure that AWS_ACCOUNT_ID is the AWS account you wish to deploy the CI stacks to.


Is there any reason to keep this support and documented?

This is helpful if we want to deploy 1 stack at a time. Say, if I only want to deploy aws-lc-ci-integration and don't want to deal with anything else, these commands will come in handy.

andrewhop · 2025-04-08T21:58:53Z

tests/ci/cdk/cdk/aws_lc_github_ci_stack.py

+from aws_cdk import (
+    Duration,
+    Stack,
+    aws_codebuild as codebuild,
+    aws_iam as iam,
+    aws_s3_assets,
+    aws_logs as logs,
+    Environment,
+)


In the future it is much easier to review big changes like this if you don't reformat unrelated code. If there wasn't consistent formatting of this cdk code before your change it would much easier to review if one PR is just reformatting everything.

tests/ci/cdk/cdk/bm_framework_stack.py

andrewhop · 2025-04-08T22:04:23Z

tests/ci/cdk/pipeline/ci_stage.py

+
+        self.build_options = []
+
+        # Define CodeBuild Batch job for testing code.


Can this reuse any of the existing CDK definitions? In the future if we add a new build project to our public CI we might forget to update this spot as well.

tests/ci/cdk/pipeline/ci_stage.py

andrewhop · 2025-04-08T22:06:02Z

tests/ci/cdk/pipeline/ci_stage.py

+
+        self.build_options = []
+
+        # Define CodeBuild Batch job for testing code.


Is this the CI that tests the code in PRs, or the CI that tests the pipeline and docker images is working?

andrewhop · 2025-04-08T22:07:27Z

tests/ci/cdk/pipeline/ci_stage.py

+                    "curl -H \"Authorization: token ${GITHUB_PAT}\" https://api.github.com/user",
+                    "git clone https://${GITHUB_PAT}@github.com/${STAGING_GITHUB_REPO_OWNER}/${STAGING_GITHUB_REPO_NAME}.git",
+                    "git remote add upstream https://github.com/aws/aws-lc.git",
+                    "git fetch upstream",
+                    "git checkout main",
+                    "git merge upstream/main",


Why are you merging aws/aws-lc into the pr branch? What happens if there is a conflict?

This is merging aws/aws-lc into our staging repo's main branch, not the pr branch. Since our staging repo is for testing only, it should never diverge from aws-lc. Conflicts should never happen. If they do, it means something is wrong with our staging repo.

Why are you updating the staging branch locally?

andrewhop · 2025-04-08T22:10:20Z

tests/ci/cdk/pipeline/codebuild_batch_step.py

+            else {}
+        )
+
+    @jsii.member(jsii_name="produceAction")


Yeah, is there some functionality missing in the Python CDK? Have we given feedback to them about it

skmcgrail · 2025-04-24T15:55:34Z

tests/ci/cdk/cdk/windows_docker_image_build_stack.py

+
+        self.output = {
+            "s3_bucket_name": f"{env.account}-{S3_FOR_WIN_DOCKER_IMG_BUILD}",
+        }


I don't see output member documented for Stack class. Are you intending for these to just be CfnOutputs or are you wanting them to be exported values from the stack? Would self.export_value(...) be more appropriate?

I tried but that wouldn't work. export_values works for cross-stack dependencies. But since everything all belongs to AwsLcCiPipeline, the export won't be created by the time we need to reference in the stage's post actions

skmcgrail · 2025-04-24T15:59:44Z

tests/ci/cdk/pipeline/ci_stage.py

+                    "git fetch upstream",
+                    "git checkout main",
+                    "git merge upstream/main",
+                    # "git push origin main",


Should be uncommented?

skmcgrail · 2025-04-24T15:59:57Z

tests/ci/cdk/pipeline/ci_stage.py

+                    "git remote add upstream https://github.com/aws/aws-lc.git",
+                    "git fetch upstream",
+                    "git checkout main",
+                    "git merge upstream/main",


Probably should specify --ff-only as well here.

skmcgrail · 2025-04-24T16:00:41Z

tests/ci/cdk/pipeline/ci_stage.py

+            input=input,
+            commands=[
+                "cd tests/ci/cdk/pipeline/scripts",
+                "chmod +x check_trigger_conditions.sh",


This should just be corrected and committed to the repository with the correct permissions.

skmcgrail · 2025-04-24T16:02:23Z

tests/ci/cdk/pipeline/ci_stage.py

+            input=input,
+            commands=[
+                "cd tests/ci/cdk/pipeline/scripts",
+                "chmod +x build_target.sh",


Ditto here about fixing the permissions and committing them.

skmcgrail · 2025-04-24T16:06:46Z

tests/ci/cdk/pipeline/linux_docker_image_build_stage.py

+            input=input,
+            commands=[
+                "cd tests/ci/cdk/pipeline/scripts",
+                "chmod +x cleanup_orphaned_images.sh check_trigger_conditions.sh build_target.sh",


Commit the fixed permissions and then remove this line.

skmcgrail · 2025-04-24T22:20:56Z

tests/ci/cdk/pipeline/scripts/util.sh

+  export AWS_ACCESS_KEY_ID=$(echo $CREDENTIALS | jq -r .Credentials.AccessKeyId)
+  export AWS_SECRET_ACCESS_KEY=$(echo $CREDENTIALS | jq -r .Credentials.SecretAccessKey)
+  export AWS_SESSION_TOKEN=$(echo $CREDENTIALS | jq -r .Credentials.SessionToken)


I think this might cause the credentials to get logged? Should we call set +x when entering this function, and then re-enabling it on exit?

skmcgrail · 2025-04-24T22:22:37Z

tests/ci/cdk/run-cdk.sh

+#    if [[ "${bucket_name}" == *"${AWS_LC_S3_BUCKET_PREFIX}"* ]]; then
+#      aws s3 rm "s3://${bucket_name}" --recursive
+#      aws s3api delete-bucket --bucket "${bucket_name}"


Just remove the code if not needed anymore.

skmcgrail · 2025-04-24T22:23:56Z

tests/ci/cdk/run-cdk.sh

@@ -178,7 +178,7 @@ function win_docker_img_build_status_check() {

 function build_linux_docker_images() {
  # Always destroy docker build stacks (which include EC2 instance) on EXIT.
-  trap destroy_docker_img_build_stack EXIT
+#  trap destroy_docker_img_build_stack EXIT


same here, git keeps the history so just remove the code if not applicable anymore.

skmcgrail · 2025-04-24T22:25:50Z

tests/ci/cdk/util/metadata.py

+from datetime import datetime
+
+# timestamp = datetime.now().strftime('%Y-%m-%d-%H-%M')


Remove comment, and is import need anymore?

andrewhop · 2025-04-22T22:15:40Z

tests/ci/cdk/app.py

+    env=Environment(account=PIPELINE_ACCOUNT, region=PIPELINE_REGION),
+)
+
+if DEPLOY_ACCOUNT and DEPLOY_REGION:


If these aren't set what happens when this calls app.synth?

nhatnghiho force-pushed the ci-pipeline branch 2 times, most recently from 267feb5 to b100ece Compare March 20, 2025 22:30

nhatnghiho added 2 commits March 25, 2025 16:53

Create pipeline

45a1b91

Update cdk instructions

310e375

nhatnghiho force-pushed the ci-pipeline branch 3 times, most recently from 3f82970 to edbd376 Compare March 26, 2025 16:37

nhatnghiho marked this pull request as ready for review March 26, 2025 16:39

nhatnghiho requested a review from a team as a code owner March 26, 2025 16:39

skmcgrail self-requested a review March 26, 2025 20:34

nhatnghiho added 3 commits March 26, 2025 14:30

Add typing and cleanup

4fadbc4

Reformat code

10d511f

Fix minor bugs

7ec6289

nhatnghiho force-pushed the ci-pipeline branch from edbd376 to b93ce94 Compare March 28, 2025 20:34

nhatnghiho added 3 commits April 2, 2025 11:37

Add step to auto-sync private repo

775eeed

Fix dev pipeline deployment bug

3537e22

More cleanup

0a19c45

nhatnghiho force-pushed the ci-pipeline branch from b93ce94 to 0a19c45 Compare April 2, 2025 18:51

skmcgrail reviewed Apr 2, 2025

View reviewed changes

andrewhop reviewed Apr 8, 2025

View reviewed changes

nhatnghiho added 2 commits April 18, 2025 14:55

Incorporate comments

1d84eea

Remove stale benchmark scripts

cf7cdeb

skmcgrail reviewed Apr 24, 2025

View reviewed changes

andrewhop reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create pre-production stage for CI pipeline #2282

Create pre-production stage for CI pipeline #2282

nhatnghiho commented Mar 20, 2025 •

edited

Loading

codecov-commenter commented Mar 20, 2025 •

edited

Loading

skmcgrail Apr 2, 2025

skmcgrail Apr 2, 2025

nhatnghiho Apr 21, 2025

andrewhop Apr 8, 2025

nhatnghiho Apr 9, 2025

andrewhop Apr 8, 2025

andrewhop Apr 8, 2025

andrewhop Apr 8, 2025

andrewhop Apr 8, 2025

nhatnghiho Apr 9, 2025

andrewhop Apr 22, 2025

andrewhop Apr 8, 2025

skmcgrail Apr 24, 2025

nhatnghiho Apr 30, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

skmcgrail Apr 24, 2025

andrewhop Apr 22, 2025


		self.build_options = []

		# Define CodeBuild Batch job for testing code.

		from datetime import datetime

		# timestamp = datetime.now().strftime('%Y-%m-%d-%H-%M')

Create pre-production stage for CI pipeline #2282

Are you sure you want to change the base?

Create pre-production stage for CI pipeline #2282

Conversation

nhatnghiho commented Mar 20, 2025 • edited Loading

Issues:

Description of changes:

codecov-commenter commented Mar 20, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhatnghiho commented Mar 20, 2025 •

edited

Loading

codecov-commenter commented Mar 20, 2025 •

edited

Loading