Skip to content
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 not render without secret #178

Open
unique-dominik opened this issue Feb 26, 2024 · 5 comments
Open

chown init-container should not render without secret #178

unique-dominik opened this issue Feb 26, 2024 · 5 comments
Labels

Comments

@unique-dominik
Copy link

unique-dominik commented Feb 26, 2024

Insecurity warning

Caution

This would only apply to insecure setups as every secure setup should at least have some secret somewhere…

Problem

When investigating #177 I discovered, that even if it is not needed for logical reasons, the chown container always runs.

- "{{ include "zitadel.joincpcommands" (dict "commands" (list

That means, even when no secret is present to be chown-ed, the container still tries to change its ownership (of the, I assume, empty volume).

It bothers me aesthetically but in our prototyping, which we did for #177 it basically blocked us. We have to currently delete the init conatiner after every deployment.

Proposals

Fix #177 to reduce impact

Fix #177 and the problem is not so grave anymore

Render only if secrets present

Render the container only into the chart if secrets are present, maybe like so:

{{ $hasInitContainer = if or (.Values.zitadel.secretConfig .Values.zitadel.configSecretName. Values.zitadel.dbSslCaCrt .Values.zitadel.dbSslCaCrtSecret .Values.zitadel.dbSslUserCrtSecret) }}

and then omit the whole container if false.

@unique-dominik
Copy link
Author

Alright, we can no longer wait. I will fork the chart and come back with a PR if we ever make it work 🤣

@eliobischof
Copy link
Member

@unique-dominik do you already have something that I could take over to finish?

@unique-dominik
Copy link
Author

Hi @eliobischof

Yes I do, but its a workaround for now since I just disable the container if its not used 😢

      initContainers:
        {{- $isConfigPresent := and 
          .Values.zitadel.secretConfig 
          .Values.zitadel.configSecretName 
          .Values.zitadel.dbSslCaCrt 
          .Values.zitadel.dbSslCaCrtSecret 
          .Values.zitadel.dbSslUserCrtSecret 
        }}
        {{- if $isConfigPresent }}
        - args:
          - "{{ include "zitadel.joincpcommands" (dict "commands" (list
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.secretConfig "path" "/zitadel-secrets-yaml/" ))
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.configSecretName "path" "/zitadel-secret-config-yaml/" ))
              (include "zitadel.makecpcommand" (dict "value" (or .Values.zitadel.dbSslCaCrt .Values.zitadel.dbSslCaCrtSecret) "path" "/db-ssl-ca-crt/" ))
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.dbSslUserCrtSecret "path" "/db-ssl-user-crt/" ))
              )) }} chown -R 1000:1000 /chowned-secrets/ && find /chowned-secrets/ -type f -exec chmod 400 -- {} + "
          command:
          - sh
          - -c
          image: "{{ .Values.chownImage.repository }}:{{ .Values.chownImage.tag }}"
          imagePullPolicy: {{ .Values.chownImage.pullPolicy }}
          name: chown
          volumeMounts:
          - name: chowned-secrets
            mountPath: /chowned-secrets
          {{- if .Values.zitadel.secretConfig }}
          - name: zitadel-secrets-yaml
            mountPath: /zitadel-secrets-yaml
          {{- end }}
          {{- if .Values.zitadel.configSecretName }}
          - name: zitadel-secret-config-yaml
            mountPath: /zitadel-secret-config-yaml
          {{- end }}
          {{- if (or .Values.zitadel.dbSslCaCrt .Values.zitadel.dbSslCaCrtSecret) }}
          - name: db-ssl-ca-crt
            mountPath: /db-ssl-ca-crt
          {{- end }}
          {{- if .Values.zitadel.dbSslUserCrtSecret }}
          - name: db-ssl-user-crt
            mountPath: /db-ssl-user-crt
          {{- end }}
          securityContext:
            runAsNonRoot: false
            runAsUser: 0 # ! FIXME this is what currently breaks
        {{- end }}

@eliobischof
Copy link
Member

Hi @eliobischof

Yes I do, but its a workaround for now since I just disable the container if its not used 😢

      initContainers:
        {{- $isConfigPresent := and 
          .Values.zitadel.secretConfig 
          .Values.zitadel.configSecretName 
          .Values.zitadel.dbSslCaCrt 
          .Values.zitadel.dbSslCaCrtSecret 
          .Values.zitadel.dbSslUserCrtSecret 
        }}
        {{- if $isConfigPresent }}
        - args:
          - "{{ include "zitadel.joincpcommands" (dict "commands" (list
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.secretConfig "path" "/zitadel-secrets-yaml/" ))
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.configSecretName "path" "/zitadel-secret-config-yaml/" ))
              (include "zitadel.makecpcommand" (dict "value" (or .Values.zitadel.dbSslCaCrt .Values.zitadel.dbSslCaCrtSecret) "path" "/db-ssl-ca-crt/" ))
              (include "zitadel.makecpcommand" (dict "value" .Values.zitadel.dbSslUserCrtSecret "path" "/db-ssl-user-crt/" ))
              )) }} chown -R 1000:1000 /chowned-secrets/ && find /chowned-secrets/ -type f -exec chmod 400 -- {} + "
          command:
          - sh
          - -c
          image: "{{ .Values.chownImage.repository }}:{{ .Values.chownImage.tag }}"
          imagePullPolicy: {{ .Values.chownImage.pullPolicy }}
          name: chown
          volumeMounts:
          - name: chowned-secrets
            mountPath: /chowned-secrets
          {{- if .Values.zitadel.secretConfig }}
          - name: zitadel-secrets-yaml
            mountPath: /zitadel-secrets-yaml
          {{- end }}
          {{- if .Values.zitadel.configSecretName }}
          - name: zitadel-secret-config-yaml
            mountPath: /zitadel-secret-config-yaml
          {{- end }}
          {{- if (or .Values.zitadel.dbSslCaCrt .Values.zitadel.dbSslCaCrtSecret) }}
          - name: db-ssl-ca-crt
            mountPath: /db-ssl-ca-crt
          {{- end }}
          {{- if .Values.zitadel.dbSslUserCrtSecret }}
          - name: db-ssl-user-crt
            mountPath: /db-ssl-user-crt
          {{- end }}
          securityContext:
            runAsNonRoot: false
            runAsUser: 0 # ! FIXME this is what currently breaks
        {{- end }}

This doesn't look unreasonable to me 🙂 I think just not rendering the init container makes sense if it's not needed. I'll have a look at the PR, thanks 🙏

@unique-dominik
Copy link
Author

👍
I try to come up with PR's (that you did not do yet) after paternity leave 👶
The filesystem PR (if the container is needed) is more relevant I'd say as only then you get the whole impact of the rendered container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🧐 Investigating
Development

No branches or pull requests

3 participants