Add securityContext definitions on container level

[rgarcia89]

Deployment hardening by adding / updating the securityContext definitions on container level.

[Vad1mo]

what are the default settings, if this is not set?

The reason I am asking is the implications of this change to backwards compatibility

[rgarcia89]

@Vad1mo default values are as followed on container level:

privileged is false per default
allowPrivilegeEscalation is false if privileged is set to false
capabilties defaults to the set of capabilities granted by the container runtime

[Kajot-dev]

Having that would partially solve the problem of non-compliance with restricted PodSecurityStandard, since it requires seccompProfile definition (see https://kubernetes.io/docs/concepts/security/pod-security-standards/#privileged) in 1.19+.
Could seccompProfile be added for k8s 1.19+ (using helm Capabilities)?

[Kajot-dev]

@rgarcia89 initContainers also should have these and container from core-pre-upgrade-job.yaml should also have that.

If these changes would get merged (with seccompProfile and missing containers/initContainers) If seccompProfile I would favour this PR over my #1666, since all containers would comply with the most restrictive k8s security standard.

[rgarcia89]

@Kajot-dev valid point. I will add the seccompProfile and also add the securityContext definition to the initContainers and the once I missed.

[Kajot-dev]

@rgarcia89 Thanks, looks nice!

[rgarcia89]

Perfect, so we only need to find a second reviewer.
Maybe @zyyw?

[MinerYang]

#456
Thanks @rgarcia89 for your contribution !
We are in review of this PR. Will merge it once it done

[Kajot-dev]

@rgarcia89 Would it be possible to add runAsNonRoot to securityContext one the pod scope since

• It already runs as non-root
• PSP restricted requires this field to be present (https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted)

[rgarcia89]

@Kajot-dev sure - I just added them.

[MinerYang]

Hi @rgarcia89 , @Kajot-dev

Thanks for both of your contributions on the PSP and security contexts. By reviewing both of pt-1763 and pr1666, I have a few thoughts combined with some of our downstream usage scenario:

• how about we still expose a containerSecurityContext like @Kajot-dev provided in his PR. But not set for every container respectively, instead using only one settings for all containers in values.yaml, e.g. Values.containerSecurityContext
• and for this containerSecurityContext settings, we have default values same in these two PRs to comply with PSP

I made a sample PR https://github.com/goharbor/harbor-helm/pull/1695/files for more details
How about we focus on this PR and I will close the other instead(#1666).

[rgarcia89]

@MinerYang imho, I'm fine with both approaches. I'm fine with making them configurable through the values file, just as much as hardcoding them into the specific YAMLs. I opted for the latter initially, believing there isn't much reason for anyone to modify them. Nevertheless, if you prefer them to be modifiable, I propose we close my PR and proceed with yours. My main goal is to ensure that we integrate this into the Helm chart, regardless of the method ;)

[MinerYang]

Thanks @rgarcia89 .
for now I think what we hard coding in the PR is enough for most of the cluster providers. But I still keep it as configurable in case of Kubernetes evolve these standards in the future and also make it more flexible.

[MinerYang]

Thanks @rgarcia89 !

Will close this PR and please help to review #1695 as we discussed.