Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nextcloud: add option to set datadir and extensions #119638

Merged
merged 6 commits into from
Oct 13, 2021

Conversation

beardhatcode
Copy link
Contributor

@beardhatcode beardhatcode commented Apr 16, 2021

Motivation for this change

I want to manage my Nextcloud extensions trough nix and move extensions out of the datadir.

This moves the data for apps managed by nix to a nix-apps folder

Feedback is welcome, my config is:

let  mkNCApp = pkgs.fetchNextcloudApp;
in {
        services.nextcloud = {
          enable = true;
          hostName = nextcloudSub;
          package = nc;
          webfinger = true;
          https = true;
          datadir = "/mnt/datafile";
          extraApps = {
            maps =
              mkNCApp rec {
                name = "maps";
                version = "0.1.8";
                url = "https://github.com/nextcloud/maps/releases/download/v${version}/maps-${version}.tar.gz";
                sha256 = "d7ffb425fa0f11d2c3b7cad857eca258c94d635d77663fe524a6e6bff5416c41";
              };
            calendar =
              mkNCApp rec {
                name = "calendar";
                version = "2.1.2";
                url = "https://github.com/nextcloud/calendar/releases/download/v${version}/calendar.tar.gz";
                sha256 = "rt3pPdDFnd67K5K/zlScUkEN/+fOTgbNTR6hbA2sQuc=";
              };
            tasks =
              mkNCApp rec {
                name = "tasks";
                version = "0.13.6";
                url = "https://github.com/nextcloud/tasks/releases/download/v${version}/tasks.tar.gz";
                sha256 = "mLvnvp4DFa8kdXLabvLG9n7hOHBlymmlaYjuCnc2fxE=";
              };
            contacts =
              mkNCApp rec {
                name = "contacts";
                version = "3.4.2";
                url = "https://github.com/nextcloud/contacts/releases/download/v${version}/contacts.tar.gz";
                sha256 = "y3JCbAu204uxmqugkbZm+9F8J2g7WtyCh+lmlu53+Uw=";
              };
            notes =
              mkNCApp rec {
                name = "notes";
                version = "4.0.2";
                url = "https://github.com/nextcloud/notes/releases/download/v${version}/notes.tar.gz";
                sha256 = "LxJ3J0n8Bvan5sWNvBZiD//PPuNNnmZ8rAn/Pnj4WKs=";
              };
            previewgenerator =
              mkNCApp rec {
                name = "previewgenerator";
                version = "3.1.0";
                url = "https://github.com/rullzer/previewgenerator/releases/download/v${version}/previewgenerator.tar.gz";
                sha256 = "TdDdzPPuqRqaLqZB0aq4BzX7grX6xuaAlj15/9zd63Q=";
              };

          };
          caching = {
            apcu = true;
          };
          phpOptions = {
            catch_workers_output = "yes";
            display_errors = "stderr";
            error_reporting = "E_ALL & ~E_DEPRECATED & ~E_STRICT";
            expose_php = "Off";
            "opcache.enable_cli" = "1";
            "opcache.fast_shutdown" = "1";
            "opcache.interned_strings_buffer" = "8";
            "opcache.max_accelerated_files" = "10000";
            "opcache.memory_consumption" = "128";
            "opcache.revalidate_freq" = "1";
            short_open_tag = "Off";
          };
          poolSettings = {
            pm = "dynamic";
            "pm.max_children" = "120";
            "pm.max_requests" = "500";
            "pm.max_spare_servers" = "18";
            "pm.min_spare_servers" = "6";
            "pm.start_servers" = "12";
          };
          config = {
            dbtype = "pgsql";
            dbuser = "nextcloud";
            dbhost = "/run/postgresql"; # nextcloud will add /.s.PGSQL.5432 by itself
            dbname = "nextcloud";
            adminpass = "wasabi4001isacoolpasswordbutyoushouldpickabetterone";
            adminuser = "root";
            trustedProxies = [ hostIP ];
          };
        };
}
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 16, 2021
@chvp
Copy link
Member

chvp commented Apr 16, 2021

It might be nice to provide a builder like the mkNCApp from your config in nixpkgs.

@beardhatcode
Copy link
Contributor Author

@chvp where should such a builder go?

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Apr 16, 2021
@chvp
Copy link
Member

chvp commented Apr 16, 2021

Probably in a file next to nextcloud's default.nix? I'm not entirely sure to be honest.

@beardhatcode
Copy link
Contributor Author

@Ma27, do you have opinions on this?

@Ma27
Copy link
Member

Ma27 commented Apr 29, 2021

Idea seems OK on a first look. Have you checked how Nextcloud behaves if you try to add or update apps that were installed with Nix?

@beardhatcode
Copy link
Contributor Author

Yes, I updated the calender app this morning and installed phonetrack last month.
But it is true that I need to check how this behaves if there are migrations (if there even is such a thing).

@beardhatcode
Copy link
Contributor Author

I'm calling ${occ}/bin/nextcloud-occ upgrade after the nix based Nextcloud apps are put in place, that should carry out any required migrations.

https://github.com/NixOS/nixpkgs/blob/ad0e917fef78838548430f9a35bd3e2dd8fa03d1/nixos/modules/services/web-apps/nextcloud.nix#L591

@Ma27
Copy link
Member

Ma27 commented May 2, 2021

Sounds OK then. I hope you understand that I'd still like to test all of that on my own as maintainer of this module. I'll try to do it today or tomorrow. In case I forget about this, don't hesitate to ping me :)

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look mostly good, just a few notes below.

Regarding the implementation itself, I'm afraid I misunderstood how to use this. I just wrote a VM expression like this:

{
  nextcloud = { lib, pkgs, config, ... }: {
    networking.firewall.allowedTCPPorts = [ 80 ];
    services.nextcloud = {
      enable = true;
      hostName = "nextcloud";
      config.adminpass = "foobar";
      datadir = "/var/lib/nextcloud";
      extraApps.maps = pkgs.fetchNextcloudApp rec {
        name = "maps";
        version = "0.1.8";
        url = "https://github.com/nextcloud/maps/releases/download/v${version}/maps-${version}.tar.gz";
        sha256 = "d7ffb425fa0f11d2c3b7cad857eca258c94d635d77663fe524a6e6bff5416c41";
      };
    };
  };
}

(I built it via NIX_PATH=nixpkgs=$(pwd) nixos-build-vms vm.nix and then ran QEMU_NET_OPTS="hostfwd=tcp::8080-:80" ./result/bin/nixos-run-vms to access it in my browser on localhost:8080).

From what I've seen, this should make the Maps app available when I login, correct? Unfortunately the top-menu only gives me Files/Folders/Activity, but no Maps. To be sure, I tried to enable it in the administrator panel. When I hit Enable for the (now-available) Maps app, I get the error The "unique" column option is not supported.. In the logs, the following message appears:

nextcloud # [   91.528339] Nextcloud[907]: {"reqId":"OYqtGMnPVHwRZRwJKLT9","level":3,"time":"2021-05-02T17:42:04+00:00","remoteAddr":"10.0.2.2","user":"root","app":"settings","method":"POST","url":"/settings/apps/enable","message":"{\"Exception\":\"Doctrine\\\\DBAL\\\\Schema\\\\Exception\\\\UnknownColumnOption\",\"Message\":\"The \\\"unique\\\" column option is not supported.\",\"Code\":0,\"Trace\":[{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/3rdparty/doctrine/dbal/src/Schema/Column.php\",\"line\":84,\"function\":\"new\",\"class\":\"Doctrine\\\\DBAL\\\\Schema\\\\Exception\\\\UnknownColumnOption\",\"type\":\"::\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/3rdparty/doctrine/dbal/src/Schema/Column.php\",\"line\":68,\"function\":\"setOptions\",\"class\":\"Doctrine\\\\DBAL\\\\Schema\\\\Column\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/3rdparty/doctrine/dbal/src/Schema/Table.php\",\"line\":355,\"function\":\"__construct\",\"class\":\"Doctrine\\\\DBAL\\\\Schema\\\\Column\",\"type\":\"->\"},{\"file\":\"/nix/store/firwnx70whrqfycdn92iiks5pm428qac-nc-app-maps/lib/Migration/Version000009Date20190625000800.php\",\"line\":43,\"function\":\"addColumn\",\"class\":\"Doctrine\\\\DBAL\\\\Schema\\\\Table\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/DB/MigrationService.php\",\"line\":455,\"function\":\"changeSchema\",\"class\":\"OCA\\\\Maps\\\\Migration\\\\Version000009Date20190625000800\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/DB/MigrationService.php\",\"line\":418,\"function\":\"migrateSchemaOnly\",\"class\":\"OC\\\\DB\\\\MigrationService\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/Installer.php\",\"line\":163,\"function\":\"migrate\",\"class\":\"OC\\\\DB\\\\MigrationService\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/apps/settings/lib/Controller/AppSettingsController.php\",\"line\":448,\"function\":\"installApp\",\"class\":\"OC\\\\Installer\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/AppFramework/Http/Dispatcher.php\",\"line\":218,\"function\":\"enableApps\",\"class\":\"OCA\\\\Settings\\\\Controller\\\\AppSettingsController\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/AppFramework/Http/Dispatcher.php\",\"line\":127,\"function\":\"executeController\",\"class\":\"OC\\\\AppFramework\\\\Http\\\\Dispatcher\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/AppFramework/App.php\",\"line\":157,\"function\":\"dispatch\",\"class\":\"OC\\\\AppFramework\\\\Http\\\\Dispatcher\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/private/Route/Router.php\",\"line\":302,\"function\":\"main\",\"class\":\"OC\\\\AppFramework\\\\App\",\"type\":\"::\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/lib/base.php\",\"line\":993,\"function\":\"match\",\"class\":\"OC\\\\Route\\\\Router\",\"type\":\"->\"},{\"file\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/index.php\",\"line\":37,\"function\":\"handleRequest\",\"class\":\"OC\",\"type\":\"::\"}],\"File\":\"/nix/store/kk42k7xk0mc4d42z2j0hy2c3s035kgf7-nextcloud-21.0.1/3rdparty/doctrine/dbal/src/Schema/Exception/UnknownColumnOption.php\",\"Line\":18,\"CustomMessage\":\"--\"}","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0","version":"21.0.1.1"}

Now I'm wondering, did I do anything wrong here?

Last but not least, we should document this new feature in nixos/modules/services/web-apps/nextcloud.xml. I know a lot of people don't like docbook, so as soon as everything else is done, I can take care of this as well :)

nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
@beardhatcode
Copy link
Contributor Author

beardhatcode commented May 15, 2021

I've been trying to find out the cause for this issue, @Ma27.

After a lot of looking, it turns out that it is a bug in the maps app ( nextcloud/maps#541 ) if you are working with SQLLite (my tests I've been using Postgres.)

This vm.nix works for me: (only change is the bumped version of maps).

{
  nextcloud = { lib, pkgs, config, ... }: {
    networking.firewall.allowedTCPPorts = [ 80 ];
    services.nextcloud = {
      enable = true;
      hostName = "nextcloud";
      config.adminpass = "foobar";
      datadir = "/var/lib/nextcloud";
      extraApps.maps = pkgs.fetchNextcloudApp rec {
        name = "maps";
        version = "0.1.9-8-nightly";
        url = "https://github.com/nextcloud/maps/releases/download/v${version}/maps-${version}.tar.gz";
        sha256 = "sha256-3uLg1rvSDcgQgUvqhNsu1v08+72jIzvyo20ckyBWJ6w=";
      };
    };
  };
}

You should still enable the app through the UI.

Thank you for the code review, I'll try to address the concerns you raised.

@beardhatcode beardhatcode force-pushed the feat/nextcloud-packages branch from ad0e917 to f683704 Compare May 15, 2021 21:55
@Kranzes
Copy link
Member

Kranzes commented Jul 22, 2021

What is the state of this PR? I am unable to test it at the moment, I'd like to know from your experience.

@Ma27
Copy link
Member

Ma27 commented Jul 22, 2021

I'm not really sure what the state is either, but it seems readier than I had in mind, so I can start a second attempt to test soonish.

@Kranzes
Copy link
Member

Kranzes commented Jul 22, 2021

Sounds good!

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Tested it in a VM with maps-0.1.9 which works perfectly fine 👍
  • It would be nice if you could document this in nixos/modules/services/web-apps/nextcloud.xml. It may be especially confusing that you have to enable these declarative modules manually in the admin-interface.

nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, mostly.

First of all, thanks a lot for your hard work on this!

To get this merged, the following things should be done:

  • please write some documentation about how this works (and that apps have to be enabled by an administrator before being able to use them) into nixos/modules/services/web-apps/nextcloud.xml. If you need help with docbook, just ping me! :)
  • Please fix the remaining review-notes.
  • Please squash the commits into logical junks and make sure that the commit-messages adhere our contribution-guidelines. Also, please remove my GH handle from the messages as these cause unnecessary notifications for me whenever someone pushes them (or rebases a branch).

nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
@Ma27
Copy link
Member

Ma27 commented Aug 4, 2021

@beardhatcode do you intend to continue working on this? :)

@beardhatcode
Copy link
Contributor Author

Yes, I do as I'm still using it, but I don't have much time at the moment.

@beardhatcode beardhatcode force-pushed the feat/nextcloud-packages branch from 9b53dae to 1d7a617 Compare August 10, 2021 09:29
@Ma27
Copy link
Member

Ma27 commented Sep 7, 2021

@beardhatcode what's the status here? :)

@beardhatcode
Copy link
Contributor Author

I don't have much time to work on this. Currently, my plan is to finish this PR on 25 September 😅

@beardhatcode beardhatcode force-pushed the feat/nextcloud-packages branch 2 times, most recently from 4c5bbde to 4957ada Compare September 25, 2021 20:21
@beardhatcode beardhatcode requested a review from Ma27 September 25, 2021 20:22
@beardhatcode beardhatcode force-pushed the feat/nextcloud-packages branch 2 times, most recently from b02f86e to 5c17483 Compare September 25, 2021 20:29
@beardhatcode
Copy link
Contributor Author

beardhatcode commented Sep 25, 2021

@Ma27 , I finished this up today and squashed the commits. Could you look at this once more?

I also added an option to disable the appstore. It is enabled by default when the extraApps feature is used and prevents Nextcloud from updating these apps.

I think some test might also be needed but I don't know how to do that.

Copy link
Member

@nlewo nlewo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @beardhatcode ! That's pretty cool!

We actually have this feature on a shared infra but we didn't find/take time to upstream it. So, i'm glad you are upstreaming this!

It would be really nice to enrich the Nextcloud NixOS test. I think we could add a simple app (such as the calendar one) and do a basic test to ensure it is working as expected (see here for instance). Do not hesitate to ping me (lewo on Matrix/IRC) if you need help on that topic.

pkgs/build-support/fetchnextcloudapp/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Show resolved Hide resolved
@beardhatcode
Copy link
Contributor Author

@nlewo I was thinking of an even simpler app that would just expose some unprotected endpoint and could be compactly written in the test file.

nixos/modules/services/web-apps/nextcloud.xml Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
This option can be used to set an alternative storage location for
files and app metadata.
Note the appstoreEnable which will prevent nextcloud form updating
nix-managed apps. This is needed because nextcloud will store an other
version of the app in /var/lib/nextcloud/store-apps and it will
no longer be manageable.
@Ma27 Ma27 force-pushed the feat/nextcloud-packages branch from 15059a2 to 1852212 Compare October 9, 2021 18:33
@Ma27
Copy link
Member

Ma27 commented Oct 9, 2021

Rebased onto latest master. Will resolve my latest notes now and test locally, if everything's fine, I'd merge.

Ma27 added 3 commits October 9, 2021 20:52
A few minor changes to get NixOS#119638 - nextcloud: add option to set
datadir and extensions - ready:

* `cfg.datadir` now gets `cfg.home` as default to make the type
  non-nullable.
* Enhanced the `basic` test to check the behavior with a custom datadir
  that's not `/var/lib/nextcloud`.
* Fix hashes for apps in option example.
* Simplify if/else for `appstoreenable` in override config.
* Simplify a few `mapAttrsToList`-expressions in
  `nextcloud-setup.service`.
@Ma27
Copy link
Member

Ma27 commented Oct 9, 2021

@beardhatcode @nlewo I pushed a few changes and tested it locally, LGTM now. Please re-review, if everything's fine from your PoV, I'd merge :)

@beardhatcode
Copy link
Contributor Author

Awesome @Ma27 (I cannot approve my own PR, but I approve your changes)

@Ma27 Ma27 merged commit eb3a372 into NixOS:master Oct 13, 2021
@Kranzes
Copy link
Member

Kranzes commented Oct 13, 2021

Huge thanks to everyone who contributed to this PR! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: fetch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants