From ac39b93c8686d61fbbe80a9fba44986769d07f5c Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Mon, 27 Jun 2022 16:43:52 +0000 Subject: [PATCH 1/3] apis: validate header and query param matches Adds webhook validation to ensure that no HTTP header or query param is matched more than once in a given route rule. Signed-off-by: Steve Kriss --- apis/v1alpha2/httproute_types.go | 4 + apis/v1alpha2/validation/httproute.go | 47 +++++++ apis/v1alpha2/validation/httproute_test.go | 122 ++++++++++++++++++ apis/v1beta1/httproute_types.go | 4 + apis/v1beta1/validation/httproute.go | 47 +++++++ apis/v1beta1/validation/httproute_test.go | 122 ++++++++++++++++++ .../gateway.networking.k8s.io_httproutes.yaml | 14 +- .../gateway.networking.k8s.io_httproutes.yaml | 14 +- 8 files changed, 370 insertions(+), 4 deletions(-) diff --git a/apis/v1alpha2/httproute_types.go b/apis/v1alpha2/httproute_types.go index 4c9be7b87a..269d4e77e0 100644 --- a/apis/v1alpha2/httproute_types.go +++ b/apis/v1alpha2/httproute_types.go @@ -403,6 +403,10 @@ type HTTPQueryParamMatch struct { // exact string match. (See // https://tools.ietf.org/html/rfc7230#section-2.7.3). // + // If multiple entries specify equivalent query param names, only the first + // entry with an equivalent name MUST be considered for a match. Subsequent + // entries with an equivalent query param name MUST be ignored. + // // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=256 Name string `json:"name"` diff --git a/apis/v1alpha2/validation/httproute.go b/apis/v1alpha2/validation/httproute.go index c892fbba7c..a695a93457 100644 --- a/apis/v1alpha2/validation/httproute.go +++ b/apis/v1alpha2/validation/httproute.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "net/http" "strings" "k8s.io/apimachinery/pkg/util/validation/field" @@ -56,6 +57,12 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi if m.Path != nil { errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("matches").Index(j).Child("path"))...) } + if len(m.Headers) > 0 { + errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("matches").Index(j).Child("headers"))...) + } + if len(m.QueryParams) > 0 { + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("matches").Index(j).Child("queryParams"))...) + } } } errs = append(errs, validateHTTPRouteBackendServicePorts(spec.Rules, path.Child("rules"))...) @@ -159,6 +166,46 @@ func validateHTTPPathMatch(path *gatewayv1a2.HTTPPathMatch, fldPath *field.Path) return allErrs } +// validateHTTPHeaderMatches validates that no header name +// is matched more than once (case-insensitive). +func validateHTTPHeaderMatches(matches []gatewayv1a2.HTTPHeaderMatch, path *field.Path) field.ErrorList { + var errs field.ErrorList + counts := map[string]int{} + + for _, match := range matches { + // Header names are case-insensitive. + counts[strings.ToLower(string(match.Name))]++ + } + + for name, count := range counts { + if count > 1 { + errs = append(errs, field.Invalid(path, http.CanonicalHeaderKey(name), "cannot match the same header multiple times in the same rule")) + } + } + + return errs +} + +// validateHTTPQueryParamMatches validates that no query param name +// is matched more than once (case-sensitive). +func validateHTTPQueryParamMatches(matches []gatewayv1a2.HTTPQueryParamMatch, path *field.Path) field.ErrorList { + var errs field.ErrorList + counts := map[string]int{} + + for _, match := range matches { + // Query param names are case-sensitive. + counts[string(match.Name)]++ + } + + for name, count := range counts { + if count > 1 { + errs = append(errs, field.Invalid(path, name, "cannot match the same query parameter multiple times in the same rule")) + } + } + + return errs +} + // validateHTTPRouteFilterTypeMatchesValue validates that only the expected fields are //// set for the specified filter type. func validateHTTPRouteFilterTypeMatchesValue(filter gatewayv1a2.HTTPRouteFilter, path *field.Path) field.ErrorList { diff --git a/apis/v1alpha2/validation/httproute_test.go b/apis/v1alpha2/validation/httproute_test.go index 48833572b5..a3456784fb 100644 --- a/apis/v1alpha2/validation/httproute_test.go +++ b/apis/v1alpha2/validation/httproute_test.go @@ -572,6 +572,128 @@ func TestValidateHTTPPathMatch(t *testing.T) { } } +func TestValidateHTTPHeaderMatches(t *testing.T) { + tests := []struct { + name string + headerMatches []gatewayv1a2.HTTPHeaderMatch + errCount int + }{{ + name: "no header matches", + headerMatches: nil, + errCount: 0, + }, { + name: "no header matched more than once", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "Header-Name-3", Value: "val-3"}, + }, + errCount: 0, + }, { + name: "header matched more than once (same case)", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "Header-Name-1", Value: "val-3"}, + }, + errCount: 1, + }, { + name: "header matched more than once (different case)", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "HEADER-NAME-2", Value: "val-3"}, + }, + errCount: 1, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + route := gatewayv1a2.HTTPRoute{Spec: gatewayv1a2.HTTPRouteSpec{ + Rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + Headers: tc.headerMatches, + }}, + BackendRefs: []gatewayv1a2.HTTPBackendRef{{ + BackendRef: gatewayv1a2.BackendRef{ + BackendObjectReference: gatewayv1a2.BackendObjectReference{ + Name: gatewayv1a2.ObjectName("test"), + Port: utils.PortNumberPtr(8080), + }, + }, + }}, + }}, + }} + + errs := ValidateHTTPRoute(&route) + if len(errs) != tc.errCount { + t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + } + }) + } +} + +func TestValidateHTTPQueryParamMatches(t *testing.T) { + tests := []struct { + name string + queryParamMatches []gatewayv1a2.HTTPQueryParamMatch + errCount int + }{{ + name: "no query param matches", + queryParamMatches: nil, + errCount: 0, + }, { + name: "no query param matched more than once", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "query-param-3", Value: "val-3"}, + }, + errCount: 0, + }, { + name: "query param matched more than once", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "query-param-1", Value: "val-3"}, + }, + errCount: 1, + }, { + name: "query param names with different casing are not considered duplicates", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "QUERY-PARAM-1", Value: "val-3"}, + }, + errCount: 0, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + route := gatewayv1a2.HTTPRoute{Spec: gatewayv1a2.HTTPRouteSpec{ + Rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + QueryParams: tc.queryParamMatches, + }}, + BackendRefs: []gatewayv1a2.HTTPBackendRef{{ + BackendRef: gatewayv1a2.BackendRef{ + BackendObjectReference: gatewayv1a2.BackendObjectReference{ + Name: gatewayv1a2.ObjectName("test"), + Port: utils.PortNumberPtr(8080), + }, + }, + }}, + }}, + }} + + errs := ValidateHTTPRoute(&route) + if len(errs) != tc.errCount { + t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + } + }) + } +} + func TestValidateServicePort(t *testing.T) { portPtr := func(n int) *gatewayv1a2.PortNumber { p := gatewayv1a2.PortNumber(n) diff --git a/apis/v1beta1/httproute_types.go b/apis/v1beta1/httproute_types.go index 36cb682026..162781a5e8 100644 --- a/apis/v1beta1/httproute_types.go +++ b/apis/v1beta1/httproute_types.go @@ -402,6 +402,10 @@ type HTTPQueryParamMatch struct { // exact string match. (See // https://tools.ietf.org/html/rfc7230#section-2.7.3). // + // If multiple entries specify equivalent query param names, only the first + // entry with an equivalent name MUST be considered for a match. Subsequent + // entries with an equivalent query param name MUST be ignored. + // // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=256 Name string `json:"name"` diff --git a/apis/v1beta1/validation/httproute.go b/apis/v1beta1/validation/httproute.go index ca91c01b16..131767f11c 100644 --- a/apis/v1beta1/validation/httproute.go +++ b/apis/v1beta1/validation/httproute.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "net/http" "strings" "k8s.io/apimachinery/pkg/util/validation/field" @@ -56,6 +57,12 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi if m.Path != nil { errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("matches").Index(j).Child("path"))...) } + if len(m.Headers) > 0 { + errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("matches").Index(j).Child("headers"))...) + } + if len(m.QueryParams) > 0 { + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("matches").Index(j).Child("queryParams"))...) + } } } errs = append(errs, validateHTTPRouteBackendServicePorts(spec.Rules, path.Child("rules"))...) @@ -159,6 +166,46 @@ func validateHTTPPathMatch(path *gatewayv1a2.HTTPPathMatch, fldPath *field.Path) return allErrs } +// validateHTTPHeaderMatches validates that no header name +// is matched more than once (case-insensitive). +func validateHTTPHeaderMatches(matches []gatewayv1a2.HTTPHeaderMatch, path *field.Path) field.ErrorList { + var errs field.ErrorList + counts := map[string]int{} + + for _, match := range matches { + // Header names are case-insensitive. + counts[strings.ToLower(string(match.Name))]++ + } + + for name, count := range counts { + if count > 1 { + errs = append(errs, field.Invalid(path, http.CanonicalHeaderKey(name), "cannot match the same header multiple times in the same rule")) + } + } + + return errs +} + +// validateHTTPQueryParamMatches validates that no query param name +// is matched more than once (case-sensitive). +func validateHTTPQueryParamMatches(matches []gatewayv1a2.HTTPQueryParamMatch, path *field.Path) field.ErrorList { + var errs field.ErrorList + counts := map[string]int{} + + for _, match := range matches { + // Query param names are case-sensitive. + counts[string(match.Name)]++ + } + + for name, count := range counts { + if count > 1 { + errs = append(errs, field.Invalid(path, name, "cannot match the same query parameter multiple times in the same rule")) + } + } + + return errs +} + // validateHTTPRouteFilterTypeMatchesValue validates that only the expected fields are //// set for the specified filter type. func validateHTTPRouteFilterTypeMatchesValue(filter gatewayv1a2.HTTPRouteFilter, path *field.Path) field.ErrorList { diff --git a/apis/v1beta1/validation/httproute_test.go b/apis/v1beta1/validation/httproute_test.go index e951eb1027..db7abf524f 100644 --- a/apis/v1beta1/validation/httproute_test.go +++ b/apis/v1beta1/validation/httproute_test.go @@ -572,6 +572,128 @@ func TestValidateHTTPPathMatch(t *testing.T) { } } +func TestValidateHTTPHeaderMatches(t *testing.T) { + tests := []struct { + name string + headerMatches []gatewayv1a2.HTTPHeaderMatch + errCount int + }{{ + name: "no header matches", + headerMatches: nil, + errCount: 0, + }, { + name: "no header matched more than once", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "Header-Name-3", Value: "val-3"}, + }, + errCount: 0, + }, { + name: "header matched more than once (same case)", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "Header-Name-1", Value: "val-3"}, + }, + errCount: 1, + }, { + name: "header matched more than once (different case)", + headerMatches: []gatewayv1a2.HTTPHeaderMatch{ + {Name: "Header-Name-1", Value: "val-1"}, + {Name: "Header-Name-2", Value: "val-2"}, + {Name: "HEADER-NAME-2", Value: "val-3"}, + }, + errCount: 1, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + route := gatewayv1a2.HTTPRoute{Spec: gatewayv1a2.HTTPRouteSpec{ + Rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + Headers: tc.headerMatches, + }}, + BackendRefs: []gatewayv1a2.HTTPBackendRef{{ + BackendRef: gatewayv1a2.BackendRef{ + BackendObjectReference: gatewayv1a2.BackendObjectReference{ + Name: gatewayv1a2.ObjectName("test"), + Port: utils.PortNumberPtr(8080), + }, + }, + }}, + }}, + }} + + errs := ValidateHTTPRoute(&route) + if len(errs) != tc.errCount { + t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + } + }) + } +} + +func TestValidateHTTPQueryParamMatches(t *testing.T) { + tests := []struct { + name string + queryParamMatches []gatewayv1a2.HTTPQueryParamMatch + errCount int + }{{ + name: "no query param matches", + queryParamMatches: nil, + errCount: 0, + }, { + name: "no query param matched more than once", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "query-param-3", Value: "val-3"}, + }, + errCount: 0, + }, { + name: "query param matched more than once", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "query-param-1", Value: "val-3"}, + }, + errCount: 1, + }, { + name: "query param names with different casing are not considered duplicates", + queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ + {Name: "query-param-1", Value: "val-1"}, + {Name: "query-param-2", Value: "val-2"}, + {Name: "QUERY-PARAM-1", Value: "val-3"}, + }, + errCount: 0, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + route := gatewayv1a2.HTTPRoute{Spec: gatewayv1a2.HTTPRouteSpec{ + Rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + QueryParams: tc.queryParamMatches, + }}, + BackendRefs: []gatewayv1a2.HTTPBackendRef{{ + BackendRef: gatewayv1a2.BackendRef{ + BackendObjectReference: gatewayv1a2.BackendObjectReference{ + Name: gatewayv1a2.ObjectName("test"), + Port: utils.PortNumberPtr(8080), + }, + }, + }}, + }}, + }} + + errs := ValidateHTTPRoute(&route) + if len(errs) != tc.errCount { + t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + } + }) + } +} + func TestValidateServicePort(t *testing.T) { portPtr := func(n int) *gatewayv1a2.PortNumber { p := gatewayv1a2.PortNumber(n) diff --git a/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml b/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml index 492ef986a3..f12e2e976f 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml @@ -1275,9 +1275,14 @@ spec: a HTTP route by matching HTTP query parameters. properties: name: - description: Name is the name of the HTTP query + description: "Name is the name of the HTTP query param to be matched. This must be an exact string match. (See https://tools.ietf.org/html/rfc7230#section-2.7.3). + \n If multiple entries specify equivalent query + param names, only the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST + be ignored." maxLength: 256 minLength: 1 type: string @@ -2807,9 +2812,14 @@ spec: a HTTP route by matching HTTP query parameters. properties: name: - description: Name is the name of the HTTP query + description: "Name is the name of the HTTP query param to be matched. This must be an exact string match. (See https://tools.ietf.org/html/rfc7230#section-2.7.3). + \n If multiple entries specify equivalent query + param names, only the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST + be ignored." maxLength: 256 minLength: 1 type: string diff --git a/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml b/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml index 82c4ebe7e0..11e66c8077 100644 --- a/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml +++ b/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml @@ -1058,9 +1058,14 @@ spec: a HTTP route by matching HTTP query parameters. properties: name: - description: Name is the name of the HTTP query + description: "Name is the name of the HTTP query param to be matched. This must be an exact string match. (See https://tools.ietf.org/html/rfc7230#section-2.7.3). + \n If multiple entries specify equivalent query + param names, only the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST + be ignored." maxLength: 256 minLength: 1 type: string @@ -2345,9 +2350,14 @@ spec: a HTTP route by matching HTTP query parameters. properties: name: - description: Name is the name of the HTTP query + description: "Name is the name of the HTTP query param to be matched. This must be an exact string match. (See https://tools.ietf.org/html/rfc7230#section-2.7.3). + \n If multiple entries specify equivalent query + param names, only the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST + be ignored." maxLength: 256 minLength: 1 type: string From eb6321c78f7efaf8d8f14d9de7d452675093a6f3 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 28 Jun 2022 14:58:30 +0000 Subject: [PATCH 2/3] check specific errors in tests, fix route match error paths Signed-off-by: Steve Kriss --- apis/v1alpha2/validation/httproute.go | 8 +++-- apis/v1alpha2/validation/httproute_test.go | 36 +++++++++++++--------- apis/v1beta1/validation/httproute.go | 8 +++-- apis/v1beta1/validation/httproute_test.go | 36 +++++++++++++--------- 4 files changed, 54 insertions(+), 34 deletions(-) diff --git a/apis/v1alpha2/validation/httproute.go b/apis/v1alpha2/validation/httproute.go index a695a93457..f3932bbb76 100644 --- a/apis/v1alpha2/validation/httproute.go +++ b/apis/v1alpha2/validation/httproute.go @@ -54,14 +54,16 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, rule.Matches, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) } for j, m := range rule.Matches { + path := path.Child("rules").Index(i).Child("matches").Index(j) + if m.Path != nil { - errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("matches").Index(j).Child("path"))...) + errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("path"))...) } if len(m.Headers) > 0 { - errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("matches").Index(j).Child("headers"))...) + errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("headers"))...) } if len(m.QueryParams) > 0 { - errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("matches").Index(j).Child("queryParams"))...) + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("queryParams"))...) } } } diff --git a/apis/v1alpha2/validation/httproute_test.go b/apis/v1alpha2/validation/httproute_test.go index a3456784fb..0920ecc19e 100644 --- a/apis/v1alpha2/validation/httproute_test.go +++ b/apis/v1alpha2/validation/httproute_test.go @@ -19,6 +19,8 @@ package validation import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/validation/field" utilpointer "k8s.io/utils/pointer" @@ -576,11 +578,11 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { tests := []struct { name string headerMatches []gatewayv1a2.HTTPHeaderMatch - errCount int + expectErr string }{{ name: "no header matches", headerMatches: nil, - errCount: 0, + expectErr: "", }, { name: "no header matched more than once", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -588,7 +590,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "Header-Name-3", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }, { name: "header matched more than once (same case)", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -596,7 +598,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "Header-Name-1", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].headers: Invalid value: \"Header-Name-1\": cannot match the same header multiple times in the same rule", }, { name: "header matched more than once (different case)", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -604,7 +606,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "HEADER-NAME-2", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].headers: Invalid value: \"Header-Name-2\": cannot match the same header multiple times in the same rule", }} for _, tc := range tests { @@ -626,8 +628,11 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { }} errs := ValidateHTTPRoute(&route) - if len(errs) != tc.errCount { - t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + if len(tc.expectErr) == 0 { + assert.Emptyf(t, errs, "expected no errors, got %d errors: %s", len(errs), errs) + } else { + require.Lenf(t, errs, 1, "expected one error, got %d errors: %s", len(errs), errs) + assert.Equal(t, tc.expectErr, errs[0].Error()) } }) } @@ -637,11 +642,11 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { tests := []struct { name string queryParamMatches []gatewayv1a2.HTTPQueryParamMatch - errCount int + expectErr string }{{ name: "no query param matches", queryParamMatches: nil, - errCount: 0, + expectErr: "", }, { name: "no query param matched more than once", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -649,7 +654,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "query-param-3", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }, { name: "query param matched more than once", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -657,7 +662,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "query-param-1", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].queryParams: Invalid value: \"query-param-1\": cannot match the same query parameter multiple times in the same rule", }, { name: "query param names with different casing are not considered duplicates", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -665,7 +670,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "QUERY-PARAM-1", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }} for _, tc := range tests { @@ -687,8 +692,11 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { }} errs := ValidateHTTPRoute(&route) - if len(errs) != tc.errCount { - t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + if len(tc.expectErr) == 0 { + assert.Emptyf(t, errs, "expected no errors, got %d errors: %s", len(errs), errs) + } else { + require.Lenf(t, errs, 1, "expected one error, got %d errors: %s", len(errs), errs) + assert.Equal(t, tc.expectErr, errs[0].Error()) } }) } diff --git a/apis/v1beta1/validation/httproute.go b/apis/v1beta1/validation/httproute.go index 131767f11c..2da0a3c14d 100644 --- a/apis/v1beta1/validation/httproute.go +++ b/apis/v1beta1/validation/httproute.go @@ -54,14 +54,16 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, rule.Matches, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) } for j, m := range rule.Matches { + path := path.Child("rules").Index(i).Child("matches").Index(j) + if m.Path != nil { - errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("matches").Index(j).Child("path"))...) + errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("path"))...) } if len(m.Headers) > 0 { - errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("matches").Index(j).Child("headers"))...) + errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("headers"))...) } if len(m.QueryParams) > 0 { - errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("matches").Index(j).Child("queryParams"))...) + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("queryParams"))...) } } } diff --git a/apis/v1beta1/validation/httproute_test.go b/apis/v1beta1/validation/httproute_test.go index db7abf524f..7c02e2d287 100644 --- a/apis/v1beta1/validation/httproute_test.go +++ b/apis/v1beta1/validation/httproute_test.go @@ -19,6 +19,8 @@ package validation import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/validation/field" utilpointer "k8s.io/utils/pointer" @@ -576,11 +578,11 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { tests := []struct { name string headerMatches []gatewayv1a2.HTTPHeaderMatch - errCount int + expectErr string }{{ name: "no header matches", headerMatches: nil, - errCount: 0, + expectErr: "", }, { name: "no header matched more than once", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -588,7 +590,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "Header-Name-3", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }, { name: "header matched more than once (same case)", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -596,7 +598,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "Header-Name-1", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].headers: Invalid value: \"Header-Name-1\": cannot match the same header multiple times in the same rule", }, { name: "header matched more than once (different case)", headerMatches: []gatewayv1a2.HTTPHeaderMatch{ @@ -604,7 +606,7 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { {Name: "Header-Name-2", Value: "val-2"}, {Name: "HEADER-NAME-2", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].headers: Invalid value: \"Header-Name-2\": cannot match the same header multiple times in the same rule", }} for _, tc := range tests { @@ -626,8 +628,11 @@ func TestValidateHTTPHeaderMatches(t *testing.T) { }} errs := ValidateHTTPRoute(&route) - if len(errs) != tc.errCount { - t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + if len(tc.expectErr) == 0 { + assert.Emptyf(t, errs, "expected no errors, got %d errors: %s", len(errs), errs) + } else { + require.Lenf(t, errs, 1, "expected one error, got %d errors: %s", len(errs), errs) + assert.Equal(t, tc.expectErr, errs[0].Error()) } }) } @@ -637,11 +642,11 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { tests := []struct { name string queryParamMatches []gatewayv1a2.HTTPQueryParamMatch - errCount int + expectErr string }{{ name: "no query param matches", queryParamMatches: nil, - errCount: 0, + expectErr: "", }, { name: "no query param matched more than once", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -649,7 +654,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "query-param-3", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }, { name: "query param matched more than once", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -657,7 +662,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "query-param-1", Value: "val-3"}, }, - errCount: 1, + expectErr: "spec.rules[0].matches[0].queryParams: Invalid value: \"query-param-1\": cannot match the same query parameter multiple times in the same rule", }, { name: "query param names with different casing are not considered duplicates", queryParamMatches: []gatewayv1a2.HTTPQueryParamMatch{ @@ -665,7 +670,7 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { {Name: "query-param-2", Value: "val-2"}, {Name: "QUERY-PARAM-1", Value: "val-3"}, }, - errCount: 0, + expectErr: "", }} for _, tc := range tests { @@ -687,8 +692,11 @@ func TestValidateHTTPQueryParamMatches(t *testing.T) { }} errs := ValidateHTTPRoute(&route) - if len(errs) != tc.errCount { - t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs) + if len(tc.expectErr) == 0 { + assert.Emptyf(t, errs, "expected no errors, got %d errors: %s", len(errs), errs) + } else { + require.Lenf(t, errs, 1, "expected one error, got %d errors: %s", len(errs), errs) + assert.Equal(t, tc.expectErr, errs[0].Error()) } }) } From 87b2f8cf74cee8754dc5a14caeaffe7666f4f45c Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 28 Jun 2022 15:34:53 +0000 Subject: [PATCH 3/3] make linter happy Signed-off-by: Steve Kriss --- apis/v1alpha2/validation/httproute.go | 8 ++++---- apis/v1beta1/validation/httproute.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apis/v1alpha2/validation/httproute.go b/apis/v1alpha2/validation/httproute.go index f3932bbb76..2b94ea0be3 100644 --- a/apis/v1alpha2/validation/httproute.go +++ b/apis/v1alpha2/validation/httproute.go @@ -54,16 +54,16 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, rule.Matches, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) } for j, m := range rule.Matches { - path := path.Child("rules").Index(i).Child("matches").Index(j) + matchPath := path.Child("rules").Index(i).Child("matches").Index(j) if m.Path != nil { - errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("path"))...) + errs = append(errs, validateHTTPPathMatch(m.Path, matchPath.Child("path"))...) } if len(m.Headers) > 0 { - errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("headers"))...) + errs = append(errs, validateHTTPHeaderMatches(m.Headers, matchPath.Child("headers"))...) } if len(m.QueryParams) > 0 { - errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("queryParams"))...) + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, matchPath.Child("queryParams"))...) } } } diff --git a/apis/v1beta1/validation/httproute.go b/apis/v1beta1/validation/httproute.go index 2da0a3c14d..974aa9e3ce 100644 --- a/apis/v1beta1/validation/httproute.go +++ b/apis/v1beta1/validation/httproute.go @@ -54,16 +54,16 @@ func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) fi errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, rule.Matches, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) } for j, m := range rule.Matches { - path := path.Child("rules").Index(i).Child("matches").Index(j) + matchPath := path.Child("rules").Index(i).Child("matches").Index(j) if m.Path != nil { - errs = append(errs, validateHTTPPathMatch(m.Path, path.Child("path"))...) + errs = append(errs, validateHTTPPathMatch(m.Path, matchPath.Child("path"))...) } if len(m.Headers) > 0 { - errs = append(errs, validateHTTPHeaderMatches(m.Headers, path.Child("headers"))...) + errs = append(errs, validateHTTPHeaderMatches(m.Headers, matchPath.Child("headers"))...) } if len(m.QueryParams) > 0 { - errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, path.Child("queryParams"))...) + errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, matchPath.Child("queryParams"))...) } } }