sloth icon indicating copy to clipboard operation
sloth copied to clipboard

feat: add securityContext for pod and container

Open ChrisFraun opened this issue 3 years ago • 2 comments

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.

ChrisFraun avatar Jul 25 '22 08:07 ChrisFraun

any thoughts @slok ? :)

ChrisFraun avatar Aug 09 '22 08:08 ChrisFraun

We are also waiting for this to get merged

peteroruba avatar Aug 09 '22 18:08 peteroruba

+1, we are waiting for this too.

bfennane avatar Oct 06 '22 11:10 bfennane

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 :)

slok avatar Oct 22 '22 10:10 slok

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?

ChrisFraun avatar Oct 24 '22 06:10 ChrisFraun

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.

slok avatar Oct 24 '22 07:10 slok

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.

ChrisFraun avatar Oct 24 '22 07:10 ChrisFraun

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! :)

ChrisFraun avatar Oct 24 '22 10:10 ChrisFraun

I think that this may help you to have a better feedback loop :)

cd ./deploy/kubernetes/helm/sloth/tests/ && go test -v ./...

slok avatar Oct 25 '22 09:10 slok

PASS ok github.com/slok/sloth/helm 0.537s :)

ChrisFraun avatar Oct 25 '22 13:10 ChrisFraun

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.

codecov-commenter avatar Oct 25 '22 16:10 codecov-commenter