Skip to content

Commit 18a08dc

Browse files
modular-magicianmodular-magician
authored
fix: Router custom-learned-route-priority undefined behavior (#12355) (#9083)
[upstream:39570f308328245bbb8de7f6bd2d8898bcbf48fc] Signed-off-by: Modular Magician <[email protected]>
1 parent c23ce7f commit 18a08dc

File tree

5 files changed

+200
-58
lines changed

5 files changed

+200
-58
lines changed

‎.changelog/12355.txt

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:enhancement
2+
compute: fixed a issue where `custom_learned_route_priority` was accidentally set to 0 during updates in 'google_compute_router_peer'
3+
```
4+
```release-note:enhancement
5+
bug: added support for setting `custom_learned_route_priority` to 0 in 'google_compute_router_peer' by adding the `zero_custom_learned_route_priority` field
6+
```

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

+136-54
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package compute_test
44

55
import (
66
"fmt"
7+
"regexp"
78
"testing"
89

910
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -26,9 +27,10 @@ func TestAccComputeRouterPeer_basic(t *testing.T) {
2627
t, "google_compute_router_peer.foobar"),
2728
},
2829
{
29-
ResourceName: "google_compute_router_peer.foobar",
30-
ImportState: true,
31-
ImportStateVerify: true,
30+
ResourceName: "google_compute_router_peer.foobar",
31+
ImportState: true,
32+
ImportStateVerify: true,
33+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
3234
},
3335
{
3436
Config: testAccComputeRouterPeerKeepRouter(routerName),
@@ -54,19 +56,21 @@ func TestAccComputeRouterPeer_advertiseMode(t *testing.T) {
5456
t, "google_compute_router_peer.foobar"),
5557
},
5658
{
57-
ResourceName: "google_compute_router_peer.foobar",
58-
ImportState: true,
59-
ImportStateVerify: true,
59+
ResourceName: "google_compute_router_peer.foobar",
60+
ImportState: true,
61+
ImportStateVerify: true,
62+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority", "is_custom_learned_priority_set"},
6063
},
6164
{
6265
Config: testAccComputeRouterPeerAdvertiseModeUpdate(routerName),
6366
Check: testAccCheckComputeRouterPeerExists(
6467
t, "google_compute_router_peer.foobar"),
6568
},
6669
{
67-
ResourceName: "google_compute_router_peer.foobar",
68-
ImportState: true,
69-
ImportStateVerify: true,
70+
ResourceName: "google_compute_router_peer.foobar",
71+
ImportState: true,
72+
ImportStateVerify: true,
73+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority", "is_custom_learned_priority_set"},
7074
},
7175
},
7276
})
@@ -87,29 +91,32 @@ func TestAccComputeRouterPeer_enable(t *testing.T) {
8791
t, "google_compute_router_peer.foobar"),
8892
},
8993
{
90-
ResourceName: "google_compute_router_peer.foobar",
91-
ImportState: true,
92-
ImportStateVerify: true,
94+
ResourceName: "google_compute_router_peer.foobar",
95+
ImportState: true,
96+
ImportStateVerify: true,
97+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
9398
},
9499
{
95100
Config: testAccComputeRouterPeerEnable(routerName, false),
96101
Check: testAccCheckComputeRouterPeerExists(
97102
t, "google_compute_router_peer.foobar"),
98103
},
99104
{
100-
ResourceName: "google_compute_router_peer.foobar",
101-
ImportState: true,
102-
ImportStateVerify: true,
105+
ResourceName: "google_compute_router_peer.foobar",
106+
ImportState: true,
107+
ImportStateVerify: true,
108+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
103109
},
104110
{
105111
Config: testAccComputeRouterPeerEnable(routerName, true),
106112
Check: testAccCheckComputeRouterPeerExists(
107113
t, "google_compute_router_peer.foobar"),
108114
},
109115
{
110-
ResourceName: "google_compute_router_peer.foobar",
111-
ImportState: true,
112-
ImportStateVerify: true,
116+
ResourceName: "google_compute_router_peer.foobar",
117+
ImportState: true,
118+
ImportStateVerify: true,
119+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
113120
},
114121
},
115122
})
@@ -130,29 +137,32 @@ func TestAccComputeRouterPeer_bfd(t *testing.T) {
130137
t, "google_compute_router_peer.foobar"),
131138
},
132139
{
133-
ResourceName: "google_compute_router_peer.foobar",
134-
ImportState: true,
135-
ImportStateVerify: true,
140+
ResourceName: "google_compute_router_peer.foobar",
141+
ImportState: true,
142+
ImportStateVerify: true,
143+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
136144
},
137145
{
138146
Config: testAccComputeRouterPeerBfd(routerName, "DISABLED"),
139147
Check: testAccCheckComputeRouterPeerExists(
140148
t, "google_compute_router_peer.foobar"),
141149
},
142150
{
143-
ResourceName: "google_compute_router_peer.foobar",
144-
ImportState: true,
145-
ImportStateVerify: true,
151+
ResourceName: "google_compute_router_peer.foobar",
152+
ImportState: true,
153+
ImportStateVerify: true,
154+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
146155
},
147156
{
148157
Config: testAccComputeRouterPeerBasic(routerName),
149158
Check: testAccCheckComputeRouterPeerExists(
150159
t, "google_compute_router_peer.foobar"),
151160
},
152161
{
153-
ResourceName: "google_compute_router_peer.foobar",
154-
ImportState: true,
155-
ImportStateVerify: true,
162+
ResourceName: "google_compute_router_peer.foobar",
163+
ImportState: true,
164+
ImportStateVerify: true,
165+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
156166
},
157167
},
158168
})
@@ -173,9 +183,10 @@ func TestAccComputeRouterPeer_routerApplianceInstance(t *testing.T) {
173183
t, "google_compute_router_peer.foobar"),
174184
},
175185
{
176-
ResourceName: "google_compute_router_peer.foobar",
177-
ImportState: true,
178-
ImportStateVerify: true,
186+
ResourceName: "google_compute_router_peer.foobar",
187+
ImportState: true,
188+
ImportStateVerify: true,
189+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
179190
},
180191
},
181192
})
@@ -200,9 +211,10 @@ func TestAccComputeRouterPeer_Ipv6Basic(t *testing.T) {
200211
),
201212
},
202213
{
203-
ResourceName: resourceName,
204-
ImportState: true,
205-
ImportStateVerify: true,
214+
ResourceName: resourceName,
215+
ImportState: true,
216+
ImportStateVerify: true,
217+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
206218
},
207219
},
208220
})
@@ -227,9 +239,10 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
227239
),
228240
},
229241
{
230-
ResourceName: resourceName,
231-
ImportState: true,
232-
ImportStateVerify: true,
242+
ResourceName: resourceName,
243+
ImportState: true,
244+
ImportStateVerify: true,
245+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
233246
},
234247
{
235248
Config: testAccComputeRouterPeerUpdateIpv4Address(routerName),
@@ -242,9 +255,37 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
242255
),
243256
},
244257
{
245-
ResourceName: resourceName,
246-
ImportState: true,
247-
ImportStateVerify: true,
258+
ResourceName: resourceName,
259+
ImportState: true,
260+
ImportStateVerify: true,
261+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
262+
},
263+
},
264+
})
265+
}
266+
267+
func TestAccComputeRouterPeer_UpdateRouterCustomLearnedRoutePriority(t *testing.T) {
268+
t.Parallel()
269+
routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10))
270+
resourceName := "google_compute_router_peer.peer"
271+
acctest.VcrTest(t, resource.TestCase{
272+
PreCheck: func() { acctest.AccTestPreCheck(t) },
273+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
274+
CheckDestroy: testAccCheckComputeRouterPeerDestroyProducer(t),
275+
Steps: []resource.TestStep{
276+
{
277+
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 100, false),
278+
Check: resource.ComposeTestCheckFunc(
279+
resource.TestCheckResourceAttr(resourceName, "custom_learned_route_priority", "100"), // Check for one element in the list
280+
),
281+
}, {
282+
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 0, false),
283+
ExpectError: regexp.MustCompile(`Error: Invalid custom_learned_route_priority value: When zero_custom_learned_route_priority is set to 'false', the custom_learned_route_priority field cannot be 0. Please provide a non-zero value.`), // Expect the specific error message
284+
}, {
285+
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 0, true),
286+
Check: resource.ComposeTestCheckFunc(
287+
resource.TestCheckResourceAttr(resourceName, "custom_learned_route_priority", "0"),
288+
),
248289
},
249290
},
250291
})
@@ -269,9 +310,10 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
269310
),
270311
},
271312
{
272-
ResourceName: resourceName,
273-
ImportState: true,
274-
ImportStateVerify: true,
313+
ResourceName: resourceName,
314+
ImportState: true,
315+
ImportStateVerify: true,
316+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
275317
},
276318
{
277319
Config: testAccComputeRouterPeerUpdateIpv6Address(routerName, true),
@@ -282,9 +324,10 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
282324
),
283325
},
284326
{
285-
ResourceName: resourceName,
286-
ImportState: true,
287-
ImportStateVerify: true,
327+
ResourceName: resourceName,
328+
ImportState: true,
329+
ImportStateVerify: true,
330+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
288331
},
289332
},
290333
})
@@ -309,9 +352,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
309352
),
310353
},
311354
{
312-
ResourceName: resourceName,
313-
ImportState: true,
314-
ImportStateVerify: true,
355+
ResourceName: resourceName,
356+
ImportState: true,
357+
ImportStateVerify: true,
358+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
315359
},
316360
{
317361
Config: testAccComputeRouterPeerIpv6(routerName, true),
@@ -322,9 +366,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
322366
),
323367
},
324368
{
325-
ResourceName: resourceName,
326-
ImportState: true,
327-
ImportStateVerify: true,
369+
ResourceName: resourceName,
370+
ImportState: true,
371+
ImportStateVerify: true,
372+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
328373
},
329374
{
330375
Config: testAccComputeRouterPeerIpv6(routerName, false),
@@ -335,9 +380,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
335380
),
336381
},
337382
{
338-
ResourceName: resourceName,
339-
ImportState: true,
340-
ImportStateVerify: true,
383+
ResourceName: resourceName,
384+
ImportState: true,
385+
ImportStateVerify: true,
386+
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
341387
},
342388
},
343389
})
@@ -999,6 +1045,42 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string {
9991045
routerName, routerName)
10001046
}
10011047

1048+
func testAccComputeRouterPeerCustomLearnedRoutePriority(routerName string, customLearnedRoutePriority int, zeroCustomLearnedRoutePriority bool) string {
1049+
return fmt.Sprintf(`
1050+
resource "google_compute_network" "network" {
1051+
name = "%s-net"
1052+
auto_create_subnetworks = false
1053+
}
1054+
1055+
resource "google_compute_subnetwork" "subnetwork" {
1056+
name = "%s-sub"
1057+
network = google_compute_network.network.self_link
1058+
ip_cidr_range = "10.0.0.0/16"
1059+
region = "us-central1"
1060+
}
1061+
1062+
resource "google_compute_router" "router" {
1063+
name = "%s-router"
1064+
region = google_compute_subnetwork.subnetwork.region
1065+
network = google_compute_network.network.self_link
1066+
bgp {
1067+
asn = 64514
1068+
}
1069+
}
1070+
1071+
resource "google_compute_router_peer" "peer" {
1072+
name = "%s-router-peer"
1073+
router = google_compute_router.router.name
1074+
region = google_compute_router.router.region
1075+
interface = "interface-1"
1076+
peer_asn = 65513
1077+
custom_learned_route_priority = %d
1078+
zero_custom_learned_route_priority = %t
1079+
}
1080+
`, routerName, routerName, routerName, routerName, customLearnedRoutePriority, zeroCustomLearnedRoutePriority)
1081+
1082+
}
1083+
10021084
func testAccComputeRouterPeerKeepRouter(routerName string) string {
10031085
return fmt.Sprintf(`
10041086
resource "google_compute_network" "foobar" {

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

+37-3
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,17 @@ CIDR-formatted string.`,
167167
This value is applied to all custom learned route ranges for the session. You can choose a value
168168
from 0 to 65335. If you don't provide a value, Google Cloud assigns a priority of 100 to the ranges.`,
169169
},
170-
170+
"zero_custom_learned_route_priority": {
171+
Type: schema.TypeBool,
172+
Optional: true,
173+
Default: false,
174+
Description: `Force the custom_learned_route_priority to be 0.`,
175+
},
176+
"is_custom_learned_priority_set": {
177+
Type: schema.TypeBool,
178+
Computed: true, // This field is computed by the provider
179+
Description: "An internal boolean field for provider use.",
180+
},
171181
"bfd": {
172182
Type: schema.TypeList,
173183
Computed: true,
@@ -450,7 +460,19 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{}
450460
if err != nil {
451461
return err
452462
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
453-
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
463+
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
464+
// Add the condition to check the present value
465+
if !d.Get("is_custom_learned_priority_set").(bool) {
466+
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
467+
} else {
468+
return fmt.Errorf("Invalid custom_learned_route_priority value: When zero_custom_learned_route_priority is set to 'false', the custom_learned_route_priority field cannot be 0. Please provide a non-zero value.")
469+
}
470+
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
471+
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
472+
} else {
473+
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
474+
d.Set("is_custom_learned_priority_set", true)
475+
}
454476
}
455477
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
456478
if err != nil {
@@ -799,7 +821,19 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{}
799821
if err != nil {
800822
return err
801823
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
802-
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
824+
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
825+
// Add the condition to check the present value
826+
if !d.Get("is_custom_learned_priority_set").(bool) {
827+
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
828+
} else {
829+
return fmt.Errorf("Invalid custom_learned_route_priority value: When zero_custom_learned_route_priority is set to 'false', the custom_learned_route_priority field cannot be 0. Please provide a non-zero value.")
830+
}
831+
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
832+
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
833+
} else {
834+
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
835+
d.Set("is_custom_learned_priority_set", true)
836+
}
803837
}
804838
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
805839
if err != nil {

0 commit comments

Comments
 (0)