unfunco / terraform-aws-oidc-github

Terraform module to configure GitHub Actions as an IAM OIDC identity provider in AWS.
https://registry.terraform.io/modules/unfunco/oidc-github/aws/latest
Apache License 2.0
99 stars 54 forks source link

Thumbprint mismatch #33

Closed NicholasFiorentini closed 1 year ago

NicholasFiorentini commented 1 year ago

I'm trying this module, and I see it obtains the thumbprint f879abce0008e4eb126e0097e46620f5aaae26ad instead of 6938fd4d98bab03faadb97b34396831e3780aea1. This value is the one in https://github.blog/changelog/2023-06-27-github-actions-update-on-oidc-integration-with-aws/ and the same value obtained when manually replicating https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc_verify-thumbprint.html.

Do you have any suggestions?

unfunco commented 1 year ago

Hello @NicholasFiorentini – I've not seen that thumbprint before, do you maybe have a proxy between you and AWS? Are you on a corporate network maybe? If you trust the certificate then you can add the thumbprint to the additional_thumbprints variable and that should get it working. If there's no proxy then I'm not sure where the thumbprint is coming from unless there are now three known intermediary certificates.

samsonquantifi commented 1 year ago

we ran into this as well.. It seems that Microsoft/Github made some changes: https://github.blog/changelog/2023-06-27-github-actions-update-on-oidc-integration-with-aws/

We ended up temporarily migrating off this module and fixing the two thumbprints for now.

## set  create_oidc_provider = false
module "aws_oidc_github" {
...
  create_oidc_provider = false
}
## Create a new resource
resource "aws_iam_openid_connect_provider" "github" {
  client_id_list = [
    "https://github.com/your-github-org",
    "sts.amazonaws.com"
  ]

  url = "https://token.actions.githubusercontent.com"
  thumbprint_list = [
    "6938fd4d98bab03faadb97b34396831e3780aea1",
    "1c58a3a8518e8759bf075b76b750d4f2df264fcd"
  ]
}

move state:

 terraform state mv "module.aws_oidc_github.aws_iam_openid_connect_provider.github[0]" aws_iam_openid_connect_provider.github
samsonquantifi commented 1 year ago

You can probably also try and use the additional_thumbprints variable, but we could not because it could collide.

A non-ideal fix in this module might be to uniq the concatentation of found thumbprints with the additiional_thumbprints parameter. However, this still causes a problem as terraform continually will detect a change from the tls_certificate.github datasource.

unfunco commented 1 year ago

Hello @samsonquantifi – There is a fix (#32) to make the thumbprints distinct and this has been released in v1.5.1

Subsequent edit: v1.5.2

samsonquantifi commented 1 year ago

https://github.com/unfunco/terraform-aws-oidc-github/pull/32 - as mentioned in my previous comment - there is still a problem with the distinct fix. Depending on which certificate you happen to hit from Github, the auto-discovered thumbprint is not stable, which causes terraform to detect a change and try to converge. This causes the resource to continue flipping back and forth - especially for a CI like environment where we are running terraform apply constantly

unfunco commented 1 year ago

In GitHub's blog post from yesterday (27 June 2023) they say there are only two known thumbprints at this time, if you're constantly applying then you could add those two thumbprints to the additional_thumbprints variable, then it doesn't matter whether GitHub returns one of them, or both of them, with your additional thumbprints and the distinct clause it should always mean you have the two correct thumbprints.

samsonquantifi commented 1 year ago

You are correct in that if you add the two thumbprints into the additional_thumbprints variable the resulting aws_iam_openid_connect_provider resource will always have two the thumbprints. However the order of the thumbprints is not stable, resulting in terraform detecting a change.

thumbprint_list = var.additional_thumbprints != null ? distinct(
    concat(
      [data.tls_certificate.github.certificates[0].sha1_fingerprint],
      [for thumbprint in var.additional_thumbprints : thumbprint]
    )
# given 
additional_thumbprints = ["6938fd4d98bab03faadb97b34396831e3780aea1","1c58a3a8518e8759bf075b76b750d4f2df264fcd"]

# depending on how which github server you hit, the resulting distinct array is either:
[ "6938fd4d98bab03faadb97b34396831e3780aea1", "1c58a3a8518e8759bf075b76b750d4f2df264fcd" ] 

or 

[ "1c58a3a8518e8759bf075b76b750d4f2df264fcd", "6938fd4d98bab03faadb97b34396831e3780aea1"]

or 

[ "some-other-value", "6938fd4d98bab03faadb97b34396831e3780aea1", "1c58a3a8518e8759bf075b76b750d4f2df264fcd" ] 
NicholasFiorentini commented 1 year ago

Still, my question: Is there no way to automatize this with Terraform?

unfunco commented 1 year ago

@samsonquantifi I'm unable to replicate, but can you try changing distinct to toset in your local copy of the module, if that works I will create a new fix release.

samsonquantifi commented 1 year ago

unfortunately, i can't change the running module on our CI without forking. But actually a toset might work

unfunco commented 1 year ago

Hey @samsonquantifi – Please try v1.5.2 https://github.com/unfunco/terraform-aws-oidc-github/releases/tag/v1.5.2

This changes distinct to toset and also adds the known thumbprints so you don't need to specify them yourself.

perryao commented 1 year ago

fwiw, we upgraded from 0.8.0 to 1.5.2 and had success. Thank you!

unfunco commented 1 year ago

@NicholasFiorentini The known thumbprints are part of the module now so upgrading to 1.5.2 and adding your additional thumbprint (i.e. additional_thumbprints = ["f879abce0008e4eb126e0097e46620f5aaae26ad"]) should hopefully solve your issue.

If there are any new thumbprints published, I'll get them added to the module.

samsonquantifi commented 1 year ago

works for us now. thanks @unfunco

unfunco commented 1 year ago

Closing as this issue has now been resolved.