Kubernetes modules hard coding of service account names leads to max char errors for IAM roles

[nitrocode]

Description

Please provide a clear and concise description of the issue you are encountering, and a reproduction of your configuration (see the examples/* directory for references that you can copy+paste and tailor to match your configs if you are unable to copy your exact configuration). The reproduction MUST be executable by running terraform init && terraform apply without any further changes.

If your request is for a new feature, please use the Feature request template.

• ✋ I have searched the open/closed issues and my issue is not listed.

⚠️ Note

Versions

• Module version [Required]: v4.16.0
• Terraform version: 1.3.4
• Provider version(s):

✗ terraform version
Alias tip: tf version
Terraform v1.3.4
on darwin_arm64
+ provider registry.terraform.io/gavinbunney/kubectl v1.14.0
+ provider registry.terraform.io/hashicorp/aws v4.39.0
+ provider registry.terraform.io/hashicorp/cloudinit v2.2.0
+ provider registry.terraform.io/hashicorp/helm v2.7.1
+ provider registry.terraform.io/hashicorp/kubernetes v2.15.0
+ provider registry.terraform.io/hashicorp/null v3.2.0
+ provider registry.terraform.io/hashicorp/random v3.4.3
+ provider registry.terraform.io/hashicorp/time v0.9.1
+ provider registry.terraform.io/hashicorp/tls v4.0.4

Reproduction Code [Required]

Steps to reproduce the behavior:

module "addons" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/kubernetes-addons?ref=v4.16.0"

  eks_cluster_id       = module.eks.cluster_id         # 30 char string name
  eks_cluster_endpoint = module.eks.cluster_endpoint
  eks_cluster_version  = module.eks.cluster_version
  eks_oidc_provider    = module.eks.oidc_provider

  enable_aws_load_balancer_controller      = true
  aws_load_balancer_controller_helm_config = {}
}

Expected behaviour

Successful terraform apply

Actual behaviour

Error: expected length of name to be in the range (1 - 64), got <cluster-name>-aws-load-balancer-controller-sa-irsa

  with module.addons.module.aws_load_balancer_controller[0].module.helm_addon.module.irsa[0].aws_iam_role.irsa[0],
  on .terraform/modules/addons/modules/irsa/main.tf line 38, in resource "aws_iam_role" "irsa":
  38:   name        = try(coalesce(var.irsa_iam_role_name, format("%s-%s-%s", var.eks_cluster_id, trim(var.kubernetes_service_account, "-*"), "irsa")), null)

Terminal Output Screenshot(s)

N/A

Additional context

I believe the bug #333 is still present for the aws-load-balancer-controller.

It's failing for the irsa module's aws_iam_role which is controlled by var.irsa_iam_role_name.

terraform-aws-eks-blueprints/modules/irsa/main.tf

Lines 16 to 19 in f233a46

	resource "kubernetes_service_account_v1" "irsa" {
	count = var.create_kubernetes_service_account ? 1 : 0
	metadata {
	name = var.kubernetes_service_account

terraform-aws-eks-blueprints/modules/irsa/main.tf

Lines 35 to 38 in f233a46

	resource "aws_iam_role" "irsa" {
	count = var.irsa_iam_policies != null ? 1 : 0
	name = try(coalesce(var.irsa_iam_role_name, format("%s-%s-%s", var.eks_cluster_id, trim(var.kubernetes_service_account, "-*"), "irsa")), null)

The irsa module is consumed by the helm_addon module

terraform-aws-eks-blueprints/modules/kubernetes-addons/helm-addon/main.tf

Line 63 in f233a46

module "irsa" {

terraform-aws-eks-blueprints/modules/kubernetes-addons/helm-addon/main.tf

Line 71 in f233a46

kubernetes_service_account = lookup(var.irsa_config, "kubernetes_service_account", "")

terraform-aws-eks-blueprints/modules/kubernetes-addons/helm-addon/main.tf

Line 74 in f233a46

irsa_iam_role_name = var.irsa_iam_role_name

The helm addon does not expose the irsa_iam_role_name input

terraform-aws-eks-blueprints/modules/kubernetes-addons/aws-load-balancer-controller/main.tf

Lines 1 to 8 in f233a46

	module "helm_addon" {
	source = "../helm-addon"
	manage_via_gitops = var.manage_via_gitops
	set_values = local.set_values
	helm_config = local.helm_config
	irsa_config = local.irsa_config
	addon_context = var.addon_context
	}

The local irsa_config has a hard coded kubernetes_service_account

terraform-aws-eks-blueprints/modules/kubernetes-addons/aws-load-balancer-controller/locals.tf

Lines 2 to 3 in f233a46

	name = "aws-load-balancer-controller"
	service_account_name = "${local.name}-sa"

terraform-aws-eks-blueprints/modules/kubernetes-addons/aws-load-balancer-controller/locals.tf

Lines 46 to 52 in f233a46

	irsa_config = {
	kubernetes_namespace = local.helm_config["namespace"]
	kubernetes_service_account = local.service_account_name
	create_kubernetes_namespace = try(local.helm_config["create_namespace"], true)
	create_kubernetes_service_account = true
	irsa_iam_policies = [aws_iam_policy.aws_load_balancer_controller.arn]
	}

terraform-aws-eks-blueprints/modules/kubernetes-addons/main.tf

Lines 180 to 186 in f233a46

	module "aws_load_balancer_controller" {
	count = var.enable_aws_load_balancer_controller ? 1 : 0
	source = "./aws-load-balancer-controller"
	helm_config = var.aws_load_balancer_controller_helm_config
	manage_via_gitops = var.argocd_manage_add_ons
	addon_context = merge(local.addon_context, { default_repository = local.amazon_container_image_registry_uris[data.aws_region.current.name] })
	}

What's also interesting is how the aws_iam_policy uses a shorter name to most likely avoid the same max char issue that the aws_iam_role role hits.

terraform-aws-eks-blueprints/modules/kubernetes-addons/aws-load-balancer-controller/main.tf

Lines 10 to 11 in f233a46

	resource "aws_iam_policy" "aws_load_balancer_controller" {
	name = "${var.addon_context.eks_cluster_id}-lb-irsa"

Options

So the only way I can currently solve without forking is to use a smaller cluster name (cluster name cannot exceed 27 chars) but I'd rather not do this.

Option 1: allow overriding local.name or local.service_account_name with helm_config

We could do this var.helm_config using something like helm_config["name"] or helm_config["service_account_name"] or both

# modules/kubernetes-addons/aws-load-balancer-controller/locals.tf
locals {
  name                 = try(local.helm_config["name"], "aws-load-balancer-controller")
  service_account_name = try(local.helm_config["service_account_name"], "${local.name}-sa")

and then we can do this

module "addons" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/kubernetes-addons?ref=v4.16.0"

  # ...

  enable_aws_load_balancer_controller = true

  aws_load_balancer_controller_helm_config = {
    # either of the below would avoid the 64 char max limit
    name = "aws-lb"

    service_account_name = "aws-lb-sa"
  }
}

Option 2: allow overriding the irsa_config

We do this already with local.helm_config which is a merge of local.default_helm_config and var.helm_config.

terraform-aws-eks-blueprints/modules/kubernetes-addons/aws-load-balancer-controller/locals.tf

Lines 16 to 25 in f233a46

	helm_config = merge(
	local.default_helm_config,
	var.helm_config
	)
	default_helm_values = [templatefile("${path.module}/values.yaml", {
	aws_region = var.addon_context.aws_region_name,
	eks_cluster_id = var.addon_context.eks_cluster_id,
	repository = "${var.addon_context.default_repository}/amazon/aws-load-balancer-controller"
	})]

We could do the same with the local.irsa_config but then we would have to expose a new input variable across all submodules e.g. aws_load_balancer_controller_irsa_config

After some thought on this, it's probably not the best idea since the local.service_account_name is used in many places so overriding the local.irsa_config would not impact the other places.

Thoughts

I think Option 1 would be easier.

I also think that doing a validation for the IAM role name to ensure it's not over 64 chars during plan-time would be good too. That way we do not have to wait until we deploy our cluster to know that we have to shorten the IAM role name.

The error could be as simple as this where the error is thrown if the length of a string is fed as the index of the array.

> range(0, 65)[64]
64
> range(0, 65)[67]
╷
│ Error: Invalid index

Edit: on second thought, the.above check is covered by tflint and its aws plugin.

[nitrocode]

@bryantbiggs can we keep this open until I can resolve it across all modules ?

ref: #1184 (comment)

[bryantbiggs]

closed in #1193