From 2316e7775ae0322ff72650caca66d6d34a92ec02 Mon Sep 17 00:00:00 2001 From: Bharath KKB Date: Tue, 3 May 2022 14:13:14 -0500 Subject: [PATCH] fix: set only one of log/mon config or service (#1240) * fix: set only one of log/mon config or service * regen * add test * remove for autopilot --- autogen/main/cluster.tf.tmpl | 6 +-- autogen/main/main.tf.tmpl | 1 + examples/simple_regional_beta/main.tf | 50 ++++++++++--------- .../cluster.tf | 6 +-- .../main.tf | 1 + modules/beta-private-cluster/cluster.tf | 6 +-- modules/beta-private-cluster/main.tf | 1 + .../cluster.tf | 6 +-- .../main.tf | 1 + modules/beta-public-cluster/cluster.tf | 6 +-- modules/beta-public-cluster/main.tf | 1 + .../beta_cluster/controls/gcloud.rb | 13 +++++ 12 files changed, 59 insertions(+), 39 deletions(-) diff --git a/autogen/main/cluster.tf.tmpl b/autogen/main/cluster.tf.tmpl index 0953ca827..ca4abd0a6 100644 --- a/autogen/main/cluster.tf.tmpl +++ b/autogen/main/cluster.tf.tmpl @@ -78,7 +78,8 @@ resource "google_container_cluster" "primary" { type = var.cluster_telemetry_type } } - logging_service = local.cluster_telemetry_type_is_set ? null : var.logging_service + # only one of logging/monitoring_service or logging/monitoring_config can be specified + logging_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.logging_service dynamic "logging_config" { for_each = length(var.logging_enabled_components) > 0 ? [1] : [] @@ -86,8 +87,7 @@ resource "google_container_cluster" "primary" { enable_components = var.logging_enabled_components } } - - monitoring_service = local.cluster_telemetry_type_is_set ? null : var.monitoring_service + monitoring_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.monitoring_service dynamic "monitoring_config" { for_each = length(var.monitoring_enabled_components) > 0 ? [1] : [] diff --git a/autogen/main/main.tf.tmpl b/autogen/main/main.tf.tmpl index a9ba1803e..e1b848ca5 100644 --- a/autogen/main/main.tf.tmpl +++ b/autogen/main/main.tf.tmpl @@ -108,6 +108,7 @@ locals { ] : [] cluster_cloudrun_enabled = var.cloudrun cluster_gce_pd_csi_config = var.gce_pd_csi_driver ? [{ enabled = true }] : [{ enabled = false }] + logmon_config_is_set = length(var.logging_enabled_components) > 0 || length(var.monitoring_enabled_components) > 0 {% endif %} cluster_authenticator_security_group = var.authenticator_security_group == null ? [] : [{ diff --git a/examples/simple_regional_beta/main.tf b/examples/simple_regional_beta/main.tf index 9cc4425ef..3c1686894 100644 --- a/examples/simple_regional_beta/main.tf +++ b/examples/simple_regional_beta/main.tf @@ -27,30 +27,32 @@ provider "kubernetes" { } module "gke" { - source = "../../modules/beta-public-cluster/" - project_id = var.project_id - name = "${local.cluster_type}-cluster${var.cluster_name_suffix}" - regional = var.regional - region = var.region - zones = var.zones - network = var.network - subnetwork = var.subnetwork - ip_range_pods = var.ip_range_pods - ip_range_services = var.ip_range_services - create_service_account = var.compute_engine_service_account == "create" - service_account = var.compute_engine_service_account - istio = var.istio - cloudrun = var.cloudrun - dns_cache = var.dns_cache - gce_pd_csi_driver = var.gce_pd_csi_driver - sandbox_enabled = var.sandbox_enabled - remove_default_node_pool = var.remove_default_node_pool - node_pools = var.node_pools - database_encryption = var.database_encryption - enable_binary_authorization = var.enable_binary_authorization - enable_pod_security_policy = var.enable_pod_security_policy - enable_identity_service = true - release_channel = "REGULAR" + source = "../../modules/beta-public-cluster/" + project_id = var.project_id + name = "${local.cluster_type}-cluster${var.cluster_name_suffix}" + regional = var.regional + region = var.region + zones = var.zones + network = var.network + subnetwork = var.subnetwork + ip_range_pods = var.ip_range_pods + ip_range_services = var.ip_range_services + create_service_account = var.compute_engine_service_account == "create" + service_account = var.compute_engine_service_account + istio = var.istio + cloudrun = var.cloudrun + dns_cache = var.dns_cache + gce_pd_csi_driver = var.gce_pd_csi_driver + sandbox_enabled = var.sandbox_enabled + remove_default_node_pool = var.remove_default_node_pool + node_pools = var.node_pools + database_encryption = var.database_encryption + enable_binary_authorization = var.enable_binary_authorization + enable_pod_security_policy = var.enable_pod_security_policy + enable_identity_service = true + release_channel = "REGULAR" + logging_enabled_components = ["SYSTEM_COMPONENTS"] + monitoring_enabled_components = ["SYSTEM_COMPONENTS", "WORKLOADS"] # Disable workload identity identity_namespace = null diff --git a/modules/beta-private-cluster-update-variant/cluster.tf b/modules/beta-private-cluster-update-variant/cluster.tf index 4f912de79..ec7e64d60 100644 --- a/modules/beta-private-cluster-update-variant/cluster.tf +++ b/modules/beta-private-cluster-update-variant/cluster.tf @@ -67,7 +67,8 @@ resource "google_container_cluster" "primary" { type = var.cluster_telemetry_type } } - logging_service = local.cluster_telemetry_type_is_set ? null : var.logging_service + # only one of logging/monitoring_service or logging/monitoring_config can be specified + logging_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.logging_service dynamic "logging_config" { for_each = length(var.logging_enabled_components) > 0 ? [1] : [] @@ -75,8 +76,7 @@ resource "google_container_cluster" "primary" { enable_components = var.logging_enabled_components } } - - monitoring_service = local.cluster_telemetry_type_is_set ? null : var.monitoring_service + monitoring_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.monitoring_service dynamic "monitoring_config" { for_each = length(var.monitoring_enabled_components) > 0 ? [1] : [] diff --git a/modules/beta-private-cluster-update-variant/main.tf b/modules/beta-private-cluster-update-variant/main.tf index c115e1a3c..d9cac2002 100644 --- a/modules/beta-private-cluster-update-variant/main.tf +++ b/modules/beta-private-cluster-update-variant/main.tf @@ -93,6 +93,7 @@ locals { ] : [] cluster_cloudrun_enabled = var.cloudrun cluster_gce_pd_csi_config = var.gce_pd_csi_driver ? [{ enabled = true }] : [{ enabled = false }] + logmon_config_is_set = length(var.logging_enabled_components) > 0 || length(var.monitoring_enabled_components) > 0 cluster_authenticator_security_group = var.authenticator_security_group == null ? [] : [{ security_group = var.authenticator_security_group diff --git a/modules/beta-private-cluster/cluster.tf b/modules/beta-private-cluster/cluster.tf index a1a36a999..532551c2b 100644 --- a/modules/beta-private-cluster/cluster.tf +++ b/modules/beta-private-cluster/cluster.tf @@ -67,7 +67,8 @@ resource "google_container_cluster" "primary" { type = var.cluster_telemetry_type } } - logging_service = local.cluster_telemetry_type_is_set ? null : var.logging_service + # only one of logging/monitoring_service or logging/monitoring_config can be specified + logging_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.logging_service dynamic "logging_config" { for_each = length(var.logging_enabled_components) > 0 ? [1] : [] @@ -75,8 +76,7 @@ resource "google_container_cluster" "primary" { enable_components = var.logging_enabled_components } } - - monitoring_service = local.cluster_telemetry_type_is_set ? null : var.monitoring_service + monitoring_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.monitoring_service dynamic "monitoring_config" { for_each = length(var.monitoring_enabled_components) > 0 ? [1] : [] diff --git a/modules/beta-private-cluster/main.tf b/modules/beta-private-cluster/main.tf index c115e1a3c..d9cac2002 100644 --- a/modules/beta-private-cluster/main.tf +++ b/modules/beta-private-cluster/main.tf @@ -93,6 +93,7 @@ locals { ] : [] cluster_cloudrun_enabled = var.cloudrun cluster_gce_pd_csi_config = var.gce_pd_csi_driver ? [{ enabled = true }] : [{ enabled = false }] + logmon_config_is_set = length(var.logging_enabled_components) > 0 || length(var.monitoring_enabled_components) > 0 cluster_authenticator_security_group = var.authenticator_security_group == null ? [] : [{ security_group = var.authenticator_security_group diff --git a/modules/beta-public-cluster-update-variant/cluster.tf b/modules/beta-public-cluster-update-variant/cluster.tf index 853b04529..964023171 100644 --- a/modules/beta-public-cluster-update-variant/cluster.tf +++ b/modules/beta-public-cluster-update-variant/cluster.tf @@ -67,7 +67,8 @@ resource "google_container_cluster" "primary" { type = var.cluster_telemetry_type } } - logging_service = local.cluster_telemetry_type_is_set ? null : var.logging_service + # only one of logging/monitoring_service or logging/monitoring_config can be specified + logging_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.logging_service dynamic "logging_config" { for_each = length(var.logging_enabled_components) > 0 ? [1] : [] @@ -75,8 +76,7 @@ resource "google_container_cluster" "primary" { enable_components = var.logging_enabled_components } } - - monitoring_service = local.cluster_telemetry_type_is_set ? null : var.monitoring_service + monitoring_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.monitoring_service dynamic "monitoring_config" { for_each = length(var.monitoring_enabled_components) > 0 ? [1] : [] diff --git a/modules/beta-public-cluster-update-variant/main.tf b/modules/beta-public-cluster-update-variant/main.tf index a3af1a009..7a20ec04b 100644 --- a/modules/beta-public-cluster-update-variant/main.tf +++ b/modules/beta-public-cluster-update-variant/main.tf @@ -93,6 +93,7 @@ locals { ] : [] cluster_cloudrun_enabled = var.cloudrun cluster_gce_pd_csi_config = var.gce_pd_csi_driver ? [{ enabled = true }] : [{ enabled = false }] + logmon_config_is_set = length(var.logging_enabled_components) > 0 || length(var.monitoring_enabled_components) > 0 cluster_authenticator_security_group = var.authenticator_security_group == null ? [] : [{ security_group = var.authenticator_security_group diff --git a/modules/beta-public-cluster/cluster.tf b/modules/beta-public-cluster/cluster.tf index 3d0ae442f..53d9ac0b3 100644 --- a/modules/beta-public-cluster/cluster.tf +++ b/modules/beta-public-cluster/cluster.tf @@ -67,7 +67,8 @@ resource "google_container_cluster" "primary" { type = var.cluster_telemetry_type } } - logging_service = local.cluster_telemetry_type_is_set ? null : var.logging_service + # only one of logging/monitoring_service or logging/monitoring_config can be specified + logging_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.logging_service dynamic "logging_config" { for_each = length(var.logging_enabled_components) > 0 ? [1] : [] @@ -75,8 +76,7 @@ resource "google_container_cluster" "primary" { enable_components = var.logging_enabled_components } } - - monitoring_service = local.cluster_telemetry_type_is_set ? null : var.monitoring_service + monitoring_service = local.cluster_telemetry_type_is_set || local.logmon_config_is_set ? null : var.monitoring_service dynamic "monitoring_config" { for_each = length(var.monitoring_enabled_components) > 0 ? [1] : [] diff --git a/modules/beta-public-cluster/main.tf b/modules/beta-public-cluster/main.tf index a3af1a009..7a20ec04b 100644 --- a/modules/beta-public-cluster/main.tf +++ b/modules/beta-public-cluster/main.tf @@ -93,6 +93,7 @@ locals { ] : [] cluster_cloudrun_enabled = var.cloudrun cluster_gce_pd_csi_config = var.gce_pd_csi_driver ? [{ enabled = true }] : [{ enabled = false }] + logmon_config_is_set = length(var.logging_enabled_components) > 0 || length(var.monitoring_enabled_components) > 0 cluster_authenticator_security_group = var.authenticator_security_group == null ? [] : [{ security_group = var.authenticator_security_group diff --git a/test/integration/beta_cluster/controls/gcloud.rb b/test/integration/beta_cluster/controls/gcloud.rb index cc66b8f9e..0fcb76f4e 100644 --- a/test/integration/beta_cluster/controls/gcloud.rb +++ b/test/integration/beta_cluster/controls/gcloud.rb @@ -103,6 +103,19 @@ "enabled" => true, }) end + + it "has the expected logging config" do + expect(data['loggingConfig']['componentConfig']['enableComponents']).to match_array([ + "SYSTEM_COMPONENTS" + ]) + end + + it "has the expected monitoring config" do + expect(data['monitoringConfig']['componentConfig']['enableComponents']).to match_array([ + "WORKLOADS", + "SYSTEM_COMPONENTS" + ]) + end end describe "default node pool" do