diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index ad5d00a0dc..d62b04ba1f 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -38,6 +38,10 @@ const ( // Route rules has a backendRef with an unsupported value. RouteReasonBackendRefUnsupportedValue v1.RouteConditionReason = "UnsupportedValue" + // RouteReasonUnsupportedField is used with the "Accepted" condition when a Route contains fields that are + // not yet supported. + RouteReasonUnsupportedField v1.RouteConditionReason = "UnsupportedField" + // RouteReasonInvalidGateway is used with the "Accepted" (false) condition when the Gateway the Route // references is invalid. RouteReasonInvalidGateway v1.RouteConditionReason = "InvalidGateway" @@ -62,6 +66,10 @@ const ( // invalid. Used with ResolvedRefs (false). RouteReasonInvalidFilter v1.RouteConditionReason = "InvalidFilter" + // GatewayReasonUnsupportedField is used with the "Accepted" condition when a Gateway contains fields + // that are not yet supported. + GatewayReasonUnsupportedField v1.GatewayConditionReason = "UnsupportedField" + // GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway // is invalid or not supported. GatewayReasonUnsupportedValue v1.GatewayConditionReason = "UnsupportedValue" @@ -354,6 +362,17 @@ func NewRouteUnsupportedValue(msg string) Condition { } } +// NewRouteAcceptedUnsupportedField returns a Condition that indicates that the Route is accepted but +// includes an unsupported field. +func NewRouteAcceptedUnsupportedField(msg string) Condition { + return Condition{ + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(RouteReasonUnsupportedField), + Message: fmt.Sprintf("The following unsupported parameters were ignored: %s", msg), + } +} + // NewRoutePartiallyInvalid returns a Condition that indicates that the Route contains a combination // of both valid and invalid rules. // @@ -945,6 +964,17 @@ func NewGatewayInvalidParameters(msg string) Condition { } } +// NewGatewayAcceptedUnsupportedField returns a Condition that indicates the Gateway is accepted but +// contains a field that is not supported. +func NewGatewayAcceptedUnsupportedField(msg string) Condition { + return Condition{ + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(GatewayReasonUnsupportedField), + Message: fmt.Sprintf("Gateway accepted but the following unsupported parameters were ignored: %s", msg), + } +} + // NewPolicyAccepted returns a Condition that indicates that the Policy is accepted. func NewPolicyAccepted() Condition { return Condition{ diff --git a/internal/controller/state/graph/gateway.go b/internal/controller/state/graph/gateway.go index 103634871e..4486c58606 100644 --- a/internal/controller/state/graph/gateway.go +++ b/internal/controller/state/graph/gateway.go @@ -193,6 +193,9 @@ func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]con // invalidate the entire Gateway. valid := len(conds) == 0 + // Validate unsupported fields - these are warnings, don't affect validity + conds = append(conds, validateUnsupportedGatewayFields(gw)...) + if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil { paramConds := validateGatewayParametersRef(npCfg, *gw.Spec.Infrastructure.ParametersRef) conds = append(conds, paramConds...) @@ -267,3 +270,17 @@ func (g *Gateway) collectSnippetsFiltersFromRoute( } } } + +func validateUnsupportedGatewayFields(gw *v1.Gateway) []conditions.Condition { + var conds []conditions.Condition + + if gw.Spec.AllowedListeners != nil { + conds = append(conds, conditions.NewGatewayAcceptedUnsupportedField("AllowedListeners")) + } + + if gw.Spec.BackendTLS != nil { + conds = append(conds, conditions.NewGatewayAcceptedUnsupportedField("BackendTLS")) + } + + return conds +} diff --git a/internal/controller/state/graph/gateway_test.go b/internal/controller/state/graph/gateway_test.go index f31fa1dd3c..e3978df1a9 100644 --- a/internal/controller/state/graph/gateway_test.go +++ b/internal/controller/state/graph/gateway_test.go @@ -286,10 +286,12 @@ func TestBuildGateway(t *testing.T) { ) type gatewayCfg struct { - name string - ref *v1.LocalParametersReference - listeners []v1.Listener - addresses []v1.GatewaySpecAddress + ref *v1.LocalParametersReference + allowedListeners *v1.AllowedListeners + backendTLS *v1.GatewayBackendTLS + name string + listeners []v1.Listener + addresses []v1.GatewaySpecAddress } var lastCreatedGateway *v1.Gateway @@ -304,6 +306,8 @@ func TestBuildGateway(t *testing.T) { GatewayClassName: gcName, Listeners: cfg.listeners, Addresses: cfg.addresses, + AllowedListeners: cfg.allowedListeners, + BackendTLS: cfg.backendTLS, }, } @@ -1513,6 +1517,103 @@ func TestBuildGateway(t *testing.T) { }, }, }, + { + name: "One unsupported field + supported fields (valid)", + gateway: createGateway(gatewayCfg{ + name: "gateway-valid-np", + listeners: []v1.Listener{foo80Listener1}, + ref: validGwNpRef, + allowedListeners: &v1.AllowedListeners{}, + }), + gatewayClass: validGCWithNp, + expected: map[types.NamespacedName]*Gateway{ + {Namespace: validGwNp.Namespace, Name: "gateway-valid-np"}: { + Source: getLastCreatedGateway(), + Listeners: []*Listener{ + { + Name: "foo-80-1", + GatewayName: client.ObjectKeyFromObject(getLastCreatedGateway()), + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, + SupportedKinds: supportedKindsForListeners, + }, + }, + DeploymentName: types.NamespacedName{ + Namespace: "test", + Name: controller.CreateNginxResourceName("gateway-valid-np", gcName), + }, + Valid: true, + NginxProxy: &NginxProxy{ + Source: validGwNp, + Valid: true, + }, + EffectiveNginxProxy: &EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelError), + }, + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), + Metrics: &ngfAPIv1alpha2.Metrics{ + Disable: helpers.GetPointer(false), + Port: helpers.GetPointer(int32(90)), + }, + }, + Conditions: []conditions.Condition{ + conditions.NewGatewayAcceptedUnsupportedField("AllowedListeners"), + conditions.NewGatewayResolvedRefs(), + }, + }, + }, + }, + { + name: "One unsupported field + NewGatewayRefInvalid (invalid)", + gateway: createGateway(gatewayCfg{ + name: "gateway-valid-np", + listeners: []v1.Listener{foo80Listener1}, + ref: &v1.LocalParametersReference{ + Kind: "wrong-kind", // Invalid reference + Name: "invalid-ref", + }, + backendTLS: &v1.GatewayBackendTLS{}, + }), + gatewayClass: validGCWithNp, + expected: map[types.NamespacedName]*Gateway{ + {Namespace: validGwNp.Namespace, Name: "gateway-valid-np"}: { + Source: getLastCreatedGateway(), + Listeners: []*Listener{ + { + Name: "foo-80-1", + GatewayName: client.ObjectKeyFromObject(getLastCreatedGateway()), + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, + SupportedKinds: supportedKindsForListeners, + }, + }, + DeploymentName: types.NamespacedName{ + Namespace: "test", + Name: controller.CreateNginxResourceName("gateway-valid-np", gcName), + }, + Valid: true, + EffectiveNginxProxy: &EffectiveNginxProxy{ + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), + }, + Conditions: []conditions.Condition{ + conditions.NewGatewayAcceptedUnsupportedField("BackendTLS"), + conditions.NewGatewayRefInvalid( + "spec.infrastructure.parametersRef.kind: Unsupported value: \"wrong-kind\": supported values: \"NginxProxy\"", + ), + conditions.NewGatewayInvalidParameters( + "spec.infrastructure.parametersRef.kind: Unsupported value: \"wrong-kind\": supported values: \"NginxProxy\"", + ), + }, + }, + }, + }, } secretResolver := newSecretResolver( @@ -1821,3 +1922,55 @@ func TestGetReferencedSnippetsFilters(t *testing.T) { emptyFilterResult := gw.GetReferencedSnippetsFilters(routes, map[types.NamespacedName]*SnippetsFilter{}) g.Expect(emptyFilterResult).To(BeEmpty()) } + +func TestValidateUnsupportedGatewayFields(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + gateway *v1.Gateway + expectedConds []conditions.Condition + }{ + { + name: "No unsupported fields", + gateway: &v1.Gateway{ + Spec: v1.GatewaySpec{}, + }, + expectedConds: nil, + }, + { + name: "One unsupported field: AllowedListeners", + gateway: &v1.Gateway{ + Spec: v1.GatewaySpec{ + AllowedListeners: &v1.AllowedListeners{}, + }, + }, + expectedConds: []conditions.Condition{ + conditions.NewGatewayAcceptedUnsupportedField("AllowedListeners"), + }, + }, + { + name: "Multiple unsupported fields: AllowedListeners and BackendTLS", + gateway: &v1.Gateway{ + Spec: v1.GatewaySpec{ + AllowedListeners: &v1.AllowedListeners{}, + BackendTLS: &v1.GatewayBackendTLS{}, + }, + }, + expectedConds: []conditions.Condition{ + conditions.NewGatewayAcceptedUnsupportedField("AllowedListeners"), + conditions.NewGatewayAcceptedUnsupportedField("BackendTLS"), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + conds := validateUnsupportedGatewayFields(test.gateway) + g.Expect(conds).To(Equal(test.expectedConds)) + }) + } +} diff --git a/internal/controller/state/graph/grpcroute.go b/internal/controller/state/graph/grpcroute.go index 68b3a6ed06..5d6e7489e3 100644 --- a/internal/controller/state/graph/grpcroute.go +++ b/internal/controller/state/graph/grpcroute.go @@ -165,6 +165,11 @@ func processGRPCRouteRule( validMatches := true + unsupportedFieldsErrors := checkForUnsupportedGRPCFields(specRule, rulePath) + if len(unsupportedFieldsErrors) > 0 { + errors.warn = append(errors.warn, unsupportedFieldsErrors...) + } + for j, match := range specRule.Matches { matchPath := rulePath.Child("matches").Index(j) @@ -260,6 +265,11 @@ func processGRPCRouteRules( conds = make([]conditions.Condition, 0, 2) valid = true + // add warning condition for unsupported fields if any + if len(allRulesErrors.warn) > 0 { + conds = append(conds, conditions.NewRouteAcceptedUnsupportedField(allRulesErrors.warn.ToAggregate().Error())) + } + if len(allRulesErrors.invalid) > 0 { msg := allRulesErrors.invalid.ToAggregate().Error() @@ -444,3 +454,26 @@ func validateGRPCHeaderMatch( return allErrs } + +func checkForUnsupportedGRPCFields(rule v1.GRPCRouteRule, rulePath *field.Path) field.ErrorList { + var ruleErrors field.ErrorList + + if rule.Name != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("name"), + "Name", + )) + } + if rule.SessionPersistence != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("sessionPersistence"), + "SessionPersistence", + )) + } + + if len(ruleErrors) == 0 { + return nil + } + + return ruleErrors +} diff --git a/internal/controller/state/graph/grpcroute_test.go b/internal/controller/state/graph/grpcroute_test.go index 8579c54627..e78f22fe53 100644 --- a/internal/controller/state/graph/grpcroute_test.go +++ b/internal/controller/state/graph/grpcroute_test.go @@ -7,12 +7,14 @@ import ( . "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" v1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/mirror" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation/validationfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" @@ -342,6 +344,46 @@ func TestBuildGRPCRoute(t *testing.T) { []v1.GRPCRouteRule{methodMatchRule, headersMatchInvalid}, ) + grValidWithUnsupportedField := createGRPCRoute( + "gr-valid-unsupported", + gatewayNsName.Name, + "example.com", + []v1.GRPCRouteRule{ + { + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + Matches: []v1.GRPCRouteMatch{ + { + Method: &v1.GRPCMethodMatch{ + Type: helpers.GetPointer(v1.GRPCMethodMatchExact), + Service: helpers.GetPointer("myService"), + Method: helpers.GetPointer("myMethod"), + }, + }, + }, + }, + }, + ) + + grInvalidWithUnsupportedField := createGRPCRoute( + "gr-invalid-unsupported", + gatewayNsName.Name, + "example.com", + []v1.GRPCRouteRule{ + { + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + Matches: []v1.GRPCRouteMatch{ + { + Method: &v1.GRPCMethodMatch{ + Type: helpers.GetPointer(v1.GRPCMethodMatchExact), + Service: helpers.GetPointer(""), + Method: helpers.GetPointer(""), + }, + }, + }, + }, + }, + ) + grDuplicateSectionName := createGRPCRoute( "gr", gatewayNsName.Name, @@ -1068,6 +1110,80 @@ func TestBuildGRPCRoute(t *testing.T) { }, name: "one invalid and one unresolvable snippet filter extension ref", }, + { + validator: createAllValidValidator(), + gr: grValidWithUnsupportedField, + expected: &L7Route{ + RouteType: RouteTypeGRPC, + Source: grValidWithUnsupportedField, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: CreateParentRefGateway(gw), + SectionName: grValidWithUnsupportedField.Spec.ParentRefs[0].SectionName, + }, + }, + Valid: true, + Attachable: true, + Spec: L7RouteSpec{ + Hostnames: grValidWithUnsupportedField.Spec.Hostnames, + Rules: []RouteRule{ + { + ValidMatches: true, + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{}, + }, + Matches: ConvertGRPCMatches(grValidWithUnsupportedField.Spec.Rules[0].Matches), + RouteBackendRefs: []RouteBackendRef{}, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("spec.rules[0].name: Forbidden: Name"), + }, + }, + name: "valid route with unsupported field", + }, + { + validator: createAllValidValidator(), + gr: grInvalidWithUnsupportedField, + expected: &L7Route{ + RouteType: RouteTypeGRPC, + Source: grInvalidWithUnsupportedField, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: CreateParentRefGateway(gw), + SectionName: grInvalidWithUnsupportedField.Spec.ParentRefs[0].SectionName, + }, + }, + Valid: false, + Attachable: true, + Spec: L7RouteSpec{ + Hostnames: grInvalidWithUnsupportedField.Spec.Hostnames, + Rules: []RouteRule{ + { + ValidMatches: false, + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{}, + }, + Matches: ConvertGRPCMatches(grInvalidWithUnsupportedField.Spec.Rules[0].Matches), + RouteBackendRefs: []RouteBackendRef{}, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("spec.rules[0].name: Forbidden: Name"), + conditions.NewRouteUnsupportedValue( + "All rules are invalid: [spec.rules[0].matches[0].method.service: Required value: service is required, " + + "spec.rules[0].matches[0].method.method: Required value: method is required]", + ), + }, + }, + name: "invalid route with unsupported field", + }, } gws := map[types.NamespacedName]*Gateway{ @@ -1376,3 +1492,128 @@ func TestConvertGRPCHeaderMatchType(t *testing.T) { }) } } + +func TestProcessGRPCRouteRule_UnsupportedFields(t *testing.T) { + t.Parallel() + + tests := []struct { + specRule v1.GRPCRouteRule + name string + expectedErrors int + }{ + { + name: "No unsupported fields", + specRule: v1.GRPCRouteRule{}, // Empty rule, no unsupported fields + expectedErrors: 0, + }, + { + name: "One unsupported field", + specRule: v1.GRPCRouteRule{ + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + }, + expectedErrors: 1, + }, + { + name: "Multiple unsupported fields", + specRule: v1.GRPCRouteRule{ + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + SessionPersistence: helpers.GetPointer( + v1.SessionPersistence{ + Type: helpers.GetPointer(v1.SessionPersistenceType("unsupported-session-persistence")), + }), + }, + expectedErrors: 2, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + rulePath := field.NewPath("spec").Child("rules") + var errors routeRuleErrors + + // Wrap the rule in GRPCRouteRuleWrapper + unsupportedFieldsErrors := checkForUnsupportedGRPCFields(test.specRule, rulePath) + if len(unsupportedFieldsErrors) > 0 { + errors.warn = append(errors.warn, unsupportedFieldsErrors...) + } + + g.Expect(errors.warn).To(HaveLen(test.expectedErrors)) + }) + } +} + +func TestProcessGRPCRouteRules_UnsupportedFields(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + specRules []v1.GRPCRouteRule + expectedConds []conditions.Condition + expectedWarns int + expectedValid bool + }{ + { + name: "No unsupported fields", + specRules: []v1.GRPCRouteRule{{}}, + expectedValid: true, + expectedConds: nil, + expectedWarns: 0, + }, + { + name: "One unsupported field", + specRules: []v1.GRPCRouteRule{ + { + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + }, + }, + expectedValid: true, + expectedConds: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("spec.rules[0].name: Forbidden: Name"), + }, + expectedWarns: 1, + }, + { + name: "Multiple unsupported fields", + specRules: []v1.GRPCRouteRule{ + { + Name: helpers.GetPointer[v1.SectionName]("unsupported-name"), + SessionPersistence: helpers.GetPointer(v1.SessionPersistence{ + Type: helpers.GetPointer(v1.SessionPersistenceType("unsupported-session-persistence")), + }), + }, + }, + expectedValid: true, + expectedConds: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("[spec.rules[0].name: Forbidden: Name, " + + "spec.rules[0].sessionPersistence: Forbidden: SessionPersistence]"), + }, + expectedWarns: 2, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + _, valid, conds := processGRPCRouteRules( + test.specRules, + validation.SkipValidator{}, + nil, + ) + + g.Expect(valid).To(Equal(test.expectedValid)) + if test.expectedConds == nil { + g.Expect(conds).To(BeEmpty()) + } else { + g.Expect(conds).To(HaveLen(len(test.expectedConds))) + for i, expectedCond := range test.expectedConds { + g.Expect(conds[i].Message).To(Equal(expectedCond.Message)) + } + } + }) + } +} diff --git a/internal/controller/state/graph/httproute.go b/internal/controller/state/graph/httproute.go index 983bd8be1b..0fbf0cdb49 100644 --- a/internal/controller/state/graph/httproute.go +++ b/internal/controller/state/graph/httproute.go @@ -169,6 +169,11 @@ func processHTTPRouteRule( ) (RouteRule, routeRuleErrors) { var errors routeRuleErrors + unsupportedFieldsErrors := checkForUnsupportedHTTPFields(specRule, rulePath) + if len(unsupportedFieldsErrors) > 0 { + errors.warn = append(errors.warn, unsupportedFieldsErrors...) + } + validMatches := true for j, match := range specRule.Matches { @@ -267,6 +272,11 @@ func processHTTPRouteRules( valid = true + // add warning condition for unsupported fields if any + if len(allRulesErrors.warn) > 0 { + conds = append(conds, conditions.NewRouteAcceptedUnsupportedField(allRulesErrors.warn.ToAggregate().Error())) + } + if len(allRulesErrors.invalid) > 0 { msg := allRulesErrors.invalid.ToAggregate().Error() @@ -524,3 +534,38 @@ func validateFilterRewrite( return allErrs } + +func checkForUnsupportedHTTPFields(rule v1.HTTPRouteRule, rulePath *field.Path) field.ErrorList { + var ruleErrors field.ErrorList + + if rule.Name != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("name"), + "Name", + )) + } + if rule.Timeouts != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("timeouts"), + "Timeouts", + )) + } + if rule.Retry != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("retry"), + "Retry", + )) + } + if rule.SessionPersistence != nil { + ruleErrors = append(ruleErrors, field.Forbidden( + rulePath.Child("sessionPersistence"), + "SessionPersistence", + )) + } + + if len(ruleErrors) == 0 { + return nil + } + + return ruleErrors +} diff --git a/internal/controller/state/graph/httproute_test.go b/internal/controller/state/graph/httproute_test.go index b75058748d..f765ec78d4 100644 --- a/internal/controller/state/graph/httproute_test.go +++ b/internal/controller/state/graph/httproute_test.go @@ -14,6 +14,7 @@ import ( ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/mirror" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation/validationfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" @@ -287,6 +288,10 @@ func TestBuildHTTPRoute(t *testing.T) { } gatewayNsName := client.ObjectKeyFromObject(gw.Source) + // Valid HTTPRoute with unsupported rule fields + hrValidWithUnsupportedField := createHTTPRoute("hr-valid-unsupported", gatewayNsName.Name, "example.com", "/") + hrValidWithUnsupportedField.Spec.Rules[0].Name = helpers.GetPointer[gatewayv1.SectionName]("unsupported-name") + // route with valid filter validFilter := gatewayv1.HTTPRouteFilter{ Type: gatewayv1.HTTPRouteFilterRequestRedirect, @@ -943,6 +948,41 @@ func TestBuildHTTPRoute(t *testing.T) { }, name: "rule with one invalid and one unresolvable snippets filter extension ref filter", }, + { + validator: &validationfakes.FakeHTTPFieldsValidator{}, + hr: hrValidWithUnsupportedField, + expected: &L7Route{ + RouteType: RouteTypeHTTP, + Source: hrValidWithUnsupportedField, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: CreateParentRefGateway(gw), + SectionName: hrValidWithUnsupportedField.Spec.ParentRefs[0].SectionName, + }, + }, + Valid: true, + Attachable: true, + Spec: L7RouteSpec{ + Hostnames: hrValidWithUnsupportedField.Spec.Hostnames, + Rules: []RouteRule{ + { + ValidMatches: true, + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{}, + }, + Matches: hrValidWithUnsupportedField.Spec.Rules[0].Matches, + RouteBackendRefs: []RouteBackendRef{expRouteBackendRef}, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("spec.rules[0].name: Forbidden: Name"), + }, + }, + name: "valid route with unsupported field", + }, } gws := map[types.NamespacedName]*Gateway{ @@ -1675,3 +1715,135 @@ func TestValidateFilterRewrite(t *testing.T) { }) } } + +func TestUnsupportedFieldsErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + specRule gatewayv1.HTTPRouteRule + name string + expectedErrors int + }{ + { + name: "No unsupported fields", + specRule: gatewayv1.HTTPRouteRule{}, // Empty rule, no unsupported fields + expectedErrors: 0, + }, + { + name: "One unsupported field", + specRule: gatewayv1.HTTPRouteRule{ + Name: helpers.GetPointer[gatewayv1.SectionName]("unsupported-name"), + }, + expectedErrors: 1, + }, + { + name: "Multiple unsupported fields", + specRule: gatewayv1.HTTPRouteRule{ + Name: helpers.GetPointer[gatewayv1.SectionName]("unsupported-name"), + Timeouts: helpers.GetPointer(gatewayv1.HTTPRouteTimeouts{ + Request: (*gatewayv1.Duration)(helpers.GetPointer("unsupported-timeouts")), + }), + Retry: helpers.GetPointer(gatewayv1.HTTPRouteRetry{Attempts: helpers.GetPointer(3)}), + SessionPersistence: helpers.GetPointer(gatewayv1.SessionPersistence{ + Type: helpers.GetPointer(gatewayv1.SessionPersistenceType("unsupported-session-persistence")), + }), + }, + expectedErrors: 4, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + rulePath := field.NewPath("spec").Child("rules") + var errors routeRuleErrors + + unsupportedFieldsErrors := checkForUnsupportedHTTPFields(test.specRule, rulePath) + if len(unsupportedFieldsErrors) > 0 { + errors.warn = append(errors.warn, unsupportedFieldsErrors...) + } + + g.Expect(errors.warn).To(HaveLen(test.expectedErrors)) + }) + } +} + +func TestProcessHTTPRouteRules_UnsupportedFields(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + specRules []gatewayv1.HTTPRouteRule + expectedConds []conditions.Condition + expectedWarns int + expectedValid bool + }{ + { + name: "No unsupported fields", + specRules: []gatewayv1.HTTPRouteRule{{}}, + expectedValid: true, + expectedConds: nil, + expectedWarns: 0, + }, + { + name: "One unsupported field", + specRules: []gatewayv1.HTTPRouteRule{ + { + Name: helpers.GetPointer[gatewayv1.SectionName]("unsupported-name"), + }, + }, + expectedValid: true, + expectedConds: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("spec.rules[0].name: Forbidden: Name"), + }, + expectedWarns: 1, + }, + { + name: "Multiple unsupported fields", + specRules: []gatewayv1.HTTPRouteRule{ + { + Name: helpers.GetPointer[gatewayv1.SectionName]("unsupported-name"), + Timeouts: helpers.GetPointer(gatewayv1.HTTPRouteTimeouts{ + Request: (*gatewayv1.Duration)(helpers.GetPointer("unsupported-timeouts")), + }), + Retry: helpers.GetPointer(gatewayv1.HTTPRouteRetry{Attempts: helpers.GetPointer(3)}), + SessionPersistence: helpers.GetPointer(gatewayv1.SessionPersistence{ + Type: helpers.GetPointer(gatewayv1.SessionPersistenceType("unsupported-session-persistence")), + }), + }, + }, + expectedValid: true, + expectedConds: []conditions.Condition{ + conditions.NewRouteAcceptedUnsupportedField("[spec.rules[0].name: Forbidden: Name, spec.rules[0].timeouts: " + + "Forbidden: Timeouts, spec.rules[0].retry: Forbidden: Retry, " + + "spec.rules[0].sessionPersistence: Forbidden: SessionPersistence]"), + }, + expectedWarns: 4, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + _, valid, conds := processHTTPRouteRules( + test.specRules, + validation.SkipValidator{}, + nil, + ) + + g.Expect(valid).To(Equal(test.expectedValid)) + if test.expectedConds == nil { + g.Expect(conds).To(BeEmpty()) + } else { + g.Expect(conds).To(HaveLen(len(test.expectedConds))) + for i, expectedCond := range test.expectedConds { + g.Expect(conds[i].Message).To(Equal(expectedCond.Message)) + } + } + }) + } +} diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go index c156ca738a..ba7e117c46 100644 --- a/internal/controller/state/graph/route_common.go +++ b/internal/controller/state/graph/route_common.go @@ -200,12 +200,14 @@ func CreateGatewayListenerKey(gwNSName types.NamespacedName, listenerName string type routeRuleErrors struct { invalid field.ErrorList resolve field.ErrorList + warn field.ErrorList } func (e routeRuleErrors) append(newErrors routeRuleErrors) routeRuleErrors { return routeRuleErrors{ invalid: append(e.invalid, newErrors.invalid...), resolve: append(e.resolve, newErrors.resolve...), + warn: append(e.warn, newErrors.warn...), } }