Skip to content

Remove OIDC IAM role secret #240

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

Merged
merged 4 commits into from
Apr 8, 2025
Merged

Remove OIDC IAM role secret #240

merged 4 commits into from
Apr 8, 2025

Conversation

ben-harvey
Copy link
Contributor

This change removes references to an OIDC IAM role secret and uses the role directly instead. When we wrote this module, we thought it was a security best practice to obfuscate the AWS account number and role name for OIDC. However, the ARN is just an identifier for the IAM role that a GitHub Actions workflow will assume. Unlike access keys, the role ARN doesn't grant access to AWS resources. Permissions on the role prevent a workflow in another repo from assuming the role even if they know the ARN

It's now our standard practice to include the ARN in workflows to avoid the maintenance of an additional secret. This change updates workflow references as well as our recommendations in the OIDC templates

@@ -11,7 +11,6 @@ As a part of your GitHub actions workflows, you may need access to internal CMS
This procedure assumes that you have already taken the steps documented in the main README. You have:

- Instantiated the requisite infrastructure in AWS ECR and ECS
- Instantiated the resources for the GitHub OIDC provider and stored the OIDC role ARN in a GitHub secret called `OIDC_IAM_ROLE_ARN`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the first part of this sentence still true:?
Instantiated the resources for the GitHub OIDC provider

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure is, good catch!

Copy link
Contributor

@leslie-corbalt leslie-corbalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one comment.

Copy link
Contributor

@leslie-corbalt leslie-corbalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ben-harvey ben-harvey merged commit a809b42 into main Apr 8, 2025
13 checks passed
@ben-harvey ben-harvey deleted the bharvey-iam-role branch April 8, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants