Add securityContext to skupper-router deployment

[fgiorgetti]

Fixes #2326

[fgiorgetti]

One point to discuss here.
Would it be better if we add a new env var + flag to the Controller, so that we can define whether
or not to use SCC / PSA at runtime, instead of using a static logic?

[c-kruse]

I think that the SecurityContexts set up on the containers are pretty unobjectionable. I suppose we could gate them, but I can't think of a situation where any of these settings would not be desirable.

[c-kruse]

I am curious where we came up with this version: Looking back in api docs history I didn't see much evidence that this wouldn't work prior to 1.24 - near as I can tell it would work as far back as 1.19 (when seccomp went GA.)

[fgiorgetti]

I noticed that as well. I left is as is, because that is the version defined in V1.
Anyway, I believe we will end up removing the seccompProfile element from the PR,
so this whole function can go away.

[c-kruse]

I suspect seccompProfile type "RuntimeDefault" is going to be suitable for most use cases, but hard coding it would likely end up being a deal breaker in some environments. We definitely need a way to either omit a seccompProfile (and defer the setting to an external controller or mutating admissions webhook) or a way to configure this (enviornment variable/flag/site setting.)

[fgiorgetti]

I agree with you. The seccompProfile should be left out of this.

[fgiorgetti]

@nluaces @c-kruse @hash-d Do you guys see any issue with keeping just the following entries:

top level securityContext (seccompProfile left out)

• deployment.spec.template.spec.securityContext.runAsNonRoot (true)

all containers

• deployment.spec.template.spec.containers[*].securityContext.allowPrivilegeEscalation (false)
• deployment.spec.template.spec.containers[*].securityContext.capabilities (drop all)
• deployment.spec.template.spec.containers[*].securityContext.runAsNonRoot (true)

all initContainers

• deployment.spec.template.spec.initContainers[*].securityContext.allowPrivilegeEscalation (false)
• deployment.spec.template.spec.initContainers[*].securityContext.capabilities (drop all)
• deployment.spec.template.spec.initContainers[*].securityContext.runAsNonRoot (true)

I believe that even these agreed upon fields, could be added by default as part of the upcoming 2.2 release,
but we could introduce a flag/env-var to disable this behavior, if someone needs that for any reason.

If you guys agree, I will make these changes and request a new round of review.

Thank you.

[c-kruse]

@fgiorgetti that approach sounds good to me!

[fgiorgetti]

@c-kruse @nluaces @hash-d I have just added a flag to disable the new default behavior of adding a securityContext with the default values we discussed here. Please give it a try and let me know your thoughts.