diff --git a/.changelog/3749.txt b/.changelog/3749.txt new file mode 100644 index 0000000000..43bc83aecd --- /dev/null +++ b/.changelog/3749.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +compute: Masked automatically applied GKE Sandbox node labels and taints on node pools +``` diff --git a/google-beta/node_config.go b/google-beta/node_config.go index a7cd3e1e5b..2715b941f3 100644 --- a/google-beta/node_config.go +++ b/google-beta/node_config.go @@ -1,6 +1,8 @@ package google import ( + "strings" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" containerBeta "google.golang.org/api/container/v1beta1" @@ -76,9 +78,10 @@ func schemaNodeConfig() *schema.Schema { Type: schema.TypeMap, Optional: true, // Computed=true because GKE Sandbox will automatically add labels to nodes that can/cannot run sandboxed pods. - Computed: true, - ForceNew: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Computed: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + DiffSuppressFunc: containerNodePoolLabelsSuppress, }, "local_ssd_count": { @@ -178,7 +181,8 @@ func schemaNodeConfig() *schema.Schema { ForceNew: true, // Legacy config mode allows explicitly defining an empty taint. // See https://www.terraform.io/docs/configuration/attr-as-blocks.html - ConfigMode: schema.SchemaConfigModeAttr, + ConfigMode: schema.SchemaConfigModeAttr, + DiffSuppressFunc: containerNodePoolTaintSuppress, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "key": { @@ -482,3 +486,118 @@ func flattenSandboxConfig(c *containerBeta.SandboxConfig) []map[string]interface } return result } + +func containerNodePoolLabelsSuppress(k, old, new string, d *schema.ResourceData) bool { + // Node configs are embedded into multiple resources (container cluster and + // container node pool) so we determine the node config key dynamically. + idx := strings.Index(k, ".labels.") + if idx < 0 { + return false + } + + root := k[:idx] + + // Right now, GKE only applies its own out-of-band labels when you enable + // Sandbox. We only need to perform diff suppression in this case; + // otherwise, the default Terraform behavior is fine. + o, n := d.GetChange(root + ".sandbox_config.0.sandbox_type") + if o == nil || n == nil { + return false + } + + // Pull the entire changeset as a list rather than trying to deal with each + // element individually. + o, n = d.GetChange(root + ".labels") + if o == nil || n == nil { + return false + } + + labels := n.(map[string]interface{}) + + // Remove all current labels, skipping GKE-managed ones if not present in + // the new configuration. + for key, value := range o.(map[string]interface{}) { + if nv, ok := labels[key]; ok && nv == value { + delete(labels, key) + } else if !strings.HasPrefix(key, "sandbox.gke.io/") { + // User-provided label removed in new configuration. + return false + } + } + + // If, at this point, the map still has elements, the new configuration + // added an additional taint. + if len(labels) > 0 { + return false + } + + return true +} + +func containerNodePoolTaintSuppress(k, old, new string, d *schema.ResourceData) bool { + // Node configs are embedded into multiple resources (container cluster and + // container node pool) so we determine the node config key dynamically. + idx := strings.Index(k, ".taint.") + if idx < 0 { + return false + } + + root := k[:idx] + + // Right now, GKE only applies its own out-of-band labels when you enable + // Sandbox. We only need to perform diff suppression in this case; + // otherwise, the default Terraform behavior is fine. + o, n := d.GetChange(root + ".sandbox_config.0.sandbox_type") + if o == nil || n == nil { + return false + } + + // Pull the entire changeset as a list rather than trying to deal with each + // element individually. + o, n = d.GetChange(root + ".taint") + if o == nil || n == nil { + return false + } + + type taintType struct { + Key, Value, Effect string + } + + taintSet := make(map[taintType]struct{}) + + // Add all new taints to set. + for _, raw := range n.([]interface{}) { + data := raw.(map[string]interface{}) + taint := taintType{ + Key: data["key"].(string), + Value: data["value"].(string), + Effect: data["effect"].(string), + } + taintSet[taint] = struct{}{} + } + + // Remove all current taints, skipping GKE-managed keys if not present in + // the new configuration. + for _, raw := range o.([]interface{}) { + data := raw.(map[string]interface{}) + taint := taintType{ + Key: data["key"].(string), + Value: data["value"].(string), + Effect: data["effect"].(string), + } + if _, ok := taintSet[taint]; ok { + delete(taintSet, taint) + } else if !strings.HasPrefix(taint.Key, "sandbox.gke.io/") { + // User-provided taint removed in new configuration. + return false + } + } + + // If, at this point, the set still has elements, the new configuration + // added an additional taint. + if len(taintSet) > 0 { + return false + } + + return true +} diff --git a/google-beta/resource_container_cluster_test.go b/google-beta/resource_container_cluster_test.go index bd93bb8b39..6fa9df9bb3 100644 --- a/google-beta/resource_container_cluster_test.go +++ b/google-beta/resource_container_cluster_test.go @@ -895,6 +895,32 @@ func TestAccContainerCluster_withSandboxConfig(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"min_master_version"}, }, + { + // GKE sets automatic labels and taints on nodes. This makes + // sure we ignore the automatic ones and keep our own. + Config: testAccContainerCluster_withSandboxConfig(clusterName), + // When we use PlanOnly without ExpectNonEmptyPlan, we're + // guaranteeing that the computed fields of the resources don't + // force an unintentional change to the plan. That is, we + // expect this part of the test to pass only if the plan + // doesn't change. + PlanOnly: true, + }, + { + // Now we'll modify the labels, which should force a change to + // the plan. We make sure we don't over-suppress and end up + // eliminating the labels or taints we asked for. This will + // destroy and recreate the cluster as labels are immutable. + Config: testAccContainerCluster_withSandboxConfig_changeLabels(clusterName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("google_container_cluster.with_sandbox_config", + "node_config.0.labels.test.terraform.io/gke-sandbox", "true"), + resource.TestCheckResourceAttr("google_container_cluster.with_sandbox_config", + "node_config.0.labels.test.terraform.io/gke-sandbox-amended", "also-true"), + resource.TestCheckResourceAttr("google_container_cluster.with_sandbox_config", + "node_config.0.taint.0.key", "test.terraform.io/gke-sandbox"), + ), + }, }, }) } @@ -2798,6 +2824,55 @@ resource "google_container_cluster" "with_sandbox_config" { sandbox_config { sandbox_type = "gvisor" } + + labels = { + "test.terraform.io/gke-sandbox" = "true" + } + + taint { + key = "test.terraform.io/gke-sandbox" + value = "true" + effect = "NO_SCHEDULE" + } + } +} +`, clusterName) +} + +func testAccContainerCluster_withSandboxConfig_changeLabels(clusterName string) string { + return fmt.Sprintf(` +data "google_container_engine_versions" "central1a" { + location = "us-central1-a" +} + +resource "google_container_cluster" "with_sandbox_config" { + name = "%s" + location = "us-central1-a" + initial_node_count = 1 + min_master_version = data.google_container_engine_versions.central1a.latest_master_version + + node_config { + oauth_scopes = [ + "https://www.googleapis.com/auth/logging.write", + "https://www.googleapis.com/auth/monitoring", + ] + + image_type = "COS_CONTAINERD" + + sandbox_config { + sandbox_type = "gvisor" + } + + labels = { + "test.terraform.io/gke-sandbox" = "true" + "test.terraform.io/gke-sandbox-amended" = "also-true" + } + + taint { + key = "test.terraform.io/gke-sandbox" + value = "true" + effect = "NO_SCHEDULE" + } } } `, clusterName) diff --git a/google-beta/resource_container_node_pool_test.go b/google-beta/resource_container_node_pool_test.go index 71d71e20b9..1065f369e8 100644 --- a/google-beta/resource_container_node_pool_test.go +++ b/google-beta/resource_container_node_pool_test.go @@ -234,6 +234,32 @@ func TestAccContainerNodePool_withSandboxConfig(t *testing.T) { ImportState: true, ImportStateVerify: true, }, + { + // GKE sets automatic labels and taints on nodes. This makes + // sure we ignore the automatic ones and keep our own. + Config: testAccContainerNodePool_withSandboxConfig(cluster, np), + // When we use PlanOnly without ExpectNonEmptyPlan, we're + // guaranteeing that the computed fields of the resources don't + // force an unintentional change to the plan. That is, we + // expect this part of the test to pass only if the plan + // doesn't change. + PlanOnly: true, + }, + { + // Now we'll modify the taints, which should force a change to + // the plan. We make sure we don't over-suppress and end up + // eliminating the labels or taints we asked for. This will + // destroy and recreate the node pool as taints are immutable. + Config: testAccContainerNodePool_withSandboxConfig_changeTaints(cluster, np), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("google_container_node_pool.with_sandbox_config", + "node_config.0.labels.test.terraform.io/gke-sandbox", "true"), + resource.TestCheckResourceAttr("google_container_node_pool.with_sandbox_config", + "node_config.0.taint.0.key", "test.terraform.io/gke-sandbox"), + resource.TestCheckResourceAttr("google_container_node_pool.with_sandbox_config", + "node_config.0.taint.1.key", "test.terraform.io/gke-sandbox-amended"), + ), + }, }, }) } @@ -1251,9 +1277,71 @@ resource "google_container_node_pool" "with_sandbox_config" { initial_node_count = 1 node_config { image_type = "COS_CONTAINERD" + + sandbox_config { + sandbox_type = "gvisor" + } + + labels = { + "test.terraform.io/gke-sandbox" = "true" + } + + taint { + key = "test.terraform.io/gke-sandbox" + value = "true" + effect = "NO_SCHEDULE" + } + + oauth_scopes = [ + "https://www.googleapis.com/auth/logging.write", + "https://www.googleapis.com/auth/monitoring", + ] + } +} +`, cluster, np) +} + +func testAccContainerNodePool_withSandboxConfig_changeTaints(cluster, np string) string { + return fmt.Sprintf(` +data "google_container_engine_versions" "central1a" { + location = "us-central1-a" +} + +resource "google_container_cluster" "cluster" { + name = "%s" + location = "us-central1-a" + initial_node_count = 1 + min_master_version = data.google_container_engine_versions.central1a.latest_master_version +} + +resource "google_container_node_pool" "with_sandbox_config" { + name = "%s" + location = "us-central1-a" + cluster = google_container_cluster.cluster.name + initial_node_count = 1 + node_config { + image_type = "COS_CONTAINERD" + sandbox_config { sandbox_type = "gvisor" } + + labels = { + "test.terraform.io/gke-sandbox" = "true" + } + + taint { + key = "test.terraform.io/gke-sandbox" + value = "true" + effect = "NO_SCHEDULE" + } + + taint { + key = "test.terraform.io/gke-sandbox-amended" + value = "also-true" + effect = "NO_SCHEDULE" + } + oauth_scopes = [ "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/monitoring",