From 5b147fffe7829b3304fe6c51f5f53cc4c35a9195 Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Thu, 6 Jul 2023 13:23:00 -0700 Subject: [PATCH 01/10] add net_admin field to container cluster for autopilot clusters --- .../resource_container_cluster.go.erb | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 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 f46444945d0b..985adc833fec 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 @@ -886,6 +886,13 @@ func ResourceContainerCluster() *schema.Resource { // ConflictsWith: many fields, see https://cloud.google.com/kubernetes-engine/docs/concepts/autopilot-overview#comparison. The conflict is only set one-way, on other fields w/ this field. }, + "allow_net_admin": { + Type: schema.TypeBool, + Optional: true, + Description: `Enable NET_ADMIN for this cluster.`, + RequiredWith: []string{"enable_autopilot"}, + }, + "authenticator_groups_config": { Type: schema.TypeList, Optional: true, @@ -2073,6 +2080,9 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er BinaryAuthorization: expandBinaryAuthorization(d.Get("binary_authorization"), d.Get("enable_binary_authorization").(bool)), Autopilot: &container.Autopilot{ Enabled: d.Get("enable_autopilot").(bool), + WorkloadPolicyConfig: &container.WorkloadPolicyConfig{ + AllowNetAdmin: d.Get("allow_net_admin").(bool), + }, ForceSendFields: []string{"Enabled"}, }, ReleaseChannel: expandReleaseChannel(d.Get("release_channel")), @@ -2493,10 +2503,15 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro return err } } - if cluster.Autopilot != nil { - if err := d.Set("enable_autopilot", cluster.Autopilot.Enabled); err != nil { + if autopilot := cluster.Autopilot; autopilot != nil { + if err := d.Set("enable_autopilot", autopilot.Enabled); err != nil { return fmt.Errorf("Error setting enable_autopilot: %s", err) } + if autopilot.WorkloadConfig != nil { + if err := d.Set("allow_net_admin", autopilot.WorkloadConfig.AllowNetAdmin); err != nil { + return fmt.Errorf("Error setting allow_net_admin: %s", err) + } + } } if cluster.ShieldedNodes != nil { if err := d.Set("enable_shielded_nodes", cluster.ShieldedNodes.Enabled); err != nil { @@ -2750,6 +2765,25 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er log.Printf("[INFO] GKE cluster %s's cluster-wide autoscaling has been updated", d.Id()) } + if d.HasChange("allow_net_admin") { + allowed := d.Get("allow_net_admin").(bool) + req := &container.UpdateClusterRequest{ + Update: &container.ClusterUpdate{ + DesiredAutopilotWorkloadPolicyConfig: &container.WorkloadPolicyConfig{ + AllowNetAdmin: allowed, + } + } + } + + updateF := updateFunc(req, "updating net admin for GKE autopilot workload policy config") + // Call update serially. + if err := transport_tpg.LockedCall(lockKey, updateF); err != nil { + return err + } + + log.Printf("[INFO] GKE cluster %s's autopilot workload policy config allow_net_admin has been set to %v", d.Id(), allowed) + } + if d.HasChange("enable_binary_authorization") { enabled := d.Get("enable_binary_authorization").(bool) req := &container.UpdateClusterRequest{ @@ -4083,11 +4117,11 @@ func expandPodCidrOverprovisionConfig(configured interface{}) *container.PodCIDR } } -func expandIPAllocationPolicy(configured interface{}, networkingMode string, autopilot bool) (*container.IPAllocationPolicy, error) { +func expandIPAllocationPolicy(configured interface{}, networkingMode string, enable_autopilot bool) (*container.IPAllocationPolicy, error) { l := configured.([]interface{}) if len(l) == 0 || l[0] == nil { if networkingMode == "VPC_NATIVE" { - if autopilot { + if enable_autopilot { return nil, nil } return nil, fmt.Errorf("`ip_allocation_policy` block is required for VPC_NATIVE clusters.") From ebb62de62ff2fa358b15817da32d4941a0217f54 Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Thu, 6 Jul 2023 16:30:26 -0700 Subject: [PATCH 02/10] add allow_net_admin tests --- .../resource_container_cluster_test.go.erb | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb index 36aa886a4066..23100587ad4e 100644 --- a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -3677,6 +3677,37 @@ func TestAccContainerCluster_autopilot_minimal(t *testing.T) { }) } +func TestAccContainerCluster_autopilot_net_admin(t *testing.T) { + t.Parallel() + + // Test that allow_net_admin can be enabled for a new autopilot cluster, + // updated to false, then re-enabled. + clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckContainerClusterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccContainerCluster_autopilot_net_admin(clusterName, true), + }, + { + ResourceName: "google_container_cluster.primary", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccContainerCluster_autopilot_net_admin(clusterName, false), + }, + { + ResourceName: "google_container_cluster.primary", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccContainerCluster_masterAuthorizedNetworksDisabled(t *testing.T, resource_name string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resource_name] @@ -7618,3 +7649,13 @@ resource "google_container_cluster" "primary" { enable_autopilot = true }`, name) } + +func testAccContainerCluster_autopilot_net_admin(name string, enabled bool) string { + return fmt.Sprintf(` +resource "google_container_cluster" "primary" { + name = "%s" + location = "us-central1" + enable_autopilot = true + allow_net_admin = enabled +}`, name) +} From f799ff38d5fb2839dc8921c7ee4e27278d96c68d Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Mon, 10 Jul 2023 14:25:08 -0700 Subject: [PATCH 03/10] tweak param name --- .../services/container/resource_container_cluster.go.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 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 985adc833fec..0c059f7929fb 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 @@ -4117,11 +4117,11 @@ func expandPodCidrOverprovisionConfig(configured interface{}) *container.PodCIDR } } -func expandIPAllocationPolicy(configured interface{}, networkingMode string, enable_autopilot bool) (*container.IPAllocationPolicy, error) { +func expandIPAllocationPolicy(configured interface{}, networkingMode string, autopilot bool) (*container.IPAllocationPolicy, error) { l := configured.([]interface{}) if len(l) == 0 || l[0] == nil { if networkingMode == "VPC_NATIVE" { - if enable_autopilot { + if autopilot { return nil, nil } return nil, fmt.Errorf("`ip_allocation_policy` block is required for VPC_NATIVE clusters.") From 94cdd6ac8044f76e2b0be9425a9a7d7161fb2fdf Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Mon, 10 Jul 2023 14:28:30 -0700 Subject: [PATCH 04/10] fix net_admin test format string --- .../terraform/tests/resource_container_cluster_test.go.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb index 23100587ad4e..b1dfa0a15719 100644 --- a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -7656,6 +7656,6 @@ resource "google_container_cluster" "primary" { name = "%s" location = "us-central1" enable_autopilot = true - allow_net_admin = enabled -}`, name) + allow_net_admin = "%t" +}`, name, enabled) } From 1f5ec62468f30344b1bd9f18f1304830abb52afc Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Mon, 10 Jul 2023 17:29:17 -0700 Subject: [PATCH 05/10] fix syntax bug --- .../services/container/resource_container_cluster.go.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 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 0c059f7929fb..c63e03daf1a0 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 @@ -2771,8 +2771,8 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er Update: &container.ClusterUpdate{ DesiredAutopilotWorkloadPolicyConfig: &container.WorkloadPolicyConfig{ AllowNetAdmin: allowed, - } - } + }, + }, } updateF := updateFunc(req, "updating net admin for GKE autopilot workload policy config") From 5dc28c0f0010774d66db003b830eb31a9eb28610 Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Mon, 10 Jul 2023 17:41:31 -0700 Subject: [PATCH 06/10] fix api field typo --- .../services/container/resource_container_cluster.go.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 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 c63e03daf1a0..a81f5a273da2 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 @@ -2507,8 +2507,8 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro if err := d.Set("enable_autopilot", autopilot.Enabled); err != nil { return fmt.Errorf("Error setting enable_autopilot: %s", err) } - if autopilot.WorkloadConfig != nil { - if err := d.Set("allow_net_admin", autopilot.WorkloadConfig.AllowNetAdmin); err != nil { + if autopilot.WorkloadPolicyConfig != nil { + if err := d.Set("allow_net_admin", autopilot.WorkloadPolicyConfig.AllowNetAdmin); err != nil { return fmt.Errorf("Error setting allow_net_admin: %s", err) } } From 7a0c37b311980f11f63a70bfa94338bb841921bd Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Tue, 11 Jul 2023 17:14:42 -0700 Subject: [PATCH 07/10] version safety and ImportStateVerifyIgnore for min_master_version in test --- .../resource_container_cluster.go.erb | 21 +++++++++++++------ .../resource_container_cluster_test.go.erb | 12 +++-------- 2 files changed, 18 insertions(+), 15 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 a81f5a273da2..eea6c28d0876 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 @@ -2055,6 +2055,13 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er return err } + var workloadPolicyConfig *container.WorkloadPolicyConfig + if allowed := d.Get("allow_net_admin").(bool); allowed { + workloadPolicyConfig = &container.WorkloadPolicyConfig{ + AllowNetAdmin: allowed, + } + } + cluster := &container.Cluster{ Name: clusterName, InitialNodeCount: int64(d.Get("initial_node_count").(int)), @@ -2080,9 +2087,7 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er BinaryAuthorization: expandBinaryAuthorization(d.Get("binary_authorization"), d.Get("enable_binary_authorization").(bool)), Autopilot: &container.Autopilot{ Enabled: d.Get("enable_autopilot").(bool), - WorkloadPolicyConfig: &container.WorkloadPolicyConfig{ - AllowNetAdmin: d.Get("allow_net_admin").(bool), - }, + WorkloadPolicyConfig: workloadPolicyConfig, ForceSendFields: []string{"Enabled"}, }, ReleaseChannel: expandReleaseChannel(d.Get("release_channel")), @@ -2766,12 +2771,16 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er } if d.HasChange("allow_net_admin") { + var workloadPolicyConfig *container.WorkloadPolicyConfig allowed := d.Get("allow_net_admin").(bool) + if allowed { + workloadPolicyConfig = &container.WorkloadPolicyConfig{ + AllowNetAdmin: allowed, + } + } req := &container.UpdateClusterRequest{ Update: &container.ClusterUpdate{ - DesiredAutopilotWorkloadPolicyConfig: &container.WorkloadPolicyConfig{ - AllowNetAdmin: allowed, - }, + DesiredAutopilotWorkloadPolicyConfig: workloadPolicyConfig, }, } diff --git a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb index b1dfa0a15719..8a5a470ca8f4 100644 --- a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -3695,14 +3695,7 @@ func TestAccContainerCluster_autopilot_net_admin(t *testing.T) { ResourceName: "google_container_cluster.primary", ImportState: true, ImportStateVerify: true, - }, - { - Config: testAccContainerCluster_autopilot_net_admin(clusterName, false), - }, - { - ResourceName: "google_container_cluster.primary", - ImportState: true, - ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version"}, }, }, }) @@ -7656,6 +7649,7 @@ resource "google_container_cluster" "primary" { name = "%s" location = "us-central1" enable_autopilot = true - allow_net_admin = "%t" + allow_net_admin = %t + min_master_version = 1.27 }`, name, enabled) } From 6c46273e33c516ac3fa672b7d9d314114f94f4ee Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Tue, 11 Jul 2023 17:23:21 -0700 Subject: [PATCH 08/10] comment change --- .../terraform/tests/resource_container_cluster_test.go.erb | 2 -- 1 file changed, 2 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb index 8a5a470ca8f4..19cd0b3f8ee1 100644 --- a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -3680,8 +3680,6 @@ func TestAccContainerCluster_autopilot_minimal(t *testing.T) { func TestAccContainerCluster_autopilot_net_admin(t *testing.T) { t.Parallel() - // Test that allow_net_admin can be enabled for a new autopilot cluster, - // updated to false, then re-enabled. clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, From d9717ec8c792b7a3226d9c6f9730149f7ba8e8df Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Wed, 19 Jul 2023 11:38:09 -0700 Subject: [PATCH 09/10] Add documentation and remove provider-side validation --- .../services/container/resource_container_cluster.go.erb | 1 - .../terraform/website/docs/r/container_cluster.html.markdown | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) 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 eea6c28d0876..199db082124a 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 @@ -890,7 +890,6 @@ func ResourceContainerCluster() *schema.Resource { Type: schema.TypeBool, Optional: true, Description: `Enable NET_ADMIN for this cluster.`, - RequiredWith: []string{"enable_autopilot"}, }, "authenticator_groups_config": { 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 df41e17b9104..bb2ba0f13e2c 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 @@ -121,6 +121,10 @@ preferred. * `addons_config` - (Optional) The configuration for addons supported by GKE. Structure is [documented below](#nested_addons_config). +* `allow_net_admin` - (Optional) Enable NET_ADMIN for the cluster. Defaults to +`false`. This field should only be enabled for Autopilot clusters (`enable_autopilot` +set to `true`). + * `cluster_ipv4_cidr` - (Optional) The IP address range of the Kubernetes pods in this cluster in CIDR notation (e.g. `10.96.0.0/14`). Leave blank to have one automatically chosen or specify a `/14` block in `10.0.0.0/8`. This field will From 2a7afabb000dcbd216484dc34076d0dbd5c191c6 Mon Sep 17 00:00:00 2001 From: Jonah Peretz Date: Thu, 20 Jul 2023 18:40:49 -0700 Subject: [PATCH 10/10] add update test --- .../resource_container_cluster.go.erb | 10 +++------- .../resource_container_cluster_test.go.erb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 7 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 199db082124a..e9c3b080c2da 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 @@ -2770,16 +2770,12 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er } if d.HasChange("allow_net_admin") { - var workloadPolicyConfig *container.WorkloadPolicyConfig allowed := d.Get("allow_net_admin").(bool) - if allowed { - workloadPolicyConfig = &container.WorkloadPolicyConfig{ - AllowNetAdmin: allowed, - } - } req := &container.UpdateClusterRequest{ Update: &container.ClusterUpdate{ - DesiredAutopilotWorkloadPolicyConfig: workloadPolicyConfig, + DesiredAutopilotWorkloadPolicyConfig: &container.WorkloadPolicyConfig{ + AllowNetAdmin: allowed, + }, }, } diff --git a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb index 19cd0b3f8ee1..3b5aafa10ec5 100644 --- a/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -3695,6 +3695,24 @@ func TestAccContainerCluster_autopilot_net_admin(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"min_master_version"}, }, + { + Config: testAccContainerCluster_autopilot_net_admin(clusterName, false), + }, + { + ResourceName: "google_container_cluster.primary", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version"}, + }, + { + Config: testAccContainerCluster_autopilot_net_admin(clusterName, true), + }, + { + ResourceName: "google_container_cluster.primary", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"min_master_version"}, + }, }, }) }