From bf1bf5b0784e201baf5c93756e9ecae1b8d07fad Mon Sep 17 00:00:00 2001 From: alparslanavci Date: Fri, 6 Oct 2023 09:17:05 +0100 Subject: [PATCH] fix(policy): improve targetRef name and tags validation Added validation for targetRef name and tags as required. Also, MeshGateway and MeshHTTPRoute kind validations are included. Signed-off-by: alparslanavci --- .../core/matchers/validators/validator.go | 53 ++++- .../matchers/validators/validator_test.go | 186 ++++++++++++++++++ 2 files changed, 238 insertions(+), 1 deletion(-) diff --git a/pkg/plugins/policies/core/matchers/validators/validator.go b/pkg/plugins/policies/core/matchers/validators/validator.go index 3684ce922e35..a2f4dec129bf 100644 --- a/pkg/plugins/policies/core/matchers/validators/validator.go +++ b/pkg/plugins/policies/core/matchers/validators/validator.go @@ -2,11 +2,18 @@ package validators import ( "fmt" + "regexp" common_api "github.com/kumahq/kuma/api/common/v1alpha1" "github.com/kumahq/kuma/pkg/core/validators" ) +var ( + nameCharacterSet = regexp.MustCompile("^[0-9a-z-_]*$") + tagNameCharacterSet = regexp.MustCompile(`^[a-zA-Z0-9\.\-_:/]*$`) + tagValueCharacterSet = regexp.MustCompile(`^([a-zA-Z0-9\.\-_:/]*|\*)$`) +) + type ValidateTargetRefOpts struct { SupportedKinds []common_api.TargetRefKind } @@ -34,18 +41,62 @@ func ValidateTargetRef( case common_api.MeshSubset: verr.Add(disallowedField("name", ref.Name, refKind)) verr.Add(disallowedField("mesh", ref.Mesh, refKind)) - case common_api.MeshService: + verr.Add(validTags(ref.Tags)) + case common_api.MeshService, common_api.MeshGateway, common_api.MeshHTTPRoute: verr.Add(requiredField("name", ref.Name, refKind)) + verr.Add(validName(ref.Name)) verr.Add(disallowedField("mesh", ref.Mesh, refKind)) verr.Add(disallowedField("tags", ref.Tags, refKind)) case common_api.MeshServiceSubset: verr.Add(requiredField("name", ref.Name, refKind)) + verr.Add(validName(ref.Name)) verr.Add(disallowedField("mesh", ref.Mesh, refKind)) + verr.Add(validTags(ref.Tags)) } } return verr } +func validTags(tags map[string]string) validators.ValidationError { + res := validators.ValidationError{} + for key, value := range tags { + if key == "" { + res.Violations = append(res.Violations, validators.Violation{ + Field: "tags", Message: "tag name must be non-empty", + }) + } + if !tagNameCharacterSet.MatchString(key) { + res.Violations = append(res.Violations, validators.Violation{ + Field: "tags", + Message: "tag name must consist of alphanumeric characters, dots, dashes, slashes and underscores", + }) + } + if value == "" { + res.Violations = append(res.Violations, validators.Violation{ + Field: "tags", Message: "tag value must be non-empty", + }) + } + if !tagValueCharacterSet.MatchString(value) { + res.Violations = append(res.Violations, validators.Violation{ + Field: "tags", + Message: `tag value must consist of alphanumeric characters, dots, dashes, slashes and underscores or be "*"`, + }) + } + } + return res +} + +func validName(value string) validators.ValidationError { + res := validators.ValidationError{} + if !nameCharacterSet.MatchString(value) { + res.Violations = append(res.Violations, validators.Violation{ + Field: "name", + Message: "invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols.", + }) + } + return res +} + func contains(array []common_api.TargetRefKind, item common_api.TargetRefKind) bool { for _, it := range array { if it == item { diff --git a/pkg/plugins/policies/core/matchers/validators/validator_test.go b/pkg/plugins/policies/core/matchers/validators/validator_test.go index 4599280db0d3..5816a09fa929 100644 --- a/pkg/plugins/policies/core/matchers/validators/validator_test.go +++ b/pkg/plugins/policies/core/matchers/validators/validator_test.go @@ -48,6 +48,7 @@ kind: Mesh kind: MeshSubset tags: kuma.io/zone: us-east + validTagName: "*" `, opts: &matcher_validators.ValidateTargetRefOpts{ SupportedKinds: []common_api.TargetRefKind{ @@ -76,6 +77,28 @@ name: backend }, }, }), + Entry("MeshGateway", testCase{ + inputYaml: ` +kind: MeshGateway +name: gateway1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshGateway, + }, + }, + }), + Entry("MeshHTTPRoute", testCase{ + inputYaml: ` +kind: MeshHTTPRoute +name: http-route1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshHTTPRoute, + }, + }, + }), Entry("MeshServiceSubset", testCase{ inputYaml: ` kind: MeshServiceSubset @@ -225,6 +248,70 @@ name: mesh-1 violations: - field: targetRef.name message: cannot be set with kind MeshSubset`, + }), + Entry("MeshSubset with empty tag name", testCase{ + inputYaml: ` +kind: MeshSubset +tags: + "": value1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshSubset, + }, + }, + expected: ` +violations: + - field: targetRef.tags + message: tag name must be non-empty`, + }), + Entry("MeshSubset with invalid tag name", testCase{ + inputYaml: ` +kind: MeshSubset +tags: + invalidTag*: value1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshSubset, + }, + }, + expected: ` +violations: + - field: targetRef.tags + message: tag name must consist of alphanumeric characters, dots, dashes, slashes and underscores`, + }), + Entry("MeshSubset with empty tag value", testCase{ + inputYaml: ` +kind: MeshSubset +tags: + tag1: "" +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshSubset, + }, + }, + expected: ` +violations: + - field: targetRef.tags + message: tag value must be non-empty`, + }), + Entry("MeshSubset with invalid tag value", testCase{ + inputYaml: ` +kind: MeshSubset +tags: + tag1: invalidValue? +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshSubset, + }, + }, + expected: ` +violations: + - field: targetRef.tags + message: tag value must consist of alphanumeric characters, dots, dashes, slashes and underscores or be "*"`, }), Entry("MeshService when it's not supported", testCase{ inputYaml: ` @@ -274,6 +361,88 @@ violations: message: must be set with kind MeshService - field: targetRef.tags message: cannot be set with kind MeshService +`, + }), + Entry("MeshService with invalid name", testCase{ + inputYaml: ` +kind: MeshService +name: "*" +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshService, + }, + }, + expected: ` +violations: + - field: targetRef.name + message: invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols. +`, + }), + Entry("MeshGateway when it's not supported", testCase{ + inputYaml: ` +kind: MeshGateway +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshService, + }, + }, + expected: ` +violations: + - field: targetRef.kind + message: value is not supported`, + }), + Entry("MeshGateway with mesh", testCase{ + inputYaml: ` +kind: MeshGateway +name: gateway1 +mesh: mesh-1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshGateway, + }, + }, + expected: ` +violations: + - field: targetRef.mesh + message: cannot be set with kind MeshGateway +`, + }), + Entry("MeshGateway without name with tags", testCase{ + inputYaml: ` +kind: MeshGateway +tags: + tag1: value1 +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshGateway, + }, + }, + expected: ` +violations: + - field: targetRef.name + message: must be set with kind MeshGateway + - field: targetRef.tags + message: cannot be set with kind MeshGateway +`, + }), + Entry("MeshGateway with invalid name", testCase{ + inputYaml: ` +kind: MeshGateway +name: "*" +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshGateway, + }, + }, + expected: ` +violations: + - field: targetRef.name + message: invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols. `, }), Entry("MeshServiceSubset when it's not supported", testCase{ @@ -305,6 +474,23 @@ tags: {} violations: - field: targetRef.name message: must be set with kind MeshServiceSubset +`, + }), + Entry("MeshServiceSubset with invalid name with empty tags", testCase{ + inputYaml: ` +kind: MeshServiceSubset +name: "*" +tags: {} +`, + opts: &matcher_validators.ValidateTargetRefOpts{ + SupportedKinds: []common_api.TargetRefKind{ + common_api.MeshServiceSubset, + }, + }, + expected: ` +violations: + - field: targetRef.name + message: invalid characters. Valid characters are numbers, lowercase latin letters and '-', '_' symbols. `, }), Entry("MeshServiceSubset with mesh", testCase{