diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 27c4a93ead..6636ff5460 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -617,7 +617,7 @@ func (t *Translator) processResponseHeaderModifierFilter( newHeader := ir.AddHeader{ Name: headerKey, Append: true, - Value: strings.Split(addHeader.Value, ","), + Value: []string{addHeader.Value}, } filterContext.AddResponseHeaders = append(filterContext.AddResponseHeaders, newHeader) @@ -672,7 +672,7 @@ func (t *Translator) processResponseHeaderModifierFilter( newHeader := ir.AddHeader{ Name: string(setHeader.Name), Append: false, - Value: strings.Split(setHeader.Value, ","), + Value: []string{setHeader.Value}, } filterContext.AddResponseHeaders = append(filterContext.AddResponseHeaders, newHeader) diff --git a/internal/gatewayapi/filters_test.go b/internal/gatewayapi/filters_test.go new file mode 100644 index 0000000000..f677e7822d --- /dev/null +++ b/internal/gatewayapi/filters_test.go @@ -0,0 +1,100 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package gatewayapi + +import ( + "reflect" + "testing" + + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/envoyproxy/gateway/internal/ir" +) + +func TestProcessResponseHeaderModifierFilter(t *testing.T) { + tests := []struct { + name string + filter *gwapiv1.HTTPHeaderFilter + expectedAdd []ir.AddHeader + expectedRemove []string + }{ + { + name: "Add headers", + filter: &gwapiv1.HTTPHeaderFilter{ + Add: []gwapiv1.HTTPHeader{ + {Name: "X-Test-Add", Value: "foo,bar"}, + }, + }, + expectedAdd: []ir.AddHeader{ + {Name: "X-Test-Add", Append: true, Value: []string{"foo,bar"}}, + }, + expectedRemove: nil, + }, + { + name: "Set headers", + filter: &gwapiv1.HTTPHeaderFilter{ + Set: []gwapiv1.HTTPHeader{ + {Name: "X-Test-Set", Value: "baz,qux"}, + }, + }, + expectedAdd: []ir.AddHeader{ + {Name: "X-Test-Set", Append: false, Value: []string{"baz,qux"}}, + }, + expectedRemove: nil, + }, + { + name: "Remove headers", + filter: &gwapiv1.HTTPHeaderFilter{ + Remove: []string{"X-Test-Remove", "X-Test-Remove2"}, + }, + expectedAdd: nil, + expectedRemove: []string{ + "X-Test-Remove", "X-Test-Remove2", + }, + }, + { + name: "Combined Add, Set, Remove", + filter: &gwapiv1.HTTPHeaderFilter{ + Add: []gwapiv1.HTTPHeader{ + {Name: "X-Add", Value: "val1,val2"}, + }, + Set: []gwapiv1.HTTPHeader{ + {Name: "X-Set", Value: "val3"}, + }, + Remove: []string{"X-Remove"}, + }, + expectedAdd: []ir.AddHeader{ + {Name: "X-Add", Append: true, Value: []string{"val1,val2"}}, + {Name: "X-Set", Append: false, Value: []string{"val3"}}, + }, + expectedRemove: []string{"X-Remove"}, + }, + { + name: "Empty config", + filter: &gwapiv1.HTTPHeaderFilter{}, + expectedAdd: nil, + expectedRemove: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filterCtx := &HTTPFiltersContext{ + HTTPFilterIR: &HTTPFilterIR{}, + } + + translator := &Translator{} + translator.processResponseHeaderModifierFilter(tt.filter, filterCtx) + + if !reflect.DeepEqual(filterCtx.AddResponseHeaders, tt.expectedAdd) { + t.Errorf("unexpected AddResponseHeaders: got=%v, want=%v", filterCtx.AddResponseHeaders, tt.expectedAdd) + } + if !reflect.DeepEqual(filterCtx.RemoveResponseHeaders, tt.expectedRemove) { + t.Errorf("unexpected RemoveResponseHeaders: got=%v, want=%v", filterCtx.RemoveResponseHeaders, tt.expectedRemove) + } + }) + } +} diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index 2d61f73327..97f42c8858 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -522,6 +522,10 @@ func mirrorPercentByPolicy(mirror *ir.MirrorPolicy) *corev3.RuntimeFractionalPer } func buildXdsAddedHeaders(headersToAdd []ir.AddHeader) []*corev3.HeaderValueOption { + if len(headersToAdd) == 0 { + return nil + } + headerValueOptions := []*corev3.HeaderValueOption{} for _, header := range headersToAdd { diff --git a/internal/xds/translator/route_test.go b/internal/xds/translator/route_test.go index 817426c97b..7dbff0a979 100644 --- a/internal/xds/translator/route_test.go +++ b/internal/xds/translator/route_test.go @@ -9,6 +9,7 @@ import ( "reflect" "testing" + corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" "k8s.io/utils/ptr" @@ -171,3 +172,110 @@ func TestBuildUpgradeConfig(t *testing.T) { }) } } + +func TestBuildXdsAddedHeaders(t *testing.T) { + tests := []struct { + name string + headersToAdd []ir.AddHeader + want []*corev3.HeaderValueOption + }{ + { + name: "No headers", + headersToAdd: nil, + want: nil, + }, + { + name: "Single header, no value (should keep empty value)", + headersToAdd: []ir.AddHeader{ + {Name: "X-Test-Empty", Value: []string{}, Append: false}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "X-Test-Empty"}, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + KeepEmptyValue: true, + }, + }, + }, + { + name: "Single header, one value, overwrite", + headersToAdd: []ir.AddHeader{ + {Name: "X-Test", Value: []string{"foo"}, Append: false}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "X-Test", Value: "foo"}, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + KeepEmptyValue: false, + }, + }, + }, + { + name: "Single header, one value, append", + headersToAdd: []ir.AddHeader{ + {Name: "X-Test", Value: []string{"foo"}, Append: true}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "X-Test", Value: "foo"}, + AppendAction: corev3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD, + KeepEmptyValue: false, + }, + }, + }, + { + name: "Header with multiple values (should join with comma)", + headersToAdd: []ir.AddHeader{ + {Name: "Cache-Control", Value: []string{"private,no-store"}, Append: false}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "Cache-Control", Value: "private,no-store"}, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + KeepEmptyValue: false, + }, + }, + }, + { + name: "Multiple headers", + headersToAdd: []ir.AddHeader{ + {Name: "X-First", Value: []string{"foo"}, Append: false}, + {Name: "X-Second", Value: []string{"bar,baz"}, Append: true}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "X-First", Value: "foo"}, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + KeepEmptyValue: false, + }, + { + Header: &corev3.HeaderValue{Key: "X-Second", Value: "bar,baz"}, + AppendAction: corev3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD, + KeepEmptyValue: false, + }, + }, + }, + { + name: "Header with explicit empty value string", + headersToAdd: []ir.AddHeader{ + {Name: "X-Explicit-Empty", Value: []string{""}, Append: false}, + }, + want: []*corev3.HeaderValueOption{ + { + Header: &corev3.HeaderValue{Key: "X-Explicit-Empty", Value: ""}, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + KeepEmptyValue: true, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildXdsAddedHeaders(tt.headersToAdd) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("buildXdsAddedHeaders() = %v, want %v", got, tt.want) + } + }) + } +}