feat(accesscontextmanager): support in-place updates for IngressPolicy and EgressPolicy

mmv1/products/accesscontextmanager/ServicePerimeterEgressPolicy.yaml

@@ -24,9 +24,6 @@ description: |
   perimeter in certain contexts (e.g. to read data from a Cloud Storage bucket
   or query against a BigQuery dataset).
 
-  ~> **Note:** By default, updates to this resource will remove the EgressPolicy from the
-  from the perimeter and add it back in a non-atomic manner. To ensure that the new EgressPolicy
-  is added before the old one is removed, add a `lifecycle` block with `create_before_destroy = true` to this resource.
   ~> **Note:** If this resource is used alongside a `google_access_context_manager_service_perimeter` resource,
   the service perimeter resource must have a `lifecycle` block with `ignore_changes = [status[0].egress_policies]` so
   they don't fight over which egress rules should be in the policy.
@@ -38,11 +35,12 @@ docs:
 id_format: '{{perimeter}}'
 base_url: ''
 self_link: '{{perimeter}}'
-create_url: '{{perimeter}}'
+create_url: '{{perimeter}}?updateMask=status.egressPolicies'
 create_verb: 'PATCH'
-update_mask: true
+update_verb: 'PATCH'
+update_url: '{{perimeter}}?updateMask=status.egressPolicies'
 delete_verb: 'PATCH'
-immutable: true
+delete_url: '{{perimeter}}?updateMask=status.egressPolicies'
 mutex: '{{access_policy_id}}'
 import_format:
   - '{{perimeter}}'
@@ -228,6 +226,7 @@ properties:
                       corresponding `serviceName` in `ApiOperation`.
   - name: 'title'
     type: String
+    immutable: true
     description: |
       Human readable title. Must be unique within the perimeter. Does not affect behavior.
   - name: 'accessPolicyId'

mmv1/products/accesscontextmanager/ServicePerimeterIngressPolicy.yaml

@@ -25,9 +25,6 @@ description: |
   Individual ingress policies can be limited by restricting which services and/
   or actions they match using the ingressTo field.
 
-  ~> **Note:** By default, updates to this resource will remove the IngressPolicy from the
-  from the perimeter and add it back in a non-atomic manner. To ensure that the new IngressPolicy
-  is added before the old one is removed, add a `lifecycle` block with `create_before_destroy = true` to this resource.
   ~> **Note:** If this resource is used alongside a `google_access_context_manager_service_perimeter` resource,
   the service perimeter resource must have a `lifecycle` block with `ignore_changes = [status[0].ingress_policies]` so
   they don't fight over which ingress rules should be in the policy.
@@ -39,11 +36,12 @@ docs:
 id_format: '{{perimeter}}'
 base_url: ''
 self_link: '{{perimeter}}'
-create_url: '{{perimeter}}'
+create_url: '{{perimeter}}?updateMask=status.ingressPolicies'
 create_verb: 'PATCH'
-update_mask: true
+update_verb: 'PATCH'
+update_url: '{{perimeter}}?updateMask=status.ingressPolicies'
 delete_verb: 'PATCH'
-immutable: true
+delete_url: '{{perimeter}}?updateMask=status.ingressPolicies'
 mutex: '{{access_policy_id}}'
 import_format:
   - '{{perimeter}}'
@@ -231,6 +229,7 @@ properties:
                       corresponding `serviceName` in `ApiOperation`.
   - name: 'title'
     type: String
+    immutable: true
     description: |
       Human readable title. Must be unique within the perimeter. Does not affect behavior.
   - name: 'accessPolicyId'

mmv1/templates/terraform/nested_query.go.tmpl

@@ -130,10 +130,58 @@ func resource{{ $.ResourceName }}PatchUpdateEncoder(d *schema.ResourceData, meta
     return nil, err
   }
 
-  idx, item, err := resource{{ $.ResourceName }}FindNestedObjectInList(d, meta, items)
+  // Use the old values of identity fields to find the existing object in the
+  // API response. During an update, d.Get() returns the new desired values,
+  // but the API still has the old values. Using d.GetChange() ensures we
+  // search for the object as it currently exists in the API.
+  {{- range $idProp := $.GetIdentity }}
+  old{{ $idProp.TitlelizeProperty }}Raw, _ := d.GetChange("{{ underscore $idProp.Name }}")
+  {{-   if $.IsSettableProperty $idProp }}
+  expectedOld{{ $idProp.TitlelizeProperty }}, err := expandNested{{ $.ResourceName }}{{ $idProp.TitlelizeProperty }}(old{{ $idProp.TitlelizeProperty }}Raw, d, meta.(*transport_tpg.Config))
   if err != nil {
     return nil, err
   }
+  {{-  else }}
+  expectedOld{{ $idProp.TitlelizeProperty }} := old{{ $idProp.TitlelizeProperty }}Raw
+  {{-  end }}{{/* if $.IsSettableProperty $idProp */}}
+  expectedOldFlattened{{ $idProp.TitlelizeProperty }} := flattenNested{{ $.ResourceName }}{{ $idProp.TitlelizeProperty }}(expectedOld{{ $idProp.TitlelizeProperty }}, d, meta.(*transport_tpg.Config))
+  {{- end }}{{/* range $idProp := $.GetIdentity  */}}
+
+  // Search list for this resource using old identity values.
+  var idx int = -1
+  var item map[string]interface{}
+  for i, itemRaw := range items {
+    if itemRaw == nil {
+      continue
+    }
+    {{- if $.NestedQuery.IsListOfIds }}
+    // List response only contains the ID - construct a response object.
+    currItem := map[string]interface{}{
+      "{{ $.FirstIdentityProp.ApiName }}": itemRaw,
+    }
+    {{- else }}
+    currItem := itemRaw.(map[string]interface{})
+    {{- end }}
+    {{ if $.CustomCode.Decoder }}
+    // Decode list item before comparing.
+    currItem, err = resource{{ $.ResourceName }}Decoder(d, meta, currItem)
+    if err != nil {
+        return nil, err
+    }
+    {{- end }}
+  {{ range $prop := $.GetIdentity }}
+    item{{ $prop.TitlelizeProperty }} := flattenNested{{ $.ResourceName }}{{ $prop.TitlelizeProperty }}(currItem["{{ $prop.ApiName }}"], d, meta.(*transport_tpg.Config))
+    // IsEmptyValue check so that if one is nil and the other is "", that's considered a match
+    if !(tpgresource.IsEmptyValue(reflect.ValueOf(item{{ $prop.TitlelizeProperty }})) && tpgresource.IsEmptyValue(reflect.ValueOf(expectedOldFlattened{{ $prop.TitlelizeProperty }}))) && !reflect.DeepEqual(item{{ $prop.TitlelizeProperty }}, expectedOldFlattened{{ $prop.TitlelizeProperty }}) {
+      log.Printf("[DEBUG] Skipping item with {{ $prop.ApiName }}= %#v, looking for %#v)", item{{ $prop.TitlelizeProperty }}, expectedOldFlattened{{ $prop.TitlelizeProperty }})
+      continue
+    }
+  {{- end }}
+    log.Printf("[DEBUG] Found item for resource %q: %#v)", d.Id(), currItem)
+    idx = i
+    item = currItem
+    break
+  }
 
   // Return error if item to update does not exist.
   if item == nil {
@@ -172,7 +220,7 @@ func resource{{ $.ResourceName }}PatchUpdateEncoder(d *schema.ResourceData, meta
 {{- range $i, $k := $.NestedQuery.Keys }}
   {{- if ne $i 0 }}
   wrapped := map[string]interface{}{
-    "{{ $k }}": res,
+    "{{ index $.NestedQuery.Keys (sub (sub (len $.NestedQuery.Keys) $i) 1) }}": res,
   }
   res = wrapped
   {{- end }}

mmv1/third_party/terraform/services/accesscontextmanager/resource_access_context_manager_access_policy_test.go.tmpl

@@ -31,8 +31,10 @@ func TestAccAccessContextManager(t *testing.T) {
 		"access_levels":				testAccAccessContextManagerAccessLevels_basicTest,
 		"access_level_condition":			testAccAccessContextManagerAccessLevelCondition_basicTest,
 		"service_perimeter_egress_policy":		testAccAccessContextManagerServicePerimeterEgressPolicy_basicTest,
+		"service_perimeter_egress_policy_update":	testAccAccessContextManagerServicePerimeterEgressPolicy_updateTest,
 		"service_perimeter_dry_run_egress_policy":		testAccAccessContextManagerServicePerimeterDryRunEgressPolicy_basicTest,
 		"service_perimeter_ingress_policy":		testAccAccessContextManagerServicePerimeterIngressPolicy_basicTest,
+		"service_perimeter_ingress_policy_update":	testAccAccessContextManagerServicePerimeterIngressPolicy_updateTest,
 		"service_perimeter_dry_run_ingress_policy":		testAccAccessContextManagerServicePerimeterDryRunIngressPolicy_basicTest,
 		"service_perimeters":				testAccAccessContextManagerServicePerimeters_basicTest,
 		"gcp_user_access_binding":			testAccAccessContextManagerGcpUserAccessBinding_basicTest,

mmv1/third_party/terraform/services/accesscontextmanager/resource_access_context_manager_service_perimeter_egress_policy_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-provider-google/google/acctest"
 	"github.com/hashicorp/terraform-provider-google/google/envvar"
@@ -170,6 +171,81 @@ resource "google_access_context_manager_service_perimeter_egress_policy" "test-i
 	// Using an uppercase service account to test normalization of IAM principal casing
 }
 
+func testAccAccessContextManagerServicePerimeterEgressPolicy_updateTest(t *testing.T) {
+	org := envvar.GetTestOrgFromEnv(t)
+	policyTitle := acctest.RandString(t, 10)
+	perimeterTitle := "perimeter"
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAccessContextManagerServicePerimeterEgressPolicy_egressPolicyUpdate_step1(org, policyTitle, perimeterTitle),
+			},
+			{
+				Config: testAccAccessContextManagerServicePerimeterEgressPolicy_egressPolicyUpdate_step2(org, policyTitle, perimeterTitle),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(
+							"google_access_context_manager_service_perimeter_egress_policy.test-access1",
+							plancheck.ResourceActionUpdate,
+						),
+					},
+				},
+			},
+			{
+				Config: testAccAccessContextManagerServicePerimeterEgressPolicy_destroy(org, policyTitle, perimeterTitle),
+				Check:  testAccCheckAccessContextManagerServicePerimeterEgressPolicyDestroyProducer(t),
+			},
+		},
+	})
+}
+
+func testAccAccessContextManagerServicePerimeterEgressPolicy_egressPolicyUpdate_step1(org, policyTitle, perimeterTitleName string) string {
+	return fmt.Sprintf(`
+%s
+
+resource "google_access_context_manager_service_perimeter_egress_policy" "test-access1" {
+  perimeter = google_access_context_manager_service_perimeter.test-access.name
+  title     = "egress policy update test"
+  egress_from {
+    identity_type = "ANY_USER_ACCOUNT"
+  }
+  egress_to {
+    operations {
+      service_name = "storage.googleapis.com"
+      method_selectors {
+        method = "*"
+      }
+    }
+  }
+}
+`, testAccAccessContextManagerServicePerimeterEgressPolicy_destroy(org, policyTitle, perimeterTitleName))
+}
+
+func testAccAccessContextManagerServicePerimeterEgressPolicy_egressPolicyUpdate_step2(org, policyTitle, perimeterTitleName string) string {
+	return fmt.Sprintf(`
+%s
+
+resource "google_access_context_manager_service_perimeter_egress_policy" "test-access1" {
+  perimeter = google_access_context_manager_service_perimeter.test-access.name
+  title     = "egress policy update test"
+  egress_from {
+    identity_type = "ANY_IDENTITY"
+  }
+  egress_to {
+    operations {
+      service_name = "bigquery.googleapis.com"
+      method_selectors {
+        method = "BigQueryStorage.ReadRows"
+      }
+    }
+  }
+}
+`, testAccAccessContextManagerServicePerimeterEgressPolicy_destroy(org, policyTitle, perimeterTitleName))
+}
+
 func testAccAccessContextManagerServicePerimeterEgressPolicy_destroy(org, policyTitle, perimeterTitleName string) string {
 	return fmt.Sprintf(`
 resource "google_access_context_manager_access_policy" "test-access" {

mmv1/third_party/terraform/services/accesscontextmanager/resource_access_context_manager_service_perimeter_ingress_policy_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-provider-google/google/acctest"
 	"github.com/hashicorp/terraform-provider-google/google/envvar"
@@ -171,6 +172,83 @@ resource "google_access_context_manager_service_perimeter_ingress_policy" "test-
 	// Using an uppercase service account to test normalization of IAM principal casing
 }
 
+func testAccAccessContextManagerServicePerimeterIngressPolicy_updateTest(t *testing.T) {
+	org := envvar.GetTestOrgFromEnv(t)
+	policyTitle := acctest.RandString(t, 10)
+	perimeterTitle := "perimeter"
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAccessContextManagerServicePerimeterIngressPolicy_ingressPolicyUpdate_step1(org, policyTitle, perimeterTitle),
+			},
+			{
+				Config: testAccAccessContextManagerServicePerimeterIngressPolicy_ingressPolicyUpdate_step2(org, policyTitle, perimeterTitle),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(
+							"google_access_context_manager_service_perimeter_ingress_policy.test-access1",
+							plancheck.ResourceActionUpdate,
+						),
+					},
+				},
+			},
+			{
+				Config: testAccAccessContextManagerServicePerimeterIngressPolicy_destroy(org, policyTitle, perimeterTitle),
+				Check:  testAccCheckAccessContextManagerServicePerimeterIngressPolicyDestroyProducer(t),
+			},
+		},
+	})
+}
+
+func testAccAccessContextManagerServicePerimeterIngressPolicy_ingressPolicyUpdate_step1(org, policyTitle, perimeterTitleName string) string {
+	return fmt.Sprintf(`
+%s
+
+resource "google_access_context_manager_service_perimeter_ingress_policy" "test-access1" {
+  perimeter = google_access_context_manager_service_perimeter.test-access.name
+  title     = "ingress policy update test"
+  ingress_from {
+    identity_type = "ANY_IDENTITY"
+  }
+  ingress_to {
+    resources = ["*"]
+    operations {
+      service_name = "storage.googleapis.com"
+      method_selectors {
+        method = "google.storage.objects.create"
+      }
+    }
+  }
+}
+`, testAccAccessContextManagerServicePerimeterIngressPolicy_destroy(org, policyTitle, perimeterTitleName))
+}
+
+func testAccAccessContextManagerServicePerimeterIngressPolicy_ingressPolicyUpdate_step2(org, policyTitle, perimeterTitleName string) string {
+	return fmt.Sprintf(`
+%s
+
+resource "google_access_context_manager_service_perimeter_ingress_policy" "test-access1" {
+  perimeter = google_access_context_manager_service_perimeter.test-access.name
+  title     = "ingress policy update test"
+  ingress_from {
+    identity_type = "ANY_USER_ACCOUNT"
+  }
+  ingress_to {
+    resources = ["*"]
+    operations {
+      service_name = "bigquery.googleapis.com"
+      method_selectors {
+        method = "BigQueryStorage.ReadRows"
+      }
+    }
+  }
+}
+`, testAccAccessContextManagerServicePerimeterIngressPolicy_destroy(org, policyTitle, perimeterTitleName))
+}
+
 func testAccAccessContextManagerServicePerimeterIngressPolicy_destroy(org, policyTitle, perimeterTitleName string) string {
 	return fmt.Sprintf(`
 resource "google_access_context_manager_access_policy" "test-access" {