[nextcloud] installer does not properly escape special characters in passwords

Quba1 commented 7 months ago

chart Name

nextcloud

Operating System

TrueNAS SCALE 22.12.4.2 or prior

Deployment Method

TrueNAS SCALE charts

Chart Version

29.10.22

Kubernetes Events

were not possible to obtain

chartlication Logs

were not possible to obtain

Chart Configuration

Related default config changes:

set admin password to string starting with %
set postgres password to string starting with %

Unrelated default config changes:

set admin username to admin
configured ingress with host, traefik and cert-manager
set phone code

I consider those changes as unrelated because in a working configuration those settings remained identical.

Describe the bug

During first attempt of chart installation I used randomly generated passwords for admin account and Postgres and this error appeared:

[EFAULT] Failed to install App: Chart - Values contain an error that may be a result of merging.

with two errors in config printed below:

configmap:
  Error: 'error converting YAML to JSON: yaml: line 79: found character that cannot
    start any token'

...
  cnpg-main-user:
    Error: 'error converting YAML to JSON: yaml: line 6: found character that cannot
      start any token'

After some googling I learned that YAML is not happy with tabs and % sign at the line beginning. And the only string that I had control over was the two passwords. So I checked them and discovered that accidentally they both start with %.

In the minimal example (see above) I was able to confirm that if the passwords start with % error occurs, but when they start with a letter everything works.

I did not investigate whether one or both passwords are improperly handled.

Part of the issue is also the fact that those errors are hidden inside three walls of texts:

One line with 54790 characters
Python traceback with config printed below and actual errors hidden in the config
2793 line long config with errors hidden in it

Thus the solving process required a fair amount of text searching, deduction and trail-and-error.

To Reproduce

Install nextcloud chart following guide on the website and with config changes as above, for admin and postgres passwords use strings starting with %.

Expected Behavior

Ideal: Passwords are properly escaped and charts installs correctly. Sufficient: Display a readable error that passwords must start with a letter.

Screenshots

n/a

Additional Context

n/a

I've read and agree with the following

[X] I've checked all open and closed issues and my issue is not there.
[X] I've prefixed my issue title with [Chart-Name]

PrivatePuffin commented 7 months ago

Sorry we cannot adapt much in the way of how SCALE handles Helm errors

I'm not sure if we are every going to 100% guarantee correct escaping on all charts though.

Quba1 commented 7 months ago

I'm not sure if we are every going to 100% guarantee correct escaping on all charts though.

I think an already useful solution would be to write a warning in Installation Notes or the config page itself

PrivatePuffin commented 7 months ago

Thats correct.

stavros-k commented 7 months ago

It’s technically a helm template issue**, that strips quotes every time you pass the value down. While it could be fixed, it’s likely to happen in 10 other places. And the mitigation for it requires extra code on each place that the value is passed by. So I’ll also agree with the docs.

**(eg even on normal helm usage, even if you add quotes yourself, it will still fail)

PrivatePuffin commented 7 months ago

@bitpushr Docs are your thing

bitpushr commented 7 months ago

Caution/note for password requirements added to installation docs with PR: https://github.com/truecharts/charts/pull/21271

@Quba1 thanks for raising this!

truecharts-admin commented 1 month ago

This issue is locked to prevent necro-posting on closed issues. Please create a new issue or contact staff on discord of the problem persists

truecharts / public