Update _helpers.tpl env vars that use secret keys

[bastikus]

Returned default values for the external database

Pull Request

Description of the change

Helm chart does not work when using an external database for NextCloud. "key" is not defined.

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

[provokateurin]

This seems wrong, you should simply specify the key values as they are mandatory and defaults are not going to work either because every setup is different.

[bastikus]

This seems wrong, you should simply specify the key values as they are mandatory and defaults are not going to work either because every setup is different.

Default values are indeed defined in values.yaml for external database, but secret for external database is not created in db-secret.yaml, nor are these values used in any other manifests.

[provokateurin]

Hm I think having default values in the values.yaml for those fields makes no sense because everyone has to override them anyway. @jessebot what do you think?

[jessebot]

Sorry for the huge delay! I meant to come back to this and just did not 🤦

Anyway! Here's my ideal proposal long term to simplify this:

## Proposed values.yaml example 1

externalDatabase:
  enabled: false

  ## Supported database engines: mysql or postgresql
  type: mysql

  ## Database host
  host:

  ## Database user
  user: nextcloud

  ## Database password
  password: ""

  ## Database name
  database: nextcloud

  ## Name of a Kubernetes existing secret to use.
  existingSecret: ""
  # keys in externalDatabase.existingSecret to use for database credentials
  secretKeys:
    username: ""
    password: ""
    host: ""
    database: ""

The above codeblock I've posted would set all the default values to empty string "" and remove the need for the externalDatabase.existingSecret.enabled value. If externalDatabase.enabled is true and externalDatabase.existingSecret is set to anything that is not an empty string, we use the existing Kubernetes Secret for any of the values that are set under externalDatabase.secretKeys. If any of the secretKeys are empty, we default back to the plain text equivalent.

Here's example codeblock 2, which can still allow for doing the first example in a future PR, but gets this stuff cleaned up faster in this PR:

## Proposed values.yaml example 2

externalDatabase:
  enabled: false

  ## Supported database engines: mysql or postgresql
  type: mysql

  ## Database host
  host:

  ## Database user
  user: nextcloud

  ## Database password
  password: ""

  ## Database name
  database: nextcloud

  ## Use a existing secret
  existingSecret:
    enabled: false
    # this can still be set to empty string, because we still have the enabled parameter here
    secretName: ""
    usernameKey: ""
    passwordKey: ""
    hostKey: ""
    databaseKey: ""

Either way, I agree that we should not have default secret keys, as they have the potential to be wrong or unused. Would you like me to submit that PR? Do we still want to merge this one in the meantime?

[jessebot]

Oh, if this is to be merged, @bastikus please also update the version in Chart.yaml 🙏

[provokateurin]

Either way, I agree that we should not have default secret keys, as they have the potential to be wrong or unused. Would you like me to submit that PR? Do we still want to merge this one in the meantime?

Yeah a PR would be nice, it's probably the same for other configs as well. Unfortunately this will be a breaking change :/