From 3421c4ae89505cd01bc75c061d4f0057c289b2ad Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Tue, 28 Nov 2023 08:15:31 -0500 Subject: [PATCH 01/23] resourceManagerTags added to Cluster Node Config schema --- .../terraform/services/container/node_config.go.erb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index b5ce9e2e29bc..96f9b0944ead 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -678,6 +678,14 @@ func schemaNodeConfig() *schema.Schema { }, }, }, + <% unless version == 'beta' -%> + "resource_manager_tags": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + Description: `A map of resource manager tag keys and values to be attached to the nodes.`, + }, + <% end -%> <% unless version == 'ga' -%> "enable_confidential_storage": { Type: schema.TypeBool, @@ -977,7 +985,7 @@ func expandNodeConfig(v interface{}) *container.NodeConfig { nc.EnableConfidentialStorage = v.(bool) } <% end -%> - + <% unless version == "ga" -%> if v, ok := nodeConfig["host_maintenance_policy"]; ok { nc.HostMaintenancePolicy = expandHostMaintenancePolicy(v) @@ -1216,7 +1224,7 @@ func flattenNodeConfig(c *container.NodeConfig, v interface{}) []map[string]inte "advanced_machine_features": flattenAdvancedMachineFeaturesConfig(c.AdvancedMachineFeatures), "sole_tenant_config": flattenSoleTenantConfig(c.SoleTenantConfig), "fast_socket": flattenFastSocket(c.FastSocket), - <% unless version == 'ga' -%> + <% unless version == 'ga' -%> "enable_confidential_storage": c.EnableConfidentialStorage, <% end -%> }) From 3e75da5d81c98f4d3127e2545b4c5f6066a82d3b Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 18 Dec 2023 08:42:42 -0500 Subject: [PATCH 02/23] update beta tag --- .../services/compute/resource_compute_instance.go.erb | 8 ++++---- .../terraform/services/container/node_config.go.erb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb index d0e8b81c8d0f..8252c1a56419 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb @@ -635,7 +635,7 @@ func ResourceComputeInstance() *schema.Resource { }, }, - "params": { + "params": { Type: schema.TypeList, MaxItems: 1, Optional: true, @@ -659,7 +659,7 @@ func ResourceComputeInstance() *schema.Resource { Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Description: `A set of key/value label pairs assigned to the instance. - + **Note**: This field is non-authoritative, and will only manage the labels present in your configuration. Please refer to the field 'effective_labels' for all of the labels present on the resource.`, }, @@ -1390,7 +1390,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("Error creating instance while setting the security policies: %s", err) } <% end -%> - + err = waitUntilInstanceHasDesiredStatus(config, d) if err != nil { return fmt.Errorf("Error waiting for status: %s", err) @@ -1841,7 +1841,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("Instance had unexpected number of network interfaces: %d", len(instance.NetworkInterfaces)) } - <% unless version == 'ga' -%> + <% unless version == 'ga' -%> updateSecurityPolicy := false for i := 0; i < len(instance.NetworkInterfaces); i++ { prefix := fmt.Sprintf("network_interface.%d", i) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index 0397c3652cd5..c8410946261b 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -675,7 +675,7 @@ func schemaNodeConfig() *schema.Schema { }, }, }, - <% unless version == 'beta' -%> + <% if version == 'beta' -%> "resource_manager_tags": { Type: schema.TypeMap, Optional: true, From 8084d1ad0d35c453c38664ed967a0636631bbae5 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 18 Dec 2023 09:10:31 -0500 Subject: [PATCH 03/23] add cluster and node proto tests --- .../resource_container_cluster_test.go.erb | 36 ++++++++++++++--- .../resource_container_node_pool_test.go.erb | 40 +++++++++++++++++-- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 1a8d981c7cb3..adac27aad5ad 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -56,6 +56,32 @@ func TestAccContainerCluster_basic(t *testing.T) { }) } +func TestAccContainerCluster_resourceManagerTags(t *testing.T) { + t.Parallel() + + var instance compute.Instance + var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + context := map[string]interface{}{ + "project": envvar.GetTestProjectFromEnv(), + "random_suffix": acctest.RandString(t, 10), + "instance_name": instanceName, + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccContainerCluster_resourceManagerTags(context), + Check: resource.ComposeTestCheckFunc( + testAccCheckContainerClusterExists( + t, "google_compute_instance.foobar", &instance)), + }, + }, + }) +} + func TestAccContainerCluster_networkingModeRoutes(t *testing.T) { t.Parallel() @@ -4398,11 +4424,11 @@ func testAccContainerCluster_withIncompatibleMasterVersionNodeVersion(name strin resource "google_container_cluster" "gke_cluster" { name = "%s" location = "us-central1" - + min_master_version = "1.10.9-gke.5" node_version = "1.10.6-gke.11" initial_node_count = 1 - + } `, name) } @@ -6607,7 +6633,7 @@ resource "google_container_cluster" "with_autoprovisioning" { min_master_version = data.google_container_engine_versions.central1a.latest_master_version initial_node_count = 1 deletion_protection = false - + network = "%s" subnetwork = "%s" @@ -9072,7 +9098,7 @@ resource "google_compute_resource_policy" "policy" { resource "google_container_cluster" "cluster" { name = "%s" location = "us-central1-a" - + node_pool { name = "%s" initial_node_count = 2 @@ -9107,7 +9133,7 @@ func testAccContainerCluster_additional_pod_ranges_config(name string, nameCount } `, podRangeNamesStr) } - + return fmt.Sprintf(` resource "google_compute_network" "main" { name = "%s" diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index e06180e6c0e9..0046b8b0acd8 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -37,6 +37,36 @@ func TestAccContainerNodePool_basic(t *testing.T) { }) } +func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { + t.Parallel() + + var instance compute.Instance + var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + context := map[string]interface{}{ + "project": envvar.GetTestProjectFromEnv(), + "random_suffix": acctest.RandString(t, 10), + "instance_name": instanceName, + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccContainerNodePool_basicWithClusterId(cluster, np, networkName, subnetworkName), + }, + { + ResourceName: "google_container_node_pool.np", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"cluster"}, + }, + }, + }) +} + + func TestAccContainerNodePool_basicWithClusterId(t *testing.T) { t.Parallel() @@ -2600,6 +2630,10 @@ resource "google_container_node_pool" "np_with_node_config" { resource_labels = { "key1" = "foo" } + + resource_manager_tags = { + "asd" = "asd" + } } } `, cluster, networkName, subnetworkName, nodePool) @@ -4132,7 +4166,7 @@ resource "google_container_node_pool" "with_confidential_boot_disk" { name = "%s" location = "us-central1-a" cluster = google_container_cluster.cluster.name - + node_config { image_type = "COS_CONTAINERD" boot_disk_kms_key = "%s" @@ -4149,7 +4183,7 @@ resource "google_container_node_pool" "with_confidential_boot_disk" { } func TestAccContainerNodePool_withoutConfidentialBootDisk(t *testing.T) { - t.Parallel() + t.Parallel() cluster := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) np := fmt.Sprintf("tf-test-np-%s", acctest.RandString(t, 10)) @@ -4193,7 +4227,7 @@ resource "google_container_node_pool" "without_confidential_boot_disk" { name = "%s" location = "us-central1-a" cluster = google_container_cluster.cluster.name - + node_config { image_type = "COS_CONTAINERD" oauth_scopes = [ From 53f515072bdf22171eb6351d933f437a5677e6cf Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 18 Dec 2023 11:34:47 -0500 Subject: [PATCH 04/23] add expand and flatten proto --- .../services/container/node_config.go.erb | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index c8410946261b..291655560d99 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -892,6 +892,10 @@ func expandNodeConfig(v interface{}) *container.NodeConfig { nc.ResourceLabels = m } + if v, ok := nodeConfig["resource_manager_tags"]; ok { + nc.ResourceManagerTags = expandResourceManagerTags(v) + } + if v, ok := nodeConfig["tags"]; ok { tagsList := v.([]interface{}) tags := []string{} @@ -996,6 +1000,11 @@ func expandNodeConfig(v interface{}) *container.NodeConfig { return nc } +func expandResourceManagerTags(v interface{}) *container.ResourceManagerTags { + // expand func + return nil +} + func expandWorkloadMetadataConfig(v interface{}) *container.WorkloadMetadataConfig { if v == nil { return nil @@ -1233,6 +1242,17 @@ func flattenNodeConfig(c *container.NodeConfig, v interface{}) []map[string]inte return config } +func flattenResourceManagerTags(c *container.ResourceManagerTags) []map[string]interface{} { + result := []map[string]interface{}{} + for _, rmtags := range c { + rmtag := container.ResourceManagerTags{ + "tags": rmtags.Tags, + } + } + + return result +} + func flattenAdvancedMachineFeaturesConfig(c *container.AdvancedMachineFeatures) []map[string]interface{} { result := []map[string]interface{}{} if c != nil { From f8046d702121eeabea9e7ded063699652e2c5927 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Tue, 19 Dec 2023 08:56:50 -0500 Subject: [PATCH 05/23] removed beta tag --- .../third_party/terraform/services/container/node_config.go.erb | 2 -- 1 file changed, 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index 291655560d99..beead743cb93 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -675,14 +675,12 @@ func schemaNodeConfig() *schema.Schema { }, }, }, - <% if version == 'beta' -%> "resource_manager_tags": { Type: schema.TypeMap, Optional: true, ForceNew: true, Description: `A map of resource manager tag keys and values to be attached to the nodes.`, }, - <% end -%> <% unless version == 'ga' -%> "enable_confidential_storage": { Type: schema.TypeBool, From 9ef5388bd796ae866967811525ea9b791a98c369 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Tue, 19 Dec 2023 12:48:45 -0500 Subject: [PATCH 06/23] added to documentation --- .../website/docs/r/container_cluster.html.markdown | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown index 7107d81fc7ed..7ca8f25c023b 100644 --- a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -905,6 +905,8 @@ gvnic { * `tags` - (Optional) The list of instance tags applied to all nodes. Tags are used to identify valid sources or targets for network firewalls. +* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}` + * `taint` - (Optional) A list of [Kubernetes taints](https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/) to apply to nodes. This field will only report drift on taint keys that are @@ -998,7 +1000,7 @@ sole_tenant_config { * `count` (Required) - The number of the guest accelerator cards exposed to this instance. * `gpu_driver_installation_config` (Optional) - Configuration for auto installation of GPU driver. Structure is [documented below](#nested_gpu_driver_installation_config). - + * `gpu_partition_size` (Optional) - Size of partitions to create on the GPU. Valid values are described in the NVIDIA mig [user guide](https://docs.nvidia.com/datacenter/tesla/mig-user-guide/#partitioning). * `gpu_sharing_config` (Optional) - Configuration for GPU sharing. Structure is [documented below](#nested_gpu_sharing_config). @@ -1345,7 +1347,7 @@ exported: * `node_config.0.effective_taints` - List of kubernetes taints applied to each node. Structure is [documented above](#nested_taint). -* `fleet.0.membership` - The resource name of the fleet Membership resource associated to this cluster with format `//gkehub.googleapis.com/projects/{{project}}/locations/{{location}}/memberships/{{name}}`. See the official doc for [fleet management](https://cloud.google.com/kubernetes-engine/docs/fleets-overview). +* `fleet.0.membership` - The resource name of the fleet Membership resource associated to this cluster with format `//gkehub.googleapis.com/projects/{{project}}/locations/{{location}}/memberships/{{name}}`. See the official doc for [fleet management](https://cloud.google.com/kubernetes-engine/docs/fleets-overview). ## Timeouts From 18e5a9c41138b0d1e4f2c913d55e0c4b31e4e576 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Tue, 19 Dec 2023 13:22:27 -0500 Subject: [PATCH 07/23] added resource manager tags to auto pilot --- .../services/container/node_config.go.erb | 3 ++- .../resource_container_cluster.go.erb | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index beead743cb93..d6d7f5ee8f1a 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -679,7 +679,8 @@ func schemaNodeConfig() *schema.Schema { Type: schema.TypeMap, Optional: true, ForceNew: true, - Description: `A map of resource manager tag keys and values to be attached to the nodes.`, + MaxItems: 5, + Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, }, <% unless version == 'ga' -%> "enable_confidential_storage": { diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index 67861db5e724..9380d6fb4c46 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -1439,6 +1439,15 @@ func ResourceContainerCluster() *schema.Resource { }, }, }, + Schema: map[string]*schema.Schema{ + "resource_manager_tags": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + MaxItems: 5, + Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, + }, + }, }, }, @@ -5267,7 +5276,7 @@ func expandFleet(configured interface{}) *container.Fleet { return &container.Fleet{ Project: config["project"].(string), } -} +} func expandEnableK8sBetaApis(configured interface{}, enabledAPIs []string) *container.K8sBetaAPIConfig { l := configured.([]interface{}) @@ -5414,6 +5423,11 @@ func expandNodePoolAutoConfig(configured interface{}) *container.NodePoolAutoCon if v, ok := config["network_tags"]; ok && len(v.([]interface{})) > 0 { npac.NetworkTags = expandNodePoolAutoConfigNetworkTags(v) } + + if v, ok := nodeConfig["resource_manager_tags"]; ok { + npac.ResourceManagerTags = expandResourceManagerTags(v) + } + return npac } @@ -6110,14 +6124,14 @@ func flattenGatewayApiConfig(c *container.GatewayAPIConfig) []map[string]interfa func flattenFleet(c *container.Fleet) []map[string]interface{} { if c == nil { return nil - } + } return []map[string]interface{}{ { "project": c.Project, "membership": c.Membership, "pre_registered": c.PreRegistered, }, - } + } } func flattenEnableK8sBetaApis(c *container.K8sBetaAPIConfig) []map[string]interface{} { From 481eda7ee6a18eb0767d56903283e4d6664e41ab Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Wed, 3 Jan 2024 14:28:48 -0500 Subject: [PATCH 08/23] migrating resourceManagerTags tests --- .../services/container/node_config.go.erb | 10 +- .../resource_container_cluster_test.go.erb | 127 ++++++++++++++---- 2 files changed, 105 insertions(+), 32 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index d6d7f5ee8f1a..8f3b6aabf87e 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -1243,11 +1243,11 @@ func flattenNodeConfig(c *container.NodeConfig, v interface{}) []map[string]inte func flattenResourceManagerTags(c *container.ResourceManagerTags) []map[string]interface{} { result := []map[string]interface{}{} - for _, rmtags := range c { - rmtag := container.ResourceManagerTags{ - "tags": rmtags.Tags, - } - } + // for _, rmtags := range c { + // rmtag := container.ResourceManagerTags{ + // "tags": rmtags.Tags, + // } + // } return result } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index f52bacf62b58..332458b20697 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -2865,16 +2865,17 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) - clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) + randomSuffix := acctest.RandString(t, 10) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, + PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, false, "", randomSuffix), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "networking_mode", "VPC_NATIVE"), ), @@ -2882,7 +2883,7 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) { { ResourceName: "google_container_cluster.with_autopilot", ImportState: true, - ImportStateVerify: true, + ImportStateVerify: true, ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, }, }, @@ -2893,17 +2894,18 @@ func TestAccContainerClusterCustomServiceAccount_withAutopilot(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) - clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) - serviceAccountName := fmt.Sprintf("tf-test-sa-%s", acctest.RandString(t, 10)) + randomSuffix := acctest.RandString(t, 10) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + serviceAccountName := fmt.Sprintf("tf-test-sa-%s", randomSuffix) acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, + PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, serviceAccountName), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, false, serviceAccountName, randomSuffix), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "cluster_autoscaling.0.enabled", "true"), @@ -2917,7 +2919,7 @@ func TestAccContainerClusterCustomServiceAccount_withAutopilot(t *testing.T) { { ResourceName: "google_container_cluster.with_autopilot", ImportState: true, - ImportStateVerify: true, + ImportStateVerify: true, ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, }, }, @@ -2928,16 +2930,17 @@ func TestAccContainerCluster_errorAutopilotLocation(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) - clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) + randomSuffix := acctest.RandString(t, 10) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, + PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", true, false, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", true, false, false, "", randomSuffix), ExpectError: regexp.MustCompile(`Autopilot clusters must be regional clusters.`), }, }, @@ -2948,16 +2951,43 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) - clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) + randomSuffix := acctest.RandString(t, 10) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, + PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, true, false, "", randomSuffix), + }, + { + ResourceName: "google_container_cluster.with_autopilot", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, + }, + }, + }) +} + +func TestAccContainerCluster_withAutopiloResourceManagerTags(t *testing.T) { + t.Parallel() + + pid := envvar.GetTestProjectFromEnv() + randomSuffix := acctest.RandString(t, 10) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, true, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, true, "", randomSuffix), }, { ResourceName: "google_container_cluster.with_autopilot", @@ -8579,16 +8609,26 @@ resource "google_container_cluster" "with_autopilot" { vertical_pod_autoscaling { enabled = true }`, containerNetName, clusterName, location, enabled, clusterAutoscaling) + if withNetworkTag || withResourceManagerTag { + config += ` + node_pool_auto_config {` + } if withNetworkTag { config += ` - node_pool_auto_config { network_tags { tags = ["test-network-tag"] - } - }` + }` + } + if withResourceManagerTag { + config += ` + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + }` } config += ` -}` + }` // closing bracket "node_pool_auto_config" + config += ` +}` // closing bracket "google_container_cluster" return config } @@ -9443,4 +9483,37 @@ func testAccContainerCluster_withWorkloadALTSConfig(projectID, name, networkName } `, projectID, networkName, subnetworkName, name, enable) } + +func testAccContainerCluster_resourceManagerTags(projectID, clusterName, randomSuffix string) string { + return fmt.Sprintf(` +resource "google_tags_tag_key" "key" { + parent = "projects/%[1]s" + short_name = "foobarbaz%[2]s" + description = "For foo/bar resources." +} + +resource "google_tags_tag_value" "value" { + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources." +} + +resource "google_container_cluster" "primary" { + name = "%[3]s" + location = "us-central1-a" + initial_node_count = 1 + + node_config { + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + } + } + + timeouts { + create = "30m" + update = "40m" + } + } +`, projectID, randomSuffix, clusterName) +} <% end -%> From 9ef00f67b8cfd933568d8e40efdccdbd61fcadf4 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Thu, 4 Jan 2024 11:25:25 -0500 Subject: [PATCH 09/23] migrating node_pools test --- .../resource_container_node_pool_test.go.erb | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index 0046b8b0acd8..d18cfe6bd1b1 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -54,10 +54,10 @@ func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerNodePool_basicWithClusterId(cluster, np, networkName, subnetworkName), + Config: testAccContainerCluster_resourceManagerTags(, np, networkName, subnetworkName), }, { - ResourceName: "google_container_node_pool.np", + ResourceName: "google_container_node_pool.", ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"cluster"}, @@ -4242,3 +4242,51 @@ resource "google_container_node_pool" "without_confidential_boot_disk" { `, cluster, networkName, subnetworkName, np) } <% end -%> + +func testAccContainerNodePool_resourceManagerTags(projectID, clusterName, randomSuffix string) string { + return fmt.Sprintf(` +resource "google_tags_tag_key" "key" { + parent = "projects/%[1]s" + short_name = "foobarbaz%[2]s" + description = "For foo/bar resources." +} + +resource "google_tags_tag_value" "value" { + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources." +} + +resource "google_container_cluster" "primary" { + name = "%[3]s" + location = "us-central1-a" + + # We can't create a cluster with no node pool defined, but we want to only use + # separately managed node pools. So we create the smallest possible default + # node pool and immediately delete it. + remove_default_node_pool = true + initial_node_count = 1 + + timeouts { + create = "30m" + update = "40m" + } + } + +# Separately Managed Node Pool +resource "google_container_node_pool" "primary_nodes" { + name = google_container_cluster.primary.name + location = var.region + cluster = google_container_cluster.primary.name + + version = data.google_container_engine_versions.gke_version.release_channel_latest_version["STABLE"] + node_count = var.gke_num_nodes + + node_config { + # preemptible = true + machine_type = "n1-standard-1" + + +} +`, projectID, randomSuffix, clusterName) +} From 581557f83e6dcec639677b85601ffebd3f7c1d40 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Wed, 17 Jan 2024 10:42:54 -0500 Subject: [PATCH 10/23] migrating additional tests --- .../services/container/node_config.go.erb | 31 +++-- .../resource_container_cluster.go.erb | 23 ++-- .../resource_container_cluster_test.go.erb | 128 ++++++++++++++---- .../resource_container_node_pool_test.go.erb | 74 ++++++---- .../docs/r/container_cluster.html.markdown | 2 +- 5 files changed, 183 insertions(+), 75 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index 8f3b6aabf87e..fb3cb2643cb2 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -679,7 +679,6 @@ func schemaNodeConfig() *schema.Schema { Type: schema.TypeMap, Optional: true, ForceNew: true, - MaxItems: 5, Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, }, <% unless version == 'ga' -%> @@ -1000,8 +999,15 @@ func expandNodeConfig(v interface{}) *container.NodeConfig { } func expandResourceManagerTags(v interface{}) *container.ResourceManagerTags { - // expand func - return nil + rmts := make(map[string]string) + + if v != nil { + rmts = tpgresource.ConvertStringMap(v.(map[string]interface{})) + } + + return &container.ResourceManagerTags{ + Tags: rmts, + } } func expandWorkloadMetadataConfig(v interface{}) *container.WorkloadMetadataConfig { @@ -1229,6 +1235,7 @@ func flattenNodeConfig(c *container.NodeConfig, v interface{}) []map[string]inte "advanced_machine_features": flattenAdvancedMachineFeaturesConfig(c.AdvancedMachineFeatures), "sole_tenant_config": flattenSoleTenantConfig(c.SoleTenantConfig), "fast_socket": flattenFastSocket(c.FastSocket), + "resource_manager_tags": flattenResourceManagerTags(c.ResourceManagerTags), <% unless version == 'ga' -%> "enable_confidential_storage": c.EnableConfidentialStorage, <% end -%> @@ -1241,15 +1248,17 @@ func flattenNodeConfig(c *container.NodeConfig, v interface{}) []map[string]inte return config } -func flattenResourceManagerTags(c *container.ResourceManagerTags) []map[string]interface{} { - result := []map[string]interface{}{} - // for _, rmtags := range c { - // rmtag := container.ResourceManagerTags{ - // "tags": rmtags.Tags, - // } - // } +func flattenResourceManagerTags(c *container.ResourceManagerTags) map[string]interface{} { + rmt := make(map[string]interface{}) - return result + if c != nil { + for k, v := range c.Tags { + rmt[k] = v + } + + } + + return rmt } func flattenAdvancedMachineFeaturesConfig(c *container.AdvancedMachineFeatures) []map[string]interface{} { diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index 45ee01a9d72e..65b07acab155 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -1415,11 +1415,11 @@ func ResourceContainerCluster() *schema.Resource { "node_pool_defaults": clusterSchemaNodePoolDefaults(), "node_pool_auto_config": { - Type: schema.TypeList, - Optional: true, - Computed: true, - MaxItems: 1, - Description: `Node pool configs that apply to all auto-provisioned node pools in autopilot clusters and node auto-provisioning enabled clusters.`, + Type: schema.TypeList, + Optional: true, + Computed: true, + MaxItems: 1, + Description: `Node pool configs that apply to all auto-provisioned node pools in autopilot clusters and node auto-provisioning enabled clusters.`, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "network_tags": { @@ -1438,14 +1438,11 @@ func ResourceContainerCluster() *schema.Resource { }, }, }, - }, - Schema: map[string]*schema.Schema{ "resource_manager_tags": { - Type: schema.TypeMap, - Optional: true, - ForceNew: true, - MaxItems: 5, - Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, }, }, }, @@ -5404,7 +5401,7 @@ func expandNodePoolAutoConfig(configured interface{}) *container.NodePoolAutoCon npac.NetworkTags = expandNodePoolAutoConfigNetworkTags(v) } - if v, ok := nodeConfig["resource_manager_tags"]; ok { + if v, ok := config["resource_manager_tags"]; ok { npac.ResourceManagerTags = expandResourceManagerTags(v) } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 61d5f89fd5eb..c1dfaf49e0ca 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -59,13 +59,13 @@ func TestAccContainerCluster_basic(t *testing.T) { func TestAccContainerCluster_resourceManagerTags(t *testing.T) { t.Parallel() - var instance compute.Instance - var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) - context := map[string]interface{}{ - "project": envvar.GetTestProjectFromEnv(), - "random_suffix": acctest.RandString(t, 10), - "instance_name": instanceName, - } + pid := envvar.GetTestProjectFromEnv() + + randomSuffix := acctest.RandString(t, 10) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + + networkName := acctest.BootstrapSharedTestNetwork(t, "gke-cluster") + subnetworkName := acctest.BootstrapSubnet(t, "gke-cluster", networkName) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -73,10 +73,16 @@ func TestAccContainerCluster_resourceManagerTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_resourceManagerTags(context), + Config: testAccContainerCluster_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), Check: resource.ComposeTestCheckFunc( - testAccCheckContainerClusterExists( - t, "google_compute_instance.foobar", &instance)), + resource.TestCheckResourceAttrSet("google_container_cluster.primary", "self_link"), + ), + }, + { + ResourceName: "google_container_cluster.primary", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_protection"}, }, }, }) @@ -2875,7 +2881,7 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, false, "", randomSuffix), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, ""), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "networking_mode", "VPC_NATIVE"), ), @@ -2905,7 +2911,7 @@ func TestAccContainerClusterCustomServiceAccount_withAutopilot(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, false, serviceAccountName, randomSuffix), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, serviceAccountName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "cluster_autoscaling.0.enabled", "true"), @@ -2940,7 +2946,7 @@ func TestAccContainerCluster_errorAutopilotLocation(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", true, false, false, "", randomSuffix), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", randomSuffix, true, false, false, ""), ExpectError: regexp.MustCompile(`Autopilot clusters must be regional clusters.`), }, }, @@ -2961,7 +2967,7 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, true, false, "", randomSuffix), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, true, false, ""), }, { ResourceName: "google_container_cluster.with_autopilot", @@ -2977,6 +2983,7 @@ func TestAccContainerCluster_withAutopiloResourceManagerTags(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() + randomSuffix := acctest.RandString(t, 10) containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) @@ -2987,7 +2994,7 @@ func TestAccContainerCluster_withAutopiloResourceManagerTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, true, "", randomSuffix), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, true, ""), }, { ResourceName: "google_container_cluster.with_autopilot", @@ -8553,7 +8560,7 @@ resource "google_container_cluster" "primary" { `, name) } -func testAccContainerCluster_withAutopilot(projectID string, containerNetName string, clusterName string, location string, enabled bool, withNetworkTag bool, serviceAccount string) string { +func testAccContainerCluster_withAutopilot(projectID, containerNetName, clusterName, location, randomSuffix string, enabled, withNetworkTag, withResourceManagerTag bool, serviceAccount string) string { config := "" clusterAutoscaling := "" if serviceAccount != "" { @@ -8581,6 +8588,41 @@ resource "google_project_iam_binding" "project" { }`, serviceAccount, projectID) } + if withResourceManagerTag { + config += fmt.Sprintf(` +data "google_project" "project" { + project_id = "%[1]s" +} + +resource "google_project_iam_binding" "tags" { + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] +} + +resource "google_tags_tag_key" "key" { + parent = "projects/%[1]s" + short_name = "foobarbaz-%[2]s" + description = "For foo/bar resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[3]s" + } +} + +resource "google_tags_tag_value" "value" { + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources" +} + +data "google_container_engine_versions" "uscentral1a" { + location = "us-central1-a" +}`, projectID, randomSuffix, containerNetName) + } + config += fmt.Sprintf(` resource "google_compute_network" "container_network" { @@ -8613,6 +8655,7 @@ data "google_container_engine_versions" "central1a" { resource "google_container_cluster" "with_autopilot" { name = "%s" location = "%s" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] enable_autopilot = %v deletion_protection = false min_master_version = "latest" @@ -8634,24 +8677,31 @@ resource "google_container_cluster" "with_autopilot" { vertical_pod_autoscaling { enabled = true }`, containerNetName, clusterName, location, enabled, clusterAutoscaling) + if withNetworkTag || withResourceManagerTag { config += ` node_pool_auto_config {` } + if withNetworkTag { config += ` network_tags { tags = ["test-network-tag"] }` } + if withResourceManagerTag { config += ` resource_manager_tags = { "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" }` } - config += ` + + if withNetworkTag || withResourceManagerTag { + config += ` }` // closing bracket "node_pool_auto_config" + } + config += ` }` // closing bracket "google_container_cluster" return config @@ -9509,36 +9559,64 @@ func testAccContainerCluster_withWorkloadALTSConfig(projectID, name, networkName `, projectID, networkName, subnetworkName, name, enable) } -func testAccContainerCluster_resourceManagerTags(projectID, clusterName, randomSuffix string) string { +func testAccContainerCluster_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` +data "google_project" "project" { + project_id = "%[1]s" +} + +resource "google_project_iam_binding" "tags" { + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] +} + resource "google_tags_tag_key" "key" { parent = "projects/%[1]s" - short_name = "foobarbaz%[2]s" - description = "For foo/bar resources." + short_name = "foobarbaz-%[2]s" + description = "For foo/bar resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } } resource "google_tags_tag_value" "value" { parent = "tagKeys/${google_tags_tag_key.key.name}" short_name = "foo-%[2]s" - description = "For foo resources." + description = "For foo resources" +} + +data "google_container_engine_versions" "uscentral1a" { + location = "us-central1-a" } resource "google_container_cluster" "primary" { name = "%[3]s" location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] initial_node_count = 1 node_config { + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 + resource_manager_tags = { "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" } } + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" + timeouts { - create = "30m" - update = "40m" + create = "30m" + update = "40m" } - } -`, projectID, randomSuffix, clusterName) +} +`, projectID, randomSuffix, clusterName, networkName, subnetworkName) } <% end -%> diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index d18cfe6bd1b1..cbe37477fddc 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -40,13 +40,13 @@ func TestAccContainerNodePool_basic(t *testing.T) { func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { t.Parallel() - var instance compute.Instance - var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) - context := map[string]interface{}{ - "project": envvar.GetTestProjectFromEnv(), - "random_suffix": acctest.RandString(t, 10), - "instance_name": instanceName, - } + pid := envvar.GetTestProjectFromEnv() + + randomSuffix := acctest.RandString(t, 10) + clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + + networkName := acctest.BootstrapSharedTestNetwork(t, "gke-cluster") + subnetworkName := acctest.BootstrapSubnet(t, "gke-cluster", networkName) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -54,7 +54,7 @@ func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_resourceManagerTags(, np, networkName, subnetworkName), + Config: testAccContainerNodePool_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), }, { ResourceName: "google_container_node_pool.", @@ -2630,10 +2630,6 @@ resource "google_container_node_pool" "np_with_node_config" { resource_labels = { "key1" = "foo" } - - resource_manager_tags = { - "asd" = "asd" - } } } `, cluster, networkName, subnetworkName, nodePool) @@ -4241,31 +4237,55 @@ resource "google_container_node_pool" "without_confidential_boot_disk" { } `, cluster, networkName, subnetworkName, np) } -<% end -%> -func testAccContainerNodePool_resourceManagerTags(projectID, clusterName, randomSuffix string) string { +func testAccContainerNodePool_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` +data "google_project" "project" { + project_id = "%[1]s" +} + +resource "google_project_iam_binding" "tags" { + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] +} + resource "google_tags_tag_key" "key" { - parent = "projects/%[1]s" - short_name = "foobarbaz%[2]s" - description = "For foo/bar resources." + parent = "projects/%[1]s" + short_name = "foobarbaz-%[2]s" + description = "For foo/bar resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } } resource "google_tags_tag_value" "value" { - parent = "tagKeys/${google_tags_tag_key.key.name}" - short_name = "foo-%[2]s" - description = "For foo resources." + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources" +} + +data "google_container_engine_versions" "uscentral1a" { + location = "us-central1-a" } resource "google_container_cluster" "primary" { name = "%[3]s" location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] # We can't create a cluster with no node pool defined, but we want to only use # separately managed node pools. So we create the smallest possible default # node pool and immediately delete it. remove_default_node_pool = true - initial_node_count = 1 + initial_node_count = 1 + + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" timeouts { create = "30m" @@ -4283,10 +4303,14 @@ resource "google_container_node_pool" "primary_nodes" { node_count = var.gke_num_nodes node_config { - # preemptible = true - machine_type = "n1-standard-1" - + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + } + } } -`, projectID, randomSuffix, clusterName) +`, projectID, randomSuffix, clusterName, networkName, subnetworkName) } +<% end -%> diff --git a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown index 7ca8f25c023b..9bff6ce9e01f 100644 --- a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -905,7 +905,7 @@ gvnic { * `tags` - (Optional) The list of instance tags applied to all nodes. Tags are used to identify valid sources or targets for network firewalls. -* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}` +* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}`. * `taint` - (Optional) A list of [Kubernetes taints](https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/) From 428484326dab0e0d7e74b6d28122330e8fd4ef2a Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 29 Jan 2024 03:23:33 -0500 Subject: [PATCH 11/23] minor fixes --- .../services/container/node_config.go.erb | 2 +- .../resource_container_cluster_test.go.erb | 16 ++-- .../resource_container_node_pool_test.go.erb | 76 +++++++++---------- .../docs/r/container_cluster.html.markdown | 2 + 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index fb3cb2643cb2..bd76430b7dc5 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -890,7 +890,7 @@ func expandNodeConfig(v interface{}) *container.NodeConfig { nc.ResourceLabels = m } - if v, ok := nodeConfig["resource_manager_tags"]; ok { + if v, ok := nodeConfig["resource_manager_tags"]; ok && len(v.(map[string]interface{})) > 0 { nc.ResourceManagerTags = expandResourceManagerTags(v) } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index c1dfaf49e0ca..1f41aebe38f3 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -9562,15 +9562,15 @@ func testAccContainerCluster_withWorkloadALTSConfig(projectID, name, networkName func testAccContainerCluster_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` data "google_project" "project" { - project_id = "%[1]s" + project_id = "%[1]s" } resource "google_project_iam_binding" "tags" { - project = "%[1]s" - role = "roles/resourcemanager.tagHoldAdmin" - members = [ - "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - ] + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] } resource "google_tags_tag_key" "key" { @@ -9579,7 +9579,7 @@ resource "google_tags_tag_key" "key" { description = "For foo/bar resources" purpose = "GCE_FIREWALL" purpose_data = { - network = "%[1]s/%[4]s" + network = "%[1]s/%[4]s" } } @@ -9590,7 +9590,7 @@ resource "google_tags_tag_value" "value" { } data "google_container_engine_versions" "uscentral1a" { - location = "us-central1-a" + location = "us-central1-a" } resource "google_container_cluster" "primary" { diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index cbe37477fddc..fd4292d06194 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4241,57 +4241,57 @@ resource "google_container_node_pool" "without_confidential_boot_disk" { func testAccContainerNodePool_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` data "google_project" "project" { - project_id = "%[1]s" + project_id = "%[1]s" } resource "google_project_iam_binding" "tags" { - project = "%[1]s" - role = "roles/resourcemanager.tagHoldAdmin" - members = [ - "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - ] + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] } resource "google_tags_tag_key" "key" { - parent = "projects/%[1]s" - short_name = "foobarbaz-%[2]s" - description = "For foo/bar resources" - purpose = "GCE_FIREWALL" - purpose_data = { - network = "%[1]s/%[4]s" - } + parent = "projects/%[1]s" + short_name = "foobarbaz-%[2]s" + description = "For foo/bar resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } } resource "google_tags_tag_value" "value" { - parent = "tagKeys/${google_tags_tag_key.key.name}" - short_name = "foo-%[2]s" - description = "For foo resources" + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources" } data "google_container_engine_versions" "uscentral1a" { - location = "us-central1-a" + location = "us-central1-a" } resource "google_container_cluster" "primary" { - name = "%[3]s" - location = "us-central1-a" - min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + name = "%[3]s" + location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] - # We can't create a cluster with no node pool defined, but we want to only use - # separately managed node pools. So we create the smallest possible default - # node pool and immediately delete it. - remove_default_node_pool = true - initial_node_count = 1 + # We can't create a cluster with no node pool defined, but we want to only use + # separately managed node pools. So we create the smallest possible default + # node pool and immediately delete it. + remove_default_node_pool = true + initial_node_count = 1 - deletion_protection = false - network = "%[4]s" - subnetwork = "%[5]s" + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" - timeouts { - create = "30m" - update = "40m" - } + timeouts { + create = "30m" + update = "40m" } +} # Separately Managed Node Pool resource "google_container_node_pool" "primary_nodes" { @@ -4299,16 +4299,16 @@ resource "google_container_node_pool" "primary_nodes" { location = var.region cluster = google_container_cluster.primary.name - version = data.google_container_engine_versions.gke_version.release_channel_latest_version["STABLE"] + version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] node_count = var.gke_num_nodes node_config { - machine_type = "n1-standard-1" // can't be e2 because of local-ssd - disk_size_gb = 15 + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 - resource_manager_tags = { - "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" - } + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + } } } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) diff --git a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown index 9bff6ce9e01f..479d4837078c 100644 --- a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -1034,6 +1034,8 @@ workload_identity_config { The `node_pool_auto_config` block supports: +* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}`. + * `network_tags` (Optional) - The network tag config for the cluster's automatically provisioned node pools. The `network_tags` block supports: From 4791576cb0c01dd13f615ab49e92a4de74f40854 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 29 Jan 2024 11:22:24 -0500 Subject: [PATCH 12/23] fixing tests --- .../resource_container_cluster_test.go.erb | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 1f41aebe38f3..bd1d7f538029 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -76,10 +76,12 @@ func TestAccContainerCluster_resourceManagerTags(t *testing.T) { Config: testAccContainerCluster_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet("google_container_cluster.primary", "self_link"), + resource.TestCheckResourceAttrSet("google_container_cluster.primary", "node_config.0.resource_manager_tags"), ), }, { ResourceName: "google_container_cluster.primary", + ImportStateId: fmt.Sprintf("us-central1-a/%s", clusterName), ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"deletion_protection"}, @@ -9594,28 +9596,34 @@ data "google_container_engine_versions" "uscentral1a" { } resource "google_container_cluster" "primary" { - name = "%[3]s" - location = "us-central1-a" - min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] - initial_node_count = 1 + name = "%[3]s" + location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + initial_node_count = 1 - node_config { - machine_type = "n1-standard-1" // can't be e2 because of local-ssd - disk_size_gb = 15 + node_config { + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 - resource_manager_tags = { - "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" - } - } + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + } + } - deletion_protection = false - network = "%[4]s" - subnetwork = "%[5]s" + node_pool_auto_config { + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + } + } - timeouts { - create = "30m" - update = "40m" - } + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" + + timeouts { + create = "30m" + update = "40m" + } } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } From 6d8088ad2c5f04ab83384bc920e57388f6e093da Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Wed, 31 Jan 2024 12:25:07 -0500 Subject: [PATCH 13/23] add in-place update support --- .../services/container/node_config.go.erb | 4 +- .../resource_container_cluster.go.erb | 21 +++++++++- .../resource_container_cluster_test.go.erb | 2 +- .../resource_container_node_pool.go.erb | 42 +++++++++++++++++++ 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index bd76430b7dc5..9e15be928860 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -678,7 +678,6 @@ func schemaNodeConfig() *schema.Schema { "resource_manager_tags": { Type: schema.TypeMap, Optional: true, - ForceNew: true, Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, }, <% unless version == 'ga' -%> @@ -1006,7 +1005,8 @@ func expandResourceManagerTags(v interface{}) *container.ResourceManagerTags { } return &container.ResourceManagerTags{ - Tags: rmts, + Tags: rmts, + ForceSendFields: [string]string{"Tags"}, } } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index 65b07acab155..de816ee02066 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -1441,7 +1441,6 @@ func ResourceContainerCluster() *schema.Resource { "resource_manager_tags": { Type: schema.TypeMap, Optional: true, - ForceNew: true, Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, }, }, @@ -4148,6 +4147,24 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er log.Printf("[INFO] GKE cluster %s node pool auto config network tags have been updated", d.Id()) } + if d.HasChange("node_pool_auto_config.0.resource_manager_tags") { + rmtags := d.Get("node_pool_auto_config.0.resource_manager_tags") + + req := &container.UpdateClusterRequest{ + Update: &container.ClusterUpdate{ + DesiredNodePoolAutoConfigResourceManagerTags: expandResourceManagerTags(rmtags), + }, + } + + updateF := updateFunc(req, "updating GKE cluster node pool auto config resource manager tags") + // Call update serially. + if err := transport_tpg.LockedCall(lockKey, updateF); err != nil { + return err + } + + log.Printf("[INFO] GKE cluster %s node pool auto config resource manager tags have been updated", d.Id()) + } + d.Partial(false) <% unless version == 'ga' -%> @@ -5401,7 +5418,7 @@ func expandNodePoolAutoConfig(configured interface{}) *container.NodePoolAutoCon npac.NetworkTags = expandNodePoolAutoConfigNetworkTags(v) } - if v, ok := config["resource_manager_tags"]; ok { + if v, ok := config["resource_manager_tags"]; ok && len(v.(map[string]interface{})) > 0 { npac.ResourceManagerTags = expandResourceManagerTags(v) } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index bd1d7f538029..ce72715e5ad4 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -76,7 +76,7 @@ func TestAccContainerCluster_resourceManagerTags(t *testing.T) { Config: testAccContainerCluster_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet("google_container_cluster.primary", "self_link"), - resource.TestCheckResourceAttrSet("google_container_cluster.primary", "node_config.0.resource_manager_tags"), + resource.TestCheckResourceAttrSet("google_container_cluster.primary", "node_config.0.resource_manager_tags.%"), ), }, { diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb index a4208ccfd799..025ed420fdde 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb @@ -1605,6 +1605,48 @@ func nodePoolUpdate(d *schema.ResourceData, meta interface{}, nodePoolInfo *Node log.Printf("[INFO] Updated tags for node pool %s", name) } + if d.HasChange(prefix + "node_config.0.resource_manager_tags") { + req := &container.UpdateNodePoolRequest{ + Name: name, + } + if v, ok := d.GetOk(prefix + "node_config.0.resource_manager_tags"); ok { + req.ResourceManagerTags = expandResourceManagerTags(v) + } + + // sets resource manager tags to the empty list when user removes a previously defined list of tags entriely + // aka the node pool goes from having tags to no longer having any + if req.ResourceManagerTags == nil { + tags := make(map[string]string) + rmTags := &container.ResourceManagerTags{ + Tags: tags, + } + req.ResourceManagerTags = rmTags + } + + updateF := func() error { + clusterNodePoolsUpdateCall := config.NewContainerClient(userAgent).Projects.Locations.Clusters.NodePools.Update(nodePoolInfo.fullyQualifiedName(name), req) + if config.UserProjectOverride { + clusterNodePoolsUpdateCall.Header().Add("X-Goog-User-Project", nodePoolInfo.project) + } + op, err := clusterNodePoolsUpdateCall.Do() + if err != nil { + return err + } + + // Wait until it's updated + return ContainerOperationWait(config, op, + nodePoolInfo.project, + nodePoolInfo.location, + "updating GKE node pool resource manager tags", userAgent, + timeout) + } + + if err := retryWhileIncompatibleOperation(timeout, npLockKey, updateF); err != nil { + return err + } + log.Printf("[INFO] Updated resource manager tags for node pool %s", name) + } + if d.HasChange(prefix + "node_config.0.resource_labels") { req := &container.UpdateNodePoolRequest{ Name: name, From 27108738c9ad3e84e4d473c89f2d4e2ef8e48964 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Thu, 8 Feb 2024 09:55:47 -0500 Subject: [PATCH 14/23] fixed tests --- .../services/container/node_config.go.erb | 2 +- .../resource_container_cluster.go.erb | 1 + .../resource_container_cluster_test.go.erb | 56 ++-- .../resource_container_node_pool_test.go.erb | 243 +++++++++++++++++- 4 files changed, 265 insertions(+), 37 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb index 9e15be928860..8d53ce85b51d 100644 --- a/mmv1/third_party/terraform/services/container/node_config.go.erb +++ b/mmv1/third_party/terraform/services/container/node_config.go.erb @@ -1006,7 +1006,7 @@ func expandResourceManagerTags(v interface{}) *container.ResourceManagerTags { return &container.ResourceManagerTags{ Tags: rmts, - ForceSendFields: [string]string{"Tags"}, + ForceSendFields: []string{"Tags"}, } } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index de816ee02066..07e78bce1c22 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -97,6 +97,7 @@ var ( forceNewClusterNodeConfigFields = []string{ "labels", "workload_metadata_config", + "resource_manager_tags", } suppressDiffForAutopilot = schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index ce72715e5ad4..d802963e0993 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -84,7 +84,7 @@ func TestAccContainerCluster_resourceManagerTags(t *testing.T) { ImportStateId: fmt.Sprintf("us-central1-a/%s", clusterName), ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_protection"}, + ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, }, }, }) @@ -2981,7 +2981,7 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { }) } -func TestAccContainerCluster_withAutopiloResourceManagerTags(t *testing.T) { +func TestAccContainerCluster_withAutopilotResourceManagerTags(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() @@ -2997,6 +2997,19 @@ func TestAccContainerCluster_withAutopiloResourceManagerTags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, true, ""), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "self_link"), + resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), + ), + }, + { + ResourceName: "google_container_cluster.with_autopilot", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, + }, + { + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, ""), }, { ResourceName: "google_container_cluster.with_autopilot", @@ -8597,32 +8610,32 @@ data "google_project" "project" { } resource "google_project_iam_binding" "tags" { - project = "%[1]s" - role = "roles/resourcemanager.tagHoldAdmin" - members = [ - "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - ] + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] } resource "google_tags_tag_key" "key" { - parent = "projects/%[1]s" - short_name = "foobarbaz-%[2]s" - description = "For foo/bar resources" - purpose = "GCE_FIREWALL" - purpose_data = { - network = "%[1]s/%[3]s" - } + parent = "projects/%[1]s" + short_name = "foobarbaz-%[2]s" + description = "For foo/bar resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/${google_compute_network.container_network.name}" + } } resource "google_tags_tag_value" "value" { - parent = "tagKeys/${google_tags_tag_key.key.name}" - short_name = "foo-%[2]s" - description = "For foo resources" + parent = "tagKeys/${google_tags_tag_key.key.name}" + short_name = "foo-%[2]s" + description = "For foo resources" } data "google_container_engine_versions" "uscentral1a" { location = "us-central1-a" -}`, projectID, randomSuffix, containerNetName) +}`, projectID, randomSuffix) } config += fmt.Sprintf(` @@ -8660,7 +8673,6 @@ resource "google_container_cluster" "with_autopilot" { min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] enable_autopilot = %v deletion_protection = false - min_master_version = "latest" release_channel { channel = "RAPID" } @@ -9610,12 +9622,6 @@ resource "google_container_cluster" "primary" { } } - node_pool_auto_config { - resource_manager_tags = { - "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" - } - } - deletion_protection = false network = "%[4]s" subnetwork = "%[5]s" diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index fd4292d06194..2a4c62ddbb46 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -55,12 +55,36 @@ func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccContainerNodePool_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_container_node_pool.primary_nodes", "node_config.0.resource_manager_tags.%"), + ), }, { - ResourceName: "google_container_node_pool.", + ResourceName: "google_container_node_pool.primary_nodes", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"cluster"}, + ImportStateVerifyIgnore: []string{"min_master_version", "cluster"}, + }, + { + Config: testAccContainerNodePool_resourceManagerTagsUpdate1(pid, clusterName, networkName, subnetworkName, randomSuffix), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_container_node_pool.primary_nodes", "node_config.0.resource_manager_tags.%"), + ), + }, + { + ResourceName: "google_container_node_pool.primary_nodes", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version", "cluster"}, + }, + { + Config: testAccContainerNodePool_resourceManagerTagsUpdate2(pid, clusterName, networkName, subnetworkName, randomSuffix), + }, + { + ResourceName: "google_container_node_pool.primary_nodes", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version", "cluster"}, }, }, }) @@ -4252,20 +4276,128 @@ resource "google_project_iam_binding" "tags" { ] } -resource "google_tags_tag_key" "key" { +resource "google_tags_tag_key" "key1" { + parent = "projects/%[1]s" + short_name = "foobarbaz1-%[2]s" + description = "For foo/bar1 resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } +} + +resource "google_tags_tag_value" "value1" { + parent = "tagKeys/${google_tags_tag_key.key1.name}" + short_name = "foo1-%[2]s" + description = "For foo1 resources" +} + +resource "google_tags_tag_key" "key2" { + parent = "projects/%[1]s" + short_name = "foobarbaz2-%[2]s" + description = "For foo/bar2 resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } +} + +resource "google_tags_tag_value" "value2" { + parent = "tagKeys/${google_tags_tag_key.key2.name}" + short_name = "foo2-%[2]s" + description = "For foo2 resources" +} + +data "google_container_engine_versions" "uscentral1a" { + location = "us-central1-a" +} + +resource "google_container_cluster" "primary" { + name = "%[3]s" + location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + + # We can't create a cluster with no node pool defined, but we want to only use + # separately managed node pools. So we create the smallest possible default + # node pool and immediately delete it. + remove_default_node_pool = true + initial_node_count = 1 + + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" + + timeouts { + create = "30m" + update = "40m" + } +} + +# Separately Managed Node Pool +resource "google_container_node_pool" "primary_nodes" { + name = google_container_cluster.primary.name + location = var.region + cluster = google_container_cluster.primary.name + + version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + node_count = var.gke_num_nodes + + node_config { + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 + + resource_manager_tags = { + "tagKeys/${google_tags_tag_key.key1.name}" = "tagValues/${google_tags_tag_value.value1.name}" + } + } +} +`, projectID, randomSuffix, clusterName, networkName, subnetworkName) +} + +func testAccContainerNodePool_resourceManagerTagsUpdate1(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { + return fmt.Sprintf(` +data "google_project" "project" { + project_id = "%[1]s" +} + +resource "google_project_iam_binding" "tags" { + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] +} + +resource "google_tags_tag_key" "key1" { + parent = "projects/%[1]s" + short_name = "foobarbaz1-%[2]s" + description = "For foo/bar1 resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } +} + +resource "google_tags_tag_value" "value1" { + parent = "tagKeys/${google_tags_tag_key.key1.name}" + short_name = "foo1-%[2]s" + description = "For foo1 resources" +} + +resource "google_tags_tag_key" "key2" { parent = "projects/%[1]s" - short_name = "foobarbaz-%[2]s" - description = "For foo/bar resources" + short_name = "foobarbaz2-%[2]s" + description = "For foo/bar2 resources" purpose = "GCE_FIREWALL" purpose_data = { - network = "%[1]s/%[4]s" + network = "%[1]s/%[4]s" } } -resource "google_tags_tag_value" "value" { - parent = "tagKeys/${google_tags_tag_key.key.name}" - short_name = "foo-%[2]s" - description = "For foo resources" +resource "google_tags_tag_value" "value2" { + parent = "tagKeys/${google_tags_tag_key.key2.name}" + short_name = "foo2-%[2]s" + description = "For foo2 resources" } data "google_container_engine_versions" "uscentral1a" { @@ -4307,10 +4439,99 @@ resource "google_container_node_pool" "primary_nodes" { disk_size_gb = 15 resource_manager_tags = { - "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" + "tagKeys/${google_tags_tag_key.key1.name}" = "tagValues/${google_tags_tag_value.value1.name}" + "tagKeys/${google_tags_tag_key.key2.name}" = "tagValues/${google_tags_tag_value.value2.name}" } } } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } + +func testAccContainerNodePool_resourceManagerTagsUpdate2(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { + return fmt.Sprintf(` +data "google_project" "project" { + project_id = "%[1]s" +} + +resource "google_project_iam_binding" "tags" { + project = "%[1]s" + role = "roles/resourcemanager.tagHoldAdmin" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + ] +} + +resource "google_tags_tag_key" "key1" { + parent = "projects/%[1]s" + short_name = "foobarbaz1-%[2]s" + description = "For foo/bar1 resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } +} + +resource "google_tags_tag_value" "value1" { + parent = "tagKeys/${google_tags_tag_key.key1.name}" + short_name = "foo1-%[2]s" + description = "For foo1 resources" +} + +resource "google_tags_tag_key" "key2" { + parent = "projects/%[1]s" + short_name = "foobarbaz2-%[2]s" + description = "For foo/bar2 resources" + purpose = "GCE_FIREWALL" + purpose_data = { + network = "%[1]s/%[4]s" + } +} + +resource "google_tags_tag_value" "value2" { + parent = "tagKeys/${google_tags_tag_key.key2.name}" + short_name = "foo2-%[2]s" + description = "For foo2 resources" +} + +data "google_container_engine_versions" "uscentral1a" { + location = "us-central1-a" +} + +resource "google_container_cluster" "primary" { + name = "%[3]s" + location = "us-central1-a" + min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + + # We can't create a cluster with no node pool defined, but we want to only use + # separately managed node pools. So we create the smallest possible default + # node pool and immediately delete it. + remove_default_node_pool = true + initial_node_count = 1 + + deletion_protection = false + network = "%[4]s" + subnetwork = "%[5]s" + + timeouts { + create = "30m" + update = "40m" + } +} + +# Separately Managed Node Pool +resource "google_container_node_pool" "primary_nodes" { + name = google_container_cluster.primary.name + location = var.region + cluster = google_container_cluster.primary.name + + version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] + node_count = var.gke_num_nodes + + node_config { + machine_type = "n1-standard-1" // can't be e2 because of local-ssd + disk_size_gb = 15 + } +} +`, projectID, randomSuffix, clusterName, networkName, subnetworkName) +} <% end -%> From 8b3bf5c32a8225ced8e617979d02a9beaf8fcc47 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Thu, 8 Feb 2024 11:42:10 -0500 Subject: [PATCH 15/23] fixed annotations --- .../services/container/resource_container_cluster_test.go.erb | 2 +- .../services/container/resource_container_node_pool_test.go.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index d802963e0993..c816d00f3499 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -9572,6 +9572,7 @@ func testAccContainerCluster_withWorkloadALTSConfig(projectID, name, networkName } `, projectID, networkName, subnetworkName, name, enable) } +<% end -%> func testAccContainerCluster_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` @@ -9633,4 +9634,3 @@ resource "google_container_cluster" "primary" { } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } -<% end -%> diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index 2a4c62ddbb46..45a558563b1b 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4261,6 +4261,7 @@ resource "google_container_node_pool" "without_confidential_boot_disk" { } `, cluster, networkName, subnetworkName, np) } +<% end -%> func testAccContainerNodePool_resourceManagerTags(projectID, clusterName, networkName, subnetworkName, randomSuffix string) string { return fmt.Sprintf(` @@ -4534,4 +4535,3 @@ resource "google_container_node_pool" "primary_nodes" { } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } -<% end -%> From 46920e4f9453785eff6c677bf70790682c599e9a Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 12 Feb 2024 02:31:39 -0500 Subject: [PATCH 16/23] validated clusters and node pools tests. Isolated node pool auto config --- .../resource_container_cluster.go.erb | 47 +-- .../resource_container_cluster_test.go.erb | 290 +++++++++++------- .../resource_container_node_pool_test.go.erb | 13 +- 3 files changed, 207 insertions(+), 143 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index 07e78bce1c22..c0fb7618c21e 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -1439,11 +1439,12 @@ func ResourceContainerCluster() *schema.Resource { }, }, }, - "resource_manager_tags": { - Type: schema.TypeMap, - Optional: true, - Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, - }, + // TODO uncomment for auto_pilot_config + // "resource_manager_tags": { + // Type: schema.TypeMap, + // Optional: true, + // Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, + // }, }, }, }, @@ -4148,23 +4149,24 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er log.Printf("[INFO] GKE cluster %s node pool auto config network tags have been updated", d.Id()) } - if d.HasChange("node_pool_auto_config.0.resource_manager_tags") { - rmtags := d.Get("node_pool_auto_config.0.resource_manager_tags") + // TODO uncomment for auto_pilot_config + // if d.HasChange("node_pool_auto_config.0.resource_manager_tags") { + // rmtags := d.Get("node_pool_auto_config.0.resource_manager_tags") - req := &container.UpdateClusterRequest{ - Update: &container.ClusterUpdate{ - DesiredNodePoolAutoConfigResourceManagerTags: expandResourceManagerTags(rmtags), - }, - } + // req := &container.UpdateClusterRequest{ + // Update: &container.ClusterUpdate{ + // DesiredNodePoolAutoConfigResourceManagerTags: expandResourceManagerTags(rmtags), + // }, + // } - updateF := updateFunc(req, "updating GKE cluster node pool auto config resource manager tags") - // Call update serially. - if err := transport_tpg.LockedCall(lockKey, updateF); err != nil { - return err - } + // updateF := updateFunc(req, "updating GKE cluster node pool auto config resource manager tags") + // // Call update serially. + // if err := transport_tpg.LockedCall(lockKey, updateF); err != nil { + // return err + // } - log.Printf("[INFO] GKE cluster %s node pool auto config resource manager tags have been updated", d.Id()) - } + // log.Printf("[INFO] GKE cluster %s node pool auto config resource manager tags have been updated", d.Id()) + // } d.Partial(false) @@ -5419,9 +5421,10 @@ func expandNodePoolAutoConfig(configured interface{}) *container.NodePoolAutoCon npac.NetworkTags = expandNodePoolAutoConfigNetworkTags(v) } - if v, ok := config["resource_manager_tags"]; ok && len(v.(map[string]interface{})) > 0 { - npac.ResourceManagerTags = expandResourceManagerTags(v) - } + // TODO uncomment for auto_pilot_config + // if v, ok := config["resource_manager_tags"]; ok && len(v.(map[string]interface{})) > 0 { + // npac.ResourceManagerTags = expandResourceManagerTags(v) + // } return npac } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index c816d00f3499..6c7621cbbe4c 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -2873,9 +2873,8 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - randomSuffix := acctest.RandString(t, 10) - containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) - clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) + clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -2883,7 +2882,7 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, ""), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "networking_mode", "VPC_NATIVE"), ), @@ -2902,10 +2901,9 @@ func TestAccContainerClusterCustomServiceAccount_withAutopilot(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - randomSuffix := acctest.RandString(t, 10) - containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) - clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) - serviceAccountName := fmt.Sprintf("tf-test-sa-%s", randomSuffix) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) + clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) + serviceAccountName := fmt.Sprintf("tf-test-sa-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -2913,7 +2911,7 @@ func TestAccContainerClusterCustomServiceAccount_withAutopilot(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, serviceAccountName), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, serviceAccountName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "cluster_autoscaling.0.enabled", "true"), @@ -2938,9 +2936,8 @@ func TestAccContainerCluster_errorAutopilotLocation(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - randomSuffix := acctest.RandString(t, 10) - containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) - clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) + clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -2948,7 +2945,7 @@ func TestAccContainerCluster_errorAutopilotLocation(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", randomSuffix, true, false, false, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1-a", true, false, ""), ExpectError: regexp.MustCompile(`Autopilot clusters must be regional clusters.`), }, }, @@ -2959,9 +2956,8 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { t.Parallel() pid := envvar.GetTestProjectFromEnv() - randomSuffix := acctest.RandString(t, 10) - containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) - clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + containerNetName := fmt.Sprintf("tf-test-container-net-%s", acctest.RandString(t, 10)) + clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -2969,7 +2965,7 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, true, false, ""), + Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, true, ""), }, { ResourceName: "google_container_cluster.with_autopilot", @@ -2981,45 +2977,58 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { }) } -func TestAccContainerCluster_withAutopilotResourceManagerTags(t *testing.T) { - t.Parallel() - - pid := envvar.GetTestProjectFromEnv() - - randomSuffix := acctest.RandString(t, 10) - containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) - clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) - - acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), - Steps: []resource.TestStep{ - { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, true, ""), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "self_link"), - resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), - ), - }, - { - ResourceName: "google_container_cluster.with_autopilot", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, - }, - { - Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", randomSuffix, true, false, false, ""), - }, - { - ResourceName: "google_container_cluster.with_autopilot", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, - }, - }, - }) -} +// TODO uncomment for auto_pilot_config +// func TestAccContainerCluster_withAutopilotResourceManagerTags(t *testing.T) { +// t.Parallel() + +// pid := envvar.GetTestProjectFromEnv() + +// randomSuffix := acctest.RandString(t, 10) +// containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) +// clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) + +// acctest.VcrTest(t, resource.TestCase{ +// PreCheck: func() { acctest.AccTestPreCheck(t) }, +// ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), +// CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), +// Steps: []resource.TestStep{ +// { +// Config: testAccContainerCluster_withAutopilotResourceManagerTags(pid, containerNetName, clusterName, "us-central1", randomSuffix), +// Check: resource.ComposeTestCheckFunc( +// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "self_link"), +// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), +// ), +// }, +// { +// ResourceName: "google_container_cluster.with_autopilot", +// ImportState: true, +// ImportStateVerify: true, +// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, +// }, +// { +// Config: testAccContainerCluster_withAutopilotResourceManagerTagsUpdate1(pid, containerNetName, clusterName, "us-central1", randomSuffix), +// Check: resource.ComposeTestCheckFunc( +// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), +// ), +// }, +// { +// ResourceName: "google_container_cluster.with_autopilot", +// ImportState: true, +// ImportStateVerify: true, +// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, +// }, +// { +// Config: testAccContainerCluster_withAutopilotResourceManagerTagsUpdate2(pid, containerNetName, clusterName, "us-central1", randomSuffix), +// }, +// { +// ResourceName: "google_container_cluster.with_autopilot", +// ImportState: true, +// ImportStateVerify: true, +// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, +// }, +// }, +// }) +// } func TestAccContainerCluster_withWorkloadIdentityConfig(t *testing.T) { t.Parallel() @@ -8575,7 +8584,7 @@ resource "google_container_cluster" "primary" { `, name) } -func testAccContainerCluster_withAutopilot(projectID, containerNetName, clusterName, location, randomSuffix string, enabled, withNetworkTag, withResourceManagerTag bool, serviceAccount string) string { +func testAccContainerCluster_withAutopilot(projectID string, containerNetName string, clusterName string, location string, enabled bool, withNetworkTag bool, serviceAccount string) string { config := "" clusterAutoscaling := "" if serviceAccount != "" { @@ -8603,41 +8612,6 @@ resource "google_project_iam_binding" "project" { }`, serviceAccount, projectID) } - if withResourceManagerTag { - config += fmt.Sprintf(` -data "google_project" "project" { - project_id = "%[1]s" -} - -resource "google_project_iam_binding" "tags" { - project = "%[1]s" - role = "roles/resourcemanager.tagHoldAdmin" - members = [ - "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - ] -} - -resource "google_tags_tag_key" "key" { - parent = "projects/%[1]s" - short_name = "foobarbaz-%[2]s" - description = "For foo/bar resources" - purpose = "GCE_FIREWALL" - purpose_data = { - network = "%[1]s/${google_compute_network.container_network.name}" - } -} - -resource "google_tags_tag_value" "value" { - parent = "tagKeys/${google_tags_tag_key.key.name}" - short_name = "foo-%[2]s" - description = "For foo resources" -} - -data "google_container_engine_versions" "uscentral1a" { - location = "us-central1-a" -}`, projectID, randomSuffix) - } - config += fmt.Sprintf(` resource "google_compute_network" "container_network" { @@ -8670,9 +8644,9 @@ data "google_container_engine_versions" "central1a" { resource "google_container_cluster" "with_autopilot" { name = "%s" location = "%s" - min_master_version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] enable_autopilot = %v deletion_protection = false + min_master_version = "latest" release_channel { channel = "RAPID" } @@ -8691,33 +8665,16 @@ resource "google_container_cluster" "with_autopilot" { vertical_pod_autoscaling { enabled = true }`, containerNetName, clusterName, location, enabled, clusterAutoscaling) - - if withNetworkTag || withResourceManagerTag { - config += ` - node_pool_auto_config {` - } - if withNetworkTag { config += ` + node_pool_auto_config { network_tags { tags = ["test-network-tag"] - }` - } - - if withResourceManagerTag { - config += ` - resource_manager_tags = { - "tagKeys/${google_tags_tag_key.key.name}" = "tagValues/${google_tags_tag_value.value.name}" - }` - } - - if withNetworkTag || withResourceManagerTag { - config += ` - }` // closing bracket "node_pool_auto_config" + } + }` } - config += ` -}` // closing bracket "google_container_cluster" +}` return config } @@ -9634,3 +9591,108 @@ resource "google_container_cluster" "primary" { } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } + +// TODO uncomment for auto_pilot_config +// func testAccContainerCluster_withAutopilotResourceManagerTags(projectID, containerNetName, clusterName, location, randomSuffix string) string { +// config := "" +// clusterAutoscaling := "" +// if serviceAccount != "" { +// config += fmt.Sprintf(` +// resource "google_service_account" "service_account" { +// account_id = "%[1]s" +// project = "%[2]s" +// display_name = "Service Account" +// } + +// resource "google_project_iam_binding" "project" { +// project = "%[2]s" +// role = "roles/container.nodeServiceAccount" +// members = [ +// "serviceAccount:%[1]s@%[2]s.iam.gserviceaccount.com", +// ] +// }`, serviceAccount, projectID) + +// clusterAutoscaling = fmt.Sprintf(` +// cluster_autoscaling { +// auto_provisioning_defaults { +// service_account = "%s@%s.iam.gserviceaccount.com" +// oauth_scopes = ["https://www.googleapis.com/auth/cloud-platform"] +// } +// }`, serviceAccount, projectID) +// } + +// config += fmt.Sprintf(` + +// resource "google_compute_network" "container_network" { +// name = "%s" +// auto_create_subnetworks = false +// } + +// resource "google_compute_subnetwork" "container_subnetwork" { +// name = google_compute_network.container_network.name +// network = google_compute_network.container_network.name +// ip_cidr_range = "10.0.36.0/24" +// region = "us-central1" +// private_ip_google_access = true + +// secondary_ip_range { +// range_name = "pod" +// ip_cidr_range = "10.0.0.0/19" +// } + +// secondary_ip_range { +// range_name = "svc" +// ip_cidr_range = "10.0.32.0/22" +// } +// } + +// data "google_container_engine_versions" "central1a" { +// location = "us-central1-a" +// } + +// resource "google_container_cluster" "with_autopilot" { +// name = "%s" +// location = "%s" +// enable_autopilot = %v +// deletion_protection = false +// min_master_version = "latest" +// release_channel { +// channel = "RAPID" +// } +// network = google_compute_network.container_network.name +// subnetwork = google_compute_subnetwork.container_subnetwork.name +// ip_allocation_policy { +// cluster_secondary_range_name = google_compute_subnetwork.container_subnetwork.secondary_ip_range[0].range_name +// services_secondary_range_name = google_compute_subnetwork.container_subnetwork.secondary_ip_range[1].range_name +// } +// addons_config { +// horizontal_pod_autoscaling { +// disabled = false +// } +// } +// %s +// vertical_pod_autoscaling { +// enabled = true +// }`, containerNetName, clusterName, location, enabled, clusterAutoscaling) +// if withNetworkTag { +// config += ` +// node_pool_auto_config { +// network_tags { +// tags = ["test-network-tag"] +// } +// }` +// } +// config += ` +// }` +// return config +// } + +// func testAccContainerCluster_withAutopilotResourceManagerTagsUpdate1(projectID, containerNetName, clusterName, location, randomSuffix string) string { +// return fmt.Sprintf(` +// `) +// } + +// func testAccContainerCluster_withAutopilotResourceManagerTagsUpdate2(projectID, containerNetName, clusterName, location, randomSuffix string) string { +// return fmt.Sprintf(` +// `) +// } diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index 45a558563b1b..d4f592a54f49 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -90,7 +90,6 @@ func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { }) } - func TestAccContainerNodePool_basicWithClusterId(t *testing.T) { t.Parallel() @@ -4337,11 +4336,11 @@ resource "google_container_cluster" "primary" { # Separately Managed Node Pool resource "google_container_node_pool" "primary_nodes" { name = google_container_cluster.primary.name - location = var.region + location = "us-central1-a" cluster = google_container_cluster.primary.name version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] - node_count = var.gke_num_nodes + node_count = 1 node_config { machine_type = "n1-standard-1" // can't be e2 because of local-ssd @@ -4429,11 +4428,11 @@ resource "google_container_cluster" "primary" { # Separately Managed Node Pool resource "google_container_node_pool" "primary_nodes" { name = google_container_cluster.primary.name - location = var.region + location = "us-central1-a" cluster = google_container_cluster.primary.name version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] - node_count = var.gke_num_nodes + node_count = 1 node_config { machine_type = "n1-standard-1" // can't be e2 because of local-ssd @@ -4522,11 +4521,11 @@ resource "google_container_cluster" "primary" { # Separately Managed Node Pool resource "google_container_node_pool" "primary_nodes" { name = google_container_cluster.primary.name - location = var.region + location = "us-central1-a" cluster = google_container_cluster.primary.name version = data.google_container_engine_versions.uscentral1a.release_channel_latest_version["STABLE"] - node_count = var.gke_num_nodes + node_count = 1 node_config { machine_type = "n1-standard-1" // can't be e2 because of local-ssd From 0ffc6b98f923f400280f47a5e314c18d94603303 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 12 Feb 2024 02:58:44 -0500 Subject: [PATCH 17/23] isolated resource manager tags from docs --- .../terraform/website/docs/r/container_cluster.html.markdown | 2 -- 1 file changed, 2 deletions(-) diff --git a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown index 479d4837078c..9bff6ce9e01f 100644 --- a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -1034,8 +1034,6 @@ workload_identity_config { The `node_pool_auto_config` block supports: -* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}`. - * `network_tags` (Optional) - The network tag config for the cluster's automatically provisioned node pools. The `network_tags` block supports: From 3f31e7a716009be490a7f072e715c58fc6610b07 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Wed, 14 Feb 2024 11:32:33 -0500 Subject: [PATCH 18/23] fixed permission issue --- .../resource_container_cluster_test.go.erb | 27 ++++++- .../resource_container_node_pool_test.go.erb | 72 +++++++++++++++++-- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 6c7621cbbe4c..778871a34170 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -70,7 +70,10 @@ func TestAccContainerCluster_resourceManagerTags(t *testing.T) { acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + ExternalProviders: map[string]resource.ExternalProvider{ + "time": {}, + }, + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { Config: testAccContainerCluster_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), @@ -9537,7 +9540,7 @@ data "google_project" "project" { project_id = "%[1]s" } -resource "google_project_iam_binding" "tags" { +resource "google_project_iam_binding" "tagHoldAdmin" { project = "%[1]s" role = "roles/resourcemanager.tagHoldAdmin" members = [ @@ -9545,6 +9548,24 @@ resource "google_project_iam_binding" "tags" { ] } +resource "google_project_iam_binding" "tagUser" { + project = "%[1]s" + role = "roles/resourcemanager.tagUser" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + ] +} + +resource "time_sleep" "wait_120_seconds" { + create_duration = "300s" + + depends_on = [ + google_project_iam_binding.tagHoldAdmin, + google_project_iam_binding.tagUser + ] +} + resource "google_tags_tag_key" "key" { parent = "projects/%[1]s" short_name = "foobarbaz-%[2]s" @@ -9588,6 +9609,8 @@ resource "google_container_cluster" "primary" { create = "30m" update = "40m" } + + depends_on = [time_sleep.wait_120_seconds] } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index d4f592a54f49..e3b938174ec4 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -39,7 +39,6 @@ func TestAccContainerNodePool_basic(t *testing.T) { func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { t.Parallel() - pid := envvar.GetTestProjectFromEnv() randomSuffix := acctest.RandString(t, 10) @@ -51,7 +50,10 @@ func TestAccContainerNodePool_resourceManagerTags(t *testing.T) { acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + ExternalProviders: map[string]resource.ExternalProvider{ + "time": {}, + }, + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), Steps: []resource.TestStep{ { Config: testAccContainerNodePool_resourceManagerTags(pid, clusterName, networkName, subnetworkName, randomSuffix), @@ -4268,7 +4270,7 @@ data "google_project" "project" { project_id = "%[1]s" } -resource "google_project_iam_binding" "tags" { +resource "google_project_iam_binding" "tagHoldAdmin" { project = "%[1]s" role = "roles/resourcemanager.tagHoldAdmin" members = [ @@ -4276,6 +4278,24 @@ resource "google_project_iam_binding" "tags" { ] } +resource "google_project_iam_binding" "tagUser" { + project = "%[1]s" + role = "roles/resourcemanager.tagUser" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + ] +} + +resource "time_sleep" "wait_120_seconds" { + create_duration = "300s" + + depends_on = [ + google_project_iam_binding.tagHoldAdmin, + google_project_iam_binding.tagUser + ] +} + resource "google_tags_tag_key" "key1" { parent = "projects/%[1]s" short_name = "foobarbaz1-%[2]s" @@ -4331,6 +4351,8 @@ resource "google_container_cluster" "primary" { create = "30m" update = "40m" } + + depends_on = [time_sleep.wait_120_seconds] } # Separately Managed Node Pool @@ -4360,7 +4382,7 @@ data "google_project" "project" { project_id = "%[1]s" } -resource "google_project_iam_binding" "tags" { +resource "google_project_iam_binding" "tagHoldAdmin" { project = "%[1]s" role = "roles/resourcemanager.tagHoldAdmin" members = [ @@ -4368,6 +4390,24 @@ resource "google_project_iam_binding" "tags" { ] } +resource "google_project_iam_binding" "tagUser" { + project = "%[1]s" + role = "roles/resourcemanager.tagUser" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + ] +} + +resource "time_sleep" "wait_120_seconds" { + create_duration = "300s" + + depends_on = [ + google_project_iam_binding.tagHoldAdmin, + google_project_iam_binding.tagUser + ] +} + resource "google_tags_tag_key" "key1" { parent = "projects/%[1]s" short_name = "foobarbaz1-%[2]s" @@ -4423,6 +4463,8 @@ resource "google_container_cluster" "primary" { create = "30m" update = "40m" } + + depends_on = [time_sleep.wait_120_seconds] } # Separately Managed Node Pool @@ -4453,7 +4495,7 @@ data "google_project" "project" { project_id = "%[1]s" } -resource "google_project_iam_binding" "tags" { +resource "google_project_iam_binding" "tagHoldAdmin" { project = "%[1]s" role = "roles/resourcemanager.tagHoldAdmin" members = [ @@ -4461,6 +4503,24 @@ resource "google_project_iam_binding" "tags" { ] } +resource "google_project_iam_binding" "tagUser" { + project = "%[1]s" + role = "roles/resourcemanager.tagUser" + members = [ + "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + ] +} + +resource "time_sleep" "wait_120_seconds" { + create_duration = "300s" + + depends_on = [ + google_project_iam_binding.tagHoldAdmin, + google_project_iam_binding.tagUser + ] +} + resource "google_tags_tag_key" "key1" { parent = "projects/%[1]s" short_name = "foobarbaz1-%[2]s" @@ -4516,6 +4576,8 @@ resource "google_container_cluster" "primary" { create = "30m" update = "40m" } + + depends_on = [time_sleep.wait_120_seconds] } # Separately Managed Node Pool From 0a5cc412cc860f395d85f18905d95213eb10f014 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Wed, 14 Feb 2024 11:43:12 -0500 Subject: [PATCH 19/23] fixed spaces --- .../container/resource_container_cluster_test.go.erb | 2 +- .../container/resource_container_node_pool_test.go.erb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 778871a34170..a3b281b81328 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -9553,7 +9553,7 @@ resource "google_project_iam_binding" "tagUser" { role = "roles/resourcemanager.tagUser" members = [ "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] } diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index e3b938174ec4..a5782d284c0f 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4283,7 +4283,7 @@ resource "google_project_iam_binding" "tagUser" { role = "roles/resourcemanager.tagUser" members = [ "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] } @@ -4395,7 +4395,7 @@ resource "google_project_iam_binding" "tagUser" { role = "roles/resourcemanager.tagUser" members = [ "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] } @@ -4508,7 +4508,7 @@ resource "google_project_iam_binding" "tagUser" { role = "roles/resourcemanager.tagUser" members = [ "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", - "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", + "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] } From 06f11a7bef79b7b8742de1d9fe90c142426112d2 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Thu, 15 Feb 2024 11:14:28 -0500 Subject: [PATCH 20/23] fixed non determinism on tag keys --- .../container/resource_container_node_pool_test.go.erb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index a5782d284c0f..86450bae1fa2 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4320,6 +4320,8 @@ resource "google_tags_tag_key" "key2" { purpose_data = { network = "%[1]s/%[4]s" } + + depends_on = [google_tags_tag_key.key1] } resource "google_tags_tag_value" "value2" { @@ -4432,6 +4434,8 @@ resource "google_tags_tag_key" "key2" { purpose_data = { network = "%[1]s/%[4]s" } + + depends_on = [google_tags_tag_key.key1] } resource "google_tags_tag_value" "value2" { @@ -4545,6 +4549,8 @@ resource "google_tags_tag_key" "key2" { purpose_data = { network = "%[1]s/%[4]s" } + + depends_on = [google_tags_tag_key.key1] } resource "google_tags_tag_value" "value2" { From 10bc6be738d81b531170c54db673720cf7aa9e7f Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 19 Feb 2024 08:49:37 -0500 Subject: [PATCH 21/23] removed auto_pilot rmts --- .../resource_container_cluster.go.erb | 30 ---- .../resource_container_cluster_test.go.erb | 158 ------------------ 2 files changed, 188 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb index c0fb7618c21e..dff096a832ce 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb @@ -1439,12 +1439,6 @@ func ResourceContainerCluster() *schema.Resource { }, }, }, - // TODO uncomment for auto_pilot_config - // "resource_manager_tags": { - // Type: schema.TypeMap, - // Optional: true, - // Description: `A map of resource manager tags. Resource manager tag keys and values have the same definition as resource manager tags. Keys must be in the format tagKeys/{tag_key_id}, and values are in the format tagValues/456. The field is ignored (both PUT & PATCH) when empty.`, - // }, }, }, }, @@ -4149,25 +4143,6 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er log.Printf("[INFO] GKE cluster %s node pool auto config network tags have been updated", d.Id()) } - // TODO uncomment for auto_pilot_config - // if d.HasChange("node_pool_auto_config.0.resource_manager_tags") { - // rmtags := d.Get("node_pool_auto_config.0.resource_manager_tags") - - // req := &container.UpdateClusterRequest{ - // Update: &container.ClusterUpdate{ - // DesiredNodePoolAutoConfigResourceManagerTags: expandResourceManagerTags(rmtags), - // }, - // } - - // updateF := updateFunc(req, "updating GKE cluster node pool auto config resource manager tags") - // // Call update serially. - // if err := transport_tpg.LockedCall(lockKey, updateF); err != nil { - // return err - // } - - // log.Printf("[INFO] GKE cluster %s node pool auto config resource manager tags have been updated", d.Id()) - // } - d.Partial(false) <% unless version == 'ga' -%> @@ -5421,11 +5396,6 @@ func expandNodePoolAutoConfig(configured interface{}) *container.NodePoolAutoCon npac.NetworkTags = expandNodePoolAutoConfigNetworkTags(v) } - // TODO uncomment for auto_pilot_config - // if v, ok := config["resource_manager_tags"]; ok && len(v.(map[string]interface{})) > 0 { - // npac.ResourceManagerTags = expandResourceManagerTags(v) - // } - return npac } diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index a3b281b81328..03de5767da25 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -2980,59 +2980,6 @@ func TestAccContainerCluster_withAutopilotNetworkTags(t *testing.T) { }) } -// TODO uncomment for auto_pilot_config -// func TestAccContainerCluster_withAutopilotResourceManagerTags(t *testing.T) { -// t.Parallel() - -// pid := envvar.GetTestProjectFromEnv() - -// randomSuffix := acctest.RandString(t, 10) -// containerNetName := fmt.Sprintf("tf-test-container-net-%s", randomSuffix) -// clusterName := fmt.Sprintf("tf-test-cluster-%s", randomSuffix) - -// acctest.VcrTest(t, resource.TestCase{ -// PreCheck: func() { acctest.AccTestPreCheck(t) }, -// ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), -// CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), -// Steps: []resource.TestStep{ -// { -// Config: testAccContainerCluster_withAutopilotResourceManagerTags(pid, containerNetName, clusterName, "us-central1", randomSuffix), -// Check: resource.ComposeTestCheckFunc( -// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "self_link"), -// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), -// ), -// }, -// { -// ResourceName: "google_container_cluster.with_autopilot", -// ImportState: true, -// ImportStateVerify: true, -// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, -// }, -// { -// Config: testAccContainerCluster_withAutopilotResourceManagerTagsUpdate1(pid, containerNetName, clusterName, "us-central1", randomSuffix), -// Check: resource.ComposeTestCheckFunc( -// resource.TestCheckResourceAttrSet("google_container_cluster.with_autopilot", "node_pool_auto_config.0.resource_manager_tags.%"), -// ), -// }, -// { -// ResourceName: "google_container_cluster.with_autopilot", -// ImportState: true, -// ImportStateVerify: true, -// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, -// }, -// { -// Config: testAccContainerCluster_withAutopilotResourceManagerTagsUpdate2(pid, containerNetName, clusterName, "us-central1", randomSuffix), -// }, -// { -// ResourceName: "google_container_cluster.with_autopilot", -// ImportState: true, -// ImportStateVerify: true, -// ImportStateVerifyIgnore: []string{"min_master_version", "deletion_protection"}, -// }, -// }, -// }) -// } - func TestAccContainerCluster_withWorkloadIdentityConfig(t *testing.T) { t.Parallel() @@ -9614,108 +9561,3 @@ resource "google_container_cluster" "primary" { } `, projectID, randomSuffix, clusterName, networkName, subnetworkName) } - -// TODO uncomment for auto_pilot_config -// func testAccContainerCluster_withAutopilotResourceManagerTags(projectID, containerNetName, clusterName, location, randomSuffix string) string { -// config := "" -// clusterAutoscaling := "" -// if serviceAccount != "" { -// config += fmt.Sprintf(` -// resource "google_service_account" "service_account" { -// account_id = "%[1]s" -// project = "%[2]s" -// display_name = "Service Account" -// } - -// resource "google_project_iam_binding" "project" { -// project = "%[2]s" -// role = "roles/container.nodeServiceAccount" -// members = [ -// "serviceAccount:%[1]s@%[2]s.iam.gserviceaccount.com", -// ] -// }`, serviceAccount, projectID) - -// clusterAutoscaling = fmt.Sprintf(` -// cluster_autoscaling { -// auto_provisioning_defaults { -// service_account = "%s@%s.iam.gserviceaccount.com" -// oauth_scopes = ["https://www.googleapis.com/auth/cloud-platform"] -// } -// }`, serviceAccount, projectID) -// } - -// config += fmt.Sprintf(` - -// resource "google_compute_network" "container_network" { -// name = "%s" -// auto_create_subnetworks = false -// } - -// resource "google_compute_subnetwork" "container_subnetwork" { -// name = google_compute_network.container_network.name -// network = google_compute_network.container_network.name -// ip_cidr_range = "10.0.36.0/24" -// region = "us-central1" -// private_ip_google_access = true - -// secondary_ip_range { -// range_name = "pod" -// ip_cidr_range = "10.0.0.0/19" -// } - -// secondary_ip_range { -// range_name = "svc" -// ip_cidr_range = "10.0.32.0/22" -// } -// } - -// data "google_container_engine_versions" "central1a" { -// location = "us-central1-a" -// } - -// resource "google_container_cluster" "with_autopilot" { -// name = "%s" -// location = "%s" -// enable_autopilot = %v -// deletion_protection = false -// min_master_version = "latest" -// release_channel { -// channel = "RAPID" -// } -// network = google_compute_network.container_network.name -// subnetwork = google_compute_subnetwork.container_subnetwork.name -// ip_allocation_policy { -// cluster_secondary_range_name = google_compute_subnetwork.container_subnetwork.secondary_ip_range[0].range_name -// services_secondary_range_name = google_compute_subnetwork.container_subnetwork.secondary_ip_range[1].range_name -// } -// addons_config { -// horizontal_pod_autoscaling { -// disabled = false -// } -// } -// %s -// vertical_pod_autoscaling { -// enabled = true -// }`, containerNetName, clusterName, location, enabled, clusterAutoscaling) -// if withNetworkTag { -// config += ` -// node_pool_auto_config { -// network_tags { -// tags = ["test-network-tag"] -// } -// }` -// } -// config += ` -// }` -// return config -// } - -// func testAccContainerCluster_withAutopilotResourceManagerTagsUpdate1(projectID, containerNetName, clusterName, location, randomSuffix string) string { -// return fmt.Sprintf(` -// `) -// } - -// func testAccContainerCluster_withAutopilotResourceManagerTagsUpdate2(projectID, containerNetName, clusterName, location, randomSuffix string) string { -// return fmt.Sprintf(` -// `) -// } From 9fc074bd3d4920d4c26993a00b291a7ed331808d Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Mon, 19 Feb 2024 08:55:23 -0500 Subject: [PATCH 22/23] fixed time_sleep --- .../container/resource_container_cluster_test.go.erb | 2 +- .../container/resource_container_node_pool_test.go.erb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index 03de5767da25..ff7e8655a6a1 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -9505,7 +9505,7 @@ resource "google_project_iam_binding" "tagUser" { } resource "time_sleep" "wait_120_seconds" { - create_duration = "300s" + create_duration = "120s" depends_on = [ google_project_iam_binding.tagHoldAdmin, diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index 86450bae1fa2..51c349096390 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4288,7 +4288,7 @@ resource "google_project_iam_binding" "tagUser" { } resource "time_sleep" "wait_120_seconds" { - create_duration = "300s" + create_duration = "120s" depends_on = [ google_project_iam_binding.tagHoldAdmin, @@ -4402,7 +4402,7 @@ resource "google_project_iam_binding" "tagUser" { } resource "time_sleep" "wait_120_seconds" { - create_duration = "300s" + create_duration = "120s" depends_on = [ google_project_iam_binding.tagHoldAdmin, @@ -4517,7 +4517,7 @@ resource "google_project_iam_binding" "tagUser" { } resource "time_sleep" "wait_120_seconds" { - create_duration = "300s" + create_duration = "120s" depends_on = [ google_project_iam_binding.tagHoldAdmin, From b863ab3c7a44573cd93a20a7ddc9aae8b2c098f9 Mon Sep 17 00:00:00 2001 From: "Max W. Portocarrero" Date: Tue, 20 Feb 2024 20:48:48 -0500 Subject: [PATCH 23/23] add depends_on to IAM policies --- .../container/resource_container_cluster_test.go.erb | 2 ++ .../container/resource_container_node_pool_test.go.erb | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb index d29ab75db60b..a542384379f6 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb @@ -9620,6 +9620,8 @@ resource "google_project_iam_binding" "tagUser" { "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] + + depends_on = [google_project_iam_binding.tagHoldAdmin] } resource "time_sleep" "wait_120_seconds" { diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb index 51c349096390..42939e9b7e9b 100644 --- a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb +++ b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb @@ -4285,6 +4285,8 @@ resource "google_project_iam_binding" "tagUser" { "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] + + depends_on = [google_project_iam_binding.tagHoldAdmin] } resource "time_sleep" "wait_120_seconds" { @@ -4399,6 +4401,8 @@ resource "google_project_iam_binding" "tagUser" { "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] + + depends_on = [google_project_iam_binding.tagHoldAdmin] } resource "time_sleep" "wait_120_seconds" { @@ -4514,6 +4518,8 @@ resource "google_project_iam_binding" "tagUser" { "serviceAccount:service-${data.google_project.project.number}@container-engine-robot.iam.gserviceaccount.com", "serviceAccount:${data.google_project.project.number}@cloudservices.gserviceaccount.com", ] + + depends_on = [google_project_iam_binding.tagHoldAdmin] } resource "time_sleep" "wait_120_seconds" {