Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions internal/controller/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -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{
Expand Down
17 changes: 17 additions & 0 deletions internal/controller/state/graph/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down Expand Up @@ -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
}
161 changes: 157 additions & 4 deletions internal/controller/state/graph/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -304,6 +306,8 @@ func TestBuildGateway(t *testing.T) {
GatewayClassName: gcName,
Listeners: cfg.listeners,
Addresses: cfg.addresses,
AllowedListeners: cfg.allowedListeners,
BackendTLS: cfg.backendTLS,
},
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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))
})
}
}
33 changes: 33 additions & 0 deletions internal/controller/state/graph/grpcroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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
}
Loading
Loading