helmify icon indicating copy to clipboard operation
helmify copied to clipboard

Add a flag to disable/enable the webhook while rendering Operator

Open dashanji opened this issue 3 years ago • 9 comments
trafficstars

Hi, there, could we provide a flag to add a judgment in webhooks and relevant resources(service, volume, etc) while rendering Operator built by kubebuilder?

dashanji avatar Nov 16 '22 07:11 dashanji

Hi @dashanji can you please provide sample operator manifest along with actual and expected helmify output? it will help to write e2e test and fix the issue.

arttor avatar Nov 26 '22 10:11 arttor

Okay, In fact, we developed an operator using kubebuilder, and automatically generated a helm chart using helmify. If we can add the webhook option, as implemented in this commit, I believe many kubebuilder users are willing to use it to automatically generate helm charts and it is relatively perfect for helm users.

Refer https://github.com/kubernetes-sigs/kubebuilder/discussions/3074

dashanji avatar Nov 29 '22 07:11 dashanji

@arttor Hi, are you working on this? If possible, I can help add the feature.

dashanji avatar Mar 24 '23 02:03 dashanji

@dashanji yes, you can create PR or help with the proposal. In commit disabling webhook affects several manifests. For example, it removes volumeMounts from deployment:

        {{- if .Values.webhook.enabled }}
        volumeMounts:
        - mountPath: /tmp/k8s-webhook-server/serving-certs
          name: cert
          readOnly: true
        {{- end }}

The problem with this approach is that it is hard to define which volume is related to the webhook and which is not.

Will it work if we will just disable MutatingWebhookConfiguration or is there any better way to isolate webhook control?

arttor avatar Mar 24 '23 09:03 arttor

If we only disable the MutatingWebhookConfiguration, it may not work as the deployment can't find the volume. Actually, for a standard operator created by kubebuilder, the generated volume is the same. So we can add a flag to disable/enable the webhook for a standard operator, and ignore the some corner cases (such as changing the volumes, etc). WDYT?

dashanji avatar Mar 24 '23 09:03 dashanji

If the goal is to simply disable webhook validation for CRD, then we can just disable webhook and leave volume, claim, service, and stuff. You can try to disable the default volume in deployment but it will require a lot of effort.

arttor avatar Mar 24 '23 09:03 arttor

@dashanji thank you for your contribution! #97 is merged and available in v0.3.34 release. Should we close this issue?

arttor avatar Mar 29 '23 09:03 arttor

@dashanji thank you for your contribution! #97 is merged and available in v0.3.34 release. Should we close this issue?

I'm sorry not now. Actually, https://github.com/arttor/helmify/pull/97 is to add the cert-manager as a subchart rather than enable/disable the webhook. I'm still working on it and will ping you if finished. WDUT?

dashanji avatar Mar 29 '23 12:03 dashanji

Hi @arttor, the relevant pr is ready, could you please take a look?

dashanji avatar Apr 11 '23 12:04 dashanji