diff --git a/pkg/ingress/config/ingress_config.go b/pkg/ingress/config/ingress_config.go index 480e40759..e78cf1fe7 100644 --- a/pkg/ingress/config/ingress_config.go +++ b/pkg/ingress/config/ingress_config.go @@ -333,7 +333,7 @@ func (m *IngressConfig) convertVirtualService(configs []common.WrapperConfig) [] IngressRouteCache: common.NewIngressRouteCache(), VirtualServices: map[string]*common.WrapperVirtualService{}, HTTPRoutes: map[string][]*common.WrapperHTTPRoute{}, - Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + Route2Ingress: map[string]*common.WrapperConfigWithRuleKey{}, } // convert http route diff --git a/pkg/ingress/kube/common/controller.go b/pkg/ingress/kube/common/controller.go index 2c8159c8d..6f22da24a 100644 --- a/pkg/ingress/kube/common/controller.go +++ b/pkg/ingress/kube/common/controller.go @@ -76,7 +76,6 @@ type WrapperHTTPRoute struct { 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 b064d7c49..7ad4d51a2 100644 --- a/pkg/ingress/kube/common/model.go +++ b/pkg/ingress/kube/common/model.go @@ -150,7 +150,7 @@ type ConvertOptions struct { IngressDomainCache *IngressDomainCache // the host, path, headers, params of rule => ingress - Route2Ingress map[uint32]*WrapperConfigWithRuleKey + Route2Ingress map[string]*WrapperConfigWithRuleKey // Record valid/invalid routes from ingress IngressRouteCache *IngressRouteCache diff --git a/pkg/ingress/kube/ingress/controller.go b/pkg/ingress/kube/ingress/controller.go index 981bdf6ff..19bbc7871 100644 --- a/pkg/ingress/kube/ingress/controller.go +++ b/pkg/ingress/kube/ingress/controller.go @@ -17,7 +17,6 @@ package ingress import ( "errors" "fmt" - "hash/fnv" "path" "reflect" "regexp" @@ -499,8 +498,7 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra 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 + tempRuleKey []string ) for _, rule := range ingressV1.Rules { @@ -573,17 +571,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute) hostAndPath := wrapperHttpRoute.PathFormat() - hash, key, err := createRuleKey(cfg.Annotations, hostAndPath) - if err != nil { - return err - } + key := createRuleKey(cfg.Annotations, hostAndPath) wrapperHttpRoute.RuleKey = key - wrapperHttpRoute.RuleHash = hash - if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist { + if WrapPreIngress, exist := convertOptions.Route2Ingress[key]; exist { ingressRouteBuilder.PreIngress = WrapPreIngress.Config ingressRouteBuilder.Event = common.DuplicatedRoute } - tempRuleHash = append(tempRuleHash, hash) tempRuleKey = append(tempRuleKey, key) // Two duplicated rules in the same ingress. @@ -619,7 +612,7 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra convertOptions.IngressRouteCache.Add(ingressRouteBuilder) } - for idx, item := range tempRuleHash { + for idx, item := range tempRuleKey { if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 { convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{ Config: cfg, @@ -808,12 +801,7 @@ 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 + canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat()) canaryConfig := wrapper.AnnotationsConfig.Canary if byWeight { @@ -1024,8 +1012,7 @@ 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.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey + return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleKey == route.RuleKey } func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, @@ -1243,48 +1230,61 @@ 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) { +func createRuleKey(annots map[string]string, hostAndPath string) string { var ( - headers []string - params []string + headers [][2]string + params [][2]string sb strings.Builder ) + sep := "\n\n" + // path sb.WriteString(hostAndPath) + sb.WriteString(sep) // methods if str, ok := annots[annotations.HigressAnnotationsPrefix+"/"+annotations.MatchMethod]; ok { sb.WriteString(str) } + sb.WriteString(sep) + + start := len(annotations.HigressAnnotationsPrefix) + 1 // example: higress.io/exact-match-header-key: value // headers && params - for k, _ := range annots { + for k, val := range annots { if idx := strings.Index(k, annotations.MatchHeader); idx != -1 { - headers = append(headers, k) + key := k[start:idx] + k[idx+len(annotations.MatchHeader)+1:] + headers = append(headers, [2]string{key, val}) } if idx := strings.Index(k, annotations.MatchQuery); idx != -1 { - params = append(params, k) + key := k[start:idx] + k[idx+len(annotations.MatchQuery)+1:] + params = append(params, [2]string{key, val}) } } sort.SliceStable(headers, func(i, j int) bool { - return headers[i] < headers[j] + return headers[i][0] < headers[j][0] }) sort.SliceStable(params, func(i, j int) bool { - return params[i] < params[j] + return params[i][0] < params[j][0] }) for idx := range headers { - sb.WriteString(headers[idx]) - sb.WriteString(annots[headers[idx]]) + if idx != 0 { + sb.WriteByte('\n') + } + sb.WriteString(headers[idx][0]) + sb.WriteByte('\t') + sb.WriteString(headers[idx][1]) } + sb.WriteString(sep) for idx := range params { - sb.WriteString(params[idx]) - sb.WriteString(annots[params[idx]]) + if idx != 0 { + sb.WriteByte('\n') + } + sb.WriteString(params[idx][0]) + sb.WriteByte('\t') + sb.WriteString(params[idx][1]) } + sb.WriteString(sep) - str, hash := sb.String(), fnv.New32() - if _, err := hash.Write([]byte(str)); err != nil { - return 0, "", err - } - - return hash.Sum32(), str, nil + return sb.String() } diff --git a/pkg/ingress/kube/ingress/controller_test.go b/pkg/ingress/kube/ingress/controller_test.go index 3528b1867..08841eb6f 100644 --- a/pkg/ingress/kube/ingress/controller_test.go +++ b/pkg/ingress/kube/ingress/controller_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "istio.io/api/networking/v1alpha3" "istio.io/istio/pilot/pkg/model" "istio.io/istio/pkg/config" @@ -103,7 +104,7 @@ func testApplyCanaryIngress(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + Route2Ingress: map[string]*common.WrapperConfigWithRuleKey{}, VirtualServices: make(map[string]*common.WrapperVirtualService), Gateways: make(map[string]*common.WrapperGateway), IngressRouteCache: &common.IngressRouteCache{}, @@ -193,7 +194,7 @@ func testApplyDefaultBackend(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + Route2Ingress: map[string]*common.WrapperConfigWithRuleKey{}, VirtualServices: make(map[string]*common.WrapperVirtualService), Gateways: make(map[string]*common.WrapperGateway), IngressRouteCache: &common.IngressRouteCache{}, @@ -385,7 +386,7 @@ func testConvertHTTPRoute(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + Route2Ingress: map[string]*common.WrapperConfigWithRuleKey{}, VirtualServices: make(map[string]*common.WrapperVirtualService), Gateways: make(map[string]*common.WrapperGateway), IngressRouteCache: &common.IngressRouteCache{}, @@ -474,7 +475,7 @@ func testConvertTrafficPolicy(t *testing.T, c common.IngressController) { Valid: make(map[string]*common.IngressDomainBuilder), Invalid: make([]model.IngressDomain, 0), }, - Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{}, + Route2Ingress: map[string]*common.WrapperConfigWithRuleKey{}, VirtualServices: make(map[string]*common.WrapperVirtualService), Gateways: make(map[string]*common.WrapperGateway), IngressRouteCache: &common.IngressRouteCache{}, @@ -1291,3 +1292,34 @@ func TestShouldProcessIngressUpdate(t *testing.T) { t.Fatal("should be true") } } + +func TestCreateRuleKey(t *testing.T) { + sep := "\n\n" + wrapperHttpRoute := &common.WrapperHTTPRoute{ + Host: "higress.com", + OriginPathType: common.Prefix, + OriginPath: "/foo", + } + + annots := annotations.Annotations{ + buildHigressAnnotationKey(annotations.MatchMethod): "GET PUT", + buildHigressAnnotationKey("exact-" + annotations.MatchHeader + "-abc"): "123", + buildHigressAnnotationKey("prefix-" + annotations.MatchHeader + "-def"): "456", + buildHigressAnnotationKey("exact-" + annotations.MatchQuery + "-region"): "beijing", + buildHigressAnnotationKey("prefix-" + annotations.MatchQuery + "-user-id"): "user-", + } + expect := "higress.com-prefix-/foo" + sep + //host-pathType-path + "GET PUT" + sep + // method + "exact-abc\t123" + "\n" + "prefix-def\t456" + sep + // header + "exact-region\tbeijing" + "\n" + "prefix-user-id\tuser-" + sep // params + + key := createRuleKey(annots, wrapperHttpRoute.PathFormat()) + if diff := cmp.Diff(expect, key); diff != "" { + + t.Errorf("CreateRuleKey() mismatch (-want +got):\n%s", diff) + } +} + +func buildHigressAnnotationKey(key string) string { + return annotations.HigressAnnotationsPrefix + "/" + key +} diff --git a/pkg/ingress/kube/ingressv1/controller.go b/pkg/ingress/kube/ingressv1/controller.go index 25fcc0174..cd04caaa8 100644 --- a/pkg/ingress/kube/ingressv1/controller.go +++ b/pkg/ingress/kube/ingressv1/controller.go @@ -17,7 +17,6 @@ package ingressv1 import ( "errors" "fmt" - "hash/fnv" "path" "reflect" "regexp" @@ -481,8 +480,7 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra 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 + tempRuleKey []string ) for _, rule := range ingressV1.Rules { @@ -555,17 +553,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute) hostAndPath := wrapperHttpRoute.PathFormat() - hash, key, err := createRuleKey(cfg.Annotations, hostAndPath) - if err != nil { - return err - } + key := createRuleKey(cfg.Annotations, hostAndPath) wrapperHttpRoute.RuleKey = key - wrapperHttpRoute.RuleHash = hash - if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist { + if WrapPreIngress, exist := convertOptions.Route2Ingress[key]; exist { ingressRouteBuilder.PreIngress = WrapPreIngress.Config ingressRouteBuilder.Event = common.DuplicatedRoute } - tempRuleHash = append(tempRuleHash, hash) tempRuleKey = append(tempRuleKey, key) // Two duplicated rules in the same ingress. @@ -601,7 +594,7 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra convertOptions.IngressRouteCache.Add(ingressRouteBuilder) } - for idx, item := range tempRuleHash { + for idx, item := range tempRuleKey { if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 { convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{ Config: cfg, @@ -777,12 +770,7 @@ 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 + canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat()) canaryConfig := wrapper.AnnotationsConfig.Canary if byWeight { @@ -980,7 +968,7 @@ func (c *controller) createServiceKey(service *ingress.IngressServiceBackend, na } func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool { - return !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey + return !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleKey == route.RuleKey } func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, @@ -1192,48 +1180,61 @@ 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) { +func createRuleKey(annots map[string]string, hostAndPath string) string { var ( - headers []string - params []string + headers [][2]string + params [][2]string sb strings.Builder ) + sep := "\n\n" + // path sb.WriteString(hostAndPath) + sb.WriteString(sep) // methods if str, ok := annots[annotations.HigressAnnotationsPrefix+"/"+annotations.MatchMethod]; ok { sb.WriteString(str) } + sb.WriteString(sep) + + start := len(annotations.HigressAnnotationsPrefix) + 1 // example: higress.io/exact-match-header-key: value // headers && params - for k, _ := range annots { + for k, val := range annots { if idx := strings.Index(k, annotations.MatchHeader); idx != -1 { - headers = append(headers, k) + key := k[start:idx] + k[idx+len(annotations.MatchHeader)+1:] + headers = append(headers, [2]string{key, val}) } if idx := strings.Index(k, annotations.MatchQuery); idx != -1 { - params = append(params, k) + key := k[start:idx] + k[idx+len(annotations.MatchQuery)+1:] + params = append(params, [2]string{key, val}) } } sort.SliceStable(headers, func(i, j int) bool { - return headers[i] < headers[j] + return headers[i][0] < headers[j][0] }) sort.SliceStable(params, func(i, j int) bool { - return params[i] < params[j] + return params[i][0] < params[j][0] }) for idx := range headers { - sb.WriteString(headers[idx]) - sb.WriteString(annots[headers[idx]]) + if idx != 0 { + sb.WriteByte('\n') + } + sb.WriteString(headers[idx][0]) + sb.WriteByte('\t') + sb.WriteString(headers[idx][1]) } + sb.WriteString(sep) for idx := range params { - sb.WriteString(params[idx]) - sb.WriteString(annots[params[idx]]) + if idx != 0 { + sb.WriteByte('\n') + } + sb.WriteString(params[idx][0]) + sb.WriteByte('\t') + sb.WriteString(params[idx][1]) } + sb.WriteString(sep) - str, hash := sb.String(), fnv.New32() - if _, err := hash.Write([]byte(str)); err != nil { - return 0, "", err - } - - return hash.Sum32(), str, nil + return sb.String() }