Full support for special characters in asterisk paths

kyma-project · Dec 27, 2024 · 2162097 · 2162097
1 parent 1afe8a4
commit 2162097
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 8 deletions.
diff --git a/internal/processing/processors/v2alpha1/reconciliation_test.go b/internal/processing/processors/v2alpha1/reconciliation_test.go
@@ -266,9 +266,9 @@ var _ = Describe("Reconciliation", func() {
 						Expect(http.Route[0].Destination.Host).To(Equal("example-service.some-namespace.svc.cluster.local"))
 
 						switch http.Match[0].Uri.MatchType.(*v1beta1.StringMatch_Regex).Regex {
-						case "/test$":
+						case "^/test$":
 							break
-						case "/different-path$":
+						case "^/different-path$":
 							break
 						default:
 							Fail("Unexpected match type")
@@ -342,9 +342,9 @@ var _ = Describe("Reconciliation", func() {
 						Expect(http.Route[0].Destination.Host).To(Equal("example-service.some-namespace.svc.cluster.local"))
 
 						switch http.Match[0].Uri.MatchType.(*v1beta1.StringMatch_Regex).Regex {
-						case "/test$":
+						case "^/test$":
 							break
-						case "/different-path$":
+						case "^/different-path$":
 							break
 						default:
 							Fail("Unexpected match type")

diff --git a/internal/processing/processors/v2alpha1/virtualservice/regex_test.go b/internal/processing/processors/v2alpha1/virtualservice/regex_test.go
@@ -0,0 +1,42 @@
+package virtualservice
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"regexp"
+)
+
+var _ = Describe("Envoy templates regex matching", func() {
+	DescribeTable(`{\*} template`,
+		func(input string, shouldMatch bool) {
+			// when
+			matched, err := regexp.MatchString(prepareRegexPath("/{*}"), input)
+
+			// then
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(matched).To(Equal(shouldMatch))
+		},
+		Entry("should not match empty path", "/", false),
+		Entry("should match path with one segment", "/segment", true),
+		Entry("should match special characters", "/segment-._~!$&'()*+,;=:@", true),
+		Entry("should match with correct % encoding", "/segment%20", true),
+		Entry("should not match with incorrect % encoding", "/segment%2", false),
+		Entry("should not match path with multiple segments", "/segment1/segment2/segment3", false),
+	)
+
+	DescribeTable(`{\*\*} template`, func(input string, shouldMatch bool) {
+		// when
+		matched, err := regexp.MatchString(prepareRegexPath("/{**}"), input)
+
+		// then
+		Expect(err).To(Not(HaveOccurred()))
+		Expect(matched).To(Equal(shouldMatch))
+	},
+		Entry("should match empty path", "/", true),
+		Entry("should match path with one segment", "/segment", true),
+		Entry("should match special characters", "/segment-._~!$&'()*+,;=:@", true),
+		Entry("should match with correct % encoding", "/segment%20", true),
+		Entry("should not match with incorrect % encoding", "/segment%2", false),
+		Entry("should match path with multiple segments", "/segment1/segment2/segment3", true),
+	)
+})
diff --git a/internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor.go b/internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor.go
@@ -20,8 +20,8 @@ const defaultHttpTimeout uint32 = 180
 
 var (
 	envoyTemplatesTranslation = map[string]string{
-		`{**}`: `[\w\.~\-\/]*`,
-		`{*}`:  `[\w\.~\-]*`,
+		`{**}`: `([A-Za-z0-9-._~!$&'()*+,;=:@/]|%[0-9a-fA-F]{2})*`,
+		`{*}`:  `([A-Za-z0-9-._~!$&'()*+,;=:@]|%[0-9a-fA-F]{2})+`,
 	}
 )
 
@@ -201,7 +201,7 @@ func prepareRegexPath(path string) string {
 		path = strings.ReplaceAll(path, key, replace)
 	}
 
-	return fmt.Sprintf("%s$", path)
+	return fmt.Sprintf("^%s$", path)
 }
 
 func GetVirtualServiceHttpTimeout(apiRuleSpec gatewayv2alpha1.APIRuleSpec, rule gatewayv2alpha1.Rule) uint32 {

diff --git a/internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor_test.go b/internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor_test.go
@@ -96,7 +96,7 @@ var _ = Describe("Fully configured APIRule happy path", func() {
 				Expect(vs.Spec.Http).To(HaveLen(2))
 
 				Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET|POST)$"))
-				Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal("/$"))
+				Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal("^/$"))
 				Expect(vs.Spec.Http[0].Route[0].Destination.Host).To(Equal("another-service.another-namespace.svc.cluster.local"))
 				Expect(vs.Spec.Http[0].Route[0].Destination.Port.Number).To(Equal(uint32(9999)))
 
@@ -155,6 +155,74 @@ var _ = Describe("VirtualServiceProcessor", func() {
 		Expect(result[0].Action.String()).To(Equal("create"))
 	})
 
+	It("should create a VirtualService with prefix match for wildcard path /*", func() {
+		// given
+		apiRule := NewAPIRuleBuilder().
+			WithGateway("example/example").
+			WithHosts("example.com").
+			WithService("example-service", "example-namespace", 8080).
+			WithTimeout(180).
+			WithRules(
+				NewRuleBuilder().
+					WithMethods("GET").
+					WithPath("/*").
+					NoAuth().Build(),
+			).
+			Build()
+
+		client := GetFakeClient()
+		processor := processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule, nil)
+
+		// when
+		checkVirtualServices(client, processor, []verifier{
+			func(vs *networkingv1beta1.VirtualService) {
+				Expect(vs.Spec.Hosts).To(ConsistOf("example.com"))
+				Expect(vs.Spec.Gateways).To(ConsistOf("example/example"))
+				Expect(vs.Spec.Http).To(HaveLen(1))
+
+				Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET)$"))
+				Expect(vs.Spec.Http[0].Match[0].Uri.GetPrefix()).To(Equal("/"))
+			},
+		}, nil, "create")
+	})
+
+	DescribeTable("should create a VirtualService with correct regex for path",
+		func(path string, expectedRegex string) {
+			// given
+			apiRule := NewAPIRuleBuilder().
+				WithGateway("example/example").
+				WithHosts("example.com").
+				WithService("example-service", "example-namespace", 8080).
+				WithTimeout(180).
+				WithRules(
+					NewRuleBuilder().
+						WithMethods("GET").
+						WithPath(path).
+						NoAuth().Build(),
+				).
+				Build()
+
+			client := GetFakeClient()
+			processor := processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule, nil)
+
+			// when
+			checkVirtualServices(client, processor, []verifier{
+				func(vs *networkingv1beta1.VirtualService) {
+					Expect(vs.Spec.Hosts).To(ConsistOf("example.com"))
+					Expect(vs.Spec.Gateways).To(ConsistOf("example/example"))
+					Expect(vs.Spec.Http).To(HaveLen(1))
+
+					Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET)$"))
+					Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal(expectedRegex))
+				},
+			}, nil, "create")
+		},
+		Entry("path is /", "/", "^/$"),
+		Entry("path is /test", "/test", "^/test$"),
+		Entry("path is /test/{*}", "/test/{*}", "^/test/([A-Za-z0-9-._~!$&'()*+,;=:@]|%[0-9a-fA-F]{2})+$"),
+		Entry("path is /test/{**}", "/test/{**}", "^/test/([A-Za-z0-9-._~!$&'()*+,;=:@/]|%[0-9a-fA-F]{2})*$"),
+	)
+
 	It("should create a VirtualService with '/' prefix, when rule in APIRule applies to all paths", func() {
 		apiRule := NewAPIRuleBuilder().
 			WithGateway("example/example").

diff --git a/tests/integration/testsuites/v2alpha1/features/asterisk.feature b/tests/integration/testsuites/v2alpha1/features/asterisk.feature
@@ -7,5 +7,7 @@ Feature: Exposing service using asterisks
     Then Calling the "/anything/random/one" endpoint with "GET" method should result in status between 200 and 200
     And Calling the "/anything/any/random/two" endpoint with "POST" method should result in status between 200 and 200
     And Calling the "/anything/any/random" endpoint with "PUT" method should result in status between 200 and 200
+    And Calling the "/anything/a+b" endpoint with "PUT" method should result in status between 200 and 200
+    And Calling the "/anything/a%20b" endpoint with "PUT" method should result in status between 200 and 200
     And Calling the "/anything/random/one/any/random/two" endpoint with "DELETE" method should result in status between 200 and 200
     And Teardown httpbin service