search

arttor / helmify

Creates Helm chart from Kubernetes yaml
MIT License
1.48k stars 136 forks source link

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

Closed dashanji closed 1 week ago

dashanji commented 2 years ago

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?

arttor commented 2 years ago

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.

dashanji commented 1 year ago

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 commented 1 year ago

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

arttor commented 1 year ago

@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?

dashanji commented 1 year ago

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?

arttor commented 1 year ago

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 commented 1 year ago

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

dashanji commented 1 year ago

@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 commented 1 year ago

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