-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: use correct IAM role for TF apply #694
Conversation
Update the Terraform apply workflow to use the OIDC role that can be assumed by workflows running on the `develop` branch. Update the Terraform plan workflow to always build and test all lambdas.
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.
LGTM!
@@ -68,7 +68,7 @@ jobs: | |||
- name: Configure AWS credentials using OIDC | |||
uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 # v4.0.2 | |||
with: | |||
role-to-assume: arn:aws:iam::${{ env.AWS_ACCOUNT_ID }}:role/forms-terraform-apply-release |
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.
So, what is going to happen to the -release
role? Will it still be used somewhere? How come we did not have a similar issue in staging?
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.
Once the 3.10.1
release goes in, I'll get a fix in to remove the -release
role since we won't need it anymore. We didn't have this problem in Staging because its TF apply workflow has always executed on pushes to develop
, which is what the default SRE OIDC -apply
roles are setup to work with.
We needed the -release
role for Prod because it used to execute against a tag reference. However, with the latest workflow change to support easier revert PRs, it now executes against develop
as well.
runs-on: ubuntu-latest | ||
env: | ||
VERSION: ${{ needs.get-version.outputs.version }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
image: ${{ fromJSON(needs.detect-lambda-changes.outputs.lambda-to-rebuild) }} | ||
image: [audit-logs, audit-logs-archiver, cognito-email-sender, cognito-pre-sign-up, form-archiver, nagware, notify-slack, reliability, reliability-dlq-consumer, response-archiver, submission, vault-integrity] |
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've seen this before. Was it done for staging but not for production?
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.
No, it was done for the TF apply prod workflow because of the same bug. It's on our list to generate this list dynamically rather than hardcoding it in both workflows.
Summary
Update the Terraform apply workflow to use the OIDC role that can be assumed by workflows running on the
develop
branch.Update the Terraform plan workflow to always build and test all lambdas.
Related