postfinance / kubelet-csr-approver

Kubernetes controller to enable automatic kubelet CSR validation after a series of (configurable) security checks
MIT License
177 stars 34 forks source link

feat: allow adding extra objects to helm chart #222

Closed rajan123456 closed 9 months ago

rajan123456 commented 9 months ago

Great project, thanks for your efforts!

This PR aims to add the ability to allow addition of extra objects that can be deployed alongside the helm chart.

Use Case: Our kubernetes cluster allows only internal image registries. With this addition, we will be able to create the secret object which provides the imagePullSecret alongside the helm chart deployment.

clementnuss commented 9 months ago

Hi Rajan!

I took a look at your PR, and could not find how to make use of it (i.e. I tried deploying another manifest using .Values.extraDeploy, used as an array, but it failed).

I think some (commented out) example would be much needed on top of that variable.

However, I also have to question your use case for this feature: IMO, deploying a secret should/could typically be done by another tool. for example a vault integration, or with Mozilla SOPS and flux. at least it's simpler than having to integrate it in the helm Chart once the tools are configured.

If you still want the future, please document it with a comment in values.yaml and I'll give it another go 🙃

rajan123456 commented 9 months ago

Hi @clementnuss -

Thanks for the review. I have added in an example as suggested. Please help to review again and let me know if you would like any further changes!

For the use case, this is useful for not just secrets but for adding any additional objects and provides greater flexibility to use the chart.

clementnuss commented 9 months ago

thanks for your changes! I did a minor change for the field name: https://github.com/postfinance/kubelet-csr-approver/pull/222/commits/30798edbb2712b0e90a79b80e17dd9263b846e8d

if that's fine for you, I'll merge it and make a new release soon!

rajan123456 commented 9 months ago

thanks for your changes! I did a minor change for the field name: 30798ed

if that's fine for you, I'll merge it and make a new release soon!

LGTM. Many thanks!