reuse validate selectors

kumahq · Nov 14, 2019 · a88dd54 · a88dd54
1 parent a402159
commit a88dd54
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 45 deletions.
diff --git a/pkg/core/resources/apis/mesh/proxytemplate_validator.go b/pkg/core/resources/apis/mesh/proxytemplate_validator.go
@@ -24,9 +24,9 @@ func init() {
 
 func (t *ProxyTemplateResource) Validate() error {
 	var verr validators.ValidationError
-	verr.AddError("", validateImports(t.Spec.Imports))
-	verr.AddError("", validateResources(t.Spec.Resources))
-	verr.AddError("", validateSelectors(t.Spec.Selectors))
+	verr.Add(validateImports(t.Spec.Imports))
+	verr.Add(validateResources(t.Spec.Resources))
+	verr.Add(validateSelectors(t.Spec.Selectors))
 	return verr.OrNil()
 }
 
@@ -63,19 +63,5 @@ func validateResources(resources []*v1alpha1.ProxyTemplateRawResource) validator
 }
 
 func validateSelectors(selectors []*v1alpha1.Selector) validators.ValidationError {
-	var verr validators.ValidationError
-	for i, selector := range selectors {
-		if len(selector.Match) == 0 {
-			verr.AddViolationAt(validators.RootedAt("selectors").Index(i), "has to contain at least one tag")
-		}
-		for key, value := range selector.Match {
-			if key == "" {
-				verr.AddViolationAt(validators.RootedAt("selectors").Index(i).Key(key), "tag cannot be empty")
-			}
-			if value == "" {
-				verr.AddViolationAt(validators.RootedAt("selectors").Index(i).Key(key), "value of tag cannot be empty")
-			}
-		}
-	}
-	return verr
+	return ValidateSelectors(validators.RootedAt("selectors"), selectors, ValidateSelectorsOpts{})
 }
diff --git a/pkg/core/resources/apis/mesh/proxytemplate_validator_test.go b/pkg/core/resources/apis/mesh/proxytemplate_validator_test.go
@@ -123,8 +123,10 @@ var _ = Describe("ProxyTemplate", func() {
                 - match:`,
 				expected: `
                 violations:
-                - field: selectors[0]
-                  message: has to contain at least one tag`,
+                - field: selectors[0].match
+                  message: must have at least one tag
+                - field: selectors[0].match
+                  message: mandatory tag "service" is missing`,
 			}),
 			Entry("empty tag", testCase{
 				proxyTemplate: `
@@ -133,8 +135,10 @@ var _ = Describe("ProxyTemplate", func() {
                     "": asdf`,
 				expected: `
                 violations:
-                - field: selectors[0][""]
-                  message: tag cannot be empty`,
+                - field: selectors[0].match
+                  message: tag key must be non-empty
+                - field: selectors[0].match
+                  message: mandatory tag "service" is missing`,
 			}),
 			Entry("empty tag value", testCase{
 				proxyTemplate: `
@@ -143,8 +147,8 @@ var _ = Describe("ProxyTemplate", func() {
                     service:`,
 				expected: `
                 violations:
-                - field: 'selectors[0]["service"]'
-                  message: value of tag cannot be empty`,
+                - field: 'selectors[0].match["service"]'
+                  message: tag value must be non-empty`,
 			}),
 			Entry("validation error from envoy protobuf resource", testCase{
 				proxyTemplate: `

diff --git a/pkg/core/resources/apis/mesh/traffic_log_validator.go b/pkg/core/resources/apis/mesh/traffic_log_validator.go
@@ -13,7 +13,9 @@ func (d *TrafficLogResource) Validate() error {
 }
 
 func (d *TrafficLogResource) validateSources() validators.ValidationError {
-	return ValidateSelectors(validators.RootedAt("sources"), d.Spec.Sources, ValidateSelectorOpts{})
+	return ValidateSelectors(validators.RootedAt("sources"), d.Spec.Sources, ValidateSelectorsOpts{
+		RequireAtLeastOneSelector: true,
+	})
 }
 
 func (d *TrafficLogResource) validateDestinations() (err validators.ValidationError) {

diff --git a/pkg/core/resources/apis/mesh/traffic_route_validator.go b/pkg/core/resources/apis/mesh/traffic_route_validator.go
@@ -13,7 +13,9 @@ func (d *TrafficRouteResource) Validate() error {
 }
 
 func (d *TrafficRouteResource) validateSources() validators.ValidationError {
-	return ValidateSelectors(validators.RootedAt("sources"), d.Spec.Sources, ValidateSelectorOpts{})
+	return ValidateSelectors(validators.RootedAt("sources"), d.Spec.Sources, ValidateSelectorsOpts{
+		RequireAtLeastOneSelector: true,
+	})
 }
 
 func (d *TrafficRouteResource) validateDestinations() (err validators.ValidationError) {

diff --git a/pkg/core/resources/apis/mesh/validators.go b/pkg/core/resources/apis/mesh/validators.go
@@ -19,12 +19,17 @@ type ValidateSelectorOpts struct {
 	ExtraTagValueValidators  []TagValueValidatorFunc
 }
 
-func ValidateSelectors(path validators.PathBuilder, sources []*mesh_proto.Selector, opts ValidateSelectorOpts) (err validators.ValidationError) {
-	if len(sources) == 0 {
+type ValidateSelectorsOpts struct {
+	ValidateSelectorOpts
+	RequireAtLeastOneSelector bool
+}
+
+func ValidateSelectors(path validators.PathBuilder, sources []*mesh_proto.Selector, opts ValidateSelectorsOpts) (err validators.ValidationError) {
+	if opts.RequireAtLeastOneSelector && len(sources) == 0 {
 		err.AddViolationAt(path, "must have at least one element")
 	}
 	for i, selector := range sources {
-		err.Add(ValidateSelector(path.Index(i).Field("match"), selector.GetMatch(), opts))
+		err.Add(ValidateSelector(path.Index(i).Field("match"), selector.GetMatch(), opts.ValidateSelectorOpts))
 	}
 	return
 }
@@ -58,23 +63,26 @@ func ValidateSelector(path validators.PathBuilder, selector map[string]string, o
 	return
 }
 
-var OnlyServiceTagAllowed = ValidateSelectorOpts{
-	SkipRequireAtLeastOneTag: true,
-	ExtraSelectorValidators: []SelectorValidatorFunc{
-		func(path validators.PathBuilder, selector map[string]string) (err validators.ValidationError) {
-			_, defined := selector[mesh_proto.ServiceTag]
-			if len(selector) != 1 || !defined {
-				err.AddViolationAt(path, fmt.Sprintf("must consist of exactly one tag %q", mesh_proto.ServiceTag))
-			}
-			return
+var OnlyServiceTagAllowed = ValidateSelectorsOpts{
+	RequireAtLeastOneSelector: true,
+	ValidateSelectorOpts: ValidateSelectorOpts{
+		SkipRequireAtLeastOneTag: true,
+		ExtraSelectorValidators: []SelectorValidatorFunc{
+			func(path validators.PathBuilder, selector map[string]string) (err validators.ValidationError) {
+				_, defined := selector[mesh_proto.ServiceTag]
+				if len(selector) != 1 || !defined {
+					err.AddViolationAt(path, fmt.Sprintf("must consist of exactly one tag %q", mesh_proto.ServiceTag))
+				}
+				return
+			},
 		},
-	},
-	ExtraTagKeyValidators: []TagKeyValidatorFunc{
-		func(path validators.PathBuilder, key string) (err validators.ValidationError) {
-			if key != mesh_proto.ServiceTag {
-				err.AddViolationAt(path.Key(key), fmt.Sprintf("tag %q is not allowed", key))
-			}
-			return
+		ExtraTagKeyValidators: []TagKeyValidatorFunc{
+			func(path validators.PathBuilder, key string) (err validators.ValidationError) {
+				if key != mesh_proto.ServiceTag {
+					err.AddViolationAt(path.Key(key), fmt.Sprintf("tag %q is not allowed", key))
+				}
+				return
+			},
 		},
 	},
 }