Skip to content

Commit 22c7cdb

Browse files
modular-magicianmodular-magician
committed
Made nested query resources replace the whole resource instead of merging new over old (#13234)
[upstream:2ba1acd7e865dce04fe3d0b4c1b136cbdb2f93e2] Signed-off-by: Modular Magician <[email protected]>
1 parent b94378b commit 22c7cdb

File tree

4 files changed

+81
-108
lines changed

4 files changed

+81
-108
lines changed

‎.changelog/13234.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
compute: fixed bug in `google_compute_router_nat` where `max_ports_per_vm` couldn't be unset once set.
3+
```

‎google-beta/services/compute/resource_compute_router_nat.go

+33-4
Original file line numberDiff line numberDiff line change
@@ -2009,11 +2009,40 @@ func resourceComputeRouterNatPatchUpdateEncoder(d *schema.ResourceData, meta int
20092009
return nil, fmt.Errorf("Unable to update RouterNat %q - not found in list", d.Id())
20102010
}
20112011

2012-
// Merge new object into old.
2013-
for k, v := range obj {
2014-
item[k] = v
2012+
// Copy over values for immutable fields
2013+
obj["name"] = item["name"]
2014+
obj["initialNatIps"] = item["initialNatIps"]
2015+
obj["endpointTypes"] = item["endpointTypes"]
2016+
obj["type"] = item["type"]
2017+
// Merge any fields in item that aren't managed by this resource into obj
2018+
// This is necessary because item might be managed by multiple resources.
2019+
settableFields := map[string]struct{}{
2020+
"natIpAllocateOption": struct{}{},
2021+
"natIps": struct{}{},
2022+
"drainNatIps": struct{}{},
2023+
"sourceSubnetworkIpRangesToNat": struct{}{},
2024+
"subnetworks": struct{}{},
2025+
"minPortsPerVm": struct{}{},
2026+
"maxPortsPerVm": struct{}{},
2027+
"enableDynamicPortAllocation": struct{}{},
2028+
"udpIdleTimeoutSec": struct{}{},
2029+
"icmpIdleTimeoutSec": struct{}{},
2030+
"tcpEstablishedIdleTimeoutSec": struct{}{},
2031+
"tcpTransitoryIdleTimeoutSec": struct{}{},
2032+
"tcpTimeWaitTimeoutSec": struct{}{},
2033+
"logConfig": struct{}{},
2034+
"rules": struct{}{},
2035+
"enableEndpointIndependentMapping": struct{}{},
2036+
"autoNetworkTier": struct{}{},
2037+
}
2038+
for k, v := range item {
2039+
if _, ok := settableFields[k]; !ok {
2040+
obj[k] = v
2041+
}
20152042
}
2016-
items[idx] = item
2043+
2044+
// Override old object with new
2045+
items[idx] = obj
20172046

20182047
// Return list with new item added
20192048
res := map[string]interface{}{

‎google-beta/services/compute/resource_compute_router_nat_address.go

+15-4
Original file line numberDiff line numberDiff line change
@@ -733,11 +733,22 @@ func resourceComputeRouterNatAddressPatchUpdateEncoder(d *schema.ResourceData, m
733733
return nil, fmt.Errorf("Unable to update RouterNatAddress %q - not found in list", d.Id())
734734
}
735735

736-
// Merge new object into old.
737-
for k, v := range obj {
738-
item[k] = v
736+
// Copy over values for immutable fields
737+
obj["name"] = item["name"]
738+
// Merge any fields in item that aren't managed by this resource into obj
739+
// This is necessary because item might be managed by multiple resources.
740+
settableFields := map[string]struct{}{
741+
"natIps": struct{}{},
742+
"drainNatIps": struct{}{},
743+
}
744+
for k, v := range item {
745+
if _, ok := settableFields[k]; !ok {
746+
obj[k] = v
747+
}
739748
}
740-
items[idx] = item
749+
750+
// Override old object with new
751+
items[idx] = obj
741752

742753
// Return list with new item added
743754
res := map[string]interface{}{

‎google-beta/services/compute/resource_compute_router_nat_test.go

+30-100
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
11+
"github.com/hashicorp/terraform-plugin-testing/plancheck"
1112
"github.com/hashicorp/terraform-plugin-testing/terraform"
1213
"github.com/hashicorp/terraform-provider-google-beta/google-beta/acctest"
1314
"github.com/hashicorp/terraform-provider-google-beta/google-beta/envvar"
@@ -84,6 +85,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
8485
},
8586
{
8687
Config: testAccComputeRouterNatUpdated(routerName),
88+
ConfigPlanChecks: resource.ConfigPlanChecks{
89+
PreApply: []plancheck.PlanCheck{
90+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate),
91+
},
92+
},
8793
},
8894
{
8995
ResourceName: "google_compute_router_nat.foobar",
@@ -92,6 +98,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
9298
},
9399
{
94100
Config: testAccComputeRouterNatUpdateToNatIPsId(routerName),
101+
ConfigPlanChecks: resource.ConfigPlanChecks{
102+
PreApply: []plancheck.PlanCheck{
103+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop),
104+
},
105+
},
95106
},
96107
{
97108
ResourceName: "google_compute_router_nat.foobar",
@@ -100,6 +111,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
100111
},
101112
{
102113
Config: testAccComputeRouterNatUpdateToNatIPsName(routerName),
114+
ConfigPlanChecks: resource.ConfigPlanChecks{
115+
PreApply: []plancheck.PlanCheck{
116+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop),
117+
},
118+
},
103119
},
104120
{
105121
ResourceName: "google_compute_router_nat.foobar",
@@ -108,37 +124,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
108124
},
109125
{
110126
Config: testAccComputeRouterNatBasicBeforeUpdate(routerName),
111-
},
112-
{
113-
ResourceName: "google_compute_router_nat.foobar",
114-
ImportState: true,
115-
ImportStateVerify: true,
116-
},
117-
},
118-
})
119-
}
120-
121-
func TestAccComputeRouterNat_removeLogConfig(t *testing.T) {
122-
t.Parallel()
123-
124-
testId := acctest.RandString(t, 10)
125-
routerName := fmt.Sprintf("tf-test-router-nat-%s", testId)
126-
127-
acctest.VcrTest(t, resource.TestCase{
128-
PreCheck: func() { acctest.AccTestPreCheck(t) },
129-
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
130-
CheckDestroy: testAccCheckComputeRouterNatDestroyProducer(t),
131-
Steps: []resource.TestStep{
132-
{
133-
Config: testAccComputeRouterNatLogConfig(routerName),
134-
},
135-
{
136-
ResourceName: "google_compute_router_nat.foobar",
137-
ImportState: true,
138-
ImportStateVerify: true,
139-
},
140-
{
141-
Config: testAccComputeRouterNatLogConfigRemoved(routerName),
127+
ConfigPlanChecks: resource.ConfigPlanChecks{
128+
PreApply: []plancheck.PlanCheck{
129+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate),
130+
},
131+
},
142132
},
143133
{
144134
ResourceName: "google_compute_router_nat.foobar",
@@ -1011,6 +1001,7 @@ resource "google_compute_router" "foobar" {
10111001
10121002
resource "google_compute_network" "foobar" {
10131003
name = "%s-net"
1004+
auto_create_subnetworks = false
10141005
}
10151006
10161007
resource "google_compute_subnetwork" "foobar" {
@@ -1032,11 +1023,6 @@ resource "google_compute_router_nat" "foobar" {
10321023
nat_ip_allocate_option = "AUTO_ONLY"
10331024
nat_ips = []
10341025
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
1035-
1036-
log_config {
1037-
enable = true
1038-
filter = "ERRORS_ONLY"
1039-
}
10401026
}
10411027
`, routerName, routerName, routerName, routerName, routerName)
10421028
}
@@ -1051,6 +1037,7 @@ resource "google_compute_router" "foobar" {
10511037
10521038
resource "google_compute_network" "foobar" {
10531039
name = "%s-net"
1040+
auto_create_subnetworks = false
10541041
}
10551042
10561043
resource "google_compute_subnetwork" "foobar" {
@@ -1085,6 +1072,7 @@ resource "google_compute_router_nat" "foobar" {
10851072
tcp_established_idle_timeout_sec = 1600
10861073
tcp_transitory_idle_timeout_sec = 60
10871074
tcp_time_wait_timeout_sec = 60
1075+
max_ports_per_vm = 128
10881076
10891077
log_config {
10901078
enable = true
@@ -1137,7 +1125,8 @@ network = google_compute_network.foobar.self_link
11371125
}
11381126
11391127
resource "google_compute_network" "foobar" {
1140-
name = "%s-net"
1128+
name = "%s-net"
1129+
auto_create_subnetworks = false
11411130
}
11421131
resource "google_compute_subnetwork" "foobar" {
11431132
name = "%s-subnet"
@@ -1171,6 +1160,7 @@ resource "google_compute_router_nat" "foobar" {
11711160
tcp_established_idle_timeout_sec = 1600
11721161
tcp_transitory_idle_timeout_sec = 60
11731162
tcp_time_wait_timeout_sec = 60
1163+
max_ports_per_vm = 128
11741164
11751165
log_config {
11761166
enable = true
@@ -1189,7 +1179,8 @@ network = google_compute_network.foobar.self_link
11891179
}
11901180
11911181
resource "google_compute_network" "foobar" {
1192-
name = "%s-net"
1182+
name = "%s-net"
1183+
auto_create_subnetworks = false
11931184
}
11941185
resource "google_compute_subnetwork" "foobar" {
11951186
name = "%s-subnet"
@@ -1223,6 +1214,7 @@ resource "google_compute_router_nat" "foobar" {
12231214
tcp_established_idle_timeout_sec = 1600
12241215
tcp_transitory_idle_timeout_sec = 60
12251216
tcp_time_wait_timeout_sec = 60
1217+
max_ports_per_vm = 128
12261218
12271219
log_config {
12281220
enable = true
@@ -1826,68 +1818,6 @@ resource "google_compute_router" "foobar" {
18261818
`, routerName, routerName, routerName)
18271819
}
18281820

1829-
func testAccComputeRouterNatLogConfig(routerName string) string {
1830-
return fmt.Sprintf(`
1831-
resource "google_compute_network" "foobar" {
1832-
name = "%s-net"
1833-
}
1834-
1835-
resource "google_compute_subnetwork" "foobar" {
1836-
name = "%s-subnet"
1837-
network = google_compute_network.foobar.self_link
1838-
ip_cidr_range = "10.0.0.0/16"
1839-
region = "us-central1"
1840-
}
1841-
1842-
resource "google_compute_router" "foobar" {
1843-
name = "%s"
1844-
region = google_compute_subnetwork.foobar.region
1845-
network = google_compute_network.foobar.self_link
1846-
}
1847-
1848-
resource "google_compute_router_nat" "foobar" {
1849-
name = "%s"
1850-
router = google_compute_router.foobar.name
1851-
region = google_compute_router.foobar.region
1852-
nat_ip_allocate_option = "AUTO_ONLY"
1853-
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
1854-
log_config {
1855-
enable = false
1856-
filter = "ALL"
1857-
}
1858-
}
1859-
`, routerName, routerName, routerName, routerName)
1860-
}
1861-
1862-
func testAccComputeRouterNatLogConfigRemoved(routerName string) string {
1863-
return fmt.Sprintf(`
1864-
resource "google_compute_network" "foobar" {
1865-
name = "%s-net"
1866-
}
1867-
1868-
resource "google_compute_subnetwork" "foobar" {
1869-
name = "%s-subnet"
1870-
network = google_compute_network.foobar.self_link
1871-
ip_cidr_range = "10.0.0.0/16"
1872-
region = "us-central1"
1873-
}
1874-
1875-
resource "google_compute_router" "foobar" {
1876-
name = "%s"
1877-
region = google_compute_subnetwork.foobar.region
1878-
network = google_compute_network.foobar.self_link
1879-
}
1880-
1881-
resource "google_compute_router_nat" "foobar" {
1882-
name = "%s"
1883-
router = google_compute_router.foobar.name
1884-
region = google_compute_router.foobar.region
1885-
nat_ip_allocate_option = "AUTO_ONLY"
1886-
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
1887-
}
1888-
`, routerName, routerName, routerName, routerName)
1889-
}
1890-
18911821
func testAccComputeRouterNatBaseResourcesWithPrivateNatSubnetworks(routerName, hubName string) string {
18921822
return fmt.Sprintf(`
18931823
resource "google_compute_network" "foobar" {

0 commit comments

Comments
 (0)