diff --git a/pkg/ingress/kube/annotations/annotations.go b/pkg/ingress/kube/annotations/annotations.go index d6bd42e09b..76d97a07a3 100644 --- a/pkg/ingress/kube/annotations/annotations.go +++ b/pkg/ingress/kube/annotations/annotations.go @@ -15,6 +15,8 @@ package annotations import ( + "strings" + networking "istio.io/api/networking/v1alpha3" "istio.io/istio/pilot/pkg/util/sets" listersv1 "k8s.io/client-go/listers/core/v1" @@ -73,8 +75,17 @@ type Ingress struct { Http2Rpc *Http2RpcConfig } -func (i *Ingress) NeedRegexMatch() bool { - return i.Rewrite != nil && (i.IsPrefixRegexMatch() || i.IsFullPathRegexMatch()) +func (i *Ingress) NeedRegexMatch(path string) bool { + if i.Rewrite == nil { + return false + } + if strings.ContainsAny(path, `\.+*?()|[]{}^$`) { + return true + } + if strings.ContainsAny(i.Rewrite.RewriteTarget, `$\`) { + return true + } + return i.IsPrefixRegexMatch() || i.IsFullPathRegexMatch() } func (i *Ingress) IsPrefixRegexMatch() bool { diff --git a/pkg/ingress/kube/annotations/annotations_test.go b/pkg/ingress/kube/annotations/annotations_test.go index fadb6dc355..993d4c441d 100644 --- a/pkg/ingress/kube/annotations/annotations_test.go +++ b/pkg/ingress/kube/annotations/annotations_test.go @@ -18,8 +18,9 @@ import "testing" func TestNeedRegexMatch(t *testing.T) { testCases := []struct { - input *Ingress - expect bool + input *Ingress + inputPath string + expect bool }{ { input: &Ingress{}, @@ -47,12 +48,41 @@ func TestNeedRegexMatch(t *testing.T) { }, expect: false, }, + { + input: &Ingress{ + Rewrite: &RewriteConfig{ + UseRegex: false, + RewriteTarget: "/$1", + }, + }, + expect: true, + }, + { + input: &Ingress{ + Rewrite: &RewriteConfig{ + UseRegex: false, + RewriteTarget: "/", + }, + }, + inputPath: "/.*", + expect: true, + }, + { + input: &Ingress{ + Rewrite: &RewriteConfig{ + UseRegex: false, + RewriteTarget: "/", + }, + }, + inputPath: "/", + expect: false, + }, } for _, testCase := range testCases { t.Run("", func(t *testing.T) { - if testCase.input.NeedRegexMatch() != testCase.expect { - t.Fatalf("Should be %t, but actual is %t", testCase.expect, testCase.input.NeedRegexMatch()) + if testCase.input.NeedRegexMatch(testCase.inputPath) != testCase.expect { + t.Fatalf("Should be %t, but actual is %t", testCase.expect, !testCase.expect) } }) } diff --git a/pkg/ingress/kube/ingress/controller.go b/pkg/ingress/kube/ingress/controller.go index 083e36de02..0dca636a4f 100644 --- a/pkg/ingress/kube/ingress/controller.go +++ b/pkg/ingress/kube/ingress/controller.go @@ -535,11 +535,11 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra var pathType common.PathType originPath := httpPath.Path - if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { - if annotationsConfig.IsPrefixRegexMatch() { - pathType = common.PrefixRegex - } else if annotationsConfig.IsFullPathRegexMatch() { + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch(originPath) { + if annotationsConfig.IsFullPathRegexMatch() { pathType = common.FullPathRegex + } else { + pathType = common.PrefixRegex } } else { switch *httpPath.PathType { @@ -746,11 +746,11 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w var pathType common.PathType originPath := httpPath.Path - if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { - if annotationsConfig.IsPrefixRegexMatch() { - pathType = common.PrefixRegex - } else if annotationsConfig.IsFullPathRegexMatch() { + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch(originPath) { + if annotationsConfig.IsFullPathRegexMatch() { pathType = common.FullPathRegex + } else { + pathType = common.PrefixRegex } } else { switch *httpPath.PathType { diff --git a/pkg/ingress/kube/ingressv1/controller.go b/pkg/ingress/kube/ingressv1/controller.go index 5c86a2cd76..5e7789c69c 100644 --- a/pkg/ingress/kube/ingressv1/controller.go +++ b/pkg/ingress/kube/ingressv1/controller.go @@ -517,11 +517,11 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra var pathType common.PathType originPath := httpPath.Path - if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { - if annotationsConfig.IsPrefixRegexMatch() { - pathType = common.PrefixRegex - } else if annotationsConfig.IsFullPathRegexMatch() { + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch(originPath) { + if annotationsConfig.IsFullPathRegexMatch() { pathType = common.FullPathRegex + } else { + pathType = common.PrefixRegex } } else { switch *httpPath.PathType { @@ -750,11 +750,11 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w var pathType common.PathType originPath := httpPath.Path - if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch() { - if annotationsConfig.IsPrefixRegexMatch() { - pathType = common.PrefixRegex - } else if annotationsConfig.IsFullPathRegexMatch() { + if annotationsConfig := wrapper.AnnotationsConfig; annotationsConfig.NeedRegexMatch(originPath) { + if annotationsConfig.IsFullPathRegexMatch() { pathType = common.FullPathRegex + } else { + pathType = common.PrefixRegex } } else { switch *httpPath.PathType { diff --git a/test/e2e/conformance/tests/httproute-rewrite-path.yaml b/test/e2e/conformance/tests/httproute-rewrite-path.yaml index 5b9cc3c7ed..ad60709d84 100644 --- a/test/e2e/conformance/tests/httproute-rewrite-path.yaml +++ b/test/e2e/conformance/tests/httproute-rewrite-path.yaml @@ -17,7 +17,6 @@ kind: Ingress metadata: annotations: nginx.ingress.kubernetes.io/rewrite-target: "/$1" - nginx.ingress.kubernetes.io/use-regex: "true" name: httproute-rewrite-path namespace: higress-conformance-infra spec: