Merge pull request #1713 from mingjie-li/main

Gateway API: Sort header filters to avoid canary restarts
fluxcd · Nov 21, 2024 · 7cd1476 · 7cd1476
2 parents a159421 + b88e080
commit 7cd1476
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 0 deletions.
diff --git a/pkg/router/gateway_api.go b/pkg/router/gateway_api.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"slices"
 	"strings"
 
 	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
@@ -647,10 +648,23 @@ func (gwr *GatewayAPIRouter) mergeMatchConditions(analysis, service []v1.HTTPRou
 	return merged
 }
 
+func sortFiltersV1(headers []v1.HTTPHeader) {
+
+	if headers != nil {
+		slices.SortFunc(headers, func(a, b v1.HTTPHeader) int {
+			if a.Name == b.Name {
+				return strings.Compare(a.Value, b.Value)
+			}
+			return strings.Compare(string(a.Name), string(b.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,
@@ -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)
 			}
@@ -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)

diff --git a/pkg/router/gateway_api_test.go b/pkg/router/gateway_api_test.go
@@ -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"
@@ -352,3 +354,40 @@ func TestGatewayAPIRouter_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 TestGatewayAPIRouter_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 := &GatewayAPIRouter{
+		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)
+	}
+}
diff --git a/pkg/router/gateway_api_v1beta1.go b/pkg/router/gateway_api_v1beta1.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"slices"
 	"strings"
 
 	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
@@ -608,6 +609,18 @@ func (gwr *GatewayAPIV1Beta1Router) mergeMatchConditions(analysis, service []v1b
 	return merged
 }
 
+func sortFiltersV1beta1(headers []v1beta1.HTTPHeader) {
+
+	if headers != nil {
+		slices.SortFunc(headers, func(a, b v1beta1.HTTPHeader) int {
+			if a.Name == b.Name {
+				return strings.Compare(a.Value, b.Value)
+			}
+			return strings.Compare(string(a.Name), string(b.Name))
+		})
+	}
+}
+
 func (gwr *GatewayAPIV1Beta1Router) makeFilters(canary *flaggerv1.Canary) []v1beta1.HTTPRouteFilter {
 	var filters []v1beta1.HTTPRouteFilter
 
@@ -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)
@@ -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)

diff --git a/pkg/router/gateway_api_v1beta1_test.go b/pkg/router/gateway_api_v1beta1_test.go
@@ -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"
@@ -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)
+	}
+}