Skip to content

Commit

Permalink
Remove default addition of IAP message in encoder and decoder, add ne…
Browse files Browse the repository at this point in the history
…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 (GoogleCloudPlatform#9581)
  • Loading branch information
arnabadg-google authored and abd-goog committed Jul 30, 2024
1 parent e89cfb8 commit 5f36363
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 71 deletions.
6 changes: 4 additions & 2 deletions mmv1/products/compute/BackendService.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions mmv1/products/compute/RegionBackendService.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions mmv1/templates/terraform/decoders/backend_service.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 1 addition & 12 deletions mmv1/templates/terraform/decoders/region_backend_service.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
18 changes: 0 additions & 18 deletions mmv1/templates/terraform/encoders/backend_service.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions mmv1/templates/terraform/encoders/region_backend_service.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
})
Expand Down Expand Up @@ -1266,6 +1266,7 @@ resource "google_compute_backend_service" "lipsum" {
}

iap {
enabled = true
oauth2_client_id = "test"
oauth2_client_secret = "test"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
})
Expand Down Expand Up @@ -1048,6 +1048,7 @@ resource "google_compute_region_backend_service" "foobar" {
}

iap {
enabled = true
oauth2_client_id = "test"
oauth2_client_secret = "test"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,15 @@ Terraform from destroying or recreating the project. In 6.0.0, existing projects

**`deletion_policy` does NOT prevent deletion outside of Terraform.**

Setting `deletion_policy` to `ABANDON` allows the resource to be abandoned rather than deleted. To disable any kind of deletion protection, explicitly set this field to `NONE` in configuration and then run `terraform apply` to apply the change.
Setting `deletion_policy` to `ABANDON` allows the resource to be abandoned rather than deleted. To disable any kind of deletion protection, explicitly set this field to `NONE` in configuration and then run `terraform apply` to apply the change.
## 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.

0 comments on commit 5f36363

Please sign in to comment.