Skip to content

Commit

Permalink
Stop setting empty header modifier in Consul when not present in HTTP…
Browse files Browse the repository at this point in the history 
…Route
  • Loading branch information
@nathancoleman
nathancoleman committed Sep 11, 2023
1 parent 30f5c47 commit 66e302b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 56 deletions.
68 changes: 43 additions & 25 deletions control-plane/api-gateway/common/translation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,46 +279,64 @@ func (t ResourceTranslator) translateHTTPQueryMatch(match gwv1beta1.HTTPQueryPar
}

func (t ResourceTranslator) translateHTTPFilters(filters []gwv1beta1.HTTPRouteFilter, resourceMap *ResourceMap, namespace string) (api.HTTPFilters, api.HTTPResponseFilters) {
var urlRewrite *api.URLRewrite
// TODO Consider only setting a filter on the resulting Consul config entry
// if we encounter a gwb1beta1.HTTPRouteFilter that requires it
consulFilter := api.HTTPHeaderFilter{
Add: make(map[string]string),
Set: make(map[string]string),
}
consulResponseFilter := api.HTTPHeaderFilter{
Add: make(map[string]string),
Set: make(map[string]string),
}
var retryFilter *api.RetryFilter
var timeoutFilter *api.TimeoutFilter
var (
urlRewrite *api.URLRewrite
retryFilter *api.RetryFilter
timeoutFilter *api.TimeoutFilter
requestHeaderFilters = []api.HTTPHeaderFilter{}
responseHeaderFilters = []api.HTTPHeaderFilter{}
)

// Convert Gateway API filters to portions of the Consul request and response filters.
// Multiple filters applying the same or conflicting operations are allowed but may
// result in unexpected behavior.
for _, filter := range filters {
if filter.RequestHeaderModifier != nil {
consulFilter.Remove = append(consulFilter.Remove, filter.RequestHeaderModifier.Remove...)
newFilter := api.HTTPHeaderFilter{}

if len(filter.RequestHeaderModifier.Remove) > 0 {
newFilter.Remove = append(newFilter.Remove, filter.RequestHeaderModifier.Remove...)
}

for _, toAdd := range filter.RequestHeaderModifier.Add {
consulFilter.Add[string(toAdd.Name)] = toAdd.Value
if len(filter.RequestHeaderModifier.Add) > 0 {
newFilter.Add = map[string]string{}
for _, toAdd := range filter.RequestHeaderModifier.Add {
newFilter.Add[string(toAdd.Name)] = toAdd.Value
}
}

for _, toSet := range filter.RequestHeaderModifier.Set {
consulFilter.Set[string(toSet.Name)] = toSet.Value
if len(filter.RequestHeaderModifier.Set) > 0 {
newFilter.Set = map[string]string{}
for _, toSet := range filter.RequestHeaderModifier.Set {
newFilter.Set[string(toSet.Name)] = toSet.Value
}
}

requestHeaderFilters = append(requestHeaderFilters, newFilter)
}

if filter.ResponseHeaderModifier != nil {
consulResponseFilter.Remove = append(consulResponseFilter.Remove, filter.ResponseHeaderModifier.Remove...)
newFilter := api.HTTPHeaderFilter{}

if len(filter.ResponseHeaderModifier.Remove) > 0 {
newFilter.Remove = append(newFilter.Remove, filter.ResponseHeaderModifier.Remove...)
}

for _, toAdd := range filter.ResponseHeaderModifier.Add {
consulResponseFilter.Add[string(toAdd.Name)] = toAdd.Value
if len(filter.ResponseHeaderModifier.Add) > 0 {
newFilter.Add = map[string]string{}
for _, toAdd := range filter.ResponseHeaderModifier.Add {
newFilter.Add[string(toAdd.Name)] = toAdd.Value
}
}

for _, toSet := range filter.ResponseHeaderModifier.Set {
consulResponseFilter.Set[string(toSet.Name)] = toSet.Value
if len(filter.ResponseHeaderModifier.Set) > 0 {
newFilter.Set = map[string]string{}
for _, toSet := range filter.ResponseHeaderModifier.Set {
newFilter.Set[string(toSet.Name)] = toSet.Value
}
}

responseHeaderFilters = append(responseHeaderFilters, newFilter)
}

// we drop any path rewrites that are not prefix matches as we don't support those
Expand Down Expand Up @@ -364,14 +382,14 @@ func (t ResourceTranslator) translateHTTPFilters(filters []gwv1beta1.HTTPRouteFi
}

requestFilter := api.HTTPFilters{
Headers: []api.HTTPHeaderFilter{consulFilter},
Headers: requestHeaderFilters,
URLRewrite: urlRewrite,
RetryFilter: retryFilter,
TimeoutFilter: timeoutFilter,
}

responseFilter := api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{consulResponseFilter},
Headers: responseHeaderFilters,
}

return requestFilter, responseFilter
Expand Down
48 changes: 17 additions & 31 deletions control-plane/api-gateway/common/translation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
URLRewrite: &api.URLRewrite{Path: "v1"},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
},
ResponseFilters: api.HTTPResponseFilters{Headers: []api.HTTPHeaderFilter{}},
Matches: []api.HTTPMatch{
{
Headers: []api.HTTPHeaderMatch{
Expand Down Expand Up @@ -558,10 +556,8 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
Path: "path",
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
},
Weight: 45,
ResponseFilters: api.HTTPResponseFilters{Headers: []api.HTTPHeaderFilter{}},
Weight: 45,
},
},
},
Expand Down Expand Up @@ -722,7 +718,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Matches: []api.HTTPMatch{
{
Expand Down Expand Up @@ -769,7 +765,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Weight: 45,
},
Expand Down Expand Up @@ -940,7 +936,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
URLRewrite: &api.URLRewrite{Path: "v1"},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Matches: []api.HTTPMatch{
{
Expand Down Expand Up @@ -987,7 +983,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Weight: 45,
},
Expand Down Expand Up @@ -1172,7 +1168,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
URLRewrite: &api.URLRewrite{Path: "v1"},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Matches: []api.HTTPMatch{
{
Expand Down Expand Up @@ -1202,9 +1198,9 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
Name: "some-override",
Namespace: "svc-ns",
Weight: 1,
Filters: api.HTTPFilters{Headers: []api.HTTPHeaderFilter{{Add: make(map[string]string), Set: make(map[string]string)}}},
Filters: api.HTTPFilters{Headers: []api.HTTPHeaderFilter{}},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
}},
{
Name: "service one",
Expand All @@ -1227,7 +1223,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Weight: 45,
},
Expand Down Expand Up @@ -1371,7 +1367,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
Rules: []api.HTTPRouteRule{
{
Filters: api.HTTPFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
URLRewrite: nil,
RetryFilter: &api.RetryFilter{
NumRetries: pointer.Uint32(3),
Expand All @@ -1381,7 +1377,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Matches: []api.HTTPMatch{
{
Expand Down Expand Up @@ -1411,7 +1407,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
Name: "service one",
Weight: 45,
Filters: api.HTTPFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
RetryFilter: &api.RetryFilter{
NumRetries: pointer.Uint32(3),
RetryOn: []string{"cancelled"},
Expand All @@ -1420,7 +1416,7 @@ func TestTranslator_ToHTTPRoute(t *testing.T) {
},
},
ResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{{Add: map[string]string{}, Set: map[string]string{}}},
Headers: []api.HTTPHeaderFilter{},
},
Namespace: "other",
},
Expand Down Expand Up @@ -1657,21 +1653,11 @@ func TestResourceTranslator_translateHTTPFilters(t1 *testing.T) {
},
},
want: api.HTTPFilters{
Headers: []api.HTTPHeaderFilter{
{
Add: map[string]string{},
Set: map[string]string{},
},
},
Headers: []api.HTTPHeaderFilter{},
URLRewrite: nil,
},
wantResponseFilters: api.HTTPResponseFilters{
Headers: []api.HTTPHeaderFilter{
{
Add: map[string]string{},
Set: map[string]string{},
},
},
Headers: []api.HTTPHeaderFilter{},
},
},
}
Expand Down

0 comments on commit 66e302b

Please sign in to comment.