Sync config files from nextcloud/docker repo; Add S3/Swift object storage config; Add S3 ci test; Add nextcloud.trustedDomains

[jessebot]

Description of the change

• Updates the following files to match the upstream nextcloud/docker config files:
  • charts/nextcloud/files/defaultConfigs/autoconfig.php.tpl
  • charts/nextcloud/files/defaultConfigs/redis.config.php.tpl
  • charts/nextcloud/files/defaultConfigs/reverse-proxy.config.php.tpl 🆕
  • charts/nextcloud/files/defaultConfigs/s3.config.php.tpl 🆕
  • charts/nextcloud/files/defaultConfigs/smtp.config.php.tpl
  • charts/nextcloud/files/defaultConfigs/swift.config.php.tpl 🆕
  • charts/nextcloud/files/defaultConfigs/upgrade-disable-web.config.php.tpl 🆕

Note

If you were already manually providing s3.config.php, reverse-proxy.config.php, swift.config.php, or upgrade-disable-web.config.php, you can disable us deploying a default file for you by setting the respective nextcloud.defaultConfigs.FILE to false. For instance, to disable the default s3.config.php, you would set nextcloud.defaultConfigs.s3.config.php=false.

• Adds nextcloud.objectStore.s3 to use S3 for primary storage:

  click me for new nextcloud.objectStore.s3 values

  nextcloud:
    objectStore:
      s3:
        enabled: false
        # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
        accessKey: ""
        # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
        secretKey: ""
        # use legacy auth method
        legacyAuth: false
        # s3 endpoint to use; only required if you're not using AWS
        host: ""
        # use TLS/SSL for S3 connections
        ssl: true
        # default port that can be changed based on your object store, e.g. for minio, you can use 9000
        port: "443"
        # this is the default in the nextcloud docs
        region: "eu-west-1"
  provokateurin marked this conversation as resolved.
        # required if using s3, the name of the bucket you'd like to use
        bucket: ""
        # object prefix in bucket
        prefix: ""
        # set to true if you are not using DNS for your buckets.
        usePathStyle: false
        # autocreate the bucket
        autoCreate: false
        # optonal parameter: you probably want to keep this as default
        storageClass: "STANDARD"
        # server side encryption key. learn more: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/primary_storage.html#s3-sse-c-encryption-support
        sse_c_key: ""
        # use an existingSecret for S3 credentials. If set, we ignore the following under nextcloud.objectStore.s3
        # endpoint, accessKey, secretKey
        existingSecret: ""
        secretKeys:
          # key in nextcloud.objectStore.s3.existingSecret to use for s3 endpoint
          host: ""
          # key in nextcloud.objectStore.s3.existingSecret to use for s3 accessKeyID
          accessKey: ""
          # key in nextcloud.objectStore.s3.existingSecret to use for s3 secretAccessKey
          secretKey: ""
          # key in nextcloud.objectStore.s3.existingSecret to use for the s3 bucket
          bucket: ""
          # key in nextcloud.objectStore.s3.existingSecret to use for the s3 sse_c_key
          sse_c_key: ""

• Adds nextcloud.objectStore.swift to use OpenStack Swift for primary storage:

  click me for new nextcloud.objectStore.swift values

  nextcloud:
    objectStore:
      ## options related to using Swift as a primary object storage 
      swift:
        enabled: false
        # swift user info
        user:
          domain: "Default"
          name: ""
          password: ""
        # swift project info
        project:
          name: ""
          domain: "Default"
        # The Identity / Keystone endpoint
        url: ""
        region: ""
        # optional on some swift implementations
        service: "swift"
        # the container to store the data in
        container: ""
        # autocreate container
        autoCreate: false

• adds a new ci test_case for enabling S3 as a primary object store

  • includes installing MinIO conditionally using the community MinIO helm chart
  • includes a k8s test job to upload a file conditionally

• adds nextcloud.trustedDomains as a helm parameter. Takes a list that, if set, we then template out to a space separated list. Otherwise, we still use our default templating for that parameter (which uses nextcloud.host)

• rename run-tests job to test since the job before it is just called lint and it keeps the check names from truncating as much in the following view (can't be longer than 30 characters):
  

Benefits

This lets users use both nextcloud.configs and still use the nextcloud/docker auto configuration via environment variables. Prior to this change, you had to use one or the other when it came to s3 as primary object storage.

Possible drawbacks

not sure 🤷 but always open to feedback :)

Applicable issues

• fixes feature: add nextcloud.objectStore.s3 section in the values.yaml #477
• fixes Why do we need these defaultConfigs? #69
• fixes Easy way to configure 'trusted_domains' in config.php #41
• fixes Feature: Separate value for NEXTCLOUD_TRUSTED_DOMAINS #425

Checklist

• I have read the CONTRIBUTING.md doc.
• 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

[jessebot]

Since it's taking a bit for my PR to be merged upstream, I'll try to spend some time on this later today to get it ready for merge without the encryption feature to start with.

Perhaps it also makes sense to have a nextcloud.objectStore.s3 section in the values.yaml like (maybe in a second PR as I'd need to play with the deployment template and add an s3 secret):

nextcloud:
  objectStore:
    s3:
      enabled: true
      # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
      accessKey: ""
      # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
      secretKey: ""
      # only required if you're not using AWS
      endpoint: ""
      # default port that can be changed based on your object store, e.g. for minio, you can use 9000
      port: "443"
      # this is the default in the nextcloud docs
      region: "eu-west-1"
      bucket: ""
      ssl: true
      # set to true if you are not using DNS for your buckets.
      usePathStyle: false
      existingSecret: ""

[jessebot]

I've removed the encryption key setting for now, so this could still be merged, but I haven't tested this, so it should still wait another week or two while I make sure it works without the encryption key.

[NikoKS]

any plan for this to be merged?

[NikoKS]

any blocker to this @jessebot

[jessebot]

No, I just was just really busy before. Let me take a look at the code now and see what's left to be done here :)

[jessebot]

gonna take a peak locally and push up some changes as nextcloud/docker#2151 was merged so we now have encryption as well :)

[jessebot]

This should be ready for review now :) Unless #480 gets merged first, then I need to update this PR again.

[provokateurin]

I'd rather wait for the other PR, it has been open so long already.

[jessebot]

Sounds good to me :)

[jessebot]

As discussed in #480, I will refactor this to use the new config file format and then we can review and merge.

[jessebot]

Actually, in the interest of resolving #477, should I just go ahead and add the following to the values?

nextcloud:
  objectStore:
    s3:
      enabled: true
      # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
      accessKey: ""
      # ignored if nextcloud.objectstore.s3.existingSecret is not empty string
      secretKey: ""
      # only required if you're not using AWS
      endpoint: ""
      # default port that can be changed based on your object store, e.g. for minio, you can use 9000
      port: "443"
      # this is the default in the nextcloud docs
      region: "eu-west-1"
      bucket: ""
      ssl: true
      # set to true if you are not using DNS for your buckets.
      usePathStyle: false
      existingSecret: ""

Open to suggestions and feedback on better organizing this, but it feels like we could accommodate this, rather than making users manually set the env vars?

[provokateurin]

How about adding the other missing configs (especially https://github.com/nextcloud/docker/blob/master/.config/upgrade-disable-web.config.php) and checking if the other ones are up-to-date?

[jessebot]

OK! This is finally ready for review, @provokateurin :)

[provokateurin]

Could you try to separate the changes into a few commits e.g.:

1. update existing config files
2. add new/missing config files
3. add s3 and swift values.yaml
4. add s3 test

It doesn't need to be exactly these commits, but if anything here breaks something down the road it will be a lot easier to bisect if it's not a single commit.

[provokateurin]

Very nice work btw ❤️

[jessebot]

Could you try to separate the changes into a few commits e.g.:

1. update existing config files

2. add new/missing config files

3. add s3 and swift values.yaml

4. add s3 test

It doesn't need to be exactly these commits, but if anything here breaks something down the road it will be a lot easier to bisect if it's not a single commit.

Sure, I can do that when I get home later. I assumed you'd ask for it to be in all one commit, so I did this to solve that, but also, since we typically do a squash and merge, does this mean you'd like me to do a normal merge commit after I finish teasing apart the commits? Happy to do it, just making sure :)

[provokateurin]

Well I prefer a clean history where every commit has a purpose and does only one thing. So neither fixup commits nor mega commits are great, just smallish concrete ones.

[jessebot]

OK, I think all the commits are reasonable now. It took me kind of a while to get it together, because splitting a commit can't really be done in any easy way, but like, we should be good now!

Oh, when merging, do you want to do a "merge commit" or "rebase and merge"?

[provokateurin]

Merge commit ;)

But honestly do what you find best, as long as it works for you 🤷‍♀️

[jessebot]

Okie dokie. I still need an approval, but then we can get this out the door :)

[provokateurin]

Thank you so much for all the care you put into this! Also reviewing the changes was super easy now with the split commits.