Webhook marker allows handler path to be changed, but a deterministic handler is always registered

[rajathagasthya]

Webhook marker like the one below lets us change path where webhook is served. This creates the appropriate Mutating|ValidatingWebhookConfiguration manifest referencing this path.

// +kubebuilder:webhook:verbs=create;update,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob.kb.io

But webhook builder always registers the same path based on GVK:

controller-runtime/pkg/builder/webhook.go

Lines 158 to 166 in 29e923a

	func generateMutatePath(gvk schema.GroupVersionKind) string {
	return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
	gvk.Version + "-" + strings.ToLower(gvk.Kind)
	}
	func generateValidatePath(gvk schema.GroupVersionKind) string {
	return "/validate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
	gvk.Version + "-" + strings.ToLower(gvk.Kind)
	}

This is confusing when the user thinks they can customize the handler path, but it's actually ignored and worse, the generated webhook manifest doesn't work because path is incorrect.

If we are letting users change the path through the marker, perhaps it's better to add an option to webhook builder to configure handler path as well? Or, if the path is not supposed to be changed in the marker, we should say that in docs.

[alenkacz]

maybe this bug should rather be in kubebuilder project? Since it could be solved on multiple places and might also need update in documentation?

Anyway - I took a look at it. One possible solution would be to allow overriding path inside the builder here

controller-runtime/pkg/builder/webhook.go

Line 38 in dc83571

config *rest.Config

and use that instead of generating one if provided. If we go that way, we would also have to update docs here https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation.html

WDYT?

[alenkacz]

/assign

[rajathagasthya]

@alenkacz Yeah, we could provide an option in webhook builder to set path and then update docs to say "path in webhook build must match path in webhook marker" or something to that effect.

We probably also need some kind of validation if we let users set their own path.

[ryanzhang-oss]

maybe this bug should rather be in kubebuilder project

kubernetes-sigs/kubebuilder#1436

[vincepri]

/kind design

We need to think more about possible solutions

[vincepri]

/help

[k8s-ci-robot]

@vincepri:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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/test-infra repository.

[ryanzhang-oss]

I am not familiar with the process here, what does "/help" mean? It seems that this is a pretty straightforward bug to fix.

[alexeldeib]

"/help" (adding the help-wanted label) is usually a good signal for issues to pick up 🙂 https://github.com/kubernetes/community/blob/master/contributors/guide/help-wanted.md#overview

However @alenkacz self-assigned, maybe she is still working on this? We did just discuss it recently.

FWIW I think adding to the builder seems reasonable. I think @DirectXMan12 mentioned some caveats with this, but i'm not seeing them on the controller-runtime side? Was it something to do with the kubebuilder side of templating things?

[DirectXMan12]

"path in webhook build must match path in webhook marker"

seems reasonable. Might have to be path prefix.

My concern was mainly that we don't want to tie those two together. Allowing you to customize the path (optionally) seems fine.

[devigned]

I was thinking about taking a look at this issue if it's not being actively worked on. Thoughts?

[DirectXMan12]

go ahead :-)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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/test-infra repository.

[damsien]

Hello, I want to reopen this issue because the solution has not been implemented. I think that there is some users who wants to have a way to configure their webhooks using a custom path.

/remove-lifecycle rotten

The following PR (#2998) implements this functionality.

[k8s-ci-robot]

@damsien: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

Hello, I am reopening this issue because the solution has not been implemented. I think that there is some users who wants to have a way to configure their webhooks using a custom path.

/reopen
/remove-lifecycle rotten

The following PR implements this functionality.

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.