nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.
GNU Affero General Public License v3.0
295 stars 258 forks source link

fix: improve handling of config file #480

Open wrenix opened 7 months ago

wrenix commented 7 months ago

Split of #478

Move all the configfiles for nginx, nextcloud, htaccess out of the kubernetes-resources as single file beside the helmchart and load them with helm function into the kubernetes-resources.

benefits:

jessebot commented 7 months ago

Please detail in the description of this PR what you're doing and why to make it easier to review. Ideally, please use the PR template.

wrenix commented 7 months ago

Done

wrenix commented 7 months ago

@jessebot do you like to review?

wrenix commented 7 months ago

rebased after https://github.com/nextcloud/helm/pull/393 was merged

ci lint says:

Update Complete. ⎈Happy Helming!⎈
Saving 3 charts
Downloading postgresql from repo oci://registry-1.docker.io/bitnamicharts
Downloading mariadb from repo oci://registry-1.docker.io/bitnamicharts
Downloading redis from repo oci://registry-1.docker.io/bitnamicharts
Deleting outdated charts
Pulled: registry-1.docker.io/bitnamicharts/postgresql:12.12.10
Digest: sha256:179d5c6d017298bb9ab868321bcacb1a9efe5eb8224902f89f8693c69a72e428
Pulled: registry-1.docker.io/bitnamicharts/mariadb:12.2.9
Digest: sha256:834d78c385839bac4a7ec8cd0d25adea6b9a3324ef8c7e284e59d9e33c1e96b6
Pulled: registry-1.docker.io/bitnamicharts/redis:17.13.2
Digest: sha256:b7892f0842f1758bb35c61aaccdbb2ca30a7ff234477a2b270de31db8c0920e0
Linting chart "nextcloud => (version: \"4.5.3\", path: \"charts/nextcloud\")"
Checking chart "nextcloud => (version: \"4.5.3\", path: \"charts/nextcloud\")" for a version bump...
Old chart version: 4.4.0
New chart version: 4.5.3
Chart version ok.
Validating /home/wrenix/src/github.com/nextcloud/helm/charts/nextcloud/Chart.yaml...
Validation success! 👍
Validating maintainers...

Linting chart with values file "charts/nextcloud/ci/ct-all-enabled-values.yaml"...

==> Linting charts/nextcloud

1 chart(s) linted, 0 chart(s) failed

Linting chart with values file "charts/nextcloud/ci/ct-empty-values.yaml"...

==> Linting charts/nextcloud

1 chart(s) linted, 0 chart(s) failed

Linting chart with values file "charts/nextcloud/ci/ct-specials-values.yaml"...

==> Linting charts/nextcloud

1 chart(s) linted, 0 chart(s) failed

------------------------------------------------------------------------------------------------------------------------
 ✔︎ nextcloud => (version: "4.5.3", path: "charts/nextcloud")
------------------------------------------------------------------------------------------------------------------------
All charts linted successfully
wrenix commented 6 months ago

rebased and version bump again

jessebot commented 6 months ago

It looks like there's an error after the nginx default config PR was merged.

https://github.com/nextcloud/helm/actions/runs/7144375839/job/19639651937?pr=480#step:9:219

wrenix commented 6 months ago

rebased again and set an valid nginx.config.custom in charts/nextcloud/ci/ct-specials-values.yaml.

wrenix commented 6 months ago

rebased and version dump again

wrenix commented 4 months ago

rebased (after #520) and version dump again

wrenix commented 4 months ago

huhu @provokateurin do you like to take a look here too?

wrenix commented 4 months ago

This is the last for nd importend PR from my big bunch ;) Thanks so much till here.

provokateurin commented 4 months ago

I'll have a look soon

wrenix commented 3 months ago

I do not know, why it / i close this

wrenix commented 2 months ago

Any update?

I found a not good default value for nginx, which i like to improve (and then i good a merge conflict)

provokateurin commented 2 weeks ago

Hi and sorry for the super long delay. In principle I agree with the changes, I just find it hard to review as it is not so easy to verify nothing in the config file was actually changed. I'll try to give it a review soon.