From c6b8e947e73c5d3b7d851a34c4f68d9a6f426cd2 Mon Sep 17 00:00:00 2001 From: The Magician Date: Thu, 18 Jul 2024 10:44:56 -0700 Subject: [PATCH] Remove default addition of IAP message in encoder and decoder, add new 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 --- .changelog/9581.txt | 6 ++ .../resource_compute_backend_service.go | 56 ++++++++----------- ..._compute_backend_service_generated_test.go | 1 + .../resource_compute_backend_service_test.go | 19 ++++--- ...resource_compute_region_backend_service.go | 56 +++++++++---------- ...e_region_backend_service_generated_test.go | 1 + ...rce_compute_region_backend_service_test.go | 17 +++--- .../guides/version_6_upgrade.html.markdown | 12 ++++ .../r/compute_backend_service.html.markdown | 9 ++- ...mpute_region_backend_service.html.markdown | 9 ++- 10 files changed, 102 insertions(+), 84 deletions(-) create mode 100644 .changelog/9581.txt diff --git a/.changelog/9581.txt b/.changelog/9581.txt new file mode 100644 index 0000000000..13321089f7 --- /dev/null +++ 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` +``` \ No newline at end of file diff --git a/google-beta/services/compute/resource_compute_backend_service.go b/google-beta/services/compute/resource_compute_backend_service.go index 707596f0ea..baf3b9bd48 100644 --- 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 index dc715aacb8..cf454f916a 100644 --- 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 index d749014952..f6556c6a2d 100644 --- 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 index 1d1c245285..9fd2d13d8d 100644 --- 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 index 009282a065..04b3db384b 100644 --- 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 index 2af6f33ab6..d53f77c022 100644 --- 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 index f9c6a0f396..519be56474 100644 --- 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 index 0b5a2a4bfc..5842db5a12 100644 --- 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: 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 index d97735c752..365f29f602 100644 --- 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: 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.