Skip to content

Commit 1e32ae3

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

File tree

5 files changed

+200
-58
lines changed

5 files changed

+200
-58
lines changed

‎.changelog/12355.txt

Lines changed: 6 additions & 0 deletions
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/services/compute/resource_compute_router_bgp_peer_test.go

Lines changed: 136 additions & 54 deletions
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
})
@@ -832,6 +878,42 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string {
832878
routerName, routerName)
833879
}
834880

881+
func testAccComputeRouterPeerCustomLearnedRoutePriority(routerName string, customLearnedRoutePriority int, zeroCustomLearnedRoutePriority bool) string {
882+
return fmt.Sprintf(`
883+
resource "google_compute_network" "network" {
884+
name = "%s-net"
885+
auto_create_subnetworks = false
886+
}
887+
888+
resource "google_compute_subnetwork" "subnetwork" {
889+
name = "%s-sub"
890+
network = google_compute_network.network.self_link
891+
ip_cidr_range = "10.0.0.0/16"
892+
region = "us-central1"
893+
}
894+
895+
resource "google_compute_router" "router" {
896+
name = "%s-router"
897+
region = google_compute_subnetwork.subnetwork.region
898+
network = google_compute_network.network.self_link
899+
bgp {
900+
asn = 64514
901+
}
902+
}
903+
904+
resource "google_compute_router_peer" "peer" {
905+
name = "%s-router-peer"
906+
router = google_compute_router.router.name
907+
region = google_compute_router.router.region
908+
interface = "interface-1"
909+
peer_asn = 65513
910+
custom_learned_route_priority = %d
911+
zero_custom_learned_route_priority = %t
912+
}
913+
`, routerName, routerName, routerName, routerName, customLearnedRoutePriority, zeroCustomLearnedRoutePriority)
914+
915+
}
916+
835917
func testAccComputeRouterPeerKeepRouter(routerName string) string {
836918
return fmt.Sprintf(`
837919
resource "google_compute_network" "foobar" {

‎google/services/compute/resource_compute_router_peer.go

Lines changed: 37 additions & 3 deletions
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,
@@ -426,7 +436,19 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{}
426436
if err != nil {
427437
return err
428438
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
429-
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
439+
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
440+
// Add the condition to check the present value
441+
if !d.Get("is_custom_learned_priority_set").(bool) {
442+
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
443+
} else {
444+
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.")
445+
}
446+
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
447+
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
448+
} else {
449+
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
450+
d.Set("is_custom_learned_priority_set", true)
451+
}
430452
}
431453
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
432454
if err != nil {
@@ -757,7 +779,19 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{}
757779
if err != nil {
758780
return err
759781
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
760-
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
782+
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
783+
// Add the condition to check the present value
784+
if !d.Get("is_custom_learned_priority_set").(bool) {
785+
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
786+
} else {
787+
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.")
788+
}
789+
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
790+
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
791+
} else {
792+
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
793+
d.Set("is_custom_learned_priority_set", true)
794+
}
761795
}
762796
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
763797
if err != nil {

0 commit comments

Comments
 (0)