search

fluxcd-community / helm-charts

Community maintained Helm charts for Flux
Apache License 2.0
121 stars 76 forks source link

feat: add imagePullPolicy to all controllers #115

Closed jlehtimaki closed 2 years ago

jlehtimaki commented 2 years ago

What this PR does / why we need it:

This PR allows users to select the imagePullPolicy for the controllers. In most cases it's even recommended to run imagePullPolicy: Always for security reasons.

Special notes for your reviewer:

Checklist

dwerder commented 2 years ago

Can you please add a test for that option?

jlehtimaki commented 2 years ago

Hey @dwerder thanks for checking. I added the tests you requested, let me know if there's anything else.

stefanprodan commented 2 years ago

In most cases it's even recommended to run imagePullPolicy: Always for security reasons.

IMO this has grave implications on availability, using imagePullPolicy: Always means you could end up with different versions of an app in the same replicaset, it also slows down the startup of every pod, during a network incident this will render your cluster usable, and not to mention the network costs this implies. If you want to ensure your cluster is running the Flux images unaltered, better use a policy agent that verifies the Flux cosgin signatures, docs here: https://fluxcd.io/blog/2022/02/security-image-provenance/#enforcing-verified-signatures-in-a-cluster

jlehtimaki commented 2 years ago

@stefanprodan sure you can use OPA or some other means but that then requires another component to be added to Kubernetes. The main reason here is a security one. Kubernetes/Docker caching does not check where the cached image originates from, it simply does not care. Having it as "Always" does not overwrite the caching mechanism underneath Kubernetes. Kubelet will always first fetch the image SHA/Digest and compare it to the cached one, if it matches it will use the cached one, if not it will download the correct one. So there's no really an argument for network costs or network incidents.

But anyways, I think this option is best to leave under the users hands, hence having the ability to select what policy to use. :)