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

fix: use GW API v1 in webhook config only when KIC >= 3.0.0 #954

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

czeslavo
Copy link
Contributor

What this PR does / why we need it:

Because of this configuration, KIC 2.12's admission webhook receives requests to validate v1 resources which it doesn't know about:

msg="failed to run validation" error="unknown resource type to validate: gateway.networking.k8s.io/v1 gateways"
msg="failed to run validation" error="unknown resource type to validate: gateway.networking.k8s.io/v1 httproutes

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the main branch.
  • Changes are documented under the "Unreleased" header in CHANGELOG.md
  • New or modified sections of values.yaml are documented in the README.md
  • Commits follow the Kong commit message guidelines

@czeslavo czeslavo force-pushed the fix/admission-webhook-config-kic-3.0 branch from 9124248 to 67ad667 Compare November 22, 2023 17:00
@czeslavo czeslavo self-assigned this Nov 22, 2023
@czeslavo czeslavo marked this pull request as ready for review November 22, 2023 17:12
@czeslavo czeslavo requested a review from a team as a code owner November 22, 2023 17:12
@rainest
Copy link
Contributor

rainest commented Nov 22, 2023

Could we remove the GWAPI configuration for our webhook altogether? Between @pmalek's findings that our validations were mostly (all?) checks we added early on but probably shouldn't have, the expectation that most users will be on v1 (with built-in CEL validation), and the availability of the upstream GWAPI webhook for older versions, I think we can probably remove ours.

https://github.com/Kong/kubernetes-ingress-controller/blob/87196d183f8e0093899de32dc6ab97be852bb471/internal/admission/validator.go#L367-L444 is still in KIC main but I thought the plan was to remove all of it.

@czeslavo
Copy link
Contributor Author

To be fair I do not have enough context on the removal of validation in KIC. If someone has investigated it and we have an agreement our checks are redundant I'd be more than happy to remove them from here as well as from KIC.

@rainest
Copy link
Contributor

rainest commented Nov 27, 2023

As best I can tell from Kong/kubernetes-ingress-controller#5197 and earlier chat discussion that is the case.

Pinged others in chat, though unfortunately everyone else involved is out today. If Mattia can confirm, we can go with #956 to remove them entirely.

@rainest
Copy link
Contributor

rainest commented Nov 28, 2023

We're apparently unfortunately stuck with it until the feature support stuff is in place.

@rainest rainest merged commit 16f5a1d into main Nov 28, 2023
22 checks passed
@rainest rainest deleted the fix/admission-webhook-config-kic-3.0 branch November 28, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants