✨ (helm/v2-alpha): add extra volumes support#5496
✨ (helm/v2-alpha): add extra volumes support#5496camilamacedo86 wants to merge 1 commit intokubernetes-sigs:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
05b5fdc to
79c2382
Compare
|
|
||
| The chart supports extra volumes and volume mounts for the manager (e.g. secrets, config files) in addition to the built-in webhook and metrics cert volumes. | ||
|
|
||
| - **From kustomize**: If your deployment in `dist/install.yaml` includes volumes other than the default `webhook-certs` and `metrics-certs` (from the scaffolded patches), the plugin adds them to `values.yaml` as `manager.extraVolumes` and `manager.extraVolumeMounts`. The manager template merges them into the deployment. |
There was a problem hiding this comment.
To clarify this point, if they are defined in the kustomize output, other than metrics-certs and web hook-certs, the values are extracted and added to the values?
I would have expected them to be added into the chart template and not the values. As any further additions would require the plugin to be run with --force.
There was a problem hiding this comment.
To clarify this point, if they are defined in the kustomize output, other than metrics-certs and web hook-certs, the values are extracted and added to the values?
Yes.
I would have expected them to be added into the chart template and not the values. As any further additions would require the plugin to be run with --force.
Yes, that is how it is. To update the values you need to run the force.
We cannot update the values without force otherwise it would lost the customizations.
There was a problem hiding this comment.
I think you missed my point or I didn't articulate myself very well.
I think if the volumes are defined in kustomize, they should be added directly to the template and NOT to the values.yaml
The values.yaml should only be for additional volumes IMO.
There was a problem hiding this comment.
I think if the volumes are defined in kustomize,
All should be defined in the kustomize.
They are the source of truth.
The Helm Chart is just a way to package the solution in this format.
It should not deviate from the project config, and we should not add anything in the Helm charts. It only package the project for this format.
There was a problem hiding this comment.
My example:
# config/manager/manager.yaml (or a kustomize patch)
spec:
template:
spec:
volumes:
- name: app-secret-1
secret:
secretName: app-secret-1# dist/chart/values.yaml
manager:
extraVolumes:
- name: app-secret-1
secret:
secretName: app-secret-1# dist/chart/templates/deployment.yaml (snippet)
spec:
template:
spec:
volumes:
{{- if .Values.manager.extraVolumes }}
{{- toYaml .Values.manager.extraVolumes | nindent 8 }}
{{- end }}Lets say the maintainer makes modifications to the values.yaml file but also adds another volume to the config....
# config/manager/manager.yaml (or a kustomize patch)
spec:
template:
spec:
volumes:
- name: app-secret-1
secret:
secretName: app-secret-1
- name: app-secret-2
secret:
secretName: app-secret-2The only way to have that new volume in the chart is either add it manually to the values.yaml or re-run kubebuilder edit --plugins=helm/v2-alpha --force which would overwrite any manual changes.
What I am saying is, I believe the generated template should be:
# dist/chart/templates/deployment.yaml (snippet)
spec:
template:
spec:
volumes:
- name: app-secret-1
secret:
secretName: app-secret-1
- name: app-secret-2
secret:
secretName: app-secret-2
{{- if .Values.manager.extraVolumes }}
{{- toYaml .Values.manager.extraVolumes | nindent 8 }}
{{- end }}There was a problem hiding this comment.
Hi @asergeant01
Let me know if the changes in place address your request.
There was a problem hiding this comment.
@camilamacedo86 I didn't test it, but it certainly looks like it addresses the request. Thank you
79c2382 to
3594c69
Compare
3594c69 to
0b4ab1b
Compare
Support manager.extraVolumes and manager.extraVolumeMounts: extract from kustomize (excluding webhook-certs/metrics-certs), inject into values when present, and template in manager deployment (including when volumes: []). Document in Helm v2-alpha plugin page. Generated-by: Cursor/Claude
0b4ab1b to
62e49e5
Compare
Support manager.extraVolumes and manager.extraVolumeMounts: extract from kustomize (excluding webhook-certs/metrics-certs), inject into values when present, and template in manager deployment (including when volumes: []). Document in Helm v2-alpha plugin page.
Closes: #5485