From 2f918dd9cc48b3e6afaa8378a400f0a9a5071653 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 4 Mar 2024 15:59:48 -0600 Subject: [PATCH] feat: push connection speed validation down to the API (#610) Previously, we maintained a list of valid connection speed values in the terraform provider in order to steer users towards selecting a valid connection speed. As shown by #595, validating values in the provider has the potential to break when the list of connection speed values supported by the API changes. This updates the code that converts speed values between integer and string representations (i.e., the code that knows that `"50Mbps"` is the same as `500000000`) to convert (roughly) arbitrary values. This means that new values are immediately "supported" by the provider, but it also means that invalid values will not be flagged until apply. The test change in this PR is sufficient to resolve the issue introduced by merging #605 without any other change to provider behavior. Closes #609 --- docs/data-sources/equinix_metal_connection.md | 2 +- docs/resources/equinix_metal_connection.md | 2 +- .../metal/connection/datasource_schema.go | 2 +- .../metal/connection/datasource_test.go | 8 ++- .../metal/connection/resource_schema.go | 3 +- internal/resources/metal/connection/speed.go | 63 ++++++++----------- .../resources/metal/connection/speed_test.go | 20 +++++- 7 files changed, 55 insertions(+), 45 deletions(-) diff --git a/docs/data-sources/equinix_metal_connection.md b/docs/data-sources/equinix_metal_connection.md index cc6659bf5..dbc2bf008 100644 --- a/docs/data-sources/equinix_metal_connection.md +++ b/docs/data-sources/equinix_metal_connection.md @@ -33,7 +33,7 @@ In addition to all arguments above, the following attributes are exported: * `redundancy` - Connection redundancy, reduntant or primary. * `type` - Connection type, dedicated or shared. * `project_id` - ID of project to which the connection belongs. -* `speed` - Connection speed, one of 50Mbps, 200Mbps, 500Mbps, 1Gbps, 2Gbps, 5Gbps, 10Gbps. +* `speed` - Connection speed - Values will be in the format 'Mbps' or 'Gpbs', for example '100Mbps`, '50Gbps', etc. * `description` - Description of the connection resource. * `mode` - Mode for connections in IBX facilities with the dedicated type - standard or tunnel. * `tags` - String list of tags. diff --git a/docs/resources/equinix_metal_connection.md b/docs/resources/equinix_metal_connection.md index 21ec43ee4..3ccd7aeb6 100644 --- a/docs/resources/equinix_metal_connection.md +++ b/docs/resources/equinix_metal_connection.md @@ -145,7 +145,7 @@ The following arguments are supported: * `type` - (Required) Connection type - dedicated or shared. * `contact_email` - (Optional) The preferred email used for communication and notifications about the Equinix Fabric interconnection. Required when using a Project API key. Optional and defaults to the primary user email address when using a User API key. * `project_id` - (Optional) ID of the project where the connection is scoped to, must be set for. -* `speed` - (Required) Connection speed - one of 50Mbps, 200Mbps, 500Mbps, 1Gbps, 2Gbps, 5Gbps, 10Gbps. +* `speed` - (Required) Connection speed - Values must be in the format 'Mbps' or 'Gpbs', for example '100Mbps' or '50Gbps'. Actual supported values will depend on the connection type and whether the connection uses VLANs or VRF. * `description` - (Optional) Description for the connection resource. * `mode` - (Optional) Mode for connections in IBX facilities with the dedicated type - standard or tunnel. Default is standard. * `tags` - (Optional) String list of tags. diff --git a/internal/resources/metal/connection/datasource_schema.go b/internal/resources/metal/connection/datasource_schema.go index cd9a92383..b3a8cc609 100644 --- a/internal/resources/metal/connection/datasource_schema.go +++ b/internal/resources/metal/connection/datasource_schema.go @@ -50,7 +50,7 @@ func dataSourceSchema(ctx context.Context) schema.Schema { Computed: true, }, "speed": schema.StringAttribute{ - Description: fmt.Sprintf("Port speed. Possible values are %s", allowedSpeedsString()), + Description: "Connection speed - Values will be in the format 'Mbps' or 'Gpbs', for example '100Mbps`, '50Gbps', etc.", Computed: true, }, "description": schema.StringAttribute{ diff --git a/internal/resources/metal/connection/datasource_test.go b/internal/resources/metal/connection/datasource_test.go index e95f118d4..a95e9c613 100644 --- a/internal/resources/metal/connection/datasource_test.go +++ b/internal/resources/metal/connection/datasource_test.go @@ -100,7 +100,13 @@ func TestAccDataSourceMetalConnection_withVlans_upgradeFromVersion(t *testing.T) "equinix_metal_connection.test", "vlans.#", "data.equinix_metal_connection.test", "vlans.#"), resource.TestCheckResourceAttr( - "data.equinix_metal_connection.test", "vlans.#", "0"), + "data.equinix_metal_connection.test", "vlans.#", "2"), + resource.TestCheckResourceAttrPair( + "equinix_metal_vlan.test1", "vxlan", + "data.equinix_metal_connection.test", "vlans.0"), + resource.TestCheckResourceAttrPair( + "equinix_metal_vlan.test2", "vxlan", + "data.equinix_metal_connection.test", "vlans.1"), ), }, { diff --git a/internal/resources/metal/connection/resource_schema.go b/internal/resources/metal/connection/resource_schema.go index 0017cadbc..29b6e866a 100644 --- a/internal/resources/metal/connection/resource_schema.go +++ b/internal/resources/metal/connection/resource_schema.go @@ -2,7 +2,6 @@ package connection import ( "context" - "fmt" "github.com/equinix/terraform-provider-equinix/internal/framework" fwtypes "github.com/equinix/terraform-provider-equinix/internal/framework/types" @@ -95,7 +94,7 @@ func resourceSchema(ctx context.Context) schema.Schema { }, }, "speed": schema.StringAttribute{ - Description: fmt.Sprintf("Port speed. Required for a_side connections. Allowed values are %s", allowedSpeedsString()), + Description: "Connection speed - Values must be in the format 'Mbps' or 'Gpbs', for example '100Mbps' or '50Gbps'. Actual supported values will depend on the connection type and whether the connection uses VLANs or VRF.", Optional: true, Computed: true, }, diff --git a/internal/resources/metal/connection/speed.go b/internal/resources/metal/connection/speed.go index ca1b64190..6f76979ad 100644 --- a/internal/resources/metal/connection/speed.go +++ b/internal/resources/metal/connection/speed.go @@ -2,53 +2,42 @@ package connection import ( "fmt" - "strings" + "regexp" + "strconv" ) var ( - mega uint64 = 1000 * 1000 - giga uint64 = 1000 * mega - allowedSpeeds = []struct { - Int uint64 - Str string - }{ - {50 * mega, "50Mbps"}, - {200 * mega, "200Mbps"}, - {500 * mega, "500Mbps"}, - {1 * giga, "1Gbps"}, - {2 * giga, "2Gbps"}, - {5 * giga, "5Gbps"}, - {10 * giga, "10Gbps"}, - {100 * giga, "100Gbps"}, - } + mega uint64 = 1000 * 1000 + giga uint64 = 1000 * mega + SpeedFormat = regexp.MustCompile(`^(\d+)((M|G)bps)$`) ) -func allowedSpeedsString() string { - allowedStrings := []string{} - for _, allowedSpeed := range allowedSpeeds { - allowedStrings = append(allowedStrings, allowedSpeed.Str) - } - return strings.Join(allowedStrings, ", ") -} - func speedStrToUint(speed string) (uint64, error) { - allowedStrings := []string{} - for _, allowedSpeed := range allowedSpeeds { - if allowedSpeed.Str == speed { - return allowedSpeed.Int, nil + parts := SpeedFormat.FindStringSubmatch(speed) + if parts != nil { + base, err := strconv.Atoi(parts[1]) + if parts[2] == "Mbps" { + return uint64(base) * mega, nil + } else if parts[2] == "Gbps" { + return uint64(base) * giga, nil } - allowedStrings = append(allowedStrings, allowedSpeed.Str) + return 0, err } - return 0, fmt.Errorf("invalid speed string: %s. Allowed strings: %s", speed, strings.Join(allowedStrings, ", ")) + return 0, fmt.Errorf("invalid speed string %v, must match %v", speed, SpeedFormat.String()) } func speedUintToStr(speed uint64) (string, error) { - allowedUints := []uint64{} - for _, allowedSpeed := range allowedSpeeds { - if speed == allowedSpeed.Int { - return allowedSpeed.Str, nil - } - allowedUints = append(allowedUints, allowedSpeed.Int) + var base uint64 + var unit string + + if (speed % giga) == 0 { + unit = "Gbps" + base = speed / giga + } else if (speed % mega) == 0 { + unit = "Mbps" + base = speed / mega + } else { + return "", fmt.Errorf("unsupported speed value %v", speed) } - return "", fmt.Errorf("%d is not allowed speed value. Allowed values: %v", speed, allowedUints) + return strconv.Itoa(int(base)) + unit, nil } diff --git a/internal/resources/metal/connection/speed_test.go b/internal/resources/metal/connection/speed_test.go index 7a8d28cf2..0e6fa203b 100644 --- a/internal/resources/metal/connection/speed_test.go +++ b/internal/resources/metal/connection/speed_test.go @@ -22,12 +22,28 @@ func TestSpeedConversion(t *testing.T) { } speedUint, err = speedStrToUint("100Gbps") - if err == nil { - t.Errorf("Expected error converting invalid speed string to uint, got: %d", speedUint) + if err != nil { + t.Errorf("Error converting speed string to uint64: %s", err) + } + if speedUint != 100*giga { + t.Errorf("Speed string conversion failed. Expected: %d, got: %d", 100*giga, speedUint) } speedStr, err = speedUintToStr(100 * giga) + if err != nil { + t.Errorf("Error converting speed uint to string: %s", err) + } + if speedStr != "100Gbps" { + t.Errorf("Speed uint conversion failed. Expected: %s, got: %s", "100Gbps", speedStr) + } + + speedStr, err = speedUintToStr(100*giga + 2) if err == nil { t.Errorf("Expected error converting invalid speed uint to string, got: %s", speedStr) } + + speedUint, err = speedStrToUint("100kWh") + if err == nil { + t.Errorf("Expected error converting invalid speed string to uint, got: %d", speedUint) + } }