From 80d6ecfddb0c349dfc453787d0b89d253530dba4 Mon Sep 17 00:00:00 2001 From: Qianglin Li Date: Fri, 2 Jun 2023 18:56:22 +0800 Subject: [PATCH] feat: support full path regex annotation (#286) Signed-off-by: charlie --- pkg/ingress/kube/annotations/annotations.go | 10 ++- pkg/ingress/kube/annotations/rewrite.go | 14 ++- pkg/ingress/kube/common/model.go | 6 +- pkg/ingress/kube/common/tool_test.go | 6 +- pkg/ingress/kube/ingress/controller.go | 22 +++-- pkg/ingress/kube/ingressv1/controller.go | 22 +++-- .../tests/httproute-full-path-regex.go | 89 +++++++++++++++++++ .../tests/httproute-full-path-regex.yaml | 53 +++++++++++ .../tests/httproute-rewrite-path.go | 14 ++- .../tests/httproute-rewrite-path.yaml | 1 + test/ingress/e2e_test.go | 1 + 11 files changed, 215 insertions(+), 23 deletions(-) create mode 100644 test/ingress/conformance/tests/httproute-full-path-regex.go create mode 100644 test/ingress/conformance/tests/httproute-full-path-regex.yaml diff --git a/pkg/ingress/kube/annotations/annotations.go b/pkg/ingress/kube/annotations/annotations.go index ef2c6f294..b3ebd3e06 100644 --- a/pkg/ingress/kube/annotations/annotations.go +++ b/pkg/ingress/kube/annotations/annotations.go @@ -76,7 +76,15 @@ func (i *Ingress) NeedRegexMatch() bool { return false } - return i.Rewrite.RewriteTarget != "" || i.Rewrite.UseRegex + return i.Rewrite.RewriteTarget != "" || i.IsPrefixRegexMatch() || i.IsFullPathRegexMatch() +} + +func (i *Ingress) IsPrefixRegexMatch() bool { + return i.Rewrite.UseRegex +} + +func (i *Ingress) IsFullPathRegexMatch() bool { + return i.Rewrite.FullPathRegex } func (i *Ingress) IsCanary() bool { diff --git a/pkg/ingress/kube/annotations/rewrite.go b/pkg/ingress/kube/annotations/rewrite.go index 825e0162b..84420aafc 100644 --- a/pkg/ingress/kube/annotations/rewrite.go +++ b/pkg/ingress/kube/annotations/rewrite.go @@ -25,6 +25,7 @@ const ( rewritePath = "rewrite-path" rewriteTarget = "rewrite-target" useRegex = "use-regex" + fullPathRegex = "full-path-regex" upstreamVhost = "upstream-vhost" re2Regex = "\\$[0-9]" @@ -38,6 +39,7 @@ var ( type RewriteConfig struct { RewriteTarget string UseRegex bool + FullPathRegex bool RewriteHost string RewritePath string } @@ -52,13 +54,16 @@ func (r rewrite) Parse(annotations Annotations, config *Ingress, _ *GlobalContex rewriteConfig := &RewriteConfig{} rewriteConfig.RewriteTarget, _ = annotations.ParseStringASAP(rewriteTarget) rewriteConfig.UseRegex, _ = annotations.ParseBoolASAP(useRegex) + rewriteConfig.FullPathRegex, _ = annotations.ParseBoolForHigress(fullPathRegex) rewriteConfig.RewriteHost, _ = annotations.ParseStringASAP(upstreamVhost) rewriteConfig.RewritePath, _ = annotations.ParseStringForHigress(rewritePath) if rewriteConfig.RewritePath == "" && rewriteConfig.RewriteTarget != "" { // When rewrite target is present and not empty, // we will enforce regex match on all rules in this ingress. - rewriteConfig.UseRegex = true + if !rewriteConfig.UseRegex && !rewriteConfig.FullPathRegex { + rewriteConfig.UseRegex = true + } // We should convert nginx regex rule to envoy regex rule. rewriteConfig.RewriteTarget = convertToRE2(rewriteConfig.RewriteTarget) @@ -108,12 +113,13 @@ func convertToRE2(target string) string { func NeedRegexMatch(annotations map[string]string) bool { target, _ := Annotations(annotations).ParseStringASAP(rewriteTarget) - regex, _ := Annotations(annotations).ParseBoolASAP(useRegex) + useRegex, _ := Annotations(annotations).ParseBoolASAP(useRegex) + fullPathRegex, _ := Annotations(annotations).ParseBoolForHigress(fullPathRegex) - return regex || target != "" + return useRegex || target != "" || fullPathRegex } func needRewriteConfig(annotations Annotations) bool { return annotations.HasASAP(rewriteTarget) || annotations.HasASAP(useRegex) || - annotations.HasASAP(upstreamVhost) || annotations.HasHigress(rewritePath) + annotations.HasASAP(upstreamVhost) || annotations.HasHigress(rewritePath) || annotations.HasHigress(fullPathRegex) } diff --git a/pkg/ingress/kube/common/model.go b/pkg/ingress/kube/common/model.go index 7ad4d51a2..e1e1479b6 100644 --- a/pkg/ingress/kube/common/model.go +++ b/pkg/ingress/kube/common/model.go @@ -59,7 +59,11 @@ const ( Prefix PathType = "prefix" - Regex PathType = "regex" + // PrefixRegex :if PathType is PrefixRegex, then the /foo/bar/[A-Z0-9]{3} is actually ^/foo/bar/[A-Z0-9]{3}.* + PrefixRegex PathType = "prefixRegex" + + // FullPathRegex :if PathType is FullPathRegex, then the /foo/bar/[A-Z0-9]{3} is actually ^/foo/bar/[A-Z0-9]{3}$ + FullPathRegex PathType = "fullPathRegex" DefaultStatusUpdateInterval = 10 * time.Second diff --git a/pkg/ingress/kube/common/tool_test.go b/pkg/ingress/kube/common/tool_test.go index 0d7db2bfa..d2f92009e 100644 --- a/pkg/ingress/kube/common/tool_test.go +++ b/pkg/ingress/kube/common/tool_test.go @@ -43,11 +43,11 @@ func TestConstructRouteName(t *testing.T) { { input: &WrapperHTTPRoute{ Host: "*.test.com", - OriginPathType: Regex, + OriginPathType: PrefixRegex, OriginPath: "/test/(.*)/?[0-9]", HTTPRoute: &networking.HTTPRoute{}, }, - expect: "*.test.com-regex-/test/(.*)/?[0-9]", + expect: "*.test.com-prefixRegex-/test/(.*)/?[0-9]", }, { input: &WrapperHTTPRoute{ @@ -390,7 +390,7 @@ func TestSortRoutes(t *testing.T) { AnnotationsConfig: &annotations.Ingress{}, }, Host: "test.com", - OriginPathType: Regex, + OriginPathType: PrefixRegex, OriginPath: "/d(.*)", ClusterId: "cluster1", HTTPRoute: &networking.HTTPRoute{ diff --git a/pkg/ingress/kube/ingress/controller.go b/pkg/ingress/kube/ingress/controller.go index 887b0730d..d84c7fd19 100644 --- a/pkg/ingress/kube/ingress/controller.go +++ b/pkg/ingress/kube/ingress/controller.go @@ -534,8 +534,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra var pathType common.PathType originPath := httpPath.Path - if wrapper.AnnotationsConfig.NeedRegexMatch() { - pathType = common.Regex + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { + if annotationsConfig.IsPrefixRegexMatch() { + pathType = common.PrefixRegex + } else if annotationsConfig.IsFullPathRegexMatch() { + pathType = common.FullPathRegex + } } else { switch *httpPath.PathType { case ingress.PathTypeExact: @@ -741,8 +745,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w var pathType common.PathType originPath := httpPath.Path - if wrapper.AnnotationsConfig.NeedRegexMatch() { - pathType = common.Regex + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { + if annotationsConfig.IsPrefixRegexMatch() { + pathType = common.PrefixRegex + } else if annotationsConfig.IsFullPathRegexMatch() { + pathType = common.FullPathRegex + } } else { switch *httpPath.PathType { case ingress.PathTypeExact: @@ -1166,10 +1174,14 @@ func (c *controller) generateHttpMatches(pathType common.PathType, path string, httpMatch := &networking.HTTPMatchRequest{} switch pathType { - case common.Regex: + case common.PrefixRegex: httpMatch.Uri = &networking.StringMatch{ MatchType: &networking.StringMatch_Regex{Regex: path + ".*"}, } + case common.FullPathRegex: + httpMatch.Uri = &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: path + "$"}, + } case common.Exact: httpMatch.Uri = &networking.StringMatch{ MatchType: &networking.StringMatch_Exact{Exact: path}, diff --git a/pkg/ingress/kube/ingressv1/controller.go b/pkg/ingress/kube/ingressv1/controller.go index 7b5a2c6fe..c170e41ec 100644 --- a/pkg/ingress/kube/ingressv1/controller.go +++ b/pkg/ingress/kube/ingressv1/controller.go @@ -517,8 +517,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra var pathType common.PathType originPath := httpPath.Path - if wrapper.AnnotationsConfig.NeedRegexMatch() { - pathType = common.Regex + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { + if annotationsConfig.IsPrefixRegexMatch() { + pathType = common.PrefixRegex + } else if annotationsConfig.IsFullPathRegexMatch() { + pathType = common.FullPathRegex + } } else { switch *httpPath.PathType { case ingress.PathTypeExact: @@ -610,10 +614,14 @@ func (c *controller) generateHttpMatches(pathType common.PathType, path string, httpMatch := &networking.HTTPMatchRequest{} switch pathType { - case common.Regex: + case common.PrefixRegex: httpMatch.Uri = &networking.StringMatch{ MatchType: &networking.StringMatch_Regex{Regex: path + ".*"}, } + case common.FullPathRegex: + httpMatch.Uri = &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: path + "$"}, + } case common.Exact: httpMatch.Uri = &networking.StringMatch{ MatchType: &networking.StringMatch_Exact{Exact: path}, @@ -747,8 +755,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w var pathType common.PathType originPath := httpPath.Path - if wrapper.AnnotationsConfig.NeedRegexMatch() { - pathType = common.Regex + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { + if annotationsConfig.IsPrefixRegexMatch() { + pathType = common.PrefixRegex + } else if annotationsConfig.IsFullPathRegexMatch() { + pathType = common.FullPathRegex + } } else { switch *httpPath.PathType { case ingress.PathTypeExact: diff --git a/test/ingress/conformance/tests/httproute-full-path-regex.go b/test/ingress/conformance/tests/httproute-full-path-regex.go new file mode 100644 index 000000000..bbdc12cb4 --- /dev/null +++ b/test/ingress/conformance/tests/httproute-full-path-regex.go @@ -0,0 +1,89 @@ +// Copyright (c) 2022 Alibaba Group Holding Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package tests + +import ( + "testing" + + "github.com/alibaba/higress/test/ingress/conformance/utils/http" + "github.com/alibaba/higress/test/ingress/conformance/utils/suite" +) + +func init() { + HigressConformanceTests = append(HigressConformanceTests, HTTPRouteFullPathRegex) +} + +var HTTPRouteFullPathRegex = suite.ConformanceTest{ + ShortName: "HTTPRouteFullPathRegex", + Description: "test for 'higress.io/full-path-regex' annotation", + Manifests: []string{"tests/httproute-full-path-regex.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + testCases := []http.Assertion{ + { + Request: http.AssertionRequest{ + ActualRequest: http.Request{Path: "/foo/1234"}, + }, + + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v1", + TargetNamespace: "higress-conformance-infra", + }, + }, { + Request: http.AssertionRequest{ + ActualRequest: http.Request{Path: "/bar/123"}, + }, + + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v2", + TargetNamespace: "higress-conformance-infra", + }, + }, { + Request: http.AssertionRequest{ + ActualRequest: http.Request{Path: "/bar/1234"}, + }, + + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 404, + }, + }, + + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v2", + TargetNamespace: "higress-conformance-infra", + }, + }, + } + + t.Run("Test for 'higress.io/full-path-regex'", func(t *testing.T) { + for _, testCase := range testCases { + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, suite.GatewayAddress, testCase) + } + + }) + }, +} diff --git a/test/ingress/conformance/tests/httproute-full-path-regex.yaml b/test/ingress/conformance/tests/httproute-full-path-regex.yaml new file mode 100644 index 000000000..047daf494 --- /dev/null +++ b/test/ingress/conformance/tests/httproute-full-path-regex.yaml @@ -0,0 +1,53 @@ +# Copyright (c) 2022 Alibaba Group Holding Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/use-regex: "true" + name: httproute-ingress-use-regex + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: ImplementationSpecific + path: "/foo/[A-Z0-9]{3}" + backend: + service: + name: infra-backend-v1 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + higress.io/full-path-regex: "true" + name: httproute-higress-full-path-regex + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: ImplementationSpecific + path: "/bar/[A-Z0-9]{3}" + backend: + service: + name: infra-backend-v2 + port: + number: 8080 diff --git a/test/ingress/conformance/tests/httproute-rewrite-path.go b/test/ingress/conformance/tests/httproute-rewrite-path.go index bebda3b95..65873c6d2 100644 --- a/test/ingress/conformance/tests/httproute-rewrite-path.go +++ b/test/ingress/conformance/tests/httproute-rewrite-path.go @@ -30,9 +30,8 @@ var HTTPRouteRewritePath = suite.ConformanceTest{ Description: "A single Ingress in the higress-conformance-infra namespace uses the rewrite path.", Manifests: []string{"tests/httproute-rewrite-path.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - - t.Run("Rewrite HTTPRoute Path", func(t *testing.T) { - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, suite.GatewayAddress, http.Assertion{ + testCases := []http.Assertion{ + { Request: http.AssertionRequest{ ActualRequest: http.Request{Path: "/svc/foo"}, ExpectedRequest: &http.ExpectedRequest{ @@ -50,7 +49,14 @@ var HTTPRouteRewritePath = suite.ConformanceTest{ TargetBackend: "infra-backend-v1", TargetNamespace: "higress-conformance-infra", }, - }) + }, + } + + t.Run("Rewrite HTTPRoute Path", func(t *testing.T) { + for _, testCase := range testCases { + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, suite.GatewayAddress, testCase) + } + }) }, } diff --git a/test/ingress/conformance/tests/httproute-rewrite-path.yaml b/test/ingress/conformance/tests/httproute-rewrite-path.yaml index ded88d937..ad60709d8 100644 --- a/test/ingress/conformance/tests/httproute-rewrite-path.yaml +++ b/test/ingress/conformance/tests/httproute-rewrite-path.yaml @@ -31,3 +31,4 @@ spec: name: infra-backend-v1 port: number: 8080 + diff --git a/test/ingress/e2e_test.go b/test/ingress/e2e_test.go index 789bc1ea4..920bb4a3a 100644 --- a/test/ingress/e2e_test.go +++ b/test/ingress/e2e_test.go @@ -74,6 +74,7 @@ func TestHigressConformanceTests(t *testing.T) { tests.HttpRedirectAsHttps, tests.HTTPRouteRequestHeaderControl, tests.HTTPRouteDownstreamEncryption, + tests.HTTPRouteFullPathRegex, } cSuite.Run(t, higressTests)