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

Drop all TCPRoute/UDPRoute-related code #4156

Closed
skriss opened this issue Nov 4, 2021 · 8 comments · Fixed by #4166
Closed

Drop all TCPRoute/UDPRoute-related code #4156

skriss opened this issue Nov 4, 2021 · 8 comments · Fixed by #4166
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@skriss
Copy link
Member

skriss commented Nov 4, 2021

It sounds like, given that Contour doesn't support TCPRoute/UDPRoute, we should not write status to them, not watch them, and not require/set up RBAC for them. Need to go through and delete all of the related code and RBAC.

Will require a corresponding operator PR as well.

@skriss skriss added kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. labels Nov 4, 2021
@stevesloka
Copy link
Member

So revert this: #4143 (comment)?

@youngnick
Copy link
Member

Yes, I spent some time rechecking the spec, and the idea is that if you don't support an object, you don't interact with it at all, which limits your RBAC exposure and makes your controller easier to write.

I think it will require some careful troubleshooting documentation (in Gateway API, not Contour) to make sure that people know that if you don't get status on a Route, you need to check the Gateway to see if your Route has been rejected.

@stevesloka
Copy link
Member

if you don't get status on a Route, you need to check the Gateway to see if your Route has been rejected.

But you'd never get status on the Gateway right? This would just look to the user like nothing is working.

@skriss
Copy link
Member Author

skriss commented Nov 4, 2021

I think the only indication you'd get is in the Gateway's listener status (https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.ListenerStatus) that supportedKinds does not include whatever kind you're trying to use.

@stevesloka
Copy link
Member

Gotcha that makes sense for the gateway status, but doesn't help with the route status.

@youngnick
Copy link
Member

Yes, it's an unfortunate downside of the fact that, although there's a core set of objects, the set of Routes is unbounded, so it's not possible for implementation authors to guarantee that all Routes that might reference a Gateway could be updated with a status. So, we (the Gateway API project) agreed that having a Condition in the Listener status was the best we could do.

That's what I meant by "There will need to be a troubleshooting guide", it's going to be important to teach everyone that when something isn't working with a Route, you check the Listener status first.

@stevesloka
Copy link
Member

The only problem I can see with a troubleshooting guide is that right now it feels like "status" is the place to go to figure out anything. Not that a troubleshooting guide is a bad idea, I think that guide is great, just not sure if that's enough.

Can wait and see though to figure out if it does become a problem.

@youngnick
Copy link
Member

youngnick commented Nov 4, 2021

I was also thinking about adding a default condition to the standard Route conditions block that puts Accepted to Unknown and has a message saying "A controller hasn't reconciled this, check the Gateway Listener status for more". (Obviously, upstream changes here).

skriss added a commit to skriss/contour that referenced this issue Nov 9, 2021
With this PR, Contour no longer watches
TCPRoutes or UDPRoutes, and does not set
an "Accepted: false" condition on them.
All RBAC related to them is dropped as well.

Closes projectcontour#4156.

Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss removed the lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. label Nov 9, 2021
@skriss skriss added this to the 1.20.0 milestone Nov 9, 2021
skriss added a commit to skriss/contour that referenced this issue Nov 9, 2021
With this PR, Contour no longer watches
TCPRoutes or UDPRoutes, and does not set
an "Accepted: false" condition on them.
All RBAC related to them is dropped as well.

Closes projectcontour#4156.

Signed-off-by: Steve Kriss <krisss@vmware.com>
stevesloka pushed a commit that referenced this issue Nov 10, 2021
With this PR, Contour no longer watches
TCPRoutes or UDPRoutes, and does not set
an "Accepted: false" condition on them.
All RBAC related to them is dropped as well.

Closes #4156.

Signed-off-by: Steve Kriss <krisss@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants