⚠ Remove deprecated Defaulter and Validator interfaces #2877

troy0820 · 2024-07-08T16:07:33Z

We deprecated the admission.Validator and the admission.Defaulter interfaces and these are said to be removed in ~~release-0.19.~~ release-0.20

Closes #2641

Remove deprecated interfaces
~~Remove tests for those types~~ Change tests for the deprecated types to the CustomDefaulter/CustomValidator
~~Change~~ Remove log message that point to the now removed interfaces for the builder.
Moved type Warnings to validator_custom.go

sbueringer · 2024-07-09T07:49:11Z

@troy0820 can you remind me where we said we're going to remove them with 0.19?

IIRC it was on some issue, can you please link it in the PR description?

That being said, not sure when we should do it. I think this will require a lot of folks to make changes, maybe we should give them more time. I don't think we have anything pressing that requires us to drop them very soon.

@vincepri @alvaroaleman wdyt?

troy0820 · 2024-07-09T10:11:20Z

can you remind me where we said we're going to remove them with 0.19?
IIRC it was on some issue, can you please link it in the PR description?

Updated the PR description with the issue.

That being said, not sure when we should do it. I think this will require a lot of folks to make changes, maybe we should give them more time. I don't think we have anything pressing that requires us to drop them very soon.

I’m not in any rush to remove them but it was put against the v0.19 milestone. We can wait if the changes need to be made other places as well as give people more time.

k8s-ci-robot · 2024-07-17T08:49:46Z

@grzesuav: changing LGTM is restricted to collaborators

In response to this:

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.

Signed-off-by: Troy Connor <[email protected]>

sbueringer · 2024-08-08T05:40:20Z

@troy0820 Thx for working on this! I talked to @alvaroaleman @vincepri and we would merge it right after the 0.19.0 release so it would be part of 0.20.0. I'll review the PR soon

sbueringer

Looks good overall. Mostly just wanting to make sure that for whatever coverage we drop we'll have the corresponding coverage with the new types

pkg/builder/webhook.go

pkg/builder/webhook_test.go

sbueringer · 2024-08-08T13:15:08Z

pkg/webhook/admission/defaulter_test.go

-
-var _ = Describe("Defaulter Handler", func() {
-
-	It("should return ok if received delete verb in defaulter handler", func() {


Please check if we have coverage like this for the customdefaulter (somewhere, e.g. might better fit in webhook_test.go, or we already have it there)

(same for validator_test.go)

Do you want this to run through the webhook_test.go in the admissions package or create this in the pkg/builder/webhook_test.go which can test the whole flow? @sbueringer

edited:
We currently do do not have any coverage for custom validator/defaulter in this package.

We currently do have any coverage for custom validator/defaulter in this package.

We currently do not have any coverage for custom validator/defaulter in this package.

Right?

The more I'm thinking about it the more I think we should have these tests in defaulter_custom_test.go to actually unit test this code (same for validator).

(to be clear I would just move the existing tests in defaulter_test.go / validator_test.go over to the corresponding defaulter_custom_test.go / .. files. Let's keep webhook_test.go tests where they are)

@troy0820 same for the tests from validator_test.go (sorry not sure if that's WIP right now :))

Indeed, a little more work for the validator_test.go one. Should be coming soon.

camilamacedo86 · 2024-08-09T08:15:23Z

Hi folks,

Could we please not merge this one for the next release?
We need ensure that those changes will be applied in the Kubebuilder scaffolds beforehand.

@sbueringer @grzesuav

/hold

Motivation: kubernetes-sigs/controller-runtime#2877

sbueringer · 2024-08-09T10:23:59Z

Hi folks,

Could we please not merge this one for the next release? We need ensure that those changes will be applied in the Kubebuilder scaffolds beforehand.

@sbueringer @grzesuav

/hold

We are not going to merge it for 0.19 (August), but we are going to merge it for 0.20 (December/January).

We had this deprecated since 0.17, I don't think we should delay it further than December

Motivation: kubernetes-sigs/controller-runtime#2877

sbueringer

Thx, last one from my side

pkg/builder/webhook_test.go

Signed-off-by: Troy Connor <[email protected]>

k8s-ci-robot · 2024-08-26T15:18:18Z

@troy0820: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-controller-runtime-apidiff	`b265d8d`	link	false	`/test pull-controller-runtime-apidiff`

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.

sbueringer · 2024-08-26T16:59:24Z

/lgtm

/assign @alvaroaleman @vincepri

k8s-ci-robot · 2024-08-26T16:59:30Z

LGTM label has been added.

Git tree hash: 768a8843959209c67747ee81ea6178d221db3198

Motivation: kubernetes-sigs/controller-runtime#2877

sbueringer · 2024-09-02T13:36:20Z

/approve

k8s-ci-robot · 2024-09-02T13:36:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grzesuav, sbueringer, troy0820

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Motivation: kubernetes-sigs/controller-runtime#2877

…ok.Defaulter interfaces (#4060) Remove the usage of deprecated functions for webhooks Controller-Runtime change the interfaces and will remove the Default and Validation ones, so this PR ensure that kubebuilder scaffold will still working and addressing the deprecations. kubernetes-sigs/controller-runtime#2877

alculquicondor · 2024-09-03T18:35:10Z

pkg/builder/webhook_test.go

@@ -77,7 +77,7 @@ func runTests(admissionReviewVersion string) {
 		close(stop)
 	})

-	It("should scaffold a defaulting webhook if the type implements the Defaulter interface", func() {
+	It("should scaffold a custom defaulting webhook if the type implements the CustomDefaulter interface", func() {


why didn't you remove this test?

There is already should scaffold a defaulting webhook with a custom defaulter below.

I think this was an oversight, we can delete this one

I think same for should scaffold a custom validating webhook if the type implements the CustomValidator interface

operator-sdk generates the webhook code that targets sigs.k8s.io/controller-runtime v0.19 or older. As such, it contains webhook.Validator and webhook.Defaulter. This API was removed in v0.20, see [1]. To make this work, we patched the code out while generating it. But now we have to fix it. Fix it now for the new API. [1] kubernetes-sigs/controller-runtime#2877

k8s-ci-robot requested review from alvaroaleman and varshaprasad96 July 8, 2024 16:07

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 8, 2024

troy0820 force-pushed the troy0820/remove-deprecated-webhooks branch from 1c3cb32 to ebdc829 Compare July 8, 2024 16:27

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 8, 2024

troy0820 mentioned this pull request Jul 9, 2024

Avoid usage of newly deprecated webhook.Validator and webhook.Defaulter interfaces kubernetes-sigs/kubebuilder#3721

Closed

grzesuav approved these changes Jul 17, 2024

View reviewed changes

a-cordier mentioned this pull request Jul 30, 2024

gko 286: apim validation gravitee-io/gravitee-kubernetes-operator#759

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2024

remove deprecated Defaulter and Validator interfaces

e5a4a04

Signed-off-by: Troy Connor <[email protected]>

troy0820 force-pushed the troy0820/remove-deprecated-webhooks branch from ebdc829 to e5a4a04 Compare August 7, 2024 13:46

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2024

sbueringer mentioned this pull request Aug 8, 2024

Remove deprecated webhook interfaces #2641

Closed

sbueringer added this to the v0.20.x milestone Aug 8, 2024

sbueringer reviewed Aug 8, 2024

View reviewed changes

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

2dfddc9

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

8c7eab9

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

21200d4

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 mentioned this pull request Aug 9, 2024

⚠️ : (go/v4): Replace usage of deprecated webhook.Validator and webhook.Defaulter interfaces kubernetes-sigs/kubebuilder#4060

Merged

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

108174f

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

ab6673f

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 9, 2024

Remove the usage of deprecated functions for webhooks

f90b496

Motivation: kubernetes-sigs/controller-runtime#2877

sbueringer reviewed Aug 26, 2024

View reviewed changes

pkg/builder/webhook_test.go Outdated Show resolved Hide resolved

minimize the diff

b265d8d

Signed-off-by: Troy Connor <[email protected]>

troy0820 force-pushed the troy0820/remove-deprecated-webhooks branch from ec45cd7 to b265d8d Compare August 26, 2024 15:12

sbueringer added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 26, 2024

k8s-ci-robot assigned alvaroaleman, vincepri and sbueringer Aug 26, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2024

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Aug 31, 2024

Remove the usage of deprecated functions for webhooks

b502672

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Sep 1, 2024

Remove the usage of deprecated functions for webhooks

63f0184

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Sep 2, 2024

Remove the usage of deprecated functions for webhooks

b0bf19f

Motivation: kubernetes-sigs/controller-runtime#2877

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Sep 2, 2024

Remove the usage of deprecated functions for webhooks

fb29cd9

Motivation: kubernetes-sigs/controller-runtime#2877

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2024

k8s-ci-robot merged commit 896f6de into kubernetes-sigs:main Sep 2, 2024
12 of 13 checks passed

camilamacedo86 added a commit to camilamacedo86/kubebuilder that referenced this pull request Sep 2, 2024

Remove the usage of deprecated functions for webhooks

7875dcd

Motivation: kubernetes-sigs/controller-runtime#2877

alculquicondor reviewed Sep 3, 2024

View reviewed changes

sbueringer mentioned this pull request Sep 16, 2024

🌱 Cleanup defaulter & validator unit tests #2945

Merged

sbueringer mentioned this pull request Sep 30, 2024

Check if the other resources referenced by an object exist within a webhook #2968

Closed

d-honeybadger mentioned this pull request Jan 21, 2025

update for 1.32 release digitalocean/do-operator#97

Merged

clyang82 mentioned this pull request Feb 10, 2025

🌱 Bump sigs.k8s.io/controller-runtime from 0.19.0 to 0.20.1 stolostron/multicluster-global-hub#1369

Closed

wjf3121 mentioned this pull request Feb 17, 2025

Remove deprecated usage of admission.Validator and admission.Defaulter in v2 code Azure/azure-service-operator#4585

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠ Remove deprecated Defaulter and Validator interfaces #2877

⚠ Remove deprecated Defaulter and Validator interfaces #2877

troy0820 commented Jul 8, 2024 •

edited

Loading

sbueringer commented Jul 9, 2024

troy0820 commented Jul 9, 2024

k8s-ci-robot commented Jul 17, 2024

sbueringer commented Aug 8, 2024

sbueringer left a comment

sbueringer Aug 8, 2024 •

edited

Loading

sbueringer Aug 8, 2024

troy0820 Aug 21, 2024 •

edited

Loading

sbueringer Aug 22, 2024 •

edited

Loading

sbueringer Aug 22, 2024

troy0820 Aug 22, 2024

camilamacedo86 commented Aug 9, 2024 •

edited

Loading

sbueringer commented Aug 9, 2024

sbueringer left a comment

k8s-ci-robot commented Aug 26, 2024

sbueringer commented Aug 26, 2024

k8s-ci-robot commented Aug 26, 2024

sbueringer commented Sep 2, 2024

k8s-ci-robot commented Sep 2, 2024

alculquicondor Sep 3, 2024

sbueringer Sep 3, 2024

sbueringer Sep 3, 2024


		var _ = Describe("Defaulter Handler", func() {

		It("should return ok if received delete verb in defaulter handler", func() {

⚠ Remove deprecated Defaulter and Validator interfaces #2877

⚠ Remove deprecated Defaulter and Validator interfaces #2877

Conversation

troy0820 commented Jul 8, 2024 • edited Loading

sbueringer commented Jul 9, 2024

troy0820 commented Jul 9, 2024

k8s-ci-robot commented Jul 17, 2024

sbueringer commented Aug 8, 2024

sbueringer left a comment

Choose a reason for hiding this comment

sbueringer Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

sbueringer Aug 8, 2024

Choose a reason for hiding this comment

troy0820 Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

sbueringer Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

sbueringer Aug 22, 2024

Choose a reason for hiding this comment

troy0820 Aug 22, 2024

Choose a reason for hiding this comment

camilamacedo86 commented Aug 9, 2024 • edited Loading

sbueringer commented Aug 9, 2024

sbueringer left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 26, 2024

sbueringer commented Aug 26, 2024

k8s-ci-robot commented Aug 26, 2024

sbueringer commented Sep 2, 2024

k8s-ci-robot commented Sep 2, 2024

alculquicondor Sep 3, 2024

Choose a reason for hiding this comment

sbueringer Sep 3, 2024

Choose a reason for hiding this comment

sbueringer Sep 3, 2024

Choose a reason for hiding this comment

troy0820 commented Jul 8, 2024 •

edited

Loading

sbueringer Aug 8, 2024 •

edited

Loading

troy0820 Aug 21, 2024 •

edited

Loading

sbueringer Aug 22, 2024 •

edited

Loading

camilamacedo86 commented Aug 9, 2024 •

edited

Loading