From 22c5a0adebe34be1f8044c28449aae28df53bd5e Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Thu, 14 May 2020 19:11:01 -0700 Subject: [PATCH 1/4] update of node pool workload metadata --- .../resource_container_cluster.go.erb | 32 +- .../resource_container_node_pool.go.erb | 35 +- .../resource_container_node_pool_test.go.erb | 19 +- .../terraform/utils/node_config.go.erb | 420 +++++++++--------- 4 files changed, 284 insertions(+), 222 deletions(-) diff --git a/third_party/terraform/resources/resource_container_cluster.go.erb b/third_party/terraform/resources/resource_container_cluster.go.erb index 805b4940b98c..2156ce65e1bb 100644 --- a/third_party/terraform/resources/resource_container_cluster.go.erb +++ b/third_party/terraform/resources/resource_container_cluster.go.erb @@ -62,8 +62,38 @@ var ( "addons_config.0.config_connector_config", <% end -%> } + + forceNewClusterNodeConfigFields = []string{ + <% unless version == 'ga' -%> + "workload_metadata_config", + <% end -%> + } ) +// This uses the node pool nodeConfig schema but sets +// node-pool-only updatable fields to ForceNew +func clusterSchemaNodeConfig() *schema.Schema { + nodeConfigSch := schemaNodeConfig() + schemaMap := nodeConfigSch.Elem.(*schema.Resource).Schema + for _, k := range forceNewClusterNodeConfigFields { + if sch, ok := schemaMap[k]; ok { + changeFieldSchemaToForceNew(sch) + } + } + return nodeConfigSch +} + +func changeFieldSchemaToForceNew(sch *schema.Schema) { + sch.ForceNew = true + if sch.Type == schema.TypeList { + if nestedR, ok := sch.Elem.(*schema.Resource); ok { + for _, nestedSch := range nestedR.Schema { + changeFieldSchemaToForceNew(nestedSch) + } + } + } +} + func rfc5545RecurrenceDiffSuppress(k, o, n string, d *schema.ResourceData) bool { // This diff gets applied in the cloud console if you specify // "FREQ=DAILY" in your config and add a maintenance exclusion. @@ -663,7 +693,7 @@ func resourceContainerCluster() *schema.Resource { }, }, - "node_config": schemaNodeConfig, + "node_config": clusterSchemaNodeConfig(), "node_pool": { Type: schema.TypeList, diff --git a/third_party/terraform/resources/resource_container_node_pool.go.erb b/third_party/terraform/resources/resource_container_node_pool.go.erb index de22d19c3512..e8ccbbe9c94a 100644 --- a/third_party/terraform/resources/resource_container_node_pool.go.erb +++ b/third_party/terraform/resources/resource_container_node_pool.go.erb @@ -181,7 +181,7 @@ var schemaNodePool = map[string]*schema.Schema{ ForceNew: true, }, - "node_config": schemaNodeConfig, + "node_config": schemaNodeConfig(), "node_count": { Type: schema.TypeInt, @@ -711,7 +711,40 @@ func nodePoolUpdate(d *schema.ResourceData, meta interface{}, nodePoolInfo *Node log.Printf("[INFO] Updated image type in Node Pool %s", d.Id()) } +<% unless version == 'ga' -%> + if d.HasChange(prefix + "node_config.0.workload_metadata_config") { + req := &containerBeta.UpdateNodePoolRequest{ + NodePoolId: name, + WorkloadMetadataConfig: expandWorkloadMetadataConfig( + d.Get(prefix + "node_config.0.workload_metadata_config")), + } + if req.WorkloadMetadataConfig == nil { + req.ForceSendFields = []string{"WorkloadMetadataConfig"} + } + updateF := func() error { + op, err := config.clientContainerBeta.Projects.Locations.Clusters.NodePools. + Update(nodePoolInfo.fullyQualifiedName(name), req).Do() + if err != nil { + return err + } + + // Wait until it's updated + return containerOperationWait(config, op, + nodePoolInfo.project, + nodePoolInfo.location, + "updating GKE node pool workload_metadata_config", + timeout) + } + + // Call update serially. + if err := lockedCall(lockKey, updateF); err != nil { + return err + } + + log.Printf("[INFO] Updated workload_metadata_config for node pool %s", name) + } +<% end -%> if prefix == "" { d.SetPartial("node_config") } diff --git a/third_party/terraform/tests/resource_container_node_pool_test.go.erb b/third_party/terraform/tests/resource_container_node_pool_test.go.erb index 6d83b8cc3b33..da83bd1fef9c 100644 --- a/third_party/terraform/tests/resource_container_node_pool_test.go.erb +++ b/third_party/terraform/tests/resource_container_node_pool_test.go.erb @@ -167,9 +167,10 @@ func TestAccContainerNodePool_withNodeConfig(t *testing.T) { } <% unless version.nil? || version == 'ga' -%> -func TestAccContainerNodePool_withWorkloadMetadataConfig(t *testing.T) { +func TestAccContainerNodePool_withWorkloadIdentityConfig(t *testing.T) { t.Parallel() + pid := getTestProjectFromEnv() cluster := fmt.Sprintf("tf-test-cluster-%s", randString(t, 10)) np := fmt.Sprintf("tf-test-np-%s", randString(t, 10)) @@ -195,22 +196,6 @@ func TestAccContainerNodePool_withWorkloadMetadataConfig(t *testing.T) { "node_config.0.workload_metadata_config.0.node_metadata", }, }, - }, - }) -} - -func TestAccContainerNodePool_withWorkloadIdentityConfig(t *testing.T) { - t.Parallel() - - pid := getTestProjectFromEnv() - cluster := fmt.Sprintf("tf-test-cluster-%s", randString(t, 10)) - np := fmt.Sprintf("tf-test-np-%s", randString(t, 10)) - - vcrTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), - Steps: []resource.TestStep{ { Config: testAccContainerNodePool_withWorkloadMetadataConfig_gkeMetadataServer(pid, cluster, np), Check: resource.ComposeTestCheckFunc( diff --git a/third_party/terraform/utils/node_config.go.erb b/third_party/terraform/utils/node_config.go.erb index 852fb8a808b0..bb6ff032ad6f 100644 --- a/third_party/terraform/utils/node_config.go.erb +++ b/third_party/terraform/utils/node_config.go.erb @@ -20,240 +20,240 @@ var defaultOauthScopes = []string{ "https://www.googleapis.com/auth/trace.append", } -var schemaNodeConfig = &schema.Schema{ - Type: schema.TypeList, - Optional: true, - Computed: true, - ForceNew: true, - MaxItems: 1, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "disk_size_gb": { - Type: schema.TypeInt, - Optional: true, - Computed: true, - ForceNew: true, - ValidateFunc: validation.IntAtLeast(10), - }, +func schemaNodeConfig() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Computed: true, + ForceNew: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "disk_size_gb": { + Type: schema.TypeInt, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validation.IntAtLeast(10), + }, - "disk_type": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{"pd-standard", "pd-ssd"}, false), - }, + "disk_type": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{"pd-standard", "pd-ssd"}, false), + }, - "guest_accelerator": &schema.Schema{ - Type: schema.TypeList, - Optional: true, - Computed: true, - ForceNew: true, - // Legacy config mode allows removing GPU's from an existing resource - // See https://www.terraform.io/docs/configuration/attr-as-blocks.html - ConfigMode: schema.SchemaConfigModeAttr, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "count": &schema.Schema{ - Type: schema.TypeInt, - Required: true, - ForceNew: true, - }, - "type": &schema.Schema{ - Type: schema.TypeString, - Required: true, - ForceNew: true, - DiffSuppressFunc: compareSelfLinkOrResourceName, + "guest_accelerator": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Computed: true, + ForceNew: true, + // Legacy config mode allows removing GPU's from an existing resource + // See https://www.terraform.io/docs/configuration/attr-as-blocks.html + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "count": &schema.Schema{ + Type: schema.TypeInt, + Required: true, + ForceNew: true, + }, + "type": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: compareSelfLinkOrResourceName, + }, }, }, }, - }, - "image_type": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, + "image_type": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, - "labels": { - Type: schema.TypeMap, - Optional: true, - // Computed=true because GKE Sandbox will automatically add labels to nodes that can/cannot run sandboxed pods. - Computed: true, - ForceNew: true, - Elem: &schema.Schema{Type: schema.TypeString}, - }, + "labels": { + Type: schema.TypeMap, + Optional: true, + // Computed=true because GKE Sandbox will automatically add labels to nodes that can/cannot run sandboxed pods. + Computed: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, - "local_ssd_count": { - Type: schema.TypeInt, - Optional: true, - Computed: true, - ForceNew: true, - ValidateFunc: validation.IntAtLeast(0), - }, + "local_ssd_count": { + Type: schema.TypeInt, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validation.IntAtLeast(0), + }, - "machine_type": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - }, + "machine_type": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, - "metadata": { - Type: schema.TypeMap, - Optional: true, - Computed: true, - ForceNew: true, - Elem: &schema.Schema{Type: schema.TypeString}, - }, + "metadata": { + Type: schema.TypeMap, + Optional: true, + Computed: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, - "min_cpu_platform": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - }, + "min_cpu_platform": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, - "oauth_scopes": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - ForceNew: true, - Elem: &schema.Schema{ - Type: schema.TypeString, - StateFunc: func(v interface{}) string { - return canonicalizeServiceScope(v.(string)) + "oauth_scopes": { + Type: schema.TypeSet, + Optional: true, + Computed: true, + ForceNew: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + StateFunc: func(v interface{}) string { + return canonicalizeServiceScope(v.(string)) + }, }, + DiffSuppressFunc: containerClusterAddedScopesSuppress, + Set: stringScopeHashcode, }, - DiffSuppressFunc: containerClusterAddedScopesSuppress, - Set: stringScopeHashcode, - }, - "preemptible": { - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - Default: false, - }, + "preemptible": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + }, - "service_account": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - }, + "service_account": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, - "tags": { - Type: schema.TypeList, - Optional: true, - ForceNew: true, - Elem: &schema.Schema{Type: schema.TypeString}, - }, + "tags": { + Type: schema.TypeList, + Optional: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, - "shielded_instance_config": &schema.Schema{ - Type: schema.TypeList, - Optional: true, - Computed: true, - ForceNew: true, - MaxItems: 1, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "enable_secure_boot": { - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - Default: false, - }, - "enable_integrity_monitoring": { - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - Default: true, + "shielded_instance_config": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Computed: true, + ForceNew: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "enable_secure_boot": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + }, + "enable_integrity_monitoring": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: true, + }, }, }, }, - }, - "taint": { - Type: schema.TypeList, - Optional: true, - // Computed=true because GKE Sandbox will automatically add taints to nodes that can/cannot run sandboxed pods. - Computed: true, - ForceNew: true, - // Legacy config mode allows explicitly defining an empty taint. - // See https://www.terraform.io/docs/configuration/attr-as-blocks.html - ConfigMode: schema.SchemaConfigModeAttr, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "key": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "value": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "effect": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{"NO_SCHEDULE", "PREFER_NO_SCHEDULE", "NO_EXECUTE"}, false), + "taint": { + Type: schema.TypeList, + Optional: true, + // Computed=true because GKE Sandbox will automatically add taints to nodes that can/cannot run sandboxed pods. + Computed: true, + ForceNew: true, + // Legacy config mode allows explicitly defining an empty taint. + // See https://www.terraform.io/docs/configuration/attr-as-blocks.html + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "key": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "value": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "effect": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{"NO_SCHEDULE", "PREFER_NO_SCHEDULE", "NO_EXECUTE"}, false), + }, }, }, }, - }, - "workload_metadata_config": { -<% if version.nil? || version == 'ga' -%> - Removed: "This field is in beta. Use it in the the google-beta provider instead. See https://terraform.io/docs/providers/google/guides/provider_versions.html for more details.", - Computed: true, -<% end -%> - Type: schema.TypeList, - Optional: true, - ForceNew: true, - MaxItems: 1, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "node_metadata": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{"UNSPECIFIED", "SECURE", "EXPOSE", "GKE_METADATA_SERVER"}, false), + "workload_metadata_config": { + <% if version.nil? || version == 'ga' -%> + Removed: "This field is in beta. Use it in the the google-beta provider instead. See https://terraform.io/docs/providers/google/guides/provider_versions.html for more details.", + Computed: true, + <% end -%> + Type: schema.TypeList, + Optional: true, + ForceNew: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "node_metadata": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{"UNSPECIFIED", "SECURE", "EXPOSE", "GKE_METADATA_SERVER"}, false), + }, }, }, }, - }, - "sandbox_config": { -<% if version.nil? || version == 'ga' -%> - Removed: "This field is in beta. Use it in the the google-beta provider instead. See https://terraform.io/docs/providers/google/guides/provider_versions.html for more details.", - Computed: true, -<% end -%> - Type: schema.TypeList, - Optional: true, - ForceNew: true, - MaxItems: 1, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "sandbox_type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"gvisor"}, false), + "sandbox_config": { + <% if version.nil? || version == 'ga' -%> + Removed: "This field is in beta. Use it in the the google-beta provider instead. See https://terraform.io/docs/providers/google/guides/provider_versions.html for more details.", + Computed: true, + <% end -%> + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "sandbox_type": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{"gvisor"}, false), + }, }, }, }, - }, -<% unless version == 'ga' -%> - "boot_disk_kms_key": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, + <% unless version == 'ga' -%> + "boot_disk_kms_key": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + <% end -%> }, -<% end -%> }, - }, + } } func expandNodeConfig(v interface{}) *containerBeta.NodeConfig { @@ -376,11 +376,8 @@ func expandNodeConfig(v interface{}) *containerBeta.NodeConfig { } <% unless version == 'ga' -%> - if v, ok := nodeConfig["workload_metadata_config"]; ok && len(v.([]interface{})) > 0 { - conf := v.([]interface{})[0].(map[string]interface{}) - nc.WorkloadMetadataConfig = &containerBeta.WorkloadMetadataConfig{ - NodeMetadata: conf["node_metadata"].(string), - } + if v, ok := nodeConfig["workload_metadata_config"]; ok { + nc.WorkloadMetadataConfig = expandWorkloadMetadataConfig(v) } if v, ok := nodeConfig["sandbox_config"]; ok && len(v.([]interface{})) > 0 { @@ -398,6 +395,23 @@ func expandNodeConfig(v interface{}) *containerBeta.NodeConfig { return nc } +<% unless version == 'ga' -%> +func expandWorkloadMetadataConfig(v interface{}) *containerBeta.WorkloadMetadataConfig { + if v == nil { + return nil + } + ls := v.([]interface{}) + if len(ls) == 0 { + return nil + } + + cfg := ls[0].(map[string]interface{}) + return &containerBeta.WorkloadMetadataConfig{ + NodeMetadata: cfg["node_metadata"].(string), + } +} + +<% end -%> func flattenNodeConfig(c *containerBeta.NodeConfig) []map[string]interface{} { config := make([]map[string]interface{}, 0, 1) From 7f7d23108cf9420df9b65386f004589e5058d9fc Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Mon, 18 May 2020 12:50:48 -0700 Subject: [PATCH 2/4] fix forcenew for nested field as well --- third_party/terraform/utils/node_config.go.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/third_party/terraform/utils/node_config.go.erb b/third_party/terraform/utils/node_config.go.erb index bb6ff032ad6f..624e48ba02c6 100644 --- a/third_party/terraform/utils/node_config.go.erb +++ b/third_party/terraform/utils/node_config.go.erb @@ -212,7 +212,6 @@ func schemaNodeConfig() *schema.Schema { <% end -%> Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ From 01f282ad0d9514fec8b25abf581a8eeffc3d5a11 Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Mon, 18 May 2020 13:16:07 -0700 Subject: [PATCH 3/4] add set to util for forcenewing field schema, move to utils --- .../resources/resource_container_cluster.go.erb | 11 ----------- third_party/terraform/utils/utils.go.erb | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/third_party/terraform/resources/resource_container_cluster.go.erb b/third_party/terraform/resources/resource_container_cluster.go.erb index 2156ce65e1bb..2097790d6b11 100644 --- a/third_party/terraform/resources/resource_container_cluster.go.erb +++ b/third_party/terraform/resources/resource_container_cluster.go.erb @@ -83,17 +83,6 @@ func clusterSchemaNodeConfig() *schema.Schema { return nodeConfigSch } -func changeFieldSchemaToForceNew(sch *schema.Schema) { - sch.ForceNew = true - if sch.Type == schema.TypeList { - if nestedR, ok := sch.Elem.(*schema.Resource); ok { - for _, nestedSch := range nestedR.Schema { - changeFieldSchemaToForceNew(nestedSch) - } - } - } -} - func rfc5545RecurrenceDiffSuppress(k, o, n string, d *schema.ResourceData) bool { // This diff gets applied in the cloud console if you specify // "FREQ=DAILY" in your config and add a maintenance exclusion. diff --git a/third_party/terraform/utils/utils.go.erb b/third_party/terraform/utils/utils.go.erb index bc553238b310..abc4967d82f3 100644 --- a/third_party/terraform/utils/utils.go.erb +++ b/third_party/terraform/utils/utils.go.erb @@ -431,3 +431,17 @@ func migrateStateNoop(v int, is *terraform.InstanceState, meta interface{}) (*te func expandString(v interface{}, d TerraformResourceData, config *Config) (string, error) { return v.(string), nil } + + +func changeFieldSchemaToForceNew(sch *schema.Schema) { + sch.ForceNew = true + switch sch.Type { + case schema.TypeList: + case schema.TypeSet: + if nestedR, ok := sch.Elem.(*schema.Resource); ok { + for _, nestedSch := range nestedR.Schema { + changeFieldSchemaToForceNew(nestedSch) + } + } + } +} From d40530629147760cd9156d9049b93e0a94d8b490 Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Mon, 18 May 2020 14:43:27 -0700 Subject: [PATCH 4/4] add back forcenew for sandbox config --- third_party/terraform/utils/node_config.go.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/third_party/terraform/utils/node_config.go.erb b/third_party/terraform/utils/node_config.go.erb index 624e48ba02c6..4c1928830b97 100644 --- a/third_party/terraform/utils/node_config.go.erb +++ b/third_party/terraform/utils/node_config.go.erb @@ -231,6 +231,7 @@ func schemaNodeConfig() *schema.Schema { <% end -%> Type: schema.TypeList, Optional: true, + ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{