Validation for same rule path (#91)

kyma-project · Feb 13, 2020 · 748cf55 · 748cf55
1 parent 295e8fc
commit 748cf55
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 7 deletions.
diff --git a/internal/validation/helpers.go b/internal/validation/helpers.go
@@ -1,19 +1,36 @@
 package validation
 
 import (
+	"fmt"
 	"net/url"
 	"regexp"
 
 	gatewayv1alpha1 "github.com/kyma-incubator/api-gateway/api/v1alpha1"
 )
 
 func hasDuplicates(rules []gatewayv1alpha1.Rule) bool {
-	encountered := map[string]bool{}
-	// Create a map of all unique elements.
-	for v := range rules {
-		encountered[rules[v].Path] = true
+	duplicates := map[string]bool{}
+
+	if len(rules) > 1 {
+		for _, rule := range rules {
+			if len(rule.Methods) > 0 {
+				for _, method := range rule.Methods {
+					tmp := fmt.Sprintf("%s:%s", rule.Path, method)
+					if duplicates[tmp] {
+						return true
+					}
+					duplicates[tmp] = true
+				}
+			} else {
+				if duplicates[rule.Path] {
+					return true
+				}
+				duplicates[rule.Path] = true
+			}
+		}
 	}
-	return len(encountered) != len(rules)
+
+	return false
 }
 
 func isValidURL(toTest string) bool {
@@ -33,6 +50,7 @@ func ValidateDomainName(domain string) bool {
 	return RegExp.MatchString(domain)
 }
 
+//ValidateServiceName ?
 func ValidateServiceName(service string) bool {
 	regExp := regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?$`)
 	return regExp.MatchString(service)

diff --git a/internal/validation/validate.go b/internal/validation/validate.go
@@ -114,7 +114,7 @@ func (v *APIRule) validateRules(attributePath string, rules []gatewayv1alpha1.Ru
 	}
 
 	if hasDuplicates(rules) {
-		problems = append(problems, Failure{AttributePath: attributePath, Message: "multiple rules defined for the same path"})
+		problems = append(problems, Failure{AttributePath: attributePath, Message: "multiple rules defined for the same path and method"})
 	}
 
 	for i, r := range rules {

diff --git a/internal/validation/validate_test.go b/internal/validation/validate_test.go
@@ -255,7 +255,7 @@ var _ = Describe("Validate function", func() {
 		//then
 		Expect(problems).To(HaveLen(6))
 		Expect(problems[0].AttributePath).To(Equal(".spec.rules"))
-		Expect(problems[0].Message).To(Equal("multiple rules defined for the same path"))
+		Expect(problems[0].Message).To(Equal("multiple rules defined for the same path and method"))
 
 		Expect(problems[1].AttributePath).To(Equal(".spec.rules[0].accessStrategies[0].config"))
 		Expect(problems[1].Message).To(Equal("strategy: noop does not support configuration"))
@@ -273,6 +273,41 @@ var _ = Describe("Validate function", func() {
 		Expect(problems[5].Message).To(Equal("No accessStrategies defined"))
 	})
 
+	It("Should fail  for the same path and method", func() {
+		//given
+		testWhiteList := []string{"foo.bar", "bar.foo", "kyma.local"}
+		input := &gatewayv1alpha1.APIRule{
+			Spec: gatewayv1alpha1.APIRuleSpec{
+				Service: getService("foo-service", uint32(8080), "non-occupied-host.foo.bar"),
+				Rules: []gatewayv1alpha1.Rule{
+					{
+						Path: "/abc",
+						AccessStrategies: []*rulev1alpha1.Authenticator{
+							toAuthenticator("noop", emptyConfig()),
+						},
+						Methods: []string{"GET"},
+					},
+					{
+						Path: "/abc",
+						AccessStrategies: []*rulev1alpha1.Authenticator{
+							toAuthenticator("anonymous", emptyConfig()),
+						},
+						Methods: []string{"GET", "POST"},
+					},
+				},
+			},
+		}
+		//when
+		problems := (&APIRule{
+			DomainWhiteList: testWhiteList,
+		}).Validate(input, v1alpha3.VirtualServiceList{})
+
+		//then
+		Expect(problems).To(HaveLen(1))
+		Expect(problems[0].AttributePath).To(Equal(".spec.rules"))
+		Expect(problems[0].Message).To(Equal("multiple rules defined for the same path and method"))
+	})
+
 	It("Should succeed for valid input", func() {
 		//given
 		testWhiteList := []string{"foo.bar", "bar.foo", "kyma.local"}
@@ -295,6 +330,14 @@ var _ = Describe("Validate function", func() {
 							toAuthenticator("noop", emptyConfig()),
 						},
 					},
+					{
+						Path: "/abc",
+						AccessStrategies: []*rulev1alpha1.Authenticator{
+							toAuthenticator("jwt", simpleJWTConfig()),
+							toAuthenticator("noop", emptyConfig()),
+						},
+						Methods: []string{"GET"},
+					},
 					{
 						Path: "/bcd",
 						AccessStrategies: []*rulev1alpha1.Authenticator{
@@ -318,6 +361,49 @@ var _ = Describe("Validate function", func() {
 		//then
 		Expect(problems).To(HaveLen(0))
 	})
+
+	It("Should succeed for the same path but different methods", func() {
+		//given
+		testWhiteList := []string{"foo.bar", "bar.foo", "kyma.local"}
+
+		existingVS := v1alpha3.VirtualService{}
+		existingVS.OwnerReferences = []v1.OwnerReference{{UID: "12345"}}
+		existingVS.Spec.Hosts = []string{"occupied-host.foo.bar"}
+
+		input := &gatewayv1alpha1.APIRule{
+			ObjectMeta: v1.ObjectMeta{
+				UID: "67890",
+			},
+			Spec: gatewayv1alpha1.APIRuleSpec{
+				Service: getService("foo-service", uint32(8080), "non-occupied-host.foo.bar"),
+				Rules: []gatewayv1alpha1.Rule{
+					{
+						Path: "/abc",
+						AccessStrategies: []*rulev1alpha1.Authenticator{
+							toAuthenticator("jwt", simpleJWTConfig()),
+							toAuthenticator("noop", emptyConfig()),
+						},
+						Methods: []string{"POST"},
+					},
+					{
+						Path: "/abc",
+						AccessStrategies: []*rulev1alpha1.Authenticator{
+							toAuthenticator("jwt", simpleJWTConfig()),
+							toAuthenticator("noop", emptyConfig()),
+						},
+						Methods: []string{"GET"},
+					},
+				},
+			},
+		}
+		//when
+		problems := (&APIRule{
+			DomainWhiteList: testWhiteList,
+		}).Validate(input, v1alpha3.VirtualServiceList{Items: []v1alpha3.VirtualService{existingVS}})
+
+		//then
+		Expect(problems).To(HaveLen(0))
+	})
 })
 
 var _ = Describe("Validator for", func() {