Skip to content

Commit

Permalink
fix: Router custom-learned-route-priority undefined behavior (#12355)
Browse files Browse the repository at this point in the history 
Co-authored-by: Sam Levenick <slevenick@google.com>
  • Loading branch information
@akshat-jindal-nit @slevenick
akshat-jindal-nit and slevenick authored Jan 17, 2025
1 parent 22ebcfb commit 39570f3
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 47 deletions.
168 changes: 125 additions & 43 deletions mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package compute_test

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand All @@ -24,9 +25,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 @@ -52,19 +54,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 @@ -85,29 +89,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 @@ -128,29 +135,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 @@ -171,9 +181,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 @@ -198,18 +209,19 @@ 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"},
},
},
})
}

func TestAccComputeRouterPeer_Ipv4BasicCreateUpdate(t *testing.T) {
t.Parallel()
t.Parallel()

routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10))
routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10))
resourceName := "google_compute_router_peer.foobar"
acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
Expand All @@ -225,24 +237,53 @@ 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),
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeRouterPeerExists(
t, resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_ipv4", "true"),
resource.TestCheckResourceAttr(resourceName, "ipv4_nexthop_address", "169.254.1.2"),
resource.TestCheckResourceAttr(resourceName, "peer_ipv4_nexthop_address", "169.254.1.1"),
resource.TestCheckResourceAttr(resourceName, "ipv4_nexthop_address", "169.254.1.2"),
resource.TestCheckResourceAttr(resourceName, "peer_ipv4_nexthop_address", "169.254.1.1"),
),
},
{
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 Down Expand Up @@ -270,6 +311,7 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerUpdateIpv6Address(routerName, true),
Expand All @@ -283,6 +325,7 @@ func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand Down Expand Up @@ -310,6 +353,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerIpv6(routerName, true),
Expand All @@ -323,6 +367,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
{
Config: testAccComputeRouterPeerIpv6(routerName, false),
Expand All @@ -336,6 +381,7 @@ func TestAccComputeRouterPeer_EnableDisableIpv6(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"zero_custom_learned_route_priority"},
},
},
})
Expand Down Expand Up @@ -1001,6 +1047,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 mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,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 @@ -803,7 +825,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
2 changes: 1 addition & 1 deletion mmv1/third_party/terraform/services/compute/resource_compute_router_peer_test.go.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestAccComputeRouterBgpPeer_routerPeerRouterAppliance(t *testing.T) {
ResourceName: "google_compute_router_peer.peer",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"router_appliance_instance", "router", "region"},
ImportStateVerifyIgnore: []string{"router_appliance_instance", "router", "region","zero_custom_learned_route_priority"},
},
},
})
Expand Down
Loading

0 comments on commit 39570f3

Please sign in to comment.