From 893191c5c92c835fddf596dfdd195e90e78ec9b4 Mon Sep 17 00:00:00 2001
From: Arnab Dasgupta <arnabadg@google.com>
Date: Mon, 4 Dec 2023 23:11:39 +0000
Subject: [PATCH 1/3] Remove default addition of IAP message and add new fleid
`enabled` under IAP to resource google_compute_region_backend_service
---
mmv1/products/compute/RegionBackendService.yaml | 6 ++++--
.../decoders/region_backend_service.go.erb | 13 +------------
.../encoders/region_backend_service.go.erb | 17 -----------------
.../region_backend_service_external_iap.tf.erb | 1 +
...e_compute_region_backend_service_test.go.erb | 9 +++++----
5 files changed, 11 insertions(+), 35 deletions(-)
diff --git a/mmv1/products/compute/RegionBackendService.yaml b/mmv1/products/compute/RegionBackendService.yaml
index 1663393aac5e..4015d498775c 100644
--- a/mmv1/products/compute/RegionBackendService.yaml
+++ b/mmv1/products/compute/RegionBackendService.yaml
@@ -754,14 +754,16 @@ properties:
description: Settings for enabling Cloud Identity Aware Proxy
send_empty_value: true
properties:
+ - !ruby/object:Api::Type::Boolean
+ name: 'enabled'
+ required: true
+ description: Whether the serving infrastructure will authenticate and authorize all incoming requests.
- !ruby/object:Api::Type::String
name: 'oauth2ClientId'
- required: true
description: |
OAuth2 Client ID for IAP
- !ruby/object:Api::Type::String
name: 'oauth2ClientSecret'
- required: true
description: |
OAuth2 Client Secret for IAP
send_empty_value: true
diff --git a/mmv1/templates/terraform/decoders/region_backend_service.go.erb b/mmv1/templates/terraform/decoders/region_backend_service.go.erb
index 7e7d747fd2b6..517866dd216e 100644
--- a/mmv1/templates/terraform/decoders/region_backend_service.go.erb
+++ b/mmv1/templates/terraform/decoders/region_backend_service.go.erb
@@ -12,22 +12,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-%>
-// 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")
-}
<% unless version == 'ga' -%>
// 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/mmv1/templates/terraform/encoders/region_backend_service.go.erb b/mmv1/templates/terraform/encoders/region_backend_service.go.erb
index 76d396b5a0c5..7612dc051f0a 100644
--- a/mmv1/templates/terraform/encoders/region_backend_service.go.erb
+++ b/mmv1/templates/terraform/encoders/region_backend_service.go.erb
@@ -12,23 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-%>
-// 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
diff --git a/mmv1/templates/terraform/examples/region_backend_service_external_iap.tf.erb b/mmv1/templates/terraform/examples/region_backend_service_external_iap.tf.erb
index ec9ca33f00b0..2f418bc0f037 100644
--- a/mmv1/templates/terraform/examples/region_backend_service_external_iap.tf.erb
+++ b/mmv1/templates/terraform/examples/region_backend_service_external_iap.tf.erb
@@ -4,6 +4,7 @@ resource "google_compute_region_backend_service" "<%= ctx[:primary_resource_id]
protocol = "HTTP"
load_balancing_scheme = "EXTERNAL"
iap {
+ enabled = true
oauth2_client_id = "abc"
oauth2_client_secret = "xyz"
}
diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_region_backend_service_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_region_backend_service_test.go.erb
index 6e6726e74f46..1e5f15a5e76b 100644
--- a/mmv1/third_party/terraform/services/compute/resource_compute_region_backend_service_test.go.erb
+++ b/mmv1/third_party/terraform/services/compute/resource_compute_region_backend_service_test.go.erb
@@ -263,22 +263,22 @@ func TestAccComputeRegionBackendService_withBackendAndIAP(t *testing.T) {
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckComputeRegionBackendServiceDestroyProducer(t),
Steps: []resource.TestStep{
- {
- Config: testAccComputeRegionBackendService_ilbBasicwithIAP(backendName, checkName),
+ {
+ Config: testAccComputeRegionBackendService_ilbBasic(backendName, checkName),
},
{
ResourceName: "google_compute_region_backend_service.foobar",
ImportState: true,
ImportStateVerify: true,
- ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
},
{
- Config: testAccComputeRegionBackendService_ilbBasic(backendName, checkName),
+ Config: testAccComputeRegionBackendService_ilbBasicwithIAP(backendName, checkName),
},
{
ResourceName: "google_compute_region_backend_service.foobar",
ImportState: true,
ImportStateVerify: true,
+ ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
},
},
})
@@ -1048,6 +1048,7 @@ resource "google_compute_region_backend_service" "foobar" {
}
iap {
+ enabled = true
oauth2_client_id = "test"
oauth2_client_secret = "test"
}
From 58ea9036482fc04a25d720c78bc9170f70bef6eb Mon Sep 17 00:00:00 2001
From: Arnab Dasgupta <arnabadg@google.com>
Date: Wed, 3 Jul 2024 15:15:40 +0000
Subject: [PATCH 2/3] Remove default addition of IAP message and add new
required fleid `enabled` under IAP to resource google_compute_backend_service
---
mmv1/products/compute/BackendService.yaml | 6 ++++--
.../terraform/decoders/backend_service.go.erb | 12 ------------
.../terraform/encoders/backend_service.go.erb | 18 ------------------
.../backend_service_external_iap.tf.erb | 1 +
...esource_compute_backend_service_test.go.erb | 7 ++++---
5 files changed, 9 insertions(+), 35 deletions(-)
diff --git a/mmv1/products/compute/BackendService.yaml b/mmv1/products/compute/BackendService.yaml
index 3a2b862d666b..9f8a767d8154 100644
--- a/mmv1/products/compute/BackendService.yaml
+++ b/mmv1/products/compute/BackendService.yaml
@@ -745,14 +745,16 @@ properties:
description: Settings for enabling Cloud Identity Aware Proxy
send_empty_value: true
properties:
+ - !ruby/object:Api::Type::Boolean
+ name: 'enabled'
+ required: true
+ description: Whether the serving infrastructure will authenticate and authorize all incoming requests.
- !ruby/object:Api::Type::String
name: 'oauth2ClientId'
- required: true
description: |
OAuth2 Client ID for IAP
- !ruby/object:Api::Type::String
name: 'oauth2ClientSecret'
- required: true
description: |
OAuth2 Client Secret for IAP
send_empty_value: true
diff --git a/mmv1/templates/terraform/decoders/backend_service.go.erb b/mmv1/templates/terraform/decoders/backend_service.go.erb
index 3e0796f151a5..2916928ce9bd 100644
--- a/mmv1/templates/terraform/decoders/backend_service.go.erb
+++ b/mmv1/templates/terraform/decoders/backend_service.go.erb
@@ -12,18 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-%>
-// 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/mmv1/templates/terraform/encoders/backend_service.go.erb b/mmv1/templates/terraform/encoders/backend_service.go.erb
index 66626bf17fe3..b018972bd0b2 100644
--- a/mmv1/templates/terraform/encoders/backend_service.go.erb
+++ b/mmv1/templates/terraform/encoders/backend_service.go.erb
@@ -12,24 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-%>
-// 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
diff --git a/mmv1/templates/terraform/examples/backend_service_external_iap.tf.erb b/mmv1/templates/terraform/examples/backend_service_external_iap.tf.erb
index 1679fa7c39e3..cf264f8565ca 100644
--- a/mmv1/templates/terraform/examples/backend_service_external_iap.tf.erb
+++ b/mmv1/templates/terraform/examples/backend_service_external_iap.tf.erb
@@ -3,6 +3,7 @@ resource "google_compute_backend_service" "<%= ctx[:primary_resource_id] %>" {
protocol = "HTTP"
load_balancing_scheme = "EXTERNAL"
iap {
+ enabled = true
oauth2_client_id = "abc"
oauth2_client_secret = "xyz"
}
diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_backend_service_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_backend_service_test.go.erb
index 8e1ecf66cb8b..31e61906671f 100644
--- a/mmv1/third_party/terraform/services/compute/resource_compute_backend_service_test.go.erb
+++ b/mmv1/third_party/terraform/services/compute/resource_compute_backend_service_test.go.erb
@@ -125,23 +125,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"},
},
{
- Config: testAccComputeBackendService_withBackend(
+ Config: testAccComputeBackendService_withBackendAndIAP(
serviceName, igName, itName, checkName, 10),
},
{
ResourceName: "google_compute_backend_service.lipsum",
ImportState: true,
ImportStateVerify: true,
+ ImportStateVerifyIgnore: []string{"iap.0.oauth2_client_secret"},
},
},
})
@@ -1266,6 +1266,7 @@ resource "google_compute_backend_service" "lipsum" {
}
iap {
+ enabled = true
oauth2_client_id = "test"
oauth2_client_secret = "test"
}
From b8afdcaf77b669344eaed81f011983cbadc86172 Mon Sep 17 00:00:00 2001
From: Arnab Dasgupta <arnabadg@google.com>
Date: Wed, 10 Jul 2024 21:44:06 +0000
Subject: [PATCH 3/3] Add upgrade guide entry
---
.../docs/guides/version_6_upgrade.html.markdown | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/mmv1/third_party/terraform/website/docs/guides/version_6_upgrade.html.markdown b/mmv1/third_party/terraform/website/docs/guides/version_6_upgrade.html.markdown
index b9f455585878..4b94e1406c35 100644
--- a/mmv1/third_party/terraform/website/docs/guides/version_6_upgrade.html.markdown
+++ b/mmv1/third_party/terraform/website/docs/guides/version_6_upgrade.html.markdown
@@ -113,3 +113,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.