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

IRSA module refactor (No Breaking changes) #832

Merged
merged 11 commits into from Aug 9, 2022
Merged

IRSA module refactor (No Breaking changes) #832

merged 11 commits into from Aug 9, 2022

Conversation

vara-bonthu
Copy link
Contributor

@vara-bonthu vara-bonthu commented Aug 9, 2022

What does this PR do?

This PR is raised to simplify the IRSA module variables. No breaking changes are introduced by this PR to EKS blueprints.
However, users who are calling IRSA module directly needs a simple change to your code. Check the AFTER code snippet. FAQ.md also provides these details

Before

This is the current usage of consuming IRSA module directly.

module "irsa" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/irsa"
  kubernetes_namespace       = "dev"
  kubernetes_service_account = "spark"
  irsa_iam_policies          = [aws_iam_policy.spark.arn]
  addon_context = {
    eks_cluster_id                = var.eks_cluster_id
    eks_oidc_provider_arn         = "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:oidc-provider/${local.eks_oidc_issuer_url}"
    eks_oidc_issuer_url           = local.eks_oidc_issuer_url
    irsa_iam_role_path            = "/"
    irsa_iam_permissions_boundary = ""
    aws_caller_identity_account_id = ""
    aws_caller_identity_arn = ""
    aws_eks_cluster_endpoint = ""
    aws_partition_id = ""
    aws_region_name = ""
    tags = {}
  }
}

After

module "irsa" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/irsa"
  kubernetes_namespace       = "dev"
  kubernetes_service_account = "spark"
  irsa_iam_policies          = [aws_iam_policy.spark.arn]
  eks_cluster_id             = module.eks_blueprints.eks_cluster_id
  eks_oidc_provider_arn      = module.eks_blueprints.eks_oidc_provider_arn
}

Related Issues
closes #697
closes #814

Motivation

  • To simplify user experience to consume IRSA module directly.
  • There are number of instances where users want to create IRSA for specific service account

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@vara-bonthu vara-bonthu temporarily deployed to EKS Blueprints Test August 9, 2022 11:12 Inactive
modules/irsa/main.tf Outdated Show resolved Hide resolved
Copy link
Contributor

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

Looks great!

Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
@vara-bonthu vara-bonthu temporarily deployed to EKS Blueprints Test August 9, 2022 13:48 Inactive
@vara-bonthu vara-bonthu temporarily deployed to EKS Blueprints Test August 9, 2022 15:03 Inactive
@bryantbiggs bryantbiggs temporarily deployed to EKS Blueprints Test August 9, 2022 19:48 Inactive
@bryantbiggs
Copy link
Contributor

plan-examples / Plan examples (examples/external-secrets-kubernetes-addon) (pull_request_target) Failing after 8s — Plan examples (examples/external-secrets-kubernetes-addon) will fail due to name change, ok to ignore

@bryantbiggs bryantbiggs merged commit fb2346f into main Aug 9, 2022
@bryantbiggs bryantbiggs deleted the irsa branch August 9, 2022 22:12
@@ -1,3 +1,7 @@
locals {
eks_oidc_issuer_url = "${replace(var.eks_oidc_provider_arn, "/^(.*provider/)/", "")}:sub"
Copy link
Contributor

Choose a reason for hiding this comment

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

@vara-bonthu should this have :sub ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is breaking things, I believe. Look at #837

allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Dec 15, 2022
Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
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.

[FEATURE] - Simplify usage of kubernetes-addons and IRSA modules
4 participants