feat: Validate network security group resource multycloud#271

- check that name matches azure's restrictions
- check security rules have a valid port range
- check that rules cidr block is valid
- check that rules protocol is valid
- refactor and create separate type for range validation: port range and priority

resources/types/network_security_group.go

@@ -1,7 +1,6 @@
 package types
 
 import (
-	"fmt"
 	"github.com/multycloud/multy/api/proto/resourcespb"
 	"github.com/multycloud/multy/resources"
 	"github.com/multycloud/multy/validate"
@@ -70,21 +69,28 @@ func NewNetworkSecurityGroup(nsg *NetworkSecurityGroup, resourceId string, args
 	}
 	return nil
 }
-func validatePort(port int32) bool {
-	return port >= 0 && port <= 65535
-}
 
 func (r *NetworkSecurityGroup) Validate(ctx resources.MultyContext) (errs []validate.ValidationError) {
 	errs = append(errs, r.ResourceWithId.Validate()...)
+	if err := validate.NewWordWithDotHyphenUnder80Validator().Check(r.Args.Name, r.ResourceId); err != nil {
+		errs = append(errs, r.NewValidationError(err, "name"))
+	}
+	cidrValidator := validate.NewCIDRIPv4Check()
+	portValidator := validate.NewPortCheck()
+	protoValidator := validate.NewProtocolCheck()
 	for _, rule := range r.Args.Rules {
-		if !validatePort(rule.PortRange.To) {
-			errs = append(errs, r.NewValidationError(fmt.Errorf("rule to_port \"%d\" is not valid", rule.PortRange.To), "rules"))
+		if err := portValidator.Check(rule.PortRange.To, "to_port"); err != nil {
+			errs = append(errs, r.NewValidationError(err, "rules"))
+		}
+		if err := portValidator.Check(rule.PortRange.From, "from_port"); err != nil {
+			errs = append(errs, r.NewValidationError(err, "rules"))
+		}
+		if err := cidrValidator.Check(rule.CidrBlock, "rule cidr"); err != nil {
+			errs = append(errs, r.NewValidationError(err, "rules"))
 		}
-		if !validatePort(rule.PortRange.From) {
-			errs = append(errs, r.NewValidationError(fmt.Errorf("rule from_port \"%d\" is not valid", rule.PortRange.From), "rules"))
+		if err := protoValidator.Check(rule.Protocol, "rule protocol"); err != nil {
+			errs = append(errs, r.NewValidationError(err, "rules"))
 		}
-		// TODO validate CIDR
-		//		validate protocol
 	}
 	// TODO validate location matches with VN location
 	return errs

resources/types/subnet.go

@@ -1,14 +1,12 @@
 package types
 
 import (
-	"fmt"
 	"github.com/apparentlymart/go-cidr/cidr"
 	"github.com/multycloud/multy/api/errors"
 	"github.com/multycloud/multy/api/proto/resourcespb"
 	"github.com/multycloud/multy/resources"
 	"github.com/multycloud/multy/validate"
 	"net"
-	"regexp"
 )
 
 /*
@@ -50,16 +48,12 @@ func NewSubnet(s *Subnet, resourceId string, subnet *resourcespb.SubnetArgs, oth
 }
 
 func (r *Subnet) Validate(ctx resources.MultyContext) (errs []validate.ValidationError) {
-	nameRestrictionRegex := regexp.MustCompile(validate.WordWithDotHyphenUnder80Pattern)
-	if !nameRestrictionRegex.MatchString(r.Args.Name) {
-		errs = append(errs, r.NewValidationError(fmt.Errorf("%s can contain only alphanumerics, underscores, periods, and hyphens;"+
-			" must start with alphanumeric and end with alphanumeric or underscore and have 1-80 lenght", r.ResourceId), "name"))
+	if err := validate.NewWordWithDotHyphenUnder80Validator().Check(r.Args.Name, r.ResourceId); err != nil {
+		errs = append(errs, r.NewValidationError(err, "name"))
 	}
-
-	if len(r.Args.CidrBlock) == 0 { // max len?
-		errs = append(errs, r.NewValidationError(fmt.Errorf("%s cidr_block length is invalid", r.ResourceId), "cidr_block"))
+	if err := validate.NewCIDRIPv4Check().Check(r.Args.CidrBlock, r.ResourceId); err != nil {
+		errs = append(errs, r.NewValidationError(err, "cidr_block"))
 	}
-
 	if _, vNetBlock, err := net.ParseCIDR(r.Args.CidrBlock); err == nil {
 		if _, subnetBlock, err := net.ParseCIDR(r.Args.CidrBlock); err != nil {
 			errs = append(errs, validate.ValidationError{

resources/types/virtual_network.go

@@ -1,12 +1,10 @@
 package types
 
 import (
-	"fmt"
 	"github.com/multycloud/multy/api/proto/resourcespb"
 	"github.com/multycloud/multy/resources"
 	"github.com/multycloud/multy/validate"
 	"net"
-	"regexp"
 )
 
 /*
@@ -52,12 +50,9 @@ func NewVirtualNetwork(r *VirtualNetwork, resourceId string, vn *resourcespb.Vir
 
 func (r *VirtualNetwork) Validate(ctx resources.MultyContext) (errs []validate.ValidationError) {
 	errs = append(errs, r.ResourceWithId.Validate()...)
-	nameRestrictionRegex := regexp.MustCompile(validate.WordWithDotHyphenUnder80Pattern)
-	if !nameRestrictionRegex.MatchString(r.Args.Name) {
-		errs = append(errs, r.NewValidationError(fmt.Errorf("%s can contain only alphanumerics, underscores, periods, and hyphens;"+
-			" must start with alphanumeric and end with alphanumeric or underscore and have 1-80 lenght", r.ResourceId), "name"))
+	if err := validate.NewWordWithDotHyphenUnder80Validator().Check(r.Args.Name, r.ResourceId); err != nil {
+		errs = append(errs, r.NewValidationError(err, "name"))
 	}
-
 	if len(r.Args.CidrBlock) == 0 { // max len?
 		errs = append(errs, validate.ValidationError{
 			ErrorMessage: "cidr_block length is invalid",

validate/validate.go

@@ -4,13 +4,96 @@ import (
 	"bufio"
 	"fmt"
 	"github.com/hashicorp/hcl/v2"
+	"golang.org/x/exp/constraints"
 	"io/ioutil"
+	"regexp"
 )
 
-// WordWithDotHyphenUnder80Pattern is a regexp pattern that matches string that contain alphanumerics, underscores, periods,
+type RegexpValidator struct {
+	pattern       string
+	errorTemplate string
+	regex         *regexp.Regexp
+}
+
+// Check validates provided string with a regexp based on the pattern and returns optional error.
+func (r *RegexpValidator) Check(value string, valueType interface{}) error {
+	r.regex = regexp.MustCompile(r.pattern)
+	if !r.regex.MatchString(value) {
+		return fmt.Errorf(r.errorTemplate, valueType)
+	}
+	return nil
+}
+
+// wordWithDotHyphenUnder80Pattern is a regexp pattern that matches string that contain alphanumerics, underscores, periods,
 // and hyphens that start with alphanumeric and End alphanumeric or underscore. Limits size to 1-80.
 // Based on https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules
-const WordWithDotHyphenUnder80Pattern = string(`^[a-zA-Z\d]$|^[a-zA-Z\d][\w\-.]{0,78}\w$`)
+const wordWithDotHyphenUnder80Pattern = string(`^[a-zA-Z\d]$|^[a-zA-Z\d][\w\-.]{0,78}\w$`)
+
+//NewWordWithDotHyphenUnder80Validator creates new RegexpValidator validating with wordWithDotHyphenUnder80Pattern.
+func NewWordWithDotHyphenUnder80Validator() *RegexpValidator {
+	return &RegexpValidator{wordWithDotHyphenUnder80Pattern, "%s can contain only alphanumerics, underscores, periods, and hyphens;" +
+		" must start with alphanumeric and end with alphanumeric or underscore and have 1-80 length", nil}
+}
+
+// cidrIPv4Pattern defines CIDR IPv4 notation with or without mask.
+const cidrIPv4Pattern = string(`^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)([\/][0-3][0-2]?|[\/][1-2][0-9]|[\/][0-9])?$`)
+
+//NewCIDRIPv4Check creates new RegexpValidator validating CIDR IPv4
+func NewCIDRIPv4Check() *RegexpValidator {
+	return &RegexpValidator{cidrIPv4Pattern, "%s not valid CIDR IPv4 value", nil}
+}
+
+// matchWholeWordsPattern creates OR words matching regexp pattern with words. Regexp special characters must be
+// escaped.
+func matchWholeWordsPattern(words []string) string {
+	var pattern string
+	for i, word := range words {
+		if len(word) == 0 {
+			continue
+		}
+		pattern += fmt.Sprintf(`^(%s)$`, word)
+		if i != len(words)-1 {
+			pattern += `|`
+		}
+	}
+	return pattern
+}
+
+// NewProtocolCheck checks if provided protocol value is allowed in every deployment environment.
+func NewProtocolCheck() *RegexpValidator {
+	return &RegexpValidator{matchWholeWordsPattern([]string{"tcp", "udp", "icmp", "\\*"}),
+		"%s didn't match any protocol allowed value", nil}
+}
+
+// InRangeIncludingCheck represents <lowerBound, upperBound> range.
+type InRangeIncludingCheck[T constraints.Ordered] struct {
+	errorTemplate string
+	lowerBound    T
+	upperBound    T
+}
+
+func (i *InRangeIncludingCheck[T]) Check(value T, valueType interface{}) error {
+	if value < i.lowerBound {
+		return fmt.Errorf(i.errorTemplate, valueType, value, "lower", i.lowerBound)
+	} else if value > i.upperBound {
+		return fmt.Errorf(i.errorTemplate, valueType, value, "higher", i.lowerBound)
+	}
+	return nil
+}
+
+func newInRangeExcludingCheck[T constraints.Ordered](errorTemplate string, lower, upper T) InRangeIncludingCheck[T] {
+	return InRangeIncludingCheck[T]{errorTemplate, lower, upper}
+}
+
+// NewPortCheck creates InRangeIncludingCheck that can validate port correctness.
+func NewPortCheck() InRangeIncludingCheck[int32] {
+	return newInRangeExcludingCheck[int32]("%v port %v cannot be %v than %v", 0, 65535)
+}
+
+// NewPriorityCheck creates InRangeIncludingCheck that can validate priority value.
+func NewPriorityCheck() InRangeIncludingCheck[int64] {
+	return newInRangeExcludingCheck[int64]("%v priority value %v cannot be %v than %v", 100, 4096)
+}
 
 type ResourceValidationInfo struct {
 	SourceRanges  map[string]hcl.Range

validate/validate_test.go

@@ -2,18 +2,25 @@ package validate_test
 
 import (
 	"github.com/multycloud/multy/validate"
-	"regexp"
 	"testing"
 )
 
-// TestWordWithDotHyphenUnder80Pattern checks whether validate.WordWithDotHyphenUnder80Pattern matches
-// expected expressions
-func TestWordWithDotHyphenUnder80Pattern(t *testing.T) {
-	testRegexp, err := regexp.Compile(validate.WordWithDotHyphenUnder80Pattern)
-	if err != nil {
-		t.Fatalf("Could not compile regex: %s", validate.WordWithDotHyphenUnder80Pattern)
+func testRegexp(validator *validate.RegexpValidator, shouldMatch, shouldntMatch []string, t *testing.T) {
+	for _, name := range shouldMatch {
+		if err := validator.Check(name, "some_val"); err != nil {
+			t.Errorf("%v should match %s, but didn't", validator, name)
+		}
 	}
+	for _, name := range shouldntMatch {
+		if err := validator.Check(name, "some_val"); err == nil {
+			t.Errorf("%v shouldn't match %s, but did", validator, name)
+		}
+	}
+}
 
+// TestWordWithDotHyphenUnder80Pattern checks whether validate.wordWithDotHyphenUnder80Pattern matches
+// expected expressions
+func TestWordWithDotHyphenUnder80Pattern(t *testing.T) {
 	shouldMatch := []string{
 		"a",
 		"9",
@@ -27,15 +34,71 @@ func TestWordWithDotHyphenUnder80Pattern(t *testing.T) {
 		"ThisIs68dots...................................................................._",
 		"Maybe?inThe.Middle_",
 	}
+	testRegexp(validate.NewWordWithDotHyphenUnder80Validator(), shouldMatch, shouldntMatch, t)
+}
 
-	for _, name := range shouldMatch {
-		if !testRegexp.MatchString(name) {
-			t.Errorf("%s should match %s, but didn't", validate.WordWithDotHyphenUnder80Pattern, name)
+// TestCIDRIPv4Matching checks the correctness of validate.cidrIPv4Pattern
+func TestCIDRIPv4Matching(t *testing.T) {
+	shouldMatch := []string{
+		"10.0.0.1",
+		"0.0.0.0/0",
+		"255.255.255.255/32",
+		"172.16.0.0/16",
+	}
+	shouldntMatch := []string{
+		"This is not CIDR",
+		"2001:0db8:85a3:0000:0000:8a2e:0370:7334",
+	}
+	testRegexp(validate.NewCIDRIPv4Check(), shouldMatch, shouldntMatch, t)
+}
+
+// TestProtocolMatching checks if only allowed protocol values match
+func TestProtocolMatching(t *testing.T) {
+	shouldMatch := []string{
+		"tcp",
+		"udp",
+		"icmp",
+		"*",
+	}
+	shouldntMatch := []string{
+		"TCP",
+		"ah",
+		"IcmpV6",
+		"ESP",
+		"Oh",
+		"*anything",
+	}
+	testRegexp(validate.NewProtocolCheck(), shouldMatch, shouldntMatch, t)
+}
+
+func TestPortRangeCheck(t *testing.T) {
+	portCheck := validate.NewPortCheck()
+	ok := []int32{0, 80, 8080, 443, 22, 65535}
+	notOk := []int32{-1, 65536}
+	for _, v := range ok {
+		if err := portCheck.Check(v, "port"); err != nil {
+			t.Errorf("%v should match, but didn't", v)
 		}
 	}
-	for _, name := range shouldntMatch {
-		if testRegexp.MatchString(name) {
-			t.Errorf("%s shouldn't match %s, but did", validate.WordWithDotHyphenUnder80Pattern, name)
+	for _, v := range notOk {
+		if err := portCheck.Check(v, "port"); err == nil {
+			t.Errorf("%v shouln't match, but did", v)
+		}
+	}
+}
+
+func TestPriorityCheck(t *testing.T) {
+	priorityCheck := validate.NewPriorityCheck()
+	ok := []int64{100, 4096, 101, 202}
+	notOk := []int64{99, 4097, 0, -1}
+	for _, v := range ok {
+		if err := priorityCheck.Check(v, "priority"); err != nil {
+			t.Errorf("%v should match, but didn't", v)
+		}
+	}
+	for _, v := range notOk {
+		if err := priorityCheck.Check(v, "priority"); err == nil {
+			t.Errorf("%v shouln't match, but did", v)
 		}
 	}
 }