nixos/dokuwiki: add support for multi-site, additional plugins and templates #83769

dadada · 2020-03-30T11:58:44Z

Motivation for this change

This MR introduces multi-site dokuwiki configurations (e.g. multiple nginx servers) using the <name?> option syntax. It also enables the use of additional plugins and templates similarly to how wordpress.nix does it.

So far everything seems to work fine, but marking as WIP since I'd like to do some manual testing first. I can split this up into two MRs if requested.

Things done

dadada · 2020-03-30T12:00:50Z

Pinging @1000101 maintainer.

1000101 · 2020-03-30T14:49:05Z

@dadada thanks a million for picking up on this, I'll give it a go in a couple of hours.

@aanderse might want to have a look at this, too!

1000101 · 2020-04-01T19:29:17Z

@dadada Let me know when you're happy with the changes and I'll test it.

dadada · 2020-04-04T12:56:53Z

Thanks for taking an interest. What's your opinion on the way the users.php is created? If I understand correctly, this was previously read-only / part of the derivation? Was there some specific reason for this? Should we put a warning if usersFile is defined?

dadada · 2020-04-04T13:03:26Z

I made two more minor changes. efc9258c97355b4ea6e313c7ab9447d894dbcf34 adds a dokuwiki user that owns all the state directories and files.

07504a1 changes the behaviour of usersFile and aclFile. Since they might contain private information, the contents should not end up in the nix store. If the options are defined, those files are generated at the given paths if they don't already exist.

If acl is specified, the contents of it end up in the nix store, but I consider this to be a valid use-case and included a warning in the documentation.

dadada · 2020-04-04T13:12:59Z

I'll need to do a quick reword of the commit messages that change nixos/dokuwiki

dadada · 2020-04-04T13:28:18Z

@dadada Let me know when you're happy with the changes and I'll test it.

I'm happy, go ahead. :-)

1000101 · 2020-04-08T17:24:21Z

@GrahamcOfBorg test dokuwiki

1000101 · 2020-04-17T11:51:51Z

Thanks for taking an interest. What's your opinion on the way the users.php is created? If I understand correctly, this was previously read-only / part of the derivation? Was there some specific reason for this? Should we put a warning if usersFile is defined?

You mean users.auth.php (i.e. usersFile option)? It was actually not part of the derivation so that 1. credentials would not end up in nix store 2. it would be mutable (users could change their passwords).

Generally, I was trying to have as much options as I can immutable, with obvious ones mutable (https://github.com/NixOS/nixpkgs/pull/77830/files#r368233914 (resolved conversation) ).

dadada · 2020-04-17T12:55:13Z

You mean users.auth.php (i.e. usersFile option)? It was actually not part of the derivation so that 1. credentials would not end up in nix store 2. it would be mutable (users could change their passwords).

Ok, I might have misunderstood how types.path works. I was under the impression that the path will automatically be copied to the derivation / package, but this does not seem to be the case.

Generally, I was trying to have as much options as I can immutable, with obvious ones mutable (https://github.com/NixOS/nixpkgs/pull/77830/files#r368233914 (resolved conversation) ).

I made some changes so that the mutable files for usersFile and aclFile will not be automatically created unless specified, which should be identical to how these parameters behaved in your version.

1000101

Other than that, works really great, awesome job!!!

nixos/modules/services/web-apps/dokuwiki.nix

1000101 · 2020-04-17T12:27:43Z

nixos/modules/services/web-apps/dokuwiki.nix

+            {
+              # Enable encryption by default,
+              options.forceSSL.default = true;
+              options.enableACME.default = true;


I know I have introduced this option, but on a second thought, it is quite unnecessary to force these settings upon user. Wouldn't it be reasonable to drop it - along with forceSSL?

But in that case, I guess: fastcgi_param HTTPS on; would have to go, too.

I'm open to discussion on this one :).

I guess it would be wise to go with the default that works in most cases. ACME does not always work depending on restrictive DNS CA records or missing internet connection.

The default seems to be to disable ACME and not to force SSL. But it should be enabled in the example.

Regarding ninx fastcgi, if_not_empty should do the trick.

If the directive is specified with if_not_empty (1.1.11) then such a parameter will be passed to the server only if its value is not empty:

fastcgi_param HTTPS $https if_not_empty;

Sounds about right, let's do it that way.

Since would be an independent change, so you could open a new MR for this.

Sure, could do that, but since you did such a great job in refactoring a few things here and there, I though it would be a nice addition to the PR. But you're right, we can definitely keep it separated, it's just nice to have and not subject of this PR.

1000101 · 2020-04-17T13:18:37Z

You mean users.auth.php (i.e. usersFile option)? It was actually not part of the derivation so that 1. credentials would not end up in nix store 2. it would be mutable (users could change their passwords).

Ok, I might have misunderstood how types.path works. I was under the impression that the path will automatically be copied to the derivation / package, but this does not seem to be the case.

Generally, I was trying to have as much options as I can immutable, with obvious ones mutable (https://github.com/NixOS/nixpkgs/pull/77830/files#r368233914 (resolved conversation) ).

I made some changes so that the mutable files for usersFile and aclFile will not be automatically created unless specified, which should be identical to how these parameters behaved in your version.

Oh, I actually meant to say that I think your approach is correct (much better than mine, for that matter) as these files stay outside of /nix/store (in datadir) and they don't need to be created by hand. I'm sorry I didn't make myself clear!!! I think you're right the whole time, or am I missing something? :)))

dadada · 2020-04-17T13:38:01Z

Oh, I actually meant to say that I think your approach is correct (much better than mine, for that matter) as these files stay outside of /nix/store (in datadir) and they don't need to be created by hand. I'm sorry I didn't make myself clear!!! I think you're right the whole time, or am I missing something? :)))

Thanks, but there might be two aspects here:

private data is not stored in the nix store
the configuration is valid (the paths for usersFile and aclFile exist)

With your approach usersFile and aclFile already were outside of /nix/store if specified, so everything should be fine. Otherwise they would be immutable, so no private data would end up in the nix store. Unless you consider the contents of acl to be private, hence the added warning in the description.
Since usersFile and aclFile are both types.path, they are checked at configuratio time and if they don't exists, nixos-reconfigure fails, which is good. So no problem here either. The same would happen with the changes I made (still types.path).

I think the best would be to not create them, if usersFile or aclFile was not specified, but make them both types.str, so building the configuration does not fail, but the files are created if they don't exist at service startup.

dadada · 2020-04-17T13:46:29Z

I think the best would be to not create them, if usersFile or aclFile was not specified, but make them both types.str, so building the configuration does not fail, but the files are created if they don't exist at service startup.

On the other hand: If the file can't be created or has the wrong permissions, the service will fail at startup, which makes this kind-of dependent on the system and not 100% reproducible. I think the wireguard module does a similar thing with types.str and the private key.

1000101 · 2020-04-17T14:16:40Z

I think the best would be to not create them, if usersFile or aclFile was not specified, but make them both types.str, so building the configuration does not fail, but the files are created if they don't exist at service startup.

On the other hand: If the file can't be created or has the wrong permissions, the service will fail at startup, which makes this kind-of dependent on the system and not 100% reproducible. I think the wireguard module does a similar thing with types.str and the private key.

Sure, but what about leaving default value of usersFile and aclFile set to datadir, i.e. /var/lib/dokuwiki/${name} ant not null (as you had in previous commits)? Would that hurt anything? I think it's quite neat that it kind of works right out of the box, but the user can still change it if needed.

dadada · 2020-04-17T15:22:10Z

I think the best would be to not create them, if usersFile or aclFile was not specified, but make them both types.str, so building the configuration does not fail, but the files are created if they don't exist at service startup.

On the other hand: If the file can't be created or has the wrong permissions, the service will fail at startup, which makes this kind-of dependent on the system and not 100% reproducible. I think the wireguard module does a similar thing with types.str and the private key.

Sure, but what about leaving default value of usersFile and aclFile set to datadir, i.e. /var/lib/dokuwiki/${name} ant not null (as you had in previous commits)? Would that hurt anything? I think it's quite neat that it kind of works right out of the box, but the user can still change it if needed.

I thought about it, but that would mean creating aclFile even if acl is used. So changes to aclFile would not affect anything. Same if aclUse is false, there would still be a usersFile.

dadada

How does one access aclUse inside of the function. The way I tried it does not seem to work. I don't get how the function is called / which arguments are passed to it.

1000101 · 2020-04-17T20:30:35Z

I think the best would be to not create them, if usersFile or aclFile was not specified, but make them both types.str, so building the configuration does not fail, but the files are created if they don't exist at service startup.

On the other hand: If the file can't be created or has the wrong permissions, the service will fail at startup, which makes this kind-of dependent on the system and not 100% reproducible. I think the wireguard module does a similar thing with types.str and the private key.

Sure, but what about leaving default value of usersFile and aclFile set to datadir, i.e. /var/lib/dokuwiki/${name} ant not null (as you had in previous commits)? Would that hurt anything? I think it's quite neat that it kind of works right out of the box, but the user can still change it if needed.

I thought about it, but that would mean creating aclFile even if acl is used. So changes to aclFile would not affect anything. Same if aclUse is false, there would still be a usersFile.

Ok, let's leave it be, it's just a minor nitpicking anyway.

dadada · 2020-04-18T09:44:12Z

Now aclUse = true; should be enough to create users.auth.php and acl.auth.php in the default locations.

1000101 · 2020-04-18T12:33:03Z

@GrahamcOfBorg test dokuwiki

Enables multi-site configurations. This break compatibility with prior configurations that expect options for a single dokuwiki instance in `services.dokuwiki`.

Adds support for additional plugins and templates similarly to how wordpress.nix does it. Plugins and templates need to be packaged as in the example.

If usersFile is not set, a file is created along the stateDir that can hold the users and supports dynamically adding users using the web GUI.

Use types.str instead of types.path to exclude private information from the derivation. Add a warinig about the contents of acl beeing included in the nix store.

`aclFile` and `usersFile` will be set to a default value if `aclUse` is specified and aclFile is not overriden by `acl`.

dadada · 2020-04-18T21:38:33Z

Rebased changes onto master and squashed fixup commits.

1000101 · 2020-04-18T21:39:33Z

Rebased changes onto master and squashed fixup commits.

Oh, and you managed to find a way to access config values, great job! I'll test and review it again.

1000101 · 2020-04-18T21:50:25Z

@GrahamcOfBorg test dokuwiki

1000101

reviewed the diff and commit messages
made sure ofBorg build succeeded for all applicable platforms
ran nix-review without any failures
ran and tested the service

1000101 · 2020-04-19T09:53:34Z

@mmahut could you please help us check and merge if everything's fine by you?

With the merge of [1], the `.enable` check for `config.services.dokuwiki` has been removed. [1]: NixOS/nixpkgs#83769

ofborg bot added 6.topic: nixos 8.has: module (update) labels Mar 30, 2020

dadada changed the title ~~WIP: Nixos/dokuwiki multi-site support and additional plugins and templates~~ WIP: dokuwiki: multi-site support and additional plugins and templates Mar 30, 2020

dadada force-pushed the nixos/dokuwiki-multi-server branch from 371e4af to 1d34444 Compare March 30, 2020 12:07

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Mar 30, 2020

dadada changed the title ~~WIP: dokuwiki: multi-site support and additional plugins and templates~~ WIP: dokuwiki: add support for support multi-site, additional plugins and templates Mar 30, 2020

dadada force-pushed the nixos/dokuwiki-multi-server branch 4 times, most recently from 54d7ca8 to efe4248 Compare March 30, 2020 20:28

ofborg bot added the 10.rebuild-linux: 1 label Apr 4, 2020

dadada changed the title ~~WIP: dokuwiki: add support for support multi-site, additional plugins and templates~~ dokuwiki: add support for support multi-site, additional plugins and templates Apr 4, 2020

dadada changed the title ~~dokuwiki: add support for support multi-site, additional plugins and templates~~ dokuwiki: add support for multi-site, additional plugins and templates Apr 4, 2020

dadada force-pushed the nixos/dokuwiki-multi-server branch from 07504a1 to 5f2f935 Compare April 4, 2020 13:15

dadada changed the title ~~dokuwiki: add support for multi-site, additional plugins and templates~~ nixos/dokuwiki: add support for multi-site, additional plugins and templates Apr 4, 2020

1000101 suggested changes Apr 17, 2020

View reviewed changes

dadada commented Apr 17, 2020

View reviewed changes

dadada force-pushed the nixos/dokuwiki-multi-server branch 3 times, most recently from 085f55d to 6ffb27b Compare April 18, 2020 09:41

dadada force-pushed the nixos/dokuwiki-multi-server branch from 6ffb27b to cfbc0c4 Compare April 18, 2020 09:50

1000101 approved these changes Apr 18, 2020 •

edited

Loading

View reviewed changes

dadada force-pushed the nixos/dokuwiki-multi-server branch from cfbc0c4 to adea2d1 Compare April 18, 2020 21:35

dadada added 9 commits April 18, 2020 23:37

nixos/dokuwiki: add <name?> option

dc7ed06

Enables multi-site configurations. This break compatibility with prior configurations that expect options for a single dokuwiki instance in `services.dokuwiki`.

nixos/dokuwiki: refactor

71baf48

nixos/dokuwiki: add plugins and templates options

af6a7a0

Adds support for additional plugins and templates similarly to how wordpress.nix does it. Plugins and templates need to be packaged as in the example.

nixos/dokuwiki: add assertion for usersFile

0228046

nixos/dokuwiki: set default value for usersFile

a58dc30

If usersFile is not set, a file is created along the stateDir that can hold the users and supports dynamically adding users using the web GUI.

nixos/dokuwiki: add option disableActions

2e699f1

nixos/dokuwiki: dokuwiki user

2b67a89

nixos/dokuwiki: modify usersFile and aclFile

9460fb5

Use types.str instead of types.path to exclude private information from the derivation. Add a warinig about the contents of acl beeing included in the nix store.

nixos/dokuwiki: change default of aclFile and usersFile

2d86cca

`aclFile` and `usersFile` will be set to a default value if `aclUse` is specified and aclFile is not overriden by `acl`.

dadada force-pushed the nixos/dokuwiki-multi-server branch from adea2d1 to 2d86cca Compare April 18, 2020 21:37

1000101 approved these changes Apr 19, 2020

View reviewed changes

mmahut merged commit 60100a7 into NixOS:master Apr 20, 2020

devhell added a commit to openlab-aux/vuizvui that referenced this pull request Apr 24, 2020

modules/tests: Fix eval of dokuwiki

ea6b239

With the merge of [1], the `.enable` check for `config.services.dokuwiki` has been removed. [1]: NixOS/nixpkgs#83769

1000101 mentioned this pull request Aug 3, 2020

nixos/dokuwiki: drop SSL forcing and document incompatibility #94609

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/dokuwiki: add support for multi-site, additional plugins and templates #83769

nixos/dokuwiki: add support for multi-site, additional plugins and templates #83769

dadada commented Mar 30, 2020 •

edited

Loading

dadada commented Mar 30, 2020

1000101 commented Mar 30, 2020

1000101 commented Apr 1, 2020

dadada commented Apr 4, 2020

dadada commented Apr 4, 2020 •

edited

Loading

dadada commented Apr 4, 2020

dadada commented Apr 4, 2020 •

edited

Loading

1000101 commented Apr 8, 2020

1000101 commented Apr 17, 2020 •

edited

Loading

dadada commented Apr 17, 2020

1000101 left a comment

1000101 Apr 17, 2020 •

edited

Loading

dadada Apr 17, 2020

1000101 Apr 17, 2020

dadada Apr 17, 2020

1000101 Apr 17, 2020

1000101 commented Apr 17, 2020

dadada commented Apr 17, 2020 •

edited

Loading

dadada commented Apr 17, 2020 •

edited

Loading

1000101 commented Apr 17, 2020 •

edited

Loading

dadada commented Apr 17, 2020

dadada left a comment

1000101 commented Apr 17, 2020

dadada commented Apr 18, 2020

1000101 commented Apr 18, 2020

dadada commented Apr 18, 2020

1000101 commented Apr 18, 2020 •

edited

Loading

1000101 commented Apr 18, 2020

1000101 left a comment

1000101 commented Apr 19, 2020

nixos/dokuwiki: add support for multi-site, additional plugins and templates #83769

nixos/dokuwiki: add support for multi-site, additional plugins and templates #83769

Conversation

dadada commented Mar 30, 2020 • edited Loading

Motivation for this change

Things done

dadada commented Mar 30, 2020

1000101 commented Mar 30, 2020

1000101 commented Apr 1, 2020

dadada commented Apr 4, 2020

dadada commented Apr 4, 2020 • edited Loading

dadada commented Apr 4, 2020

dadada commented Apr 4, 2020 • edited Loading

1000101 commented Apr 8, 2020

1000101 commented Apr 17, 2020 • edited Loading

dadada commented Apr 17, 2020

1000101 left a comment

Choose a reason for hiding this comment

1000101 Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

dadada Apr 17, 2020

Choose a reason for hiding this comment

1000101 Apr 17, 2020

Choose a reason for hiding this comment

dadada Apr 17, 2020

Choose a reason for hiding this comment

1000101 Apr 17, 2020

Choose a reason for hiding this comment

1000101 commented Apr 17, 2020

dadada commented Apr 17, 2020 • edited Loading

dadada commented Apr 17, 2020 • edited Loading

1000101 commented Apr 17, 2020 • edited Loading

dadada commented Apr 17, 2020

dadada left a comment

Choose a reason for hiding this comment

1000101 commented Apr 17, 2020

dadada commented Apr 18, 2020

1000101 commented Apr 18, 2020

dadada commented Apr 18, 2020

1000101 commented Apr 18, 2020 • edited Loading

1000101 commented Apr 18, 2020

1000101 left a comment

Choose a reason for hiding this comment

1000101 commented Apr 19, 2020

dadada commented Mar 30, 2020 •

edited

Loading

dadada commented Apr 4, 2020 •

edited

Loading

dadada commented Apr 4, 2020 •

edited

Loading

1000101 commented Apr 17, 2020 •

edited

Loading

1000101 Apr 17, 2020 •

edited

Loading

dadada commented Apr 17, 2020 •

edited

Loading

dadada commented Apr 17, 2020 •

edited

Loading

1000101 commented Apr 17, 2020 •

edited

Loading

1000101 commented Apr 18, 2020 •

edited

Loading