Skip to content

fix: Router custom-learned-route-priority undefined behavior #9083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 17, 2025
Merged

fix: Router custom-learned-route-priority undefined behavior #9083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changelog/12355.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:enhancement
compute: fixed a issue where `custom_learned_route_priority` was accidentally set to 0 during updates in 'google_compute_router_peer'
```
```release-note:enhancement
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
```
190 changes: 136 additions & 54 deletions google-beta/services/compute/resource_compute_router_bgp_peer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package compute_test

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand All @@ -26,9 +27,10 @@ func TestAccComputeRouterPeer_basic(t *testing.T) {
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerKeepRouter(routerName),
Expand All @@ -54,19 +56,21 @@ func TestAccComputeRouterPeer_advertiseMode(t *testing.T) {
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority", "is_custom_learned_priority_set"},
},
{
Config: testAccComputeRouterPeerAdvertiseModeUpdate(routerName),
Check: testAccCheckComputeRouterPeerExists(
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority", "is_custom_learned_priority_set"},
},
},
})
Expand All @@ -87,29 +91,32 @@ func TestAccComputeRouterPeer_enable(t *testing.T) {
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerEnable(routerName, false),
Check: testAccCheckComputeRouterPeerExists(
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerEnable(routerName, true),
Check: testAccCheckComputeRouterPeerExists(
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand All @@ -130,29 +137,32 @@ func TestAccComputeRouterPeer_bfd(t *testing.T) {
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerBfd(routerName, "DISABLED"),
Check: testAccCheckComputeRouterPeerExists(
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerBasic(routerName),
Check: testAccCheckComputeRouterPeerExists(
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand All @@ -173,9 +183,10 @@ func TestAccComputeRouterPeer_routerApplianceInstance(t *testing.T) {
t, "google_compute_router_peer.foobar"),
},
{
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_router_peer.foobar",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand All @@ -200,9 +211,10 @@ func TestAccComputeRouterPeer_Ipv6Basic(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand All @@ -227,9 +239,10 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerUpdateIpv4Address(routerName),
Expand All @@ -242,9 +255,37 @@ func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
}

func TestAccComputeRouterPeer_UpdateRouterCustomLearnedRoutePriority(t *testing.T) {
t.Parallel()
routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10))
resourceName := "google_compute_router_peer.peer"
acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckComputeRouterPeerDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 100, false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "custom_learned_route_priority", "100"), // Check for one element in the list
),
}, {
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 0, false),
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
}, {
Config: testAccComputeRouterPeerCustomLearnedRoutePriority(routerName, 0, true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "custom_learned_route_priority", "0"),
),
},
},
})
Expand All @@ -269,9 +310,10 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerUpdateIpv6Address(routerName, true),
Expand All @@ -282,9 +324,10 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand All @@ -309,9 +352,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerIpv6(routerName, true),
Expand All @@ -322,9 +366,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerIpv6(routerName, false),
Expand All @@ -335,9 +380,10 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand Down Expand Up @@ -999,6 +1045,42 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string {
routerName, routerName)
}

func testAccComputeRouterPeerCustomLearnedRoutePriority(routerName string, customLearnedRoutePriority int, zeroCustomLearnedRoutePriority bool) string {
return fmt.Sprintf(`
resource "google_compute_network" "network" {
name = "%s-net"
auto_create_subnetworks = false
}

resource "google_compute_subnetwork" "subnetwork" {
name = "%s-sub"
network = google_compute_network.network.self_link
ip_cidr_range = "10.0.0.0/16"
region = "us-central1"
}

resource "google_compute_router" "router" {
name = "%s-router"
region = google_compute_subnetwork.subnetwork.region
network = google_compute_network.network.self_link
bgp {
asn = 64514
}
}

resource "google_compute_router_peer" "peer" {
name = "%s-router-peer"
router = google_compute_router.router.name
region = google_compute_router.router.region
interface = "interface-1"
peer_asn = 65513
custom_learned_route_priority = %d
zero_custom_learned_route_priority = %t
}
`, routerName, routerName, routerName, routerName, customLearnedRoutePriority, zeroCustomLearnedRoutePriority)

}

func testAccComputeRouterPeerKeepRouter(routerName string) string {
return fmt.Sprintf(`
resource "google_compute_network" "foobar" {
Expand Down
40 changes: 37 additions & 3 deletions google-beta/services/compute/resource_compute_router_peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,17 @@ CIDR-formatted string.`,
This value is applied to all custom learned route ranges for the session. You can choose a value
from 0 to 65335. If you don't provide a value, Google Cloud assigns a priority of 100 to the ranges.`,
},

"zero_custom_learned_route_priority": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: `Force the custom_learned_route_priority to be 0.`,
},
"is_custom_learned_priority_set": {
Type: schema.TypeBool,
Computed: true, // This field is computed by the provider
Description: "An internal boolean field for provider use.",
},
"bfd": {
Type: schema.TypeList,
Computed: true,
Expand Down Expand Up @@ -450,7 +460,19 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{}
if err != nil {
return err
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
// Add the condition to check the present value
if !d.Get("is_custom_learned_priority_set").(bool) {
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
} else {
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.")
}
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
} else {
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
d.Set("is_custom_learned_priority_set", true)
}
}
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
if err != nil {
Expand Down Expand Up @@ -799,7 +821,19 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{}
if err != nil {
return err
} else if v, ok := d.GetOkExists("custom_learned_route_priority"); ok || !reflect.DeepEqual(v, customLearnedRoutePriorityProp) {
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
if !d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp == 0 {
// Add the condition to check the present value
if !d.Get("is_custom_learned_priority_set").(bool) {
log.Printf("[WARN] custom_learned_route_priority can't be 0 unless zero_custom_learned_route_priority set to true")
} else {
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.")
}
} else if d.Get("zero_custom_learned_route_priority").(bool) && customLearnedRoutePriorityProp != 0 {
return fmt.Errorf("[ERROR] custom_learned_route_priority cannot be set to value other than zero unless zero_custom_learned_route_priority is false")
} else {
obj["customLearnedRoutePriority"] = customLearnedRoutePriorityProp
d.Set("is_custom_learned_priority_set", true)
}
}
bfdProp, err := expandNestedComputeRouterBgpPeerBfd(d.Get("bfd"), d, config)
if err != nil {
Expand Down
Loading
Loading