optimize createRuleKey func (#240)

Signed-off-by: charlie <qianglin98@qq.com>

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

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 {

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

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)
+			key := createRuleKey(cfg.Annotations, hostAndPath)
-			if err != nil {
-				return err
-			}
 			wrapperHttpRoute.RuleKey = key
-			wrapperHttpRoute.RuleHash = hash
+			if WrapPreIngress, exist := convertOptions.Route2Ingress[key]; exist {
-			if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; 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())
+			canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat())
-			if err != nil {
-				return err
-			}
-			canary.RuleKey = key
-			canary.RuleHash = hash
 
 			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() &&
+	return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleKey == route.RuleKey
-		canary.RuleHash == route.RuleHash && 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
+		headers [][2]string
-		params  []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])
+		if idx != 0 {
-		sb.WriteString(annots[headers[idx]])
+			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])
+		if idx != 0 {
-		sb.WriteString(annots[params[idx]])
+			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()
+	return sb.String()
-	if _, err := hash.Write([]byte(str)); err != nil {
-		return 0, "", err
-	}
-
-	return hash.Sum32(), str, nil
 }

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
+}

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)
+			key := createRuleKey(cfg.Annotations, hostAndPath)
-			if err != nil {
-				return err
-			}
 			wrapperHttpRoute.RuleKey = key
-			wrapperHttpRoute.RuleHash = hash
+			if WrapPreIngress, exist := convertOptions.Route2Ingress[key]; exist {
-			if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; 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())
+			canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat())
-			if err != nil {
-				return err
-			}
-			canary.RuleKey = key
-			canary.RuleHash = hash
 
 			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
+		headers [][2]string
-		params  []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])
+		if idx != 0 {
-		sb.WriteString(annots[headers[idx]])
+			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])
+		if idx != 0 {
-		sb.WriteString(annots[params[idx]])
+			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()
+	return sb.String()
-	if _, err := hash.Write([]byte(str)); err != nil {
-		return 0, "", err
-	}
-
-	return hash.Sum32(), str, nil
 }