Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made nested query resources replace the whole resource instead of merging new over old #21721

Merged
merged 1 commit into from
Mar 5, 2025
Merged

Made nested query resources replace the whole resource instead of merging new over old #21721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/13234.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
```
36 changes: 32 additions & 4 deletions google/services/compute/resource_compute_router_nat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand Down
19 changes: 15 additions & 4 deletions google/services/compute/resource_compute_router_nat_address.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand Down
130 changes: 30 additions & 100 deletions google/services/compute/resource_compute_router_nat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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" {
Expand All @@ -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)
}
Expand All @@ -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" {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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" {
Expand Down