From c68962aef76ac1d2be20aa0baf6d424aaaacdb41 Mon Sep 17 00:00:00 2001 From: Robert Williams Date: Thu, 2 Jun 2022 17:28:21 +0100 Subject: [PATCH 1/4] Allow namespace creation to be optional It's possible that the user may wish to deploy this to an existing namespace, or the kube-system namespace. That would not be possible, with the current configuration. --- modules/kubernetes-addons/adot-collector-haproxy/main.tf | 2 +- modules/kubernetes-addons/adot-collector-java/main.tf | 2 +- modules/kubernetes-addons/adot-collector-memcached/main.tf | 2 +- modules/kubernetes-addons/adot-collector-nginx/main.tf | 2 +- modules/kubernetes-addons/agones/main.tf | 1 + modules/kubernetes-addons/argo-rollouts/main.tf | 1 + modules/kubernetes-addons/argocd/main.tf | 1 + modules/kubernetes-addons/crossplane/main.tf | 1 + modules/kubernetes-addons/ingress-nginx/main.tf | 1 + modules/kubernetes-addons/kubernetes-dashboard/main.tf | 2 +- modules/kubernetes-addons/metrics-server/main.tf | 2 +- modules/kubernetes-addons/prometheus/main.tf | 1 + modules/kubernetes-addons/spark-k8s-operator/main.tf | 1 + modules/kubernetes-addons/traefik/main.tf | 1 + modules/kubernetes-addons/vpa/main.tf | 1 + modules/kubernetes-addons/yunikorn/main.tf | 1 + 16 files changed, 16 insertions(+), 6 deletions(-) diff --git a/modules/kubernetes-addons/adot-collector-haproxy/main.tf b/modules/kubernetes-addons/adot-collector-haproxy/main.tf index 2a730fc59b..361568f1db 100644 --- a/modules/kubernetes-addons/adot-collector-haproxy/main.tf +++ b/modules/kubernetes-addons/adot-collector-haproxy/main.tf @@ -51,7 +51,7 @@ module "helm_addon" { ] irsa_config = { - create_kubernetes_namespace = true + create_kubernetes_namespace = try(var.helm_config["create_namespace"], true) kubernetes_namespace = local.namespace create_kubernetes_service_account = true kubernetes_service_account = try(var.helm_config.service_account, local.name) diff --git a/modules/kubernetes-addons/adot-collector-java/main.tf b/modules/kubernetes-addons/adot-collector-java/main.tf index b5e9d3bebc..e168d9a792 100644 --- a/modules/kubernetes-addons/adot-collector-java/main.tf +++ b/modules/kubernetes-addons/adot-collector-java/main.tf @@ -51,7 +51,7 @@ module "helm_addon" { ] irsa_config = { - create_kubernetes_namespace = true + create_kubernetes_namespace = try(var.helm_config["create_namespace"], true) kubernetes_namespace = local.namespace create_kubernetes_service_account = true kubernetes_service_account = try(var.helm_config.service_account, local.name) diff --git a/modules/kubernetes-addons/adot-collector-memcached/main.tf b/modules/kubernetes-addons/adot-collector-memcached/main.tf index 4270d08f6d..44fdebb188 100644 --- a/modules/kubernetes-addons/adot-collector-memcached/main.tf +++ b/modules/kubernetes-addons/adot-collector-memcached/main.tf @@ -51,7 +51,7 @@ module "helm_addon" { ] irsa_config = { - create_kubernetes_namespace = true + create_kubernetes_namespace = try(var.helm_config["create_namespace"], true) kubernetes_namespace = local.namespace create_kubernetes_service_account = true kubernetes_service_account = try(var.helm_config.service_account, local.name) diff --git a/modules/kubernetes-addons/adot-collector-nginx/main.tf b/modules/kubernetes-addons/adot-collector-nginx/main.tf index b2f5c6f2b6..d9f1ff62b5 100644 --- a/modules/kubernetes-addons/adot-collector-nginx/main.tf +++ b/modules/kubernetes-addons/adot-collector-nginx/main.tf @@ -51,7 +51,7 @@ module "helm_addon" { ] irsa_config = { - create_kubernetes_namespace = true + create_kubernetes_namespace = try(var.helm_config["create_namespace"], true) kubernetes_namespace = local.namespace create_kubernetes_service_account = true kubernetes_service_account = try(var.helm_config.service_account, local.name) diff --git a/modules/kubernetes-addons/agones/main.tf b/modules/kubernetes-addons/agones/main.tf index 2c9b4ffa82..c38340fdd5 100644 --- a/modules/kubernetes-addons/agones/main.tf +++ b/modules/kubernetes-addons/agones/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/argo-rollouts/main.tf b/modules/kubernetes-addons/argo-rollouts/main.tf index 9688365f70..1ad1291c3d 100644 --- a/modules/kubernetes-addons/argo-rollouts/main.tf +++ b/modules/kubernetes-addons/argo-rollouts/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/argocd/main.tf b/modules/kubernetes-addons/argocd/main.tf index 6164f9b346..fd22d78344 100644 --- a/modules/kubernetes-addons/argocd/main.tf +++ b/modules/kubernetes-addons/argocd/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/crossplane/main.tf b/modules/kubernetes-addons/crossplane/main.tf index e638806d2f..de1354b16e 100644 --- a/modules/kubernetes-addons/crossplane/main.tf +++ b/modules/kubernetes-addons/crossplane/main.tf @@ -1,4 +1,5 @@ resource "kubernetes_namespace_v1" "crossplane" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.namespace } diff --git a/modules/kubernetes-addons/ingress-nginx/main.tf b/modules/kubernetes-addons/ingress-nginx/main.tf index 9fe93f8aaf..f2ab1060b5 100644 --- a/modules/kubernetes-addons/ingress-nginx/main.tf +++ b/modules/kubernetes-addons/ingress-nginx/main.tf @@ -17,6 +17,7 @@ module "helm_addon" { #------------------------------------- resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/kubernetes-dashboard/main.tf b/modules/kubernetes-addons/kubernetes-dashboard/main.tf index c61ed3526b..1e039842d2 100644 --- a/modules/kubernetes-addons/kubernetes-dashboard/main.tf +++ b/modules/kubernetes-addons/kubernetes-dashboard/main.tf @@ -9,7 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { - count = local.helm_config["namespace"] == "kube-system" ? 0 : 1 + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] diff --git a/modules/kubernetes-addons/metrics-server/main.tf b/modules/kubernetes-addons/metrics-server/main.tf index 1522f9e6da..cf8814fc12 100644 --- a/modules/kubernetes-addons/metrics-server/main.tf +++ b/modules/kubernetes-addons/metrics-server/main.tf @@ -9,7 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { - count = local.helm_config["namespace"] == "kube-system" ? 0 : 1 + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] diff --git a/modules/kubernetes-addons/prometheus/main.tf b/modules/kubernetes-addons/prometheus/main.tf index b4ea134dd7..6d59d64861 100644 --- a/modules/kubernetes-addons/prometheus/main.tf +++ b/modules/kubernetes-addons/prometheus/main.tf @@ -61,6 +61,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "prometheus" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = try(var.helm_config.namespace, "prometheus") } diff --git a/modules/kubernetes-addons/spark-k8s-operator/main.tf b/modules/kubernetes-addons/spark-k8s-operator/main.tf index 9688365f70..1ad1291c3d 100644 --- a/modules/kubernetes-addons/spark-k8s-operator/main.tf +++ b/modules/kubernetes-addons/spark-k8s-operator/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/traefik/main.tf b/modules/kubernetes-addons/traefik/main.tf index 9688365f70..1ad1291c3d 100644 --- a/modules/kubernetes-addons/traefik/main.tf +++ b/modules/kubernetes-addons/traefik/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "this" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/vpa/main.tf b/modules/kubernetes-addons/vpa/main.tf index 9fee99f6de..828758a8df 100644 --- a/modules/kubernetes-addons/vpa/main.tf +++ b/modules/kubernetes-addons/vpa/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "vpa" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } diff --git a/modules/kubernetes-addons/yunikorn/main.tf b/modules/kubernetes-addons/yunikorn/main.tf index e0f561e4bc..21e921b3de 100644 --- a/modules/kubernetes-addons/yunikorn/main.tf +++ b/modules/kubernetes-addons/yunikorn/main.tf @@ -9,6 +9,7 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "yunikorn" { + count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 metadata { name = local.helm_config["namespace"] } From b6861fa851aeeb6f0493d0ac4502522a3c5b7ad7 Mon Sep 17 00:00:00 2001 From: Robert Williams Date: Fri, 3 Jun 2022 20:03:57 +0100 Subject: [PATCH 2/4] IRSA module created namespaces should be optional Some of the addon modules create namespaces via the IRSA module. These should also be optional. --- modules/kubernetes-addons/aws-cloudwatch-metrics/locals.tf | 2 +- modules/kubernetes-addons/aws-for-fluentbit/locals.tf | 2 +- modules/kubernetes-addons/aws-privateca-issuer/locals.tf | 4 ++-- modules/kubernetes-addons/external-dns/locals.tf | 2 +- modules/kubernetes-addons/karpenter/locals.tf | 2 +- modules/kubernetes-addons/keda/locals.tf | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/kubernetes-addons/aws-cloudwatch-metrics/locals.tf b/modules/kubernetes-addons/aws-cloudwatch-metrics/locals.tf index cd8dc2b16d..9a54a4b197 100644 --- a/modules/kubernetes-addons/aws-cloudwatch-metrics/locals.tf +++ b/modules/kubernetes-addons/aws-cloudwatch-metrics/locals.tf @@ -36,7 +36,7 @@ locals { irsa_config = { kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = true + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) create_kubernetes_service_account = true irsa_iam_policies = concat(["arn:${var.addon_context.aws_partition_id}:iam::aws:policy/CloudWatchAgentServerPolicy"], var.irsa_policies) } diff --git a/modules/kubernetes-addons/aws-for-fluentbit/locals.tf b/modules/kubernetes-addons/aws-for-fluentbit/locals.tf index 280505bb90..3195eac65c 100644 --- a/modules/kubernetes-addons/aws-for-fluentbit/locals.tf +++ b/modules/kubernetes-addons/aws-for-fluentbit/locals.tf @@ -44,7 +44,7 @@ locals { irsa_config = { kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = true + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) create_kubernetes_service_account = true irsa_iam_policies = concat([aws_iam_policy.aws_for_fluent_bit.arn], var.irsa_policies) } diff --git a/modules/kubernetes-addons/aws-privateca-issuer/locals.tf b/modules/kubernetes-addons/aws-privateca-issuer/locals.tf index 6da0dd6fe3..9ac51ad3e9 100644 --- a/modules/kubernetes-addons/aws-privateca-issuer/locals.tf +++ b/modules/kubernetes-addons/aws-privateca-issuer/locals.tf @@ -32,8 +32,8 @@ locals { ] irsa_config = { - create_kubernetes_namespace = true - kubernetes_namespace = local.name + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) + kubernetes_namespace = local.helm_config["namespace"] create_kubernetes_service_account = true kubernetes_service_account = local.service_account_name irsa_iam_policies = concat([aws_iam_policy.aws_privateca_issuer.arn], var.irsa_policies) diff --git a/modules/kubernetes-addons/external-dns/locals.tf b/modules/kubernetes-addons/external-dns/locals.tf index 5ed9428091..61933b91f6 100644 --- a/modules/kubernetes-addons/external-dns/locals.tf +++ b/modules/kubernetes-addons/external-dns/locals.tf @@ -37,7 +37,7 @@ locals { irsa_config = { kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = true + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) create_kubernetes_service_account = true irsa_iam_policies = concat([aws_iam_policy.external_dns.arn], var.irsa_policies) } diff --git a/modules/kubernetes-addons/karpenter/locals.tf b/modules/kubernetes-addons/karpenter/locals.tf index 0464c56c5b..dd3efc2ed4 100644 --- a/modules/kubernetes-addons/karpenter/locals.tf +++ b/modules/kubernetes-addons/karpenter/locals.tf @@ -33,7 +33,7 @@ locals { irsa_config = { kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = true + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) create_kubernetes_service_account = true irsa_iam_policies = concat([aws_iam_policy.karpenter.arn], var.irsa_policies) } diff --git a/modules/kubernetes-addons/keda/locals.tf b/modules/kubernetes-addons/keda/locals.tf index 5d84ade160..26d8786ce2 100644 --- a/modules/kubernetes-addons/keda/locals.tf +++ b/modules/kubernetes-addons/keda/locals.tf @@ -33,7 +33,7 @@ locals { irsa_config = { kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = true + create_kubernetes_namespace = try(local.helm_config["create_namespace"], true) create_kubernetes_service_account = true irsa_iam_policies = concat([aws_iam_policy.keda_irsa.arn], var.irsa_policies) } From 3f49a9eee504083db03750ec30391b59fe872dbb Mon Sep 17 00:00:00 2001 From: Robert Williams Date: Fri, 3 Jun 2022 20:13:05 +0100 Subject: [PATCH 3/4] Ensure namespace is passed correctly to IRSA Some of the modules use the IRSA module, but don't create the namespace via that module. They need to pass the value set in the helm_config, so as to ensure both ServiceAccount and Helm chart are created in the same namespace. --- modules/kubernetes-addons/aws-efs-csi-driver/locals.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/kubernetes-addons/aws-efs-csi-driver/locals.tf b/modules/kubernetes-addons/aws-efs-csi-driver/locals.tf index 82e1482d1d..41d27e764e 100644 --- a/modules/kubernetes-addons/aws-efs-csi-driver/locals.tf +++ b/modules/kubernetes-addons/aws-efs-csi-driver/locals.tf @@ -41,9 +41,9 @@ locals { ] irsa_config = { - kubernetes_namespace = local.namespace + kubernetes_namespace = local.helm_config["namespace"] kubernetes_service_account = local.service_account_name - create_kubernetes_namespace = false + create_kubernetes_namespace = try(var.helm_config.create_namespace, false) create_kubernetes_service_account = true iam_role_path = "/" eks_cluster_id = var.addon_context.eks_cluster_id From 7fe5962ed902b1648e8ff80158823c87e2646419 Mon Sep 17 00:00:00 2001 From: Bryant Biggs Date: Thu, 30 Jun 2022 17:53:56 -0400 Subject: [PATCH 4/4] fix: Correct for CI checks --- .pre-commit-config.yaml | 2 +- modules/kubernetes-addons/prometheus/main.tf | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 56313c6b23..df04d78cbe 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,7 +10,7 @@ repos: - id: detect-aws-credentials args: ['--allow-missing-credentials'] - repo: https://github.com/antonbabenko/pre-commit-terraform - rev: v1.72.2 + rev: v1.73.0 hooks: - id: terraform_fmt - id: terraform_docs diff --git a/modules/kubernetes-addons/prometheus/main.tf b/modules/kubernetes-addons/prometheus/main.tf index 7d858c8e88..1d4857ecce 100644 --- a/modules/kubernetes-addons/prometheus/main.tf +++ b/modules/kubernetes-addons/prometheus/main.tf @@ -1,6 +1,11 @@ locals { - name = try(var.helm_config.name, "prometheus") - namespace = kubernetes_namespace_v1.prometheus.metadata[0].name + name = try(var.helm_config.name, "prometheus") + namespace_name = try(var.helm_config.namespace, "prometheus") + create_namespace = try(var.helm_config.create_namespace, true) && local.namespace_name != "kube-system" + + # `namespace_name` is just the string representation of the namespace name + # `namespace` is the name of the resultant namespace to use - created or not + namespace = local.create_namespace ? kubernetes_namespace_v1.prometheus[0].metadata[0].name : local.namespace_name workspace_url = var.amazon_prometheus_workspace_endpoint != null ? "${var.amazon_prometheus_workspace_endpoint}api/v1/remote_write" : "" ingest_service_account = "amp-ingest" @@ -24,7 +29,7 @@ module "helm_addon" { chart = local.name version = "15.10.1" repository = "https://prometheus-community.github.io/helm-charts" - namespace = local.namespace + namespace = local.namespace_name description = "Prometheus helm Chart deployment configuration" values = [templatefile("${path.module}/values.yaml", { operating_system = try(var.helm_config.operating_system, "linux") @@ -61,9 +66,10 @@ module "helm_addon" { } resource "kubernetes_namespace_v1" "prometheus" { - count = try(local.helm_config["create_namespace"], true) && local.helm_config["namespace"] != "kube-system" ? 1 : 0 + count = local.create_namespace ? 1 : 0 + metadata { - name = try(var.helm_config.namespace, "prometheus") + name = local.namespace_name } }