search

nextcloud / helm

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

Feature: Add configuring database port via helm parameters maybe #632

Closed jessebot closed 1 month ago

jessebot commented 2 months ago

Description of the change

We should support configuring the database port for external databases. When I looked further, it doesn't look like port is included here in values.yaml: https://github.com/nextcloud/helm/blob/0565cdba9843040bc7e2641b8ac6955d2feba9ae/charts/nextcloud/values.yaml#L354-L382

we'd need to also update _helpers.tpl: https://github.com/nextcloud/helm/blob/0565cdba9843040bc7e2641b8ac6955d2feba9ae/charts/nextcloud/templates/_helpers.tpl#L75-L169

Would also need to update deployment.tpl (both for the is_ready initContainer and the main nextcloud container): https://github.com/nextcloud/helm/blob/0565cdba9843040bc7e2641b8ac6955d2feba9ae/charts/nextcloud/templates/deployment.yaml#L295-L336

Finally, and this is the most interesting part, we'd need to update autoconfig.php.tpl: https://github.com/nextcloud/helm/blob/0565cdba9843040bc7e2641b8ac6955d2feba9ae/charts/nextcloud/files/defaultConfigs/autoconfig.php.tpl

Read possible drawbacks for why this is titled with maybe haha

Benefits

For both MariaDB/PostgreSQL, you can set the database ports: For mariadb, you'd want mariadb.primary.containerPorts.mysql. For postgresql, you'd want postgresql.containerPorts.postgresql

Possible drawbacks

If we update autoconfig.php.tpl, we should also update that in nextcloud/docker: https://github.com/nextcloud/docker/blob/master/.config/autoconfig.php

But then I realized... is this even supported in nextcloud/server? https://docs.nextcloud.com/server/latest/admin_manual/configuration_database/linux_database_configuration.html

I see mysql port briefly mentioned in the php section here: https://docs.nextcloud.com/server/latest/admin_manual/configuration_database/linux_database_configuration.html#configuring-a-mysql-or-mariadb-database

I didn't have time to look into nextcloud/server and see if this has been brought up, but changing the default postgresql/mysql port is often a security through obscurity tactic, to avoid script kiddies finding your server (if it is public for some reason), so it seems like a common request...

Additional information

This was originally brought up in https://github.com/nextcloud/helm/discussions/618.

Started the discussion in nextcloud/docker here: https://github.com/nextcloud/docker/issues/2300

jessebot commented 1 month ago

So it looks like we don't need to do anything as in https://github.com/nextcloud/docker/issues/2300 josh pointed out that you can specify port as part of the database host like if the custom port is 1234 the dbhost can be set to hostname:1234 as documented here. I think the only thing needed for this ticket would be to just put this into our README.md and maybe a comment in the values.yaml, but other than that, seems fine to me :)