✨ Allow customizing generated webhook K8s Service

[davidxia]

Allow customizing the webhook K8s Service's name, namespace, and port.

example usage

Look for the new serviceName, serviceNamespace, and servicePort fields.

❯ GOBIN=(pwd)/bin go install ./cmd/*

❯ ./bin/controller-gen webhook -w

Webhook

+kubebuilder:webhook:admissionReviewVersions=<[]string>,failurePolicy=<string>,groups=<[]string>[,matchPolicy=<string>],mutating=<bool>,name=<string>[,path=<string>][,reinvocationPolicy=<string>],resources=<[]string>[,serviceName=<string>][,serviceNamespace=<string>][,servicePort=<int>][,sideEffects=<string>][,timeoutSeconds=<int>][,url=<string>],verbs=<[]string>,versions=<[]string>[,webhookVersions=<[]string>]  package  specifies how a webhook should be served.

...

contributes to #865

[k8s-ci-robot]

Hi @davidxia. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

[davidxia]

tested manually by

1. GOBIN=(pwd)/bin go install ./cmd/*
2. create new kubebuilder-scaffolded project in a directory named project
   a. kubebuilder init --domain tutorial.kubebuilder.io --repo tutorial.kubebuilder.io/project
   b. kubebuilder create api --group batch --version v1 --kind CronJob
   c. kubebuilder create webhook --group batch --version v1 --kind CronJob --defaulting --programmatic-validation
   d. comment out the line namespace: project-system in config/default/kustomization.yaml so it doesn't override the K8s Service namespace we're testing.
3. copy the ./bin/controller-gen executable into the kubebuilder project's bin/controller-gen-vX.Y.Z path
4. remove the kubectl apply from the kubebuilder project's Makefile's deploy target

case 1: backwards compatibility

1. run make deploy and check that kind: MutatingWebhookConfiguration and kind: ValidatingWebhookConfiguration have the same .webhooks[0].clientConfig.service as before.

   apiVersion: admissionregistration.k8s.io/v1
   kind: MutatingWebhookConfiguration
   metadata:
     name: project-mutating-webhook-configuration
   webhooks:
   - admissionReviewVersions:
     - v1
     clientConfig:
       service:
         name: project-webhook-service
         namespace: system
         path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
   

   apiVersion: admissionregistration.k8s.io/v1
   kind: ValidatingWebhookConfiguration
   metadata:
     name: project-validating-webhook-configuration
   webhooks:
   - admissionReviewVersions:
     - v1
     clientConfig:
       service:
         name: project-webhook-service
         namespace: system
         path: /validate-batch-tutorial-kubebuilder-io-v1-cronjob
   

case 2: modify marker only for MutatingWebhookConfiguration

1. append serviceName=DEAD,serviceNamespace=BEEF,servicePort=1234 to the line kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,... in internal/webhook/v1/cronjob_webhook.go

2. run make deploy and check that kind: MutatingWebhookConfiguration has the following.

   kind: MutatingWebhookConfiguration
   metadata:
     name: project-mutating-webhook-configuration
   webhooks:
   - admissionReviewVersions:
     - v1
     clientConfig:
       service:
         name: DEAD
         namespace: BEEF
         path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
         port: 1234
   

   instead of

   kind: MutatingWebhookConfiguration
   metadata:
     name: project-mutating-webhook-configuration
   webhooks:
   - admissionReviewVersions:
     - v1
     clientConfig:
       service:
         name: project-webhook-service
         namespace: system
         path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
   

3. check that kind: ValidatingWebhookConfiguration has no changes

[sbueringer]

Overall lgtm so far

[sbueringer]

/ok-to-test

[davidxia]

Thanks for the initial review. Wrote an integration test. Ready for another review.

[sbueringer]

Thank you!

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Details

Git tree hash: babb9bdd65bafd28d6a0a9743b709b706d2bcc68

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidxia, sbueringer

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

The pull request process is described here

Details

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

[davidxia]

thank you!

[sbueringer]

You're welcome :) Thx for the PR(s)!

[camilamacedo86]

@davidxia

It looks like this change may have broken the default behavior 😞

GitHub Actions Run

Is this the intended outcome? Or is it an unexpected side effect of the change?

cc/ @sbueringer

[sbueringer]

I don't see how this change could affect the name of your CRD?

CustomResourceDefinition.apiextensions.k8s.io "project-v4-system" is invalid: metadata.name: Invalid value: "project-v4-system": must be spec.names.plural+"."+spec.group

[sbueringer]

At most this will affect the content of *WebhookConfiguration objects

[davidxia]

This change is meant to be backwards-compat.

I also see

      error when retrieving current configuration of:
      Resource: "admissionregistration.k8s.io/v1, Resource=mutatingwebhookconfigurations", GroupVersionKind: "admissionregistration.k8s.io/v1, Kind=MutatingWebhookConfiguration"
      Name: "project-v4-mutating-webhook-configuration/project-v4-serving-cert", Namespace: ""
      from server for: "STDIN": invalid resource name "project-v4-mutating-webhook-configuration/project-v4-serving-cert": [may not contain '/']

Is this another error separate from must be spec.names.plural+?

[camilamacedo86]

You can close this one, @davidxia, as it's sorted out. I figured out what was wrong—it was my bad. Everything is working as before. Sorry for the unnecessary disturbance!