Skip to content

Commit dcae139

Browse files
modular-magicianmodular-magician
authored
Made nested query resources replace the whole resource instead of merging new over old (#13234) (#21721)
[upstream:2ba1acd7e865dce04fe3d0b4c1b136cbdb2f93e2] Signed-off-by: Modular Magician <magic-modules@google.com>
1 parent 010cbc1 commit dcae139

File tree

4 files changed

+80
-108
lines changed

4 files changed

+80
-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/services/compute/resource_compute_router_nat.go

+32-4
Original file line numberDiff line numberDiff line change
@@ -1816,11 +1816,39 @@ func resourceComputeRouterNatPatchUpdateEncoder(d *schema.ResourceData, meta int
18161816
return nil, fmt.Errorf("Unable to update RouterNat %q - not found in list", d.Id())
18171817
}
18181818

1819-
// Merge new object into old.
1820-
for k, v := range obj {
1821-
item[k] = v
1819+
// Copy over values for immutable fields
1820+
obj["name"] = item["name"]
1821+
obj["initialNatIps"] = item["initialNatIps"]
1822+
obj["endpointTypes"] = item["endpointTypes"]
1823+
// Merge any fields in item that aren't managed by this resource into obj
1824+
// This is necessary because item might be managed by multiple resources.
1825+
settableFields := map[string]struct{}{
1826+
"natIpAllocateOption": struct{}{},
1827+
"natIps": struct{}{},
1828+
"drainNatIps": struct{}{},
1829+
"sourceSubnetworkIpRangesToNat": struct{}{},
1830+
"subnetworks": struct{}{},
1831+
"minPortsPerVm": struct{}{},
1832+
"maxPortsPerVm": struct{}{},
1833+
"enableDynamicPortAllocation": struct{}{},
1834+
"udpIdleTimeoutSec": struct{}{},
1835+
"icmpIdleTimeoutSec": struct{}{},
1836+
"tcpEstablishedIdleTimeoutSec": struct{}{},
1837+
"tcpTransitoryIdleTimeoutSec": struct{}{},
1838+
"tcpTimeWaitTimeoutSec": struct{}{},
1839+
"logConfig": struct{}{},
1840+
"rules": struct{}{},
1841+
"enableEndpointIndependentMapping": struct{}{},
1842+
"autoNetworkTier": struct{}{},
1843+
}
1844+
for k, v := range item {
1845+
if _, ok := settableFields[k]; !ok {
1846+
obj[k] = v
1847+
}
18221848
}
1823-
items[idx] = item
1849+
1850+
// Override old object with new
1851+
items[idx] = obj
18241852

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

‎google/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/services/compute/resource_compute_router_nat_test.go

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

99
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
10+
"github.com/hashicorp/terraform-plugin-testing/plancheck"
1011
"github.com/hashicorp/terraform-plugin-testing/terraform"
1112
"github.com/hashicorp/terraform-provider-google/google/acctest"
1213
"github.com/hashicorp/terraform-provider-google/google/envvar"
@@ -83,6 +84,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
8384
},
8485
{
8586
Config: testAccComputeRouterNatUpdated(routerName),
87+
ConfigPlanChecks: resource.ConfigPlanChecks{
88+
PreApply: []plancheck.PlanCheck{
89+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate),
90+
},
91+
},
8692
},
8793
{
8894
ResourceName: "google_compute_router_nat.foobar",
@@ -91,6 +97,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
9197
},
9298
{
9399
Config: testAccComputeRouterNatUpdateToNatIPsId(routerName),
100+
ConfigPlanChecks: resource.ConfigPlanChecks{
101+
PreApply: []plancheck.PlanCheck{
102+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop),
103+
},
104+
},
94105
},
95106
{
96107
ResourceName: "google_compute_router_nat.foobar",
@@ -99,6 +110,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
99110
},
100111
{
101112
Config: testAccComputeRouterNatUpdateToNatIPsName(routerName),
113+
ConfigPlanChecks: resource.ConfigPlanChecks{
114+
PreApply: []plancheck.PlanCheck{
115+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionNoop),
116+
},
117+
},
102118
},
103119
{
104120
ResourceName: "google_compute_router_nat.foobar",
@@ -107,37 +123,11 @@ func TestAccComputeRouterNat_update(t *testing.T) {
107123
},
108124
{
109125
Config: testAccComputeRouterNatBasicBeforeUpdate(routerName),
110-
},
111-
{
112-
ResourceName: "google_compute_router_nat.foobar",
113-
ImportState: true,
114-
ImportStateVerify: true,
115-
},
116-
},
117-
})
118-
}
119-
120-
func TestAccComputeRouterNat_removeLogConfig(t *testing.T) {
121-
t.Parallel()
122-
123-
testId := acctest.RandString(t, 10)
124-
routerName := fmt.Sprintf("tf-test-router-nat-%s", testId)
125-
126-
acctest.VcrTest(t, resource.TestCase{
127-
PreCheck: func() { acctest.AccTestPreCheck(t) },
128-
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
129-
CheckDestroy: testAccCheckComputeRouterNatDestroyProducer(t),
130-
Steps: []resource.TestStep{
131-
{
132-
Config: testAccComputeRouterNatLogConfig(routerName),
133-
},
134-
{
135-
ResourceName: "google_compute_router_nat.foobar",
136-
ImportState: true,
137-
ImportStateVerify: true,
138-
},
139-
{
140-
Config: testAccComputeRouterNatLogConfigRemoved(routerName),
126+
ConfigPlanChecks: resource.ConfigPlanChecks{
127+
PreApply: []plancheck.PlanCheck{
128+
plancheck.ExpectResourceAction("google_compute_router_nat.foobar", plancheck.ResourceActionUpdate),
129+
},
130+
},
141131
},
142132
{
143133
ResourceName: "google_compute_router_nat.foobar",
@@ -759,6 +749,7 @@ resource "google_compute_router" "foobar" {
759749
760750
resource "google_compute_network" "foobar" {
761751
name = "%s-net"
752+
auto_create_subnetworks = false
762753
}
763754
764755
resource "google_compute_subnetwork" "foobar" {
@@ -780,11 +771,6 @@ resource "google_compute_router_nat" "foobar" {
780771
nat_ip_allocate_option = "AUTO_ONLY"
781772
nat_ips = []
782773
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
783-
784-
log_config {
785-
enable = true
786-
filter = "ERRORS_ONLY"
787-
}
788774
}
789775
`, routerName, routerName, routerName, routerName, routerName)
790776
}
@@ -799,6 +785,7 @@ resource "google_compute_router" "foobar" {
799785
800786
resource "google_compute_network" "foobar" {
801787
name = "%s-net"
788+
auto_create_subnetworks = false
802789
}
803790
804791
resource "google_compute_subnetwork" "foobar" {
@@ -833,6 +820,7 @@ resource "google_compute_router_nat" "foobar" {
833820
tcp_established_idle_timeout_sec = 1600
834821
tcp_transitory_idle_timeout_sec = 60
835822
tcp_time_wait_timeout_sec = 60
823+
max_ports_per_vm = 128
836824
837825
log_config {
838826
enable = true
@@ -885,7 +873,8 @@ network = google_compute_network.foobar.self_link
885873
}
886874
887875
resource "google_compute_network" "foobar" {
888-
name = "%s-net"
876+
name = "%s-net"
877+
auto_create_subnetworks = false
889878
}
890879
resource "google_compute_subnetwork" "foobar" {
891880
name = "%s-subnet"
@@ -919,6 +908,7 @@ resource "google_compute_router_nat" "foobar" {
919908
tcp_established_idle_timeout_sec = 1600
920909
tcp_transitory_idle_timeout_sec = 60
921910
tcp_time_wait_timeout_sec = 60
911+
max_ports_per_vm = 128
922912
923913
log_config {
924914
enable = true
@@ -937,7 +927,8 @@ network = google_compute_network.foobar.self_link
937927
}
938928
939929
resource "google_compute_network" "foobar" {
940-
name = "%s-net"
930+
name = "%s-net"
931+
auto_create_subnetworks = false
941932
}
942933
resource "google_compute_subnetwork" "foobar" {
943934
name = "%s-subnet"
@@ -971,6 +962,7 @@ resource "google_compute_router_nat" "foobar" {
971962
tcp_established_idle_timeout_sec = 1600
972963
tcp_transitory_idle_timeout_sec = 60
973964
tcp_time_wait_timeout_sec = 60
965+
max_ports_per_vm = 128
974966
975967
log_config {
976968
enable = true
@@ -1465,68 +1457,6 @@ resource "google_compute_router" "foobar" {
14651457
`, routerName, routerName, routerName)
14661458
}
14671459

1468-
func testAccComputeRouterNatLogConfig(routerName string) string {
1469-
return fmt.Sprintf(`
1470-
resource "google_compute_network" "foobar" {
1471-
name = "%s-net"
1472-
}
1473-
1474-
resource "google_compute_subnetwork" "foobar" {
1475-
name = "%s-subnet"
1476-
network = google_compute_network.foobar.self_link
1477-
ip_cidr_range = "10.0.0.0/16"
1478-
region = "us-central1"
1479-
}
1480-
1481-
resource "google_compute_router" "foobar" {
1482-
name = "%s"
1483-
region = google_compute_subnetwork.foobar.region
1484-
network = google_compute_network.foobar.self_link
1485-
}
1486-
1487-
resource "google_compute_router_nat" "foobar" {
1488-
name = "%s"
1489-
router = google_compute_router.foobar.name
1490-
region = google_compute_router.foobar.region
1491-
nat_ip_allocate_option = "AUTO_ONLY"
1492-
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
1493-
log_config {
1494-
enable = false
1495-
filter = "ALL"
1496-
}
1497-
}
1498-
`, routerName, routerName, routerName, routerName)
1499-
}
1500-
1501-
func testAccComputeRouterNatLogConfigRemoved(routerName string) string {
1502-
return fmt.Sprintf(`
1503-
resource "google_compute_network" "foobar" {
1504-
name = "%s-net"
1505-
}
1506-
1507-
resource "google_compute_subnetwork" "foobar" {
1508-
name = "%s-subnet"
1509-
network = google_compute_network.foobar.self_link
1510-
ip_cidr_range = "10.0.0.0/16"
1511-
region = "us-central1"
1512-
}
1513-
1514-
resource "google_compute_router" "foobar" {
1515-
name = "%s"
1516-
region = google_compute_subnetwork.foobar.region
1517-
network = google_compute_network.foobar.self_link
1518-
}
1519-
1520-
resource "google_compute_router_nat" "foobar" {
1521-
name = "%s"
1522-
router = google_compute_router.foobar.name
1523-
region = google_compute_router.foobar.region
1524-
nat_ip_allocate_option = "AUTO_ONLY"
1525-
source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES"
1526-
}
1527-
`, routerName, routerName, routerName, routerName)
1528-
}
1529-
15301460
func testAccComputeRouterNatBaseResourcesWithPrivateNatSubnetworks(routerName, hubName string) string {
15311461
return fmt.Sprintf(`
15321462
resource "google_compute_network" "foobar" {

0 commit comments

Comments
 (0)