diff --git a/.changelog/13234.txt b/.changelog/13234.txt new file mode 100644 index 00000000000..fc7dee062ab --- /dev/null +++ b/.changelog/13234.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed bug in `google_compute_router_nat` where `max_ports_per_vm` couldn't be unset once set. +``` \ No newline at end of file diff --git a/google/services/compute/resource_compute_router_nat.go b/google/services/compute/resource_compute_router_nat.go index abc4a6c7d2e..ab8b2ec479c 100644 --- a/google/services/compute/resource_compute_router_nat.go +++ b/google/services/compute/resource_compute_router_nat.go @@ -1816,11 +1816,39 @@ func resourceComputeRouterNatPatchUpdateEncoder(d *schema.ResourceData, meta int return nil, fmt.Errorf("Unable to update RouterNat %q - not found in list", d.Id()) } - // Merge new object into old. - for k, v := range obj { - item[k] = v + // Copy over values for immutable fields + obj["name"] = item["name"] + obj["initialNatIps"] = item["initialNatIps"] + obj["endpointTypes"] = item["endpointTypes"] + // Merge any fields in item that aren't managed by this resource into obj + // This is necessary because item might be managed by multiple resources. + settableFields := map[string]struct{}{ + "natIpAllocateOption": struct{}{}, + "natIps": struct{}{}, + "drainNatIps": struct{}{}, + "sourceSubnetworkIpRangesToNat": struct{}{}, + "subnetworks": struct{}{}, + "minPortsPerVm": struct{}{}, + "maxPortsPerVm": struct{}{}, + "enableDynamicPortAllocation": struct{}{}, + "udpIdleTimeoutSec": struct{}{}, + "icmpIdleTimeoutSec": struct{}{}, + "tcpEstablishedIdleTimeoutSec": struct{}{}, + "tcpTransitoryIdleTimeoutSec": struct{}{}, + "tcpTimeWaitTimeoutSec": struct{}{}, + "logConfig": struct{}{}, + "rules": struct{}{}, + "enableEndpointIndependentMapping": struct{}{}, + "autoNetworkTier": struct{}{}, + } + for k, v := range item { + if _, ok := settableFields[k]; !ok { + obj[k] = v + } } - items[idx] = item + + // Override old object with new + items[idx] = obj // Return list with new item added res := map[string]interface{}{ diff --git a/google/services/compute/resource_compute_router_nat_address.go b/google/services/compute/resource_compute_router_nat_address.go index adaec2b86c5..f9b72a70bbf 100644 --- a/google/services/compute/resource_compute_router_nat_address.go +++ b/google/services/compute/resource_compute_router_nat_address.go @@ -733,11 +733,22 @@ func resourceComputeRouterNatAddressPatchUpdateEncoder(d *schema.ResourceData, m return nil, fmt.Errorf("Unable to update RouterNatAddress %q - not found in list", d.Id()) } - // Merge new object into old. - for k, v := range obj { - item[k] = v + // Copy over values for immutable fields + obj["name"] = item["name"] + // Merge any fields in item that aren't managed by this resource into obj + // This is necessary because item might be managed by multiple resources. + settableFields := map[string]struct{}{ + "natIps": struct{}{}, + "drainNatIps": struct{}{}, + } + for k, v := range item { + if _, ok := settableFields[k]; !ok { + obj[k] = v + } } - items[idx] = item + + // Override old object with new + items[idx] = obj // Return list with new item added res := map[string]interface{}{ diff --git a/google/services/compute/resource_compute_router_nat_test.go b/google/services/compute/resource_compute_router_nat_test.go index 8fcddcf8961..19e23774ea3 100644 --- a/google/services/compute/resource_compute_router_nat_test.go +++ b/google/services/compute/resource_compute_router_nat_test.go @@ -7,6 +7,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" @@ -83,6 +84,11 @@ func TestAccComputeRouterNat_update(t *testing.T) { }, { Config: testAccComputeRouterNatUpdated(routerName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate), + }, + }, }, { ResourceName: "google_compute_router_nat.foobar", @@ -91,6 +97,11 @@ func TestAccComputeRouterNat_update(t *testing.T) { }, { Config: testAccComputeRouterNatUpdateToNatIPsId(routerName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop), + }, + }, }, { ResourceName: "google_compute_router_nat.foobar", @@ -99,6 +110,11 @@ func TestAccComputeRouterNat_update(t *testing.T) { }, { Config: testAccComputeRouterNatUpdateToNatIPsName(routerName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop), + }, + }, }, { ResourceName: "google_compute_router_nat.foobar", @@ -107,37 +123,11 @@ func TestAccComputeRouterNat_update(t *testing.T) { }, { Config: testAccComputeRouterNatBasicBeforeUpdate(routerName), - }, - { - ResourceName: "google_compute_router_nat.foobar", - ImportState: true, - ImportStateVerify: true, - }, - }, - }) -} - -func TestAccComputeRouterNat_removeLogConfig(t *testing.T) { - t.Parallel() - - testId := acctest.RandString(t, 10) - routerName := fmt.Sprintf("tf-test-router-nat-%s", testId) - - acctest.VcrTest(t, resource.TestCase{ - PreCheck: func() { acctest.AccTestPreCheck(t) }, - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), - CheckDestroy: testAccCheckComputeRouterNatDestroyProducer(t), - Steps: []resource.TestStep{ - { - Config: testAccComputeRouterNatLogConfig(routerName), - }, - { - ResourceName: "google_compute_router_nat.foobar", - ImportState: true, - ImportStateVerify: true, - }, - { - Config: testAccComputeRouterNatLogConfigRemoved(routerName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate), + }, + }, }, { ResourceName: "google_compute_router_nat.foobar", @@ -759,6 +749,7 @@ resource "google_compute_router" "foobar" { resource "google_compute_network" "foobar" { name = "%s-net" + auto_create_subnetworks = false } resource "google_compute_subnetwork" "foobar" { @@ -780,11 +771,6 @@ resource "google_compute_router_nat" "foobar" { nat_ip_allocate_option = "AUTO_ONLY" nat_ips = [] source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES" - - log_config { - enable = true - filter = "ERRORS_ONLY" - } } `, routerName, routerName, routerName, routerName, routerName) } @@ -799,6 +785,7 @@ resource "google_compute_router" "foobar" { resource "google_compute_network" "foobar" { name = "%s-net" + auto_create_subnetworks = false } resource "google_compute_subnetwork" "foobar" { @@ -833,6 +820,7 @@ resource "google_compute_router_nat" "foobar" { tcp_established_idle_timeout_sec = 1600 tcp_transitory_idle_timeout_sec = 60 tcp_time_wait_timeout_sec = 60 + max_ports_per_vm = 128 log_config { enable = true @@ -885,7 +873,8 @@ network = google_compute_network.foobar.self_link } resource "google_compute_network" "foobar" { -name = "%s-net" + name = "%s-net" + auto_create_subnetworks = false } resource "google_compute_subnetwork" "foobar" { name = "%s-subnet" @@ -919,6 +908,7 @@ resource "google_compute_router_nat" "foobar" { tcp_established_idle_timeout_sec = 1600 tcp_transitory_idle_timeout_sec = 60 tcp_time_wait_timeout_sec = 60 + max_ports_per_vm = 128 log_config { enable = true @@ -937,7 +927,8 @@ network = google_compute_network.foobar.self_link } resource "google_compute_network" "foobar" { -name = "%s-net" + name = "%s-net" + auto_create_subnetworks = false } resource "google_compute_subnetwork" "foobar" { name = "%s-subnet" @@ -971,6 +962,7 @@ resource "google_compute_router_nat" "foobar" { tcp_established_idle_timeout_sec = 1600 tcp_transitory_idle_timeout_sec = 60 tcp_time_wait_timeout_sec = 60 + max_ports_per_vm = 128 log_config { enable = true @@ -1465,68 +1457,6 @@ resource "google_compute_router" "foobar" { `, routerName, routerName, routerName) } -func testAccComputeRouterNatLogConfig(routerName string) string { - return fmt.Sprintf(` -resource "google_compute_network" "foobar" { - name = "%s-net" -} - -resource "google_compute_subnetwork" "foobar" { - name = "%s-subnet" - network = google_compute_network.foobar.self_link - ip_cidr_range = "10.0.0.0/16" - region = "us-central1" -} - -resource "google_compute_router" "foobar" { - name = "%s" - region = google_compute_subnetwork.foobar.region - network = google_compute_network.foobar.self_link -} - -resource "google_compute_router_nat" "foobar" { - name = "%s" - router = google_compute_router.foobar.name - region = google_compute_router.foobar.region - nat_ip_allocate_option = "AUTO_ONLY" - source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES" - log_config { - enable = false - filter = "ALL" - } -} -`, routerName, routerName, routerName, routerName) -} - -func testAccComputeRouterNatLogConfigRemoved(routerName string) string { - return fmt.Sprintf(` -resource "google_compute_network" "foobar" { - name = "%s-net" -} - -resource "google_compute_subnetwork" "foobar" { - name = "%s-subnet" - network = google_compute_network.foobar.self_link - ip_cidr_range = "10.0.0.0/16" - region = "us-central1" -} - -resource "google_compute_router" "foobar" { - name = "%s" - region = google_compute_subnetwork.foobar.region - network = google_compute_network.foobar.self_link -} - -resource "google_compute_router_nat" "foobar" { - name = "%s" - router = google_compute_router.foobar.name - region = google_compute_router.foobar.region - nat_ip_allocate_option = "AUTO_ONLY" - source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES" -} -`, routerName, routerName, routerName, routerName) -} - func testAccComputeRouterNatBaseResourcesWithPrivateNatSubnetworks(routerName, hubName string) string { return fmt.Sprintf(` resource "google_compute_network" "foobar" {