From d37e993f39f7ec7556b57707d34742695c742f8e Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Tue, 13 Aug 2024 21:14:15 +0000 Subject: [PATCH] add send_secondary_ip_range_if_empty (#11410) Co-authored-by: Riley Karson [upstream:4472fffc4067e62b24edb37e22f4fe61dfcc555c] Signed-off-by: Modular Magician --- .changelog/11410.txt | 6 + .../compute/resource_compute_subnetwork.go | 123 ++++++++++++- .../resource_compute_subnetwork_test.go | 171 ++++++++++++++++++ .../guides/version_6_upgrade.html.markdown | 10 + .../docs/r/compute_subnetwork.html.markdown | 13 +- 5 files changed, 315 insertions(+), 8 deletions(-) create mode 100644 .changelog/11410.txt diff --git a/.changelog/11410.txt b/.changelog/11410.txt new file mode 100644 index 0000000000..cea4bd9715 --- /dev/null +++ b/.changelog/11410.txt @@ -0,0 +1,6 @@ +```release-note:deprecation +compute: setting `google_compute_subnetwork.secondary_ip_range = []` to explicitly set a list of empty objects is deprecated and will produce an error in the upcoming major release. Use `send_secondary_ip_range_if_empty` while removing `secondary_ip_range` from config instead. +``` +```release-note:enhancement +compute: added `send_secondary_ip_range_if_empty` to `google_compute_subnetwork` +``` \ No newline at end of file diff --git a/google-beta/services/compute/resource_compute_subnetwork.go b/google-beta/services/compute/resource_compute_subnetwork.go index f81f049fd1..dbde728a5b 100644 --- a/google-beta/services/compute/resource_compute_subnetwork.go +++ b/google-beta/services/compute/resource_compute_subnetwork.go @@ -55,6 +55,37 @@ func IsShrinkageIpCidr(_ context.Context, old, new, _ interface{}) bool { return true } +func sendSecondaryIpRangeIfEmptyDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error { + // on create, return immediately as we don't need to determine if the value is empty or not + if diff.Id() == "" { + return nil + } + + sendZero := diff.Get("send_secondary_ip_range_if_empty").(bool) + if !sendZero { + return nil + } + + configSecondaryIpRange := diff.GetRawConfig().GetAttr("secondary_ip_range") + if !configSecondaryIpRange.IsKnown() { + return nil + } + configValueIsEmpty := configSecondaryIpRange.IsNull() || configSecondaryIpRange.LengthInt() == 0 + + stateSecondaryIpRange := diff.GetRawState().GetAttr("secondary_ip_range") + if !stateSecondaryIpRange.IsKnown() { + return nil + } + stateValueIsEmpty := stateSecondaryIpRange.IsNull() || stateSecondaryIpRange.LengthInt() == 0 + + if configValueIsEmpty && !stateValueIsEmpty { + log.Printf("[DEBUG] setting secondary_ip_range to newly empty") + diff.SetNew("secondary_ip_range", make([]interface{}, 0)) + } + + return nil +} + func ResourceComputeSubnetwork() *schema.Resource { return &schema.Resource{ Create: resourceComputeSubnetworkCreate, @@ -75,6 +106,7 @@ func ResourceComputeSubnetwork() *schema.Resource { CustomizeDiff: customdiff.All( resourceComputeSubnetworkSecondaryIpRangeSetStyleDiff, customdiff.ForceNewIfChange("ip_cidr_range", IsShrinkageIpCidr), + sendSecondaryIpRangeIfEmptyDiff, tpgresource.DefaultProviderProject, ), @@ -260,10 +292,8 @@ to the primary ipCidrRange of the subnetwork. The alias IPs may belong to either primary or secondary ranges. **Note**: This field uses [attr-as-block mode](https://www.terraform.io/docs/configuration/attr-as-blocks.html) to avoid -breaking users during the 0.12 upgrade. To explicitly send a list -of zero objects you must use the following syntax: -'example=[]' -For more details about this behavior, see [this section](https://www.terraform.io/docs/configuration/attr-as-blocks.html#defining-a-fixed-object-collection-value).`, +breaking users during the 0.12 upgrade. To explicitly send a list of zero objects, +set 'send_secondary_ip_range_if_empty = true'`, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ip_cidr_range": { @@ -316,6 +346,16 @@ outside this subnetwork.`, Computed: true, Description: `The range of internal IPv6 addresses that are owned by this subnetwork.`, }, + "send_secondary_ip_range_if_empty": { + Type: schema.TypeBool, + Optional: true, + Description: `Controls the removal behavior of secondary_ip_range. +When false, removing secondary_ip_range from config will not produce a diff as +the provider will default to the API's value. +When true, the provider will treat removing secondary_ip_range as sending an +empty list of secondary IP ranges to the API. +Defaults to false.`, + }, "fingerprint": { Type: schema.TypeString, Computed: true, @@ -572,6 +612,7 @@ func resourceComputeSubnetworkRead(d *schema.ResourceData, meta interface{}) err return transport_tpg.HandleNotFoundError(err, d, fmt.Sprintf("ComputeSubnetwork %q", d.Id())) } + // Explicitly set virtual fields to default values if unset if err := d.Set("project", project); err != nil { return fmt.Errorf("Error reading Subnetwork: %s", err) } @@ -1040,6 +1081,78 @@ func resourceComputeSubnetworkUpdate(d *schema.ResourceData, meta interface{}) e d.Partial(false) + if v, ok := d.GetOk("send_secondary_ip_range_if_empty"); ok && v.(bool) { + if sv, ok := d.GetOk("secondary_ip_range"); ok { + configValue := d.GetRawConfig().GetAttr("secondary_ip_range") + stateValue := sv.([]interface{}) + if configValue.LengthInt() == 0 && len(stateValue) != 0 { + log.Printf("[DEBUG] Sending empty secondary_ip_range in update") + obj := make(map[string]interface{}) + obj["secondaryIpRanges"] = make([]interface{}, 0) + + // The rest is the same as the secondary_ip_range generated update code + // without the secondaryIpRangesProp logic + + getUrl, err := tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/subnetworks/{{name}}") + if err != nil { + return err + } + + // err == nil indicates that the billing_project value was found + if bp, err := tpgresource.GetBillingProject(d, config); err == nil { + billingProject = bp + } + + getRes, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{ + Config: config, + Method: "GET", + Project: billingProject, + RawURL: getUrl, + UserAgent: userAgent, + }) + if err != nil { + return transport_tpg.HandleNotFoundError(err, d, fmt.Sprintf("ComputeSubnetwork %q", d.Id())) + } + + obj["fingerprint"] = getRes["fingerprint"] + + url, err := tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/subnetworks/{{name}}") + if err != nil { + return err + } + + 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: "PATCH", + Project: billingProject, + RawURL: url, + UserAgent: userAgent, + Body: obj, + Timeout: d.Timeout(schema.TimeoutUpdate), + Headers: headers, + }) + if err != nil { + return fmt.Errorf("Error updating Subnetwork %q: %s", d.Id(), err) + } else { + log.Printf("[DEBUG] Finished updating Subnetwork %q: %#v", d.Id(), res) + } + + err = ComputeOperationWaitTime( + config, res, project, "Updating Subnetwork", userAgent, + d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return err + } + } + } + } return resourceComputeSubnetworkRead(d, meta) } @@ -1117,6 +1230,8 @@ func resourceComputeSubnetworkImport(d *schema.ResourceData, meta interface{}) ( } d.SetId(id) + // Explicitly set virtual fields to default values on import + return []*schema.ResourceData{d}, nil } diff --git a/google-beta/services/compute/resource_compute_subnetwork_test.go b/google-beta/services/compute/resource_compute_subnetwork_test.go index 2aeaafe740..961d742f35 100644 --- a/google-beta/services/compute/resource_compute_subnetwork_test.go +++ b/google-beta/services/compute/resource_compute_subnetwork_test.go @@ -205,6 +205,92 @@ func TestAccComputeSubnetwork_secondaryIpRanges(t *testing.T) { }) } +func TestAccComputeSubnetwork_secondaryIpRanges_sendEmpty(t *testing.T) { + t.Parallel() + + var subnetwork compute.Subnetwork + + cnName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + subnetworkName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckComputeSubnetworkDestroyProducer(t), + Steps: []resource.TestStep{ + // Start without secondary_ip_range at all + { + Config: testAccComputeSubnetwork_sendEmpty_removed(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + ), + }, + // Add one secondary_ip_range + { + Config: testAccComputeSubnetwork_sendEmpty_single(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + ), + }, + // Remove it with send_secondary_ip_range_if_empty = true + { + Config: testAccComputeSubnetwork_sendEmpty_removed(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasNotSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + ), + }, + // Check that empty block secondary_ip_range = [] is not different + { + Config: testAccComputeSubnetwork_sendEmpty_emptyBlock(cnName, subnetworkName, "true"), + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, + // Apply two secondary_ip_range + { + Config: testAccComputeSubnetwork_sendEmpty_double(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update2", "192.168.11.0/24"), + ), + }, + // Remove both with send_secondary_ip_range_if_empty = true + { + Config: testAccComputeSubnetwork_sendEmpty_removed(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasNotSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + testAccCheckComputeSubnetworkHasNotSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update2", "192.168.11.0/24"), + ), + }, + // Apply one secondary_ip_range + { + Config: testAccComputeSubnetwork_sendEmpty_single(cnName, subnetworkName, "false"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + ), + }, + // Check removing without send_secondary_ip_range_if_empty produces no diff (normal computed behavior) + { + Config: testAccComputeSubnetwork_sendEmpty_removed(cnName, subnetworkName, "false"), + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, + // Remove with empty block [] + { + Config: testAccComputeSubnetwork_sendEmpty_emptyBlock(cnName, subnetworkName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeSubnetworkExists(t, "google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork), + testAccCheckComputeSubnetworkHasNotSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"), + ), + }, + }, + }) +} + func TestAccComputeSubnetwork_flowLogs(t *testing.T) { t.Parallel() @@ -619,6 +705,91 @@ resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" `, cnName, subnetworkName) } +func testAccComputeSubnetwork_sendEmpty_removed(cnName, subnetworkName, sendEmpty string) string { + return fmt.Sprintf(` +resource "google_compute_network" "custom-test" { + name = "%s" + auto_create_subnetworks = false +} + +resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" { + name = "%s" + ip_cidr_range = "10.2.0.0/16" + region = "us-central1" + network = google_compute_network.custom-test.self_link + send_secondary_ip_range_if_empty = "%s" +} +`, cnName, subnetworkName, sendEmpty) +} + +func testAccComputeSubnetwork_sendEmpty_emptyBlock(cnName, subnetworkName, sendEmpty string) string { + return fmt.Sprintf(` +resource "google_compute_network" "custom-test" { + name = "%s" + auto_create_subnetworks = false +} + +resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" { + name = "%s" + ip_cidr_range = "10.2.0.0/16" + region = "us-central1" + network = google_compute_network.custom-test.self_link + secondary_ip_range = [] + send_secondary_ip_range_if_empty = "%s" +} +`, cnName, subnetworkName, sendEmpty) +} + +func testAccComputeSubnetwork_sendEmpty_single(cnName, subnetworkName, sendEmpty string) string { + return fmt.Sprintf(` +resource "google_compute_network" "custom-test" { + name = "%s" + auto_create_subnetworks = false +} + +resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" { + name = "%s" + ip_cidr_range = "10.2.0.0/16" + region = "us-central1" + network = google_compute_network.custom-test.self_link + secondary_ip_range { + range_name = "tf-test-secondary-range-update2" + ip_cidr_range = "192.168.11.0/24" + } + secondary_ip_range { + range_name = "tf-test-secondary-range-update1" + ip_cidr_range = "192.168.10.0/24" + } + send_secondary_ip_range_if_empty = "%s" +} +`, cnName, subnetworkName, sendEmpty) +} + +func testAccComputeSubnetwork_sendEmpty_double(cnName, subnetworkName, sendEmpty string) string { + return fmt.Sprintf(` +resource "google_compute_network" "custom-test" { + name = "%s" + auto_create_subnetworks = false +} + +resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" { + name = "%s" + ip_cidr_range = "10.2.0.0/16" + region = "us-central1" + network = google_compute_network.custom-test.self_link + secondary_ip_range { + range_name = "tf-test-secondary-range-update2" + ip_cidr_range = "192.168.11.0/24" + } + secondary_ip_range { + range_name = "tf-test-secondary-range-update1" + ip_cidr_range = "192.168.10.0/24" + } + send_secondary_ip_range_if_empty = "%s" +} +`, cnName, subnetworkName, sendEmpty) +} + func testAccComputeSubnetwork_flowLogs(cnName, subnetworkName string) string { return fmt.Sprintf(` resource "google_compute_network" "custom-test" { diff --git a/website/docs/guides/version_6_upgrade.html.markdown b/website/docs/guides/version_6_upgrade.html.markdown index 1d47ed35f1..be00dc38c1 100644 --- a/website/docs/guides/version_6_upgrade.html.markdown +++ b/website/docs/guides/version_6_upgrade.html.markdown @@ -174,6 +174,16 @@ Previously, `containers.env` was a list, making it order-dependent. It is now a If you were relying on accessing an individual environment variable by index (for example, `google_cloud_run_v2_service.template.containers.0.env.0.name`), then that will now need to by hash (for example, `google_cloud_run_v2_service.template.containers.0.env..name`). +## Resource: `google_compute_subnetwork` + +### `secondary_ip_range = []` is no longer valid configuration + +To explicitly set an empty list of objects, use `send_secondary_ip_range_if_empty = true` and completely remove `secondary_ip_range` from config. + +Previously, to explicitly set `secondary_ip_range` as an empty list of objects, the specific configuration `secondary_ip_range = []` was necessary. +This was to maintain compatability in behavior between Terraform versions 0.11 and 0.12 using a special setting ["attributes as blocks"](https://developer.hashicorp.com/terraform/language/attr-as-blocks). +This special setting causes other breakages so it is now removed, with `send_secondary_ip_range_if_empty` available instead. + ## Resource: `google_compute_backend_service` ## Resource: `google_compute_region_backend_service` diff --git a/website/docs/r/compute_subnetwork.html.markdown b/website/docs/r/compute_subnetwork.html.markdown index a45ba97b9a..c9c8552ace 100644 --- a/website/docs/r/compute_subnetwork.html.markdown +++ b/website/docs/r/compute_subnetwork.html.markdown @@ -299,10 +299,8 @@ The following arguments are supported: to the primary ipCidrRange of the subnetwork. The alias IPs may belong to either primary or secondary ranges. **Note**: This field uses [attr-as-block mode](https://www.terraform.io/docs/configuration/attr-as-blocks.html) to avoid - breaking users during the 0.12 upgrade. To explicitly send a list - of zero objects you must use the following syntax: - `example=[]` - For more details about this behavior, see [this section](https://www.terraform.io/docs/configuration/attr-as-blocks.html#defining-a-fixed-object-collection-value). + breaking users during the 0.12 upgrade. To explicitly send a list of zero objects, + set `send_secondary_ip_range_if_empty = true` Structure is [documented below](#nested_secondary_ip_range). * `private_ip_google_access` - @@ -353,6 +351,13 @@ The following arguments are supported: * `project` - (Optional) The ID of the project in which the resource belongs. If it is not provided, the provider project is used. +* `send_secondary_ip_range_if_empty` - (Optional) Controls the removal behavior of secondary_ip_range. +When false, removing secondary_ip_range from config will not produce a diff as +the provider will default to the API's value. +When true, the provider will treat removing secondary_ip_range as sending an +empty list of secondary IP ranges to the API. +Defaults to false. + The `secondary_ip_range` block supports: