Skip to content
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.

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

Add CIDR Validator that matches backend validator #214

Merged
merged 7 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions internal/provider/resource_hvn.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
networkmodels "github.com/hashicorp/hcp-sdk-go/clients/cloud-network/preview/2020-09-07/models"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"

"github.com/hashicorp/terraform-provider-hcp/internal/clients"
)
Expand Down Expand Up @@ -71,12 +70,12 @@ func resourceHvn() *schema.Resource {
},
// Optional inputs
"cidr_block": {
Description: "The CIDR range of the HVN. If this is not provided, the service will provide a default value.",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IsCIDR,
Computed: true,
Description: "The CIDR range of the HVN. If this is not provided, the service will provide a default value.",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateDiagFunc: validateCIDRBlock,
Computed: true,
},
// Computed outputs
"organization_id": {
Expand Down
11 changes: 5 additions & 6 deletions internal/provider/resource_hvn_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
sharedmodels "github.com/hashicorp/hcp-sdk-go/clients/cloud-shared/v1/models"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"

"github.com/hashicorp/terraform-provider-hcp/internal/clients"
)
Expand Down Expand Up @@ -52,11 +51,11 @@ func resourceHvnRoute() *schema.Resource {
ValidateDiagFunc: validateSlugID,
},
"destination_cidr": {
Description: "The destination CIDR of the HVN route.",
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.IsCIDR,
Description: "The destination CIDR of the HVN route.",
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateDiagFunc: validateCIDRBlock,
},
"target_link": {
Description: "A unique URL identifying the target of the HVN route. Examples of the target: [`aws_network_peering`](aws_network_peering.md), [`aws_transit_gateway_attachment`](aws_transit_gateway_attachment.md)",
Expand Down
78 changes: 78 additions & 0 deletions internal/provider/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package provider

import (
"fmt"
"net"
"regexp"
"strings"

Expand Down Expand Up @@ -195,3 +196,80 @@ func validateVaultClusterTier(v interface{}, path cty.Path) diag.Diagnostics {

return diagnostics
}

func validateCIDRBlock(v interface{}, path cty.Path) diag.Diagnostics {
var diagnostics diag.Diagnostics

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment here clarifying what rules are being enforced here (so, the first two documented here: https://registry.terraform.io/providers/hashicorp/hcp/latest/docs/resources/hvn)

// validRanges contains the set of IP ranges considered valid.
var validRanges = []net.IPNet{
{
// 10.*.*.*
IP: net.IPv4(10, 0, 0, 0),
Mask: net.IPv4Mask(255, 0, 0, 0),
},
{
// 192.168.*.*
IP: net.IPv4(192, 168, 0, 0),
Mask: net.IPv4Mask(255, 255, 0, 0),
},
{
// 172.[16-31].*.*
IP: net.IPv4(172, 16, 0, 0),
Mask: net.IPv4Mask(255, 240, 0, 0),
},
}

// parse the string as CIDR notation IP address and prefix length.
ip, net, err := net.ParseCIDR(v.(string))
if err != nil {
msg := "unable to parse string as CIDR notation IP address"
diagnostics = append(diagnostics, diag.Diagnostic{
Severity: diag.Error,
Summary: msg,
Detail: msg,
AttributePath: path,
})

return diagnostics
}

// validate if the IP address is contained in one of the expected ranges.
valid := false
for _, validRange := range validRanges {
valueSize, _ := net.Mask.Size()
validRangeSize, _ := validRange.Mask.Size()
if validRange.Contains(ip) && valueSize >= validRangeSize {
// Flip flag if IP is found within any 1 of 3 ranges.
valid = true
}
}

// Check flag and return an error if the IP address is not contained within
// any of the expected ranges.
if !valid {
msg := fmt.Sprintf("must match pattern of 10.*.*.* with prefix greater than /8," +
"or 172.[16-31].*.* with prefix greater than /12, or " +
"192.168.*.* with prefix greater than /16; where * is any number from [0-255]")
diagnostics = append(diagnostics, diag.Diagnostic{
Severity: diag.Error,
Summary: msg,
Detail: msg,
AttributePath: path,
})
}

// Validate the address passed is the start of the CIDR range.
// This happens after we verify the IP address is a valid RFC 1819
// range to avoid causing confusion with a misguiding error message.
if !ip.Equal(net.IP) {
msg := fmt.Sprintf("invalid CIDR range start %s, should have been %s", ip, net.IP)
diagnostics = append(diagnostics, diag.Diagnostic{
Severity: diag.Error,
Summary: msg,
Detail: msg,
AttributePath: path,
})
}

return diagnostics
}
242 changes: 242 additions & 0 deletions internal/provider/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,3 +391,245 @@ func Test_validateVaultClusterTier(t *testing.T) {
})
}
}

func Test_validateCIDRBlock(t *testing.T) {
tcs := map[string]struct {
input string
expected diag.Diagnostics
}{
"blank input string": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit on test case organization: I would start with all of the valid test cases together followed by the invalid test cases. and probably worth throwing in a comment line to break them up

input: "",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"invalid cidr notation": {
input: "192.168.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"invalid characters": {
input: "someString",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"valid cidr block": {
input: "10.0.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 2": {
input: "10.255.255.0/24",
expected: diag.Diagnostics(nil),
},
"invalid cidr notation 2": {
input: "10.256.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"invalid cidr notation 3": {
input: "10.0.256.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"invalid characters 2": {
input: "10.255.255asdfasdfaqsd.250",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"valid cidr block 3": {
input: "192.168.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 4": {
input: "192.168.255.0/24",
expected: diag.Diagnostics(nil),
},
"invalid cidr notation 4": {
input: "192.168.256.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "unable to parse string as CIDR notation IP address",
Detail: "unable to parse string as CIDR notation IP address",
AttributePath: nil,
},
},
},
"invalid pattern": {
input: "192.0.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"valid cidr block 5": {
input: "172.16.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 6": {
input: "172.17.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 7": {
input: "172.18.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 8": {
input: "172.30.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 9": {
input: "172.20.0.0/24",
expected: diag.Diagnostics(nil),
},
"valid cidr block 10": {
input: "172.31.0.0/24",
expected: diag.Diagnostics(nil),
},
"invalid pattern 2": {
input: "172.15.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern 3": {
input: "172.32.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern 4": {
input: "172.192.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern 5": {
input: "172.255.0.0/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern 6": {
input: "10.0.0.0/7",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern 7": {
input: "192.168.0.0/15",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
},
},
"invalid pattern and invalid range": {
input: "87.70.141.1/22",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
Detail: "must match pattern of 10.*.*.* with prefix greater than /8,or 172.[16-31].*.* with prefix greater than /12, or 192.168.*.* with prefix greater than /16; where * is any number from [0-255]",
AttributePath: nil,
},
diag.Diagnostic{
Severity: diag.Error,
Summary: "invalid CIDR range start 87.70.141.1, should have been 87.70.140.0",
Detail: "invalid CIDR range start 87.70.141.1, should have been 87.70.140.0",
AttributePath: nil,
},
},
},
"invalid range": {
input: "192.168.255.255/24",
expected: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "invalid CIDR range start 192.168.255.255, should have been 192.168.255.0",
Detail: "invalid CIDR range start 192.168.255.255, should have been 192.168.255.0",
AttributePath: nil,
},
},
},
"valid cidr block 11": {
input: "172.25.16.0/24",
expected: diag.Diagnostics(nil),
},
}

for n, tc := range tcs {
t.Run(n, func(t *testing.T) {
r := require.New(t)
result := validateCIDRBlock(tc.input, nil)
r.Equal(tc.expected, result)
})
}
}