Skip to content

Conversation

@alaa
Copy link
Contributor

@alaa alaa commented Dec 12, 2019

PR o'clock

Description

Added support to create IAM OpenID Connect Identity Provider to enable EKS Identity Roles for Service Accounts (IRSA).

This should address the issue in TF: hashicorp/terraform-provider-aws#10104

Checklist

@alaa alaa force-pushed the feat/irsa branch 8 times, most recently from 58c30d2 to 3d23f8c Compare December 12, 2019 14:54
irsa.tf Outdated

data "external" "fetch_thumbprint" {
count = var.enable_irsa ? 1 : 0
program = ["sh", "-c", "s_client -connect oidc.eks.${aws_region}.amazonaws.com:443 2>&- -showcerts | awk '/-----BEGIN/{f=\"/tmp/cert.\"(n++)} f{print>f} /-----END/{f=\"\"}' | openssl x509 -noout -fingerprint -in /tmp/cert.3 | cut -d'=' -f2 | tr -d ':' | awk '{print tolower($1)}' | jq -rR 'split(\" \")|{fingerprint:.[0]}'"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is workable. For example many people don't have jq installed. Also won't work on Windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have it already working for 2 clusters and tested it many times. But we use only Linux that's why I did not consider generalizing it to different OS.

If the jq is the only problem, I think I can refactor this to output JSON result without the use of jq. Maybe we could mention on the README that this feature requires Linux OS to run on until the issue is resolved in Terraform or AWS side.

Copy link
Contributor Author

@alaa alaa Dec 13, 2019

Choose a reason for hiding this comment

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

So, It turned out that this Root CA certificate is valid until Jun 28 17:39:16 2034 GMT
and is valid across all regions so passing it as a default value without calculating the certificate fingerprint on the runtime would have a very low security impact. This way there wont be any external dependencies.

Ref:
https://crt.sh/?q=9E99A48A9960B14926BB7F3B02E22DA2B0AB7280

#!/bin/bash

regions=("us-east-2" "us-east-1" "us-west-1" "us-west-2" "ap-east-1" "ap-south-1" "ap-northeast-3" "ap-northeast-2" "ap-southeast-1" "ap-southeast-2" "ap-northeast-1" "ca-central-1" "cn-north-1" "cn-northwest-1" "eu-central-1" "eu-west-1" "eu-west-2" "eu-west-3" "eu-north-1" "me-south-1" "sa-east-1" "us-gov-east-1" "us-gov-west-1")

for region in ${regions[@]};
do
  echo | openssl s_client -connect oidc.eks.$region.amazonaws.com:443 -showcerts  2>&- | keytool -printcert | grep "Certificate fingerprints" -A1  | tail -n1 | cut -d' ' -f3 | tr -d ':' | awk '{print tolower($1)}'
done;`

@max-rocket-internet
Copy link
Contributor

I like the idea. I am also planning on using the new EKS Identity Roles. But does it need to be in the module?

@wbertelsen
Copy link
Contributor

I use the workaround listed here for this
hashicorp/terraform-provider-aws#10104

I create the cluster and then use kubergrunt to manually fill in the thumbprint and then apply that.

resource "aws_iam_openid_connect_provider" "my_cluster_eks_oidc" {
  url = module.my_cluster.cluster_oidc_issuer_url

  client_id_list = [
    "sts.amazonaws.com",
  ]
  # see https://github.com/terraform-providers/terraform-provider-aws/issues/10104
  # kubergrunt eks oidc-thumbprint --issuer-url https://oidc.eks.us-west-2.amazonaws.com/id/yyy
  thumbprint_list = ["xxx"]
}

@alaa
Copy link
Contributor Author

alaa commented Dec 12, 2019

@max-rocket-internet Yeah IRSA is pretty powerful feature that bridges both AWS and Kubernetes worlds and I think enabling it belongs to the EKS module itself. The IAM Role/Policy associated with it can be placed outside the module.

@alaa
Copy link
Contributor Author

alaa commented Dec 16, 2019

@max-rocket-internet If it is eventually decided to split additional features into modules, please just let me know so I can refactor this into a module. Cheers!

Copy link
Contributor

@max-rocket-internet max-rocket-internet left a comment

Choose a reason for hiding this comment

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

OK nice. Thanks @alaa

@max-rocket-internet max-rocket-internet merged commit 583c32d into terraform-aws-modules:master Dec 19, 2019
shaunc pushed a commit to shaunc/terraform-aws-eks that referenced this pull request Dec 19, 2019
@scottybrisbane
Copy link

Hey guys, we're really keen to use this functionality. How close to the next release are we @max-rocket-internet?

@barryib
Copy link
Member

barryib commented Jan 8, 2020

@scottybrisbane There is already the PR #662 for a new release. It's actually on hold for #650 and it's pretty much finish. We'll release soon.

@scottybrisbane
Copy link

Perfect! Thanks @barryib

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants