Mask GKE Sandbox-specific labels and taints (#3749)

third_party/terraform/tests/resource_container_cluster_test.go.erb

@@ -902,6 +902,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"),
+				),
+			},
 		},
 	})
 }
@@ -2825,6 +2851,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)

third_party/terraform/tests/resource_container_node_pool_test.go.erb

@@ -235,6 +235,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"),
+				),
+			},
 		},
 	})
 }
@@ -1256,9 +1282,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",

third_party/terraform/utils/node_config.go.erb

@@ -2,7 +2,6 @@
 package google
 
 import (
-	"strconv"
 	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
@@ -83,6 +82,9 @@ func schemaNodeConfig() *schema.Schema {
 					Computed: true,
 					ForceNew: true,
 					Elem:     &schema.Schema{Type: schema.TypeString},
+	<% unless version.nil? || version == 'ga' -%>
+					DiffSuppressFunc: containerNodePoolLabelsSuppress,
+	<% end -%>
 				},
 
 				"local_ssd_count": {
@@ -183,6 +185,9 @@ func schemaNodeConfig() *schema.Schema {
 					// Legacy config mode allows explicitly defining an empty taint.
 					// See https://www.terraform.io/docs/configuration/attr-as-blocks.html
 					ConfigMode: schema.SchemaConfigModeAttr,
+	<% unless version.nil? || version == 'ga' -%>
+					DiffSuppressFunc: containerNodePoolTaintSuppress,
+	<% end -%>
 					Elem: &schema.Resource{
 						Schema: map[string]*schema.Schema{
 							"key": {
@@ -502,4 +507,119 @@ 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
+}
 <% end -%>