make Firewall Policy Association mutable (GoogleCloudPlatform#13078)

NickElliot · c2thorn · JaylonmcShan03 · commit 9b55f1070e9d · 2025-03-30T08:38:01.000-05:00
Co-authored-by: Cameron Thornton &lt;camthornton@google.com&gt;
diff --git a/mmv1/products/compute/FirewallPolicyAssociation.yaml b/mmv1/products/compute/FirewallPolicyAssociation.yaml
@@ -28,7 +28,8 @@ self_link: 'locations/global/firewallPolicies/{{firewall_policy}}/getAssociation
 create_url: 'locations/global/firewallPolicies/{{firewall_policy}}/addAssociation'
 delete_url: 'locations/global/firewallPolicies/{{firewall_policy}}/removeAssociation?name={{name}}'
 delete_verb: 'POST'
-immutable: true
+update_url: 'locations/global/firewallPolicies/{{firewall_policy}}/addAssociation?replaceExistingAssociation=true'
+update_verb: 'POST'
 import_format:
   - 'locations/global/firewallPolicies/{{firewall_policy}}/associations/{{name}}'
   - '{{firewall_policy}}/{{name}}'
@@ -37,6 +38,7 @@ timeouts:
   update_minutes: 20
   delete_minutes: 20
 custom_code:
+  custom_update: 'templates/terraform/custom_update/firewall_policy_association_update.go.tmpl'
   pre_read: 'templates/terraform/pre_read/compute_firewall_policy_association.go.tmpl'
   post_create: 'templates/terraform/post_create/compute_firewall_policy_association_operation.go.tmpl'
   post_delete: 'templates/terraform/post_delete/compute_firewall_policy_association_operation.go.tmpl'
@@ -52,14 +54,28 @@ examples:
     test_env_vars:
       org_id: 'ORG_ID'
     exclude_test: true
+  - name: 'firewall_policy_association_swapover'
+    primary_resource_id: 'default'
+    vars:
+      policy_name: 'my-policy'
+      association_name: 'my-association'
+      folder_name: 'my-folder'
+    test_env_vars:
+      org_id: 'ORG_ID'
+    exclude_test: true
 parameters:
   - name: 'firewallPolicy'
     type: ResourceRef
     description: |
       The firewall policy of the resource.
+
+      This field can be updated to refer to a different Firewall Policy, which will create a new association from that new
+      firewall policy with the flag to override the existing attachmentTarget's policy association.
+
+      **Note** Due to potential risks with this operation it is *highly* recommended to use the `create_before_destroy` life cycle option
+      on your exisiting firewall policy so as to prevent a situation where your attachment target has no associated policy.
     ignore_read: true
     required: true
-    immutable: true
     diff_suppress_func: 'tpgresource.CompareResourceNames'
     custom_expand: 'templates/terraform/custom_expand/compute_firewall_policy_association.go.tmpl'
     resource: 'FirewallPolicy'
@@ -70,11 +86,13 @@ properties:
     description: |
       The name for an association.
     required: true
+    immutable: true
   - name: 'attachmentTarget'
     type: String
     description: |
       The target that the firewall policy is attached to.
     required: true
+    immutable: true
     diff_suppress_func: 'tpgresource.CompareSelfLinkOrResourceName'
   - name: 'shortName'
     type: String
diff --git a/mmv1/templates/terraform/custom_update/firewall_policy_association_update.go.tmpl b/mmv1/templates/terraform/custom_update/firewall_policy_association_update.go.tmpl
@@ -0,0 +1,74 @@
+userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
+	if err != nil {
+		return err
+	}
+
+	billingProject := ""
+
+	obj := make(map[string]interface{})
+	nameProp, err := expandComputeFirewallPolicyAssociationName(d.Get("name"), d, config)
+	if err != nil {
+		return err
+	} else if v, ok := d.GetOkExists("name"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, nameProp)) {
+		obj["name"] = nameProp
+	}
+	attachmentTargetProp, err := expandComputeFirewallPolicyAssociationAttachmentTarget(d.Get("attachment_target"), d, config)
+	if err != nil {
+		return err
+	} else if v, ok := d.GetOkExists("attachment_target"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, attachmentTargetProp)) {
+		obj["attachmentTarget"] = attachmentTargetProp
+	}
+	firewallPolicyProp, err := expandComputeFirewallPolicyAssociationFirewallPolicy(d.Get("firewall_policy"), d, config)
+	if err != nil {
+		return err
+	} else if v, ok := d.GetOkExists("firewall_policy"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, firewallPolicyProp)) {
+		obj["firewallPolicy"] = firewallPolicyProp
+	}
+
+	url, err := tpgresource.ReplaceVars(d, config, "{{"{{"}}ComputeBasePath{{"}}"}}locations/global/firewallPolicies/{{"{{"}}firewall_policy{{"}}"}}/addAssociation?replaceExistingAssociation=true")
+	if err != nil {
+		return err
+	}
+
+	log.Printf("[DEBUG] Updating FirewallPolicyAssociation %q: %#v", d.Id(), obj)
+	headers := make(http.Header)
+
+	// err == nil indicates that the billing_project value was found
+	if bp, err := tpgresource.GetBillingProject(d, config); err == nil {
+		billingProject = bp
+	}
+
+	res, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{
+		Config:    config,
+		Method:    "POST",
+		Project:   billingProject,
+		RawURL:    url,
+		UserAgent: userAgent,
+		Body:      obj,
+		Timeout:   d.Timeout(schema.TimeoutUpdate),
+		Headers:   headers,
+	})
+	//following section is the area of the `custom_update` function for this resource that is different
+	//`custom_update` was necessary over a `post_update"` because of the change to the error handler
+	if err != nil {
+		//before failing an update, restores the old firewall_policy value to prevent terraform state becoming broken
+		parts := strings.Split(d.Id(), "/")
+		oldPolicyPath := "locations/global/firewallPolicies/" + parts[len(parts)-3]
+		d.Set("firewall_policy", oldPolicyPath)
+		return fmt.Errorf("Error updating FirewallPolicyAssociation %q: %s", d.Id(), err)
+	} else {
+		log.Printf("[DEBUG] Finished updating FirewallPolicyAssociation %q: %#v", d.Id(), res)
+	}
+
+	// store the ID now, needed because this update function is changing a normally immutable URL parameter field
+	id, err := tpgresource.ReplaceVars(d, config, "locations/global/firewallPolicies/{{"{{"}}firewall_policy{{"}}"}}/associations/{{"{{"}}name{{"}}"}}")
+	if err != nil {
+		return fmt.Errorf("Error constructing id: %s", err)
+	}
+	d.SetId(id)
+
+	//following the swapover briefly both associations exist and this can cause operations to fail
+	time.Sleep(60 * time.Second)
+	//end `custom_update` changed zone
+
+	return resourceComputeFirewallPolicyAssociationRead(d, meta)
diff --git a/mmv1/templates/terraform/examples/firewall_policy_association_swapover.tf.tmpl b/mmv1/templates/terraform/examples/firewall_policy_association_swapover.tf.tmpl
@@ -0,0 +1,20 @@
+resource "google_folder" "folder" {
+  display_name = "{{index $.Vars "folder_name"}}"
+  parent       = "organizations/{{index $.TestEnvVars "org_id"}}"
+  deletion_protection = false
+}
+
+resource "google_compute_firewall_policy" "policy" {
+  parent      = "organizations/{{index $.TestEnvVars "org_id"}}"
+  short_name  = "{{index $.Vars "policy_name"}}" -> "{{index $.Vars "policy_name"}}-recreate"
+  description = "Example Resource"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "google_compute_firewall_policy_association" "{{$.PrimaryResourceId}}" {
+  firewall_policy = google_compute_firewall_policy.policy.id
+  attachment_target = google_folder.folder.name
+  name = "{{index $.Vars "association_name"}}"
+}
diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_firewall_policy_association_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_firewall_policy_association_test.go
@@ -112,3 +112,70 @@ resource "google_compute_firewall_policy_association" "default" {
 }
 `, context)
 }
+
+func TestAccComputeFirewallPolicyAssociation_swapover(t *testing.T) {
+	t.Parallel()
+
+	context := map[string]interface{}{
+		"random_suffix": acctest.RandString(t, 10),
+		"org_name":      fmt.Sprintf("organizations/%s", envvar.GetTestOrgFromEnv(t)),
+	}
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeFirewallPolicyAssociation_basic(context),
+			},
+			{
+				ResourceName:      "google_compute_firewall_policy_association.default",
+				ImportState:       true,
+				ImportStateVerify: true,
+				// Referencing using ID causes import to fail
+				ImportStateVerifyIgnore: []string{"firewall_policy"},
+			},
+			{
+				Config: testAccComputeFirewallPolicyAssociation_swapover(context),
+			},
+			{
+				ResourceName:      "google_compute_firewall_policy_association.default",
+				ImportState:       true,
+				ImportStateVerify: true,
+				// Referencing using ID causes import to fail
+				ImportStateVerifyIgnore: []string{"firewall_policy"},
+			},
+		},
+	})
+}
+
+func testAccComputeFirewallPolicyAssociation_swapover(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+resource "google_folder" "folder" {
+  display_name = "tf-test-folder-%{random_suffix}"
+  parent       = "%{org_name}"
+  deletion_protection = false
+}
+
+resource "google_folder" "target_folder" {
+  display_name = "tf-test-target-%{random_suffix}"
+  parent       = "%{org_name}"
+  deletion_protection = false
+}
+
+resource "google_compute_firewall_policy" "default" {
+  parent      = google_folder.folder.id
+  short_name  = "tf-test-policy-%{random_suffix}-swapover"
+  description = "Resource created for Terraform acceptance testing"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "google_compute_firewall_policy_association" "default" {
+  firewall_policy = google_compute_firewall_policy.default.id
+  attachment_target = google_folder.target_folder.name
+  name = "tf-test-association-%{random_suffix}"
+}
+`, context)
+}
