From 823e0aeeef78a017473da793a026282f5bf10ad5 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Fri, 27 Oct 2023 20:10:05 -0400 Subject: [PATCH 1/2] introduce RT validator --- apstra/apstra_validator/parse_rt.go | 115 ++++++++++++++++++ apstra/apstra_validator/parse_rt_test.go | 77 ++++++++++++ apstra/blueprint/datacenter_routing_zone.go | 12 +- .../resource_datacenter_routing_zone_test.go | 28 +++-- 4 files changed, 215 insertions(+), 17 deletions(-) create mode 100644 apstra/apstra_validator/parse_rt.go create mode 100644 apstra/apstra_validator/parse_rt_test.go diff --git a/apstra/apstra_validator/parse_rt.go b/apstra/apstra_validator/parse_rt.go new file mode 100644 index 00000000..d83ee8d6 --- /dev/null +++ b/apstra/apstra_validator/parse_rt.go @@ -0,0 +1,115 @@ +package apstravalidator + +import ( + "context" + "github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "math" + "net" + "strconv" + "strings" +) + +const ( + rtSep = ":" + rtFormatErr = `A Route Target must take one of the following forms (leading zeros not permitted): + - <2-byte-value>:<4-byte-value> + - <4-byte-value>:<2-byte-value> + - :<2-byte-value> +` +) + +var _ validator.String = ParseRtValidator{} + +type ParseRtValidator struct { + requireIpv4 bool + requireIpv6 bool +} + +func (o ParseRtValidator) Description(_ context.Context) string { + return "Ensures that the supplied can be parsed as a Route Target" +} + +func (o ParseRtValidator) MarkdownDescription(ctx context.Context) string { + return o.Description(ctx) +} + +func (o ParseRtValidator) ValidateString(_ context.Context, req validator.StringRequest, resp *validator.StringResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { + return + } + + // split the RT string + parts := strings.Split(req.ConfigValue.ValueString(), rtSep) + if len(parts) != 2 { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + + var ipFound bool + // check to see if part 1 is an IPv4 address + ip := net.ParseIP(parts[0]) + if ip != nil { + ipFound = true // we got an IP! + + // is it IPv4? + if len(ip.To4()) != net.IPv4len { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + } + + // make sure "part 1" has length and no prepended zeros (if we haven't already decided it's an IPv4 address) + if !ipFound && (len(parts[0]) == 0 || (len(parts[0]) >= 2 && strings.HasPrefix(parts[0], "0"))) { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + + // make sure "part 2" has length and no prepended zeros + if len(parts[1]) == 0 || (len(parts[1]) >= 2 && strings.HasPrefix(parts[1], "0")) { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + + // determine whether we've got a 32-bit "first part", and thus require a 16-bit "second part" + var firstPartIs32bits bool + if ipFound { + firstPartIs32bits = true + } else { + // try parsing p1 as a 32-bit value + p1, err := strconv.ParseUint(parts[0], 10, 32) + if err != nil { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + + // does p1 require 32 bits? + if p1 > math.MaxUint16 { + firstPartIs32bits = true + } + } + + // try parsing p2 as a 32-bit value + p2, err := strconv.ParseUint(parts[1], 10, 32) + if err != nil { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } + + // p1 and p2 can't both be 32 bits + if firstPartIs32bits && p2 > math.MaxUint16 { + resp.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + req.Path, rtFormatErr, req.ConfigValue.ValueString())) + return + } +} + +func ParseRT() validator.String { + return ParseRtValidator{} +} diff --git a/apstra/apstra_validator/parse_rt_test.go b/apstra/apstra_validator/parse_rt_test.go new file mode 100644 index 00000000..668c3786 --- /dev/null +++ b/apstra/apstra_validator/parse_rt_test.go @@ -0,0 +1,77 @@ +package apstravalidator + +import ( + "context" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "testing" +) + +func TestParseRtValidator(t *testing.T) { + ctx := context.Background() + + validRTs := []string{ + "0:0", + "1:1", + "65535:65535", + "65536:65535", + "65535:65536", + "0.0.0.0:0", + "255.255.255.255:0", + "0.0.0.0:65535", + "255.255.255.255:65535", + "4294967295:65535", + "65535:4294967295", + } + + invalidRTs := []string{ + "", + ":", + "1:", + ":1", + "1", + "4294967296:65535", + "4294967295:65536", + "65536:4294967295", + "65535:4294967296", + "-1:1", + "1:-1", + "256.1.2.3:1", + "bogus", + } + + type testCase struct { + rt string + expectErr bool + } + + var testCases []testCase + for _, rt := range validRTs { // load valid test cases + testCases = append(testCases, testCase{rt: rt, expectErr: false}) + } + for _, rt := range invalidRTs { // load invalid test cases + testCases = append(testCases, testCase{rt: rt, expectErr: true}) + } + + for _, tCase := range testCases { + tCase := tCase + t.Run(tCase.rt, func(t *testing.T) { + t.Parallel() + request := validator.StringRequest{ + Path: path.Root("test"), + PathExpression: path.MatchRoot("test"), + ConfigValue: types.StringValue(tCase.rt), + } + response := validator.StringResponse{} + + ParseRtValidator{}.ValidateString(ctx, request, &response) + if response.Diagnostics.HasError() && !tCase.expectErr { + t.Fail() // error where none expected + } + if !response.Diagnostics.HasError() && tCase.expectErr { + t.Fail() // expected error not found + } + }) + } +} diff --git a/apstra/blueprint/datacenter_routing_zone.go b/apstra/blueprint/datacenter_routing_zone.go index 78c0b5c8..66d9fade 100644 --- a/apstra/blueprint/datacenter_routing_zone.go +++ b/apstra/blueprint/datacenter_routing_zone.go @@ -241,11 +241,7 @@ func (o DatacenterRoutingZone) ResourceAttributes() map[string]resourceSchema.At ElementType: types.StringType, Validators: []validator.Set{ setvalidator.SizeAtLeast(1), - setvalidator.ValueStringsAre( - stringvalidator.RegexMatches( - regexp.MustCompile("^[0-9]+:[0-9]+$"), - "import_route_targets must take the form: \":\""), - ), + setvalidator.ValueStringsAre(apstravalidator.ParseRT()), }, }, "export_route_targets": resourceSchema.SetAttribute{ @@ -254,11 +250,7 @@ func (o DatacenterRoutingZone) ResourceAttributes() map[string]resourceSchema.At ElementType: types.StringType, Validators: []validator.Set{ setvalidator.SizeAtLeast(1), - setvalidator.ValueStringsAre( - stringvalidator.RegexMatches( - regexp.MustCompile("^[0-9]+:[0-9]+$"), - "export_route_targets must take the form: \":\""), - ), + setvalidator.ValueStringsAre(apstravalidator.ParseRT()), }, }, } diff --git a/apstra/resource_datacenter_routing_zone_test.go b/apstra/resource_datacenter_routing_zone_test.go index 01fd950d..c2f9121e 100644 --- a/apstra/resource_datacenter_routing_zone_test.go +++ b/apstra/resource_datacenter_routing_zone_test.go @@ -2,12 +2,15 @@ package tfapstra import ( "context" + "encoding/binary" "errors" "fmt" "github.com/Juniper/apstra-go-sdk/apstra" testutils "github.com/Juniper/terraform-provider-apstra/apstra/test_utils" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "math/rand" + "net" "strconv" "strings" "sync" @@ -31,15 +34,26 @@ type routeTargets struct { rts []string } -func (o *routeTargets) init(n int) { - s1 := make([]uint32, n) - s2 := make([]uint16, n) +// init loads o.rts with random RT strings +func (o *routeTargets) init(count int) { + s1 := make([]uint32, count) + s2 := make([]uint32, count) FillWithRandomIntegers(s1) FillWithRandomIntegers(s2) - o.rts = make([]string, n) - for i := 0; i < n; i++ { - o.rts[i] = fmt.Sprintf("%d:%d", s1[i], s2[i]) + o.rts = make([]string, count) + for i := 0; i < count; i++ { + r := rand.Intn(3) + switch r { + case 0: // force to 16-bits:32-bits + o.rts[i] = fmt.Sprintf("%d:%d", uint16(s1[i]), s2[i]) + case 1: // force to 32-bits:16-bits + o.rts[i] = fmt.Sprintf("%d:%d", s1[i], uint16(s2[i])) + case 2: // force to IPv4:16-bits + ip := make(net.IP, 4) + binary.BigEndian.PutUint32(ip, s1[i]) + o.rts[i] = fmt.Sprintf("%s:%d", ip.String(), uint16(s2[i])) + } } } @@ -138,7 +152,7 @@ func TestResourceDatacenterRoutingZone_A(t *testing.T) { rtsB := routeTargets{} rtsA.init(1) - rtsB.init(3) + rtsB.init(6) testCases := []testCase{ { From cdb3750c11921cd0f0bfae506aedd1740cf60093 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Fri, 27 Oct 2023 20:13:07 -0400 Subject: [PATCH 2/2] eliminate unused struct elements --- apstra/apstra_validator/parse_rt.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apstra/apstra_validator/parse_rt.go b/apstra/apstra_validator/parse_rt.go index d83ee8d6..33f63637 100644 --- a/apstra/apstra_validator/parse_rt.go +++ b/apstra/apstra_validator/parse_rt.go @@ -21,10 +21,7 @@ const ( var _ validator.String = ParseRtValidator{} -type ParseRtValidator struct { - requireIpv4 bool - requireIpv6 bool -} +type ParseRtValidator struct{} func (o ParseRtValidator) Description(_ context.Context) string { return "Ensures that the supplied can be parsed as a Route Target"