-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chown init-container should run as non-root #177
Comments
Alright, we can no longer wait. I will fork the chart and come back with a PR if we ever make it work 🤣 |
@unique-dominik I would be glad if you could open a draft PR with something that works for you. |
Here I do not have a solution yet as I am overasked by what the container tries to solve 🤣 |
I'm looking into this, too, from the standpoint of wanting to run on kubernetes clusters where I'm definitely no k8s security expert, but it seems to me that using an appropriate securityContext here should allow the files to have the correct ownership, from the statement:
(from Configure volume permission and ownership change policy for Pods) There is a long-standing issues in kubernetes of defaultMode not being fully set when using runAsUser, but I think that bringing those files to an emptyDir volume, with current context, should allow specific user/group from the securityContext and which ever mode we want. From what I can tell, this is how things were handled in the bitnami postgresql chart. |
…ing user) This relies on the fact that an emptyDir is set up with appropriate permissions for whichever userid is running in the pod. The prior solution required running chown/chmod by root in the init container. This obviates the need for root, which is not permitted in many kubernetes installs as a security precaution. Fixes zitadel#177 and also fixes zitadel#178 (since there is no chown anymore).
We are very happy users of Zitadel and we use it in heavily regulated environments (financial services industry). Some of these environments forbid the usage of root users (also for init containers), e.g. via Gatekeeper I believe.
Meta
Chart version:
latest
Related Discord discussion: Ref
Problem
The init container
chown
that changes the ownership of the secret directories to1000:1000
uses root privileges.zitadel-charts/charts/zitadel/templates/deployment.yaml
Line 190 in bcfdd4f
Potential solutions
Configure volume permission and ownership change policy for Pods
Use Configure volume permission and ownership change policy for Pods to let the chart consumer decide, which
user:group
they want to use for the secrets and zitadel itself.In fact, it would maybe even be smart to reuse the same podSecurityContext for all containers but I am no Security Context expert.
Do not change any ownership
There also the option to try and not change the ownership anymore and see how it goes.
Known constraints
@eliobischof Already mentioned, that this might be due to older k8s versions not being able to mount volumes/files with correct ownership. It is to be seen if a change to the chart would break its backward compatibility.
The same should apply to all init containers, so namely also the
generate-self-signed-cert
. This does not run as root though but uses a hardcoded1000:1000
which could limit someone else somewhere else.I also see that the chart basically allows any context:
https://github.com/zitadel/zitadel-charts/blob/bcfdd4f8306de34cb7869f0d7204fdb97a9d54f3/charts/zitadel/templates/deployment.yaml#L38C7-L38C16
Which could lead to new ownership problems again if you run
chown
as2000:2000
andzitadel
itself as1000:1000
wouldn't we have the same ownership issue again?Help
I can contribute a PR but my key issue right now is not even time; it's I do literally not know how one would solve that 🤣 I mean I can add the PSC but then I might just break everyone else's Zitadel since I have no clue what I am doing. Maybe it needs a major version chart bump.
Sidenote
By the way this week we will need to use custom certificates for the customers setup which will certainly lead to us using secerts, which will force us to run the init container which in turn will force me to fork the chart for the time being 😢 But good note is: I can then contribute the change as I can live test the
podSecurityContext
or what ever you might tell me here 🚀The text was updated successfully, but these errors were encountered: