fix: after #465 (remove before <?php, indent of extraVolumes and podLabels)

[wrenix]

Fixes:

• "remove everthing before <?php":
  • chore: lint all templates to fix indentation and update style #465 (comment)
  • [bug] extra newline in config files  #475
• indent of extraVolumes: [bug] extra newline in config files  #475 (comment)
• inside of with using .Values (e.g. podLabels)

Additional information

Checklist

• DCO has been signed off on the commit.
• Chart version bumped in Chart.yaml according to semver.
• (optional) Variables are documented in the README.md

[wrenix]

@DanishVaid could you verify, that this patch solves that problem?
(with fluxcd it should be easy to take an branch/commitid from my fork-repo)

[jessebot]

@wrenix thanks for popping back in to fix this quickly 💙

[jessebot]

I'll approve the workflow run after the comments are addressed, but thanks again for quickly taking note of the bug :)

Also, before I forget, you can test it by adding a custom config and then doing a helm template . from the helm chart directory of your nextcloud fork's patch branch. If you want to specify custom values, you can use --values myvalues.file.yaml and if you want to specify a directory to write the rendered templates out to for checking, you can use --output-dir mytestdir.

[wrenix]

you are correct, after a second look i start to cleanup more - so i split it again and make more with #478.

i know for helm template command, but i do not deploy it - so the bug is more a php problemen then an helm.
We should setup chart-testing, that try a deployen with kind and maintaine a example value.yaml file which enable all features in this helmchart

[DanishVaid]

Hmm, I'm trying to test but seeing an odd error during chart rendering (shown below). I don't think this is tied to your change. Seeing if I can figure out what it is

Error: template: test/charts/postgresql/templates/primary/svc.yaml:11:14: executing "test/charts/postgresql/templates/primary/svc.yaml" at <include "common.labels.standard" (dict "customLabels" .Values.commonLabels "context" $)>: error calling include: template: test/charts/mariadb/charts/common/templates/_labels.tpl:11:27: executing "common.labels.standard" at <include "common.names.name" .>: error calling include: template: test/charts/mariadb/charts/common/templates/_names.tpl:11:18: executing "common.names.name" at <.Chart.Name>: nil pointer evaluating interface {}.Name helm.go:84: [debug] template: test/charts/postgresql/templates/primary/svc.yaml:11:14: executing "test/charts/postgresql/templates/primary/svc.yaml" at <include "common.labels.standard" (dict "customLabels" .Values.commonLabels "context" $)>: error calling include: template: test/charts/mariadb/charts/common/templates/_labels.tpl:11:27: executing "common.labels.standard" at <include "common.names.name" .>: error calling include: template: test/charts/mariadb/charts/common/templates/_names.tpl:11:18: executing "common.names.name" at <.Chart.Name>: nil pointer evaluating interface {}.Name

[wrenix]

@DanishVaid i am really interestest in your value.yaml / environment.

it look like it has nothing todo with this or #465 - that looks like an subchart problem (where you enable both, mariadb and postgresql at the same time? that does not look like the problem here, it is an recusive template part down to the sub-sub-chart and there the access to .Chart.Name)

But maybe i use a {{ template "" . }} inside of a with somewhere (instatt of {{ template "" $ }})

[DanishVaid]

Ah, the issue was definitely on my end. I spun up an extra deployment to ensure my main one wouldn't go down (being used by friends and family) and forgot to copy the base values so only my overlay was being applied. Can confirm that this PR fixes the issue I was seeing with the PHP config (new rendered configmaps shown below):

[wrenix]

No worry, i take another look and find another bug (see podLabel commit), so thanks to get me spending timd to reviewe my own changes again xD

PS: if you like, you could review #478 also - there i refactory the main logic for defaultConfigs (in hopefull a better way, that such nindent in php by using helm wrong could not happen again so easy) ...

[jessebot]

i know for helm template command, but i do not deploy it - so the bug is more a php problemen then an helm.

That's fair. Since helm template didn't catch it, perhaps you should also install kind and ct locally? I added a PR here, #479, for updating the contributing docs

We should setup chart-testing, that try a deployen with kind and maintaine a example value.yaml file which enable all features in this helmchart

Chart testing is set up here: https://github.com/nextcloud/helm/blob/main/.github/workflows/lint-test.yaml

We could potentially add a ci/test-values.yaml and it would be picked up according to the docs:

Charts may have multiple custom values files matching the glob pattern '*-values.yaml' in a directory named 'ci' in the root of the chart's directory. The chart is installed and tested for each of these files. If no custom values file is present, the chart is installed and tested with defaults.

I'd review a PR for that, but perhaps it needs more discussion as we have a few conflicting options, for instance mysql vs postgresql vs sqlite for the database. We'd maybe need a couple of test values files, and I'm not sure off the top of my head how that works if there's multiple. We'd need to do some testing.

We could add more steps to test different configurations, but there is also this PR for helm tests:
#462

[jessebot]

Please either use syntax highlighted code blocks or add alt text to your images so that users that utilize screen readers or other accessibility tools can also read your post.

[jessebot]

I'm testing this fix locally and will report back after that.

dimissing stale review as this isn't ready for approval yet, but all changes were fixed

[jessebot]

ok, this appears to be working from testing on my end :)

Here's Argo CD ApplicationSet I used to test:
https://github.com/small-hack/argocd-apps/blob/61ddf523c9f02386fc1fdc08c96efdd6bc0e49aa/nextcloud/app_of_apps/nextcloud_argocd_appset.yaml