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

Align gateway class supported features with upstream changes #4059

Open
levikobi opened this issue Aug 15, 2024 · 5 comments
Open

Align gateway class supported features with upstream changes #4059

levikobi opened this issue Aug 15, 2024 · 5 comments
Assignees
Labels

Comments

@levikobi
Copy link
Contributor

Description:

Describe the desired behavior, what scenario it enables and how it
would be used.

kubernetes-sigs/gateway-api#3200 introduced a breaking change to SupportedFeatures experimental API. We need to align the controller with the new API structure.

[optional Relevant Links:]

Any extra documentation required to understand the issue.

@levikobi
Copy link
Contributor Author

In #3888 the feature was turned off. I will not turn it on (unless @arkodg wants me to), but I still think it's worth updating the controller with this change.

/assign

@arkodg
Copy link
Contributor

arkodg commented Aug 15, 2024

my vote is to turn it on when the field becomes stable (else it introduces a lot of breaking changes) or when we have many users asking for this

@levikobi
Copy link
Contributor Author

levikobi commented Sep 2, 2024

@arkodg I aligned how we update supported features in the gateway class status. To do so, I used the following version of gateway-api: v1.1.1-0.20240819173812-8f5f9d1f515b.

While testing it out, I saw another breaking change (as the code could no longer compile):

internal/gatewayapi/route.go:610:34: cannot use headerMatch.Type (variable of type *"sigs.k8s.io/gateway-api/apis/v1".GRPCHeaderMatchType) as *"sigs.k8s.io/gateway-api/apis/v1".HeaderMatchType value in argument to HeaderMatchTypeDerefOr
make[1]: *** [tools/make/golang.mk:31: go.build.linux_amd64.envoy-gateway] Error 1

Is this known and addressed in another issue?

If not, I think we have two options:

  1. I will continue with the original intent of this issue and create a PR that only contains the fix for it. This PR will probably be cherry-picked into a different PR that will update the gateway-api version with all the breaking changes fixes.
  2. I can fix the new error and open one PR, which will update the gateway-api version and contain the fixes for the breaking changes.

I think the 2nd way is the way to go, but please let me know what you prefer.

@arkodg
Copy link
Contributor

arkodg commented Sep 5, 2024

hey @levikobi, we discussed this in the community meeting this week and decided that we should put this on hold until the feature is stable since we feel that this feature doesnt add any extra value yet

@levikobi
Copy link
Contributor Author

levikobi commented Sep 6, 2024

No problem
Thanks for the update @arkodg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants