Skip to content

Commit 2139edf

Browse files
modular-magicianmodular-magician
authored
fix: Router advertised-route-priority undefined behavior (#12808) (#21024)
[upstream:535310692459f90208ee7c24d27af48a42c16c1e] Signed-off-by: Modular Magician <[email protected]>
1 parent c29b789 commit 2139edf

File tree

4 files changed

+141
-21
lines changed

4 files changed

+141
-21
lines changed

‎.changelog/12808.txt

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:bug
2+
compute: made `google_compute_router_peer.advertised_route_priority` use server-side default if unset. To set the value to `0` you must also set `zero_advertised_route_priority = true`.
3+
```
4+
```release-note:enhancement
5+
compute: added `zero_advertised_route_priority` field to 'google_compute_router_peer'
6+
```

‎google/services/compute/resource_compute_router_bgp_peer_test.go

+78-16
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestAccComputeRouterPeer_basic(t *testing.T) {
3030
ResourceName: "google_compute_router_peer.foobar",
3131
ImportState: true,
3232
ImportStateVerify: true,
33-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
33+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
3434
},
3535
{
3636
Config: testAccComputeRouterPeerKeepRouter(routerName),
@@ -94,7 +94,7 @@ func TestAccComputeRouterPeer_enable(t *testing.T) {
9494
ResourceName: "google_compute_router_peer.foobar",
9595
ImportState: true,
9696
ImportStateVerify: true,
97-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
97+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
9898
},
9999
{
100100
Config: testAccComputeRouterPeerEnable(routerName, false),
@@ -105,7 +105,7 @@ func TestAccComputeRouterPeer_enable(t *testing.T) {
105105
ResourceName: "google_compute_router_peer.foobar",
106106
ImportState: true,
107107
ImportStateVerify: true,
108-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
108+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
109109
},
110110
{
111111
Config: testAccComputeRouterPeerEnable(routerName, true),
@@ -116,7 +116,7 @@ func TestAccComputeRouterPeer_enable(t *testing.T) {
116116
ResourceName: "google_compute_router_peer.foobar",
117117
ImportState: true,
118118
ImportStateVerify: true,
119-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
119+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
120120
},
121121
},
122122
})
@@ -140,7 +140,7 @@ func TestAccComputeRouterPeer_bfd(t *testing.T) {
140140
ResourceName: "google_compute_router_peer.foobar",
141141
ImportState: true,
142142
ImportStateVerify: true,
143-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
143+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
144144
},
145145
{
146146
Config: testAccComputeRouterPeerBfd(routerName, "DISABLED"),
@@ -151,7 +151,7 @@ func TestAccComputeRouterPeer_bfd(t *testing.T) {
151151
ResourceName: "google_compute_router_peer.foobar",
152152
ImportState: true,
153153
ImportStateVerify: true,
154-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
154+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
155155
},
156156
{
157157
Config: testAccComputeRouterPeerBasic(routerName),
@@ -162,7 +162,7 @@ func TestAccComputeRouterPeer_bfd(t *testing.T) {
162162
ResourceName: "google_compute_router_peer.foobar",
163163
ImportState: true,
164164
ImportStateVerify: true,
165-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
165+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
166166
},
167167
},
168168
})
@@ -214,7 +214,7 @@ func TestAccComputeRouterPeer_Ipv6Basic(t *testing.T) {
214214
ResourceName: resourceName,
215215
ImportState: true,
216216
ImportStateVerify: true,
217-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
217+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
218218
},
219219
},
220220
})
@@ -242,7 +242,7 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
242242
ResourceName: resourceName,
243243
ImportState: true,
244244
ImportStateVerify: true,
245-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
245+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
246246
},
247247
{
248248
Config: testAccComputeRouterPeerUpdateIpv4Address(routerName),
@@ -258,7 +258,7 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
258258
ResourceName: resourceName,
259259
ImportState: true,
260260
ImportStateVerify: true,
261-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
261+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
262262
},
263263
},
264264
})
@@ -291,6 +291,33 @@ func TestAccComputeRouterPeer_UpdateRouterCustomLearnedRoutePriority(t *testing.
291291
})
292292
}
293293

294+
func TestAccComputeRouterPeer_UpdateRouterAdvertisedRoutePriority(t *testing.T) {
295+
t.Parallel()
296+
routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10))
297+
resourceName := "google_compute_router_peer.peer"
298+
acctest.VcrTest(t, resource.TestCase{
299+
PreCheck: func() { acctest.AccTestPreCheck(t) },
300+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
301+
CheckDestroy: testAccCheckComputeRouterPeerDestroyProducer(t),
302+
Steps: []resource.TestStep{
303+
{
304+
Config: testAccComputeRouterPeerAdvertisedRoutePriority(routerName, 100, false),
305+
Check: resource.ComposeTestCheckFunc(
306+
resource.TestCheckResourceAttr(resourceName, "advertised_route_priority", "100"), // Check for one element in the list
307+
),
308+
}, {
309+
Config: testAccComputeRouterPeerAdvertisedRoutePriority(routerName, 0, false),
310+
ExpectError: regexp.MustCompile(`Error: Invalid advertised_route_priority value: When zero_advertised_route_priority is set to 'false', the advertised_route_priority field cannot be 0. Please provide a non-zero value.`), // Expect the specific error message
311+
}, {
312+
Config: testAccComputeRouterPeerAdvertisedRoutePriority(routerName, 0, true),
313+
Check: resource.ComposeTestCheckFunc(
314+
resource.TestCheckResourceAttr(resourceName, "advertised_route_priority", "0"),
315+
),
316+
},
317+
},
318+
})
319+
}
320+
294321
func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
295322
t.Parallel()
296323

@@ -313,7 +340,7 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
313340
ResourceName: resourceName,
314341
ImportState: true,
315342
ImportStateVerify: true,
316-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
343+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
317344
},
318345
{
319346
Config: testAccComputeRouterPeerUpdateIpv6Address(routerName, true),
@@ -327,7 +354,7 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
327354
ResourceName: resourceName,
328355
ImportState: true,
329356
ImportStateVerify: true,
330-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
357+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
331358
},
332359
},
333360
})
@@ -355,7 +382,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
355382
ResourceName: resourceName,
356383
ImportState: true,
357384
ImportStateVerify: true,
358-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
385+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
359386
},
360387
{
361388
Config: testAccComputeRouterPeerIpv6(routerName, true),
@@ -369,7 +396,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
369396
ResourceName: resourceName,
370397
ImportState: true,
371398
ImportStateVerify: true,
372-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
399+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
373400
},
374401
{
375402
Config: testAccComputeRouterPeerIpv6(routerName, false),
@@ -383,7 +410,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
383410
ResourceName: resourceName,
384411
ImportState: true,
385412
ImportStateVerify: true,
386-
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
413+
ImportStateVerifyIgnore: []string{"is_advertised_route_priority_set", "zero_custom_learned_route_priority"},
387414
},
388415
},
389416
})
@@ -878,6 +905,42 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string {
878905
routerName, routerName)
879906
}
880907

908+
func testAccComputeRouterPeerAdvertisedRoutePriority(routerName string, advertisedRoutePriority int, zeroAdvertisedRoutePriority bool) string {
909+
return fmt.Sprintf(`
910+
resource "google_compute_network" "network" {
911+
name = "%s-net"
912+
auto_create_subnetworks = false
913+
}
914+
915+
resource "google_compute_subnetwork" "subnetwork" {
916+
name = "%s-sub"
917+
network = google_compute_network.network.self_link
918+
ip_cidr_range = "10.0.0.0/16"
919+
region = "us-central1"
920+
}
921+
922+
resource "google_compute_router" "router" {
923+
name = "%s-router"
924+
region = google_compute_subnetwork.subnetwork.region
925+
network = google_compute_network.network.self_link
926+
bgp {
927+
asn = 64514
928+
}
929+
}
930+
931+
resource "google_compute_router_peer" "peer" {
932+
name = "%s-router-peer"
933+
router = google_compute_router.router.name
934+
region = google_compute_router.router.region
935+
interface = "interface-1"
936+
peer_asn = 65513
937+
advertised_route_priority = %d
938+
zero_advertised_route_priority = %t
939+
}
940+
`, routerName, routerName, routerName, routerName, advertisedRoutePriority, zeroAdvertisedRoutePriority)
941+
942+
}
943+
881944
func testAccComputeRouterPeerCustomLearnedRoutePriority(routerName string, customLearnedRoutePriority int, zeroCustomLearnedRoutePriority bool) string {
882945
return fmt.Sprintf(`
883946
resource "google_compute_network" "network" {
@@ -911,7 +974,6 @@ resource "google_compute_router_peer" "peer" {
911974
zero_custom_learned_route_priority = %t
912975
}
913976
`, routerName, routerName, routerName, routerName, customLearnedRoutePriority, zeroCustomLearnedRoutePriority)
914-
915977
}
916978

917979
func testAccComputeRouterPeerKeepRouter(routerName string) string {

‎google/services/compute/resource_compute_router_peer.go

+38-4
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,16 @@ CIDR-formatted string.`,
144144
Where there is more than one matching route of maximum
145145
length, the routes with the lowest priority value win.`,
146146
},
147+
"zero_advertised_route_priority": {
148+
Type: schema.TypeBool,
149+
Optional: true,
150+
Description: `Force the advertised_route_priority to be 0.`,
151+
},
152+
"is_advertised_route_priority_set": {
153+
Type: schema.TypeBool,
154+
Computed: true, // This field is computed by the provider
155+
Description: "An internal boolean field for provider use for zero_advertised_route_priority.",
156+
},
147157
"custom_learned_ip_ranges": {
148158
Type: schema.TypeList,
149159
Optional: true,
@@ -405,8 +415,20 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{}
405415
advertisedRoutePriorityProp, err := expandNestedComputeRouterBgpPeerAdvertisedRoutePriority(d.Get("advertised_route_priority"), d, config)
406416
if err != nil {
407417
return err
408-
} else if v, ok := d.GetOk("advertised_route_priority"); ok || !reflect.DeepEqual(v, advertisedRoutePriorityProp) {
409-
obj["advertisedRoutePriority"] = advertisedRoutePriorityProp
418+
} else if v, ok := d.GetOkExists("advertised_route_priority"); ok || !reflect.DeepEqual(v, advertisedRoutePriorityProp) {
419+
if !d.Get("zero_advertised_route_priority").(bool) && advertisedRoutePriorityProp == 0 {
420+
// Add the condition to check the present value
421+
if !d.Get("is_advertised_route_priority_set").(bool) {
422+
log.Printf("[WARN] advertised_route_priority can't be 0 unless zero_advertised_route_priority set to true")
423+
} else {
424+
return fmt.Errorf("Invalid advertised_route_priority value: When zero_advertised_route_priority is set to 'false', the advertised_route_priority field cannot be 0. Please provide a non-zero value.")
425+
}
426+
} else if d.Get("zero_advertised_route_priority").(bool) && advertisedRoutePriorityProp != 0 {
427+
return fmt.Errorf("[ERROR] advertised_route_priority cannot be set to value other than zero unless zero_advertised_route_priority is false")
428+
} else {
429+
obj["advertisedRoutePriority"] = advertisedRoutePriorityProp
430+
d.Set("is_advertised_route_priority_set", true)
431+
}
410432
}
411433
advertiseModeProp, err := expandNestedComputeRouterBgpPeerAdvertiseMode(d.Get("advertise_mode"), d, config)
412434
if err != nil {
@@ -748,8 +770,20 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{}
748770
advertisedRoutePriorityProp, err := expandNestedComputeRouterBgpPeerAdvertisedRoutePriority(d.Get("advertised_route_priority"), d, config)
749771
if err != nil {
750772
return err
751-
} else if v, ok := d.GetOk("advertised_route_priority"); ok || !reflect.DeepEqual(v, advertisedRoutePriorityProp) {
752-
obj["advertisedRoutePriority"] = advertisedRoutePriorityProp
773+
} else if v, ok := d.GetOkExists("advertised_route_priority"); ok || !reflect.DeepEqual(v, advertisedRoutePriorityProp) {
774+
if !d.Get("zero_advertised_route_priority").(bool) && advertisedRoutePriorityProp == 0 {
775+
// Add the condition to check the present value
776+
if !d.Get("is_advertised_route_priority_set").(bool) {
777+
log.Printf("[WARN] advertised_route_priority can't be 0 unless zero_advertised_route_priority set to true")
778+
} else {
779+
return fmt.Errorf("Invalid advertised_route_priority value: When zero_advertised_route_priority is set to 'false', the advertised_route_priority field cannot be 0. Please provide a non-zero value.")
780+
}
781+
} else if d.Get("zero_advertised_route_priority").(bool) && advertisedRoutePriorityProp != 0 {
782+
return fmt.Errorf("[ERROR] advertised_route_priority cannot be set to value other than zero unless zero_advertised_route_priority is false")
783+
} else {
784+
obj["advertisedRoutePriority"] = advertisedRoutePriorityProp
785+
d.Set("is_advertised_route_priority_set", true)
786+
}
753787
}
754788
advertiseModeProp, err := expandNestedComputeRouterBgpPeerAdvertiseMode(d.Get("advertise_mode"), d, config)
755789
if err != nil {

‎website/docs/r/compute_router_peer.html.markdown

+19-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ resource "google_compute_router_peer" "peer" {
7272

7373

7474
```hcl
75-
7675
resource "google_compute_router_peer" "peer" {
7776
name = "my-router-peer"
7877
router = "my-router"
@@ -83,6 +82,20 @@ resource "google_compute_router_peer" "peer" {
8382
zero_custom_learned_route_priority = true
8483
}
8584
```
85+
## Example Usage - Router Zero Advertised Route Priority
86+
87+
88+
```hcl
89+
resource "google_compute_router_peer" "peer" {
90+
name = "my-router-peer"
91+
router = "my-router"
92+
region = "us-central1"
93+
interface = "interface-1"
94+
peer_asn = 65513
95+
advertised_route_priority = 0
96+
zero_advertised_route_priority = true
97+
}
98+
```
8699
<div class = "oics-button" style="float: right; margin: 0 0 -15px">
87100
<a href="https://console.cloud.google.com/cloudshell/open?cloudshell_git_repo=https%3A%2F%2Fgithub.jpy.wang%2Fterraform-google-modules%2Fdocs-examples.git&cloudshell_working_dir=router_peer_router_appliance&cloudshell_image=gcr.io%2Fcloudshell-images%2Fcloudshell%3Alatest&open_in_editor=main.tf&cloudshell_print=.%2Fmotd&cloudshell_tutorial=.%2Ftutorial.md" target="_blank">
88101
<img alt="Open in Cloud Shell" src="//gstatic.com/cloudssh/images/open-btn.svg" style="max-height: 44px; margin: 32px auto; max-width: 100%;">
@@ -399,6 +412,11 @@ The following arguments are supported:
399412
Where there is more than one matching route of maximum
400413
length, the routes with the lowest priority value win.
401414

415+
* `zero_advertised_route_priority` -
416+
(Optional)
417+
The user-defined zero-advertised-route-priority for a advertised-route-priority in BGP session.
418+
This value has to be set true to force the advertised_route_priority to be 0.
419+
402420
* `advertise_mode` -
403421
(Optional)
404422
User-specified flag to indicate which mode to use for advertisement.

0 commit comments

Comments
 (0)