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

Postgresql credentials should be percent encoded #251

Open
hsahmed opened this issue Feb 21, 2024 · 7 comments
Open

Postgresql credentials should be percent encoded #251

hsahmed opened this issue Feb 21, 2024 · 7 comments

Comments

@hsahmed
Copy link

hsahmed commented Feb 21, 2024

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

@mgruner
Copy link
Collaborator

mgruner commented Feb 22, 2024

@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
Copy link
Collaborator

mgruner commented Feb 23, 2024

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

@hsahmed
Copy link
Author

hsahmed commented Feb 23, 2024

@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.

@mgruner
Copy link
Collaborator

mgruner commented Feb 23, 2024

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
Copy link
Collaborator

mgruner commented Mar 11, 2024

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.

Copy link

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 github-actions bot added the stale label May 11, 2024
@mgruner
Copy link
Collaborator

mgruner commented May 13, 2024

.

@mgruner mgruner removed the stale label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants