feat: add more precedence rules for route matching (#214)

Signed-off-by: charlie <qianglin98@qq.com>
This commit is contained in:
Qianglin Li
2023-03-08 10:29:32 +08:00
committed by GitHub
parent 7f6b157a19
commit ff560b8d25
15 changed files with 814 additions and 63 deletions

View File

@@ -330,10 +330,10 @@ func (m *IngressConfig) convertGateways(configs []common.WrapperConfig) []config
func (m *IngressConfig) convertVirtualService(configs []common.WrapperConfig) []config.Config { func (m *IngressConfig) convertVirtualService(configs []common.WrapperConfig) []config.Config {
convertOptions := common.ConvertOptions{ convertOptions := common.ConvertOptions{
HostAndPath2Ingress: map[string]*config.Config{}, IngressRouteCache: common.NewIngressRouteCache(),
IngressRouteCache: common.NewIngressRouteCache(), VirtualServices: map[string]*common.WrapperVirtualService{},
VirtualServices: map[string]*common.WrapperVirtualService{}, HTTPRoutes: map[string][]*common.WrapperHTTPRoute{},
HTTPRoutes: map[string][]*common.WrapperHTTPRoute{}, Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{},
} }
// convert http route // convert http route

View File

@@ -26,9 +26,9 @@ const (
exact = "exact" exact = "exact"
regex = "regex" regex = "regex"
prefix = "prefix" prefix = "prefix"
matchMethod = "match-method" MatchMethod = "match-method"
matchQuery = "match-query" MatchQuery = "match-query"
matchHeader = "match-header" MatchHeader = "match-header"
sep = " " 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) 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) 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) 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 { func (m match) matchByMethod(annotations Annotations, ingress *Ingress) error {
if !annotations.HasHigress(matchMethod) { if !annotations.HasHigress(MatchMethod) {
return nil return nil
} }
config := ingress.Match config := ingress.Match
str, err := annotations.ParseStringForHigress(matchMethod) str, err := annotations.ParseStringForHigress(MatchMethod)
if err != nil { if err != nil {
return err return err
} }
@@ -119,7 +119,7 @@ func (m match) matchByMethod(annotations Annotations, ingress *Ingress) error {
return nil 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) { 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 { for k, v := range annotations {
if idx := strings.Index(k, key); idx != -1 { if idx := strings.Index(k, key); idx != -1 {

View File

@@ -33,7 +33,7 @@ func TestMatch_ParseMethods(t *testing.T) {
}, },
{ {
input: Annotations{ input: Annotations{
buildHigressAnnotationKey(matchMethod): "PUT POST PATCH", buildHigressAnnotationKey(MatchMethod): "PUT POST PATCH",
}, },
expect: &MatchConfig{ expect: &MatchConfig{
Methods: []string{"PUT", "POST", "PATCH"}, Methods: []string{"PUT", "POST", "PATCH"},
@@ -41,7 +41,7 @@ func TestMatch_ParseMethods(t *testing.T) {
}, },
{ {
input: Annotations{ input: Annotations{
buildHigressAnnotationKey(matchMethod): "PUT PUT", buildHigressAnnotationKey(MatchMethod): "PUT PUT",
}, },
expect: &MatchConfig{ expect: &MatchConfig{
Methods: []string{"PUT"}, Methods: []string{"PUT"},
@@ -49,7 +49,7 @@ func TestMatch_ParseMethods(t *testing.T) {
}, },
{ {
input: Annotations{ input: Annotations{
buildHigressAnnotationKey(matchMethod): "put post patch", buildHigressAnnotationKey(MatchMethod): "put post patch",
}, },
expect: &MatchConfig{ expect: &MatchConfig{
Methods: []string{"PUT", "POST", "PATCH"}, Methods: []string{"PUT", "POST", "PATCH"},
@@ -57,7 +57,7 @@ func TestMatch_ParseMethods(t *testing.T) {
}, },
{ {
input: Annotations{ input: Annotations{
buildHigressAnnotationKey(matchMethod): "geet", buildHigressAnnotationKey(MatchMethod): "geet",
}, },
expect: &MatchConfig{}, expect: &MatchConfig{},
}, },
@@ -116,7 +116,7 @@ func TestMatch_ParseHeaders(t *testing.T) {
for _, tt := range testCases { for _, tt := range testCases {
t.Run("", func(t *testing.T) { 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} input := Annotations{key: tt.value}
config := &Ingress{} config := &Ingress{}
_ = parser.Parse(input, config, nil) _ = parser.Parse(input, config, nil)
@@ -169,7 +169,7 @@ func TestMatch_ParseQueryParams(t *testing.T) {
for _, tt := range testCases { for _, tt := range testCases {
t.Run("", func(t *testing.T) { 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} input := Annotations{key: tt.value}
config := &Ingress{} config := &Ingress{}
_ = parser.Parse(input, config, nil) _ = parser.Parse(input, config, nil)

View File

@@ -38,6 +38,11 @@ type WrapperConfig struct {
AnnotationsConfig *annotations.Ingress AnnotationsConfig *annotations.Ingress
} }
type WrapperConfigWithRuleKey struct {
Config *config.Config
RuleKey string
}
type WrapperGateway struct { type WrapperGateway struct {
Gateway *networking.Gateway Gateway *networking.Gateway
WrapperConfig *WrapperConfig WrapperConfig *WrapperConfig
@@ -70,6 +75,8 @@ type WrapperHTTPRoute struct {
OriginPathType PathType OriginPathType PathType
WeightTotal int32 WeightTotal int32
IsDefaultBackend bool IsDefaultBackend bool
RuleKey string
RuleHash uint32
} }
func (w *WrapperHTTPRoute) Meta() string { func (w *WrapperHTTPRoute) Meta() string {

View File

@@ -149,7 +149,8 @@ type ConvertOptions struct {
IngressDomainCache *IngressDomainCache 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 // Record valid/invalid routes from ingress
IngressRouteCache *IngressRouteCache IngressRouteCache *IngressRouteCache

View File

@@ -175,6 +175,9 @@ func SortHTTPRoutes(routes []*WrapperHTTPRoute) {
return false 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 { sort.SliceStable(routes, func(i, j int) bool {
// Move default backend to end // Move default backend to end
if isDefaultBackend(routes[i]) { if isDefaultBackend(routes[i]) {
@@ -193,7 +196,27 @@ func SortHTTPRoutes(routes []*WrapperHTTPRoute) {
} }
if routes[i].OriginPathType == routes[j].OriginPathType { 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 { if routes[i].OriginPathType == Exact {

View File

@@ -416,3 +416,143 @@ func TestSortRoutes(t *testing.T) {
t.Fatal("should be test-3") 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)
}
}
}

View File

@@ -17,9 +17,11 @@ package ingress
import ( import (
"errors" "errors"
"fmt" "fmt"
"hash/fnv"
"path" "path"
"reflect" "reflect"
"regexp" "regexp"
"sort"
"strings" "strings"
"sync" "sync"
"time" "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. // When the host, pathType, path of two rule are same, we think there is a conflict event.
definedRules := sets.NewSet() definedRules := sets.NewSet()
// But in across ingresses case, we will restrict this limit. var (
// When the host, path of two rule in different ingress are same, we think there is a conflict event. // But in across ingresses case, we will restrict this limit.
var tempHostAndPath []string // 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 { for _, rule := range ingressV1.Rules {
if rule.HTTP == nil || len(rule.HTTP.Paths) == 0 { 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) 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) ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute)
// host and path overlay check across different ingresses. hostAndPath := wrapperHttpRoute.PathFormat()
hostAndPath := wrapperHttpRoute.BasePathFormat() hash, key, err := createRuleKey(cfg.Annotations, hostAndPath)
if preIngress, exist := convertOptions.HostAndPath2Ingress[hostAndPath]; exist { if err != nil {
ingressRouteBuilder.PreIngress = preIngress return err
}
wrapperHttpRoute.RuleKey = key
wrapperHttpRoute.RuleHash = hash
if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist {
ingressRouteBuilder.PreIngress = WrapPreIngress.Config
ingressRouteBuilder.Event = common.DuplicatedRoute ingressRouteBuilder.Event = common.DuplicatedRoute
} }
tempHostAndPath = append(tempHostAndPath, hostAndPath) tempRuleHash = append(tempRuleHash, hash)
tempRuleKey = append(tempRuleKey, key)
// Two duplicated rules in the same ingress. // Two duplicated rules in the same ingress.
if ingressRouteBuilder.Event == common.Normal { if ingressRouteBuilder.Event == common.Normal {
@@ -607,10 +619,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
convertOptions.IngressRouteCache.Add(ingressRouteBuilder) convertOptions.IngressRouteCache.Add(ingressRouteBuilder)
} }
for _, item := range tempHostAndPath { for idx, item := range tempRuleHash {
// We only record the first if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 {
if _, exist := convertOptions.HostAndPath2Ingress[item]; !exist { convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{
convertOptions.HostAndPath2Ingress[item] = cfg Config: cfg,
RuleKey: tempRuleKey[idx],
}
} }
} }
@@ -794,6 +808,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
convertOptions.IngressRouteCache.Add(ingressRouteBuilder) convertOptions.IngressRouteCache.Add(ingressRouteBuilder)
continue 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 canaryConfig := wrapper.AnnotationsConfig.Canary
if byWeight { if byWeight {
@@ -1004,8 +1024,8 @@ func (c *controller) createServiceKey(service *ingress.IngressBackend, namespace
} }
func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool { func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool {
return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.OriginPath == route.OriginPath && return route != nil && canary != nil && !route.WrapperConfig.AnnotationsConfig.IsCanary() &&
canary.OriginPathType == route.OriginPathType canary.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey
} }
func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, 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
}

View File

@@ -103,10 +103,10 @@ func testApplyCanaryIngress(t *testing.T, c common.IngressController) {
Valid: make(map[string]*common.IngressDomainBuilder), Valid: make(map[string]*common.IngressDomainBuilder),
Invalid: make([]model.IngressDomain, 0), Invalid: make([]model.IngressDomain, 0),
}, },
HostAndPath2Ingress: make(map[string]*config.Config), Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{},
VirtualServices: make(map[string]*common.WrapperVirtualService), VirtualServices: make(map[string]*common.WrapperVirtualService),
Gateways: make(map[string]*common.WrapperGateway), Gateways: make(map[string]*common.WrapperGateway),
IngressRouteCache: &common.IngressRouteCache{}, IngressRouteCache: &common.IngressRouteCache{},
HTTPRoutes: map[string][]*common.WrapperHTTPRoute{ HTTPRoutes: map[string][]*common.WrapperHTTPRoute{
"test1": make([]*common.WrapperHTTPRoute, 0), "test1": make([]*common.WrapperHTTPRoute, 0),
}, },
@@ -193,11 +193,11 @@ func testApplyDefaultBackend(t *testing.T, c common.IngressController) {
Valid: make(map[string]*common.IngressDomainBuilder), Valid: make(map[string]*common.IngressDomainBuilder),
Invalid: make([]model.IngressDomain, 0), Invalid: make([]model.IngressDomain, 0),
}, },
HostAndPath2Ingress: make(map[string]*config.Config), Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{},
VirtualServices: make(map[string]*common.WrapperVirtualService), VirtualServices: make(map[string]*common.WrapperVirtualService),
Gateways: make(map[string]*common.WrapperGateway), Gateways: make(map[string]*common.WrapperGateway),
IngressRouteCache: &common.IngressRouteCache{}, IngressRouteCache: &common.IngressRouteCache{},
HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute),
}, },
wrapperConfig: &common.WrapperConfig{Config: &config.Config{ wrapperConfig: &common.WrapperConfig{Config: &config.Config{
Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{ Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{
@@ -385,11 +385,11 @@ func testConvertHTTPRoute(t *testing.T, c common.IngressController) {
Valid: make(map[string]*common.IngressDomainBuilder), Valid: make(map[string]*common.IngressDomainBuilder),
Invalid: make([]model.IngressDomain, 0), Invalid: make([]model.IngressDomain, 0),
}, },
HostAndPath2Ingress: make(map[string]*config.Config), Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{},
VirtualServices: make(map[string]*common.WrapperVirtualService), VirtualServices: make(map[string]*common.WrapperVirtualService),
Gateways: make(map[string]*common.WrapperGateway), Gateways: make(map[string]*common.WrapperGateway),
IngressRouteCache: &common.IngressRouteCache{}, IngressRouteCache: &common.IngressRouteCache{},
HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute), HTTPRoutes: make(map[string][]*common.WrapperHTTPRoute),
}, },
wrapperConfig: &common.WrapperConfig{Config: &config.Config{ wrapperConfig: &common.WrapperConfig{Config: &config.Config{
Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{ Spec: ingress.IngressSpec{Rules: []ingress.IngressRule{
@@ -474,7 +474,7 @@ func testConvertTrafficPolicy(t *testing.T, c common.IngressController) {
Valid: make(map[string]*common.IngressDomainBuilder), Valid: make(map[string]*common.IngressDomainBuilder),
Invalid: make([]model.IngressDomain, 0), Invalid: make([]model.IngressDomain, 0),
}, },
HostAndPath2Ingress: make(map[string]*config.Config), Route2Ingress: map[uint32]*common.WrapperConfigWithRuleKey{},
VirtualServices: make(map[string]*common.WrapperVirtualService), VirtualServices: make(map[string]*common.WrapperVirtualService),
Gateways: make(map[string]*common.WrapperGateway), Gateways: make(map[string]*common.WrapperGateway),
IngressRouteCache: &common.IngressRouteCache{}, IngressRouteCache: &common.IngressRouteCache{},

View File

@@ -17,9 +17,11 @@ package ingressv1
import ( import (
"errors" "errors"
"fmt" "fmt"
"hash/fnv"
"path" "path"
"reflect" "reflect"
"regexp" "regexp"
"sort"
"strings" "strings"
"sync" "sync"
"time" "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. // When the host, pathType, path of two rule are same, we think there is a conflict event.
definedRules := sets.NewSet() definedRules := sets.NewSet()
// But in across ingresses case, we will restrict this limit. var (
// When the host, path of two rule in different ingress are same, we think there is a conflict event. // But in across ingresses case, we will restrict this limit.
var tempHostAndPath []string // 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 { for _, rule := range ingressV1.Rules {
if rule.HTTP == nil || len(rule.HTTP.Paths) == 0 { 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) 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) ingressRouteBuilder := convertOptions.IngressRouteCache.New(wrapperHttpRoute)
// host and path overlay check across different ingresses. hostAndPath := wrapperHttpRoute.PathFormat()
hostAndPath := wrapperHttpRoute.BasePathFormat() hash, key, err := createRuleKey(cfg.Annotations, hostAndPath)
if preIngress, exist := convertOptions.HostAndPath2Ingress[hostAndPath]; exist { if err != nil {
ingressRouteBuilder.PreIngress = preIngress return err
}
wrapperHttpRoute.RuleKey = key
wrapperHttpRoute.RuleHash = hash
if WrapPreIngress, exist := convertOptions.Route2Ingress[hash]; exist {
ingressRouteBuilder.PreIngress = WrapPreIngress.Config
ingressRouteBuilder.Event = common.DuplicatedRoute ingressRouteBuilder.Event = common.DuplicatedRoute
} }
tempHostAndPath = append(tempHostAndPath, hostAndPath) tempRuleHash = append(tempRuleHash, hash)
tempRuleKey = append(tempRuleKey, key)
// Two duplicated rules in the same ingress. // Two duplicated rules in the same ingress.
if ingressRouteBuilder.Event == common.Normal { if ingressRouteBuilder.Event == common.Normal {
@@ -589,10 +601,12 @@ func (c *controller) ConvertHTTPRoute(convertOptions *common.ConvertOptions, wra
convertOptions.IngressRouteCache.Add(ingressRouteBuilder) convertOptions.IngressRouteCache.Add(ingressRouteBuilder)
} }
for _, item := range tempHostAndPath { for idx, item := range tempRuleHash {
// We only record the first if val, exist := convertOptions.Route2Ingress[item]; !exist || strings.Compare(val.RuleKey, tempRuleKey[idx]) != 0 {
if _, exist := convertOptions.HostAndPath2Ingress[item]; !exist { convertOptions.Route2Ingress[item] = &common.WrapperConfigWithRuleKey{
convertOptions.HostAndPath2Ingress[item] = cfg Config: cfg,
RuleKey: tempRuleKey[idx],
}
} }
} }
@@ -763,6 +777,12 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
convertOptions.IngressRouteCache.Add(ingressRouteBuilder) convertOptions.IngressRouteCache.Add(ingressRouteBuilder)
continue 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 canaryConfig := wrapper.AnnotationsConfig.Canary
if byWeight { if byWeight {
@@ -960,8 +980,7 @@ func (c *controller) createServiceKey(service *ingress.IngressServiceBackend, na
} }
func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool { func isCanaryRoute(canary, route *common.WrapperHTTPRoute) bool {
return !strings.HasSuffix(route.HTTPRoute.Name, "-canary") && canary.OriginPath == route.OriginPath && return !route.WrapperConfig.AnnotationsConfig.IsCanary() && canary.RuleHash == route.RuleHash && canary.RuleKey == route.RuleKey
canary.OriginPathType == route.OriginPathType
} }
func (c *controller) backendToRouteDestination(backend *ingress.IngressBackend, namespace string, 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
}

View File

@@ -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)
}
})
},
}

View File

@@ -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

View File

@@ -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)
}
})
},
}

View File

@@ -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

View File

@@ -65,6 +65,8 @@ func TestHigressConformanceTests(t *testing.T) {
tests.HTTPRoutePermanentRedirect, tests.HTTPRoutePermanentRedirect,
tests.HTTPRoutePermanentRedirectCode, tests.HTTPRoutePermanentRedirectCode,
tests.HTTPRouteTemporalRedirect, tests.HTTPRouteTemporalRedirect,
tests.HTTPRouteSameHostAndPath,
tests.HTTPRouteCanaryHeaderWithCustomizedHeader,
tests.HTTPRouteWhitelistSourceRange, tests.HTTPRouteWhitelistSourceRange,
} }