Fix the problem that ignoreUriCase does not work when the path type is prefix (#260)

Signed-off-by: charlie <qianglin98@qq.com>
2026-04-21 20:17:29 +08:00 · 2023-04-04 21:01:02 +08:00
parent e18557d2ea
commit affa1207d2
11 changed files with 551 additions and 129 deletions
--- a/pkg/ingress/kube/annotations/canary.go
+++ b/pkg/ingress/kube/annotations/canary.go
@@ -134,37 +134,41 @@ func ApplyByHeader(canary, route *networking.HTTPRoute, canaryIngress *Ingress)

 	// Modified match base on by header
 	if canaryConfig.Header != "" {
-		canary.Match[0].Headers = map[string]*networking.StringMatch{
-			canaryConfig.Header: {
-				MatchType: &networking.StringMatch_Exact{
-					Exact: "always",
-				},
-			},
-		}
-		if canaryConfig.HeaderValue != "" {
-			canary.Match[0].Headers = map[string]*networking.StringMatch{
+		for _, match := range canary.Match {
+			match.Headers = map[string]*networking.StringMatch{
 				canaryConfig.Header: {
-					MatchType: &networking.StringMatch_Regex{
-						Regex: "always|" + canaryConfig.HeaderValue,
+					MatchType: &networking.StringMatch_Exact{
+						Exact: "always",
 					},
 				},
 			}
-		} else if canaryConfig.HeaderPattern != "" {
-			canary.Match[0].Headers = map[string]*networking.StringMatch{
-				canaryConfig.Header: {
-					MatchType: &networking.StringMatch_Regex{
-						Regex: canaryConfig.HeaderPattern,
+			if canaryConfig.HeaderValue != "" {
+				match.Headers = map[string]*networking.StringMatch{
+					canaryConfig.Header: {
+						MatchType: &networking.StringMatch_Regex{
+							Regex: "always|" + canaryConfig.HeaderValue,
+						},
 					},
-				},
+				}
+			} else if canaryConfig.HeaderPattern != "" {
+				match.Headers = map[string]*networking.StringMatch{
+					canaryConfig.Header: {
+						MatchType: &networking.StringMatch_Regex{
+							Regex: canaryConfig.HeaderPattern,
+						},
+					},
+				}
 			}
 		}
 	} else if canaryConfig.Cookie != "" {
-		canary.Match[0].Headers = map[string]*networking.StringMatch{
-			"cookie": {
-				MatchType: &networking.StringMatch_Regex{
-					Regex: "^(.\\*?;)?(" + canaryConfig.Cookie + "=always)(;.\\*)?$",
+		for _, match := range canary.Match {
+			match.Headers = map[string]*networking.StringMatch{
+				"cookie": {
+					MatchType: &networking.StringMatch_Regex{
+						Regex: "^(.\\*?;)?(" + canaryConfig.Cookie + "=always)(;.\\*)?$",
+					},
 				},
-			},
+			}
 		}
 	}

--- a/pkg/ingress/kube/ingress/controller.go
+++ b/pkg/ingress/kube/ingress/controller.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"path"
 	"reflect"
-	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -532,40 +531,25 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
 				Host:          rule.Host,
 				ClusterId:     c.options.ClusterId,
 			}
-			httpMatch := &networking.HTTPMatchRequest{}

-			path := httpPath.Path
+			var pathType common.PathType
+			originPath := httpPath.Path
 			if wrapper.AnnotationsConfig.NeedRegexMatch() {
-				wrapperHttpRoute.OriginPathType = common.Regex
-				httpMatch.Uri = &networking.StringMatch{
-					MatchType: &networking.StringMatch_Regex{Regex: httpPath.Path + ".*"},
-				}
+				pathType = common.Regex
 			} else {
 				switch *httpPath.PathType {
 				case ingress.PathTypeExact:
-					wrapperHttpRoute.OriginPathType = common.Exact
-					httpMatch.Uri = &networking.StringMatch{
-						MatchType: &networking.StringMatch_Exact{Exact: httpPath.Path},
-					}
+					pathType = common.Exact
 				case ingress.PathTypePrefix:
-					wrapperHttpRoute.OriginPathType = common.Prefix
-					// borrow from implement of official istio code.
-					if path == "/" {
-						wrapperVS.ConfiguredDefaultBackend = true
-						// Optimize common case of / to not needed regex
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Prefix{Prefix: path},
-						}
-					} else {
-						path = strings.TrimSuffix(path, "/")
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Regex{Regex: regexp.QuoteMeta(path) + common.PrefixMatchRegex},
-						}
+					pathType = common.Prefix
+					if httpPath.Path != "/" {
+						originPath = strings.TrimSuffix(httpPath.Path, "/")
 					}
 				}
 			}
-			wrapperHttpRoute.OriginPath = path
-			wrapperHttpRoute.HTTPRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
+			wrapperHttpRoute.OriginPath = originPath
+			wrapperHttpRoute.OriginPathType = pathType
+			wrapperHttpRoute.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, wrapperVS)
 			wrapperHttpRoute.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, wrapperHttpRoute)

 			ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute)
@@ -748,46 +732,31 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
 		}

 		for _, httpPath := range rule.HTTP.Paths {
-			path := httpPath.Path
-
 			canary := &common.WrapperHTTPRoute{
 				HTTPRoute:     &networking.HTTPRoute{},
 				WrapperConfig: wrapper,
 				Host:          rule.Host,
 				ClusterId:     c.options.ClusterId,
 			}
-			httpMatch := &networking.HTTPMatchRequest{}

+			var pathType common.PathType
+			originPath := httpPath.Path
 			if wrapper.AnnotationsConfig.NeedRegexMatch() {
-				canary.OriginPathType = common.Regex
-				httpMatch.Uri = &networking.StringMatch{
-					MatchType: &networking.StringMatch_Regex{Regex: httpPath.Path + ".*"},
-				}
+				pathType = common.Regex
 			} else {
 				switch *httpPath.PathType {
 				case ingress.PathTypeExact:
-					canary.OriginPathType = common.Exact
-					httpMatch.Uri = &networking.StringMatch{
-						MatchType: &networking.StringMatch_Exact{Exact: httpPath.Path},
-					}
+					pathType = common.Exact
 				case ingress.PathTypePrefix:
-					canary.OriginPathType = common.Prefix
-					// borrow from implement of official istio code.
-					if path == "/" {
-						// Optimize common case of / to not needed regex
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Prefix{Prefix: path},
-						}
-					} else {
-						path = strings.TrimSuffix(path, "/")
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Regex{Regex: regexp.QuoteMeta(path) + common.PrefixMatchRegex},
-						}
+					pathType = common.Prefix
+					if httpPath.Path != "/" {
+						originPath = strings.TrimSuffix(httpPath.Path, "/")
 					}
 				}
 			}
-			canary.OriginPath = path
-			canary.HTTPRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
+			canary.OriginPath = originPath
+			canary.OriginPathType = pathType
+			canary.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, nil)
 			canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)

 			ingressRouteBuilder := convertOptions.IngressRouteCache.New(canary)
@@ -1180,6 +1149,42 @@ func (c *controller) shouldProcessIngressUpdate(ing *ingress.Ingress) (bool, err
 	return preProcessed, nil
 }

+func (c *controller) generateHttpMatches(pathType common.PathType, path string, wrapperVS *common.WrapperVirtualService) []*networking.HTTPMatchRequest {
+	var httpMatches []*networking.HTTPMatchRequest
+
+	httpMatch := &networking.HTTPMatchRequest{}
+	switch pathType {
+	case common.Regex:
+		httpMatch.Uri = &networking.StringMatch{
+			MatchType: &networking.StringMatch_Regex{Regex: path + ".*"},
+		}
+	case common.Exact:
+		httpMatch.Uri = &networking.StringMatch{
+			MatchType: &networking.StringMatch_Exact{Exact: path},
+		}
+	case common.Prefix:
+		if path == "/" {
+			if wrapperVS != nil {
+				wrapperVS.ConfiguredDefaultBackend = true
+			}
+			// Optimize common case of / to not needed regex
+			httpMatch.Uri = &networking.StringMatch{
+				MatchType: &networking.StringMatch_Prefix{Prefix: path},
+			}
+		} else {
+			newPath := strings.TrimSuffix(path, "/")
+			httpMatches = append(httpMatches, c.generateHttpMatches(common.Exact, newPath, wrapperVS)...)
+			httpMatch.Uri = &networking.StringMatch{
+				MatchType: &networking.StringMatch_Prefix{Prefix: newPath + "/"},
+			}
+		}
+	}
+
+	httpMatches = append(httpMatches, httpMatch)
+
+	return httpMatches
+}
+
 // setDefaultMSEIngressOptionalField sets a default value for optional fields when is not defined.
 func setDefaultMSEIngressOptionalField(ing *ingress.Ingress) {
 	if ing == nil {
--- a/pkg/ingress/kube/ingressv1/controller.go
+++ b/pkg/ingress/kube/ingressv1/controller.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"path"
 	"reflect"
-	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -507,6 +506,7 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
 		}

 		wrapperHttpRoutes := make([]*common.WrapperHTTPRoute, 0, len(rule.HTTP.Paths))
+
 		for _, httpPath := range rule.HTTP.Paths {
 			wrapperHttpRoute := &common.WrapperHTTPRoute{
 				HTTPRoute:     &networking.HTTPRoute{},
@@ -514,40 +514,25 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
 				Host:          rule.Host,
 				ClusterId:     c.options.ClusterId,
 			}
-			httpMatch := &networking.HTTPMatchRequest{}

-			path := httpPath.Path
+			var pathType common.PathType
+			originPath := httpPath.Path
 			if wrapper.AnnotationsConfig.NeedRegexMatch() {
-				wrapperHttpRoute.OriginPathType = common.Regex
-				httpMatch.Uri = &networking.StringMatch{
-					MatchType: &networking.StringMatch_Regex{Regex: httpPath.Path + ".*"},
-				}
+				pathType = common.Regex
 			} else {
 				switch *httpPath.PathType {
 				case ingress.PathTypeExact:
-					wrapperHttpRoute.OriginPathType = common.Exact
-					httpMatch.Uri = &networking.StringMatch{
-						MatchType: &networking.StringMatch_Exact{Exact: httpPath.Path},
-					}
+					pathType = common.Exact
 				case ingress.PathTypePrefix:
-					wrapperHttpRoute.OriginPathType = common.Prefix
-					// borrow from implement of official istio code.
-					if path == "/" {
-						wrapperVS.ConfiguredDefaultBackend = true
-						// Optimize common case of / to not needed regex
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Prefix{Prefix: path},
-						}
-					} else {
-						path = strings.TrimSuffix(path, "/")
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Regex{Regex: regexp.QuoteMeta(path) + common.PrefixMatchRegex},
-						}
+					pathType = common.Prefix
+					if httpPath.Path != "/" {
+						originPath = strings.TrimSuffix(httpPath.Path, "/")
 					}
 				}
 			}
-			wrapperHttpRoute.OriginPath = path
-			wrapperHttpRoute.HTTPRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
+			wrapperHttpRoute.OriginPath = originPath
+			wrapperHttpRoute.OriginPathType = pathType
+			wrapperHttpRoute.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, wrapperVS)
 			wrapperHttpRoute.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, wrapperHttpRoute)

 			ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute)
@@ -620,6 +605,42 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
 	return nil
 }

+func (c *controller) generateHttpMatches(pathType common.PathType, path string, wrapperVS *common.WrapperVirtualService) []*networking.HTTPMatchRequest {
+	var httpMatches []*networking.HTTPMatchRequest
+
+	httpMatch := &networking.HTTPMatchRequest{}
+	switch pathType {
+	case common.Regex:
+		httpMatch.Uri = &networking.StringMatch{
+			MatchType: &networking.StringMatch_Regex{Regex: path + ".*"},
+		}
+	case common.Exact:
+		httpMatch.Uri = &networking.StringMatch{
+			MatchType: &networking.StringMatch_Exact{Exact: path},
+		}
+	case common.Prefix:
+		if path == "/" {
+			if wrapperVS != nil {
+				wrapperVS.ConfiguredDefaultBackend = true
+			}
+			// Optimize common case of / to not needed regex
+			httpMatch.Uri = &networking.StringMatch{
+				MatchType: &networking.StringMatch_Prefix{Prefix: path},
+			}
+		} else {
+			newPath := strings.TrimSuffix(path, "/")
+			httpMatches = append(httpMatches, c.generateHttpMatches(common.Exact, newPath, wrapperVS)...)
+			httpMatch.Uri = &networking.StringMatch{
+				MatchType: &networking.StringMatch_Prefix{Prefix: newPath + "/"},
+			}
+		}
+	}
+
+	httpMatches = append(httpMatches, httpMatch)
+
+	return httpMatches
+}
+
 func (c *controller) ApplyDefaultBackend(convertOptions *common.ConvertOptions, wrapper *common.WrapperConfig) error {
 	if wrapper.AnnotationsConfig.IsCanary() {
 		return nil
@@ -717,46 +738,31 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
 		}

 		for _, httpPath := range rule.HTTP.Paths {
-			path := httpPath.Path
-
 			canary := &common.WrapperHTTPRoute{
 				HTTPRoute:     &networking.HTTPRoute{},
 				WrapperConfig: wrapper,
 				Host:          rule.Host,
 				ClusterId:     c.options.ClusterId,
 			}
-			httpMatch := &networking.HTTPMatchRequest{}

+			var pathType common.PathType
+			originPath := httpPath.Path
 			if wrapper.AnnotationsConfig.NeedRegexMatch() {
-				canary.OriginPathType = common.Regex
-				httpMatch.Uri = &networking.StringMatch{
-					MatchType: &networking.StringMatch_Regex{Regex: httpPath.Path + ".*"},
-				}
+				pathType = common.Regex
 			} else {
 				switch *httpPath.PathType {
 				case ingress.PathTypeExact:
-					canary.OriginPathType = common.Exact
-					httpMatch.Uri = &networking.StringMatch{
-						MatchType: &networking.StringMatch_Exact{Exact: httpPath.Path},
-					}
+					pathType = common.Exact
 				case ingress.PathTypePrefix:
-					canary.OriginPathType = common.Prefix
-					// borrow from implement of official istio code.
-					if path == "/" {
-						// Optimize common case of / to not needed regex
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Prefix{Prefix: path},
-						}
-					} else {
-						path = strings.TrimSuffix(path, "/")
-						httpMatch.Uri = &networking.StringMatch{
-							MatchType: &networking.StringMatch_Regex{Regex: regexp.QuoteMeta(path) + common.PrefixMatchRegex},
-						}
+					pathType = common.Prefix
+					if httpPath.Path != "/" {
+						originPath = strings.TrimSuffix(httpPath.Path, "/")
 					}
 				}
 			}
-			canary.OriginPath = path
-			canary.HTTPRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
+			canary.OriginPath = originPath
+			canary.OriginPathType = pathType
+			canary.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, nil)
 			canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)

 			ingressRouteBuilder := convertOptions.IngressRouteCache.New(canary)
@@ -819,6 +825,7 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
 			} else {
 				convertOptions.IngressRouteCache.Update(targetRoute)
 			}
+
 		}
 	}
 	return nil
--- a/pkg/ingress/kube/ingressv1/controller_test.go
+++ b/pkg/ingress/kube/ingressv1/controller_test.go
@@ -17,6 +17,8 @@ package ingressv1
 import (
 	"testing"

+	"github.com/google/go-cmp/cmp"
+	networking "istio.io/api/networking/v1alpha3"
 	v1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

@@ -76,3 +78,38 @@ func TestShouldProcessIngressUpdate(t *testing.T) {
 		t.Fatal("should be true")
 	}
 }
+
+func TestGenerateHttpMatches(t *testing.T) {
+	c := controller{}
+
+	tt := []struct {
+		pathType common.PathType
+		path     string
+		expect   []*networking.HTTPMatchRequest
+	}{
+		{
+			pathType: common.Prefix,
+			path:     "/foo",
+			expect: []*networking.HTTPMatchRequest{
+				{
+					Uri: &networking.StringMatch{
+						MatchType: &networking.StringMatch_Exact{Exact: "/foo"},
+					},
+				}, {
+					Uri: &networking.StringMatch{
+						MatchType: &networking.StringMatch_Prefix{Prefix: "/foo/"},
+					},
+				},
+			},
+		},
+	}
+
+	for _, testcase := range tt {
+		httpMatches := c.generateHttpMatches(testcase.pathType, testcase.path, nil)
+		for idx, httpMatch := range httpMatches {
+			if diff := cmp.Diff(httpMatch, testcase.expect[idx]); diff != "" {
+				t.Errorf("generateHttpMatches() mismatch (-want +got):\n%s", diff)
+			}
+		}
+	}
+}