Added GEP: provide a name field to http route

[hzxuzhonghu]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #986

Does this PR introduce a user-facing change?:

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hzxuzhonghu
To complete the pull request process, please assign dcbw after the PR has been reviewed.
You can assign the PR to them by writing /assign @dcbw in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[robscott]

Thanks for starting this GEP @hzxuzhonghu! I'm a bit hesitant to move forward with this new field, but it is very helpful to see the rationale here. Interested in what others think.

[robscott]

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. It would be helpful to clarify that here + link to what that change would look like.

[robscott]

I think some more details would be helpful here, maybe something like this?

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. 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.

[robscott]

I think we can leave this out. We do want to at least define support levels associated with any new field we propose. My guess is that this would start with Support: Core, but open to what others think.

[robscott]

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.

[hzxuzhonghu]

For istio, it will be used to generate route name, so that user can use envoyfilter to patch the generated one.

[hzxuzhonghu]

The two levels of hierarchy name can be used to prevent duplicate names.

[hbagdi]

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?

[hzxuzhonghu]

We don't want the hack, because it will be used by users

[robscott]

I think there's a few alternatives here:

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.

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, meaning this GEP would not be useful for that approach.

3. Logging could assign numerical indexes to Route rules.
   Instead of rules[sectionName] logs would include rules[0] or similar.

[hbagdi]

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.

Although more flexible, this feels clunky and error-prone to me.
Wouldn't the API be easier to grok if we recommend users to keep their HTTPRoute resources smaller and then ask them to target policies only at the resource level?

The more granular the references, the more careful one has to be at different levels.
I'm not convinced this is a good motivation. We should wait and hear more experience reports before trying to satisfy this use case.

[hzxuzhonghu]

Although more flexible, this feels clunky and error-prone to me. Wouldn't the API be easier to grok if we recommend users to keep their HTTPRoute resources smaller and then ask them to target policies only at the resource level?

It is always trade-off to keep simple or provide more features. TBH, I donot think there is much diff from Gateway + HTTPRoute

This is an optional field, it is for experienced users. And also it is just like the semantics widely used in the repo(reference other objects with high flexibility)

[hzxuzhonghu]

ping @hbagdi @robscott Is it ready to go?

[youngnick]

Like @robscott and @hbagdi, I'm worried about how this will interact with sectionName and if it will end up being more confusing. In general, I've in favor of having listMapType instead of plain lists of objects, because having a name is clearer. But the Route object is an extremely important part of the API.

Ironically, this would actually be a little easier if we transitioned each of these lists to listMapTypes, and made the name field required. Here, I'm thinking:

• How do you refer to a routerule or a match if not all of them have a name?
• Do controller authors need to do indexing unless there's names, then have the "name" for structs that don't have a name field set be the index?

That would make the name required in practice, if optional on input.

Maybe some examples would help? How would something refer to this name if other names were set. How do we refer to a specific named match when the RouteRule also has a name? If the field is optional, how do you handle the some-have-a-name case?

As I said above, having a name field for disambiguation of HTTPRoutes with multiple RouteRules does make sense to me, although I also agree that we should encourage people to have more, smaller HTTPRoutes rather than large ones. Aside from the Istio use case given, the effect on logging and metrics shouldn't be denied - but if those are to be useful, the name would probably need to be either required or defaulted somehow.

[hzxuzhonghu]

@youngnick This is a good suggestion, but make it required will break the compatability

[youngnick]

Yes, I know it will break compatibility, but I don't think that this is really viable unless we do. I think our options are either:

• don't do this at all
• break compatibility, issue a v1alpha3 and do this right.

Without having this required, I think it will create more implementation problems than it solves.

[robscott]

I agree that a route rule name that is similar to gateway listener name could get confusing if it were not subject to the same unique validation and constraints. We could potentially leave it as optional but introduce webhook validation that enforced uniqueness, but @youngnick's questions have me second guessing that idea:

How do you refer to a routerule or a match if not all of them have a name?
Do controller authors need to do indexing unless there's names, then have the "name" for structs that don't have a name field set be the index?

These are great questions, and make me question even an optional name field here. I can only imagine the confusion if someone chose to name a Route rule "1" and left their other Route rule at index 1 unnamed...

Despite previously proposing something very similar to what this GEP proposes, I'm not entirely sold on this concept, especially if it would require a v1alpha3. My current preference would be to leave this out of the API for the following reasons:

1. If we require a Route Rule name, that's significantly more costly to UX than requiring a Gateway Listener name. I'd expect that in most cases, there will be at least 5-10 Route Rules per Gateway Listener.
2. If users need to target specific sections of Routes, they can just split those sections into different Routes.
3. For the sake of logging and debugging, it's likely sufficient to use an index to reference the specific rule (rules[1]).

[youngnick]

We discussed this in the community meeting on Monday 31-Jan-2022:

• at its heart, this proposal will add an indication in the API if people should prefer more, smaller HTTPRoute objects or less, larger ones with more Routes (since having a name and mechanisms to address it makes it far more viable to have large routes).
• there's been an implicit assumption in the design up until this point that more, smaller HTTPRoute objects were preferable, and having the Routes be specified in a list is basically syntactic sugar intended to support simple use cases. Implementing name for Route rules kind of flips that around.

I've committed to checking in with api-machinery about if there's any preference for more, smaller objects vs larger ones, so I will do so and follow up here.

[robscott]

Thanks for planning to follow up with apimachinery @youngnick! Very interested to hear their perspective. I agree that we've generally been pushing for more small routes instead of fewer large routes. I'm concerned that introducing a rule name here could lead to less than ideal configuration.

On the other hand, it sounds like a rule name will also be part of a proposal for Route delegation. So that would add to the potential use cases for this (policy attachment, logging/debugging). Of course, all of those are possible with smaller Routes, but that could also become unwieldy at a certain scale.

If we were to add a name field to Route rule, maybe it could be optional, with validation ensuring that non-empty values were unique within the Route. That would provide a level of consistency with listener.name while still keeping the API simple for the majority of users that likely would not use this or other complex features. It would also be a backwards compatible change, preventing the need for a v1alpha3 release.

[hzxuzhonghu]

smaller HTTPRoute is just a choice for end-users, but for infra developers, they still face the challenge to merge them, how to merge regex matches? It seems very hard.

An optional field is to provide a flexible extensibility way for both users and infrastructure developers

How do you refer to a routerule or a match if not all of them have a name?
Do controller authors need to do indexing unless there's names, then have the "name" for structs that don't have a name field set be the index?

I have to admit, it is a little confusing. how about declaring the default value, such as rules[index]

[howardjohn]

I have to admit, it is a little confusing. how about declaring the default value, such as rules[index]

FWIW this is how Istio works. Name is not required

[robscott]

Hey @hzxuzhonghu, we discussed this PR again at today's community meeting. We want to make some progress here, but think it will be helpful to have a clearer picture of how route delegation will work first, since it's likely very closely related to this. @Jeff-Apple is working on a proposal for this, and we're planning on discussing all of this at the next community meeting (Feb 28).

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[johnlanni]

Hey @hzxuzhonghu, we discussed this PR again at today's community meeting. We want to make some progress here, but think it will be helpful to have a clearer picture of how route delegation will work first, since it's likely very closely related to this. @Jeff-Apple is working on a proposal for this, and we're planning on discussing all of this at the next community meeting (Feb 28).

Is there any progress on this recently?

[robscott]

Hey @johnlanni, I think @mikemorris was going to try to pick this up, but I'm not sure what the timeline is.