From 52fab05f8455ad9d9c0f08a11c0dd6e992b5a797 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 16 Mar 2023 16:46:11 +0000 Subject: [PATCH] Add NKG-specific field validation for HTTPRoutes (#455) * Add NKG-specific field validation for HTTPRoutes - Introduce HTTPFieldsValidator interface for validating fields of HTTP-related Gateway API resources according to the data-plane specific rules. - Validate HTTPRoute resources when building the graph using data-plane agnostic rules. - Validate HTTPRoute resources when building the graph using HTTPFieldsValidator according to the data-plane rules. - Implement an HTTPFieldsValidator for NGINX-specific validation rules. Fixes https://github.com/nginxinc/nginx-kubernetes-gateway/issues/412 * Apply suggestions on GitHub Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/helpers/helpers.go | 9 +- internal/manager/manager.go | 11 +- internal/nginx/config/generator.go | 4 + internal/nginx/config/http/config.go | 4 +- internal/nginx/config/servers.go | 27 +- internal/nginx/config/servers_template.go | 2 +- internal/nginx/config/servers_test.go | 47 +- internal/nginx/config/validation/common.go | 48 + .../nginx/config/validation/common_test.go | 32 + internal/nginx/config/validation/doc.go | 15 + internal/nginx/config/validation/framework.go | 33 + .../nginx/config/validation/framework_test.go | 98 ++ .../nginx/config/validation/http_filters.go | 41 + .../config/validation/http_filters_test.go | 46 + .../nginx/config/validation/http_njs_match.go | 109 ++ .../config/validation/http_njs_match_test.go | 85 + .../nginx/config/validation/http_validator.go | 22 + .../config/validation/http_validator_test.go | 14 + internal/state/change_processor.go | 13 +- internal/state/change_processor_test.go | 14 + internal/state/conditions/conditions.go | 61 +- internal/state/dataplane/configuration.go | 31 +- .../state/dataplane/configuration_test.go | 373 ++-- internal/state/graph/backend_group.go | 1 - internal/state/graph/backend_refs.go | 195 ++- internal/state/graph/backend_refs_test.go | 607 ++++--- internal/state/graph/doc.go | 7 + internal/state/graph/gateway.go | 76 +- internal/state/graph/gateway_test.go | 592 ++++--- internal/state/graph/graph.go | 27 +- internal/state/graph/graph_test.go | 125 +- internal/state/graph/httproute.go | 606 ++++++- internal/state/graph/httproute_test.go | 1541 ++++++++++++++--- internal/state/graph/validation.go | 26 + internal/state/graph/validation_test.go | 50 + internal/state/resolver/resolver.go | 3 +- internal/state/statuses.go | 29 +- internal/state/statuses_test.go | 60 +- .../fake_httpfields_validator.go | 866 +++++++++ internal/state/validation/validator.go | 24 + 40 files changed, 4760 insertions(+), 1214 deletions(-) create mode 100644 internal/nginx/config/validation/common.go create mode 100644 internal/nginx/config/validation/common_test.go create mode 100644 internal/nginx/config/validation/doc.go create mode 100644 internal/nginx/config/validation/framework.go create mode 100644 internal/nginx/config/validation/framework_test.go create mode 100644 internal/nginx/config/validation/http_filters.go create mode 100644 internal/nginx/config/validation/http_filters_test.go create mode 100644 internal/nginx/config/validation/http_njs_match.go create mode 100644 internal/nginx/config/validation/http_njs_match_test.go create mode 100644 internal/nginx/config/validation/http_validator.go create mode 100644 internal/nginx/config/validation/http_validator_test.go create mode 100644 internal/state/graph/validation.go create mode 100644 internal/state/graph/validation_test.go create mode 100644 internal/state/validation/validationfakes/fake_httpfields_validator.go create mode 100644 internal/state/validation/validator.go diff --git a/internal/helpers/helpers.go b/internal/helpers/helpers.go index 48c70017da..1a243950e5 100644 --- a/internal/helpers/helpers.go +++ b/internal/helpers/helpers.go @@ -9,8 +9,8 @@ import ( // Diff prints the diff between two structs. // It is useful in testing to compare two structs when they are large. In such a case, without Diff it will be difficult // to pinpoint the difference between the two structs. -func Diff(x, y interface{}) string { - r := cmp.Diff(x, y) +func Diff(want, got any) string { + r := cmp.Diff(want, got) if r != "" { return "(-want +got)\n" + r @@ -57,3 +57,8 @@ func GetTLSModePointer(t v1beta1.TLSModeType) *v1beta1.TLSModeType { func GetBoolPointer(b bool) *bool { return &b } + +// GetPointer takes a value of any type and returns a pointer to it. +func GetPointer[T any](v T) *T { + return &v +} diff --git a/internal/manager/manager.go b/internal/manager/manager.go index ec7e79d042..991287c8de 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -14,7 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" - "sigs.k8s.io/gateway-api/apis/v1beta1/validation" + gwapivalidation "sigs.k8s.io/gateway-api/apis/v1beta1/validation" "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" @@ -22,12 +22,14 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate" ngxcfg "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" + ngxvalidation "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/validation" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file" ngxruntime "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" ) @@ -80,13 +82,13 @@ func Start(cfg config.Config) error { { objectType: &gatewayv1beta1.Gateway{}, options: []controllerOption{ - withWebhookValidator(createValidator(validation.ValidateGateway)), + withWebhookValidator(createValidator(gwapivalidation.ValidateGateway)), }, }, { objectType: &gatewayv1beta1.HTTPRoute{}, options: []controllerOption{ - withWebhookValidator(createValidator(validation.ValidateHTTPRoute)), + withWebhookValidator(createValidator(gwapivalidation.ValidateHTTPRoute)), }, }, { @@ -129,6 +131,9 @@ func Start(cfg config.Config) error { ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), RelationshipCapturer: relationship.NewCapturerImpl(), Logger: cfg.Logger.WithName("changeProcessor"), + Validators: validation.Validators{ + HTTPFieldsValidator: ngxvalidation.HTTPValidator{}, + }, }) configGenerator := ngxcfg.NewGeneratorImpl() diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go index 59403d24f7..0e756d36dc 100644 --- a/internal/nginx/config/generator.go +++ b/internal/nginx/config/generator.go @@ -24,6 +24,10 @@ func NewGeneratorImpl() GeneratorImpl { // executeFunc is a function that generates NGINX configuration from internal representation. type executeFunc func(configuration dataplane.Configuration) []byte +// Generate generates NGINX configuration from internal representation. +// It is the responsibility of the caller to validate the configuration before calling this function. +// In case of invalid configuration, NGINX will fail to reload or could be configured with malicious configuration. +// To validate, use the validators from the validation package. func (g GeneratorImpl) Generate(conf dataplane.Configuration) []byte { var generated []byte for _, execute := range getExecuteFuncs() { diff --git a/internal/nginx/config/http/config.go b/internal/nginx/config/http/config.go index 9e4088d5a2..f799bfb345 100644 --- a/internal/nginx/config/http/config.go +++ b/internal/nginx/config/http/config.go @@ -20,7 +20,7 @@ type Location struct { // Return represents an HTTP return. type Return struct { - URL string + Body string Code StatusCode } @@ -38,6 +38,8 @@ const ( StatusFound StatusCode = 302 // StatusNotFound is the HTTP 404 status code. StatusNotFound StatusCode = 404 + // StatusInternalServerError is the HTTP 500 status code. + StatusInternalServerError StatusCode = 500 ) // Upstream holds all configuration for an HTTP upstream. diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 1771a7d0c7..513ffe6d8c 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -14,7 +14,11 @@ import ( var serversTemplate = gotemplate.Must(gotemplate.New("servers").Parse(serversTemplateText)) -const rootPath = "/" +const ( + // HeaderMatchSeparator is the separator for constructing header-based match for NJS. + HeaderMatchSeparator = ":" + rootPath = "/" +) func executeServers(conf dataplane.Configuration) []byte { servers := createServers(conf.HTTPServers, conf.SSLServers) @@ -106,16 +110,19 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo matches = append(matches, createHTTPMatch(m, path)) } - // FIXME(pleshakov): There could be a case when the filter has the type set but not the corresponding field. + if r.Filters.InvalidFilter != nil { + loc.Return = &http.Return{Code: http.StatusInternalServerError} + locs = append(locs, loc) + continue + } + + // There could be a case when the filter has the type set but not the corresponding field. // For example, type is v1beta1.HTTPRouteFilterRequestRedirect, but RequestRedirect field is nil. - // The validation webhook catches that. - // If it doesn't work as expected, such situation is silently handled below in findFirstFilters. - // Consider reporting an error. But that should be done in a separate validation layer. + // The imported Webhook validation webhook catches that. // RequestRedirect and proxying are mutually exclusive. if r.Filters.RequestRedirect != nil { loc.Return = createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) - locs = append(locs, loc) continue } @@ -172,9 +179,6 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, hostname = string(*filter.Hostname) } - // FIXME(pleshakov): Unknown values here must result in the implementation setting the Attached Condition for - // the Route to `status: False`, with a Reason of `UnsupportedValue`. In that case, all routes of the Route will be - // ignored. NGINX will return 500. This should be implemented in the validation layer. code := http.StatusFound if filter.StatusCode != nil { code = http.StatusCode(*filter.StatusCode) @@ -185,7 +189,6 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, port = int(*filter.Port) } - // FIXME(pleshakov): Same as the FIXME about StatusCode above. scheme := "$scheme" if filter.Scheme != nil { scheme = *filter.Scheme @@ -193,7 +196,7 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, return &http.Return{ Code: code, - URL: fmt.Sprintf("%s://%s:%d$request_uri", scheme, hostname, port), + Body: fmt.Sprintf("%s://%s:%d$request_uri", scheme, hostname, port), } } @@ -275,7 +278,7 @@ func createQueryParamKeyValString(p v1beta1.HTTPQueryParamMatch) string { // We preserve the case of the name here because NGINX allows us to look up the header names in a case-insensitive // manner. func createHeaderKeyValString(h v1beta1.HTTPHeaderMatch) string { - return string(h.Name) + ":" + h.Value + return string(h.Name) + HeaderMatchSeparator + h.Value } func isPathOnlyMatch(match v1beta1.HTTPRouteMatch) bool { diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index c96da61371..0f8a617129 100644 --- a/internal/nginx/config/servers_template.go +++ b/internal/nginx/config/servers_template.go @@ -36,7 +36,7 @@ server { {{ end }} {{ if $l.Return }} - return {{ $l.Return.Code }} {{ $l.Return.URL }}; + return {{ $l.Return.Code }} "{{ $l.Return.Body }}"; {{ end }} {{ if $l.HTTPMatchVar }} diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 793bb556ae..043835e194 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -271,6 +271,16 @@ func TestCreateServers(t *testing.T) { }, // redirect is set in the corresponding state.MatchRule }, + { + // A match with an invalid filter + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetPointer("/invalid-filter"), + }, + }, + }, + }, }, }, } @@ -324,6 +334,8 @@ func TestCreateServers(t *testing.T) { filterGroup2 := graph.BackendGroup{Source: hrNsName, RuleIdx: 4} + invalidFilterGroup := graph.BackendGroup{Source: hrNsName, RuleIdx: 5} + cafePathRules := []dataplane.PathRule{ { Path: "/", @@ -403,6 +415,20 @@ func TestCreateServers(t *testing.T) { }, }, }, + { + Path: "/invalid-filter", + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 5, + Source: hr, + Filters: dataplane.Filters{ + InvalidFilter: &dataplane.InvalidFilter{}, + }, + BackendGroup: invalidFilterGroup, + }, + }, + }, } httpServers := []dataplane.VirtualServer{ @@ -491,14 +517,20 @@ func TestCreateServers(t *testing.T) { Path: "/redirect-implicit-port", Return: &http.Return{ Code: 302, - URL: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), + Body: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), }, }, { Path: "/redirect-explicit-port", Return: &http.Return{ Code: 302, - URL: "$scheme://bar.example.com:8080$request_uri", + Body: "$scheme://bar.example.com:8080$request_uri", + }, + }, + { + Path: "/invalid-filter", + Return: &http.Return{ + Code: http.StatusInternalServerError, }, }, } @@ -522,11 +554,10 @@ func TestCreateServers(t *testing.T) { }, } - result := createServers(httpServers, sslServers) + g := NewGomegaWithT(t) - if diff := cmp.Diff(expectedServers, result); diff != "" { - t.Errorf("createServers() mismatch (-want +got):\n%s", diff) - } + result := createServers(httpServers, sslServers) + g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) } func TestCreateLocationsRootPath(t *testing.T) { @@ -713,7 +744,7 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { filter: &v1beta1.HTTPRequestRedirectFilter{}, expected: &http.Return{ Code: http.StatusFound, - URL: "$scheme://$host:123$request_uri", + Body: "$scheme://$host:123$request_uri", }, msg: "all fields are empty", }, @@ -726,7 +757,7 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { }, expected: &http.Return{ Code: 101, - URL: "https://foo.example.com:2022$request_uri", + Body: "https://foo.example.com:2022$request_uri", }, msg: "all fields are set", }, diff --git a/internal/nginx/config/validation/common.go b/internal/nginx/config/validation/common.go new file mode 100644 index 0000000000..dd409bc474 --- /dev/null +++ b/internal/nginx/config/validation/common.go @@ -0,0 +1,48 @@ +package validation + +import ( + "errors" + "regexp" + + k8svalidation "k8s.io/apimachinery/pkg/util/validation" +) + +const ( + escapedStringsFmt = `([^"\\]|\\.)*` + escapedStringsErrMsg = `must have all '"' (double quotes) escaped and must not end with an unescaped '\' ` + + `(backslash)` +) + +var escapedStringsFmtRegexp = regexp.MustCompile("^" + escapedStringsFmt + "$") + +// validateEscapedString is used to validate a string that is surrounded by " in the NGINX config for a directive +// that doesn't support any regex rules or variables (it doesn't try to expand the variable name behind $). +// For example, server_name "hello $not_a_var world" +// If the value is invalid, the function returns an error that includes the specified examples of valid values. +func validateEscapedString(value string, examples []string) error { + if !escapedStringsFmtRegexp.MatchString(value) { + msg := k8svalidation.RegexError(escapedStringsErrMsg, escapedStringsFmt, examples...) + return errors.New(msg) + } + return nil +} + +const ( + escapedStringsNoVarExpansionFmt = `([^"$\\]|\\[^$])*` + escapedStringsNoVarExpansionErrMsg string = `a valid header must have all '"' escaped and must not contain any ` + + `'$' or end with an unescaped '\'` +) + +var escapedStringsNoVarExpansionFmtRegexp = regexp.MustCompile("^" + escapedStringsNoVarExpansionFmt + "$") + +// validateEscapedStringNoVarExpansion is the same as validateEscapedString except it doesn't allow $ to +// prevent variable expansion. +// If the value is invalid, the function returns an error that includes the specified examples of valid values. +func validateEscapedStringNoVarExpansion(value string, examples []string) error { + if !escapedStringsNoVarExpansionFmtRegexp.MatchString(value) { + msg := k8svalidation.RegexError(escapedStringsNoVarExpansionErrMsg, escapedStringsNoVarExpansionFmt, + examples...) + return errors.New(msg) + } + return nil +} diff --git a/internal/nginx/config/validation/common_test.go b/internal/nginx/config/validation/common_test.go new file mode 100644 index 0000000000..711b316b80 --- /dev/null +++ b/internal/nginx/config/validation/common_test.go @@ -0,0 +1,32 @@ +package validation + +import ( + "testing" +) + +func TestValidateEscapedString(t *testing.T) { + validator := func(value string) error { return validateEscapedString(value, []string{"example"}) } + + testValidValuesForSimpleValidator(t, validator, + `test`, + `test test`, + `\"`, + `\\`) + testInvalidValuesForSimpleValidator(t, validator, + `\`, + `test"test`) +} + +func TestValidateEscapedStringNoVarExpansion(t *testing.T) { + validator := func(value string) error { return validateEscapedStringNoVarExpansion(value, []string{"example"}) } + + testValidValuesForSimpleValidator(t, validator, + `test`, + `test test`, + `\"`, + `\\`) + testInvalidValuesForSimpleValidator(t, validator, + `\`, + `test"test`, + `$test`) +} diff --git a/internal/nginx/config/validation/doc.go b/internal/nginx/config/validation/doc.go new file mode 100644 index 0000000000..f733288138 --- /dev/null +++ b/internal/nginx/config/validation/doc.go @@ -0,0 +1,15 @@ +/* +Package validation includes validators to validate values that will propagate to the NGINX configuration. + +The validation rules prevent two cases: +(1) Invalid values. Such values will cause NGINX to fail to reload the configuration. +(2) Malicious values. Such values will cause NGINX to succeed to reload, but will configure NGINX maliciously, outside +of the NKG capabilities. For example, configuring NGINX to serve the contents of the file system of its container. + +The validation rules are based on the types in the parent config package and how they are used in the NGINX +configuration templates. Changes to those might require changing the validation rules. + +The rules are much looser for NGINX than for the Gateway API. However, some valid Gateway API values are not valid for +NGINX. +*/ +package validation diff --git a/internal/nginx/config/validation/framework.go b/internal/nginx/config/validation/framework.go new file mode 100644 index 0000000000..4b91938bf2 --- /dev/null +++ b/internal/nginx/config/validation/framework.go @@ -0,0 +1,33 @@ +package validation + +import ( + "fmt" + "sort" +) + +type configValue interface { + int | int32 | string +} + +func validateInSupportedValues[T configValue]( + value T, + supportedValues map[T]struct{}, +) (valid bool, supportedValuesAsStrings []string) { + if _, exist := supportedValues[value]; exist { + return true, nil + } + + return false, getSortedKeysAsString(supportedValues) +} + +func getSortedKeysAsString[T configValue](m map[T]struct{}) []string { + keysAsString := make([]string, 0, len(m)) + + for k := range m { + keysAsString = append(keysAsString, fmt.Sprint(k)) + } + + sort.Strings(keysAsString) + + return keysAsString +} diff --git a/internal/nginx/config/validation/framework_test.go b/internal/nginx/config/validation/framework_test.go new file mode 100644 index 0000000000..33f935ca50 --- /dev/null +++ b/internal/nginx/config/validation/framework_test.go @@ -0,0 +1,98 @@ +package validation + +import ( + "fmt" + "testing" + + . "github.com/onsi/gomega" +) + +type simpleValidatorFunc[T configValue] func(v T) error + +type supportedValuesValidatorFunc[T configValue] func(v T) (bool, []string) + +func runValidatorTests[T configValue](t *testing.T, run func(g *WithT, v T), caseNamePrefix string, values ...T) { + for i, v := range values { + t.Run(fmt.Sprintf("%s_case_#%d", caseNamePrefix, i), func(t *testing.T) { + g := NewGomegaWithT(t) + run(g, v) + }) + } +} + +func createFailureMessage[T any](v T) string { + return fmt.Sprintf("value: %v", v) +} + +func testValidValuesForSimpleValidator[T configValue](t *testing.T, f simpleValidatorFunc[T], values ...T) { + runValidatorTests(t, func(g *WithT, v T) { + err := f(v) + g.Expect(err).ToNot(HaveOccurred(), createFailureMessage(v)) + }, "valid_value", values...) +} + +func testInvalidValuesForSimpleValidator[T configValue](t *testing.T, f simpleValidatorFunc[T], values ...T) { + runValidatorTests(t, func(g *WithT, v T) { + err := f(v) + g.Expect(err).To(HaveOccurred(), createFailureMessage(v)) + }, "invalid_value", values...) +} + +func testValidValuesForSupportedValuesValidator[T configValue]( + t *testing.T, + f supportedValuesValidatorFunc[T], + values ...T, +) { + runValidatorTests(t, func(g *WithT, v T) { + valid, supportedValues := f(v) + g.Expect(valid).To(BeTrue(), createFailureMessage(v)) + g.Expect(supportedValues).To(BeNil(), createFailureMessage(v)) + }, "valid_value", values...) +} + +func testInvalidValuesForSupportedValuesValidator[T configValue]( + t *testing.T, + f supportedValuesValidatorFunc[T], + supportedValuesMap map[T]struct{}, + values ...T, +) { + runValidatorTests(t, func(g *WithT, v T) { + valid, supportedValues := f(v) + g.Expect(valid).To(BeFalse(), createFailureMessage(v)) + g.Expect(supportedValues).To(Equal(getSortedKeysAsString(supportedValuesMap)), createFailureMessage(v)) + }, "invalid_value", values...) +} + +func TestValidateInSupportedValues(t *testing.T) { + supportedValues := map[string]struct{}{ + "value1": {}, + "value2": {}, + "value3": {}, + } + + validator := func(value string) (bool, []string) { + return validateInSupportedValues(value, supportedValues) + } + + testValidValuesForSupportedValuesValidator(t, validator, + "value1", + "value2", + "value3") + testInvalidValuesForSupportedValuesValidator(t, validator, supportedValues, + "value4") +} + +func TestGetSortedKeysAsString(t *testing.T) { + values := map[string]struct{}{ + "value3": {}, + "value1": {}, + "value2": {}, + } + + expected := []string{"value1", "value2", "value3"} + + g := NewGomegaWithT(t) + + result := getSortedKeysAsString(values) + g.Expect(result).To(Equal(expected)) +} diff --git a/internal/nginx/config/validation/http_filters.go b/internal/nginx/config/validation/http_filters.go new file mode 100644 index 0000000000..4875d28c3d --- /dev/null +++ b/internal/nginx/config/validation/http_filters.go @@ -0,0 +1,41 @@ +package validation + +// HTTPRedirectValidator validates values for a redirect, which in NGINX is done with the return directive. +// For example, return 302 "https://example.com:8080"; +type HTTPRedirectValidator struct{} + +var supportedRedirectSchemes = map[string]struct{}{ + "http": {}, + "https": {}, +} + +// ValidateRedirectScheme validates a scheme to be used in the return directive for a redirect. +// NGINX rules are not restrictive, but it is easier to validate just for two allowed values http and https, +// dictated by the Gateway API spec. +func (HTTPRedirectValidator) ValidateRedirectScheme(scheme string) (valid bool, supportedValues []string) { + return validateInSupportedValues(scheme, supportedRedirectSchemes) +} + +var redirectHostnameExamples = []string{"host", "example.com"} + +func (HTTPRedirectValidator) ValidateRedirectHostname(hostname string) error { + return validateEscapedStringNoVarExpansion(hostname, redirectHostnameExamples) +} + +func (HTTPRedirectValidator) ValidateRedirectPort(_ int32) error { + // any value is allowed + return nil +} + +var supportedRedirectStatusCodes = map[int]struct{}{ + 301: {}, + 302: {}, +} + +// ValidateRedirectStatusCode validates a status code to be used in the return directive for a redirect. +// NGINX allows 0..999. However, let's be conservative and only allow 301 and 302 (the values allowed by the Gateway API +// spec). Note that in the future, we might reserve some codes for internal redirects, so better not to allow all +// possible code values. We can always relax the validation later in case there is a need. +func (HTTPRedirectValidator) ValidateRedirectStatusCode(statusCode int) (valid bool, supportedValues []string) { + return validateInSupportedValues(statusCode, supportedRedirectStatusCodes) +} diff --git a/internal/nginx/config/validation/http_filters_test.go b/internal/nginx/config/validation/http_filters_test.go new file mode 100644 index 0000000000..7a11b061dc --- /dev/null +++ b/internal/nginx/config/validation/http_filters_test.go @@ -0,0 +1,46 @@ +package validation + +import ( + "math" + "testing" +) + +func TestValidateRedirectScheme(t *testing.T) { + validator := HTTPRedirectValidator{} + + testValidValuesForSupportedValuesValidator(t, validator.ValidateRedirectScheme, + "http", + "https") + + testInvalidValuesForSupportedValuesValidator(t, validator.ValidateRedirectScheme, supportedRedirectSchemes, + "test") +} + +func TestValidateRedirectHostname(t *testing.T) { + validator := HTTPRedirectValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateRedirectHostname, + "example.com") + + testInvalidValuesForSimpleValidator(t, validator.ValidateRedirectHostname, + "example.com$") +} + +func TestValidateRedirectPort(t *testing.T) { + validator := HTTPRedirectValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateRedirectPort, + math.MinInt32, + math.MaxInt32) +} + +func TestValidateRedirectStatusCode(t *testing.T) { + validator := HTTPRedirectValidator{} + + testValidValuesForSupportedValuesValidator(t, validator.ValidateRedirectStatusCode, + 301, + 302) + + testInvalidValuesForSupportedValuesValidator(t, validator.ValidateRedirectStatusCode, supportedRedirectStatusCodes, + 404) +} diff --git a/internal/nginx/config/validation/http_njs_match.go b/internal/nginx/config/validation/http_njs_match.go new file mode 100644 index 0000000000..13e68124e0 --- /dev/null +++ b/internal/nginx/config/validation/http_njs_match.go @@ -0,0 +1,109 @@ +package validation + +import ( + "errors" + "fmt" + "regexp" + "strings" + + k8svalidation "k8s.io/apimachinery/pkg/util/validation" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" +) + +// HTTPNJSMatchValidator validates values used for matching a request. +// The matching is implemented in NJS (except for path matching), +// so changes to the implementation change the validation rules here. +type HTTPNJSMatchValidator struct{} + +const ( + prefixPathFmt = `/[^\s{};]*` + prefixPathErrMsg = "must start with / and must not include any whitespace character, `{`, `}` or `;`" +) + +var ( + prefixPathRegexp = regexp.MustCompile("^" + prefixPathFmt + "$") + prefixPathExamples = []string{"/", "/path", "/path/subpath-123"} +) + +// ValidatePathInPrefixMatch a prefix path used in the location directive. +func (HTTPNJSMatchValidator) ValidatePathInPrefixMatch(path string) error { + if path == "" { + return errors.New("cannot be empty") + } + + if !prefixPathRegexp.MatchString(path) { + msg := k8svalidation.RegexError(prefixPathErrMsg, prefixPathFmt, prefixPathExamples...) + return errors.New(msg) + } + + // FIXME(pleshakov): This is temporary until https://github.com/nginxinc/nginx-kubernetes-gateway/issues/428 + // is fixed. + // That's because the location path gets into the set directive in the location block. + // Example: set $http_matches "[{\"redirectPath\":\"/coffee_route0\" ... + // Where /coffee is tha path. + return validateCommonNJSMatchPart(path) +} + +func (HTTPNJSMatchValidator) ValidateHeaderNameInMatch(name string) error { + return validateNJSHeaderPart(name) +} + +func (HTTPNJSMatchValidator) ValidateHeaderValueInMatch(value string) error { + return validateNJSHeaderPart(value) +} + +func validateNJSHeaderPart(value string) error { + // if it contains the separator, it will break NJS code. + if strings.Contains(value, config.HeaderMatchSeparator) { + return fmt.Errorf("cannot contain %q", config.HeaderMatchSeparator) + } + + return validateCommonNJSMatchPart(value) +} + +func (HTTPNJSMatchValidator) ValidateQueryParamNameInMatch(name string) error { + return validateCommonNJSMatchPart(name) +} + +func (HTTPNJSMatchValidator) ValidateQueryParamValueInMatch(value string) error { + return validateCommonNJSMatchPart(value) +} + +// validateCommonNJSMatchPart validates a string value used in NJS-based matching. +func validateCommonNJSMatchPart(value string) error { + // empty values do not make sense, so we don't allow them. + + if value == "" { + return errors.New("cannot be empty") + } + + trimmed := strings.TrimSpace(value) + if len(trimmed) == 0 { + return errors.New("cannot be empty after trimming whitespace") + } + + // the JSON marshaled match (see config.httpMatch) is used as a value of the set directive in a location. + // The directive supports NGINX variables. + // We don't want to allow them, as any undefined variable will cause NGINX to fail to reload. + if strings.Contains(value, "$") { + return errors.New("cannot contain $") + } + + return nil +} + +// NGINX does not support CONNECT, TRACE methods (it will return 405 Not Allowed to clients). +var supportedMethods = map[string]struct{}{ + "GET": {}, + "HEAD": {}, + "POST": {}, + "PUT": {}, + "DELETE": {}, + "OPTIONS": {}, + "PATCH": {}, +} + +func (HTTPNJSMatchValidator) ValidateMethodInMatch(method string) (valid bool, supportedValues []string) { + return validateInSupportedValues(method, supportedMethods) +} diff --git a/internal/nginx/config/validation/http_njs_match_test.go b/internal/nginx/config/validation/http_njs_match_test.go new file mode 100644 index 0000000000..0f13613efe --- /dev/null +++ b/internal/nginx/config/validation/http_njs_match_test.go @@ -0,0 +1,85 @@ +package validation + +import ( + "testing" +) + +func TestValidatePathInPrefixMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidatePathInPrefixMatch, + "/", + "/path", + "/path/subpath-123") + testInvalidValuesForSimpleValidator(t, validator.ValidatePathInPrefixMatch, + "/ ", + "/path{", + "/path}", + "/path;", + "path", + "", + "/path$") +} + +func TestValidateHeaderNameInMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateHeaderNameInMatch, + "header") + testInvalidValuesForSimpleValidator(t, validator.ValidateHeaderNameInMatch, + ":", + "") +} + +func TestValidateHeaderValueInMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateHeaderValueInMatch, + "value") + testInvalidValuesForSimpleValidator(t, validator.ValidateHeaderValueInMatch, + ":", + "") +} + +func TestValidateQueryParamNameInMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateQueryParamNameInMatch, + "param") + testInvalidValuesForSimpleValidator(t, validator.ValidateQueryParamNameInMatch, + "") +} + +func TestValidateQueryParamValueInMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateQueryParamValueInMatch, + "value") + testInvalidValuesForSimpleValidator(t, validator.ValidateQueryParamValueInMatch, + "") +} + +func TestValidateMethodInMatch(t *testing.T) { + validator := HTTPNJSMatchValidator{} + + testValidValuesForSupportedValuesValidator(t, validator.ValidateMethodInMatch, + "GET", + "HEAD", + "POST", + "PUT", + "DELETE", + "OPTIONS", + "PATCH") + testInvalidValuesForSupportedValuesValidator(t, validator.ValidateMethodInMatch, supportedMethods, + "GOT", + "TRACE") +} + +func TestValidateCommonMatchPart(t *testing.T) { + testValidValuesForSimpleValidator(t, validateCommonNJSMatchPart, + "test") + testInvalidValuesForSimpleValidator(t, validateCommonNJSMatchPart, + "", + " ", + "$") +} diff --git a/internal/nginx/config/validation/http_validator.go b/internal/nginx/config/validation/http_validator.go new file mode 100644 index 0000000000..ad6348c42f --- /dev/null +++ b/internal/nginx/config/validation/http_validator.go @@ -0,0 +1,22 @@ +package validation + +import ( + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" +) + +// HTTPValidator validates values that will propagate into the NGINX configuration http context. +// The validation rules are based on the nginx/config/http types and how they are used in the configuration templates +// of the nginx/config package. Changes to those might require changing the validation rules +type HTTPValidator struct { + HTTPNJSMatchValidator + HTTPRedirectValidator +} + +var _ validation.HTTPFieldsValidator = HTTPValidator{} + +var hostnameInServerExamples = []string{"host", "example.com"} + +// ValidateHostnameInServer validates a hostname to be used in the server_name directive. +func (HTTPValidator) ValidateHostnameInServer(hostname string) error { + return validateEscapedString(hostname, hostnameInServerExamples) +} diff --git a/internal/nginx/config/validation/http_validator_test.go b/internal/nginx/config/validation/http_validator_test.go new file mode 100644 index 0000000000..4a366ded32 --- /dev/null +++ b/internal/nginx/config/validation/http_validator_test.go @@ -0,0 +1,14 @@ +package validation + +import "testing" + +func TestValidateHostnameInServer(t *testing.T) { + validator := HTTPValidator{} + + testValidValuesForSimpleValidator(t, validator.ValidateHostnameInServer, + "", + "example.com") + testInvalidValuesForSimpleValidator(t, validator.ValidateHostnameInServer, + `\`, + `"`) +} diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index eb87a0a4dd..77b7232d3e 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -17,6 +17,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ChangeProcessor @@ -41,18 +42,20 @@ type ChangeProcessor interface { // ChangeProcessorConfig holds configuration parameters for ChangeProcessorImpl. type ChangeProcessorConfig struct { - // GatewayCtlrName is the name of the Gateway controller. - GatewayCtlrName string - // GatewayClassName is the name of the GatewayClass resource. - GatewayClassName string // SecretMemoryManager is the secret memory manager. SecretMemoryManager secrets.SecretDiskMemoryManager // ServiceResolver resolves Services to Endpoints. ServiceResolver resolver.ServiceResolver // RelationshipCapturer captures relationships between Kubernetes API resources and Gateway API resources. RelationshipCapturer relationship.Capturer + // Validators validate resources according to data-plane specific rules. + Validators validation.Validators // Logger is the logger for this Change Processor. Logger logr.Logger + // GatewayCtlrName is the name of the Gateway controller. + GatewayCtlrName string + // GatewayClassName is the name of the GatewayClass resource. + GatewayClassName string } // ChangeProcessorImpl is an implementation of ChangeProcessor. @@ -162,6 +165,7 @@ func (c *ChangeProcessorImpl) Process( c.cfg.GatewayCtlrName, c.cfg.GatewayClassName, c.cfg.SecretMemoryManager, + c.cfg.Validators, ) var warnings dataplane.Warnings @@ -170,6 +174,7 @@ func (c *ChangeProcessorImpl) Process( for obj, objWarnings := range warnings { for _, w := range objWarnings { // FIXME(pleshakov): report warnings via Object status + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/467 c.cfg.Logger.Info("Got warning while building Graph", "kind", obj.GetObjectKind().GroupVersionKind().Kind, "namespace", obj.GetNamespace(), diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index db0da93323..b2d9a7f4a3 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -24,6 +24,8 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship/relationshipfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation/validationfakes" ) const ( @@ -71,6 +73,7 @@ func createRoute( Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ + Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(v1beta1.PathMatchPathPrefix))), Value: helpers.GetStringPointer("/"), }, }, @@ -166,6 +169,14 @@ func createBackendRef( } } +func createAlwaysValidValidators() validation.Validators { + http := &validationfakes.FakeHTTPFieldsValidator{} + + return validation.Validators{ + HTTPFieldsValidator: http, + } +} + // FIXME(kate-osborn): Consider refactoring these tests to reduce code duplication. var _ = Describe("ChangeProcessor", func() { Describe("Normal cases of processing changes", func() { @@ -192,6 +203,7 @@ var _ = Describe("ChangeProcessor", func() { SecretMemoryManager: fakeSecretMemoryMgr, RelationshipCapturer: relationship.NewCapturerImpl(), Logger: zap.New(), + Validators: createAlwaysValidValidators(), }) fakeSecretMemoryMgr.RequestReturns(certificatePath, nil) @@ -1577,6 +1589,7 @@ var _ = Describe("ChangeProcessor", func() { GatewayClassName: "my-class", SecretMemoryManager: fakeSecretMemoryMgr, RelationshipCapturer: fakeRelationshipCapturer, + Validators: createAlwaysValidValidators(), }) gcNsName = types.NamespacedName{Name: "my-class"} @@ -1879,6 +1892,7 @@ var _ = Describe("ChangeProcessor", func() { GatewayClassName: "my-class", SecretMemoryManager: fakeSecretMemoryMgr, RelationshipCapturer: fakeRelationshipCapturer, + Validators: createAlwaysValidValidators(), }) }) diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index 205e061fcd..85a6e04bdc 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -8,11 +8,16 @@ import ( ) const ( - // RouteReasonInvalidListener is used with the "Accepted" condition when the route references an invalid listener. + // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener" + // ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener // is invalid or not supported. ListenerReasonUnsupportedValue v1beta1.ListenerConditionReason = "UnsupportedValue" + + // RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the + // Route rules has a backendRef with an unsupported value. + RouteReasonBackendRefUnsupportedValue = "UnsupportedValue" ) // Condition defines a condition to be reported in the status of resources. @@ -83,6 +88,16 @@ func NewRouteAccepted() Condition { } } +// NewRouteUnsupportedValue returns a Condition that indicates that the HTTPRoute includes an unsupported value. +func NewRouteUnsupportedValue(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonUnsupportedValue), + Message: msg, + } +} + // NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented. func NewTODO(msg string) Condition { return Condition{ @@ -204,3 +219,47 @@ func NewListenerUnsupportedProtocol(msg string) Condition { Message: msg, } } + +// NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an +// invalid kind. +func NewRouteBackendRefInvalidKind(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonInvalidKind), + Message: msg, + } +} + +// NewRouteBackendRefRefNotPermitted returns a Condition that indicates that the Route has a backendRef that +// is not permitted. +func NewRouteBackendRefRefNotPermitted(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonRefNotPermitted), + Message: msg, + } +} + +// NewRouteBackendRefRefBackendNotFound returns a Condition that indicates that the Route has a backendRef that +// points to non-existing backend. +func NewRouteBackendRefRefBackendNotFound(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonBackendNotFound), + Message: msg, + } +} + +// NewRouteBackendRefUnsupportedValue returns a Condition that indicates that the Route has a backendRef with +// an unsupported value. +func NewRouteBackendRefUnsupportedValue(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: RouteReasonBackendRefUnsupportedValue, + Message: msg, + } +} diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 7251d824d7..8de8baf17a 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -62,8 +62,12 @@ type PathRule struct { MatchRules []MatchRule } +// InvalidFilter is a special filter for handling the case when configured filters are invalid. +type InvalidFilter struct{} + // Filters hold the filters for a MatchRule. type Filters struct { + InvalidFilter *InvalidFilter RequestRedirect *v1beta1.HTTPRequestRedirectFilter } @@ -150,12 +154,7 @@ func buildWarnings(graph *graph.Graph, upstreams map[string]Upstream) Warnings { continue } - for _, group := range r.BackendGroups { - - for _, errMsg := range group.Errors { - warnings.AddWarningf(r.Source, "invalid backend ref: %s", errMsg) - } - + for _, group := range r.GetAllBackendGroups() { for _, backend := range group.Backends { if backend.Name != "" { upstream, ok := upstreams[backend.Name] @@ -201,13 +200,12 @@ func buildBackendGroups(listeners map[string]*graph.Listener) []graph.BackendGro } for _, r := range l.Routes { - for _, group := range r.BackendGroups { + for _, group := range r.GetAllBackendGroups() { if _, ok := uniqueGroups[group.GroupName()]; !ok { uniqueGroups[group.GroupName()] = group } } } - } numGroups := len(uniqueGroups) @@ -282,7 +280,18 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { } for i, rule := range r.Source.Spec.Rules { - filters := createFilters(rule.Filters) + if !r.Rules[i].ValidMatches { + continue + } + + var filters Filters + if r.Rules[i].ValidFilters { + filters = createFilters(rule.Filters) + } else { + filters = Filters{ + InvalidFilter: &InvalidFilter{}, + } + } for _, h := range hostnames { for j, m := range rule.Matches { @@ -297,7 +306,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { MatchIdx: j, RuleIdx: i, Source: r.Source, - BackendGroup: r.BackendGroups[i], + BackendGroup: r.Rules[i].BackendGroup, Filters: filters, }) @@ -387,7 +396,7 @@ func buildUpstreamsMap( } for _, route := range l.Routes { - for _, group := range route.BackendGroups { + for _, group := range route.GetAllBackendGroups() { for _, backend := range group.Backends { if name := backend.Name; name != "" { _, exist := uniqueUpstreams[name] diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 6f024b068f..56ce57b425 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -8,19 +8,24 @@ import ( "testing" "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver/resolverfakes" ) func TestBuildConfiguration(t *testing.T) { + const ( + invalidMatchesPath = "/not-valid-matches" + invalidFiltersPath = "/not-valid-filters" + ) + createRoute := func(name, hostname, listenerName string, paths ...string) *v1beta1.HTTPRoute { rules := make([]v1beta1.HTTPRouteRule, 0, len(paths)) for _, p := range paths { @@ -57,11 +62,10 @@ func TestBuildConfiguration(t *testing.T) { } } - addFilters := func(hr *v1beta1.HTTPRoute, filters []v1beta1.HTTPRouteFilter) *v1beta1.HTTPRoute { + addFilters := func(hr *v1beta1.HTTPRoute, filters []v1beta1.HTTPRouteFilter) { for i := range hr.Spec.Rules { hr.Spec.Rules[i].Filters = filters } - return hr } fooUpstreamName := "test_foo_80" @@ -83,7 +87,11 @@ func TestBuildConfiguration(t *testing.T) { fakeResolver := &resolverfakes.FakeServiceResolver{} fakeResolver.ResolveReturns(fooEndpoints, nil) - createBackendGroup := func(nsname types.NamespacedName, idx int) graph.BackendGroup { + createBackendGroup := func(nsname types.NamespacedName, idx int, validRule bool) graph.BackendGroup { + if !validRule { + return graph.BackendGroup{} + } + return graph.BackendGroup{ Source: nsname, RuleIdx: idx, @@ -99,16 +107,31 @@ func TestBuildConfiguration(t *testing.T) { } } + createRules := func(hr *v1beta1.HTTPRoute, paths []string) []graph.Rule { + rules := make([]graph.Rule, len(hr.Spec.Rules)) + + for i := range paths { + validMatches := paths[i] != invalidMatchesPath + validFilters := paths[i] != invalidFiltersPath + validRule := validMatches && validFilters + + rules[i] = graph.Rule{ + ValidMatches: validMatches, + ValidFilters: validFilters, + BackendGroup: createBackendGroup(types.NamespacedName{Namespace: "test", Name: hr.Name}, i, validRule), + } + } + + return rules + } + createInternalRoute := func( source *v1beta1.HTTPRoute, - validSectionName string, - groups ...graph.BackendGroup, + paths []string, ) *graph.Route { r := &graph.Route{ - Source: source, - InvalidSectionNameRefs: make(map[string]conditions.Condition), - ValidSectionNameRefs: map[string]struct{}{validSectionName: {}}, - BackendGroups: groups, + Source: source, + Rules: createRules(source, paths), } return r } @@ -117,13 +140,8 @@ func TestBuildConfiguration(t *testing.T) { *v1beta1.HTTPRoute, []graph.BackendGroup, *graph.Route, ) { hr := createRoute(name, hostname, listenerName, paths...) - groups := make([]graph.BackendGroup, 0, len(paths)) - for idx := range paths { - groups = append(groups, createBackendGroup(types.NamespacedName{Namespace: "test", Name: name}, idx)) - } - - route := createInternalRoute(hr, listenerName, groups...) - return hr, groups, route + route := createInternalRoute(hr, paths) + return hr, route.GetAllBackendGroups(), route } hr1, hr1Groups, routeHR1 := createTestResources("hr-1", "foo.example.com", "listener-80-1", "/") @@ -131,6 +149,22 @@ func TestBuildConfiguration(t *testing.T) { hr3, hr3Groups, routeHR3 := createTestResources("hr-3", "foo.example.com", "listener-80-1", "/", "/third") hr4, hr4Groups, routeHR4 := createTestResources("hr-4", "foo.example.com", "listener-80-1", "/fourth", "/") + hr5, hr5Groups, routeHR5 := createTestResources("hr-5", "foo.example.com", "listener-80-1", "/", invalidFiltersPath) + redirect := v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), + }, + } + addFilters(hr5, []v1beta1.HTTPRouteFilter{redirect}) + + hr6, hr6Groups, routeHR6 := createTestResources( + "hr-6", + "foo.example.com", + "listener-80-1", + "/valid", invalidMatchesPath, + ) + httpsHR1, httpsHR1Groups, httpsRouteHR1 := createTestResources( "https-hr-1", "foo.example.com", @@ -166,30 +200,14 @@ func TestBuildConfiguration(t *testing.T) { "/", ) - redirect := v1beta1.HTTPRouteFilter{ - Type: v1beta1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ - Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), - }, - } - - hr5 := addFilters( - createRoute("hr-5", "foo.example.com", "listener-80-1", "/"), - []v1beta1.HTTPRouteFilter{redirect}, + httpsHR6, httpsHR6Groups, httpsRouteHR6 := createTestResources( + "https-hr-6", + "foo.example.com", + "listener-443-1", + "/valid", + invalidMatchesPath, ) - hr5BackendGroup := graph.BackendGroup{ - Source: types.NamespacedName{Namespace: hr5.Namespace, Name: hr5.Name}, - RuleIdx: 0, - } - - routeHR5 := &graph.Route{ - Source: hr5, - InvalidSectionNameRefs: make(map[string]conditions.Condition), - ValidSectionNameRefs: map[string]struct{}{"listener-80-1": {}}, - BackendGroups: []graph.BackendGroup{hr5BackendGroup}, - } - listener80 := v1beta1.Listener{ Name: "listener-80-1", Hostname: nil, @@ -833,41 +851,152 @@ func TestBuildConfiguration(t *testing.T) { MatchIdx: 0, RuleIdx: 0, Source: hr5, - BackendGroup: hr5BackendGroup, + BackendGroup: hr5Groups[0], Filters: Filters{ RequestRedirect: redirect.RequestRedirect, }, }, }, }, + { + Path: invalidFiltersPath, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + Source: hr5, + Filters: Filters{ + InvalidFilter: &InvalidFilter{}, + }, + }, + }, + }, }, }, }, SSLServers: []VirtualServer{}, - BackendGroups: []graph.BackendGroup{hr5BackendGroup}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: hr5Groups, }, msg: "one http listener with one route with filters", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1beta1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1beta1.Gateway{}, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-6"}: routeHR6, + }, + AcceptedHostnames: map[string]struct{}{ + "foo.example.com": {}, + }, + }, + "listener-443-1": { + Source: listener443, + Valid: true, + SecretPath: secretPath, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "https-hr-6"}: httpsRouteHR6, + }, + AcceptedHostnames: map[string]struct{}{ + "foo.example.com": {}, + }, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-6"}: routeHR6, + {Namespace: "test", Name: "https-hr-6"}: httpsRouteHR6, + }, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, + { + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/valid", + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr6Groups[0], + Source: hr6, + }, + }, + }, + }, + }, + }, + SSLServers: []VirtualServer{ + { + IsDefault: true, + }, + { + Hostname: "foo.example.com", + SSL: &SSL{ + CertificatePath: secretPath, + }, + PathRules: []PathRule{ + { + Path: "/valid", + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: httpsHR6Groups[0], + Source: httpsHR6, + }, + }, + }, + }, + }, + { + Hostname: wildcardHostname, + SSL: &SSL{CertificatePath: secretPath}, + }, + }, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []graph.BackendGroup{ + hr6Groups[0], + httpsHR6Groups[0], + }, + }, + msg: "one http and one https listener with routes with valid and invalid rules", + }, } for _, test := range tests { - result, warns := BuildConfiguration(context.TODO(), test.graph, fakeResolver) + t.Run(test.msg, func(t *testing.T) { + result, warns := BuildConfiguration(context.TODO(), test.graph, fakeResolver) - sort.Slice(result.BackendGroups, func(i, j int) bool { - return result.BackendGroups[i].GroupName() < result.BackendGroups[j].GroupName() - }) + sort.Slice(result.BackendGroups, func(i, j int) bool { + return result.BackendGroups[i].GroupName() < result.BackendGroups[j].GroupName() + }) - sort.Slice(result.Upstreams, func(i, j int) bool { - return result.Upstreams[i].Name < result.Upstreams[j].Name - }) + sort.Slice(result.Upstreams, func(i, j int) bool { + return result.Upstreams[i].Name < result.Upstreams[j].Name + }) - if diff := cmp.Diff(test.expConf, result); diff != "" { - t.Errorf("BuildConfiguration() %q mismatch for configuration (-want +got):\n%s", test.msg, diff) - } + if diff := cmp.Diff(test.expConf, result); diff != "" { + t.Errorf("BuildConfiguration() %q mismatch for configuration (-want +got):\n%s", test.msg, diff) + } - if diff := cmp.Diff(test.expWarns, warns); diff != "" { - t.Errorf("BuildConfiguration() %q mismatch for warnings (-want +got):\n%s", test.msg, diff) - } + if diff := cmp.Diff(test.expWarns, warns); diff != "" { + t.Errorf("BuildConfiguration() %q mismatch for warnings (-want +got):\n%s", test.msg, diff) + } + }) } } @@ -1070,6 +1199,20 @@ func TestGetListenerHostname(t *testing.T) { } } +func groupsToValidRules(groups ...graph.BackendGroup) []graph.Rule { + rules := make([]graph.Rule, 0, len(groups)) + + for _, group := range groups { + rules = append(rules, graph.Rule{ + ValidMatches: true, + ValidFilters: true, + BackendGroup: group, + }) + } + + return rules +} + func TestBuildUpstreams(t *testing.T) { fooEndpoints := []resolver.Endpoint{ { @@ -1150,25 +1293,25 @@ func TestBuildUpstreams(t *testing.T) { routes := map[types.NamespacedName]*graph.Route{ {Name: "hr1", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr1Group0, hr1Group1}, + Rules: groupsToValidRules(hr1Group0, hr1Group1), }, {Name: "hr2", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr2Group0, hr2Group1}, + Rules: groupsToValidRules(hr2Group0, hr2Group1), }, {Name: "hr3", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr3Group0}, + Rules: groupsToValidRules(hr3Group0), }, } routes2 := map[types.NamespacedName]*graph.Route{ {Name: "hr4", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr4Group0, hr4Group1}, + Rules: groupsToValidRules(hr4Group0, hr4Group1), }, } invalidRoutes := map[types.NamespacedName]*graph.Route{ {Name: "invalid", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{invalidGroup}, + Rules: groupsToValidRules(invalidGroup), }, } @@ -1238,11 +1381,11 @@ func TestBuildUpstreams(t *testing.T) { } }) + g := NewGomegaWithT(t) + upstreams := buildUpstreamsMap(context.TODO(), listeners, fakeResolver) - if diff := cmp.Diff(expUpstreams, upstreams); diff != "" { - t.Errorf("buildUpstreamsMap() mismatch (-want +got):\n%s", diff) - } + g.Expect(helpers.Diff(upstreams, expUpstreams)).To(BeEmpty()) } func TestBuildBackendGroups(t *testing.T) { @@ -1275,26 +1418,26 @@ func TestBuildBackendGroups(t *testing.T) { invalidRoutes := map[types.NamespacedName]*graph.Route{ {Name: "invalid", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hrInvalid}, + Rules: groupsToValidRules(hrInvalid), }, } routes := map[types.NamespacedName]*graph.Route{ {Name: "hr1", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr1Rule0, hr1Rule1}, + Rules: groupsToValidRules(hr1Rule0, hr1Rule1), }, {Name: "hr2", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr2Rule0, hr2Rule1}, + Rules: groupsToValidRules(hr2Rule0, hr2Rule1), }, } routes2 := map[types.NamespacedName]*graph.Route{ // this backend group is a dupe and should be ignored. {Name: "hr1", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr1Rule0, hr1Rule1}, + Rules: groupsToValidRules(hr1Rule0, hr1Rule1), }, {Name: "hr3", Namespace: "test"}: { - BackendGroups: []graph.BackendGroup{hr3Rule0, hr3Rule1}, + Rules: groupsToValidRules(hr3Rule0, hr3Rule1), }, } @@ -1322,15 +1465,11 @@ func TestBuildBackendGroups(t *testing.T) { hr3Rule1, } - result := buildBackendGroups(listeners) + g := NewGomegaWithT(t) - sort.Slice(result, func(i, j int) bool { - return result[i].GroupName() < result[j].GroupName() - }) + result := buildBackendGroups(listeners) - if diff := helpers.Diff(expGroups, result); diff != "" { - t.Errorf("buildBackendGroups() mismatch: %+v", diff) - } + g.Expect(result).To(ConsistOf(expGroups)) } func TestBuildWarnings(t *testing.T) { @@ -1343,80 +1482,42 @@ func TestBuildWarnings(t *testing.T) { return backends } - createBackendGroup := func(sourceName string, backends []graph.BackendRef, errMsgs ...string) graph.BackendGroup { + createBackendGroup := func(sourceName string, backends []graph.BackendRef) graph.BackendGroup { return graph.BackendGroup{ Source: types.NamespacedName{Namespace: "test", Name: sourceName}, Backends: backends, - Errors: errMsgs, } } - hr1BackendGroup0 := createBackendGroup( - "hr1", - createBackendRefs("foo"), - "error1-1", "error1-2", "error1-3", - ) - - hr1BackendGroup1 := createBackendGroup( - "hr1", - createBackendRefs("bar"), - ) - - hr2BackendGroup0 := createBackendGroup( - "hr2", - createBackendRefs("foo", "bar"), - ) - - hr2BackendGroup1 := createBackendGroup( - "hr2", - createBackendRefs("resolve-error"), - "error2", - ) - - hr3BackendGroup0 := createBackendGroup( - "hr3", + hrBackendGroup0 := createBackendGroup( + "hr", createBackendRefs(""), // empty backend name should be skipped - "error3", ) - hr3BackendGroup1 := createBackendGroup( - "hr3", + hrBackendGroup1 := createBackendGroup( + "hr", createBackendRefs("dne"), ) hrInvalidGroup := createBackendGroup( "hr-invalid", createBackendRefs("invalid"), - "invalid", ) - hr1 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr1", Namespace: "test"}} - hr2 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr2", Namespace: "test"}} - hr3 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr3", Namespace: "test"}} + hr := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr", Namespace: "test"}} hrInvalid := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr-invalid", Namespace: "test"}} invalidRoutes := map[types.NamespacedName]*graph.Route{ {Name: "invalid", Namespace: "test"}: { - Source: hrInvalid, - BackendGroups: []graph.BackendGroup{hrInvalidGroup}, + Source: hrInvalid, + Rules: groupsToValidRules(hrInvalidGroup), }, } routes := map[types.NamespacedName]*graph.Route{ - {Name: "hr1", Namespace: "test"}: { - Source: hr1, - BackendGroups: []graph.BackendGroup{hr1BackendGroup0, hr1BackendGroup1}, - }, - {Name: "hr2", Namespace: "test"}: { - Source: hr2, - BackendGroups: []graph.BackendGroup{hr2BackendGroup0, hr2BackendGroup1}, - }, - } - - routes2 := map[types.NamespacedName]*graph.Route{ - {Name: "hr3", Namespace: "test"}: { - Source: hr3, - BackendGroups: []graph.BackendGroup{hr3BackendGroup0, hr3BackendGroup1}, + {Name: "hr", Namespace: "test"}: { + Source: hr, + Rules: groupsToValidRules(hrBackendGroup0, hrBackendGroup1), }, } @@ -1443,38 +1544,22 @@ func TestBuildWarnings(t *testing.T) { Valid: true, Routes: routes, }, - "listener2": { - Source: v1beta1.Listener{ - Name: "valid2", - }, - Valid: true, - Routes: routes2, - }, }, }, } expWarns := Warnings{ - hr1: []string{ - "invalid backend ref: error1-1", - "invalid backend ref: error1-2", - "invalid backend ref: error1-3", - }, - hr2: []string{ - "invalid backend ref: error2", - "cannot resolve backend ref: resolve error", - }, - hr3: []string{ - "invalid backend ref: error3", + hr: []string{ "cannot resolve backend ref; internal error: upstream dne not found in map", }, hrInvalid: []string{"cannot configure routes for listener invalid; listener is invalid"}, } + g := NewGomegaWithT(t) + warns := buildWarnings(graph, upstreamMap) - if diff := cmp.Diff(expWarns, warns); diff != "" { - t.Errorf("buildWarnings() mismatch (-want +got):\n%s", diff) - } + + g.Expect(helpers.Diff(warns, expWarns)).To(BeEmpty()) } func TestUpstreamsMapToSlice(t *testing.T) { diff --git a/internal/state/graph/backend_group.go b/internal/state/graph/backend_group.go index d21fd1771a..9c36cc2ff3 100644 --- a/internal/state/graph/backend_group.go +++ b/internal/state/graph/backend_group.go @@ -10,7 +10,6 @@ import ( // BackendGroup represents a group of backends for a rule in an HTTPRoute. type BackendGroup struct { Source types.NamespacedName - Errors []string Backends []BackendRef RuleIdx int } diff --git a/internal/state/graph/backend_refs.go b/internal/state/graph/backend_refs.go index be2139e01f..10b8d64765 100644 --- a/internal/state/graph/backend_refs.go +++ b/internal/state/graph/backend_refs.go @@ -1,111 +1,194 @@ package graph import ( - "errors" "fmt" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" ) -// addBackendGroupsToRoutes iterates over the routes and adds BackendGroups to the routes. -// The routes are modified in place. -// If a backend ref is invalid it will store an error message in the BackendGroup.Errors field. -// A backend ref is invalid if: -// - the Kind is not Service -// - the Namespace is not the same as the HTTPRoute namespace -// - the Port is nil func addBackendGroupsToRoutes( routes map[types.NamespacedName]*Route, services map[types.NamespacedName]*v1.Service, ) { for _, r := range routes { - r.BackendGroups = make([]BackendGroup, len(r.Source.Spec.Rules)) + addBackendGroupsToRoute(r, services) + } +} - for idx, rule := range r.Source.Spec.Rules { +// addBackendGroupsToRoute iterates over the rules of a route and adds BackendGroups to the rules. +// The route is modified in place. +// If a reference in a rule is invalid, the function will add a condition to the rule. +func addBackendGroupsToRoute(route *Route, services map[types.NamespacedName]*v1.Service) { + if !route.Valid { + return + } - group := BackendGroup{ - Source: client.ObjectKeyFromObject(r.Source), - RuleIdx: idx, - } + for idx, rule := range route.Source.Spec.Rules { + if !route.Rules[idx].ValidMatches { + continue + } + if !route.Rules[idx].ValidFilters { + continue + } - if len(rule.BackendRefs) == 0 { + // zero backendRefs is OK. For example, a rule can include a redirect filter. + if len(rule.BackendRefs) == 0 { + continue + } - r.BackendGroups[idx] = group - continue - } + group := &route.Rules[idx].BackendGroup - group.Errors = make([]string, 0, len(rule.BackendRefs)) - group.Backends = make([]BackendRef, 0, len(rule.BackendRefs)) + group.Backends = make([]BackendRef, 0, len(rule.BackendRefs)) - for _, ref := range rule.BackendRefs { + for refIdx, ref := range rule.BackendRefs { + refPath := field.NewPath("spec").Child("rules").Index(idx).Child("backendRefs").Index(refIdx) - weight := int32(1) - if ref.Weight != nil { - weight = *ref.Weight - } + backend, cond := createBackend(ref, route.Source.Namespace, services, refPath) + + group.Backends = append(group.Backends, backend) + if cond != nil { + route.Conditions = append(route.Conditions, *cond) + } + } + } +} - svc, port, err := getServiceAndPortFromRef(ref.BackendRef, r.Source.Namespace, services) - if err != nil { - group.Backends = append(group.Backends, BackendRef{Weight: weight}) +func createBackend( + ref v1beta1.HTTPBackendRef, + sourceNamespace string, + services map[types.NamespacedName]*v1.Service, + refPath *field.Path, +) (BackendRef, *conditions.Condition) { + // Data plane will handle invalid ref by responding with 500. + // Because of that, we always need to add a BackendRef to group.Backends, even if the ref is invalid. + // Additionally, we always calculate the weight, even if it is invalid. + weight := int32(1) + if ref.Weight != nil { + if validateWeight(*ref.Weight) != nil { + // We don't need to add a condition because validateHTTPBackendRef will do that. + weight = 0 // 0 will get no traffic + } else { + weight = *ref.Weight + } + } - group.Errors = append(group.Errors, err.Error()) + var backend BackendRef - continue - } + valid, cond := validateHTTPBackendRef(ref, sourceNamespace, refPath) + if !valid { + backend = BackendRef{ + Weight: weight, + Valid: false, + } - group.Backends = append(group.Backends, BackendRef{ - Name: fmt.Sprintf("%s_%s_%d", svc.Namespace, svc.Name, port), - Svc: svc, - Port: port, - Valid: true, - Weight: weight, - }) - } + return backend, &cond + } - r.BackendGroups[idx] = group + svc, port, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + if err != nil { + backend = BackendRef{ + Weight: weight, + Valid: false, } + + cond := conditions.NewRouteBackendRefRefBackendNotFound(err.Error()) + return backend, &cond } + + backend = BackendRef{ + Name: fmt.Sprintf("%s_%s_%d", svc.Namespace, svc.Name, port), + Svc: svc, + Port: port, + Valid: true, + Weight: weight, + } + + return backend, nil } func getServiceAndPortFromRef( ref v1beta1.BackendRef, routeNamespace string, services map[types.NamespacedName]*v1.Service, + refPath *field.Path, ) (*v1.Service, int32, error) { - err := validateBackendRef(ref, routeNamespace) - if err != nil { - return nil, 0, err - } - svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: routeNamespace} svc, ok := services[svcNsName] if !ok { - return nil, 0, fmt.Errorf("the Service %s does not exist", svcNsName) + return nil, 0, field.NotFound(refPath.Child("name"), ref.Name) } // safe to dereference port here because we already validated that the port is not nil. return svc, int32(*ref.Port), nil } -func validateBackendRef(ref v1beta1.BackendRef, routeNs string) error { +func validateHTTPBackendRef( + ref v1beta1.HTTPBackendRef, + routeNs string, + path *field.Path, +) (valid bool, cond conditions.Condition) { + // Because all errors cause the same condition but different reasons, we return as soon as we find an error + + if len(ref.Filters) > 0 { + valErr := field.TooMany(path.Child("filters"), len(ref.Filters), 0) + return false, conditions.NewRouteBackendRefUnsupportedValue(valErr.Error()) + } + + return validateBackendRef(ref.BackendRef, routeNs, path) +} + +func validateBackendRef( + ref v1beta1.BackendRef, + routeNs string, + path *field.Path, +) (valid bool, cond conditions.Condition) { + // Because all errors cause same condition but different reasons, we return as soon as we find an error + + if ref.Group != nil && !(*ref.Group == "core" || *ref.Group == "") { + valErr := field.NotSupported(path.Child("group"), *ref.Group, []string{"core", ""}) + return false, conditions.NewRouteBackendRefUnsupportedValue(valErr.Error()) + } + if ref.Kind != nil && *ref.Kind != "Service" { - return fmt.Errorf("the Kind must be Service; got %s", *ref.Kind) + valErr := field.NotSupported(path.Child("kind"), *ref.Kind, []string{"Service"}) + return false, conditions.NewRouteBackendRefInvalidKind(valErr.Error()) } + // no need to validate ref.Name + if ref.Namespace != nil && string(*ref.Namespace) != routeNs { - return fmt.Errorf( - "cross-namespace routing is not permitted; namespace %s does not match the HTTPRoute namespace %s", - *ref.Namespace, - routeNs, - ) + valErr := field.Invalid(path.Child("namespace"), *ref.Namespace, "cross-namespace routing is not permitted") + return false, conditions.NewRouteBackendRefRefNotPermitted(valErr.Error()) } - if ref.Port == nil { - return errors.New("port is missing") + // The imported Webhook validation ensures ref.Port is set + // any value is OK + // FIXME(pleshakov): Add a unit test for the imported Webhook validation code for this case. + + if ref.Weight != nil { + if err := validateWeight(*ref.Weight); err != nil { + valErr := field.Invalid(path.Child("weight"), *ref.Weight, err.Error()) + return false, conditions.NewRouteBackendRefUnsupportedValue(valErr.Error()) + } + } + + return true, conditions.Condition{} +} + +func validateWeight(weight int32) error { + const ( + minWeight = 0 + maxWeight = 1_000_000 + ) + + if weight < minWeight || weight > maxWeight { + return fmt.Errorf("must be in the range [%d, %d]", minWeight, maxWeight) } return nil diff --git a/internal/state/graph/backend_refs_test.go b/internal/state/graph/backend_refs_test.go index 5d4876a547..17c67dfd6f 100644 --- a/internal/state/graph/backend_refs_test.go +++ b/internal/state/graph/backend_refs_test.go @@ -3,14 +3,16 @@ package graph import ( "testing" - "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" ) func getNormalRef() v1beta1.BackendRef { @@ -21,7 +23,7 @@ func getNormalRef() v1beta1.BackendRef { Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), }, - Weight: helpers.GetInt32Pointer(1), + Weight: helpers.GetInt32Pointer(5), } } @@ -29,65 +31,162 @@ func getModifiedRef(mod func(ref v1beta1.BackendRef) v1beta1.BackendRef) v1beta1 return mod(getNormalRef()) } +func TestValidateHTTPBackendRef(t *testing.T) { + tests := []struct { + expectedCondition conditions.Condition + name string + ref v1beta1.HTTPBackendRef + expectedValid bool + }{ + { + name: "normal case", + ref: v1beta1.HTTPBackendRef{ + BackendRef: getNormalRef(), + Filters: nil, + }, + expectedValid: true, + }, + { + name: "filters not supported", + ref: v1beta1.HTTPBackendRef{ + BackendRef: getNormalRef(), + Filters: []v1beta1.HTTPRouteFilter{ + { + Type: v1beta1.HTTPRouteFilterRequestHeaderModifier, + }, + }, + }, + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefUnsupportedValue( + "test.filters: Too many: 1: must have at most 0 items", + ), + }, + { + name: "invalid base ref", + ref: v1beta1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Kind = helpers.GetPointer[v1beta1.Kind]("NotService") + return backend + }), + }, + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefInvalidKind( + `test.kind: Unsupported value: "NotService": supported values: "Service"`, + ), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + valid, cond := validateHTTPBackendRef(test.ref, "test", field.NewPath("test")) + + g.Expect(valid).To(Equal(test.expectedValid)) + g.Expect(cond).To(Equal(test.expectedCondition)) + }) + } +} + func TestValidateBackendRef(t *testing.T) { tests := []struct { - ref v1beta1.BackendRef - msg string - expErr bool + ref v1beta1.BackendRef + expectedCondition conditions.Condition + name string + expectedValid bool }{ { - msg: "normal case", - ref: getNormalRef(), - expErr: false, + name: "normal case", + ref: getNormalRef(), + expectedValid: true, }, { - msg: "normal case with implicit namespace", + name: "normal case with implicit namespace", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { backend.Namespace = nil return backend }), - expErr: false, + expectedValid: true, }, { - msg: "normal case with implicit kind Service", + name: "normal case with implicit kind Service", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { backend.Kind = nil return backend }), - expErr: false, + expectedValid: true, + }, + { + name: "invalid group", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Group = helpers.GetPointer[v1beta1.Group]("invalid") + return backend + }), + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefUnsupportedValue( + `test.group: Unsupported value: "invalid": supported values: "core", ""`, + ), }, { - msg: "not a service kind", + name: "not a service kind", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { backend.Kind = (*v1beta1.Kind)(helpers.GetStringPointer("NotService")) return backend }), - expErr: true, + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefInvalidKind( + `test.kind: Unsupported value: "NotService": supported values: "Service"`, + ), }, { - msg: "invalid namespace", + name: "invalid namespace", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { backend.Namespace = (*v1beta1.Namespace)(helpers.GetStringPointer("invalid")) return backend }), - expErr: true, + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefRefNotPermitted( + `test.namespace: Invalid value: "invalid": cross-namespace routing is not permitted`, + ), }, { - msg: "missing port", + name: "invalid weight", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { - backend.Port = nil + backend.Weight = helpers.GetPointer[int32](-1) return backend }), - expErr: true, + expectedValid: false, + expectedCondition: conditions.NewRouteBackendRefUnsupportedValue( + "test.weight: Invalid value: -1: must be in the range [0, 1000000]", + ), }, } for _, test := range tests { - err := validateBackendRef(test.ref, "test") - errOccurred := err != nil - if errOccurred != test.expErr { - t.Errorf("validateBackendRef() returned incorrect error for %q; error: %v", test.msg, err) - } + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + valid, cond := validateBackendRef(test.ref, "test", field.NewPath("test")) + + g.Expect(valid).To(Equal(test.expectedValid)) + g.Expect(cond).To(Equal(test.expectedCondition)) + }) + } +} + +func TestValidateWeight(t *testing.T) { + validWeights := []int32{0, 1, 1000000} + invalidWeights := []int32{-1, 1000001} + + g := NewGomegaWithT(t) + + for _, w := range validWeights { + err := validateWeight(w) + g.Expect(err).ToNot(HaveOccurred(), "Expected weight %d to be valid", w) + } + for _, w := range invalidWeights { + err := validateWeight(w) + g.Expect(err).To(HaveOccurred(), "Expected weight %d to be invalid", w) } } @@ -109,28 +208,20 @@ func TestGetServiceAndPortFromRef(t *testing.T) { tests := []struct { ref v1beta1.BackendRef expService *v1.Service - msg string + name string expPort int32 expErr bool }{ { - msg: "normal case", + name: "normal case", ref: getNormalRef(), expService: svc1, expPort: 80, }, { - msg: "invalid backend ref", + name: "service does not exist", ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { - backend.Port = nil - return backend - }), - expErr: true, - }, - { - msg: "service does not exist", - ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { - backend.Name = "dne" + backend.Name = "does-not-exist" return backend }), expErr: true, @@ -142,28 +233,23 @@ func TestGetServiceAndPortFromRef(t *testing.T) { {Namespace: "test", Name: "service2"}: svc2, } - for _, test := range tests { - svc, port, err := getServiceAndPortFromRef(test.ref, "test", services) + refPath := field.NewPath("test") - errOccurred := err != nil - if errOccurred != test.expErr { - t.Errorf("getServiceAndPortFromRef() returned incorrect error for %q; error: %v", test.msg, err) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) - if svc != test.expService { - t.Errorf("getServiceAndPortFromRef() returned incorrect service for %q; expected: %v, got: %v", - test.msg, test.expService, svc) - } + svc, port, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) - if port != test.expPort { - t.Errorf("getServiceAndPortFromRef() returned incorrect port for %q; expected: %d, got: %d", - test.msg, test.expPort, port) - } + g.Expect(err != nil).To(Equal(test.expErr)) + g.Expect(svc).To(Equal(test.expService)) + g.Expect(port).To(Equal(test.expPort)) + }) } } -func TestResolveBackendRefs(t *testing.T) { - createRoute := func(name string, kind string, serviceNames ...string) *v1beta1.HTTPRoute { +func TestAddBackendGroupsToRouteTest(t *testing.T) { + createRoute := func(name string, kind v1beta1.Kind, refsPerBackend int, serviceNames ...string) *v1beta1.HTTPRoute { hr := &v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -171,84 +257,95 @@ func TestResolveBackendRefs(t *testing.T) { }, } + createHTTPBackendRef := func(svcName string, port v1beta1.PortNumber, weight *int32) v1beta1.HTTPBackendRef { + return v1beta1.HTTPBackendRef{ + BackendRef: v1beta1.BackendRef{ + BackendObjectReference: v1beta1.BackendObjectReference{ + Kind: helpers.GetPointer(kind), + Name: v1beta1.ObjectName(svcName), + Namespace: helpers.GetPointer[v1beta1.Namespace]("test"), + Port: helpers.GetPointer(port), + }, + Weight: weight, + }, + } + } + hr.Spec.Rules = make([]v1beta1.HTTPRouteRule, len(serviceNames)) for idx, svcName := range serviceNames { + refs := []v1beta1.HTTPBackendRef{ + createHTTPBackendRef(svcName, 80, nil), + } + if refsPerBackend == 2 { + refs = append(refs, createHTTPBackendRef(svcName, 81, helpers.GetPointer[int32](5))) + } + if refsPerBackend != 1 && refsPerBackend != 2 { + panic("invalid refsPerBackend") + } + hr.Spec.Rules[idx] = v1beta1.HTTPRouteRule{ - BackendRefs: []v1beta1.HTTPBackendRef{ - { - BackendRef: v1beta1.BackendRef{ - BackendObjectReference: v1beta1.BackendObjectReference{ - Kind: (*v1beta1.Kind)(helpers.GetStringPointer(kind)), - Name: v1beta1.ObjectName(svcName), - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), - }, - Weight: nil, // should use default weight of 1 - }, - }, - { - BackendRef: v1beta1.BackendRef{ - BackendObjectReference: v1beta1.BackendObjectReference{ - Kind: (*v1beta1.Kind)(helpers.GetStringPointer(kind)), - Name: v1beta1.ObjectName(svcName), - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(81)), - }, - Weight: helpers.GetInt32Pointer(5), - }, - }, - }, + BackendRefs: refs, } } return hr } - removeRefs := func(route *v1beta1.HTTPRoute) *v1beta1.HTTPRoute { - route.Spec.Rules[0].BackendRefs = nil - return route - } + const ( + allValid = true + allInvalid = false + ) - hr1 := createRoute("hr1", "Service", "svc1", "svc2", "svc3") - hr2 := createRoute("hr2", "Service", "svc1", "svc4") - hr3 := createRoute("hr3", "NotService", "not-svc") - hr4 := removeRefs(createRoute("hr4", "Service", "no-backend-refs")) + createRules := func(hr *v1beta1.HTTPRoute, validMatches, validFilters bool) []Rule { + rules := make([]Rule, len(hr.Spec.Rules)) + for i := range rules { + rules[i].ValidMatches = validMatches + rules[i].ValidFilters = validFilters + rules[i].BackendGroup = BackendGroup{ + Source: client.ObjectKeyFromObject(hr), + RuleIdx: i, + } + } + return rules + } - routes := map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr1"}: { - Source: hr1, - }, - {Namespace: "test", Name: "hr2"}: { - Source: hr2, - }, - {Namespace: "test", Name: "hr3"}: { - Source: hr3, - }, - {Namespace: "test", Name: "hr4"}: { - Source: hr4, + const sectionName = "test" + sectionNameRefs := map[string]ParentRef{ + sectionName: { + Idx: 0, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway"}, }, } + hrWithOneBackend := createRoute("hr1", "Service", 1, "svc1") + hrWithTwoBackends := createRoute("hr2", "Service", 2, "svc1") + hrWithInvalidRule := createRoute("hr3", "NotService", 1, "svc1") + hrWithZeroBackendRefs := createRoute("hr4", "Service", 1, "svc1") + hrWithZeroBackendRefs.Spec.Rules[0].BackendRefs = nil + svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc1"}} - svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc2"}} - svc3 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc3"}} - svc4 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc4"}} services := map[types.NamespacedName]*v1.Service{ {Namespace: "test", Name: "svc1"}: svc1, - {Namespace: "test", Name: "svc2"}: svc2, - {Namespace: "test", Name: "svc3"}: svc3, - {Namespace: "test", Name: "svc4"}: svc4, } - expRoutes := map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr1"}: { - Source: hr1, - BackendGroups: []BackendGroup{ + tests := []struct { + name string + route *Route + expectedBackendGroups []BackendGroup + expectedConditions []conditions.Condition + }{ + { + route: &Route{ + Source: hrWithOneBackend, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithOneBackend, allValid, allValid), + }, + expectedBackendGroups: []BackendGroup{ { - Source: client.ObjectKeyFromObject(hr1), + Source: client.ObjectKeyFromObject(hrWithOneBackend), RuleIdx: 0, - Errors: []string{}, Backends: []BackendRef{ { Name: "test_svc1_80", @@ -257,66 +354,23 @@ func TestResolveBackendRefs(t *testing.T) { Valid: true, Weight: 1, }, - { - Name: "test_svc1_81", - Svc: svc1, - Port: 81, - Valid: true, - Weight: 5, - }, - }, - }, - { - Source: client.ObjectKeyFromObject(hr1), - RuleIdx: 1, - Errors: []string{}, - Backends: []BackendRef{ - { - Name: "test_svc2_80", - Svc: svc2, - Port: 80, - Valid: true, - Weight: 1, - }, - { - Name: "test_svc2_81", - Svc: svc2, - Port: 81, - Valid: true, - Weight: 5, - }, - }, - }, - { - Source: client.ObjectKeyFromObject(hr1), - RuleIdx: 2, - Errors: []string{}, - Backends: []BackendRef{ - { - Name: "test_svc3_80", - Svc: svc3, - Port: 80, - Valid: true, - Weight: 1, - }, - { - Name: "test_svc3_81", - Svc: svc3, - Port: 81, - Valid: true, - Weight: 5, - }, }, }, }, + expectedConditions: nil, + name: "normal case with one rule with one backend", }, - {Namespace: "test", Name: "hr2"}: { - Source: hr2, - BackendGroups: []BackendGroup{ + { + route: &Route{ + Source: hrWithTwoBackends, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithTwoBackends, allValid, allValid), + }, + expectedBackendGroups: []BackendGroup{ { - Source: client.ObjectKeyFromObject(hr2), + Source: client.ObjectKeyFromObject(hrWithTwoBackends), RuleIdx: 0, - Errors: []string{}, Backends: []BackendRef{ { Name: "test_svc1_80", @@ -334,64 +388,215 @@ func TestResolveBackendRefs(t *testing.T) { }, }, }, - { - Source: client.ObjectKeyFromObject(hr2), - RuleIdx: 1, - Errors: []string{}, - Backends: []BackendRef{ - { - Name: "test_svc4_80", - Svc: svc4, - Port: 80, - Valid: true, - Weight: 1, - }, - { - Name: "test_svc4_81", - Svc: svc4, - Port: 81, - Valid: true, - Weight: 5, - }, - }, - }, }, + expectedConditions: nil, + name: "normal case with one rule with two backends", + }, + { + route: &Route{ + Source: hrWithOneBackend, + SectionNameRefs: sectionNameRefs, + Valid: false, + }, + expectedBackendGroups: nil, + expectedConditions: nil, + name: "invalid route", + }, + { + route: &Route{ + Source: hrWithOneBackend, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithOneBackend, allInvalid, allValid), + }, + expectedBackendGroups: nil, + expectedConditions: nil, + name: "invalid matches", + }, + { + route: &Route{ + Source: hrWithOneBackend, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithOneBackend, allValid, allInvalid), + }, + expectedBackendGroups: nil, + expectedConditions: nil, + name: "invalid filters", }, - {Namespace: "test", Name: "hr3"}: { - Source: hr3, - BackendGroups: []BackendGroup{ + { + route: &Route{ + Source: hrWithInvalidRule, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithInvalidRule, allValid, allValid), + }, + expectedBackendGroups: []BackendGroup{ { - Errors: []string{ - "the Kind must be Service; got NotService", - "the Kind must be Service; got NotService", - }, - Source: client.ObjectKeyFromObject(hr3), + Source: client.ObjectKeyFromObject(hrWithInvalidRule), RuleIdx: 0, Backends: []BackendRef{ { Weight: 1, }, - { - Weight: 5, - }, }, }, }, + expectedConditions: []conditions.Condition{ + conditions.NewRouteBackendRefInvalidKind( + `spec.rules[0].backendRefs[0].kind: Unsupported value: "NotService": supported values: "Service"`, + ), + }, + name: "invalid backendRef", }, - {Namespace: "test", Name: "hr4"}: { - Source: hr4, - BackendGroups: []BackendGroup{ + { + route: &Route{ + Source: hrWithZeroBackendRefs, + SectionNameRefs: sectionNameRefs, + Valid: true, + Rules: createRules(hrWithZeroBackendRefs, allValid, allValid), + }, + expectedBackendGroups: []BackendGroup{ { - Source: client.ObjectKeyFromObject(hr4), + Source: client.ObjectKeyFromObject(hrWithZeroBackendRefs), RuleIdx: 0, }, }, + expectedConditions: nil, + name: "zero backendRefs", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + addBackendGroupsToRoute(test.route, services) + + g.Expect(helpers.Diff(test.expectedBackendGroups, test.route.GetAllBackendGroups())).To(BeEmpty()) + g.Expect(test.route.GetAllConditionsForSectionName(sectionName)).To(Equal(test.expectedConditions)) + }) + } +} + +func TestCreateBackend(t *testing.T) { + svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "service1"}} + + tests := []struct { + name string + ref v1beta1.HTTPBackendRef + expectedCondition *conditions.Condition + expectedBackend BackendRef + }{ + { + ref: v1beta1.HTTPBackendRef{ + BackendRef: getNormalRef(), + }, + expectedBackend: BackendRef{ + Svc: svc1, + Name: "test_service1_80", + Port: 80, + Weight: 5, + Valid: true, + }, + expectedCondition: nil, + name: "normal case", + }, + { + ref: v1beta1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Weight = nil + return backend + }), + }, + expectedBackend: BackendRef{ + Svc: svc1, + Name: "test_service1_80", + Port: 80, + Weight: 1, + Valid: true, + }, + expectedCondition: nil, + name: "normal with nil weight", + }, + { + ref: v1beta1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Weight = helpers.GetPointer[int32](-1) + return backend + }), + }, + expectedBackend: BackendRef{ + Svc: nil, + Name: "", + Port: 0, + Weight: 0, + Valid: false, + }, + expectedCondition: helpers.GetPointer( + conditions.NewRouteBackendRefUnsupportedValue( + "test.weight: Invalid value: -1: must be in the range [0, 1000000]", + ), + ), + name: "invalid weight", + }, + { + ref: v1beta1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Kind = helpers.GetPointer[v1beta1.Kind]("NotService") + return backend + }), + }, + expectedBackend: BackendRef{ + Svc: nil, + Name: "", + Port: 0, + Weight: 5, + Valid: false, + }, + expectedCondition: helpers.GetPointer( + conditions.NewRouteBackendRefInvalidKind( + `test.kind: Unsupported value: "NotService": supported values: "Service"`, + ), + ), + name: "invalid kind", + }, + { + ref: v1beta1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Name = "not-exist" + return backend + }), + }, + expectedBackend: BackendRef{ + Svc: nil, + Name: "", + Port: 0, + Weight: 5, + Valid: false, + }, + expectedCondition: helpers.GetPointer( + conditions.NewRouteBackendRefRefBackendNotFound(`test.name: Not found: "not-exist"`), + ), + name: "service doesn't exist", }, } - addBackendGroupsToRoutes(routes, services) + services := map[types.NamespacedName]*v1.Service{ + client.ObjectKeyFromObject(svc1): svc1, + } + sourceNamespace := "test" + + refPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + backend, cond := createBackend(test.ref, sourceNamespace, services, refPath) - if diff := cmp.Diff(expRoutes, routes); diff != "" { - t.Errorf("resolveBackendRefs() mismatch on routes (-want +got):\n%s", diff) + g.Expect(helpers.Diff(test.expectedBackend, backend)).To(BeEmpty()) + g.Expect(cond).To(Equal(test.expectedCondition)) + }) } } diff --git a/internal/state/graph/doc.go b/internal/state/graph/doc.go index 1b4e99a7be..a74b2fd42b 100644 --- a/internal/state/graph/doc.go +++ b/internal/state/graph/doc.go @@ -10,5 +10,12 @@ Those three points make it easier to generate intermediate data plane configurat The package includes the types to represent the graph and the functions to convert resources into their graph representation. + +The validation of the resource fields consists of two parts: +- Data-plane specific validation. For example, validating the value of an HTTP header. Such validation is delegated +to the data-plane specific implementation of a Validator. +- Data-plane agnostic validation. For such validation, the values either don't affect the data-plane configuration +directly or they must be validated to process a resource. For example, hostnames must be validated to be able to bind +an HTTPRoute to a Listener. */ package graph diff --git a/internal/state/graph/gateway.go b/internal/state/graph/gateway.go index d97425db0b..5b9c35b77e 100644 --- a/internal/state/graph/gateway.go +++ b/internal/state/graph/gateway.go @@ -1,13 +1,10 @@ package graph import ( - "errors" "fmt" "sort" - "strings" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -30,6 +27,7 @@ type Listener struct { // Source holds the source of the Listener from the Gateway resource. Source v1beta1.Listener // Routes holds the routes attached to the Listener. + // Only valid routes are attached. Routes map[types.NamespacedName]*Route // AcceptedHostnames is an intersection between the hostnames supported by the Listener and the hostnames // from the attached routes. @@ -43,13 +41,41 @@ type Listener struct { Valid bool } -// processGateways determines which Gateway resource the NGINX Gateway will use (the winner) and which Gateway(s) will -// be ignored. Note that the function will not take into the account any unrelated Gateway resources - the ones with the -// different GatewayClassName field. +// processedGateways holds the resources that belong to NKG. +type processedGateways struct { + Winner *v1beta1.Gateway + Ignored map[types.NamespacedName]*v1beta1.Gateway +} + +// GetAllNsNames returns all the NamespacedNames of the Gateway resources that belong to NKG +func (gws processedGateways) GetAllNsNames() []types.NamespacedName { + winnerCnt := 0 + if gws.Winner != nil { + winnerCnt = 1 + } + + length := winnerCnt + len(gws.Ignored) + if length == 0 { + return nil + } + + allNsNames := make([]types.NamespacedName, 0, length) + + if gws.Winner != nil { + allNsNames = append(allNsNames, client.ObjectKeyFromObject(gws.Winner)) + } + for nsName := range gws.Ignored { + allNsNames = append(allNsNames, nsName) + } + + return allNsNames +} + +// processGateways determines which Gateway resource belong to NKG (determined by the Gateway GatewayClassName field). func processGateways( gws map[types.NamespacedName]*v1beta1.Gateway, gcName string, -) (winner *v1beta1.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway) { +) processedGateways { referencedGws := make([]*v1beta1.Gateway, 0, len(gws)) for _, gw := range gws { @@ -61,7 +87,7 @@ func processGateways( } if len(referencedGws) == 0 { - return nil, nil + return processedGateways{} } sort.Slice(referencedGws, func(i, j int) bool { @@ -74,20 +100,29 @@ func processGateways( ignoredGws[client.ObjectKeyFromObject(gw)] = gw } - return referencedGws[0], ignoredGws + return processedGateways{ + Winner: referencedGws[0], + Ignored: ignoredGws, + } +} + +func buildGateway(gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryManager) *Gateway { + if gw == nil { + return nil + } + + return &Gateway{ + Source: gw, + Listeners: buildListeners(gw, secretMemoryMgr), + } } func buildListeners( gw *v1beta1.Gateway, - gcName string, secretMemoryMgr secrets.SecretDiskMemoryManager, ) map[string]*Listener { listeners := make(map[string]*Listener) - if gw == nil || string(gw.Spec.GatewayClassName) != gcName { - return listeners - } - listenerFactory := newListenerConfiguratorFactory(gw, secretMemoryMgr) for _, gl := range gw.Spec.Listeners { @@ -338,16 +373,5 @@ func validateListenerHostname(host *v1beta1.Hostname) error { return nil } - // FIXME(pleshakov): For now, we don't support wildcard hostnames - if strings.HasPrefix(h, "*") { - return fmt.Errorf("wildcard hostnames are not supported") - } - - msgs := validation.IsDNS1123Subdomain(h) - if len(msgs) > 0 { - combined := strings.Join(msgs, ",") - return errors.New(combined) - } - - return nil + return validateHostname(h) } diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index c9d70fb622..b97cd44059 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -1,19 +1,69 @@ +//nolint:gosec package graph import ( + "errors" "testing" - "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" ) +func TestProcessedGatewaysGetAllNsNames(t *testing.T) { + winner := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway-1", + }, + } + loser := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway-2", + }, + } + + tests := []struct { + gws processedGateways + name string + expected []types.NamespacedName + }{ + { + gws: processedGateways{}, + expected: nil, + name: "no gateways", + }, + { + gws: processedGateways{ + Winner: winner, + Ignored: map[types.NamespacedName]*v1beta1.Gateway{ + client.ObjectKeyFromObject(loser): loser, + }, + }, + expected: []types.NamespacedName{ + client.ObjectKeyFromObject(winner), + client.ObjectKeyFromObject(loser), + }, + name: "winner and ignored", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := test.gws.GetAllNsNames() + g.Expect(result).To(Equal(test.expected)) + }) + } +} + func TestProcessGateways(t *testing.T) { const gcName = "test-gc" @@ -37,16 +87,14 @@ func TestProcessGateways(t *testing.T) { } tests := []struct { - gws map[types.NamespacedName]*v1beta1.Gateway - expectedWinner *v1beta1.Gateway - expectedIgnoredGws map[types.NamespacedName]*v1beta1.Gateway - msg string + gws map[types.NamespacedName]*v1beta1.Gateway + expected processedGateways + name string }{ { - gws: nil, - expectedWinner: nil, - expectedIgnoredGws: nil, - msg: "no gateways", + gws: nil, + expected: processedGateways{}, + name: "no gateways", }, { gws: map[types.NamespacedName]*v1beta1.Gateway{ @@ -54,44 +102,44 @@ func TestProcessGateways(t *testing.T) { Spec: v1beta1.GatewaySpec{GatewayClassName: "some-class"}, }, }, - expectedWinner: nil, - expectedIgnoredGws: nil, - msg: "unrelated gateway", + expected: processedGateways{}, + name: "unrelated gateway", }, { gws: map[types.NamespacedName]*v1beta1.Gateway{ - {Namespace: "test", Name: "gateway"}: winner, + {Namespace: "test", Name: "gateway-1"}: winner, }, - expectedWinner: winner, - expectedIgnoredGws: map[types.NamespacedName]*v1beta1.Gateway{}, - msg: "one gateway", + expected: processedGateways{ + Winner: winner, + Ignored: map[types.NamespacedName]*v1beta1.Gateway{}, + }, + name: "one gateway", }, { gws: map[types.NamespacedName]*v1beta1.Gateway{ {Namespace: "test", Name: "gateway-1"}: winner, {Namespace: "test", Name: "gateway-2"}: loser, }, - expectedWinner: winner, - expectedIgnoredGws: map[types.NamespacedName]*v1beta1.Gateway{ - {Namespace: "test", Name: "gateway-2"}: loser, + expected: processedGateways{ + Winner: winner, + Ignored: map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "gateway-2"}: loser, + }, }, - msg: "multiple gateways", + name: "multiple gateways", }, } for _, test := range tests { - winner, ignoredGws := processGateways(test.gws, gcName) - - if diff := cmp.Diff(winner, test.expectedWinner); diff != "" { - t.Errorf("processGateways() '%s' mismatch for winner (-want +got):\n%s", test.msg, diff) - } - if diff := cmp.Diff(ignoredGws, test.expectedIgnoredGws); diff != "" { - t.Errorf("processGateways() '%s' mismatch for ignored gateways (-want +got):\n%s", test.msg, diff) - } + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := processGateways(test.gws, gcName) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) } } -func TestBuildListeners(t *testing.T) { +func TestBuildGateway(t *testing.T) { const gcName = "my-gateway-class" listener801 := v1beta1.Listener{ @@ -203,316 +251,272 @@ func TestBuildListeners(t *testing.T) { conflictedHostnamesMsg = `Multiple listeners for the same port use the same hostname "foo.example.com"; ` + "ensure only one listener uses that hostname" + + secretPath = "/etc/nginx/secrets/test_secret" ) + type gatewayCfg struct { + listeners []v1beta1.Listener + addresses []v1beta1.GatewayAddress + } + + var lastCreatedGateway *v1beta1.Gateway + createGateway := func(cfg gatewayCfg) *v1beta1.Gateway { + lastCreatedGateway = &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + }, + Spec: v1beta1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: cfg.listeners, + Addresses: cfg.addresses, + }, + } + return lastCreatedGateway + } + getLastCreatedGetaway := func() *v1beta1.Gateway { + return lastCreatedGateway + } + tests := []struct { gateway *v1beta1.Gateway - expected map[string]*Listener + expected *Gateway name string }{ { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener801, + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-1": { + Source: listener801, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, }, }, - Status: v1beta1.GatewayStatus{}, - }, - expected: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - }, }, name: "valid http listener", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener4431, + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4431}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-443-1": { + Source: listener4431, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + SecretPath: secretPath, }, }, }, - expected: map[string]*Listener{ - "listener-443-1": { - Source: listener4431, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, - }, - }, name: "valid https listener", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener802, - }, - }, - }, - expected: map[string]*Listener{ - "listener-80-2": { - Source: listener802, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedProtocol(`Protocol "TCP" is not supported, use "HTTP" ` + - `or "HTTPS"`), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener802}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-2": { + Source: listener802, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedProtocol(`Protocol "TCP" is not supported, use "HTTP" ` + + `or "HTTPS"`), + }, }, }, }, name: "invalid listener protocol", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener805, - }, - }, - }, - expected: map[string]*Listener{ - "listener-80-5": { - Source: listener805, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable("Port 81 is not supported for HTTP, use 80"), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener805}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-5": { + Source: listener805, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerPortUnavailable("Port 81 is not supported for HTTP, use 80"), + }, }, }, }, name: "invalid http listener", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener4436, - }, - }, - }, - expected: map[string]*Listener{ - "listener-443-6": { - Source: listener4436, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable("Port 444 is not supported for HTTPS, use 443"), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4436}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-443-6": { + Source: listener4436, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerPortUnavailable("Port 444 is not supported for HTTPS, use 443"), + }, }, }, }, name: "invalid https listener", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener806, - listener4434, + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener806, listener4434}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-6": { + Source: listener806, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue(invalidHostnameMsg), + }, }, - }, - }, - expected: map[string]*Listener{ - "listener-80-6": { - Source: listener806, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedValue(invalidHostnameMsg), - }, - }, - "listener-443-4": { - Source: listener4434, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedValue(invalidHostnameMsg), + "listener-443-4": { + Source: listener4434, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue(invalidHostnameMsg), + }, }, }, }, name: "invalid hostnames", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener4435, + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4435}}), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-443-5": { + Source: listener4435, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: conditions.NewListenerInvalidCertificateRef("Failed to get the certificate " + + "test/does-not-exist: secret not found"), }, }, }, - expected: map[string]*Listener{ - "listener-443-5": { - Source: listener4435, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerInvalidCertificateRef("Failed to get the certificate " + - "test/does-not-exist: secret test/does-not-exist does not exist"), - }, - }, name: "invalid https listener (secret does not exist)", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener801, listener803, - listener4431, listener4432, + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{listener801, listener803, listener4431, listener4432}}, + ), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-1": { + Source: listener801, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + }, + "listener-80-3": { + Source: listener803, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + }, + "listener-443-1": { + Source: listener4431, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + SecretPath: secretPath, + }, + "listener-443-2": { + Source: listener4432, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + SecretPath: secretPath, }, - }, - }, - expected: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - }, - "listener-80-3": { - Source: listener803, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - }, - "listener-443-1": { - Source: listener4431, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, - }, - "listener-443-2": { - Source: listener4432, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, }, }, name: "multiple valid http/https listeners", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener801, listener804, - listener4431, listener4433, + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{listener801, listener804, listener4431, listener4433}}, + ), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-1": { + Source: listener801, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + }, + "listener-80-4": { + Source: listener804, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + }, + "listener-443-1": { + Source: listener4431, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + }, + "listener-443-3": { + Source: listener4433, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), }, - }, - }, - expected: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), - }, - "listener-80-4": { - Source: listener804, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), - }, - "listener-443-1": { - Source: listener4431, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), - }, - "listener-443-3": { - Source: listener4433, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), }, }, name: "collisions", }, { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1beta1.Listener{ - listener801, - listener4431, - }, - Addresses: []v1beta1.GatewayAddress{ - {}, + gateway: createGateway(gatewayCfg{ + listeners: []v1beta1.Listener{listener801, listener4431}, + addresses: []v1beta1.GatewayAddress{ + {}, + }, + }), + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-80-1": { + Source: listener801, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedAddress("Specifying Gateway addresses is not supported"), + }, }, - }, - }, - expected: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedAddress("Specifying Gateway addresses is not supported"), - }, - }, - "listener-443-1": { - Source: listener4431, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: "", - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedAddress("Specifying Gateway addresses is not supported"), + "listener-443-1": { + Source: listener4431, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + AcceptedHostnames: map[string]struct{}{}, + SecretPath: "", + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedAddress("Specifying Gateway addresses is not supported"), + }, }, }, }, @@ -520,37 +524,23 @@ func TestBuildListeners(t *testing.T) { }, { gateway: nil, - expected: map[string]*Listener{}, - name: "no gateway", - }, - { - gateway: &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: v1beta1.GatewaySpec{ - GatewayClassName: "wrong-class", - Listeners: []v1beta1.Listener{ - listener801, listener804, - }, - }, - }, - expected: map[string]*Listener{}, - name: "wrong gatewayclass", + expected: nil, + name: "nil gateway", }, } - // add secret to store - secretStore := secrets.NewSecretStore() - secretStore.Upsert(testSecret) - - secretMemoryMgr := secrets.NewSecretDiskMemoryManager(secretsDirectory, secretStore) + secretMemoryMgr := &secretsfakes.FakeSecretDiskMemoryManager{} + secretMemoryMgr.RequestCalls(func(nsname types.NamespacedName) (string, error) { + if (nsname == types.NamespacedName{Namespace: "test", Name: "secret"}) { + return secretPath, nil + } + return "", errors.New("secret not found") + }) for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - - result := buildListeners(test.gateway, gcName, secretMemoryMgr) + result := buildGateway(test.gateway, secretMemoryMgr) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index 32d221cd25..c1ee3001ab 100644 --- a/internal/state/graph/graph.go +++ b/internal/state/graph/graph.go @@ -3,10 +3,10 @@ package graph import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" ) // ClusterStore includes cluster resources necessary to build the Graph. @@ -37,34 +37,23 @@ func BuildGraph( controllerName string, gcName string, secretMemoryMgr secrets.SecretDiskMemoryManager, + validators validation.Validators, ) *Graph { gc := buildGatewayClass(store.GatewayClass, controllerName) - gw, ignoredGws := processGateways(store.Gateways, gcName) + processedGws := processGateways(store.Gateways, gcName) - listeners := buildListeners(gw, gcName, secretMemoryMgr) - - routes := make(map[types.NamespacedName]*Route) - for _, ghr := range store.HTTPRoutes { - ignored, r := bindHTTPRouteToListeners(ghr, gw, ignoredGws, listeners) - if !ignored { - routes[client.ObjectKeyFromObject(ghr)] = r - } - } + gw := buildGateway(processedGws.Winner, secretMemoryMgr) + routes := buildRoutesForGateways(validators.HTTPFieldsValidator, store.HTTPRoutes, processedGws.GetAllNsNames()) + bindRoutesToListeners(routes, gw) addBackendGroupsToRoutes(routes, store.Services) g := &Graph{ GatewayClass: gc, + Gateway: gw, Routes: routes, - IgnoredGateways: ignoredGws, - } - - if gw != nil { - g.Gateway = &Gateway{ - Source: gw, - Listeners: listeners, - } + IgnoredGateways: processedGws.Ignored, } return g diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index 61be3767de..2041ebaaf3 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -4,85 +4,35 @@ package graph import ( "testing" - "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" -) - -var testSecret = &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "secret", - Namespace: "test", - }, - Data: map[string][]byte{ - v1.TLSCertKey: []byte(`-----BEGIN CERTIFICATE----- -MIIDLjCCAhYCCQDAOF9tLsaXWjANBgkqhkiG9w0BAQsFADBaMQswCQYDVQQGEwJV -UzELMAkGA1UECAwCQ0ExITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0 -ZDEbMBkGA1UEAwwSY2FmZS5leGFtcGxlLmNvbSAgMB4XDTE4MDkxMjE2MTUzNVoX -DTIzMDkxMTE2MTUzNVowWDELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMSEwHwYD -VQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQxGTAXBgNVBAMMEGNhZmUuZXhh -bXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCp6Kn7sy81 -p0juJ/cyk+vCAmlsfjtFM2muZNK0KtecqG2fjWQb55xQ1YFA2XOSwHAYvSdwI2jZ -ruW8qXXCL2rb4CZCFxwpVECrcxdjm3teViRXVsYImmJHPPSyQgpiobs9x7DlLc6I -BA0ZjUOyl0PqG9SJexMV73WIIa5rDVSF2r4kSkbAj4Dcj7LXeFlVXH2I5XwXCptC -n67JCg42f+k8wgzcRVp8XZkZWZVjwq9RUKDXmFB2YyN1XEWdZ0ewRuKYUJlsm692 -skOrKQj0vkoPn41EE/+TaVEpqLTRoUY3rzg7DkdzfdBizFO2dsPNFx2CW0jXkNLv -Ko25CZrOhXAHAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAKHFCcyOjZvoHswUBMdL -RdHIb383pWFynZq/LuUovsVA58B0Cg7BEfy5vWVVrq5RIkv4lZ81N29x21d1JH6r -jSnQx+DXCO/TJEV5lSCUpIGzEUYaUPgRyjsM/NUdCJ8uHVhZJ+S6FA+CnOD9rn2i -ZBePCI5rHwEXwnnl8ywij3vvQ5zHIuyBglWr/Qyui9fjPpwWUvUm4nv5SMG9zCV7 -PpuwvuatqjO1208BjfE/cZHIg8Hw9mvW9x9C+IQMIMDE7b/g6OcK7LGTLwlFxvA8 -7WjEequnayIphMhKRXVf1N349eN98Ez38fOTHTPbdJjFA/PcC+Gyme+iGt5OQdFh -yRE= ------END CERTIFICATE-----`), - v1.TLSPrivateKeyKey: []byte(`-----BEGIN RSA PRIVATE KEY----- -MIIEowIBAAKCAQEAqeip+7MvNadI7if3MpPrwgJpbH47RTNprmTStCrXnKhtn41k -G+ecUNWBQNlzksBwGL0ncCNo2a7lvKl1wi9q2+AmQhccKVRAq3MXY5t7XlYkV1bG -CJpiRzz0skIKYqG7Pcew5S3OiAQNGY1DspdD6hvUiXsTFe91iCGuaw1Uhdq+JEpG -wI+A3I+y13hZVVx9iOV8FwqbQp+uyQoONn/pPMIM3EVafF2ZGVmVY8KvUVCg15hQ -dmMjdVxFnWdHsEbimFCZbJuvdrJDqykI9L5KD5+NRBP/k2lRKai00aFGN684Ow5H -c33QYsxTtnbDzRcdgltI15DS7yqNuQmazoVwBwIDAQABAoIBAQCPSdSYnQtSPyql -FfVFpTOsoOYRhf8sI+ibFxIOuRauWehhJxdm5RORpAzmCLyL5VhjtJme223gLrw2 -N99EjUKb/VOmZuDsBc6oCF6QNR58dz8cnORTewcotsJR1pn1hhlnR5HqJJBJask1 -ZEnUQfcXZrL94lo9JH3E+Uqjo1FFs8xxE8woPBqjZsV7pRUZgC3LhxnwLSExyFo4 -cxb9SOG5OmAJozStFoQ2GJOes8rJ5qfdvytgg9xbLaQL/x0kpQ62BoFMBDdqOePW -KfP5zZ6/07/vpj48yA1Q32PzobubsBLd3Kcn32jfm1E7prtWl+JeOFiOznBQFJbN -4qPVRz5hAoGBANtWyxhNCSLu4P+XgKyckljJ6F5668fNj5CzgFRqJ09zn0TlsNro -FTLZcxDqnR3HPYM42JERh2J/qDFZynRQo3cg3oeivUdBVGY8+FI1W0qdub/L9+yu -edOZTQ5XmGGp6r6jexymcJim/OsB3ZnYOpOrlD7SPmBvzNLk4MF6gxbXAoGBAMZO -0p6HbBmcP0tjFXfcKE77ImLm0sAG4uHoUx0ePj/2qrnTnOBBNE4MvgDuTJzy+caU -k8RqmdHCbHzTe6fzYq/9it8sZ77KVN1qkbIcuc+RTxA9nNh1TjsRne74Z0j1FCLk -hHcqH0ri7PYSKHTE8FvFCxZYdbuB84CmZihvxbpRAoGAIbjqaMYPTYuklCda5S79 -YSFJ1JzZe1Kja//tDw1zFcgVCKa31jAwciz0f/lSRq3HS1GGGmezhPVTiqLfeZqc -R0iKbhgbOcVVkJJ3K0yAyKwPTumxKHZ6zImZS0c0am+RY9YGq5T7YrzpzcfvpiOU -ffe3RyFT7cfCmfoOhDCtzukCgYB30oLC1RLFOrqn43vCS51zc5zoY44uBzspwwYN -TwvP/ExWMf3VJrDjBCH+T/6sysePbJEImlzM+IwytFpANfiIXEt/48Xf60Nx8gWM -uHyxZZx/NKtDw0V8vX1POnq2A5eiKa+8jRARYKJLYNdfDuwolxvG6bZhkPi/4EtT -3Y18sQKBgHtKbk+7lNJVeswXE5cUG6EDUsDe/2Ua7fXp7FcjqBEoap1LSw+6TXp0 -ZgrmKE8ARzM47+EJHUviiq/nupE15g0kJW3syhpU9zZLO7ltB0KIkO9ZRcmUjo8Q -cpLlHMAqbLJ8WYGJCkhiWxyal6hYTyWY4cVkC0xtTl/hUE9IeNKo ------END RSA PRIVATE KEY-----`), - }, - Type: v1.SecretTypeTLS, -} - -var ( - secretPath = "/etc/nginx/secrets/test_secret" - secretsDirectory = "/etc/nginx/secrets" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation/validationfakes" ) func TestBuildGraph(t *testing.T) { const ( gcName = "my-class" controllerName = "my.controller" + secretPath = "/etc/nginx/secrets/test_secret" ) + createValidRuleWithBackendGroup := func(group BackendGroup) Rule { + return Rule{ + ValidMatches: true, + ValidFilters: true, + BackendGroup: group, + } + } + createRoute := func(name string, gatewayName string, listenerName string) *v1beta1.HTTPRoute { return &v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -107,6 +57,7 @@ func TestBuildGraph(t *testing.T) { Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), Value: helpers.GetStringPointer("/"), }, }, @@ -136,7 +87,6 @@ func TestBuildGraph(t *testing.T) { fooSvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"}} hr1Group := BackendGroup{ - Errors: []string{}, Source: types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, RuleIdx: 0, Backends: []BackendRef{ @@ -151,7 +101,6 @@ func TestBuildGraph(t *testing.T) { } hr3Group := BackendGroup{ - Errors: []string{}, Source: types.NamespacedName{Namespace: hr3.Namespace, Name: hr3.Name}, RuleIdx: 0, Backends: []BackendRef{ @@ -228,27 +177,38 @@ func TestBuildGraph(t *testing.T) { } routeHR1 := &Route{ + Valid: true, Source: hr1, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, + SectionNameRefs: map[string]ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + }, }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, - BackendGroups: []BackendGroup{hr1Group}, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Rules: []Rule{createValidRuleWithBackendGroup(hr1Group)}, } routeHR3 := &Route{ + Valid: true, Source: hr3, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-1": {}, + SectionNameRefs: map[string]ParentRef{ + "listener-443-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + }, }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, - BackendGroups: []BackendGroup{hr3Group}, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Rules: []Rule{createValidRuleWithBackendGroup(hr3Group)}, } - // add test secret to store - secretStore := secrets.NewSecretStore() - secretStore.Upsert(testSecret) - secretMemoryMgr := secrets.NewSecretDiskMemoryManager(secretsDirectory, secretStore) + secretMemoryMgr := &secretsfakes.FakeSecretDiskMemoryManager{} + secretMemoryMgr.RequestCalls(func(nsname types.NamespacedName) (string, error) { + if (nsname == types.NamespacedName{Namespace: "test", Name: "secret"}) { + return secretPath, nil + } + panic("unexpected secret request") + }) expected := &Graph{ GatewayClass: &GatewayClass{ @@ -290,8 +250,9 @@ func TestBuildGraph(t *testing.T) { }, } - result := BuildGraph(store, controllerName, gcName, secretMemoryMgr) - if diff := cmp.Diff(expected, result); diff != "" { - t.Errorf("BuildGraph() mismatch (-want +got):\n%s", diff) - } + g := NewGomegaWithT(t) + + result := BuildGraph(store, controllerName, gcName, secretMemoryMgr, + validation.Validators{HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}}) + g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index 1c5e02803b..b2adae02e1 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -1,13 +1,37 @@ package graph import ( + "fmt" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" ) +// Rule represents a rule of an HTTPRoute. +type Rule struct { + // BackendGroup is the BackendGroup of the rule. + BackendGroup BackendGroup + // ValidMatches indicates whether the matches of the rule are valid. + // If the matches are invalid, NGK should not generate any configuration for the rule. + ValidMatches bool + // ValidFilters indicates whether the filters of the rule are valid. + // If the filters are invalid, the data-plane should return 500 error provided that the matches are valid. + ValidFilters bool +} + +// ParentRef describes a reference to a parent in an HTTPRoute. +type ParentRef struct { + // Gateway is the NamespacedName of the referenced Gateway + Gateway types.NamespacedName + // Idx is the index of the reference in the HTTPRoute. + Idx int +} + // Route represents an HTTPRoute. type Route struct { // Source is the source resource of the Route. @@ -15,132 +39,332 @@ type Route struct { // For now, we assume that the source is only HTTPRoute. // Later we can support more types - TLSRoute, TCPRoute and UDPRoute. Source *v1beta1.HTTPRoute + // SectionNameRefs is a map of section names to the referenced NKG Gateways + SectionNameRefs map[string]ParentRef + // UnattachedSectionNameRefs is a subset of SectionNameRefs that includes sections that could not be attached + // to the referenced Gateway. For example, because section does not exist in the Gateway. + UnattachedSectionNameRefs map[string]conditions.Condition + // Conditions include Conditions for the HTTPRoute. + Conditions []conditions.Condition + // Rules include Rules for the HTTPRoute. Each Rule[i] corresponds to the ith HTTPRouteRule. + // If the Route is invalid, this field is nil + Rules []Rule + // Valid tells if the Route is valid. + // If it is invalid, NGK should not generate any configuration for it. + Valid bool +} - // ValidSectionNameRefs includes the sectionNames from the parentRefs of the HTTPRoute that are valid -- i.e. - // the Gateway resource has a corresponding valid listener. - ValidSectionNameRefs map[string]struct{} - // InvalidSectionNameRefs includes the sectionNames from the parentRefs of the HTTPRoute that are invalid. - // The Condition describes why the sectionName is invalid. - InvalidSectionNameRefs map[string]conditions.Condition - // BackendGroups includes the backend groups of the HTTPRoute. - // There's one BackendGroup per rule in the HTTPRoute. - // The BackendGroups are stored in order of the rules. - // Ex: Source.Spec.Rules[0] -> BackendGroups[0]. - BackendGroups []BackendGroup -} - -// bindHTTPRouteToListeners tries to bind an HTTPRoute to listener. -// There are three possibilities: -// (1) HTTPRoute will be ignored. -// (2) HTTPRoute will be processed but not bound. -// (3) HTTPRoute will be processed and bound to a listener. -func bindHTTPRouteToListeners( - ghr *v1beta1.HTTPRoute, - gw *v1beta1.Gateway, - ignoredGws map[types.NamespacedName]*v1beta1.Gateway, - listeners map[string]*Listener, -) (ignored bool, r *Route) { - if len(ghr.Spec.ParentRefs) == 0 { - // ignore HTTPRoute without refs - return true, nil +// GetAllBackendGroups returns BackendGroups for all rules with valid matches and filters in the Route. +// +// FIXME(pleshakov) Improve the name once https://github.com/nginxinc/nginx-kubernetes-gateway/issues/468 is +// implemented. The current name doesn't reflect the filtering of rules inside the loops. +// See also the discussion below for more context: +// https://github.com/nginxinc/nginx-kubernetes-gateway/pull/455#discussion_r1136277589 +func (r *Route) GetAllBackendGroups() []BackendGroup { + count := 0 + + for _, rule := range r.Rules { + if rule.ValidMatches && rule.ValidFilters { + count++ + } + } + + if count == 0 { + return nil + } + + groups := make([]BackendGroup, 0, count) + + for _, rule := range r.Rules { + if rule.ValidMatches && rule.ValidFilters { + groups = append(groups, rule.BackendGroup) + } + } + + return groups +} + +// GetAllConditionsForSectionName returns all Conditions for the referenced section name. +// It panics if the section name does not exist. +func (r *Route) GetAllConditionsForSectionName(name string) []conditions.Condition { + if _, exist := r.SectionNameRefs[name]; !exist { + panic(fmt.Errorf("section name %q does not exist", name)) + } + + count := len(r.Conditions) + + unattachedCond, sectionIsUnattached := r.UnattachedSectionNameRefs[name] + if sectionIsUnattached { + count++ } - r = &Route{ - Source: ghr, - ValidSectionNameRefs: make(map[string]struct{}), - InvalidSectionNameRefs: make(map[string]conditions.Condition), + if count == 0 { + return nil } - // FIXME (pleshakov) Handle the case when parent refs are duplicated + conds := make([]conditions.Condition, 0, count) - processed := false + if sectionIsUnattached { + conds = append(conds, unattachedCond) + } + + conds = append(conds, r.Conditions...) + + return conds +} + +// buildRoutesForGateways builds routes from HTTPRoutes that reference any of the specified Gateways. +func buildRoutesForGateways( + validator validation.HTTPFieldsValidator, + httpRoutes map[types.NamespacedName]*v1beta1.HTTPRoute, + gatewayNsNames []types.NamespacedName, +) map[types.NamespacedName]*Route { + if len(gatewayNsNames) == 0 { + return nil + } - for _, p := range ghr.Spec.ParentRefs { - // FIXME(pleshakov) Support empty section name - if p.SectionName == nil || *p.SectionName == "" { + routes := make(map[types.NamespacedName]*Route) + + for _, ghr := range httpRoutes { + r := buildRoute(validator, ghr, gatewayNsNames) + if r != nil { + routes[client.ObjectKeyFromObject(ghr)] = r + } + } + + return routes +} + +func buildSectionNameRefs( + parentRefs []v1beta1.ParentReference, + routeNamespace string, + gatewayNsNames []types.NamespacedName, +) map[string]ParentRef { + sectionNameRefs := make(map[string]ParentRef) + + for i, p := range parentRefs { + gw, found := findGatewayForParentRef(p, routeNamespace, gatewayNsNames) + if !found { continue } - // if the namespace is missing, assume the namespace of the HTTPRoute - ns := ghr.Namespace - if p.Namespace != nil { - ns = string(*p.Namespace) + // The imported Webhook validation ensures unique section names. + // Additionally, it ensures that if multiple refs reference the same Gateway, their section names + // are not empty + // FIXME(pleshakov): Add a unit test for the imported Webhook validation code for this case. + + // FIXME(pleshakov): SectionNames across multiple Gateways might collide. Fix that. + var sectionName string + if p.SectionName != nil { + sectionName = string(*p.SectionName) } - name := string(*p.SectionName) + sectionNameRefs[sectionName] = ParentRef{ + Idx: i, + Gateway: gw, + } + } - // Below we will figure out what Gateway resource the parentRef references and act accordingly. There are 3 cases. + return sectionNameRefs +} - // Case 1: the parentRef references the winning Gateway. +func findGatewayForParentRef( + ref v1beta1.ParentReference, + routeNamespace string, + gatewayNsNames []types.NamespacedName, +) (gwNsName types.NamespacedName, found bool) { + if ref.Kind != nil && *ref.Kind != "Gateway" { + return types.NamespacedName{}, false + } + if ref.Group != nil && *ref.Group != v1beta1.GroupName { + return types.NamespacedName{}, false + } - if gw != nil && gw.Namespace == ns && gw.Name == string(p.Name) { + // if the namespace is missing, assume the namespace of the HTTPRoute + ns := routeNamespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } - // Find a listener + for _, gw := range gatewayNsNames { + if gw.Namespace == ns && gw.Name == string(ref.Name) { + return gw, true + } + } + + return types.NamespacedName{}, false +} + +func buildRoute( + validator validation.HTTPFieldsValidator, + ghr *v1beta1.HTTPRoute, + gatewayNsNames []types.NamespacedName, +) *Route { + sectionNameRefs := buildSectionNameRefs(ghr.Spec.ParentRefs, ghr.Namespace, gatewayNsNames) + // route doesn't belong to any of the Gateways + if len(sectionNameRefs) == 0 { + return nil + } + + r := &Route{ + Source: ghr, + SectionNameRefs: sectionNameRefs, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + + err := validateHostnames(validator, ghr.Spec.Hostnames, field.NewPath("spec").Child("hostnames")) + if err != nil { + r.Valid = false + r.Conditions = append(r.Conditions, conditions.NewRouteUnsupportedValue(err.Error())) + + return r + } - // FIXME(pleshakov) - // For now, let's do simple matching. - // However, we need to also support wildcard matching. - // More over, we need to handle cases when a Route host matches multiple HTTP listeners on the same port when - // sectionName is empty and only choose one listener. - // For example: - // - Route with host foo.example.com; - // - listener 1 for port 80 with hostname foo.example.com - // - listener 2 for port 80 with hostname *.example.com; - // In this case, the Route host foo.example.com should choose listener 1, as it is a more specific match. + r.Valid = true - processed = true + r.Rules = make([]Rule, len(ghr.Spec.Rules)) - l, exists := listeners[name] - if !exists { - // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. - // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/306 - r.InvalidSectionNameRefs[name] = conditions.NewTODO("listener is not found") - continue - } + atLeastOneValid := false + var allRulesErrs field.ErrorList - if !l.Valid { - r.InvalidSectionNameRefs[name] = conditions.NewRouteInvalidListener() - continue - } + for i, rule := range ghr.Spec.Rules { + rulePath := field.NewPath("spec").Child("rules").Index(i) - accepted := findAcceptedHostnames(l.Source.Hostname, ghr.Spec.Hostnames) + var matchesErrs field.ErrorList + for j, match := range rule.Matches { + matchPath := rulePath.Child("matches").Index(j) + matchesErrs = append(matchesErrs, validateMatch(validator, match, matchPath)...) + } + + var filtersErrs field.ErrorList + for j, filter := range rule.Filters { + filterPath := rulePath.Child("filters").Index(j) + filtersErrs = append(filtersErrs, validateFilter(validator, filter, filterPath)...) + } + + // rule.BackendRefs are validated separately because of their special requirements + + var allErrs field.ErrorList + allErrs = append(allErrs, matchesErrs...) + allErrs = append(allErrs, filtersErrs...) + allRulesErrs = append(allRulesErrs, allErrs...) + + if len(allErrs) == 0 { + atLeastOneValid = true + } + + r.Rules[i] = Rule{ + ValidMatches: len(matchesErrs) == 0, + ValidFilters: len(filtersErrs) == 0, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(r.Source), + RuleIdx: i, + }, + } + } - if len(accepted) > 0 { - for _, h := range accepted { - l.AcceptedHostnames[h] = struct{}{} - } - r.ValidSectionNameRefs[name] = struct{}{} - l.Routes[client.ObjectKeyFromObject(ghr)] = r - } else { - r.InvalidSectionNameRefs[name] = conditions.NewRouteNoMatchingListenerHostname() - } + if len(allRulesErrs) > 0 { + msg := allRulesErrs.ToAggregate().Error() + if atLeastOneValid { + // FIXME(pleshakov): Partial validity for HTTPRoute rules is not defined in the Gateway API spec yet. + // See https://github.com/kubernetes-sigs/gateway-api/issues/1696 + msg = "Some rules are invalid: " + msg + r.Conditions = append(r.Conditions, conditions.NewTODO(msg)) + } else { + msg = "All rules are invalid: " + msg + r.Conditions = append(r.Conditions, conditions.NewRouteUnsupportedValue(msg)) + + r.Valid = false + } + } + + return r +} + +func bindRoutesToListeners(routes map[types.NamespacedName]*Route, gw *Gateway) { + if gw == nil { + return + } + + for _, r := range routes { + bindRouteToListeners(r, gw) + } +} + +func bindRouteToListeners(r *Route, gw *Gateway) { + if !r.Valid { + return + } + + for name, ref := range r.SectionNameRefs { + routeRef := r.Source.Spec.ParentRefs[ref.Idx] + + path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx) + + // Case 1: Attachment is not possible due to unsupported configuration + + if routeRef.SectionName == nil || *routeRef.SectionName == "" { + valErr := field.Required(path.Child("sectionName"), "cannot be empty") + r.UnattachedSectionNameRefs[name] = conditions.NewRouteUnsupportedValue(valErr.Error()) + continue + } + + if routeRef.Port != nil { + valErr := field.Forbidden(path.Child("port"), "cannot be set") + r.UnattachedSectionNameRefs[name] = conditions.NewRouteUnsupportedValue(valErr.Error()) continue } // Case 2: the parentRef references an ignored Gateway resource. - key := types.NamespacedName{Namespace: ns, Name: string(p.Name)} + referencesWinningGw := ref.Gateway.Namespace == gw.Source.Namespace && ref.Gateway.Name == gw.Source.Name + + if !referencesWinningGw { + r.UnattachedSectionNameRefs[name] = conditions.NewTODO("Gateway is ignored") + continue + } + + // Case 3 - winning Gateway - if _, exist := ignoredGws[key]; exist { - // FIXME(pleshakov): Add a proper condition. + // Find a listener + + // FIXME(pleshakov) + // For now, let's do simple matching. + // However, we need to also support wildcard matching. + // More over, we need to handle cases when a Route host matches multiple HTTP listeners on the same port + // when sectionName is empty and only choose one listener. + // For example: + // - Route with host foo.example.com; + // - listener 1 for port 80 with hostname foo.example.com + // - listener 2 for port 80 with hostname *.example.com; + // In this case, the Route host foo.example.com should choose listener 1, as it is a more specific match. + + l, exists := gw.Listeners[name] + if !exists { + // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/306 - r.InvalidSectionNameRefs[name] = conditions.NewTODO("Gateway is ignored") + r.UnattachedSectionNameRefs[name] = conditions.NewTODO("listener is not found") + continue + } - processed = true + if !l.Valid { + r.UnattachedSectionNameRefs[name] = conditions.NewRouteInvalidListener() continue } - // Case 3: the parentRef references some unrelated to this NGINX Gateway Gateway or other resource. + accepted := findAcceptedHostnames(l.Source.Hostname, r.Source.Spec.Hostnames) - // Do nothing - } + if len(accepted) == 0 { + r.UnattachedSectionNameRefs[name] = conditions.NewRouteNoMatchingListenerHostname() + continue + } - if !processed { - return true, nil + for _, h := range accepted { + l.AcceptedHostnames[h] = struct{}{} + } + l.Routes[client.ObjectKeyFromObject(r.Source)] = r } - - return false, r } func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames []v1beta1.Hostname) []string { @@ -170,3 +394,207 @@ func getHostname(h *v1beta1.Hostname) string { } return string(*h) } + +func validateHostnames(validator validation.HTTPFieldsValidator, hostnames []v1beta1.Hostname, path *field.Path) error { + var allErrs field.ErrorList + + for i := range hostnames { + if err := validateHostname(string(hostnames[i])); err != nil { + allErrs = append(allErrs, field.Invalid(path.Index(i), hostnames[i], err.Error())) + continue + } + if err := validator.ValidateHostnameInServer(string(hostnames[i])); err != nil { + allErrs = append(allErrs, field.Invalid(path.Index(i), hostnames[i], err.Error())) + } + } + + return allErrs.ToAggregate() +} + +func validateMatch( + validator validation.HTTPFieldsValidator, + match v1beta1.HTTPRouteMatch, + matchPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + pathPath := matchPath.Child("path") + allErrs = append(allErrs, validatePathMatch(validator, match.Path, pathPath)...) + + for j, h := range match.Headers { + headerPath := matchPath.Child("headers").Index(j) + allErrs = append(allErrs, validateHeaderMatch(validator, h, headerPath)...) + } + + for j, q := range match.QueryParams { + queryParamPath := matchPath.Child("queryParams").Index(j) + allErrs = append(allErrs, validateQueryParamMatch(validator, q, queryParamPath)...) + } + + if err := validateMethodMatch(validator, match.Method, matchPath.Child("method")); err != nil { + allErrs = append(allErrs, err) + } + + return allErrs +} + +func validateMethodMatch( + validator validation.HTTPFieldsValidator, + method *v1beta1.HTTPMethod, + methodPath *field.Path, +) *field.Error { + if method == nil { + return nil + } + + if valid, supportedValues := validator.ValidateMethodInMatch(string(*method)); !valid { + return field.NotSupported(methodPath, *method, supportedValues) + } + + return nil +} + +func validateQueryParamMatch( + validator validation.HTTPFieldsValidator, + q v1beta1.HTTPQueryParamMatch, + queryParamPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + if q.Type == nil { + allErrs = append(allErrs, field.Required(queryParamPath.Child("type"), "cannot be empty")) + } else if *q.Type != v1beta1.QueryParamMatchExact { + valErr := field.NotSupported(queryParamPath.Child("type"), *q.Type, []string{string(v1beta1.QueryParamMatchExact)}) + allErrs = append(allErrs, valErr) + } + + if err := validator.ValidateQueryParamNameInMatch(q.Name); err != nil { + valErr := field.Invalid(queryParamPath.Child("name"), q.Name, err.Error()) + allErrs = append(allErrs, valErr) + } + + if err := validator.ValidateQueryParamValueInMatch(q.Value); err != nil { + valErr := field.Invalid(queryParamPath.Child("value"), q.Value, err.Error()) + allErrs = append(allErrs, valErr) + } + + return allErrs +} + +func validateHeaderMatch( + validator validation.HTTPFieldsValidator, + header v1beta1.HTTPHeaderMatch, + headerPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + if header.Type == nil { + allErrs = append(allErrs, field.Required(headerPath.Child("type"), "cannot be empty")) + } else if *header.Type != v1beta1.HeaderMatchExact { + valErr := field.NotSupported( + headerPath.Child("type"), + *header.Type, + []string{string(v1beta1.HeaderMatchExact)}, + ) + allErrs = append(allErrs, valErr) + } + + if err := validator.ValidateHeaderNameInMatch(string(header.Name)); err != nil { + valErr := field.Invalid(headerPath.Child("name"), header.Name, err.Error()) + allErrs = append(allErrs, valErr) + } + + if err := validator.ValidateHeaderValueInMatch(header.Value); err != nil { + valErr := field.Invalid(headerPath.Child("value"), header.Value, err.Error()) + allErrs = append(allErrs, valErr) + } + + return allErrs +} + +func validatePathMatch( + validator validation.HTTPFieldsValidator, + path *v1beta1.HTTPPathMatch, + fieldPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + if path == nil { + return allErrs + } + + // The imported Webhook validation ensures the path type and value are not nil + // FIXME(pleshakov): Add a unit test for the imported Webhook validation code for this case. + + if *path.Type != v1beta1.PathMatchPathPrefix { + valErr := field.NotSupported(fieldPath, *path.Type, []string{string(v1beta1.PathMatchPathPrefix)}) + allErrs = append(allErrs, valErr) + } + + if err := validator.ValidatePathInPrefixMatch(*path.Value); err != nil { + valErr := field.Invalid(fieldPath, *path.Value, err.Error()) + allErrs = append(allErrs, valErr) + } + + return allErrs +} + +func validateFilter( + validator validation.HTTPFieldsValidator, + filter v1beta1.HTTPRouteFilter, + filterPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + if filter.Type != v1beta1.HTTPRouteFilterRequestRedirect { + valErr := field.NotSupported( + filterPath.Child("type"), + filter.Type, + []string{string(v1beta1.HTTPRouteFilterRequestRedirect)}, + ) + allErrs = append(allErrs, valErr) + return allErrs + } + + // The imported Webhook validation ensures filter.RequestRedirect is not nil + // FIXME(pleshakov): Add a unit test for the imported Webhook validation code for this case. + + redirect := filter.RequestRedirect + + redirectPath := filterPath.Child("requestRedirect") + + if redirect.Scheme != nil { + if valid, supportedValues := validator.ValidateRedirectScheme(*redirect.Scheme); !valid { + valErr := field.NotSupported(redirectPath.Child("scheme"), *redirect.Scheme, supportedValues) + allErrs = append(allErrs, valErr) + } + } + + if redirect.Hostname != nil { + if err := validator.ValidateRedirectHostname(string(*redirect.Hostname)); err != nil { + valErr := field.Invalid(redirectPath.Child("hostname"), *redirect.Hostname, err.Error()) + allErrs = append(allErrs, valErr) + } + } + + if redirect.Port != nil { + if err := validator.ValidateRedirectPort(int32(*redirect.Port)); err != nil { + valErr := field.Invalid(redirectPath.Child("port"), *redirect.Port, err.Error()) + allErrs = append(allErrs, valErr) + } + } + + if redirect.Path != nil { + valErr := field.Forbidden(redirectPath.Child("path"), "path is not supported") + allErrs = append(allErrs, valErr) + } + + if redirect.StatusCode != nil { + if valid, supportedValues := validator.ValidateRedirectStatusCode(*redirect.StatusCode); !valid { + valErr := field.NotSupported(redirectPath.Child("statusCode"), *redirect.StatusCode, supportedValues) + allErrs = append(allErrs, valErr) + } + } + + return allErrs +} diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 12bd8b7601..98c1bfc0c2 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -1,69 +1,675 @@ package graph import ( + "errors" "testing" "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation/validationfakes" ) -func TestBindRouteToListeners(t *testing.T) { - createRoute := func(hostname string, parentRefs ...v1beta1.ParentReference) *v1beta1.HTTPRoute { - return &v1beta1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "hr-1", - }, - Spec: v1beta1.HTTPRouteSpec{ - CommonRouteSpec: v1beta1.CommonRouteSpec{ - ParentRefs: parentRefs, +const ( + sectionNameOfCreateHTTPRoute = "test-section" +) + +func createHTTPRoute( + name string, + refName string, + hostname v1beta1.Hostname, + paths ...string, +) *v1beta1.HTTPRoute { + rules := make([]v1beta1.HTTPRouteRule, 0, len(paths)) + + for _, path := range paths { + rules = append(rules, v1beta1.HTTPRouteRule{ + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), + Value: helpers.GetPointer(path), + }, }, - Hostnames: []v1beta1.Hostname{ - v1beta1.Hostname(hostname), + }, + }) + } + + return &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: v1beta1.HTTPRouteSpec{ + CommonRouteSpec: v1beta1.CommonRouteSpec{ + ParentRefs: []v1beta1.ParentReference{ + { + Namespace: helpers.GetPointer[v1beta1.Namespace]("test"), + Name: v1beta1.ObjectName(refName), + SectionName: helpers.GetPointer[v1beta1.SectionName](sectionNameOfCreateHTTPRoute), + }, }, }, + Hostnames: []v1beta1.Hostname{hostname}, + Rules: rules, + }, + } +} + +func addFilterToPath(hr *v1beta1.HTTPRoute, path string, filter v1beta1.HTTPRouteFilter) { + for i := range hr.Spec.Rules { + for _, match := range hr.Spec.Rules[i].Matches { + if match.Path == nil { + panic("unexpected nil path") + } + if *match.Path.Value == path { + hr.Spec.Rules[i].Filters = append(hr.Spec.Rules[i].Filters, filter) + } } } +} - hrNonExistingSectionName := createRoute("foo.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "gateway", - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-2")), - }) +func TestRouteGetAllBackendGroups(t *testing.T) { + group0 := BackendGroup{ + RuleIdx: 0, + } + group1 := BackendGroup{ + RuleIdx: 1, + } + group2 := BackendGroup{ + RuleIdx: 2, + } + group3 := BackendGroup{ + RuleIdx: 3, + } - hrEmptySectionName := createRoute("foo.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "gateway", - }) + tests := []struct { + route *Route + name string + expected []BackendGroup + }{ + { + route: &Route{}, + expected: nil, + name: "no rules", + }, + { + route: &Route{ + Rules: []Rule{ + { + ValidMatches: true, + ValidFilters: true, + BackendGroup: group0, + }, + { + ValidMatches: false, + ValidFilters: true, + BackendGroup: group1, + }, + { + ValidMatches: true, + ValidFilters: false, + BackendGroup: group2, + }, + { + ValidMatches: false, + ValidFilters: false, + BackendGroup: group3, + }, + }, + }, + expected: []BackendGroup{group0}, + name: "mix of valid and invalid rules", + }, + } - hrIgnoredGateway := createRoute("foo.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "ignored-gateway", - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-1")), - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := test.route.GetAllBackendGroups() + g.Expect(result).To(Equal(test.expected)) + }) + } +} - hrFoo := createRoute("foo.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "gateway", - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-1")), - }) +func TestGetAllConditionsForSectionName(t *testing.T) { + const ( + sectionName = "foo" + ) - hrFooImplicitNamespace := createRoute("foo.example.com", v1beta1.ParentReference{ - Name: "gateway", - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-1")), - }) + sectionNameRefs := map[string]ParentRef{ + sectionName: { + Idx: 0, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway"}, + }, + } - hrBar := createRoute("bar.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "gateway", - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-1")), - }) + tests := []struct { + route *Route + name string + expected []conditions.Condition + }{ + { + route: &Route{ + SectionNameRefs: sectionNameRefs, + Conditions: nil, + }, + expected: nil, + name: "no conditions", + }, + { + route: &Route{ + SectionNameRefs: sectionNameRefs, + UnattachedSectionNameRefs: map[string]conditions.Condition{ + sectionName: conditions.NewTODO("unattached"), + }, + }, + expected: []conditions.Condition{ + conditions.NewTODO("unattached"), + }, + name: "unattached section", + }, + { + route: &Route{ + SectionNameRefs: sectionNameRefs, + UnattachedSectionNameRefs: map[string]conditions.Condition{ + sectionName: conditions.NewTODO("unattached"), + }, + Conditions: []conditions.Condition{conditions.NewTODO("route")}, + }, + expected: []conditions.Condition{ + conditions.NewTODO("unattached"), + conditions.NewTODO("route"), + }, + name: "unattached section and route", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + result := test.route.GetAllConditionsForSectionName(sectionName) + g.Expect(result).To(Equal(test.expected)) + }) + } +} + +func TestGetAllConditionsForSectionNamePanics(t *testing.T) { + route := &Route{ + SectionNameRefs: map[string]ParentRef{ + "foo": { + Idx: 0, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway"}, + }, + }, + } + + invoke := func() { _ = route.GetAllConditionsForSectionName("bar") } + + g := NewGomegaWithT(t) + g.Expect(invoke).To(Panic()) +} + +func TestBuildRoutes(t *testing.T) { + gwNsName := types.NamespacedName{Namespace: "test", Name: "gateway"} + + hr := createHTTPRoute("hr-1", gwNsName.Name, "example.com", "/") + hrWrongGateway := createHTTPRoute("hr-2", "some-gateway", "example.com", "/") + + hrRoutes := map[types.NamespacedName]*v1beta1.HTTPRoute{ + client.ObjectKeyFromObject(hr): hr, + client.ObjectKeyFromObject(hrWrongGateway): hrWrongGateway, + } + + tests := []struct { + expected map[types.NamespacedName]*Route + name string + gwNsNames []types.NamespacedName + }{ + { + gwNsNames: []types.NamespacedName{gwNsName}, + expected: map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): { + Source: hr, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gwNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Valid: true, + Rules: []Rule{ + { + ValidMatches: true, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hr), + RuleIdx: 0, + }, + }, + }, + }, + }, + name: "normal case", + }, + { + gwNsNames: []types.NamespacedName{}, + expected: nil, + name: "no gateways", + }, + } + + validator := &validationfakes.FakeHTTPFieldsValidator{} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + routes := buildRoutesForGateways(validator, hrRoutes, test.gwNsNames) + g.Expect(helpers.Diff(test.expected, routes)).To(BeEmpty()) + }) + } +} + +func TestBuildSectionNameRefs(t *testing.T) { + gwNsName1 := types.NamespacedName{Namespace: "test", Name: "gateway-1"} + gwNsName2 := types.NamespacedName{Namespace: "test", Name: "gateway-2"} + + parentRefs := []v1beta1.ParentReference{ + { + Name: v1beta1.ObjectName(gwNsName1.Name), + SectionName: helpers.GetPointer[v1beta1.SectionName]("one"), + }, + { + Name: v1beta1.ObjectName("some-gateway"), + SectionName: helpers.GetPointer[v1beta1.SectionName]("other"), + }, + { + Name: v1beta1.ObjectName(gwNsName2.Name), + SectionName: helpers.GetPointer[v1beta1.SectionName]("two"), + }, + } + + gwNsNames := []types.NamespacedName{gwNsName1, gwNsName2} + routeNamespace := "test" + + expected := map[string]ParentRef{ + "one": { + Idx: 0, + Gateway: gwNsName1, + }, + "two": { + Idx: 2, + Gateway: gwNsName2, + }, + } + + g := NewGomegaWithT(t) + + result := buildSectionNameRefs(parentRefs, routeNamespace, gwNsNames) + g.Expect(result).To(Equal(expected)) +} +func TestFindGatewayForParentRef(t *testing.T) { + gwNsName1 := types.NamespacedName{Namespace: "test-1", Name: "gateway-1"} + gwNsName2 := types.NamespacedName{Namespace: "test-2", Name: "gateway-2"} + + tests := []struct { + ref v1beta1.ParentReference + expectedGwNsName types.NamespacedName + name string + expectedFound bool + }{ + { + ref: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: v1beta1.ObjectName(gwNsName1.Name), + }, + expectedFound: true, + expectedGwNsName: gwNsName1, + name: "found", + }, + { + ref: v1beta1.ParentReference{ + Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), + Kind: helpers.GetPointer[v1beta1.Kind]("Gateway"), + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: v1beta1.ObjectName(gwNsName1.Name), + }, + expectedFound: true, + expectedGwNsName: gwNsName1, + name: "found with explicit group and kind", + }, + { + ref: v1beta1.ParentReference{ + Name: v1beta1.ObjectName(gwNsName2.Name), + }, + expectedFound: true, + expectedGwNsName: gwNsName2, + name: "found with implicit namespace", + }, + { + ref: v1beta1.ParentReference{ + Kind: helpers.GetPointer[v1beta1.Kind]("NotGateway"), + Name: v1beta1.ObjectName(gwNsName2.Name), + }, + expectedFound: false, + expectedGwNsName: types.NamespacedName{}, + name: "wrong kind", + }, + { + ref: v1beta1.ParentReference{ + Group: helpers.GetPointer[v1beta1.Group]("wrong-group"), + Name: v1beta1.ObjectName(gwNsName2.Name), + }, + expectedFound: false, + expectedGwNsName: types.NamespacedName{}, + name: "wrong group", + }, + { + ref: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: "some-gateway", + }, + expectedFound: false, + expectedGwNsName: types.NamespacedName{}, + name: "not found", + }, + } + + routeNamespace := "test-2" + + gwNsNames := []types.NamespacedName{ + gwNsName1, + gwNsName2, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + gw, found := findGatewayForParentRef(test.ref, routeNamespace, gwNsNames) + g.Expect(found).To(Equal(test.expectedFound)) + g.Expect(gw).To(Equal(test.expectedGwNsName)) + }) + } +} + +func TestBuildRoute(t *testing.T) { + const ( + invalidPath = "/invalid" + invalidRedirectHostname = "invalid.example.com" + ) + + gatewayNsName := types.NamespacedName{Namespace: "test", Name: "gateway"} + + validFilter := v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{}, + } + invalidFilter := v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer[v1beta1.PreciseHostname](invalidRedirectHostname), + }, + } + + hr := createHTTPRoute("hr", gatewayNsName.Name, "example.com", "/", "/filter") + addFilterToPath(hr, "/filter", validFilter) + + hrInvalidHostname := createHTTPRoute("hr", gatewayNsName.Name, "", "/") + hrNotNKG := createHTTPRoute("hr", "some-gateway", "example.com", "/") + hrInvalidMatches := createHTTPRoute("hr", gatewayNsName.Name, "example.com", invalidPath) + + hrInvalidFilters := createHTTPRoute("hr", gatewayNsName.Name, "example.com", "/filter") + addFilterToPath(hrInvalidFilters, "/filter", invalidFilter) + + hrInvalidValidRules := createHTTPRoute("hr", gatewayNsName.Name, "example.com", invalidPath, "/filter", "/") + addFilterToPath(hrInvalidValidRules, "/filter", invalidFilter) + + validatorInvalidFieldsInRule := &validationfakes.FakeHTTPFieldsValidator{ + ValidatePathInPrefixMatchStub: func(path string) error { + if path == invalidPath { + return errors.New("invalid path") + } + return nil + }, + ValidateRedirectHostnameStub: func(h string) error { + if h == invalidRedirectHostname { + return errors.New("invalid hostname") + } + return nil + }, + } + + tests := []struct { + validator *validationfakes.FakeHTTPFieldsValidator + hr *v1beta1.HTTPRoute + expected *Route + name string + }{ + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hr: hr, + expected: &Route{ + Source: hr, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Valid: true, + Rules: []Rule{ + { + ValidMatches: true, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: types.NamespacedName{ + Namespace: hr.Namespace, + Name: hr.Name, + }, + RuleIdx: 0, + }, + }, + { + ValidMatches: true, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: types.NamespacedName{ + Namespace: hr.Namespace, + Name: hr.Name, + }, + RuleIdx: 1, + }, + }, + }, + }, + name: "normal case", + }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hr: hrNotNKG, + expected: nil, + name: "not NKG route", + }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hr: hrInvalidHostname, + expected: &Route{ + Source: hrInvalidHostname, + Valid: false, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Conditions: []conditions.Condition{ + conditions.NewRouteUnsupportedValue(`spec.hostnames[0]: Invalid value: "": cannot be empty string`), + }, + }, + name: "invalid hostname", + }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{ + ValidateHostnameInServerStub: func(string) error { + return errors.New("invalid hostname") + }, + }, + hr: hr, + expected: &Route{ + Source: hr, + Valid: false, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Conditions: []conditions.Condition{ + conditions.NewRouteUnsupportedValue(`spec.hostnames[0]: Invalid value: "example.com": invalid hostname`), + }, + }, + name: "invalid hostname by the data-plane", + }, + { + validator: validatorInvalidFieldsInRule, + hr: hrInvalidMatches, + expected: &Route{ + Source: hrInvalidMatches, + Valid: false, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Conditions: []conditions.Condition{ + conditions.NewRouteUnsupportedValue( + `All rules are invalid: spec.rules[0].matches[0].path: Invalid value: "/invalid": invalid path`, + ), + }, + Rules: []Rule{ + { + ValidMatches: false, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hr), + RuleIdx: 0, + }, + }, + }, + }, + name: "all rules invalid, with invalid matches", + }, + { + validator: validatorInvalidFieldsInRule, + hr: hrInvalidFilters, + expected: &Route{ + Source: hrInvalidFilters, + Valid: false, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Conditions: []conditions.Condition{ + conditions.NewRouteUnsupportedValue( + `All rules are invalid: spec.rules[0].filters[0].requestRedirect.hostname: ` + + `Invalid value: "invalid.example.com": invalid hostname`), + }, + Rules: []Rule{ + { + ValidMatches: true, + ValidFilters: false, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hr), + RuleIdx: 0, + }, + }, + }, + }, + name: "all rules invalid, with invalid filters", + }, + { + validator: validatorInvalidFieldsInRule, + hr: hrInvalidValidRules, + expected: &Route{ + Source: hrInvalidValidRules, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + sectionNameOfCreateHTTPRoute: { + Idx: 0, + Gateway: gatewayNsName, + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + Conditions: []conditions.Condition{ + conditions.NewTODO( + `Some rules are invalid: ` + + `[spec.rules[0].matches[0].path: Invalid value: "/invalid": invalid path, ` + + `spec.rules[1].filters[0].requestRedirect.hostname: Invalid value: ` + + `"invalid.example.com": invalid hostname]`, + ), + }, + Rules: []Rule{ + { + ValidMatches: false, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hrInvalidValidRules), + RuleIdx: 0, + }, + }, + { + ValidMatches: true, + ValidFilters: false, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hrInvalidValidRules), + RuleIdx: 1, + }, + }, + { + ValidMatches: true, + ValidFilters: true, + BackendGroup: BackendGroup{ + Source: client.ObjectKeyFromObject(hrInvalidValidRules), + RuleIdx: 2, + }, + }, + }, + }, + name: "invalid with invalid and valid rules", + }, + } + + gatewayNsNames := []types.NamespacedName{gatewayNsName} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + route := buildRoute(test.validator, test.hr, gatewayNsNames) + g.Expect(helpers.Diff(test.expected, route)).To(BeEmpty()) + }) + } +} + +func TestBindRouteToListeners(t *testing.T) { // we create a new listener each time because the function under test can modify it createListener := func() *Listener { return &Listener{ @@ -75,7 +681,6 @@ func TestBindRouteToListeners(t *testing.T) { AcceptedHostnames: map[string]struct{}{}, } } - createModifiedListener := func(m func(*Listener)) *Listener { l := createListener() m(l) @@ -89,237 +694,287 @@ func TestBindRouteToListeners(t *testing.T) { }, } - tests := []struct { - httpRoute *v1beta1.HTTPRoute - gw *v1beta1.Gateway - ignoredGws map[types.NamespacedName]*v1beta1.Gateway - listeners map[string]*Listener - expectedRoute *Route - expectedListeners map[string]*Listener - msg string - expectedIgnored bool - }{ - { - httpRoute: createRoute("foo.example.com"), - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), - }, - expectedIgnored: true, - expectedRoute: nil, - expectedListeners: map[string]*Listener{ - "listener-80-1": createListener(), + createHTTPRouteWithSectionNameAndPort := func( + sectionName *v1beta1.SectionName, + port *v1beta1.PortNumber, + ) *v1beta1.HTTPRoute { + return &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "hr", }, - msg: "HTTPRoute without parent refs", - }, - { - httpRoute: createRoute("foo.example.com", v1beta1.ParentReference{ - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: "some-gateway", // wrong gateway - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-1")), - }), - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), + Spec: v1beta1.HTTPRouteSpec{ + CommonRouteSpec: v1beta1.CommonRouteSpec{ + ParentRefs: []v1beta1.ParentReference{ + { + Name: v1beta1.ObjectName(gw.Name), + SectionName: sectionName, + Port: port, + }, + }, + }, + Hostnames: []v1beta1.Hostname{ + "foo.example.com", + }, }, - expectedIgnored: true, - expectedRoute: nil, - expectedListeners: map[string]*Listener{ - "listener-80-1": createListener(), + } + } + + hr := createHTTPRouteWithSectionNameAndPort(helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), nil) + hrWithNilSectionName := createHTTPRouteWithSectionNameAndPort(nil, nil) + hrWithEmptySectionName := createHTTPRouteWithSectionNameAndPort(helpers.GetPointer[v1beta1.SectionName](""), nil) + hrWithPort := createHTTPRouteWithSectionNameAndPort( + helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), + helpers.GetPointer[v1beta1.PortNumber](80), + ) + hrWithNonExistingListener := createHTTPRouteWithSectionNameAndPort( + helpers.GetPointer[v1beta1.SectionName]("listener-80-2"), + nil, + ) + + normalRoute := &Route{ + Source: hr, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), }, - msg: "HTTPRoute without good parent refs", }, - { - httpRoute: hrNonExistingSectionName, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + routeWithMissingSectionName := &Route{ + Source: hrWithNilSectionName, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrNonExistingSectionName, - ValidSectionNameRefs: map[string]struct{}{}, - InvalidSectionNameRefs: map[string]conditions.Condition{ - "listener-80-2": conditions.NewTODO("listener is not found"), - }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + routeWithEmptySectionName := &Route{ + Source: hrWithEmptySectionName, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), }, - expectedListeners: map[string]*Listener{ - "listener-80-1": createListener(), + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + routeWithNonExistingListener := &Route{ + Source: hrWithNonExistingListener, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "listener-80-2": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), }, - msg: "HTTPRoute with non-existing section name", }, - { - httpRoute: hrEmptySectionName, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + routeWithPort := &Route{ + Source: hrWithPort, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), }, - expectedIgnored: true, - expectedRoute: nil, - expectedListeners: map[string]*Listener{ - "listener-80-1": createListener(), + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + routeWithIgnoredGateway := &Route{ + Source: hr, + Valid: true, + SectionNameRefs: map[string]ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, }, - msg: "HTTPRoute with empty section name", }, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + notValidRoute := &Route{ + Valid: false, + UnattachedSectionNameRefs: map[string]conditions.Condition{}, + } + + notValidListener := createModifiedListener(func(l *Listener) { + l.Valid = false + }) + nonMatchingHostnameListener := createModifiedListener(func(l *Listener) { + l.Source.Hostname = helpers.GetPointer[v1beta1.Hostname]("bar.example.com") + }) + + tests := []struct { + route *Route + gateway *Gateway + expectedRouteUnattachedSectionNameRefs map[string]conditions.Condition + expectedGatewayListeners map[string]*Listener + name string + }{ { - httpRoute: hrFoo, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), - }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrFoo, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, + route: normalRoute, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, }, - expectedListeners: map[string]*Listener{ + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{}, + expectedGatewayListeners: map[string]*Listener{ "listener-80-1": createModifiedListener(func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr-1"}: { - Source: hrFoo, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, - }, + client.ObjectKeyFromObject(hr): normalRoute, } l.AcceptedHostnames = map[string]struct{}{ "foo.example.com": {}, } }), }, - msg: "HTTPRoute with one accepted hostname", + name: "normal case", }, { - httpRoute: hrFooImplicitNamespace, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createListener(), - }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrFooImplicitNamespace, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, + route: routeWithMissingSectionName, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, }, - expectedListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { - l.Routes = map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr-1"}: { - Source: hrFooImplicitNamespace, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]conditions.Condition{}, - }, - } - l.AcceptedHostnames = map[string]struct{}{ - "foo.example.com": {}, - } - }), + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "": conditions.NewRouteUnsupportedValue(`spec.parentRefs[0].sectionName: Required value: cannot be empty`), }, - msg: "HTTPRoute with one accepted hostname with implicit namespace in parentRef", + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + name: "section name is nil", }, { - httpRoute: hrBar, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ + route: routeWithEmptySectionName, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + }, + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "": conditions.NewRouteUnsupportedValue(`spec.parentRefs[0].sectionName: Required value: cannot be empty`), + }, + expectedGatewayListeners: map[string]*Listener{ "listener-80-1": createListener(), }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrBar, - ValidSectionNameRefs: map[string]struct{}{}, - InvalidSectionNameRefs: map[string]conditions.Condition{ - "listener-80-1": conditions.NewRouteNoMatchingListenerHostname(), + name: "section name is empty", + }, + { + route: routeWithPort, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), }, }, - expectedListeners: map[string]*Listener{ + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "listener-80-1": conditions.NewRouteUnsupportedValue(`spec.parentRefs[0].port: Forbidden: cannot be set`), + }, + expectedGatewayListeners: map[string]*Listener{ "listener-80-1": createListener(), }, - msg: "HTTPRoute with zero accepted hostnames", + name: "port is configured", }, { - httpRoute: hrIgnoredGateway, - gw: gw, - ignoredGws: map[types.NamespacedName]*v1beta1.Gateway{ - {Namespace: "test", Name: "ignored-gateway"}: {}, + route: routeWithNonExistingListener, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + }, + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "listener-80-2": conditions.NewTODO("listener is not found"), }, - listeners: map[string]*Listener{ + expectedGatewayListeners: map[string]*Listener{ "listener-80-1": createListener(), }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrIgnoredGateway, - ValidSectionNameRefs: map[string]struct{}{}, - InvalidSectionNameRefs: map[string]conditions.Condition{ - "listener-80-1": conditions.NewTODO("Gateway is ignored"), + name: "listener doesn't exist", + }, + { + route: normalRoute, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": notValidListener, }, }, - expectedListeners: map[string]*Listener{ - "listener-80-1": createListener(), + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "listener-80-1": conditions.NewRouteInvalidListener(), + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": notValidListener, }, - msg: "HTTPRoute with ignored gateway reference", + name: "listener isn't valid", }, { - httpRoute: hrFoo, - gw: nil, - ignoredGws: nil, - listeners: nil, - expectedIgnored: true, - expectedRoute: nil, - expectedListeners: nil, - msg: "HTTPRoute when no gateway exists", + route: normalRoute, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": nonMatchingHostnameListener, + }, + }, + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "listener-80-1": conditions.NewRouteNoMatchingListenerHostname(), + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": nonMatchingHostnameListener, + }, + name: "no matching listener hostname", }, { - httpRoute: hrFoo, - gw: gw, - ignoredGws: nil, - listeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { - l.Valid = false - }), + route: routeWithIgnoredGateway, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + }, + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{ + "listener-80-1": conditions.NewTODO("Gateway is ignored"), + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createListener(), }, - expectedIgnored: false, - expectedRoute: &Route{ - Source: hrFoo, - ValidSectionNameRefs: map[string]struct{}{}, - InvalidSectionNameRefs: map[string]conditions.Condition{ - "listener-80-1": conditions.NewRouteInvalidListener(), + name: "gateway is ignored", + }, + { + route: notValidRoute, + gateway: &Gateway{ + Source: gw, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), }, }, - expectedListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { - l.Valid = false - }), + expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{}, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createListener(), }, - msg: "HTTPRoute with invalid listener parentRef", + name: "route isn't valid", }, } for _, test := range tests { - ignored, route := bindHTTPRouteToListeners(test.httpRoute, test.gw, test.ignoredGws, test.listeners) - if diff := cmp.Diff(test.expectedIgnored, ignored); diff != "" { - t.Errorf("bindHTTPRouteToListeners() %q mismatch on ignored (-want +got):\n%s", test.msg, diff) - } - if diff := cmp.Diff(test.expectedRoute, route); diff != "" { - t.Errorf("bindHTTPRouteToListeners() %q mismatch on route (-want +got):\n%s", test.msg, diff) - } - if diff := cmp.Diff(test.expectedListeners, test.listeners); diff != "" { - t.Errorf("bindHTTPRouteToListeners() %q mismatch on listeners (-want +got):\n%s", test.msg, diff) - } + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + bindRouteToListeners(test.route, test.gateway) + + g.Expect(test.route.UnattachedSectionNameRefs).To(Equal(test.expectedRouteUnattachedSectionNameRefs)) + g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty()) + }) } } @@ -395,3 +1050,455 @@ func TestGetHostname(t *testing.T) { } } } + +func TestValidateHostnames(t *testing.T) { + const validHostname = "example.com" + + tests := []struct { + validator *validationfakes.FakeHTTPFieldsValidator + name string + hostnames []v1beta1.Hostname + expectErr bool + }{ + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hostnames: []v1beta1.Hostname{ + validHostname, + "example.org", + "foo.example.net", + }, + expectErr: false, + name: "multiple valid", + }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hostnames: []v1beta1.Hostname{ + validHostname, + "", + }, + expectErr: true, + name: "valid and invalid", + }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{ + ValidateHostnameInServerStub: func(h string) error { + if h == validHostname { + return nil + } + return errors.New("invalid hostname") + }, + }, + hostnames: []v1beta1.Hostname{ + validHostname, + "value", // invalid by the validator + }, + expectErr: true, + name: "valid and invalid by the validator", + }, + } + + path := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateHostnames(test.validator, test.hostnames, path) + + if test.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +func TestValidateMatch(t *testing.T) { + createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { + v := &validationfakes.FakeHTTPFieldsValidator{} + v.ValidateMethodInMatchReturns(true, nil) + return v + } + + tests := []struct { + match v1beta1.HTTPRouteMatch + validator *validationfakes.FakeHTTPFieldsValidator + name string + expectErrCount int + }{ + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), + Value: helpers.GetPointer("/"), + }, + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetPointer(v1beta1.HeaderMatchExact), + Name: "header", + Value: "x", + }, + }, + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetPointer(v1beta1.QueryParamMatchExact), + Name: "param", + Value: "y", + }, + }, + Method: helpers.GetPointer(v1beta1.HTTPMethodGet), + }, + expectErrCount: 0, + name: "valid", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchRegularExpression), + Value: helpers.GetPointer("/"), + }, + }, + expectErrCount: 1, + name: "wrong path type", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidatePathInPrefixMatchReturns(errors.New("invalid path value")) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), + Value: helpers.GetPointer("/"), + }, + }, + expectErrCount: 1, + name: "wrong path value", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: nil, + Name: "header", + Value: "x", + }, + }, + }, + expectErrCount: 1, + name: "header match type is nil", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetPointer(v1beta1.HeaderMatchRegularExpression), + Name: "header", + Value: "x", + }, + }, + }, + expectErrCount: 1, + name: "header match type is invalid", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateHeaderNameInMatchReturns(errors.New("invalid header name")) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetPointer(v1beta1.HeaderMatchExact), + Name: "header", // any value is invalid by the validator + Value: "x", + }, + }, + }, + expectErrCount: 1, + name: "header name is invalid", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateHeaderValueInMatchReturns(errors.New("invalid header value")) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetPointer(v1beta1.HeaderMatchExact), + Name: "header", + Value: "x", // any value is invalid by the validator + }, + }, + }, + expectErrCount: 1, + name: "header value is invalid", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: nil, + Name: "param", + Value: "y", + }, + }, + }, + expectErrCount: 1, + name: "query param match type is nil", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetPointer(v1beta1.QueryParamMatchRegularExpression), + Name: "param", + Value: "y", + }, + }, + }, + expectErrCount: 1, + name: "query param match type is invalid", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateQueryParamNameInMatchReturns(errors.New("invalid query param name")) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetPointer(v1beta1.QueryParamMatchExact), + Name: "param", // any value is invalid by the validator + Value: "y", + }, + }, + }, + expectErrCount: 1, + name: "query param name is invalid", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateQueryParamValueInMatchReturns(errors.New("invalid query param value")) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetPointer(v1beta1.QueryParamMatchExact), + Name: "param", + Value: "y", // any value is invalid by the validator + }, + }, + }, + expectErrCount: 1, + name: "query param value is invalid", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateMethodInMatchReturns(false, []string{"VALID_METHOD"}) + return validator + }(), + match: v1beta1.HTTPRouteMatch{ + Method: helpers.GetPointer(v1beta1.HTTPMethodGet), // any value is invalid by the validator + }, + expectErrCount: 1, + name: "method is invalid", + }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchRegularExpression), // invalid + Value: helpers.GetPointer("/"), + }, + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetPointer(v1beta1.HeaderMatchRegularExpression), // invalid + Name: "header", + Value: "x", + }, + }, + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetPointer(v1beta1.QueryParamMatchRegularExpression), // invalid + Name: "param", + Value: "y", + }, + }, + }, + expectErrCount: 3, + name: "multiple errors", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + allErrs := validateMatch(test.validator, test.match, field.NewPath("test")) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} + +func TestValidateFilter(t *testing.T) { + createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { + v := &validationfakes.FakeHTTPFieldsValidator{} + + v.ValidateRedirectSchemeReturns(true, nil) + v.ValidateRedirectStatusCodeReturns(true, nil) + + return v + } + + tests := []struct { + filter v1beta1.HTTPRouteFilter + validator *validationfakes.FakeHTTPFieldsValidator + name string + expectErrCount int + }{ + { + validator: createAllValidValidator(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), + Hostname: helpers.GetPointer[v1beta1.PreciseHostname]("example.com"), + Port: helpers.GetPointer[v1beta1.PortNumber](80), + StatusCode: helpers.GetPointer(301), + }, + }, + expectErrCount: 0, + name: "valid redirect filter", + }, + { + validator: createAllValidValidator(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{}, + }, + expectErrCount: 0, + name: "valid redirect filter with no fields set", + }, + { + validator: createAllValidValidator(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterURLRewrite, + }, + expectErrCount: 1, + name: "unsupported filter", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateRedirectSchemeReturns(false, []string{"valid-scheme"}) + return validator + }(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), // any value is invalid by the validator + }, + }, + expectErrCount: 1, + name: "redirect filter with invalid scheme", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateRedirectHostnameReturns(errors.New("invalid hostname")) + return validator + }(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer[v1beta1.PreciseHostname]("example.com"), // any value is invalid by the validator + }, + }, + expectErrCount: 1, + name: "redirect filter with invalid hostname", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateRedirectPortReturns(errors.New("invalid port")) + return validator + }(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Port: helpers.GetPointer[v1beta1.PortNumber](80), // any value is invalid by the validator + }, + }, + expectErrCount: 1, + name: "redirect filter with invalid port", + }, + { + validator: createAllValidValidator(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Path: &v1beta1.HTTPPathModifier{}, + }, + }, + expectErrCount: 1, + name: "redirect filter with unsupported path modifier", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateRedirectStatusCodeReturns(false, []string{"200"}) + return validator + }(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + StatusCode: helpers.GetPointer(301), // any value is invalid by the validator + }, + }, + expectErrCount: 1, + name: "redirect filter with invalid status code", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + validator := createAllValidValidator() + validator.ValidateRedirectHostnameReturns(errors.New("invalid hostname")) + validator.ValidateRedirectPortReturns(errors.New("invalid port")) + return validator + }(), + filter: v1beta1.HTTPRouteFilter{ + Type: v1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer[v1beta1.PreciseHostname]("example.com"), // any value is invalid by the validator + Port: helpers.GetPointer[v1beta1.PortNumber](80), // any value is invalid by the validator + }, + }, + expectErrCount: 2, + name: "redirect filter with multiple errors", + }, + } + + filterPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + allErrs := validateFilter(test.validator, test.filter, filterPath) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} diff --git a/internal/state/graph/validation.go b/internal/state/graph/validation.go new file mode 100644 index 0000000000..8825d32b95 --- /dev/null +++ b/internal/state/graph/validation.go @@ -0,0 +1,26 @@ +package graph + +import ( + "errors" + "strings" + + "k8s.io/apimachinery/pkg/util/validation" +) + +func validateHostname(hostname string) error { + if hostname == "" { + return errors.New("cannot be empty string") + } + + if strings.Contains(hostname, "*") { + return errors.New("wildcards are not supported") + } + + msgs := validation.IsDNS1123Subdomain(hostname) + if len(msgs) > 0 { + combined := strings.Join(msgs, ",") + return errors.New(combined) + } + + return nil +} diff --git a/internal/state/graph/validation_test.go b/internal/state/graph/validation_test.go new file mode 100644 index 0000000000..dc629999b1 --- /dev/null +++ b/internal/state/graph/validation_test.go @@ -0,0 +1,50 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestValidateHostname(t *testing.T) { + tests := []struct { + name string + hostname string + expectErr bool + }{ + { + hostname: "example.com", + expectErr: false, + name: "valid hostname", + }, + { + hostname: "", + expectErr: true, + name: "empty hostname", + }, + { + hostname: "*.example.com", + expectErr: true, + name: "wildcard hostname", + }, + { + hostname: "example$com", + expectErr: true, + name: "invalid hostname", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateHostname(test.hostname) + + if test.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} diff --git a/internal/state/resolver/resolver.go b/internal/state/resolver/resolver.go index a862c17c49..7df65b690c 100644 --- a/internal/state/resolver/resolver.go +++ b/internal/state/resolver/resolver.go @@ -2,6 +2,7 @@ package resolver import ( "context" + "errors" "fmt" v1 "k8s.io/api/core/v1" @@ -42,7 +43,7 @@ func NewServiceResolverImpl(client client.Client) *ServiceResolverImpl { // Returns an error if the Service or Port cannot be resolved. func (e *ServiceResolverImpl) Resolve(ctx context.Context, svc *v1.Service, port int32) ([]Endpoint, error) { if svc == nil { - return nil, fmt.Errorf("cannot resolve a nil Service") + return nil, errors.New("cannot resolve a nil Service") } // We list EndpointSlices using the Service Name Index Field we added as an index to the EndpointSlice cache. diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 4a30768312..466b100077 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -133,24 +133,27 @@ func buildStatuses(graph *graph.Graph) Statuses { for nsname, r := range graph.Routes { parentStatuses := make(map[string]ParentStatus) - for ref := range r.ValidSectionNameRefs { - parentStatuses[ref] = ParentStatus{ - Conditions: conditions.DeduplicateConditions( - buildBaseRouteConditions(gcValidAndExist), - ), - } - } - for ref, cond := range r.InvalidSectionNameRefs { - baseConds := buildBaseRouteConditions(gcValidAndExist) + baseConds := buildBaseRouteConditions(gcValidAndExist) + for ref := range r.SectionNameRefs { + conds := r.GetAllConditionsForSectionName(ref) + + allConds := make([]conditions.Condition, 0, len(conds)+len(baseConds)) // We add baseConds first, so that any additional conditions will override them, which is // ensured by DeduplicateConditions. - conds := make([]conditions.Condition, 0, len(baseConds)+1) - conds = append(conds, baseConds...) - conds = append(conds, cond) + allConds = append(allConds, baseConds...) + allConds = append(allConds, conds...) + + if ref == "" { + // FIXME(pleshakov): Gateway API spec does allow empty section names in the status. + // However, NKG doesn't yet support the empty section names. + // Once NKG supports them, it will be able to determine which section name the HTTPRoute was + // attached to. So we won't need this workaround. + ref = "unattached" + } parentStatuses[ref] = ParentStatus{ - Conditions: conditions.DeduplicateConditions(conds), + Conditions: conditions.DeduplicateConditions(allConds), } } diff --git a/internal/state/statuses_test.go b/internal/state/statuses_test.go index d4d5c9459c..543e482b2a 100644 --- a/internal/state/statuses_test.go +++ b/internal/state/statuses_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" @@ -28,6 +29,22 @@ func TestBuildStatuses(t *testing.T) { }, } + gw := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + Generation: 2, + }, + } + + ignoredGw := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "ignored-gateway", + Generation: 1, + }, + } + routes := map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: { Source: &v1beta1.HTTPRoute{ @@ -35,10 +52,17 @@ func TestBuildStatuses(t *testing.T) { Generation: 3, }, }, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, + SectionNameRefs: map[string]graph.ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + }, + "listener-80-2": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(ignoredGw), + }, }, - InvalidSectionNameRefs: map[string]conditions.Condition{ + UnattachedSectionNameRefs: map[string]conditions.Condition{ "listener-80-2": invalidCondition, }, }, @@ -51,29 +75,23 @@ func TestBuildStatuses(t *testing.T) { Generation: 4, }, }, - InvalidSectionNameRefs: map[string]conditions.Condition{ - "listener-80-2": invalidCondition, + SectionNameRefs: map[string]graph.ParentRef{ + "listener-80-1": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + }, + "listener-80-2": { + Idx: 0, + Gateway: client.ObjectKeyFromObject(ignoredGw), + }, + }, + UnattachedSectionNameRefs: map[string]conditions.Condition{ "listener-80-1": invalidCondition, + "listener-80-2": invalidCondition, }, }, } - gw := &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "gateway", - Generation: 2, - }, - } - - ignoredGw := &v1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "ignored-gateway", - Generation: 1, - }, - } - tests := []struct { graph *graph.Graph expected Statuses diff --git a/internal/state/validation/validationfakes/fake_httpfields_validator.go b/internal/state/validation/validationfakes/fake_httpfields_validator.go new file mode 100644 index 0000000000..a21944a372 --- /dev/null +++ b/internal/state/validation/validationfakes/fake_httpfields_validator.go @@ -0,0 +1,866 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package validationfakes + +import ( + "sync" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" +) + +type FakeHTTPFieldsValidator struct { + ValidateHeaderNameInMatchStub func(string) error + validateHeaderNameInMatchMutex sync.RWMutex + validateHeaderNameInMatchArgsForCall []struct { + arg1 string + } + validateHeaderNameInMatchReturns struct { + result1 error + } + validateHeaderNameInMatchReturnsOnCall map[int]struct { + result1 error + } + ValidateHeaderValueInMatchStub func(string) error + validateHeaderValueInMatchMutex sync.RWMutex + validateHeaderValueInMatchArgsForCall []struct { + arg1 string + } + validateHeaderValueInMatchReturns struct { + result1 error + } + validateHeaderValueInMatchReturnsOnCall map[int]struct { + result1 error + } + ValidateHostnameInServerStub func(string) error + validateHostnameInServerMutex sync.RWMutex + validateHostnameInServerArgsForCall []struct { + arg1 string + } + validateHostnameInServerReturns struct { + result1 error + } + validateHostnameInServerReturnsOnCall map[int]struct { + result1 error + } + ValidateMethodInMatchStub func(string) (bool, []string) + validateMethodInMatchMutex sync.RWMutex + validateMethodInMatchArgsForCall []struct { + arg1 string + } + validateMethodInMatchReturns struct { + result1 bool + result2 []string + } + validateMethodInMatchReturnsOnCall map[int]struct { + result1 bool + result2 []string + } + ValidatePathInPrefixMatchStub func(string) error + validatePathInPrefixMatchMutex sync.RWMutex + validatePathInPrefixMatchArgsForCall []struct { + arg1 string + } + validatePathInPrefixMatchReturns struct { + result1 error + } + validatePathInPrefixMatchReturnsOnCall map[int]struct { + result1 error + } + ValidateQueryParamNameInMatchStub func(string) error + validateQueryParamNameInMatchMutex sync.RWMutex + validateQueryParamNameInMatchArgsForCall []struct { + arg1 string + } + validateQueryParamNameInMatchReturns struct { + result1 error + } + validateQueryParamNameInMatchReturnsOnCall map[int]struct { + result1 error + } + ValidateQueryParamValueInMatchStub func(string) error + validateQueryParamValueInMatchMutex sync.RWMutex + validateQueryParamValueInMatchArgsForCall []struct { + arg1 string + } + validateQueryParamValueInMatchReturns struct { + result1 error + } + validateQueryParamValueInMatchReturnsOnCall map[int]struct { + result1 error + } + ValidateRedirectHostnameStub func(string) error + validateRedirectHostnameMutex sync.RWMutex + validateRedirectHostnameArgsForCall []struct { + arg1 string + } + validateRedirectHostnameReturns struct { + result1 error + } + validateRedirectHostnameReturnsOnCall map[int]struct { + result1 error + } + ValidateRedirectPortStub func(int32) error + validateRedirectPortMutex sync.RWMutex + validateRedirectPortArgsForCall []struct { + arg1 int32 + } + validateRedirectPortReturns struct { + result1 error + } + validateRedirectPortReturnsOnCall map[int]struct { + result1 error + } + ValidateRedirectSchemeStub func(string) (bool, []string) + validateRedirectSchemeMutex sync.RWMutex + validateRedirectSchemeArgsForCall []struct { + arg1 string + } + validateRedirectSchemeReturns struct { + result1 bool + result2 []string + } + validateRedirectSchemeReturnsOnCall map[int]struct { + result1 bool + result2 []string + } + ValidateRedirectStatusCodeStub func(int) (bool, []string) + validateRedirectStatusCodeMutex sync.RWMutex + validateRedirectStatusCodeArgsForCall []struct { + arg1 int + } + validateRedirectStatusCodeReturns struct { + result1 bool + result2 []string + } + validateRedirectStatusCodeReturnsOnCall map[int]struct { + result1 bool + result2 []string + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatch(arg1 string) error { + fake.validateHeaderNameInMatchMutex.Lock() + ret, specificReturn := fake.validateHeaderNameInMatchReturnsOnCall[len(fake.validateHeaderNameInMatchArgsForCall)] + fake.validateHeaderNameInMatchArgsForCall = append(fake.validateHeaderNameInMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateHeaderNameInMatchStub + fakeReturns := fake.validateHeaderNameInMatchReturns + fake.recordInvocation("ValidateHeaderNameInMatch", []interface{}{arg1}) + fake.validateHeaderNameInMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatchCallCount() int { + fake.validateHeaderNameInMatchMutex.RLock() + defer fake.validateHeaderNameInMatchMutex.RUnlock() + return len(fake.validateHeaderNameInMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatchCalls(stub func(string) error) { + fake.validateHeaderNameInMatchMutex.Lock() + defer fake.validateHeaderNameInMatchMutex.Unlock() + fake.ValidateHeaderNameInMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatchArgsForCall(i int) string { + fake.validateHeaderNameInMatchMutex.RLock() + defer fake.validateHeaderNameInMatchMutex.RUnlock() + argsForCall := fake.validateHeaderNameInMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatchReturns(result1 error) { + fake.validateHeaderNameInMatchMutex.Lock() + defer fake.validateHeaderNameInMatchMutex.Unlock() + fake.ValidateHeaderNameInMatchStub = nil + fake.validateHeaderNameInMatchReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderNameInMatchReturnsOnCall(i int, result1 error) { + fake.validateHeaderNameInMatchMutex.Lock() + defer fake.validateHeaderNameInMatchMutex.Unlock() + fake.ValidateHeaderNameInMatchStub = nil + if fake.validateHeaderNameInMatchReturnsOnCall == nil { + fake.validateHeaderNameInMatchReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateHeaderNameInMatchReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatch(arg1 string) error { + fake.validateHeaderValueInMatchMutex.Lock() + ret, specificReturn := fake.validateHeaderValueInMatchReturnsOnCall[len(fake.validateHeaderValueInMatchArgsForCall)] + fake.validateHeaderValueInMatchArgsForCall = append(fake.validateHeaderValueInMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateHeaderValueInMatchStub + fakeReturns := fake.validateHeaderValueInMatchReturns + fake.recordInvocation("ValidateHeaderValueInMatch", []interface{}{arg1}) + fake.validateHeaderValueInMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatchCallCount() int { + fake.validateHeaderValueInMatchMutex.RLock() + defer fake.validateHeaderValueInMatchMutex.RUnlock() + return len(fake.validateHeaderValueInMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatchCalls(stub func(string) error) { + fake.validateHeaderValueInMatchMutex.Lock() + defer fake.validateHeaderValueInMatchMutex.Unlock() + fake.ValidateHeaderValueInMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatchArgsForCall(i int) string { + fake.validateHeaderValueInMatchMutex.RLock() + defer fake.validateHeaderValueInMatchMutex.RUnlock() + argsForCall := fake.validateHeaderValueInMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatchReturns(result1 error) { + fake.validateHeaderValueInMatchMutex.Lock() + defer fake.validateHeaderValueInMatchMutex.Unlock() + fake.ValidateHeaderValueInMatchStub = nil + fake.validateHeaderValueInMatchReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateHeaderValueInMatchReturnsOnCall(i int, result1 error) { + fake.validateHeaderValueInMatchMutex.Lock() + defer fake.validateHeaderValueInMatchMutex.Unlock() + fake.ValidateHeaderValueInMatchStub = nil + if fake.validateHeaderValueInMatchReturnsOnCall == nil { + fake.validateHeaderValueInMatchReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateHeaderValueInMatchReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServer(arg1 string) error { + fake.validateHostnameInServerMutex.Lock() + ret, specificReturn := fake.validateHostnameInServerReturnsOnCall[len(fake.validateHostnameInServerArgsForCall)] + fake.validateHostnameInServerArgsForCall = append(fake.validateHostnameInServerArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateHostnameInServerStub + fakeReturns := fake.validateHostnameInServerReturns + fake.recordInvocation("ValidateHostnameInServer", []interface{}{arg1}) + fake.validateHostnameInServerMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServerCallCount() int { + fake.validateHostnameInServerMutex.RLock() + defer fake.validateHostnameInServerMutex.RUnlock() + return len(fake.validateHostnameInServerArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServerCalls(stub func(string) error) { + fake.validateHostnameInServerMutex.Lock() + defer fake.validateHostnameInServerMutex.Unlock() + fake.ValidateHostnameInServerStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServerArgsForCall(i int) string { + fake.validateHostnameInServerMutex.RLock() + defer fake.validateHostnameInServerMutex.RUnlock() + argsForCall := fake.validateHostnameInServerArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServerReturns(result1 error) { + fake.validateHostnameInServerMutex.Lock() + defer fake.validateHostnameInServerMutex.Unlock() + fake.ValidateHostnameInServerStub = nil + fake.validateHostnameInServerReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateHostnameInServerReturnsOnCall(i int, result1 error) { + fake.validateHostnameInServerMutex.Lock() + defer fake.validateHostnameInServerMutex.Unlock() + fake.ValidateHostnameInServerStub = nil + if fake.validateHostnameInServerReturnsOnCall == nil { + fake.validateHostnameInServerReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateHostnameInServerReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatch(arg1 string) (bool, []string) { + fake.validateMethodInMatchMutex.Lock() + ret, specificReturn := fake.validateMethodInMatchReturnsOnCall[len(fake.validateMethodInMatchArgsForCall)] + fake.validateMethodInMatchArgsForCall = append(fake.validateMethodInMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateMethodInMatchStub + fakeReturns := fake.validateMethodInMatchReturns + fake.recordInvocation("ValidateMethodInMatch", []interface{}{arg1}) + fake.validateMethodInMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchCallCount() int { + fake.validateMethodInMatchMutex.RLock() + defer fake.validateMethodInMatchMutex.RUnlock() + return len(fake.validateMethodInMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchCalls(stub func(string) (bool, []string)) { + fake.validateMethodInMatchMutex.Lock() + defer fake.validateMethodInMatchMutex.Unlock() + fake.ValidateMethodInMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchArgsForCall(i int) string { + fake.validateMethodInMatchMutex.RLock() + defer fake.validateMethodInMatchMutex.RUnlock() + argsForCall := fake.validateMethodInMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchReturns(result1 bool, result2 []string) { + fake.validateMethodInMatchMutex.Lock() + defer fake.validateMethodInMatchMutex.Unlock() + fake.ValidateMethodInMatchStub = nil + fake.validateMethodInMatchReturns = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchReturnsOnCall(i int, result1 bool, result2 []string) { + fake.validateMethodInMatchMutex.Lock() + defer fake.validateMethodInMatchMutex.Unlock() + fake.ValidateMethodInMatchStub = nil + if fake.validateMethodInMatchReturnsOnCall == nil { + fake.validateMethodInMatchReturnsOnCall = make(map[int]struct { + result1 bool + result2 []string + }) + } + fake.validateMethodInMatchReturnsOnCall[i] = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatch(arg1 string) error { + fake.validatePathInPrefixMatchMutex.Lock() + ret, specificReturn := fake.validatePathInPrefixMatchReturnsOnCall[len(fake.validatePathInPrefixMatchArgsForCall)] + fake.validatePathInPrefixMatchArgsForCall = append(fake.validatePathInPrefixMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidatePathInPrefixMatchStub + fakeReturns := fake.validatePathInPrefixMatchReturns + fake.recordInvocation("ValidatePathInPrefixMatch", []interface{}{arg1}) + fake.validatePathInPrefixMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchCallCount() int { + fake.validatePathInPrefixMatchMutex.RLock() + defer fake.validatePathInPrefixMatchMutex.RUnlock() + return len(fake.validatePathInPrefixMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchCalls(stub func(string) error) { + fake.validatePathInPrefixMatchMutex.Lock() + defer fake.validatePathInPrefixMatchMutex.Unlock() + fake.ValidatePathInPrefixMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchArgsForCall(i int) string { + fake.validatePathInPrefixMatchMutex.RLock() + defer fake.validatePathInPrefixMatchMutex.RUnlock() + argsForCall := fake.validatePathInPrefixMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchReturns(result1 error) { + fake.validatePathInPrefixMatchMutex.Lock() + defer fake.validatePathInPrefixMatchMutex.Unlock() + fake.ValidatePathInPrefixMatchStub = nil + fake.validatePathInPrefixMatchReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchReturnsOnCall(i int, result1 error) { + fake.validatePathInPrefixMatchMutex.Lock() + defer fake.validatePathInPrefixMatchMutex.Unlock() + fake.ValidatePathInPrefixMatchStub = nil + if fake.validatePathInPrefixMatchReturnsOnCall == nil { + fake.validatePathInPrefixMatchReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validatePathInPrefixMatchReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatch(arg1 string) error { + fake.validateQueryParamNameInMatchMutex.Lock() + ret, specificReturn := fake.validateQueryParamNameInMatchReturnsOnCall[len(fake.validateQueryParamNameInMatchArgsForCall)] + fake.validateQueryParamNameInMatchArgsForCall = append(fake.validateQueryParamNameInMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateQueryParamNameInMatchStub + fakeReturns := fake.validateQueryParamNameInMatchReturns + fake.recordInvocation("ValidateQueryParamNameInMatch", []interface{}{arg1}) + fake.validateQueryParamNameInMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatchCallCount() int { + fake.validateQueryParamNameInMatchMutex.RLock() + defer fake.validateQueryParamNameInMatchMutex.RUnlock() + return len(fake.validateQueryParamNameInMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatchCalls(stub func(string) error) { + fake.validateQueryParamNameInMatchMutex.Lock() + defer fake.validateQueryParamNameInMatchMutex.Unlock() + fake.ValidateQueryParamNameInMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatchArgsForCall(i int) string { + fake.validateQueryParamNameInMatchMutex.RLock() + defer fake.validateQueryParamNameInMatchMutex.RUnlock() + argsForCall := fake.validateQueryParamNameInMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatchReturns(result1 error) { + fake.validateQueryParamNameInMatchMutex.Lock() + defer fake.validateQueryParamNameInMatchMutex.Unlock() + fake.ValidateQueryParamNameInMatchStub = nil + fake.validateQueryParamNameInMatchReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamNameInMatchReturnsOnCall(i int, result1 error) { + fake.validateQueryParamNameInMatchMutex.Lock() + defer fake.validateQueryParamNameInMatchMutex.Unlock() + fake.ValidateQueryParamNameInMatchStub = nil + if fake.validateQueryParamNameInMatchReturnsOnCall == nil { + fake.validateQueryParamNameInMatchReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateQueryParamNameInMatchReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatch(arg1 string) error { + fake.validateQueryParamValueInMatchMutex.Lock() + ret, specificReturn := fake.validateQueryParamValueInMatchReturnsOnCall[len(fake.validateQueryParamValueInMatchArgsForCall)] + fake.validateQueryParamValueInMatchArgsForCall = append(fake.validateQueryParamValueInMatchArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateQueryParamValueInMatchStub + fakeReturns := fake.validateQueryParamValueInMatchReturns + fake.recordInvocation("ValidateQueryParamValueInMatch", []interface{}{arg1}) + fake.validateQueryParamValueInMatchMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchCallCount() int { + fake.validateQueryParamValueInMatchMutex.RLock() + defer fake.validateQueryParamValueInMatchMutex.RUnlock() + return len(fake.validateQueryParamValueInMatchArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchCalls(stub func(string) error) { + fake.validateQueryParamValueInMatchMutex.Lock() + defer fake.validateQueryParamValueInMatchMutex.Unlock() + fake.ValidateQueryParamValueInMatchStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchArgsForCall(i int) string { + fake.validateQueryParamValueInMatchMutex.RLock() + defer fake.validateQueryParamValueInMatchMutex.RUnlock() + argsForCall := fake.validateQueryParamValueInMatchArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchReturns(result1 error) { + fake.validateQueryParamValueInMatchMutex.Lock() + defer fake.validateQueryParamValueInMatchMutex.Unlock() + fake.ValidateQueryParamValueInMatchStub = nil + fake.validateQueryParamValueInMatchReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchReturnsOnCall(i int, result1 error) { + fake.validateQueryParamValueInMatchMutex.Lock() + defer fake.validateQueryParamValueInMatchMutex.Unlock() + fake.ValidateQueryParamValueInMatchStub = nil + if fake.validateQueryParamValueInMatchReturnsOnCall == nil { + fake.validateQueryParamValueInMatchReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateQueryParamValueInMatchReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostname(arg1 string) error { + fake.validateRedirectHostnameMutex.Lock() + ret, specificReturn := fake.validateRedirectHostnameReturnsOnCall[len(fake.validateRedirectHostnameArgsForCall)] + fake.validateRedirectHostnameArgsForCall = append(fake.validateRedirectHostnameArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateRedirectHostnameStub + fakeReturns := fake.validateRedirectHostnameReturns + fake.recordInvocation("ValidateRedirectHostname", []interface{}{arg1}) + fake.validateRedirectHostnameMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostnameCallCount() int { + fake.validateRedirectHostnameMutex.RLock() + defer fake.validateRedirectHostnameMutex.RUnlock() + return len(fake.validateRedirectHostnameArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostnameCalls(stub func(string) error) { + fake.validateRedirectHostnameMutex.Lock() + defer fake.validateRedirectHostnameMutex.Unlock() + fake.ValidateRedirectHostnameStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostnameArgsForCall(i int) string { + fake.validateRedirectHostnameMutex.RLock() + defer fake.validateRedirectHostnameMutex.RUnlock() + argsForCall := fake.validateRedirectHostnameArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostnameReturns(result1 error) { + fake.validateRedirectHostnameMutex.Lock() + defer fake.validateRedirectHostnameMutex.Unlock() + fake.ValidateRedirectHostnameStub = nil + fake.validateRedirectHostnameReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectHostnameReturnsOnCall(i int, result1 error) { + fake.validateRedirectHostnameMutex.Lock() + defer fake.validateRedirectHostnameMutex.Unlock() + fake.ValidateRedirectHostnameStub = nil + if fake.validateRedirectHostnameReturnsOnCall == nil { + fake.validateRedirectHostnameReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateRedirectHostnameReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPort(arg1 int32) error { + fake.validateRedirectPortMutex.Lock() + ret, specificReturn := fake.validateRedirectPortReturnsOnCall[len(fake.validateRedirectPortArgsForCall)] + fake.validateRedirectPortArgsForCall = append(fake.validateRedirectPortArgsForCall, struct { + arg1 int32 + }{arg1}) + stub := fake.ValidateRedirectPortStub + fakeReturns := fake.validateRedirectPortReturns + fake.recordInvocation("ValidateRedirectPort", []interface{}{arg1}) + fake.validateRedirectPortMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPortCallCount() int { + fake.validateRedirectPortMutex.RLock() + defer fake.validateRedirectPortMutex.RUnlock() + return len(fake.validateRedirectPortArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPortCalls(stub func(int32) error) { + fake.validateRedirectPortMutex.Lock() + defer fake.validateRedirectPortMutex.Unlock() + fake.ValidateRedirectPortStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPortArgsForCall(i int) int32 { + fake.validateRedirectPortMutex.RLock() + defer fake.validateRedirectPortMutex.RUnlock() + argsForCall := fake.validateRedirectPortArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPortReturns(result1 error) { + fake.validateRedirectPortMutex.Lock() + defer fake.validateRedirectPortMutex.Unlock() + fake.ValidateRedirectPortStub = nil + fake.validateRedirectPortReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPortReturnsOnCall(i int, result1 error) { + fake.validateRedirectPortMutex.Lock() + defer fake.validateRedirectPortMutex.Unlock() + fake.ValidateRedirectPortStub = nil + if fake.validateRedirectPortReturnsOnCall == nil { + fake.validateRedirectPortReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateRedirectPortReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectScheme(arg1 string) (bool, []string) { + fake.validateRedirectSchemeMutex.Lock() + ret, specificReturn := fake.validateRedirectSchemeReturnsOnCall[len(fake.validateRedirectSchemeArgsForCall)] + fake.validateRedirectSchemeArgsForCall = append(fake.validateRedirectSchemeArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateRedirectSchemeStub + fakeReturns := fake.validateRedirectSchemeReturns + fake.recordInvocation("ValidateRedirectScheme", []interface{}{arg1}) + fake.validateRedirectSchemeMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectSchemeCallCount() int { + fake.validateRedirectSchemeMutex.RLock() + defer fake.validateRedirectSchemeMutex.RUnlock() + return len(fake.validateRedirectSchemeArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectSchemeCalls(stub func(string) (bool, []string)) { + fake.validateRedirectSchemeMutex.Lock() + defer fake.validateRedirectSchemeMutex.Unlock() + fake.ValidateRedirectSchemeStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectSchemeArgsForCall(i int) string { + fake.validateRedirectSchemeMutex.RLock() + defer fake.validateRedirectSchemeMutex.RUnlock() + argsForCall := fake.validateRedirectSchemeArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectSchemeReturns(result1 bool, result2 []string) { + fake.validateRedirectSchemeMutex.Lock() + defer fake.validateRedirectSchemeMutex.Unlock() + fake.ValidateRedirectSchemeStub = nil + fake.validateRedirectSchemeReturns = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectSchemeReturnsOnCall(i int, result1 bool, result2 []string) { + fake.validateRedirectSchemeMutex.Lock() + defer fake.validateRedirectSchemeMutex.Unlock() + fake.ValidateRedirectSchemeStub = nil + if fake.validateRedirectSchemeReturnsOnCall == nil { + fake.validateRedirectSchemeReturnsOnCall = make(map[int]struct { + result1 bool + result2 []string + }) + } + fake.validateRedirectSchemeReturnsOnCall[i] = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCode(arg1 int) (bool, []string) { + fake.validateRedirectStatusCodeMutex.Lock() + ret, specificReturn := fake.validateRedirectStatusCodeReturnsOnCall[len(fake.validateRedirectStatusCodeArgsForCall)] + fake.validateRedirectStatusCodeArgsForCall = append(fake.validateRedirectStatusCodeArgsForCall, struct { + arg1 int + }{arg1}) + stub := fake.ValidateRedirectStatusCodeStub + fakeReturns := fake.validateRedirectStatusCodeReturns + fake.recordInvocation("ValidateRedirectStatusCode", []interface{}{arg1}) + fake.validateRedirectStatusCodeMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCodeCallCount() int { + fake.validateRedirectStatusCodeMutex.RLock() + defer fake.validateRedirectStatusCodeMutex.RUnlock() + return len(fake.validateRedirectStatusCodeArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCodeCalls(stub func(int) (bool, []string)) { + fake.validateRedirectStatusCodeMutex.Lock() + defer fake.validateRedirectStatusCodeMutex.Unlock() + fake.ValidateRedirectStatusCodeStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCodeArgsForCall(i int) int { + fake.validateRedirectStatusCodeMutex.RLock() + defer fake.validateRedirectStatusCodeMutex.RUnlock() + argsForCall := fake.validateRedirectStatusCodeArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCodeReturns(result1 bool, result2 []string) { + fake.validateRedirectStatusCodeMutex.Lock() + defer fake.validateRedirectStatusCodeMutex.Unlock() + fake.ValidateRedirectStatusCodeStub = nil + fake.validateRedirectStatusCodeReturns = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectStatusCodeReturnsOnCall(i int, result1 bool, result2 []string) { + fake.validateRedirectStatusCodeMutex.Lock() + defer fake.validateRedirectStatusCodeMutex.Unlock() + fake.ValidateRedirectStatusCodeStub = nil + if fake.validateRedirectStatusCodeReturnsOnCall == nil { + fake.validateRedirectStatusCodeReturnsOnCall = make(map[int]struct { + result1 bool + result2 []string + }) + } + fake.validateRedirectStatusCodeReturnsOnCall[i] = struct { + result1 bool + result2 []string + }{result1, result2} +} + +func (fake *FakeHTTPFieldsValidator) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.validateHeaderNameInMatchMutex.RLock() + defer fake.validateHeaderNameInMatchMutex.RUnlock() + fake.validateHeaderValueInMatchMutex.RLock() + defer fake.validateHeaderValueInMatchMutex.RUnlock() + fake.validateHostnameInServerMutex.RLock() + defer fake.validateHostnameInServerMutex.RUnlock() + fake.validateMethodInMatchMutex.RLock() + defer fake.validateMethodInMatchMutex.RUnlock() + fake.validatePathInPrefixMatchMutex.RLock() + defer fake.validatePathInPrefixMatchMutex.RUnlock() + fake.validateQueryParamNameInMatchMutex.RLock() + defer fake.validateQueryParamNameInMatchMutex.RUnlock() + fake.validateQueryParamValueInMatchMutex.RLock() + defer fake.validateQueryParamValueInMatchMutex.RUnlock() + fake.validateRedirectHostnameMutex.RLock() + defer fake.validateRedirectHostnameMutex.RUnlock() + fake.validateRedirectPortMutex.RLock() + defer fake.validateRedirectPortMutex.RUnlock() + fake.validateRedirectSchemeMutex.RLock() + defer fake.validateRedirectSchemeMutex.RUnlock() + fake.validateRedirectStatusCodeMutex.RLock() + defer fake.validateRedirectStatusCodeMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeHTTPFieldsValidator) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ validation.HTTPFieldsValidator = new(FakeHTTPFieldsValidator) diff --git a/internal/state/validation/validator.go b/internal/state/validation/validator.go new file mode 100644 index 0000000000..875f182458 --- /dev/null +++ b/internal/state/validation/validator.go @@ -0,0 +1,24 @@ +package validation + +// Validators include validators for Gateway API resources from the perspective of a data-plane. +type Validators struct { + HTTPFieldsValidator HTTPFieldsValidator +} + +// HTTPFieldsValidator validates the HTTP-related fields of Gateway API resources from the perspective of +// a data-plane. Data-plane implementations must implement this interface. +// +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . HTTPFieldsValidator +type HTTPFieldsValidator interface { + ValidateHostnameInServer(hostname string) error + ValidatePathInPrefixMatch(path string) error + ValidateHeaderNameInMatch(name string) error + ValidateHeaderValueInMatch(value string) error + ValidateQueryParamNameInMatch(name string) error + ValidateQueryParamValueInMatch(name string) error + ValidateMethodInMatch(method string) (valid bool, supportedValues []string) + ValidateRedirectScheme(scheme string) (valid bool, supportedValues []string) + ValidateRedirectHostname(hostname string) error + ValidateRedirectPort(port int32) error + ValidateRedirectStatusCode(statusCode int) (valid bool, supportedValues []string) +}