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

nixos/nextcloud: write config to additional config file #63900

Merged
merged 1 commit into from Jul 22, 2019

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jun 28, 2019

Motivation for this change

One of the main problems of the Nextcloud module is that it's currently
not possible to alter e.g. database configuration after the initial
setup as it's written by their imperative installer to a file.

After some research[1] it turned out that it's possible to override all values
with an additional config file. The documentation has been
slightly updated to remain up-to-date, but the warnings should
remain there as the imperative configuration is still used and may cause
unwanted side-effects.

Also simplified the postgresql test which uses ensure{Databases,Users} to
configure the database.

Fixes #49783

[1] #49783 (comment)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ma27
Copy link
Member Author

Ma27 commented Jun 28, 2019

I checked in a VM whether config changes actually work and confirmed several cases like invalid dbpass files and deployed this to my personal Nextcloud after that to check this with an existing environment and to ensure that this doesn't break any existing setups.

However it would be great if more people could take a look at this :)

@GrahamcOfBorg test nextcloud

nixos/modules/services/web-apps/nextcloud.xml Show resolved Hide resolved
You can activate auto updates for your apps via
<literal><link linkend="opt-services.nextcloud.autoUpdateApps.enable">services.nextcloud.autoUpdateApps</link></literal>.
</para>
and can cause unwanted side-effects!</para>
Copy link
Contributor

Choose a reason for hiding this comment

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

You could link to #49783 as a place for discussion in case anyone has questions.

@turion
Copy link
Contributor

turion commented Jun 30, 2019

One thing I don't understand is whether existing nextcloud installations will migrate smoothly to your system, or whether there is manual work to be done. In case that is so, it would be good to document that somewhere. Maybe in a wiki article or in the issue #49783.

@Ma27
Copy link
Member Author

Ma27 commented Jun 30, 2019

One thing I don't understand is whether existing nextcloud installations will migrate smoothly to your system, or whether there is manual work to be done.

I already deployed the change onto my system and I didn't encounter any issues. Actually this simply persists stuff like database configuration in an additional configuration file to ensure that it's actually possible to change this after the first install.

One of the main problems of the Nextcloud module is that it's currently
not possible to alter e.g. database configuration after the initial
setup as it's written by their imperative installer to a file.

After some research[1] it turned out that it's possible to override all values
with an additional config file. The documentation has been
slightly updated to remain up-to-date, but the warnings should
remain there as the imperative configuration is still used and may cause
unwanted side-effects.

Also simplified the postgresql test which uses `ensure{Databases,Users}` to
configure the database.

Fixes NixOS#49783

[1] NixOS#49783 (comment)
@Ma27 Ma27 force-pushed the nextcloud-declarative-dbconfig branch from 603b34f to 3944aa0 Compare July 22, 2019 16:30
@Ma27
Copy link
Member Author

Ma27 commented Jul 22, 2019

@fpletz @globin resolved the merge conflict. Let me know if there's anything else to fix :)

@GrahamcOfBorg test nextcloud

@globin globin merged commit e891178 into NixOS:master Jul 22, 2019
@Ma27 Ma27 deleted the nextcloud-declarative-dbconfig branch July 22, 2019 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nextcloud module: changing config doesn't propagate to the module
4 participants