Skip to content

Commit

Permalink
fix #1712 : sort gateway api header fileter to fix canary restart
Browse files Browse the repository at this point in the history
  • Loading branch information
Mingjie Li committed Oct 18, 2024
1 parent a159421 commit 96d56f8
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 63 deletions.
18 changes: 18 additions & 0 deletions pkg/router/gateway_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"reflect"
"sort"
"strings"

flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
Expand Down Expand Up @@ -647,10 +648,23 @@ func (gwr *GatewayAPIRouter) mergeMatchConditions(analysis, service []v1.HTTPRou
return merged
}

func sortFiltersV1(headers []v1.HTTPHeader) {

if headers != nil {
sort.Slice(headers, func(i, j int) bool {
if headers[i].Name == headers[j].Name {
return headers[i].Value < headers[j].Value
}
return headers[i].Name < headers[j].Name
})
}
}

func (gwr *GatewayAPIRouter) makeFilters(canary *flaggerv1.Canary) []v1.HTTPRouteFilter {
var filters []v1.HTTPRouteFilter

if canary.Spec.Service.Headers != nil {

if canary.Spec.Service.Headers.Request != nil {
requestHeaderFilter := v1.HTTPRouteFilter{
Type: v1.HTTPRouteFilterRequestHeaderModifier,
Expand All @@ -663,13 +677,15 @@ func (gwr *GatewayAPIRouter) makeFilters(canary *flaggerv1.Canary) []v1.HTTPRout
Value: val,
})
}
sortFiltersV1(requestHeaderFilter.RequestHeaderModifier.Add)
for name, val := range canary.Spec.Service.Headers.Request.Set {
requestHeaderFilter.RequestHeaderModifier.Set = append(requestHeaderFilter.RequestHeaderModifier.Set, v1.HTTPHeader{
Name: v1.HTTPHeaderName(name),
Value: val,
})
}

sortFiltersV1(requestHeaderFilter.RequestHeaderModifier.Set)
for _, name := range canary.Spec.Service.Headers.Request.Remove {
requestHeaderFilter.RequestHeaderModifier.Remove = append(requestHeaderFilter.RequestHeaderModifier.Remove, name)
}
Expand All @@ -688,12 +704,14 @@ func (gwr *GatewayAPIRouter) makeFilters(canary *flaggerv1.Canary) []v1.HTTPRout
Value: val,
})
}
sortFiltersV1(responseHeaderFilter.ResponseHeaderModifier.Add)
for name, val := range canary.Spec.Service.Headers.Response.Set {
responseHeaderFilter.ResponseHeaderModifier.Set = append(responseHeaderFilter.ResponseHeaderModifier.Set, v1.HTTPHeader{
Name: v1.HTTPHeaderName(name),
Value: val,
})
}
sortFiltersV1(responseHeaderFilter.ResponseHeaderModifier.Set)

for _, name := range canary.Spec.Service.Headers.Response.Remove {
responseHeaderFilter.ResponseHeaderModifier.Remove = append(responseHeaderFilter.ResponseHeaderModifier.Remove, name)
Expand Down
88 changes: 25 additions & 63 deletions pkg/router/gateway_api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (

flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
v1 "github.com/fluxcd/flagger/pkg/apis/gatewayapi/v1"
istiov1beta1 "github.com/fluxcd/flagger/pkg/apis/istio/v1beta1"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -276,79 +278,39 @@ func TestGatewayAPIRouter_Routes(t *testing.T) {
})
}

func TestGatewayAPIRouter_getSessionAffinityRouteRules(t *testing.T) {
func TestGatewayAPIRouter_makeFilters(t *testing.T) {
canary := newTestGatewayAPICanary()
mocks := newFixture(canary)
cookieKey := "flagger-cookie"
canary.Spec.Analysis.SessionAffinity = &flaggerv1.SessionAffinity{
CookieName: cookieKey,
MaxAge: 300,
canary.Spec.Service.Headers = &istiov1beta1.Headers{
Response: &istiov1beta1.HeaderOperations{
Set: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
Add: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
},
Request: &istiov1beta1.HeaderOperations{
Set: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
Add: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
},
}

router := &GatewayAPIRouter{
gatewayAPIClient: mocks.meshClient,
kubeClient: mocks.kubeClient,
logger: mocks.logger,
}
_, pSvcName, cSvcName := canary.GetServiceNames()
weightedRouteRule := &v1.HTTPRouteRule{
BackendRefs: []v1.HTTPBackendRef{
{
BackendRef: router.makeBackendRef(pSvcName, initialPrimaryWeight, canary.Spec.Service.Port),
},
{
BackendRef: router.makeBackendRef(cSvcName, initialCanaryWeight, canary.Spec.Service.Port),
},
},
}
rules, err := router.getSessionAffinityRouteRules(canary, 10, weightedRouteRule)
require.NoError(t, err)
assert.Equal(t, len(rules), 2)
assert.True(t, strings.HasPrefix(canary.Status.SessionAffinityCookie, cookieKey))

stickyRule := rules[0]
cookieMatch := stickyRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, len(stickyRule.BackendRefs), 2)
for _, backendRef := range stickyRule.BackendRefs {
if string(backendRef.BackendRef.Name) == pSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(0))
}
if string(backendRef.BackendRef.Name) == cSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(100))
}

ignoreCmpOptions := []cmp.Option{
cmpopts.IgnoreFields(v1.BackendRef{}, "Weight"),
cmpopts.EquateEmpty(),
}

weightedRule := rules[1]
var found bool
for _, backendRef := range weightedRule.BackendRefs {
if string(backendRef.Name) == cSvcName {
found = true
filter := backendRef.Filters[0]
assert.Equal(t, filter.Type, v1.HTTPRouteFilterResponseHeaderModifier)
assert.NotNil(t, filter.ResponseHeaderModifier)
assert.Equal(t, string(filter.ResponseHeaderModifier.Add[0].Name), setCookieHeader)
assert.Equal(t, filter.ResponseHeaderModifier.Add[0].Value, fmt.Sprintf("%s; %s=%d", canary.Status.SessionAffinityCookie, maxAgeAttr, 300))
}
filters := router.makeFilters(canary)

for i := 0; i < 10; i++ {
newFilters := router.makeFilters(canary)
filtersDiff := cmp.Diff(
filters, newFilters,
ignoreCmpOptions...,
)
assert.Equal(t, "", filtersDiff)
}
assert.True(t, found)

rules, err = router.getSessionAffinityRouteRules(canary, 0, weightedRouteRule)
assert.Empty(t, canary.Status.SessionAffinityCookie)
assert.Contains(t, canary.Status.PreviousSessionAffinityCookie, cookieKey)

stickyRule = rules[0]
cookieMatch = stickyRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, stickyRule.Filters[0].Type, v1.HTTPRouteFilterResponseHeaderModifier)
headerModifier := stickyRule.Filters[0].ResponseHeaderModifier
assert.NotNil(t, headerModifier)
assert.Equal(t, string(headerModifier.Add[0].Name), setCookieHeader)
assert.Equal(t, headerModifier.Add[0].Value, fmt.Sprintf("%s; %s=%d", canary.Status.PreviousSessionAffinityCookie, maxAgeAttr, -1))
}
17 changes: 17 additions & 0 deletions pkg/router/gateway_api_v1beta1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"reflect"
"sort"
"strings"

flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
Expand Down Expand Up @@ -608,6 +609,18 @@ func (gwr *GatewayAPIV1Beta1Router) mergeMatchConditions(analysis, service []v1b
return merged
}

func sortFiltersV1beta1(headers []v1beta1.HTTPHeader) {

if headers != nil {
sort.Slice(headers, func(i, j int) bool {
if headers[i].Name == headers[j].Name {
return headers[i].Value < headers[j].Value
}
return headers[i].Name < headers[j].Name
})
}
}

func (gwr *GatewayAPIV1Beta1Router) makeFilters(canary *flaggerv1.Canary) []v1beta1.HTTPRouteFilter {
var filters []v1beta1.HTTPRouteFilter

Expand All @@ -624,12 +637,14 @@ func (gwr *GatewayAPIV1Beta1Router) makeFilters(canary *flaggerv1.Canary) []v1be
Value: val,
})
}
sortFiltersV1beta1(requestHeaderFilter.RequestHeaderModifier.Add)
for name, val := range canary.Spec.Service.Headers.Request.Set {
requestHeaderFilter.RequestHeaderModifier.Set = append(requestHeaderFilter.RequestHeaderModifier.Set, v1beta1.HTTPHeader{
Name: v1beta1.HTTPHeaderName(name),
Value: val,
})
}
sortFiltersV1beta1(requestHeaderFilter.RequestHeaderModifier.Set)

for _, name := range canary.Spec.Service.Headers.Request.Remove {
requestHeaderFilter.RequestHeaderModifier.Remove = append(requestHeaderFilter.RequestHeaderModifier.Remove, name)
Expand All @@ -649,12 +664,14 @@ func (gwr *GatewayAPIV1Beta1Router) makeFilters(canary *flaggerv1.Canary) []v1be
Value: val,
})
}
sortFiltersV1beta1(responseHeaderFilter.ResponseHeaderModifier.Add)
for name, val := range canary.Spec.Service.Headers.Response.Set {
responseHeaderFilter.ResponseHeaderModifier.Set = append(responseHeaderFilter.ResponseHeaderModifier.Set, v1beta1.HTTPHeader{
Name: v1beta1.HTTPHeaderName(name),
Value: val,
})
}
sortFiltersV1beta1(responseHeaderFilter.ResponseHeaderModifier.Set)

for _, name := range canary.Spec.Service.Headers.Response.Remove {
responseHeaderFilter.ResponseHeaderModifier.Remove = append(responseHeaderFilter.ResponseHeaderModifier.Remove, name)
Expand Down
40 changes: 40 additions & 0 deletions pkg/router/gateway_api_v1beta1_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ import (
"testing"

flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
v1 "github.com/fluxcd/flagger/pkg/apis/gatewayapi/v1"
"github.com/fluxcd/flagger/pkg/apis/gatewayapi/v1beta1"
istiov1beta1 "github.com/fluxcd/flagger/pkg/apis/istio/v1beta1"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -349,3 +352,40 @@ func TestGatewayAPIV1Beta1Router_getSessionAffinityRouteRules(t *testing.T) {
assert.Equal(t, string(headerModifier.Add[0].Name), setCookieHeader)
assert.Equal(t, headerModifier.Add[0].Value, fmt.Sprintf("%s; %s=%d", canary.Status.PreviousSessionAffinityCookie, maxAgeAttr, -1))
}

func TestGatewayAPIV1Beta1Router_makeFilters(t *testing.T) {
canary := newTestGatewayAPICanary()
mocks := newFixture(canary)
canary.Spec.Service.Headers = &istiov1beta1.Headers{
Response: &istiov1beta1.HeaderOperations{
Set: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
Add: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
},
Request: &istiov1beta1.HeaderOperations{
Set: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
Add: map[string]string{"h1": "v1", "h2": "v2", "h3": "v3"},
},
}

router := &GatewayAPIV1Beta1Router{
gatewayAPIClient: mocks.meshClient,
kubeClient: mocks.kubeClient,
logger: mocks.logger,
}

ignoreCmpOptions := []cmp.Option{
cmpopts.IgnoreFields(v1.BackendRef{}, "Weight"),
cmpopts.EquateEmpty(),
}

filters := router.makeFilters(canary)

for i := 0; i < 10; i++ {
newFilters := router.makeFilters(canary)
filtersDiff := cmp.Diff(
filters, newFilters,
ignoreCmpOptions...,
)
assert.Equal(t, "", filtersDiff)
}
}

0 comments on commit 96d56f8

Please sign in to comment.