generated from kubernetes/kubernetes-template-project
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Added GEP: provide a name field to http route #996
Closed
Closed
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
fb0f069
Added GEP: provide a name field to http route
hzxuzhonghu 29e5ef2
Update site-src/geps/gep-995.md
hzxuzhonghu 6d064db
Update site-src/geps/gep-995.md
hzxuzhonghu 1848bb4
Update site-src/geps/gep-995.md
hzxuzhonghu 8adebe0
Update site-src/geps/gep-995.md
hzxuzhonghu d67e045
Update site-src/geps/gep-995.md
hzxuzhonghu 295b10c
Address comments
hzxuzhonghu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
# GEP-995: HTTP Route Naming | ||
|
||
* Issue: [#995](https://github.com/kubernetes-sigs/gateway-api/issues/995) | ||
* Status: Implementable | ||
|
||
## TLDR | ||
|
||
* Add `Name` field to `Rules` field in HTTPRoute. | ||
* Add `Name` field to `Matches` field in HTTPRouteRule. | ||
|
||
## Goals | ||
|
||
Provide a path for implementations to support: | ||
|
||
* Policy Attachment: | ||
Attach policies to Route rules. For this to be useful, we'd also need to add SectionName to PolicyTargetReference | ||
as described here: https://gateway-api.sigs.k8s.io/geps/gep-713/#apply-policies-to-sections-of-a-resource-future-extension. | ||
* Route Attachment/Delegation: | ||
Ability to have more fine-grained matches for policy references if we add sectionName to the policy object. | ||
Although Route Attachment/Delegation is not part of the API yet, it is likely to be added in the future. | ||
This concept is tracked by [#634](https://github.com/kubernetes-sigs/gateway-api/issues/634). | ||
One of the solutions that has been proposed for this involves using ParentRefs on a Route to refer to a parent Route. | ||
If we were to do that, it may be useful to attach to a specific Route rule instead of the entire Route. | ||
Adding name to Route rule could enable that. | ||
* Improved Debugging: | ||
Ability to add debugging information, such as adding the specific matching route rule name to an access log. | ||
|
||
## Introduction | ||
|
||
As described above, there are a number of potential use cases for providing an identity to a route rule. | ||
The most straightforward reason for this is to allow extending a particular route rule. | ||
This change will enable implementations to support the above use cases while still providing a portable core. | ||
|
||
## API | ||
|
||
A `Name` field would be added to `HTTPRouteMatch`: | ||
|
||
```go | ||
type HTTPRouteRule struct { | ||
// The name assigned to the route for debugging purposes. The | ||
// route's name will be concatenated with the match's name and may | ||
// be logged in the access logs for requests matching this | ||
// route/match depends on the implementor. | ||
// | ||
// +optional | ||
Name *string `json:"name,omitempty"` | ||
... | ||
} | ||
``` | ||
|
||
|
||
A `Name` field would be added to `HTTPRouteRule `: | ||
|
||
```go | ||
type HTTPRouteMatch struct { | ||
// Name specifies the HTTP route match name. The match's name will be | ||
// concatenated with the parent route's name and may be logged in | ||
// the access logs for requests matching this route depends on the implementor. | ||
// | ||
// +optional | ||
Name *string `json:"name,omitempty"` | ||
... | ||
} | ||
``` | ||
## Alternatives | ||
|
||
1. Users can split out Routes into different resources when they need to be referenced separately. | ||
Route rules are really more of a convenience than a necessity. Instead of having 10 rules in 1 route, the same config could be represented by 10 routes with 1 rule each. | ||
This does not apply to istio, which will generate a route per Rule here, and they are all combined into one single RDS, so it is not easy for users to patch a specific route. | ||
|
||
2. Route attachment/delegation may choose to use a different approach. For example, each Route could choose to include a set of child routes by direct reference. | ||
|
||
3. Logging could assign numerical indexes to Route rules. Instead of rules[sectionName] logs would include rules[0] or similar. | ||
However, this is not suitable for implementors that merge the rules before generating underlying configs. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a clear use case for this? Unfortunately it would result in two levels of hierarchy which could get confusing when using
sectionName
in references.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For istio, it will be used to generate route name, so that user can use envoyfilter to patch the generated one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two levels of hierarchy name can be used to prevent duplicate names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you generate a name at this level using the index or a hash? Would that amount to enough determinism for the code and the human to get to the right place when the need arises?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want the hack, because it will be used by users