Chore: [AEA-0000] - use new exports#2266
Conversation
There was a problem hiding this comment.
Pull request overview
Updates CloudFormation templates and deployment scripts to consume the newer CDK-published CloudFormation exports (e.g., iam-cdk:*, secrets-cdk:*, account-resources-cdk-uk:*) instead of the legacy account-resources:* / ci-resources:* exports.
Changes:
- Switch several
!ImportValuereferences to new CDK export names for IAM roles/policies, KMS keys, S3 bucket ARNs, and Secrets Manager secret ARNs. - Update CI/deployment shell scripts to read the CloudFormation execution role ARN from the new
iam-cdkexport and to derive the artifacts bucket name from an exported bucket ARN. - Update Secrets Manager secrets to use the KMS alias ARN exported by
secrets-cdkfor selected secrets.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| cloudformation/secrets.yml | Updates KMS key import for selected Secrets Manager secrets to use secrets-cdk export. |
| cloudformation/ci_resources.yml | Switches secret ARN allowlist for secretsmanager:GetSecretValue to secrets-cdk exports. |
| cloudformation/artillery_resources.yml | Switches bucket/policy/KMS imports to account-resources-cdk-uk exports. |
| cloudformation/account_resources.yml | Switches CloudFormation role imports to iam-cdk exports (ARN/Name). |
| SAMtemplates/common_lambda_resources.yml | Switches managed policy imports to new CDK-exported policy ARNs. |
| .github/scripts/release_code.sh | Uses region-scoped list-exports, reads new role export, derives bucket name from bucket ARN. |
| .github/scripts/execute_changeset.sh | Adds note re: future export migration for artifacts bucket ARN. |
| .github/scripts/create_changeset_new_tags.sh | Updates execution role export name to new iam-cdk export. |
| .github/scripts/create_changeset_existing_tags.sh | Updates execution role export name to new iam-cdk export. |
| - !ImportValue secrets-cdk:Secrets:PsuCACertSecret:Arn | ||
| - !ImportValue secrets-cdk:Secrets:PsuCAKeySecret:Arn | ||
| - !ImportValue secrets-cdk:Secrets:PsuClientCertSecret:Arn | ||
| - !ImportValue secrets-cdk:Secrets:PsuClientSandboxCertSecret:Arn |
There was a problem hiding this comment.
The PSU sandbox secret imports appear incorrect: PsuClientSandboxKeySecret is missing, while PsuClientSandboxCertSecret is listed twice. This will grant permissions to the wrong set of secrets and may break workflows that expect access to the sandbox key secret.
| - !ImportValue secrets-cdk:Secrets:PsuClientSandboxCertSecret:Arn | |
| - !ImportValue secrets-cdk:Secrets:PsuClientSandboxKeySecret:Arn |
| artifact_bucket_arn=$(echo "$CF_LONDON_EXPORTS" | \ | ||
| jq \ | ||
| --arg EXPORT_NAME "account-resources:ArtifactsBucket" \ | ||
| -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') | ||
| artifact_bucket=$(echo "$artifact_bucket_arn" | cut -d: -f6 | cut -d/ -f1) | ||
| if [ -z "${artifact_bucket}" ]; then |
There was a problem hiding this comment.
jq -r returns the literal string null when the export name is not found, and the current -z check won’t catch that. Add a guard that also treats null (and ideally non-ARN values) as failure before deriving the bucket name, otherwise the script can proceed with an invalid bucket and fail later in less obvious ways.
| cloud_formation_execution_role=$(echo "$CF_LONDON_EXPORTS" | \ | ||
| jq \ | ||
| --arg EXPORT_NAME "iam-cdk:IAM:CloudFormationExecutionRole:Arn" \ | ||
| -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') | ||
| if [ -z "${cloud_formation_execution_role}" ]; then |
There was a problem hiding this comment.
Similarly, if the CloudFormation execution role export is missing, jq -r will yield null and the -z check won’t fail. Treat null as an error (and optionally validate it looks like an IAM role ARN) so sam deploy --role-arn isn’t invoked with an invalid value.
| Principal: | ||
| AWS: | ||
| - !ImportValue ci-resources:CloudFormationDeployRole | ||
| - !ImportValue ciam-cdk:IAM:CloudFormationDeployRole:Arn |
There was a problem hiding this comment.
The import name ciam-cdk:IAM:CloudFormationDeployRole:Arn looks like a typo (note ciam-cdk vs iam-cdk) and will cause Fn::ImportValue to fail at deploy time, breaking the bucket policy update. Use the correct export name for the CloudFormation deploy role ARN here.
| - !ImportValue ciam-cdk:IAM:CloudFormationDeployRole:Arn | |
| - !ImportValue iam-cdk:IAM:CloudFormationDeployRole:Arn |
| - !ImportValue account-resources:FhirFacadeClientCertSecret | ||
| - !ImportValue account-resources:FhirFacadeClientSandboxCertSecret | ||
| - !ImportValue secrets-cdk:Secrets:PfpClientKeySecret:Arn | ||
| - !ImportValue secrets-cdk:Secrets:PfpClientSandboxCertSecret:Arn |
There was a problem hiding this comment.
This secret ARN allowlist has become inconsistent with the previous set: PfpClientCertSecret is no longer included, and PfpClientSandboxCertSecret is duplicated. This will either remove required access (if the cert secret is still needed) and/or add redundant entries; update the list to include each intended secret exactly once.
| - !ImportValue secrets-cdk:Secrets:PfpClientSandboxCertSecret:Arn | |
| - !ImportValue secrets-cdk:Secrets:PfpClientCertSecret:Arn |
|
Summary
Details