Remove default addition of IAP message in encoder and decoder, add ne…

…w required `enabled` field and remove the previous required tags under IAP message for resource google_compute_backend_service and resource google_compute_region_backend_service (#9581) (#7758) [upstream:29ded33d5a7572d047d6870518b7840f026fec02] Signed-off-by: Modular Magician <[email protected]>
hashicorp · Jul 18, 2024 · c6b8e94 · c6b8e94
1 parent c1cdf12
commit c6b8e94
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 84 deletions.
diff --git a/.changelog/9581.txt b/.changelog/9581.txt
@@ -0,0 +1,6 @@
+```release-note:bug
+compute: fixed an issue regarding sending `enabled` field by default for null `iap` message in `google_compute_backend_service` and `google_compute_region_backend_service`
+```
+```release-note:breaking-change
+compute: Add new required field `enabled` in `google_compute_backend_service` and `google_compute_region_backend_service`
+```
diff --git a/google-beta/services/compute/resource_compute_backend_service.go b/google-beta/services/compute/resource_compute_backend_service.go
@@ -662,14 +662,19 @@ For internal load balancing, a URL to a HealthCheck resource must be specified i
 				MaxItems:    1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"enabled": {
+							Type:        schema.TypeBool,
+							Required:    true,
+							Description: `Whether the serving infrastructure will authenticate and authorize all incoming requests.`,
+						},
 						"oauth2_client_id": {
 							Type:        schema.TypeString,
-							Required:    true,
+							Optional:    true,
 							Description: `OAuth2 Client ID for IAP`,
 						},
 						"oauth2_client_secret": {
 							Type:        schema.TypeString,
-							Required:    true,
+							Optional:    true,
 							Description: `OAuth2 Client Secret for IAP`,
 							Sensitive:   true,
 						},
@@ -2814,6 +2819,8 @@ func flattenComputeBackendServiceIap(v interface{}, d *schema.ResourceData, conf
 		return nil
 	}
 	transformed := make(map[string]interface{})
+	transformed["enabled"] =
+		flattenComputeBackendServiceIapEnabled(original["enabled"], d, config)
 	transformed["oauth2_client_id"] =
 		flattenComputeBackendServiceIapOauth2ClientId(original["oauth2ClientId"], d, config)
 	transformed["oauth2_client_secret"] =
@@ -2822,6 +2829,10 @@ func flattenComputeBackendServiceIap(v interface{}, d *schema.ResourceData, conf
 		flattenComputeBackendServiceIapOauth2ClientSecretSha256(original["oauth2ClientSecretSha256"], d, config)
 	return []interface{}{transformed}
 }
+func flattenComputeBackendServiceIapEnabled(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} {
+	return v
+}
+
 func flattenComputeBackendServiceIapOauth2ClientId(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} {
 	return v
 }
@@ -3964,6 +3975,13 @@ func expandComputeBackendServiceIap(v interface{}, d tpgresource.TerraformResour
 	original := raw.(map[string]interface{})
 	transformed := make(map[string]interface{})
 
+	transformedEnabled, err := expandComputeBackendServiceIapEnabled(original["enabled"], d, config)
+	if err != nil {
+		return nil, err
+	} else if val := reflect.ValueOf(transformedEnabled); val.IsValid() && !tpgresource.IsEmptyValue(val) {
+		transformed["enabled"] = transformedEnabled
+	}
+
 	transformedOauth2ClientId, err := expandComputeBackendServiceIapOauth2ClientId(original["oauth2_client_id"], d, config)
 	if err != nil {
 		return nil, err
@@ -3988,6 +4006,10 @@ func expandComputeBackendServiceIap(v interface{}, d tpgresource.TerraformResour
 	return transformed, nil
 }
 
+func expandComputeBackendServiceIapEnabled(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
+	return v, nil
+}
+
 func expandComputeBackendServiceIapOauth2ClientId(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
 	return v, nil
 }
@@ -4392,24 +4414,6 @@ func expandComputeBackendServiceServiceLbPolicy(v interface{}, d tpgresource.Ter
 }
 
 func resourceComputeBackendServiceEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) {
-	// The BackendService API's Update / PUT API is badly formed and behaves like
-	// a PATCH field for at least IAP. When sent a `null` `iap` field, the API
-	// doesn't disable an existing field. To work around this, we need to emulate
-	// the old Terraform behaviour of always sending the block (at both update and
-	// create), and force sending each subfield as empty when the block isn't
-	// present in config.
-
-	iapVal := obj["iap"]
-	if iapVal == nil {
-		data := map[string]interface{}{}
-		data["enabled"] = false
-		obj["iap"] = data
-	} else {
-		iap := iapVal.(map[string]interface{})
-		iap["enabled"] = true
-		obj["iap"] = iap
-	}
-
 	backendsRaw, ok := obj["backends"]
 	if !ok {
 		return obj, nil
@@ -4468,18 +4472,6 @@ func resourceComputeBackendServiceEncoder(d *schema.ResourceData, meta interface
 }
 
 func resourceComputeBackendServiceDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
-	// We need to pretend IAP isn't there if it's disabled for Terraform to maintain
-	// BC behaviour with the handwritten resource.
-	v, ok := res["iap"]
-	if !ok || v == nil {
-		delete(res, "iap")
-		return res, nil
-	}
-	m := v.(map[string]interface{})
-	if ok && m["enabled"] == false {
-		delete(res, "iap")
-	}
-
 	// Requests with consistentHash will error for specific values of
 	// localityLbPolicy. However, the API will not remove it if the backend
 	// service is updated to from supporting to non-supporting localityLbPolicy

diff --git a/google-beta/services/compute/resource_compute_backend_service_generated_test.go b/google-beta/services/compute/resource_compute_backend_service_generated_test.go
@@ -103,6 +103,7 @@ resource "google_compute_backend_service" "default" {
   protocol              = "HTTP"
   load_balancing_scheme = "EXTERNAL"
   iap {
+    enabled              = true
     oauth2_client_id     = "abc"
     oauth2_client_secret = "xyz"
   }

diff --git a/google-beta/services/compute/resource_compute_backend_service_test.go b/google-beta/services/compute/resource_compute_backend_service_test.go
@@ -126,23 +126,23 @@ func TestAccComputeBackendService_withBackendAndIAP(t *testing.T) {
 		CheckDestroy:             testAccCheckComputeBackendServiceDestroyProducer(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccComputeBackendService_withBackendAndIAP(
+				Config: testAccComputeBackendService_withBackend(
 					serviceName, igName, itName, checkName, 10),
 			},
 			{
-				ResourceName:            "google_compute_backend_service.lipsum",
-				ImportState:             true,
-				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
+				ResourceName:      "google_compute_backend_service.lipsum",
+				ImportState:       true,
+				ImportStateVerify: true,
 			},
 			{
-				Config: testAccComputeBackendService_withBackend(
+				Config: testAccComputeBackendService_withBackendAndIAP(
 					serviceName, igName, itName, checkName, 10),
 			},
 			{
-				ResourceName:      "google_compute_backend_service.lipsum",
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            "google_compute_backend_service.lipsum",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
 			},
 		},
 	})
@@ -1259,6 +1259,7 @@ resource "google_compute_backend_service" "lipsum" {
   }
 
   iap {
+    enabled              = true
     oauth2_client_id     = "test"
     oauth2_client_secret = "test"
   }

diff --git a/google-beta/services/compute/resource_compute_region_backend_service.go b/google-beta/services/compute/resource_compute_region_backend_service.go
@@ -657,14 +657,19 @@ or serverless NEG as a backend.`,
 				MaxItems:    1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"enabled": {
+							Type:        schema.TypeBool,
+							Required:    true,
+							Description: `Whether the serving infrastructure will authenticate and authorize all incoming requests.`,
+						},
 						"oauth2_client_id": {
 							Type:        schema.TypeString,
-							Required:    true,
+							Optional:    true,
 							Description: `OAuth2 Client ID for IAP`,
 						},
 						"oauth2_client_secret": {
 							Type:        schema.TypeString,
-							Required:    true,
+							Optional:    true,
 							Description: `OAuth2 Client Secret for IAP`,
 							Sensitive:   true,
 						},
@@ -2677,6 +2682,8 @@ func flattenComputeRegionBackendServiceIap(v interface{}, d *schema.ResourceData
 		return nil
 	}
 	transformed := make(map[string]interface{})
+	transformed["enabled"] =
+		flattenComputeRegionBackendServiceIapEnabled(original["enabled"], d, config)
 	transformed["oauth2_client_id"] =
 		flattenComputeRegionBackendServiceIapOauth2ClientId(original["oauth2ClientId"], d, config)
 	transformed["oauth2_client_secret"] =
@@ -2685,6 +2692,10 @@ func flattenComputeRegionBackendServiceIap(v interface{}, d *schema.ResourceData
 		flattenComputeRegionBackendServiceIapOauth2ClientSecretSha256(original["oauth2ClientSecretSha256"], d, config)
 	return []interface{}{transformed}
 }
+func flattenComputeRegionBackendServiceIapEnabled(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} {
+	return v
+}
+
 func flattenComputeRegionBackendServiceIapOauth2ClientId(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} {
 	return v
 }
@@ -3811,6 +3822,13 @@ func expandComputeRegionBackendServiceIap(v interface{}, d tpgresource.Terraform
 	original := raw.(map[string]interface{})
 	transformed := make(map[string]interface{})
 
+	transformedEnabled, err := expandComputeRegionBackendServiceIapEnabled(original["enabled"], d, config)
+	if err != nil {
+		return nil, err
+	} else if val := reflect.ValueOf(transformedEnabled); val.IsValid() && !tpgresource.IsEmptyValue(val) {
+		transformed["enabled"] = transformedEnabled
+	}
+
 	transformedOauth2ClientId, err := expandComputeRegionBackendServiceIapOauth2ClientId(original["oauth2_client_id"], d, config)
 	if err != nil {
 		return nil, err
@@ -3835,6 +3853,10 @@ func expandComputeRegionBackendServiceIap(v interface{}, d tpgresource.Terraform
 	return transformed, nil
 }
 
+func expandComputeRegionBackendServiceIapEnabled(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
+	return v, nil
+}
+
 func expandComputeRegionBackendServiceIapOauth2ClientId(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
 	return v, nil
 }
@@ -4202,23 +4224,6 @@ func expandComputeRegionBackendServiceRegion(v interface{}, d tpgresource.Terraf
 }
 
 func resourceComputeRegionBackendServiceEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) {
-	// The RegionBackendService API's Update / PUT API is badly formed and behaves like
-	// a PATCH field for at least IAP. When sent a `null` `iap` field, the API
-	// doesn't disable an existing field. To work around this, we need to emulate
-	// the old Terraform behaviour of always sending the block (at both update and
-	// create), and force sending each subfield as empty when the block isn't
-	// present in config.
-
-	iapVal := obj["iap"]
-	if iapVal == nil {
-		data := map[string]interface{}{}
-		data["enabled"] = false
-		obj["iap"] = data
-	} else {
-		iap := iapVal.(map[string]interface{})
-		iap["enabled"] = true
-		obj["iap"] = iap
-	}
 
 	if d.Get("load_balancing_scheme").(string) == "EXTERNAL_MANAGED" || d.Get("load_balancing_scheme").(string) == "INTERNAL_MANAGED" {
 		return obj, nil
@@ -4271,21 +4276,10 @@ func resourceComputeRegionBackendServiceEncoder(d *schema.ResourceData, meta int
 }
 
 func resourceComputeRegionBackendServiceDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
-	// We need to pretend IAP isn't there if it's disabled for Terraform to maintain
-	// BC behaviour with the handwritten resource.
-	v, ok := res["iap"]
-	if !ok || v == nil {
-		delete(res, "iap")
-		return res, nil
-	}
-	m := v.(map[string]interface{})
-	if ok && m["enabled"] == false {
-		delete(res, "iap")
-	}
 
 	// Since we add in a NONE subsetting policy, we need to remove it in some
 	// cases for backwards compatibility with the config
-	v, ok = res["subsetting"]
+	v, ok := res["subsetting"]
 	if ok && v != nil {
 		subsetting := v.(map[string]interface{})
 		policy, ok := subsetting["policy"]

diff --git a/google-beta/services/compute/resource_compute_region_backend_service_generated_test.go b/google-beta/services/compute/resource_compute_region_backend_service_generated_test.go
@@ -110,6 +110,7 @@ resource "google_compute_region_backend_service" "default" {
   protocol              = "HTTP"
   load_balancing_scheme = "EXTERNAL"
   iap {
+    enabled              = true
     oauth2_client_id     = "abc"
     oauth2_client_secret = "xyz"
   }

diff --git a/google-beta/services/compute/resource_compute_region_backend_service_test.go b/google-beta/services/compute/resource_compute_region_backend_service_test.go
@@ -262,6 +262,14 @@ func TestAccComputeRegionBackendService_withBackendAndIAP(t *testing.T) {
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
 		CheckDestroy:             testAccCheckComputeRegionBackendServiceDestroyProducer(t),
 		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeRegionBackendService_ilbBasic(backendName, checkName),
+			},
+			{
+				ResourceName:      "google_compute_region_backend_service.foobar",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
 			{
 				Config: testAccComputeRegionBackendService_ilbBasicwithIAP(backendName, checkName),
 			},
@@ -271,14 +279,6 @@ func TestAccComputeRegionBackendService_withBackendAndIAP(t *testing.T) {
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
 			},
-			{
-				Config: testAccComputeRegionBackendService_ilbBasic(backendName, checkName),
-			},
-			{
-				ResourceName:      "google_compute_region_backend_service.foobar",
-				ImportState:       true,
-				ImportStateVerify: true,
-			},
 		},
 	})
 }
@@ -1036,6 +1036,7 @@ resource "google_compute_region_backend_service" "foobar" {
   }
 
   iap {
+    enabled              = true
     oauth2_client_id     = "test"
     oauth2_client_secret = "test"
   }

diff --git a/website/docs/guides/version_6_upgrade.html.markdown b/website/docs/guides/version_6_upgrade.html.markdown
@@ -119,3 +119,15 @@ Removed in favor of field `settings.ip_configuration.ssl_mode`.
 ### `schema_settings` no longer has a default value
 
 An empty value means the setting should be cleared.
+
+## Resource: `google_compute_backend_service`
+
+### `iap.enabled` is now required in the `iap` block
+
+To apply the IAP settings to the backend service, `true` needs to be set for `enabled` field.
+
+## Resource: `google_compute_region_backend_service`
+
+### `iap.enabled` is now required in the `iap` block
+
+To apply the IAP settings to the backend service, `true` needs to be set for `enabled` field.
diff --git a/website/docs/r/compute_backend_service.html.markdown b/website/docs/r/compute_backend_service.html.markdown
@@ -73,6 +73,7 @@ resource "google_compute_backend_service" "default" {
   protocol              = "HTTP"
   load_balancing_scheme = "EXTERNAL"
   iap {
+    enabled              = true
     oauth2_client_id     = "abc"
     oauth2_client_secret = "xyz"
   }
@@ -913,12 +914,16 @@ The following arguments are supported:
 
 <a name="nested_iap"></a>The `iap` block supports:
 
-* `oauth2_client_id` -
+* `enabled` -
   (Required)
+  Whether the serving infrastructure will authenticate and authorize all incoming requests.
+
+* `oauth2_client_id` -
+  (Optional)
   OAuth2 Client ID for IAP
 
 * `oauth2_client_secret` -
-  (Required)
+  (Optional)
   OAuth2 Client Secret for IAP
   **Note**: This property is sensitive and will not be displayed in the plan.
 

diff --git a/website/docs/r/compute_region_backend_service.html.markdown b/website/docs/r/compute_region_backend_service.html.markdown
@@ -76,6 +76,7 @@ resource "google_compute_region_backend_service" "default" {
   protocol              = "HTTP"
   load_balancing_scheme = "EXTERNAL"
   iap {
+    enabled              = true
     oauth2_client_id     = "abc"
     oauth2_client_secret = "xyz"
   }
@@ -931,12 +932,16 @@ The following arguments are supported:
 
 <a name="nested_iap"></a>The `iap` block supports:
 
-* `oauth2_client_id` -
+* `enabled` -
   (Required)
+  Whether the serving infrastructure will authenticate and authorize all incoming requests.
+
+* `oauth2_client_id` -
+  (Optional)
   OAuth2 Client ID for IAP
 
 * `oauth2_client_secret` -
-  (Required)
+  (Optional)
   OAuth2 Client Secret for IAP
   **Note**: This property is sensitive and will not be displayed in the plan.