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
terraform-google-modules · May 3, 2022 · 2316e77 · 2316e77
1 parent a9a69ed
commit 2316e77
Show file tree

Hide file tree

Showing 12 changed files with 59 additions and 39 deletions.
diff --git a/autogen/main/cluster.tf.tmpl b/autogen/main/cluster.tf.tmpl
@@ -78,16 +78,16 @@ 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] : []
 
     content {
       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
@@ -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
@@ -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
@@ -67,16 +67,16 @@ 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] : []
 
     content {
       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
@@ -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
@@ -67,16 +67,16 @@ 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] : []
 
     content {
       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
@@ -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
@@ -67,16 +67,16 @@ 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] : []
 
     content {
       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
@@ -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
@@ -67,16 +67,16 @@ 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] : []
 
     content {
       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
@@ -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
@@ -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