-
Notifications
You must be signed in to change notification settings - Fork 275
[release-0.9] 🌱 Move webhooks into pkg/webhooks #2612
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
base: release-0.9
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fwiw, this backport should allow us to address an issue that is currently blocking work on the migration path from MAPO to CAPO in OpenShift in OpenShift. While I appreciate this is primarily for downstream, I'm proposing it upstream as I hope it should introduce minimal risk while allowing us/me to stay as close to upstream as possible (always my preferred approach, personally). I'm happy to discuss why we need this (tl;dr: MAPO is stuck with CAPO v0.9.x for $reasons) and build a case for doing this here, as needed, though if we really don't want to do this here I'd understand. |
(marking as ready-for-review with a hold to get CI results) |
anything that helps an active downstream get closer to upstream is something I am in favour of :) |
bb45ad9
to
6e5f0d9
Compare
Moves webhooks from api to pkg/webhooks making only mechanical code changes except for the removal of the defaulting webhooks, because they weren't used. This results in there now being no mutating webhook configured. NOTE(stephenfin): There were a lot of conflicts here. These were mostly mitigated by faking the addition of v1alpha8, which moved the webhooks to the 'api/v1alpha8' package (commit 750b84d), followed by the subsequent rename of this package to v1beta1 (commit e9fb53c), for the webhook files and tests. This still resulted in some merge conflicts due the v1alpha8 changes such as 564b6bd and 4368c4f (which we obviously don't want to include here) but it made the backport much simpler. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> (cherry picked from commit 750b84d)
@stephenfin: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
I looks like the problems with the CI are due to a mismatch of versions between go and golangci-lint. By the look of https://storage.googleapis.com/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-a[…]-provider-openstack-test/1938281313530286080/prowjob.json it seems the job uses gcr.io/k8s-staging-test-infra/kubekins-e2e:v20250613-876fb90a97-master with go 1.24. But golangci-lint is at version v1.54.2 which is not compatible with go 1.24. I wonder if bumping it to 1.64 (first version to support go 1.24) helps. |
I tried bumping golangci-lint with #2619 and it's now effectively showing different errors, with |
This is a manual backport of #1920.
/hold