nixos/paperless: add configureNginx option#385637
nixos/paperless: add configureNginx option#385637SuperSandro2000 merged 1 commit intoNixOS:masterfrom
Conversation
8ad492b to
93a26f8
Compare
93a26f8 to
02d1a19
Compare
02d1a19 to
ccf5059
Compare
|
@erikarvstedt can you take a look, please? |
Atemu
left a comment
There was a problem hiding this comment.
I'm not entirely convinced why we should have this option. Please elaborate why this is necessary.
6e2e865 to
5dfc47e
Compare
That's easy to explain: You don't want to copy the following snippet around: {
enable = true;
upstreams.paperless.servers."http://${cfg.address}:${toString cfg.port}" = { };
virtualHosts.${cfg.domain} = {
forceSSL = lib.mkDefault true;
locations = {
"/".proxyPass = "http://paperless";
"/static/" = {
root = config.services.paperless.package;
extraConfig = ''
rewrite ^/(.*)$ /lib/paperless-ngx/$1 break;
'';
};
"/ws/status" = {
proxyPass = "http://paperless";
proxyWebsockets = true;
};
};
};
} |
5dfc47e to
6a8546a
Compare
6a8546a to
af11913
Compare
503fbf5 to
cac343c
Compare
cac343c to
4de7774
Compare
teutat3s
left a comment
There was a problem hiding this comment.
Thanks for working on this. Late review.
| virtualHosts.${cfg.domain} = { | ||
| forceSSL = lib.mkDefault true; | ||
| locations = { | ||
| "/".proxyPass = "http://paperless"; |
There was a problem hiding this comment.
I guess we need priority here to ensure the correct order of locations in the final /etc/nginx/nginx.conf?
There was a problem hiding this comment.
Isn't the most specific path matching by default?
There was a problem hiding this comment.
True, I was thinking of regex matching. Nvm as well 🙂
|
|
||
| services.paperless.settings = lib.mkMerge [ | ||
| (lib.mkIf (cfg.domain != "") { | ||
| PAPERLESS_URL = "https://${cfg.domain}"; |
There was a problem hiding this comment.
Ah, this can be. I tested this on my instance and with the test.
|
minor note that this caused a breaking change for prior working config, at least for one iteration of the channel between these two PRs So this option is now apparently needed even if not using the new nginx wrapper/support. I just set it to null as per above. |
|
I've opened 2 days ago #439217 to fix that 😅 can you give that a shot and if it fixes your problem I am going to merge it. |
|
Yes, that's what I meant by "as per above" (though as it turned out I had to set it to |
depends on #397828
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.