diff --git a/pkg/ingress/config/ingress_config.go b/pkg/ingress/config/ingress_config.go index 4d254cfc6..480e40759 100644 --- a/pkg/ingress/config/ingress_config.go +++ b/pkg/ingress/config/ingress_config.go @@ -330,10 +330,10 @@ func (m *IngressConfig) convertGateways(configs []common.WrapperConfig) []config func (m *IngressConfig) convertVirtualService(configs []common.WrapperConfig) []config.Config { convertOptions := common.ConvertOptions{ - HostAndPath2Ingress: map[string]*config.Config{}, - IngressRouteCache: common.NewIngressRouteCache(), - VirtualServices: map[string]*common.WrapperVirtualService{}, - HTTPRoutes: map[string][]*common.WrapperHTTPRoute{}, + IngressRouteCache: common.NewIngressRouteCache(), + VirtualServices: map[string]*common.WrapperVirtualService{}, + HTTPRoutes: map[string][]*common.WrapperHTTPRoute{}, + Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, } // convert http route diff --git a/pkg/ingress/kube/annotations/match.go b/pkg/ingress/kube/annotations/match.go index cda1060bf..ce58ab71a 100644 --- a/pkg/ingress/kube/annotations/match.go +++ b/pkg/ingress/kube/annotations/match.go @@ -26,9 +26,9 @@ const ( exact = "exact" regex = "regex" prefix = "prefix" - matchMethod = "match-method" - matchQuery = "match-query" - matchHeader = "match-header" + MatchMethod = "match-method" + MatchQuery = "match-query" + MatchHeader = "match-header" sep = " " ) @@ -52,11 +52,11 @@ func (m match) Parse(annotations Annotations, config *Ingress, _ *GlobalContext) IngressLog.Errorf("parse methods error %v within ingress %s/%s", err, config.Namespace, config.Name) } - if config.Match.Headers, err = m.matchByHeaderOrQueryParma(annotations, matchHeader, config.Match.Headers); err != nil { + if config.Match.Headers, err = m.matchByHeaderOrQueryParma(annotations, MatchHeader, config.Match.Headers); err != nil { IngressLog.Errorf("parse headers error %v within ingress %s/%s", err, config.Namespace, config.Name) } - if config.Match.QueryParams, err = m.matchByHeaderOrQueryParma(annotations, matchQuery, config.Match.QueryParams); err != nil { + if config.Match.QueryParams, err = m.matchByHeaderOrQueryParma(annotations, MatchQuery, config.Match.QueryParams); err != nil { IngressLog.Errorf("parse query params error %v within ingress %s/%s", err, config.Namespace, config.Name) } @@ -96,12 +96,12 @@ func (m match) ApplyRoute(route *networking.HTTPRoute, ingressCfg *Ingress) { } func (m match) matchByMethod(annotations Annotations, ingress *Ingress) error { - if !annotations.HasHigress(matchMethod) { + if !annotations.HasHigress(MatchMethod) { return nil } config := ingress.Match - str, err := annotations.ParseStringForHigress(matchMethod) + str, err := annotations.ParseStringForHigress(MatchMethod) if err != nil { return err } @@ -119,7 +119,7 @@ func (m match) matchByMethod(annotations Annotations, ingress *Ingress) error { return nil } -// matchByHeader to parse annotations to find matchHeader config +// matchByHeader to parse annotations to find MatchHeader config func (m match) matchByHeaderOrQueryParma(annotations Annotations, key string, mmap map[string]map[string]string) (map[string]map[string]string, error) { for k, v := range annotations { if idx := strings.Index(k, key); idx != -1 { diff --git a/pkg/ingress/kube/annotations/match_test.go b/pkg/ingress/kube/annotations/match_test.go index e28c97e6d..8d3d7a992 100644 --- a/pkg/ingress/kube/annotations/match_test.go +++ b/pkg/ingress/kube/annotations/match_test.go @@ -33,7 +33,7 @@ func TestMatch_ParseMethods(t *testing.T) { }, { input: Annotations{ - buildHigressAnnotationKey(matchMethod): "PUT POST PATCH", + buildHigressAnnotationKey(MatchMethod): "PUT POST PATCH", }, expect: &MatchConfig{ Methods: []string{"PUT", "POST", "PATCH"}, @@ -41,7 +41,7 @@ func TestMatch_ParseMethods(t *testing.T) { }, { input: Annotations{ - buildHigressAnnotationKey(matchMethod): "PUT PUT", + buildHigressAnnotationKey(MatchMethod): "PUT PUT", }, expect: &MatchConfig{ Methods: []string{"PUT"}, @@ -49,7 +49,7 @@ func TestMatch_ParseMethods(t *testing.T) { }, { input: Annotations{ - buildHigressAnnotationKey(matchMethod): "put post patch", + buildHigressAnnotationKey(MatchMethod): "put post patch", }, expect: &MatchConfig{ Methods: []string{"PUT", "POST", "PATCH"}, @@ -57,7 +57,7 @@ func TestMatch_ParseMethods(t *testing.T) { }, { input: Annotations{ - buildHigressAnnotationKey(matchMethod): "geet", + buildHigressAnnotationKey(MatchMethod): "geet", }, expect: &MatchConfig{}, }, @@ -116,7 +116,7 @@ func TestMatch_ParseHeaders(t *testing.T) { for _, tt := range testCases { t.Run("", func(t *testing.T) { - key := buildHigressAnnotationKey(tt.typ + "-" + matchHeader + "-" + tt.key) + key := buildHigressAnnotationKey(tt.typ + "-" + MatchHeader + "-" + tt.key) input := Annotations{key: tt.value} config := &Ingress{} _ = parser.Parse(input, config, nil) @@ -169,7 +169,7 @@ func TestMatch_ParseQueryParams(t *testing.T) { for _, tt := range testCases { t.Run("", func(t *testing.T) { - key := buildHigressAnnotationKey(tt.typ + "-" + matchQuery + "-" + tt.key) + key := buildHigressAnnotationKey(tt.typ + "-" + MatchQuery + "-" + tt.key) input := Annotations{key: tt.value} config := &Ingress{} _ = parser.Parse(input, config, nil) diff --git a/pkg/ingress/kube/common/controller.go b/pkg/ingress/kube/common/controller.go index a4421f772..2c8159c8d 100644 --- a/pkg/ingress/kube/common/controller.go +++ b/pkg/ingress/kube/common/controller.go @@ -38,6 +38,11 @@ type WrapperConfig struct { AnnotationsConfig *annotations.Ingress } +type WrapperConfigWithRuleKey struct { + Config *config.Config + RuleKey string +} + type WrapperGateway struct { Gateway *networking.Gateway WrapperConfig *WrapperConfig @@ -70,6 +75,8 @@ type WrapperHTTPRoute struct { OriginPathType PathType WeightTotal int32 IsDefaultBackend bool + RuleKey string + RuleHash uint32 } func (w *WrapperHTTPRoute) Meta() string { diff --git a/pkg/ingress/kube/common/model.go b/pkg/ingress/kube/common/model.go index dd5e60188..b064d7c49 100644 --- a/pkg/ingress/kube/common/model.go +++ b/pkg/ingress/kube/common/model.go @@ -149,7 +149,8 @@ type ConvertOptions struct { IngressDomainCache *IngressDomainCache - HostAndPath2Ingress map[string]*config.Config + // the host, path, headers, params of rule => ingress + Route2Ingress map[uint32]*WrapperConfigWithRuleKey // Record valid/invalid routes from ingress IngressRouteCache *IngressRouteCache diff --git a/pkg/ingress/kube/common/tool.go b/pkg/ingress/kube/common/tool.go index 25e0c0048..8c3257970 100644 --- a/pkg/ingress/kube/common/tool.go +++ b/pkg/ingress/kube/common/tool.go @@ -175,6 +175,9 @@ func SortHTTPRoutes(routes []*WrapperHTTPRoute) { return false } + // default backend,user specified root path => path type => path length => + // methods => header => query param + // refer https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec sort.SliceStable(routes, func(i, j int) bool { // Move default backend to end if isDefaultBackend(routes[i]) { @@ -193,7 +196,27 @@ func SortHTTPRoutes(routes []*WrapperHTTPRoute) { } if routes[i].OriginPathType == routes[j].OriginPathType { - return len(routes[i].OriginPath) > len(routes[j].OriginPath) + if in, jn := len(routes[i].OriginPath), len(routes[j].OriginPath); in != jn { + return in > jn + } + + match1, match2 := routes[i].HTTPRoute.Match[0], routes[j].HTTPRoute.Match[0] + // methods + if in, jn := len(match1.Method.GetRegex()), len(match2.Method.GetRegex()); in != jn { + if in != 0 && jn != 0 { + return in < jn + } + return in != 0 + } + // headers + if in, jn := len(match1.Headers), len(match2.Headers); in != jn { + return in > jn + } + // query params + if in, jn := len(match1.QueryParams), len(match2.QueryParams); in != jn { + return in > jn + } + return false } if routes[i].OriginPathType == Exact { diff --git a/pkg/ingress/kube/common/tool_test.go b/pkg/ingress/kube/common/tool_test.go index 8a4c35bcb..0d7db2bfa 100644 --- a/pkg/ingress/kube/common/tool_test.go +++ b/pkg/ingress/kube/common/tool_test.go @@ -416,3 +416,143 @@ func TestSortRoutes(t *testing.T) { t.Fatal("should be test-3") } } + +// TestSortHTTPRoutesWithMoreRules include headers, query params, methods +func TestSortHTTPRoutesWithMoreRules(t *testing.T) { + input := []struct { + order string + pathType PathType + path string + method *networking.StringMatch + header map[string]*networking.StringMatch + queryParam map[string]*networking.StringMatch + }{ + { + order: "1", + pathType: Exact, + path: "/bar", + }, + { + order: "2", + pathType: Prefix, + path: "/bar", + }, + { + order: "3", + pathType: Prefix, + path: "/bar", + method: &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: "GET|PUT"}, + }, + }, + { + order: "4", + pathType: Prefix, + path: "/bar", + method: &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: "GET"}, + }, + }, + { + order: "5", + pathType: Prefix, + path: "/bar", + header: map[string]*networking.StringMatch{ + "foo": { + MatchType: &networking.StringMatch_Exact{Exact: "bar"}, + }, + }, + }, + { + order: "6", + pathType: Prefix, + path: "/bar", + header: map[string]*networking.StringMatch{ + "foo": { + MatchType: &networking.StringMatch_Exact{Exact: "bar"}, + }, + "bar": { + MatchType: &networking.StringMatch_Exact{Exact: "foo"}, + }, + }, + }, + { + order: "7", + pathType: Prefix, + path: "/bar", + queryParam: map[string]*networking.StringMatch{ + "foo": { + MatchType: &networking.StringMatch_Exact{Exact: "bar"}, + }, + }, + }, + { + order: "8", + pathType: Prefix, + path: "/bar", + queryParam: map[string]*networking.StringMatch{ + "foo": { + MatchType: &networking.StringMatch_Exact{Exact: "bar"}, + }, + "bar": { + MatchType: &networking.StringMatch_Exact{Exact: "foo"}, + }, + }, + }, + { + order: "9", + pathType: Prefix, + path: "/bar", + method: &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: "GET"}, + }, + queryParam: map[string]*networking.StringMatch{ + "foo": { + MatchType: &networking.StringMatch_Exact{Exact: "bar"}, + }, + }, + }, + { + order: "10", + pathType: Prefix, + path: "/bar", + method: &networking.StringMatch{ + MatchType: &networking.StringMatch_Regex{Regex: "GET"}, + }, + queryParam: map[string]*networking.StringMatch{ + "bar": { + MatchType: &networking.StringMatch_Exact{Exact: "foo"}, + }, + }, + }, + } + + origin := []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10"} + expect := []string{"1", "9", "10", "4", "3", "6", "5", "8", "7", "2"} + + var list []*WrapperHTTPRoute + for idx, val := range input { + list = append(list, &WrapperHTTPRoute{ + OriginPath: val.path, + OriginPathType: val.pathType, + HTTPRoute: &networking.HTTPRoute{ + Name: origin[idx], + Match: []*networking.HTTPMatchRequest{ + { + Method: val.method, + Headers: val.header, + QueryParams: val.queryParam, + }, + }, + }, + }) + } + + SortHTTPRoutes(list) + + for idx, val := range list { + if val.HTTPRoute.Name != expect[idx] { + t.Fatalf("should be %s, but got %s", expect[idx], val.HTTPRoute.Name) + } + } +} diff --git a/pkg/ingress/kube/ingress/controller.go b/pkg/ingress/kube/ingress/controller.go index 57a4d01a9..981bdf6ff 100644 --- a/pkg/ingress/kube/ingress/controller.go +++ b/pkg/ingress/kube/ingress/controller.go @@ -17,9 +17,11 @@ package ingress import ( "errors" "fmt" + "hash/fnv" "path" "reflect" "regexp" + "sort" "strings" "sync" "time" @@ -494,9 +496,13 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra // When the host, pathType, path of two rule are same, we think there is a conflict event. definedRules := sets.NewSet() - // But in across ingresses case, we will restrict this limit. - // When the host, path of two rule in different ingress are same, we think there is a conflict event. - var tempHostAndPath []string + var ( + // But in across ingresses case, we will restrict this limit. + // When the {host, path, headers, method, params} of two rule in different ingress are same, we think there is a conflict event. + tempRuleHash []uint32 + tempRuleKey []string + ) + for _, rule := range ingressV1.Rules { if rule.HTTP == nil || len(rule.HTTP.Paths) == 0 { IngressLog.Warnf("invalid ingress rule %s:%s for host %q in cluster %s, no paths defined", cfg.Namespace, cfg.Name, rule.Host, c.options.ClusterId) @@ -566,13 +572,19 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute) - // host and path overlay check across different ingresses. - hostAndPath := wrapperHttpRoute.BasePathFormat() - if preIngress, exist := convertOptions.HostAndPath2Ingress[hostAndPath]; exist { - ingressRouteBuilder.PreIngress = preIngress + hostAndPath := wrapperHttpRoute.PathFormat() + hash, key, err := createRuleKey(cfg.Annotations, hostAndPath) + if err != nil { + return err + } + wrapperHttpRoute.RuleKey = key + wrapperHttpRoute.RuleHash = hash + if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist { + ingressRouteBuilder.PreIngress = WrapPreIngress.Config ingressRouteBuilder.Event = common.DuplicatedRoute } - tempHostAndPath = append(tempHostAndPath, hostAndPath) + tempRuleHash = append(tempRuleHash, hash) + tempRuleKey = append(tempRuleKey, key) // Two duplicated rules in the same ingress. if ingressRouteBuilder.Event == common.Normal { @@ -607,10 +619,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra convertOptions.IngressRouteCache.Add(ingressRouteBuilder) } - for _, item := range tempHostAndPath { - // We only record the first - if _, exist := convertOptions.HostAndPath2Ingress[item]; !exist { - convertOptions.HostAndPath2Ingress[item] = cfg + for idx, item := range tempRuleHash { + if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 { + convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{ + Config: cfg, + RuleKey: tempRuleKey[idx], + } } } @@ -794,6 +808,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w convertOptions.IngressRouteCache.Add(ingressRouteBuilder) continue } + hash, key, err := createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat()) + if err != nil { + return err + } + canary.RuleKey = key + canary.RuleHash = hash canaryConfig := wrapper.AnnotationsConfig.Canary if byWeight { @@ -1004,8 +1024,8 @@ func (c *controller) createServiceKey(service *ingress.IngressBackend, namespace } func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool { - return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.OriginPath == route.OriginPath && - canary.OriginPathType == route.OriginPathType + return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() && + canary.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey } func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, @@ -1221,3 +1241,50 @@ func setDefaultMSEIngressOptionalField(ing *ingress.Ingress) { } } } + +// createRuleKey according to the pathType, path, methods, headers, params of rules +func createRuleKey(annots map[string]string, hostAndPath string) (uint32, string, error) { + var ( + headers []string + params []string + sb strings.Builder + ) + + // path + sb.WriteString(hostAndPath) + + // methods + if str, ok := annots[annotations.HigressAnnotationsPrefix+"/"+annotations.MatchMethod]; ok { + sb.WriteString(str) + } + // headers && params + for k, _ := range annots { + if idx := strings.Index(k, annotations.MatchHeader); idx != -1 { + headers = append(headers, k) + } + if idx := strings.Index(k, annotations.MatchQuery); idx != -1 { + params = append(params, k) + } + } + sort.SliceStable(headers, func(i, j int) bool { + return headers[i] < headers[j] + }) + sort.SliceStable(params, func(i, j int) bool { + return params[i] < params[j] + }) + for idx := range headers { + sb.WriteString(headers[idx]) + sb.WriteString(annots[headers[idx]]) + } + for idx := range params { + sb.WriteString(params[idx]) + sb.WriteString(annots[params[idx]]) + } + + str, hash := sb.String(), fnv.New32() + if _, err := hash.Write([]byte(str)); err != nil { + return 0, "", err + } + + return hash.Sum32(), str, nil +} diff --git a/pkg/ingress/kube/ingress/controller_test.go b/pkg/ingress/kube/ingress/controller_test.go index 8a22bb0d3..3528b1867 100644 --- a/pkg/ingress/kube/ingress/controller_test.go +++ b/pkg/ingress/kube/ingress/controller_test.go @@ -103,10 +103,10 @@ func testApplyCanaryIngress(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - HostAndPath2Ingress: make(map[string]*config.Config), - VirtualServices: make(map[string]*common.WrapperVirtualService), - Gateways: make(map[string]*common.WrapperGateway), - IngressRouteCache: &common.IngressRouteCache{}, + Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + VirtualServices: make(map[string]*common.WrapperVirtualService), + Gateways: make(map[string]*common.WrapperGateway), + IngressRouteCache: &common.IngressRouteCache{}, HTTPRoutes: map[string][]*common.WrapperHTTPRoute{ "test1": make([]*common.WrapperHTTPRoute, 0), }, @@ -193,11 +193,11 @@ func testApplyDefaultBackend(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - HostAndPath2Ingress: make(map[string]*config.Config), - VirtualServices: make(map[string]*common.WrapperVirtualService), - Gateways: make(map[string]*common.WrapperGateway), - IngressRouteCache: &common.IngressRouteCache{}, - HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), + Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + VirtualServices: make(map[string]*common.WrapperVirtualService), + Gateways: make(map[string]*common.WrapperGateway), + IngressRouteCache: &common.IngressRouteCache{}, + HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), }, wrapperConfig: &common.WrapperConfig{Config: &config.Config{ Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{ @@ -385,11 +385,11 @@ func testConvertHTTPRoute(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - HostAndPath2Ingress: make(map[string]*config.Config), - VirtualServices: make(map[string]*common.WrapperVirtualService), - Gateways: make(map[string]*common.WrapperGateway), - IngressRouteCache: &common.IngressRouteCache{}, - HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), + Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + VirtualServices: make(map[string]*common.WrapperVirtualService), + Gateways: make(map[string]*common.WrapperGateway), + IngressRouteCache: &common.IngressRouteCache{}, + HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), }, wrapperConfig: &common.WrapperConfig{Config: &config.Config{ Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{ @@ -474,7 +474,7 @@ func testConvertTrafficPolicy(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - HostAndPath2Ingress: make(map[string]*config.Config), + Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, VirtualServices: make(map[string]*common.WrapperVirtualService), Gateways: make(map[string]*common.WrapperGateway), IngressRouteCache: &common.IngressRouteCache{}, diff --git a/pkg/ingress/kube/ingressv1/controller.go b/pkg/ingress/kube/ingressv1/controller.go index ebacbdd71..25fcc0174 100644 --- a/pkg/ingress/kube/ingressv1/controller.go +++ b/pkg/ingress/kube/ingressv1/controller.go @@ -17,9 +17,11 @@ package ingressv1 import ( "errors" "fmt" + "hash/fnv" "path" "reflect" "regexp" + "sort" "strings" "sync" "time" @@ -476,9 +478,13 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra // When the host, pathType, path of two rule are same, we think there is a conflict event. definedRules := sets.NewSet() - // But in across ingresses case, we will restrict this limit. - // When the host, path of two rule in different ingress are same, we think there is a conflict event. - var tempHostAndPath []string + var ( + // But in across ingresses case, we will restrict this limit. + // When the {host, path, headers, method, params} of two rule in different ingress are same, we think there is a conflict event. + tempRuleHash []uint32 + tempRuleKey []string + ) + for _, rule := range ingressV1.Rules { if rule.HTTP == nil || len(rule.HTTP.Paths) == 0 { IngressLog.Warnf("invalid ingress rule %s:%s for host %q in cluster %s, no paths defined", cfg.Namespace, cfg.Name, rule.Host, c.options.ClusterId) @@ -548,13 +554,19 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute) - // host and path overlay check across different ingresses. - hostAndPath := wrapperHttpRoute.BasePathFormat() - if preIngress, exist := convertOptions.HostAndPath2Ingress[hostAndPath]; exist { - ingressRouteBuilder.PreIngress = preIngress + hostAndPath := wrapperHttpRoute.PathFormat() + hash, key, err := createRuleKey(cfg.Annotations, hostAndPath) + if err != nil { + return err + } + wrapperHttpRoute.RuleKey = key + wrapperHttpRoute.RuleHash = hash + if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist { + ingressRouteBuilder.PreIngress = WrapPreIngress.Config ingressRouteBuilder.Event = common.DuplicatedRoute } - tempHostAndPath = append(tempHostAndPath, hostAndPath) + tempRuleHash = append(tempRuleHash, hash) + tempRuleKey = append(tempRuleKey, key) // Two duplicated rules in the same ingress. if ingressRouteBuilder.Event == common.Normal { @@ -589,10 +601,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra convertOptions.IngressRouteCache.Add(ingressRouteBuilder) } - for _, item := range tempHostAndPath { - // We only record the first - if _, exist := convertOptions.HostAndPath2Ingress[item]; !exist { - convertOptions.HostAndPath2Ingress[item] = cfg + for idx, item := range tempRuleHash { + if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 { + convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{ + Config: cfg, + RuleKey: tempRuleKey[idx], + } } } @@ -763,6 +777,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w convertOptions.IngressRouteCache.Add(ingressRouteBuilder) continue } + hash, key, err := createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat()) + if err != nil { + return err + } + canary.RuleKey = key + canary.RuleHash = hash canaryConfig := wrapper.AnnotationsConfig.Canary if byWeight { @@ -960,8 +980,7 @@ func (c *controller) createServiceKey(service *ingress.IngressServiceBackend, na } func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool { - return !strings.HasSuffix(route.HTTPRoute.Name, "-canary") && canary.OriginPath == route.OriginPath && - canary.OriginPathType == route.OriginPathType + return !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey } func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, @@ -1171,3 +1190,50 @@ func setDefaultMSEIngressOptionalField(ing *ingress.Ingress) { } } } + +// createRuleKey according to the pathType, path, methods, headers, params of rules +func createRuleKey(annots map[string]string, hostAndPath string) (uint32, string, error) { + var ( + headers []string + params []string + sb strings.Builder + ) + + // path + sb.WriteString(hostAndPath) + + // methods + if str, ok := annots[annotations.HigressAnnotationsPrefix+"/"+annotations.MatchMethod]; ok { + sb.WriteString(str) + } + // headers && params + for k, _ := range annots { + if idx := strings.Index(k, annotations.MatchHeader); idx != -1 { + headers = append(headers, k) + } + if idx := strings.Index(k, annotations.MatchQuery); idx != -1 { + params = append(params, k) + } + } + sort.SliceStable(headers, func(i, j int) bool { + return headers[i] < headers[j] + }) + sort.SliceStable(params, func(i, j int) bool { + return params[i] < params[j] + }) + for idx := range headers { + sb.WriteString(headers[idx]) + sb.WriteString(annots[headers[idx]]) + } + for idx := range params { + sb.WriteString(params[idx]) + sb.WriteString(annots[params[idx]]) + } + + str, hash := sb.String(), fnv.New32() + if _, err := hash.Write([]byte(str)); err != nil { + return 0, "", err + } + + return hash.Sum32(), str, nil +} diff --git a/test/ingress/conformance/tests/httproute-canary-header-with-customized-header.go b/test/ingress/conformance/tests/httproute-canary-header-with-customized-header.go new file mode 100644 index 000000000..674ef183a --- /dev/null +++ b/test/ingress/conformance/tests/httproute-canary-header-with-customized-header.go @@ -0,0 +1,133 @@ +// 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, HTTPRouteCanaryHeaderWithCustomizedHeader) +} + +var HTTPRouteCanaryHeaderWithCustomizedHeader = suite.ConformanceTest{ + ShortName: "HTTPRouteCanaryHeaderWithCustomizedHeader", + Description: "The Ingress in the higress-conformance-infra namespace uses the canary header traffic split when same host and path but different header", + Manifests: []string{"tests/httproute-canary-header-with-customized-header.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + testcases := []http.Assertion{ + { + // test canary ingress with different customized header + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/echo", + Host: "canary.higress.io", + Headers: map[string]string{ + "traffic-split-higress": "true", + }, + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 404, + }, + }, + }, { + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/echo", + Host: "canary.higress.io", + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 404, + }, + }, + }, + { + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v2", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/echo", + Host: "canary.higress.io", + Headers: map[string]string{ + "abc": "123", + }, + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + }, + // test canary ingress with same customized header + { + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v1", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/echo", + Host: "same.canary.higress.io", + Headers: map[string]string{ + "with-same-customized-header": "true", + "user": "higress", + }, + }, + }, + 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: "/echo", + Host: "same.canary.higress.io", + Headers: map[string]string{ + "user": "higress", + }, + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + }, + } + + t.Run("Canary HTTPRoute Traffic Split With customized header", 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-canary-header-with-customized-header.yaml b/test/ingress/conformance/tests/httproute-canary-header-with-customized-header.yaml new file mode 100644 index 000000000..66cb24c28 --- /dev/null +++ b/test/ingress/conformance/tests/httproute-canary-header-with-customized-header.yaml @@ -0,0 +1,107 @@ +# 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. + + +# ingress-canary-header-with-customized-header-01 and -02 is to test canary ingress can't match ingress with different customized header + +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/canary: "true" + nginx.ingress.kubernetes.io/canary-by-header: "traffic-split-higress" + nginx.ingress.kubernetes.io/canary-by-header-value: "true" + name: ingress-canary-header-with-customized-header-01 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: canary.higress.io + http: + paths: + - path: /echo + pathType: Exact + backend: + service: + name: infra-backend-v1 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + higress.io/exact-match-header-abc: "123" + name: ingress-canary-header-with-customized-header-02 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: canary.higress.io + http: + paths: + - path: /echo + pathType: Exact + backend: + service: + name: infra-backend-v2 + port: + number: 8080 +--- +# ingress-canary-header-with-customized-header-03 and -04 is to test canary ingress can match ingress with same customized header + +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/canary: "true" + nginx.ingress.kubernetes.io/canary-by-header: "with-same-customized-header" + nginx.ingress.kubernetes.io/canary-by-header-value: "true" + higress.io/exact-match-header-user: "higress" + name: ingress-canary-header-with-customized-header-03 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: same.canary.higress.io + http: + paths: + - path: /echo + pathType: Exact + backend: + service: + name: infra-backend-v1 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + higress.io/exact-match-header-user: "higress" + name: ingress-canary-header-with-customized-header-04 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: same.canary.higress.io + http: + paths: + - path: /echo + pathType: Exact + backend: + service: + name: infra-backend-v2 + port: + number: 8080 diff --git a/test/ingress/conformance/tests/httproute-same-host-and-path.go b/test/ingress/conformance/tests/httproute-same-host-and-path.go new file mode 100644 index 000000000..1eb7e6138 --- /dev/null +++ b/test/ingress/conformance/tests/httproute-same-host-and-path.go @@ -0,0 +1,101 @@ +// 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, HTTPRouteSameHostAndPath) +} + +var HTTPRouteSameHostAndPath = suite.ConformanceTest{ + ShortName: "HTTPRouteSameHostAndPath", + Description: "A single Ingress in the higress-conformance-infra namespace demonstrates the situation with same path and host", + Manifests: []string{"tests/httproute-same-host-and-path.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + testcases := []http.Assertion{ + { + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v1", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/hello-world", + Headers: map[string]string{ + "abc": "123", + "def": "456", + }, + }, + }, + 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: "/hello-world", + Headers: map[string]string{ + "abc": "123", + "def": "def", + }, + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + }, + { + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v3", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/hello-world", + Headers: map[string]string{ + "abc": "123", + }, + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + }, + } + + t.Run("Match Routes With same host and 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-same-host-and-path.yaml b/test/ingress/conformance/tests/httproute-same-host-and-path.yaml new file mode 100644 index 000000000..8f77f64d4 --- /dev/null +++ b/test/ingress/conformance/tests/httproute-same-host-and-path.yaml @@ -0,0 +1,104 @@ +# 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. + +# higress-same-host-and-path-01 and -02: to test same header key and different header value +# higress-same-host-and-path-03 and -04: to test route match precedence (04 > 03) +# higress-same-host-and-path-01 and -04: to test same header key and value but different order + +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + # exact matching + higress.io/exact-match-header-abc: "123" + higress.io/exact-match-header-def: "456" + name: higress-same-host-and-path-01 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: Prefix + path: "/hello-world" + backend: + service: + name: infra-backend-v1 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + # exact matching + higress.io/exact-match-header-abc: "123" + higress.io/exact-match-header-def: "def" + name: higress-same-host-and-path-02 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: Prefix + path: "/hello-world" + backend: + service: + name: infra-backend-v2 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + # exact matching + higress.io/exact-match-header-abc: "123" + name: higress-same-host-and-path-03 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: Prefix + path: "/hello-world" + backend: + service: + name: infra-backend-v3 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + # exact matching + higress.io/exact-match-header-def: "456" + higress.io/exact-match-header-abc: "123" + name: higress-same-host-and-path-04 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - http: + paths: + - pathType: Prefix + path: "/hello-world" + backend: + service: + name: infra-backend-v2 + port: + number: 8080 diff --git a/test/ingress/e2e_test.go b/test/ingress/e2e_test.go index 9a2482e72..219e8f754 100644 --- a/test/ingress/e2e_test.go +++ b/test/ingress/e2e_test.go @@ -65,6 +65,8 @@ func TestHigressConformanceTests(t *testing.T) { tests.HTTPRoutePermanentRedirect, tests.HTTPRoutePermanentRedirectCode, tests.HTTPRouteTemporalRedirect, + tests.HTTPRouteSameHostAndPath, + tests.HTTPRouteCanaryHeaderWithCustomizedHeader, tests.HTTPRouteWhitelistSourceRange, }