From bd4953e291b2599ed0728a84b88ca4eb8f34a7b7 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Mon, 14 Aug 2023 14:05:06 +0100 Subject: [PATCH 1/3] Add websocket support --- internal/mode/static/nginx/config/maps_template.go | 7 +++++++ internal/mode/static/nginx/config/maps_test.go | 1 + internal/mode/static/nginx/config/servers_template.go | 2 ++ internal/mode/static/nginx/config/validation/common.go | 8 ++++---- .../mode/static/nginx/config/validation/common_test.go | 2 ++ .../static/nginx/config/validation/http_filters_test.go | 2 +- .../mode/static/state/dataplane/configuration_test.go | 4 ++-- internal/mode/static/state/graph/httproute_test.go | 2 +- 8 files changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index ebbbbe3ff8..9233b0e403 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -17,4 +17,11 @@ map $http_host $gw_api_compliant_host { '' $host; default $http_host; } + +# Set $connection_header variable to upgrade when the $http_upgrade header is set, otherwise, set it to close. This +# allows support for websocket connections. See https://nginx.org/en/docs/http/websocket.html. +map $http_upgrade $connection_upgrade { + default upgrade; + '' close; +} ` diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 3e7529655f..4c64c6781a 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -85,6 +85,7 @@ func TestExecuteMaps(t *testing.T) { "~.* ${http_my_second_add_header},;": 1, "map ${http_my_set_header} $my_set_header_header_var {": 0, "map $http_host $gw_api_compliant_host {": 1, + "map $http_upgrade $connection_upgrade {": 1, } maps := string(executeMaps(conf)) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 8691823bd3..486c381db1 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -51,6 +51,8 @@ server { proxy_set_header {{ $h.Name }} "{{ $h.Value }}"; {{- end }} proxy_set_header Host $gw_api_compliant_host; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; proxy_pass {{ $l.ProxyPass }}$request_uri; {{- end }} } diff --git a/internal/mode/static/nginx/config/validation/common.go b/internal/mode/static/nginx/config/validation/common.go index ca2b04d97a..f13c3e2232 100644 --- a/internal/mode/static/nginx/config/validation/common.go +++ b/internal/mode/static/nginx/config/validation/common.go @@ -52,8 +52,8 @@ func validateEscapedStringNoVarExpansion(value string, examples []string) error } const ( - invalidHostHeaderErrMsg string = "redefining the Host request header is not supported" - maxHeaderLength int = 256 + invalidHeadersErrMsg string = "redefining the Host, Connection, or Upgrade request headers is not supported" + maxHeaderLength int = 256 ) func validateHeaderName(name string) error { @@ -63,8 +63,8 @@ func validateHeaderName(name string) error { if msg := k8svalidation.IsHTTPHeaderName(name); msg != nil { return errors.New(msg[0]) } - if strings.ToLower(name) == "host" { - return errors.New(invalidHostHeaderErrMsg) + if strings.ToLower(name) == "host" || strings.ToLower(name) == "connection" || strings.ToLower(name) == "upgrade" { + return errors.New(invalidHeadersErrMsg) } return nil } diff --git a/internal/mode/static/nginx/config/validation/common_test.go b/internal/mode/static/nginx/config/validation/common_test.go index 9d8525b797..f7c2f3be74 100644 --- a/internal/mode/static/nginx/config/validation/common_test.go +++ b/internal/mode/static/nginx/config/validation/common_test.go @@ -64,6 +64,8 @@ func TestValidateValidHeaderName(t *testing.T) { `$test`, "Host", "host", + "connection", + "upgrade", "my-header[]", "my-header&", strings.Repeat("very-long-header", 16)+"1", diff --git a/internal/mode/static/nginx/config/validation/http_filters_test.go b/internal/mode/static/nginx/config/validation/http_filters_test.go index a3a14a2652..9ebc269469 100644 --- a/internal/mode/static/nginx/config/validation/http_filters_test.go +++ b/internal/mode/static/nginx/config/validation/http_filters_test.go @@ -74,7 +74,7 @@ func TestValidateRequestHeaderName(t *testing.T) { t, validator.ValidateRequestHeaderName, "Content-Encoding", - "Connection", + "MyBespokeHeader", ) testInvalidValuesForSimpleValidator(t, validator.ValidateRequestHeaderName, "$Content-Encoding") diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 0a4b04951c..94a934996f 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -1609,8 +1609,8 @@ func TestCreateFilters(t *testing.T) { RequestHeaderModifier: &v1beta1.HTTPHeaderFilter{ Set: []v1beta1.HTTPHeader{ { - Name: "Connection", - Value: "close", + Name: "MyBespokeHeader", + Value: "my-value", }, }, }, diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go index 715148d564..b356348fc3 100644 --- a/internal/mode/static/state/graph/httproute_test.go +++ b/internal/mode/static/state/graph/httproute_test.go @@ -1836,7 +1836,7 @@ func TestValidateFilterRequestHeaderModifier(t *testing.T) { Type: v1beta1.HTTPRouteFilterRequestHeaderModifier, RequestHeaderModifier: &v1beta1.HTTPHeaderFilter{ Set: []v1beta1.HTTPHeader{ - {Name: "Connection", Value: "close"}, + {Name: "MyBespokeHeader", Value: "my-value"}, }, Add: []v1beta1.HTTPHeader{ {Name: "Accept-Encoding", Value: "gzip"}, From a7fc0fcb0d27983697feae9b71e94479b6a7c0d5 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Mon, 14 Aug 2023 14:21:00 +0100 Subject: [PATCH 2/3] Ignore resource not found error in conformance cleanup make command --- conformance/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance/Makefile b/conformance/Makefile index 4f929dbbb7..d081f454ba 100644 --- a/conformance/Makefile +++ b/conformance/Makefile @@ -89,7 +89,7 @@ uninstall-nkg: uninstall-k8s-components undo-manifests-update ## Uninstall NKG o .PHONY: uninstall-k8s-components uninstall-k8s-components: ## Uninstall installed components on configured kind cluster - kubectl delete -f $(NKG_MANIFEST) + -kubectl delete -f $(NKG_MANIFEST) ./scripts/uninstall-gateway.sh $(GW_API_VERSION) kubectl delete clusterrole nginx-gateway-provisioner kubectl delete clusterrolebinding nginx-gateway-provisioner From f5b988f7e88b6973c839f7d75baa959e70215395 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Tue, 15 Aug 2023 12:15:09 +0100 Subject: [PATCH 3/3] Review feedback --- .../static/nginx/config/servers_template.go | 1 + .../static/nginx/config/validation/common.go | 12 ++++++-- .../nginx/config/validation/framework.go | 10 +++++++ .../nginx/config/validation/framework_test.go | 28 +++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 486c381db1..2a62a4fab7 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -51,6 +51,7 @@ server { proxy_set_header {{ $h.Name }} "{{ $h.Value }}"; {{- end }} proxy_set_header Host $gw_api_compliant_host; + proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection $connection_upgrade; proxy_pass {{ $l.ProxyPass }}$request_uri; diff --git a/internal/mode/static/nginx/config/validation/common.go b/internal/mode/static/nginx/config/validation/common.go index f13c3e2232..fc98931da6 100644 --- a/internal/mode/static/nginx/config/validation/common.go +++ b/internal/mode/static/nginx/config/validation/common.go @@ -52,10 +52,16 @@ func validateEscapedStringNoVarExpansion(value string, examples []string) error } const ( - invalidHeadersErrMsg string = "redefining the Host, Connection, or Upgrade request headers is not supported" + invalidHeadersErrMsg string = "unsupported header name configured, unsupported names are: " maxHeaderLength int = 256 ) +var invalidHeaders = map[string]struct{}{ + "host": {}, + "connection": {}, + "upgrade": {}, +} + func validateHeaderName(name string) error { if len(name) > maxHeaderLength { return errors.New(k8svalidation.MaxLenError(maxHeaderLength)) @@ -63,8 +69,8 @@ func validateHeaderName(name string) error { if msg := k8svalidation.IsHTTPHeaderName(name); msg != nil { return errors.New(msg[0]) } - if strings.ToLower(name) == "host" || strings.ToLower(name) == "connection" || strings.ToLower(name) == "upgrade" { - return errors.New(invalidHeadersErrMsg) + if valid, invalidHeadersAsStrings := validateNoUnsupportedValues(strings.ToLower(name), invalidHeaders); !valid { + return errors.New(invalidHeadersErrMsg + strings.Join(invalidHeadersAsStrings, ", ")) } return nil } diff --git a/internal/mode/static/nginx/config/validation/framework.go b/internal/mode/static/nginx/config/validation/framework.go index 4b91938bf2..ec77d827f3 100644 --- a/internal/mode/static/nginx/config/validation/framework.go +++ b/internal/mode/static/nginx/config/validation/framework.go @@ -20,6 +20,16 @@ func validateInSupportedValues[T configValue]( return false, getSortedKeysAsString(supportedValues) } +func validateNoUnsupportedValues[T configValue]( + value T, + unsupportedValues map[T]struct{}, +) (valid bool, unsupportedValuesAsStrings []string) { + if _, exist := unsupportedValues[value]; exist { + return false, getSortedKeysAsString(unsupportedValues) + } + return true, nil +} + func getSortedKeysAsString[T configValue](m map[T]struct{}) []string { keysAsString := make([]string, 0, len(m)) diff --git a/internal/mode/static/nginx/config/validation/framework_test.go b/internal/mode/static/nginx/config/validation/framework_test.go index 412deaa04c..d23cbe0f71 100644 --- a/internal/mode/static/nginx/config/validation/framework_test.go +++ b/internal/mode/static/nginx/config/validation/framework_test.go @@ -99,6 +99,34 @@ func TestValidateInSupportedValues(t *testing.T) { ) } +func TestValidateNoUnsupportedValues(t *testing.T) { + unsupportedValues := map[string]struct{}{ + "badvalue1": {}, + "badvalue2": {}, + "badvalue3": {}, + } + + validator := func(value string) (bool, []string) { + return validateNoUnsupportedValues(value, unsupportedValues) + } + + testValidValuesForSupportedValuesValidator( + t, + validator, + "value1", + "value2", + "value3", + ) + testInvalidValuesForSupportedValuesValidator( + t, + validator, + unsupportedValues, + "badvalue1", + "badvalue2", + "badvalue3", + ) +} + func TestGetSortedKeysAsString(t *testing.T) { values := map[string]struct{}{ "value3": {},