controller-tools icon indicating copy to clipboard operation
controller-tools copied to clipboard

allow customizing generated webhook's name and its service's namespace and name

Open davidxia opened this issue 1 year ago • 14 comments

I'm using kubebuilder which generates webhooks with these markers. The generated mutating webhook has a hardcoded name of mutating-webhook-configuration here, and the validating webhook has a hardcoded name of validating-webhook-configuration here.

The webhook's .webhooks[].clientConfig.service also have their namespace hardcoded to system and name to webhook-service, respectively.

I'd like to be able to customize these three parameters with the markers. My webhook Service might be in a different namespace and name. I might also have different webhooks that need different [Validating|Mutating]WebhookConfiguration names. It'd be nice to not have to generate these webhook YAMLs and then have to use kustomize to alter them.

Is this a feature request you're open to?

related to https://github.com/kubernetes-sigs/controller-tools/issues/490

### Tasks
- [ ] https://github.com/kubernetes-sigs/controller-tools/pull/1002
- [ ] allow customizing webhook Service's namespace and name

davidxia avatar Dec 01 '23 22:12 davidxia

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Feb 29 '24 23:02 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Mar 31 '24 00:03 k8s-triage-robot

@davidxia sounds reasonable, if you want to open a PR for it

sbueringer avatar Apr 03 '24 05:04 sbueringer

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-triage-robot avatar May 03 '24 05:05 k8s-triage-robot

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

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.

k8s-ci-robot avatar May 03 '24 05:05 k8s-ci-robot

@davidxia sounds reasonable, if you want to open a PR for it

@sbueringer thanks. Can you point to the places in code where I'll need to change things?

davidxia avatar Jul 02 '24 18:07 davidxia

@camilamacedo86 don't know if you're also a maintainer for this repo, but are you able to give me some pointers to start a PR for this one too? 🙏

davidxia avatar Jul 03 '24 14:07 davidxia

@davidxia we had a very similar PR before https://github.com/kubernetes-sigs/controller-tools/pull/824

I would expect roughly the same changes are necessary here

/reopen /remove-lifecycle frozen /lifecycle frozen

sbueringer avatar Jul 10 '24 06:07 sbueringer

@sbueringer: Reopened this issue.

In response to this:

@davidxia we had a very similar PR before https://github.com/kubernetes-sigs/controller-tools/pull/824

I would expect roughly the same changes are necessary here

/reopen /remove-lifecycle frozen /lifecycle frozen

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.

k8s-ci-robot avatar Jul 10 '24 06:07 k8s-ci-robot

@sbueringer thanks! Just curious why the url marker doesn't show up in the webhook markers doc?

davidxia avatar Jul 10 '24 13:07 davidxia

I would say because nobody thought about updating the docs over there :)

Although I wonder if that page is generated (but I don't know)

sbueringer avatar Jul 10 '24 14:07 sbueringer

I would say because nobody thought about updating the docs over there :)

Although I wonder if that page is generated (but I don't know)

Filed doc issue here.

davidxia avatar Jul 10 '24 14:07 davidxia

@sbueringer I added a task list to the issue description. We've finished the first one. For the second of allow customizing webhook Service's namespace and name, what do you think of adding an optional serviceName and serviceNamespace to the kubebuilder:webhookconfiguration marker?

Currently +kubebuilder:webhookconfiguration:mutating=<bool>[,name=<string>]

Proposed +kubebuilder:webhookconfiguration:mutating=<bool>[,name=<string>][,serviceName=<string>][,serviceNamespace=<string>]

davidxia avatar Sep 12 '24 19:09 davidxia

Thx. Sounds good

sbueringer avatar Sep 13 '24 06:09 sbueringer

Also it would be nice to have a servicePort argument for mutatingwebhookconfiguration.webhooks.clientConfig.service.port

risentveber avatar Nov 13 '24 16:11 risentveber

@sbueringer based on existing code, it made more sense to me to add Service name, namespace, and port to the Config struct. If https://github.com/kubernetes-sigs/controller-tools/pull/1128 looks good, I'll write tests and move the PR out of draft status.

davidxia avatar Jan 17 '25 16:01 davidxia

Can be probably closed, now that #1128 is merged/merging?

sbueringer avatar Jan 31 '25 13:01 sbueringer

yes!

davidxia avatar Jan 31 '25 14:01 davidxia