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.

Sign up for GitHub

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

Jump to bottom

GEP: introduce GEP 820 #822

Merged
merged 1 commit into from
Aug 26, 2021
Merged

GEP: introduce GEP 820 #822

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 62 additions & 0 deletions site-src/geps/gep-820.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# GEP-820: Drop extension points from Route matches

* Issue: [#820](https://github.com/kubernetes-sigs/gateway-api/issues/820)
* Status: Implementable

## TLDR

Drop extension points within Route match block. These extension points are
not well understood.

## Goals

- Drop the extension points within Route match block.

## Non-Goals

- Figure out a replacement solution for the use-case that these extension
points addressed

## Introduction

As the API moves towards `v1alpha2`, the maintainers intend to make the API
stable and forward compatible for the foreseeable future. To that end,
maintainers intend to minimize (eliminate if possible) breaking changes post
`v1alpha2`. This GEP is part of that initiative.

Extension points for match criteria in Routes were added to enable use-cases
where match criteria defined by implementation was a super-set of match
criteria defined within this API. To the best of our knowledge, even though
extension points were added, no concrete examples or use-cases were known at
that time and none have been discovered so far.

This proposal advocates removal of these extension points because:

- It goes against the unwritten design principles this API has followed so far:

- minimize number of API types as much as possible
- minimize strongly coupled API types and instead shoot for self-contained
types.
- extension points are introduced with clear use-cases and possibilities in
mind. Vague extension points are avoided as they become harder to maintain.
- It is unlikely that the user experience resulting from defining two k8s
resources for defining a Route will be optimal.
- There is not prior art on splitting match criteria and backend forwarding
semantics (spec.backends) in the community. We believe they are kept together
for good reasons.

## API

The following fields and all associated documentation will be removed:

- HTTPRouteMatch.ExtensionRef
- TCPRouteMatch.ExtensionRef will be removed. This results in a struct without
any members: TCPRouteMatch. The struct will be kept as it is expected that
more match criterias might be added to L4 routes.

- Do the same to UDPRoute and TLSRoute

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still give a patch to support more match types -- as they are going to come up.

What about this:

Modify HTTPPathMatch.Type:

  • Allow qualified names (e.g. acme.io/my-custom-match). These represent custom match types
  • The standard match types (Exact, Prefix, RegularExpression) stay
  • Remove ImplementationSpecific

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robscott

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few related conversations going on here, but it's helpful to reference Dan's earlier comment in the sig-net review that showed the limitations of a single ImplementationSpecific match type: #780 (comment).

Although not completely in scope for the original purpose of this GEP, I think there's enough overlap to discuss it here. I think @bowei's suggestion to bundle this removal of the ExtensionRef with the ability to provide custom match types would help here. Although we'd be removing one way to extend path matching, we'd be adding other more targeted/clearly scoped extension points. It would also match the pattern we've already chosen to take with Protocol on Gateway listeners (a core set of supported values with clear meaning + a way to use your own custom types when necessary).

What do you think @hbagdi @jpeach @youngnick?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is something we want to move forward with, should we take the same approach for header and query param matching that also include ImplementationSpecific match types? I don't think we had a clear purpose for the match types in those cases other than to be consistent with path match types, where we were really trying to be consistent with Ingress...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to just remove it in this iteration. Specifying a match as a separate object is awkward and verbose, and I haven't heard anyone that plans to implement this.

I also don't think that "ImplementationSpecific" is workable in general, because it doesn't actually let you say what kind of match the implementation wants to make.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're all in agreement on removing it. I hate to scope creep, but it does seem like it could make sense to combine the removal of this extension mechanism with loosened validation on match types as an alternative extension point.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to just remove it in this iteration

@jpeach To clarify, were you referring to "extensionRef" or "ImplementationSpecific" match types?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to just remove it in this iteration

@jpeach To clarify, were you referring to "extensionRef" or "ImplementationSpecific" match types?

The former, per the proposal in this GEP. I was just saying that I don't really think we should also try to replace it in this GEP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the idea of adding qualified path match types is fine, but I don't think that we should push to get it into v1alpha2. It's additive anyway.

I'd support just dumping ImplementationSpecific for now, and ensuring that the thing we're designing actually does what we want.

## Alternatives

N/A