Skip to content

Commit f9dab03

Browse files
committed
Add PartiallyInvalid HTTPRoute Condition (nginx#1160)
Replace previously used TODO Conditions with PartiallyInvalid HTTPRoute Condition. Problem: There was no HTTPRoute Condition that conveyed when a Route was PartiallyInvalid. Solution: Added the PartiallyInvalid HTTPRoute Condition.
1 parent a31edc8 commit f9dab03

File tree

4 files changed

+102
-12
lines changed

4 files changed

+102
-12
lines changed

docs/gateway-api-compatibility.md

+1
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ Fields:
174174
- `ResolvedRefs/False/BackendNotFound`
175175
- `ResolvedRefs/False/UnsupportedValue` - custom reason for when one of the HTTPRoute rules has a backendRef
176176
with an unsupported value.
177+
- `PartiallyInvalid/True/UnsupportedValue`
177178

178179
### ReferenceGrant
179180

internal/mode/static/state/conditions/conditions.go

+21
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ const (
5757
RouteMessageFailedNginxReload = GatewayMessageFailedNginxReload + ". NGINX may still be configured " +
5858
"for this HTTPRoute. However, future updates to this resource will not be configured until the Gateway " +
5959
"is programmed again"
60+
// RouteConditionPartiallyInvalid is a condition which indicates that the Route contains
61+
// a combination of both valid and invalid rules.
62+
//
63+
// FIXME(bjee19): Update to Gateway sig v1 version when released.
64+
// https://github.com/nginxinc/nginx-gateway-fabric/issues/1168
65+
RouteConditionPartiallyInvalid v1beta1.RouteConditionType = "PartiallyInvalid"
6066
)
6167

6268
// DeduplicateConditions removes duplicate conditions based on the condition type.
@@ -151,6 +157,21 @@ func NewRouteUnsupportedValue(msg string) conditions.Condition {
151157
}
152158
}
153159

160+
// NewRoutePartiallyInvalid returns a Condition that indicates that the HTTPRoute contains a combination
161+
// of both valid and invalid rules.
162+
//
163+
// // nolint:lll
164+
// The message must start with "Dropped Rules(s)" according to the Gateway API spec
165+
// See https://github.com/kubernetes-sigs/gateway-api/blob/37d81593e5a965ed76582dbc1a2f56bbd57c0622/apis/v1/shared_types.go#L408-L413
166+
func NewRoutePartiallyInvalid(msg string) conditions.Condition {
167+
return conditions.Condition{
168+
Type: string(RouteConditionPartiallyInvalid),
169+
Status: metav1.ConditionTrue,
170+
Reason: string(v1beta1.RouteReasonUnsupportedValue),
171+
Message: "Dropped Rule(s): " + msg,
172+
}
173+
}
174+
154175
// NewRouteInvalidListener returns a Condition that indicates that the HTTPRoute is not accepted because of an
155176
// invalid listener.
156177
func NewRouteInvalidListener() conditions.Condition {

internal/mode/static/state/graph/httproute.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,7 @@ func buildRoute(
233233
msg := allRulesErrs.ToAggregate().Error()
234234

235235
if atLeastOneValid {
236-
// FIXME(pleshakov): Partial validity for HTTPRoute rules is not defined in the Gateway API spec yet.
237-
// See https://github.com/nginxinc/nginx-gateway-fabric/issues/485
238-
msg = "Some rules are invalid: " + msg
239-
r.Conditions = append(r.Conditions, staticConds.NewTODO(msg))
236+
r.Conditions = append(r.Conditions, staticConds.NewRoutePartiallyInvalid(msg))
240237
} else {
241238
msg = "All rules are invalid: " + msg
242239
r.Conditions = append(r.Conditions, staticConds.NewRouteUnsupportedValue(msg))

internal/mode/static/state/graph/httproute_test.go

+79-8
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,18 @@ func TestBuildRoute(t *testing.T) {
354354
hrInvalidFilters := createHTTPRoute("hr", gatewayNsName.Name, "example.com", "/filter")
355355
addFilterToPath(hrInvalidFilters, "/filter", invalidFilter)
356356

357-
hrInvalidValidRules := createHTTPRoute("hr", gatewayNsName.Name, "example.com", invalidPath, "/filter", "/")
358-
addFilterToPath(hrInvalidValidRules, "/filter", invalidFilter)
357+
hrDroppedInvalidMatches := createHTTPRoute("hr", gatewayNsName.Name, "example.com", invalidPath, "/")
358+
359+
hrDroppedInvalidMatchesAndInvalidFilters := createHTTPRoute(
360+
"hr",
361+
gatewayNsName.Name,
362+
"example.com",
363+
invalidPath, "/filter", "/")
364+
addFilterToPath(hrDroppedInvalidMatchesAndInvalidFilters, "/filter", invalidFilter)
365+
366+
hrDroppedInvalidFilters := createHTTPRoute("hr", gatewayNsName.Name, "example.com", "/filter", "/")
367+
addFilterToPath(hrDroppedInvalidFilters, "/filter", validFilter)
368+
addFilterToPath(hrDroppedInvalidFilters, "/", invalidFilter)
359369

360370
validatorInvalidFieldsInRule := &validationfakes.FakeHTTPFieldsValidator{
361371
ValidatePathInMatchStub: func(path string) error {
@@ -484,9 +494,9 @@ func TestBuildRoute(t *testing.T) {
484494
},
485495
{
486496
validator: validatorInvalidFieldsInRule,
487-
hr: hrInvalidValidRules,
497+
hr: hrDroppedInvalidMatches,
488498
expected: &Route{
489-
Source: hrInvalidValidRules,
499+
Source: hrDroppedInvalidMatches,
490500
Valid: true,
491501
ParentRefs: []ParentRef{
492502
{
@@ -495,9 +505,39 @@ func TestBuildRoute(t *testing.T) {
495505
},
496506
},
497507
Conditions: []conditions.Condition{
498-
staticConds.NewTODO(
499-
`Some rules are invalid: ` +
500-
`[spec.rules[0].matches[0].path.value: Invalid value: "/invalid": invalid path, ` +
508+
staticConds.NewRoutePartiallyInvalid(
509+
`spec.rules[0].matches[0].path.value: Invalid value: "/invalid": invalid path`,
510+
),
511+
},
512+
Rules: []Rule{
513+
{
514+
ValidMatches: false,
515+
ValidFilters: true,
516+
},
517+
{
518+
ValidMatches: true,
519+
ValidFilters: true,
520+
},
521+
},
522+
},
523+
name: "dropped invalid rule with invalid matches",
524+
},
525+
526+
{
527+
validator: validatorInvalidFieldsInRule,
528+
hr: hrDroppedInvalidMatchesAndInvalidFilters,
529+
expected: &Route{
530+
Source: hrDroppedInvalidMatchesAndInvalidFilters,
531+
Valid: true,
532+
ParentRefs: []ParentRef{
533+
{
534+
Idx: 0,
535+
Gateway: gatewayNsName,
536+
},
537+
},
538+
Conditions: []conditions.Condition{
539+
staticConds.NewRoutePartiallyInvalid(
540+
`[spec.rules[0].matches[0].path.value: Invalid value: "/invalid": invalid path, ` +
501541
`spec.rules[1].filters[0].requestRedirect.hostname: Invalid value: ` +
502542
`"invalid.example.com": invalid hostname]`,
503543
),
@@ -517,7 +557,38 @@ func TestBuildRoute(t *testing.T) {
517557
},
518558
},
519559
},
520-
name: "invalid with invalid and valid rules",
560+
name: "dropped invalid rule with invalid filters and invalid rule with invalid matches",
561+
},
562+
{
563+
validator: validatorInvalidFieldsInRule,
564+
hr: hrDroppedInvalidFilters,
565+
expected: &Route{
566+
Source: hrDroppedInvalidFilters,
567+
Valid: true,
568+
ParentRefs: []ParentRef{
569+
{
570+
Idx: 0,
571+
Gateway: gatewayNsName,
572+
},
573+
},
574+
Conditions: []conditions.Condition{
575+
staticConds.NewRoutePartiallyInvalid(
576+
`spec.rules[1].filters[0].requestRedirect.hostname: Invalid value: ` +
577+
`"invalid.example.com": invalid hostname`,
578+
),
579+
},
580+
Rules: []Rule{
581+
{
582+
ValidMatches: true,
583+
ValidFilters: true,
584+
},
585+
{
586+
ValidMatches: true,
587+
ValidFilters: false,
588+
},
589+
},
590+
},
591+
name: "dropped invalid rule with invalid filters",
521592
},
522593
}
523594

0 commit comments

Comments
 (0)