Skip to content
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

use dynamic data pull for GHA cert thumbprint #70

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

tylerthome
Copy link
Member

@tylerthome tylerthome commented Sep 5, 2024

What changes did you make?

  • Add a data block for the GitHub Actions token issuer's TLS certificate

Rationale behind the changes?

  • Avoid hard-coding cert thumbprint in shared module, as this is subject to change

Testing done for these changes

  • None: existing implementation encounters an IAM error, suggesting this role assignment is no longer valid for the integration

What did you learn or can share that is new?(optional)

N/A

Notes

Copy link

github-actions bot commented Sep 5, 2024

Terraform plan in terraform
With backend config files: terraform/prod.backend.tfvars

Plan: 0 to add, 1 to change, 0 to destroy.
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~  update in-place

Terraform will perform the following actions:

  # module.iam_oidc_gha_incubator.aws_iam_openid_connect_provider.github_actions will be updated in-place
!~  resource "aws_iam_openid_connect_provider" "github_actions" {
        id              = "arn:aws:iam::035866691871:oidc-provider/token.actions.githubusercontent.com"
        tags            = {}
!~      thumbprint_list = [
!~          "1b511abead59c6ce207077c0bf0e0043b1382612" -> "d89e3bd43d5d909b47a18977aa9d5ce36cee184c",
        ]
#        (4 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

❌ Error applying plan in Apply Terraform changes on merge #17

Copy link

@alex-english-elvt alex-english-elvt left a comment

Choose a reason for hiding this comment

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

@tylerthome this is a great approach, but according to the plan it does change the thumbrint of the cert used for oidc. Is that intentional?

Looks like github may have eaten the link to the blog post you provided - can you add back in? And for the IAM error, assuming this is what you are referring to?

@tylerthome
Copy link
Member Author

@tylerthome this is a great approach, but according to the plan it does change the thumbrint of the cert used for oidc. Is that intentional?

Yes, this is expected. I saw that the GitHub team was posting these on occasion when they cycled the cert, like here but haven't seen any recent updates. The initial hard-coded thumbprint in this module came from a GHA or AWS doc about the OIDC mechanism, where it seemed like a semi-fixed value -- I'll update if I can find that one again just for posterity.

Looks like github may have eaten the link to the blog post you provided - can you add back in?

Thanks for catching that, added to the original comment again.

And for the IAM error, assuming this is what you are referring to?

Yes that's the one - not too certain this will resolve that since I hadn't set TF_LOG high enough to see what actually threw the auth there (assuming there was more info to log), but seemed like a better approach in either case. AWS docs use some openssl scripting to directly extract this thumbprint, hoping the terraform module is equivalent

@ale210
Copy link
Contributor

ale210 commented Sep 5, 2024

Thanks tyler - according to that article we can prob remove the thumbprint entirely, if I'm interpreting that correctly? If you'd like to keep it for now that's ok with me.

@tylerthome
Copy link
Member Author

Thanks! Will plan to follow this up to see if we can remove the thumbprint, as soon as I have a moment to test that in another env

@tylerthome tylerthome removed the request for review from chelseybeck September 5, 2024 16:51
@tylerthome tylerthome merged commit 58ba8b1 into main Sep 5, 2024
1 check passed
@tylerthome tylerthome deleted the iam/oidc-add-gha-thumbprint branch September 5, 2024 16:52
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.

3 participants