From d43b74c57d2a3e38f1b268a5a51b332a866846ab Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Wed, 20 Dec 2023 19:55:34 +0000 Subject: [PATCH] Fix crash in containerattached when removing admin_groups or admin_users. (#9647) * Fix crash in containerattached when removing admin_groups or admin_users. * Format test file. [upstream:1bb86d9a9272c4b2ab46186dd35fe76e9c3a2296] Signed-off-by: Modular Magician --- .changelog/9647.txt | 3 + .../resource_container_attached_cluster.go | 26 ++-- ..._container_attached_cluster_update_test.go | 119 +++++++++++++++++- 3 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 .changelog/9647.txt diff --git a/.changelog/9647.txt b/.changelog/9647.txt new file mode 100644 index 00000000000..59d8d988b68 --- /dev/null +++ b/.changelog/9647.txt @@ -0,0 +1,3 @@ +```release-note:bug +containerattached: fixed crash when updating a cluster to remove `admin_users` or `admin_groups` +``` diff --git a/google/services/containerattached/resource_container_attached_cluster.go b/google/services/containerattached/resource_container_attached_cluster.go index e021ede70b5..08c5467375c 100644 --- a/google/services/containerattached/resource_container_attached_cluster.go +++ b/google/services/containerattached/resource_container_attached_cluster.go @@ -1167,23 +1167,27 @@ func flattenContainerAttachedClusterErrorsMessage(v interface{}, d *schema.Resou // ], // } func flattenContainerAttachedClusterAuthorization(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { - if v == nil { + if v == nil || len(v.(map[string]interface{})) == 0 { return nil } - orig := v.(map[string]interface{})["adminUsers"].([]interface{}) transformed := make(map[string][]string) - transformed["admin_users"] = make([]string, len(orig)) - for i, u := range orig { - if u != nil { - transformed["admin_users"][i] = u.(map[string]interface{})["username"].(string) + if v.(map[string]interface{})["adminUsers"] != nil { + orig := v.(map[string]interface{})["adminUsers"].([]interface{}) + transformed["admin_users"] = make([]string, len(orig)) + for i, u := range orig { + if u != nil { + transformed["admin_users"][i] = u.(map[string]interface{})["username"].(string) + } } } - orig = v.(map[string]interface{})["adminGroups"].([]interface{}) - transformed["admin_groups"] = make([]string, len(orig)) - for i, u := range orig { - if u != nil { - transformed["admin_groups"][i] = u.(map[string]interface{})["group"].(string) + if v.(map[string]interface{})["adminGroups"] != nil { + orig := v.(map[string]interface{})["adminGroups"].([]interface{}) + transformed["admin_groups"] = make([]string, len(orig)) + for i, u := range orig { + if u != nil { + transformed["admin_groups"][i] = u.(map[string]interface{})["group"].(string) + } } } diff --git a/google/services/containerattached/resource_container_attached_cluster_update_test.go b/google/services/containerattached/resource_container_attached_cluster_update_test.go index 9cde80fefb1..e0eb6292df0 100644 --- a/google/services/containerattached/resource_container_attached_cluster_update_test.go +++ b/google/services/containerattached/resource_container_attached_cluster_update_test.go @@ -39,6 +39,24 @@ func TestAccContainerAttachedCluster_update(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"location", "annotations"}, }, + { + Config: testAccContainerAttachedCluster_containerAttachedCluster_removeAuthorizationUsers(context), + }, + { + ResourceName: "google_container_attached_cluster.primary", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"location", "annotations"}, + }, + { + Config: testAccContainerAttachedCluster_containerAttachedCluster_removeAuthorizationGroups(context), + }, + { + ResourceName: "google_container_attached_cluster.primary", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"location", "annotations"}, + }, { Config: testAccContainerAttachedCluster_containerAttachedCluster_destroy(context), }, @@ -157,9 +175,7 @@ resource "google_container_attached_cluster" "primary" { `, context) } -// Duplicate of testAccContainerAttachedCluster_containerAttachedCluster_update without lifecycle.prevent_destroy set -// so the test can clean up the resource after the update. -func testAccContainerAttachedCluster_containerAttachedCluster_destroy(context map[string]interface{}) string { +func testAccContainerAttachedCluster_containerAttachedCluster_removeAuthorizationUsers(context map[string]interface{}) string { return acctest.Nprintf(` data "google_project" "project" { } @@ -180,7 +196,6 @@ resource "google_container_attached_cluster" "primary" { label-two = "value-two" } authorization { - admin_users = [ "user2@example.com", "user3@example.com"] admin_groups = [ "group3@example.com"] } oidc_config { @@ -203,6 +218,102 @@ resource "google_container_attached_cluster" "primary" { namespace = "custom-ns" } } + lifecycle { + prevent_destroy = true + } +} +`, context) +} + +func testAccContainerAttachedCluster_containerAttachedCluster_removeAuthorizationGroups(context map[string]interface{}) string { + return acctest.Nprintf(` +data "google_project" "project" { +} + +data "google_container_attached_versions" "versions" { + location = "us-west1" + project = data.google_project.project.project_id +} + +resource "google_container_attached_cluster" "primary" { + name = "update%{random_suffix}" + project = data.google_project.project.project_id + location = "us-west1" + description = "Test cluster updated" + distribution = "aks" + annotations = { + label-one = "value-one" + label-two = "value-two" + } + oidc_config { + issuer_url = "https://oidc.issuer.url" + jwks = base64encode("{\"keys\":[{\"use\":\"sig\",\"kty\":\"RSA\",\"kid\":\"testid\",\"alg\":\"RS256\",\"n\":\"somedata\",\"e\":\"AQAB\"}]}") + } + platform_version = data.google_container_attached_versions.versions.valid_versions[0] + fleet { + project = "projects/${data.google_project.project.number}" + } + monitoring_config { + managed_prometheus_config {} + } + binary_authorization { + evaluation_mode = "DISABLED" + } + proxy_config { + kubernetes_secret { + name = "new-proxy-config" + namespace = "custom-ns" + } + } + lifecycle { + prevent_destroy = true + } +} +`, context) +} + +// Duplicate of testAccContainerAttachedCluster_containerAttachedCluster_update without lifecycle.prevent_destroy set +// so the test can clean up the resource after the update. +func testAccContainerAttachedCluster_containerAttachedCluster_destroy(context map[string]interface{}) string { + return acctest.Nprintf(` +data "google_project" "project" { +} + +data "google_container_attached_versions" "versions" { + location = "us-west1" + project = data.google_project.project.project_id +} + +resource "google_container_attached_cluster" "primary" { + name = "update%{random_suffix}" + project = data.google_project.project.project_id + location = "us-west1" + description = "Test cluster updated" + distribution = "aks" + annotations = { + label-one = "value-one" + label-two = "value-two" + } + oidc_config { + issuer_url = "https://oidc.issuer.url" + jwks = base64encode("{\"keys\":[{\"use\":\"sig\",\"kty\":\"RSA\",\"kid\":\"testid\",\"alg\":\"RS256\",\"n\":\"somedata\",\"e\":\"AQAB\"}]}") + } + platform_version = data.google_container_attached_versions.versions.valid_versions[0] + fleet { + project = "projects/${data.google_project.project.number}" + } + monitoring_config { + managed_prometheus_config {} + } + binary_authorization { + evaluation_mode = "DISABLED" + } + proxy_config { + kubernetes_secret { + name = "new-proxy-config" + namespace = "custom-ns" + } + } } `, context) }