feat: add securityContext for pod and container
What: Deployments can have security settings in their manifest on two levels: pod and container. However, there are some capabilities only configurable in one of the respective levels(https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#securitycontext-v1-core). This PR sets a default configuration for container securityContext, which drops all POSIX capabilities and denies privilege escalation and for pod securityContext adds user, group fsGroup and supplementalGroups and also denies root usage. These are and should be standard settings in the context of Kubernetes. It also adds the possibility of running vault-injector in a Kubernetes environment without PSP (to be removed in v1.25 https://kubernetes.io/docs/concepts/security/pod-security-policy/), but with OpenPolicyAgent (possibly the PSP substitute) with the same capabilities as a restricted PSP instead.
This PR sets the respective settings to the values.yaml and is defaulting them as well. With this they can be adopted if it is needed.
any thoughts @slok ? :)
We are also waiting for this to get merged
+1, we are waiting for this too.
Hi @ChrisFraun, thanks for the PR!
I will try reviewing this next week, however, I would prefer to not have this as a default, but as an option. I think that most people lean towards not having all this complexity by default.
Also, could you add some tests please :pray:
Security is important, but simplicity too, it all depends on the context you are running things :)
I understand! whats better than simply basic security by default then (which can even be modified if needed)? can you point me out where exactly I should write those tests?
Here are the tests: https://github.com/slok/sloth/tree/main/deploy/kubernetes/helm/sloth/tests
I don't want to derail the PR in another kind of discussion about security philosophy, I just going to say that security adds complexity (always, and at multiple layers), and context matters (people forget about this).
If this default doesn't have any side effects, why Kubernetes doesn't set this as a default when not specifying anything? I'm not, the one forcing users to select their pod security if Kubernetes doesn't do it either.
Don't get me wrong, I'm not saying security is not important, security it's so important matter that at the moment I would need this kind of security level, I would not be comfortable depending on a random third-party helm chart to control my cluster security, I would use webhooks and/or controllers to have that sorted at a cluster level.
True, we can leave the settings empty as well. For me the most important that the contexts are set correctly for pod and containers respectively.
Not completely sure if I did it right. I ran the test commands make test locally and it seems to work. Would be grateful for a look and some feedback @slok! :)
I think that this may help you to have a better feedback loop :)
cd ./deploy/kubernetes/helm/sloth/tests/ && go test -v ./...
PASS ok github.com/slok/sloth/helm 0.537s :)
Codecov Report
Merging #357 (1afdf91) into main (932d116) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## main #357 +/- ##
=======================================
Coverage 75.58% 75.58%
=======================================
Files 17 17
Lines 1491 1491
=======================================
Hits 1127 1127
Misses 286 286
Partials 78 78
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.