zammad-helm icon indicating copy to clipboard operation
zammad-helm copied to clipboard

Postgresql credentials should be percent encoded

Open hsahmed opened this issue 1 year ago • 9 comments

Is this a request for help?:


Is this a BUG REPORT or FEATURE REQUEST? (choose one): BUG REPORT

Version of Helm and Kubernetes: Kubernetes: 1.28.2 Zammad Chart: 10.2.1

What happened: It is not possible to use a Postgresql password containing any characters that have a special meaning within the URL (& : % / and possible others).

What you expected to happen: Any characters in the password or other elements of the URL that have special meaning within the URL should percent-encoded.

How to reproduce it (as minimally and precisely as possible): Install the Helm release using the following values:

zammadConfig:
  postgresql:
    pass: "zammad:test"

postgresql:
  auth:
    postgresPassword: "zammad:test"
    password: "zammad:test"
    replicationPassword: "zammad:test"

Anything else we need to know: Helm has a template function urlquery that can be used to encode the values https://helm.sh/docs/chart_template_guide/function_list/#urlquery

hsahmed avatar Feb 21 '24 10:02 hsahmed

@hsahmed you are right, thanks for the report. However, since the password comes from a kubernetes secret object, we have no chance to perform Helm string manipulation on it. I am not sure how to best solve this at present.

One possibility might be fixing this in the Zammad stack by adding an initializer that creates DATABASE_URL if it is not present (moving it from the current docker-entrypoint.sh to an initializer).

@monotek would you have any suggestion here?

mgruner avatar Feb 22 '24 11:02 mgruner

@hsahmed as a workaround, you can override DATABASE_URL via extraEnv. It will produce a helm warning, but it will work.

mgruner avatar Feb 23 '24 09:02 mgruner

@mgruner we're using PGO operator to deploy Postgresql which auto generates a password, and I was thinking of having a permanent fix for this by adding percent-encoding of password to both docker-entrypoint.sh for Zammad docker (it already has an encoding function that can be enhanced) and also changing the command for the containers to include password encoding before starting services. Let me know if this approach is acceptable and I will submit a PR.

hsahmed avatar Feb 23 '24 10:02 hsahmed

An alternative might be the linked pull request in Zammad. What do you think about it? We would then need to switch helm and docker-compose from passing DATABASE_URL to separate variables.

mgruner avatar Feb 23 '24 10:02 mgruner

As a short follow-up for this: so far there was no proper solution for this problem, because:

  • we cannot use the Helm lookup function to retrieve and process the value at installation time
  • we need to keep using secrets even if the configuration values are local for security reasons, and secrets cannot be processed but only directly used in the target system

However, I added a change to Zammad develop which would allow us to place custom pre-init code to the Zammad stack. With that we could be able to build the config values at runtime, by mounting a new initializer file. My plan is to look at this again later after the other changes for upcoming Zammad 6.3 have been addressed.

mgruner avatar Mar 11 '24 10:03 mgruner

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar May 11 '24 05:05 github-actions[bot]

.

mgruner avatar May 13 '24 04:05 mgruner

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jul 12 '24 05:07 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 14 '24 05:09 github-actions[bot]