Sync inner fix (#634)

This commit is contained in:
澄潭
2023-11-14 11:15:26 +08:00
committed by GitHub
parent cb0479510f
commit 5174397e7c
14 changed files with 228 additions and 101 deletions

View File

@@ -74,11 +74,7 @@ type Ingress struct {
}
func (i *Ingress) NeedRegexMatch() bool {
if i.Rewrite == nil {
return false
}
return i.Rewrite.RewriteTarget != "" || i.IsPrefixRegexMatch() || i.IsFullPathRegexMatch()
return i.Rewrite != nil && (i.IsPrefixRegexMatch() || i.IsFullPathRegexMatch())
}
func (i *Ingress) IsPrefixRegexMatch() bool {

View File

@@ -34,7 +34,7 @@ func TestNeedRegexMatch(t *testing.T) {
{
input: &Ingress{
Rewrite: &RewriteConfig{
RewriteTarget: "/test",
UseRegex: true,
},
},
expect: true,
@@ -42,10 +42,10 @@ func TestNeedRegexMatch(t *testing.T) {
{
input: &Ingress{
Rewrite: &RewriteConfig{
UseRegex: true,
UseRegex: false,
},
},
expect: true,
expect: false,
},
}

View File

@@ -108,6 +108,8 @@ func ApplyByWeight(canary, route *networking.HTTPRoute, canaryIngress *Ingress)
// canary route use the header control applied on itself.
headerControl{}.ApplyRoute(canary, canaryIngress)
// reset
canary.Route[0].FallbackClusters = nil
// Move route level to destination level
canary.Route[0].Headers = canary.Headers
@@ -127,8 +129,6 @@ func ApplyByHeader(canary, route *networking.HTTPRoute, canaryIngress *Ingress)
// Inherit configuration from non-canary rule
route.DeepCopyInto(canary)
// Assign temp copied canary route match
canary.Match = temp.Match
// Assign temp copied canary route destination
canary.Route = temp.Route
@@ -165,7 +165,7 @@ func ApplyByHeader(canary, route *networking.HTTPRoute, canaryIngress *Ingress)
match.Headers = map[string]*networking.StringMatch{
"cookie": {
MatchType: &networking.StringMatch_Regex{
Regex: "^(.\\*?;)?(" + canaryConfig.Cookie + "=always)(;.\\*)?$",
Regex: "^(.*?;\\s*)?(" + canaryConfig.Cookie + "=always)(;.*)?$",
},
},
}

View File

@@ -15,6 +15,7 @@
package annotations
import (
"regexp"
"strings"
networking "istio.io/api/networking/v1alpha3"
@@ -37,6 +38,8 @@ const (
var (
_ Parser = headerControl{}
_ RouteHandler = headerControl{}
pattern = regexp.MustCompile(`\s+`)
)
type HeaderOperation struct {
@@ -138,6 +141,18 @@ func needHeaderControlConfig(annotations Annotations) bool {
annotations.HasHigress(responseHeaderRemove)
}
func trimQuotes(s string) string {
if len(s) >= 2 {
if s[0] == '"' && s[len(s)-1] == '"' {
return s[1 : len(s)-1]
}
if s[0] == '\'' && s[len(s)-1] == '\'' {
return s[1 : len(s)-1]
}
}
return s
}
func convertAddOrUpdate(headers string) map[string]string {
result := map[string]string{}
parts := strings.Split(headers, "\n")
@@ -147,13 +162,13 @@ func convertAddOrUpdate(headers string) map[string]string {
continue
}
keyValue := strings.Fields(part)
keyValue := pattern.Split(part, 2)
if len(keyValue) != 2 {
IngressLog.Errorf("Header format %s is invalid.", keyValue)
continue
}
key := strings.TrimSpace(keyValue[0])
value := strings.TrimSpace(keyValue[1])
key := trimQuotes(strings.TrimSpace(keyValue[0]))
value := trimQuotes(strings.TrimSpace(keyValue[1]))
result[key] = value
}
return result

View File

@@ -48,8 +48,8 @@ func TestHeaderControlParse(t *testing.T) {
},
{
input: map[string]string{
buildHigressAnnotationKey(requestHeaderAdd): "one 1\n two 2\nthree 3 \n",
buildHigressAnnotationKey(requestHeaderUpdate): "two 2",
buildHigressAnnotationKey(requestHeaderAdd): "one 1\n two 2\nthree 3 \nx-test mse; test=true\nx-pro mse; pro=true\n",
buildHigressAnnotationKey(requestHeaderUpdate): "two 2\n set-cookie name=test; sameage=111\nset-stage name=stage; stage=true\n",
buildHigressAnnotationKey(requestHeaderRemove): "one, two,three\n",
buildHigressAnnotationKey(responseHeaderAdd): "A a\nB b\n",
buildHigressAnnotationKey(responseHeaderUpdate): "X x\nY y\n",
@@ -58,12 +58,16 @@ func TestHeaderControlParse(t *testing.T) {
expect: &HeaderControlConfig{
Request: &HeaderOperation{
Add: map[string]string{
"one": "1",
"two": "2",
"three": "3",
"one": "1",
"two": "2",
"three": "3",
"x-test": "mse; test=true",
"x-pro": "mse; pro=true",
},
Update: map[string]string{
"two": "2",
"two": "2",
"set-cookie": "name=test; sameage=111",
"set-stage": "name=stage; stage=true",
},
Remove: []string{"one", "two", "three"},
},
@@ -122,12 +126,16 @@ func TestHeaderControlApplyRoute(t *testing.T) {
HeaderControl: &HeaderControlConfig{
Request: &HeaderOperation{
Add: map[string]string{
"one": "1",
"two": "2",
"three": "3",
"one": "1",
"two": "2",
"three": "3",
"x-test": "mse; test=true",
"x-pro": "mse; pro=true",
},
Update: map[string]string{
"two": "2",
"two": "2",
"set-cookie": "name=test; sameage=111",
"set-stage": "name=stage; sameage=111",
},
Remove: []string{"one", "two", "three"},
},
@@ -138,12 +146,16 @@ func TestHeaderControlApplyRoute(t *testing.T) {
Headers: &networking.Headers{
Request: &networking.Headers_HeaderOperations{
Add: map[string]string{
"one": "1",
"two": "2",
"three": "3",
"one": "1",
"two": "2",
"three": "3",
"x-test": "mse; test=true",
"x-pro": "mse; pro=true",
},
Set: map[string]string{
"two": "2",
"two": "2",
"set-cookie": "name=test; sameage=111",
"set-stage": "name=stage; sameage=111",
},
Remove: []string{"one", "two", "three"},
},

View File

@@ -70,8 +70,13 @@ func (r retry) Parse(annotations Annotations, config *Ingress, _ *GlobalContext)
}
if retryOn, err := annotations.ParseStringASAP(retryOn); err == nil {
extraConfigs := splitBySeparator(retryOn, ",")
conditions := toSet(extraConfigs)
var retryOnConditions []string
if strings.Contains(retryOn, ",") {
retryOnConditions = splitBySeparator(retryOn, ",")
} else {
retryOnConditions = strings.Fields(retryOn)
}
conditions := toSet(retryOnConditions)
if len(conditions) > 0 {
if conditions.Contains("off") {
retryConfig.retryCount = 0
@@ -88,7 +93,7 @@ func (r retry) Parse(annotations Annotations, config *Ingress, _ *GlobalContext)
stringBuilder.WriteString("non_idempotent,")
}
// Append the status codes.
statusCodes := convertStatusCodes(extraConfigs)
statusCodes := convertStatusCodes(retryOnConditions)
if len(statusCodes) > 0 {
stringBuilder.WriteString(retryStatusCode + ",")
for _, code := range statusCodes {

View File

@@ -37,8 +37,8 @@ func TestRetryParse(t *testing.T) {
},
expect: &RetryConfig{
retryCount: 1,
perRetryTimeout: &types.Duration{},
retryOn: "5xx",
perRetryTimeout: &types.Duration{},
},
},
{
@@ -60,8 +60,8 @@ func TestRetryParse(t *testing.T) {
},
expect: &RetryConfig{
retryCount: 0,
perRetryTimeout: &types.Duration{},
retryOn: "5xx",
perRetryTimeout: &types.Duration{},
},
},
{
@@ -71,8 +71,19 @@ func TestRetryParse(t *testing.T) {
},
expect: &RetryConfig{
retryCount: 2,
perRetryTimeout: &types.Duration{},
retryOn: "5xx",
perRetryTimeout: &types.Duration{},
},
},
{
input: map[string]string{
buildNginxAnnotationKey(retryCount): "2",
buildNginxAnnotationKey(retryOn): "error timeout",
},
expect: &RetryConfig{
retryCount: 2,
retryOn: "5xx",
perRetryTimeout: &types.Duration{},
},
},
{
@@ -81,8 +92,18 @@ func TestRetryParse(t *testing.T) {
},
expect: &RetryConfig{
retryCount: 3,
perRetryTimeout: &types.Duration{},
retryOn: "5xx,non_idempotent",
perRetryTimeout: &types.Duration{},
},
},
{
input: map[string]string{
buildNginxAnnotationKey(retryOn): "timeout non_idempotent",
},
expect: &RetryConfig{
retryCount: 3,
retryOn: "5xx,non_idempotent",
perRetryTimeout: &types.Duration{},
},
},
{
@@ -91,18 +112,18 @@ func TestRetryParse(t *testing.T) {
},
expect: &RetryConfig{
retryCount: 3,
perRetryTimeout: &types.Duration{},
retryOn: "5xx,retriable-status-codes,503,502,404",
perRetryTimeout: &types.Duration{},
},
},
{
input: map[string]string{
buildNginxAnnotationKey(retryOn): "timeout,http_505,http_503,http_502,http_404,http_403",
buildNginxAnnotationKey(retryOn): "timeout http_503 http_502 http_404",
},
expect: &RetryConfig{
retryCount: 3,
retryOn: "5xx,retriable-status-codes,503,502,404",
perRetryTimeout: &types.Duration{},
retryOn: "5xx,retriable-status-codes,505,503,502,404,403",
},
},
}

View File

@@ -59,12 +59,6 @@ func (r rewrite) Parse(annotations Annotations, config *Ingress, _ *GlobalContex
rewriteConfig.RewritePath, _ = annotations.ParseStringForHigress(rewritePath)
if rewriteConfig.RewritePath == "" && rewriteConfig.RewriteTarget != "" {
// When rewrite target is present and not empty,
// we will enforce regex match on all rules in this ingress.
if !rewriteConfig.UseRegex && !rewriteConfig.FullPathRegex {
rewriteConfig.UseRegex = true
}
// We should convert nginx regex rule to envoy regex rule.
rewriteConfig.RewriteTarget = convertToRE2(rewriteConfig.RewriteTarget)
}
@@ -92,9 +86,22 @@ func (r rewrite) ApplyRoute(route *networking.HTTPRoute, config *Ingress) {
}
}
} else if rewriteConfig.RewriteTarget != "" {
route.Rewrite.UriRegex = &networking.RegexMatchAndSubstitute{
Pattern: route.Match[0].Uri.GetRegex(),
Substitution: rewriteConfig.RewriteTarget,
uri := route.Match[0].Uri
if uri.GetExact() != "" {
route.Rewrite.UriRegex = &networking.RegexMatchAndSubstitute{
Pattern: uri.GetExact(),
Substitution: rewriteConfig.RewriteTarget,
}
} else if uri.GetPrefix() != "" {
route.Rewrite.UriRegex = &networking.RegexMatchAndSubstitute{
Pattern: uri.GetPrefix(),
Substitution: rewriteConfig.RewriteTarget,
}
} else {
route.Rewrite.UriRegex = &networking.RegexMatchAndSubstitute{
Pattern: uri.GetRegex(),
Substitution: rewriteConfig.RewriteTarget,
}
}
}

View File

@@ -77,16 +77,13 @@ func TestRewriteParse(t *testing.T) {
},
expect: &RewriteConfig{
RewriteTarget: "/test",
UseRegex: true,
},
},
{
input: Annotations{
buildNginxAnnotationKey(rewriteTarget): "",
},
expect: &RewriteConfig{
UseRegex: false,
},
expect: &RewriteConfig{},
},
{
input: Annotations{
@@ -94,7 +91,6 @@ func TestRewriteParse(t *testing.T) {
},
expect: &RewriteConfig{
RewriteTarget: "/\\2/\\1",
UseRegex: true,
},
},
{
@@ -113,6 +109,16 @@ func TestRewriteParse(t *testing.T) {
RewriteHost: "test.com",
},
},
{
input: Annotations{
buildNginxAnnotationKey(useRegex): "true",
buildNginxAnnotationKey(rewriteTarget): "/$1",
},
expect: &RewriteConfig{
UseRegex: true,
RewriteTarget: "/\\1",
},
},
{
input: Annotations{
buildNginxAnnotationKey(rewriteTarget): "/$2/$1",
@@ -120,7 +126,6 @@ func TestRewriteParse(t *testing.T) {
},
expect: &RewriteConfig{
RewriteTarget: "/\\2/\\1",
UseRegex: true,
RewriteHost: "test.com",
},
},
@@ -330,6 +335,76 @@ func TestRewriteApplyRoute(t *testing.T) {
},
},
},
{
config: &Ingress{
Rewrite: &RewriteConfig{
RewriteTarget: "/test",
},
},
input: &networking.HTTPRoute{
Match: []*networking.HTTPMatchRequest{
{
Uri: &networking.StringMatch{
MatchType: &networking.StringMatch_Exact{
Exact: "/exact",
},
},
},
},
},
expect: &networking.HTTPRoute{
Match: []*networking.HTTPMatchRequest{
{
Uri: &networking.StringMatch{
MatchType: &networking.StringMatch_Exact{
Exact: "/exact",
},
},
},
},
Rewrite: &networking.HTTPRewrite{
UriRegex: &networking.RegexMatchAndSubstitute{
Pattern: "/exact",
Substitution: "/test",
},
},
},
},
{
config: &Ingress{
Rewrite: &RewriteConfig{
RewriteTarget: "/test",
},
},
input: &networking.HTTPRoute{
Match: []*networking.HTTPMatchRequest{
{
Uri: &networking.StringMatch{
MatchType: &networking.StringMatch_Prefix{
Prefix: "/prefix",
},
},
},
},
},
expect: &networking.HTTPRoute{
Match: []*networking.HTTPMatchRequest{
{
Uri: &networking.StringMatch{
MatchType: &networking.StringMatch_Prefix{
Prefix: "/prefix",
},
},
},
},
Rewrite: &networking.HTTPRewrite{
UriRegex: &networking.RegexMatchAndSubstitute{
Pattern: "/prefix",
Substitution: "/test",
},
},
},
},
}
for _, inputCase := range inputCases {

View File

@@ -712,7 +712,7 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
return fmt.Errorf("wrapperConfig is nil")
}
byHeader, byWeight := wrapper.AnnotationsConfig.CanaryKind()
byHeader, _ := wrapper.AnnotationsConfig.CanaryKind()
cfg := wrapper.Config
ingressV1Beta, ok := cfg.Spec.(ingress.IngressSpec)
@@ -765,9 +765,6 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
}
canary.OriginPath = originPath
canary.OriginPathType = pathType
canary.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, nil)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
ingressRouteBuilder := convertOptions.IngressRouteCache.New(canary)
// backend service check
var event common.Event
@@ -781,39 +778,37 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
}
canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat())
canaryConfig := wrapper.AnnotationsConfig.Canary
if byWeight {
canary.HTTPRoute.Route[0].Weight = int32(canaryConfig.Weight)
}
// find the base ingress
pos := 0
var targetRoute *common.WrapperHTTPRoute
for _, route := range routes {
if isCanaryRoute(canary, route) {
targetRoute = route
// Header, Cookie
if byHeader {
IngressLog.Debug("Insert canary route by header")
annotations.ApplyByHeader(canary.HTTPRoute, route.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
} else {
IngressLog.Debug("Merge canary route by weight")
if route.WeightTotal == 0 {
route.WeightTotal = int32(canaryConfig.WeightTotal)
}
annotations.ApplyByWeight(canary.HTTPRoute, route.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
}
break
}
pos += 1
}
IngressLog.Debugf("Canary route is %v", canary)
if targetRoute == nil {
continue
}
canaryConfig := wrapper.AnnotationsConfig.Canary
// Header, Cookie
if byHeader {
IngressLog.Debug("Insert canary route by header")
annotations.ApplyByHeader(canary.HTTPRoute, targetRoute.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
} else {
IngressLog.Debug("Merge canary route by weight")
if targetRoute.WeightTotal == 0 {
targetRoute.WeightTotal = int32(canaryConfig.WeightTotal)
}
annotations.ApplyByWeight(canary.HTTPRoute, targetRoute.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
}
IngressLog.Debugf("Canary route is %v", canary)
if byHeader {
// Inherit policy from normal route
canary.WrapperConfig.AnnotationsConfig.Auth = targetRoute.WrapperConfig.AnnotationsConfig.Auth

View File

@@ -716,7 +716,7 @@ func (c *controller) ApplyDefaultBackend(convertOptions *common.ConvertOptions,
}
func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, wrapper *common.WrapperConfig) error {
byHeader, byWeight := wrapper.AnnotationsConfig.CanaryKind()
byHeader, _ := wrapper.AnnotationsConfig.CanaryKind()
cfg := wrapper.Config
ingressV1, ok := cfg.Spec.(ingress.IngressSpec)
@@ -769,8 +769,6 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
}
canary.OriginPath = originPath
canary.OriginPathType = pathType
canary.HTTPRoute.Match = c.generateHttpMatches(pathType, httpPath.Path, nil)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
ingressRouteBuilder := convertOptions.IngressRouteCache.New(canary)
// backend service check
@@ -785,39 +783,37 @@ func (c *controller) ApplyCanaryIngress(convertOptions *common.ConvertOptions, w
}
canary.RuleKey = createRuleKey(canary.WrapperConfig.Config.Annotations, canary.PathFormat())
canaryConfig := wrapper.AnnotationsConfig.Canary
if byWeight {
canary.HTTPRoute.Route[0].Weight = int32(canaryConfig.Weight)
}
// find the base ingress
pos := 0
var targetRoute *common.WrapperHTTPRoute
for _, route := range routes {
if isCanaryRoute(canary, route) {
targetRoute = route
// Header, Cookie
if byHeader {
IngressLog.Debug("Insert canary route by header")
annotations.ApplyByHeader(canary.HTTPRoute, route.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
} else {
IngressLog.Debug("Merge canary route by weight")
if route.WeightTotal == 0 {
route.WeightTotal = int32(canaryConfig.WeightTotal)
}
annotations.ApplyByWeight(canary.HTTPRoute, route.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
}
break
}
pos += 1
}
IngressLog.Debugf("Canary route is %v", canary)
if targetRoute == nil {
continue
}
canaryConfig := wrapper.AnnotationsConfig.Canary
// Header, Cookie
if byHeader {
IngressLog.Debug("Insert canary route by header")
annotations.ApplyByHeader(canary.HTTPRoute, targetRoute.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
canary.HTTPRoute.Name = common.GenerateUniqueRouteName(c.options.SystemNamespace, canary)
} else {
IngressLog.Debug("Merge canary route by weight")
if targetRoute.WeightTotal == 0 {
targetRoute.WeightTotal = int32(canaryConfig.WeightTotal)
}
annotations.ApplyByWeight(canary.HTTPRoute, targetRoute.HTTPRoute, canary.WrapperConfig.AnnotationsConfig)
}
IngressLog.Debugf("Canary route is %v", canary)
if byHeader {
// Inherit policy from normal route
canary.WrapperConfig.AnnotationsConfig.Auth = targetRoute.WrapperConfig.AnnotationsConfig.Auth

View File

@@ -78,8 +78,10 @@ var HTTPRouteRequestHeaderControl = suite.ConformanceTest{
Path: "/foo2",
Host: "foo.com",
Headers: map[string]string{
"stage": "test",
"canary": "true",
"stage": "test",
"canary": "true",
"x-test": "higress; test=true",
"x-test2": "higress; test=false",
},
},
},

View File

@@ -40,6 +40,8 @@ metadata:
higress.io/request-header-control-add: |
stage test
canary true
x-test "higress; test=true"
'x-test2' "higress; test=false"
name: httproute-request-header-control-add-more
namespace: higress-conformance-infra
spec:

View File

@@ -17,6 +17,7 @@ kind: Ingress
metadata:
annotations:
nginx.ingress.kubernetes.io/rewrite-target: "/$1"
nginx.ingress.kubernetes.io/use-regex: "true"
name: httproute-rewrite-path
namespace: higress-conformance-infra
spec: