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

OIDC thumbprint_list can be retrieved and tags added to OIDC provider #115

Merged
merged 6 commits into from
Jun 4, 2021
Merged

Conversation

nnsense
Copy link
Contributor

@nnsense nnsense commented May 31, 2021

what/why

references

Matteo Migliaccio added 2 commits May 31, 2021 02:40
- With hashicorp/terraform-provider-tls#62 sha1_fingerprint can be retrieved instead of hardcoding it (which isn't safe as not all regions have the same)
@nnsense nnsense requested review from a team as code owners May 31, 2021 02:04
@nnsense nnsense requested a review from a team as a code owner May 31, 2021 02:04
@aknysh
Copy link
Member

aknysh commented May 31, 2021

/rebuild-readme

@aknysh
Copy link
Member

aknysh commented May 31, 2021

/test all

main.tf Outdated
@@ -84,13 +84,17 @@ resource "aws_eks_cluster" "default" {
# https://docs.aws.amazon.com/eks/latest/userguide/enable-iam-roles-for-service-accounts.html
# https://medium.com/@marcincuber/amazon-eks-with-oidc-provider-iam-roles-for-kubernetes-services-accounts-59015d15cb0c
#

data "tls_certificate" "cluster" {
url = join("", aws_eks_cluster.default.*.identity.0.oidc.0.issuer)
Copy link
Member

Choose a reason for hiding this comment

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

please add

count = (local.enabled && var.oidc_provider_enabled) ? 1 : 0

so if the module is disabled, the data sources won't be read

main.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thank you @nnsense, those are very good improvements.
Just a minor nitpick, please see comments

Matteo Migliaccio and others added 3 commits June 1, 2021 10:34
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@nnsense
Copy link
Contributor Author

nnsense commented Jun 1, 2021

Hi @aknysh , I've added the count thanks :)

@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode requested a review from aknysh June 4, 2021 23:29
@nitrocode nitrocode merged commit 361f8a9 into cloudposse:master Jun 4, 2021
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.

4 participants