Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate all HTTPRoute features #5253

Closed
1 of 2 tasks
mlavacca opened this issue Nov 29, 2023 · 2 comments · Fixed by #5312
Closed
1 of 2 tasks

Validate all HTTPRoute features #5253

mlavacca opened this issue Nov 29, 2023 · 2 comments · Fixed by #5312
Assignees
Labels
area/admission area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API
Milestone

Comments

@mlavacca
Copy link
Member

mlavacca commented Nov 29, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

The current admission logic in the validatingWebhook for HTTPRoutes checks only a subset of the conditions that make an HTTPRoute invalid for our implementation:

  • QueryParamMatching if expression router is disabled
  • ParentRef different from core/Service

We need to check ALL the below conditions that make a HTTPRoute invalid for our implementation:

  • requestMirror rule filter
  • urlRewrite rule filter
  • HTTPBackendRef.Filters should be empty
  • rules Timeouts
  • ParentRef should always be gateway.networking.k8s.io/Gateway or /Gateway

Proposed Solution

No response

Additional information

No response

Acceptance Criteria

  • all the conditions specified above are checked in the validation webhook
@randmonkey
Copy link
Contributor

I found such comments in the spec of v1.Listener:

	// All valid rules within a Route attached to this Listener should be
	// implemented. Invalid Route rules can be ignored (sometimes that will mean
	// the full Route). If a Route rule transitions from valid to invalid,
	// support for that Route rule should be dropped to ensure consistency. For
	// example, even if a filter specified by a Route rule is invalid, the rest
	// of the rules within that Route should still be supported.

According to the comments, if a rule is invalid while there are other valid routes, should we reject this HTTPRoute?

@mlavacca
Copy link
Member Author

I found such comments in the spec of v1.Listener:

	// All valid rules within a Route attached to this Listener should be
	// implemented. Invalid Route rules can be ignored (sometimes that will mean
	// the full Route). If a Route rule transitions from valid to invalid,
	// support for that Route rule should be dropped to ensure consistency. For
	// example, even if a filter specified by a Route rule is invalid, the rest
	// of the rules within that Route should still be supported.

According to the comments, if a rule is invalid while there are other valid routes, should we reject this HTTPRoute?

A route can be invalid for many reasons, and usage of unsupported features is only one of them. In that case, there is no condition that can be used to mark the route as invalid.
Since there is a lack of status condition upstream to mark a route as rejected because of unsupported features (this discussion is about it), I think that (at least for the time being) what we should do is rejecting a route in such a scenario via the validating webhook. Hence, I think that even if a route has a single rule declaring an unsupported feature, we should reject it.

@pmalek pmalek added the area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admission area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants