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: fix `nginx`-config for Nextcloud 19 and older #97666

Merged
merged 1 commit into from Oct 4, 2020

Conversation

@Ma27
Copy link
Member

@Ma27 Ma27 commented Sep 10, 2020

Motivation for this change

It seems as I misconfigured nginx for certain cases such as the
ldap-plugin[1] in 42f6244. This patch
fixes the nginx-config to match the upstream recommendations[2].

Also added a comment to the module to remind myself to ensure that
nginx will work with both v19 and v20 as soon as the latter is
released and can be packaged in nixpkgs.

Co-authored-by: nivadis nivadis@users.noreply.github.com

[1] nextcloud/server#16194 (comment)
[2] https://docs.nextcloud.com/server/19/admin_manual/installation/nginx.html

cc @mweinelt @nivadis @flokli
Needs backport to 20.09 and 20.03.

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.
It seems as I misconfigured `nginx` for certain cases such as the
`ldap`-plugin[1] in 42f6244. This patch
fixes the `nginx`-config to match the upstream recommendations[2].

Also added a comment to the module to remind myself to ensure that
`nginx` will work with both v19 and v20 as soon as the latter is
released and can be packaged in `nixpkgs`.

Co-authored-by: nivadis <nivadis@users.noreply.github.com>

[1] nextcloud/server#16194 (comment)
[2] https://docs.nextcloud.com/server/19/admin_manual/installation/nginx.html
@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 10, 2020

cc @matthiasbeyer can you check if this fixes your issue as well? :)

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 10, 2020

Will test this weekend.

@Ma27 Ma27 added this to the 20.09 milestone Sep 11, 2020
@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 11, 2020

So I replaced package = pkgs.nextcloud18 with

    package =
      let
        url = "https://github.com/nixos/nixpkgs/archive/8d8871c565373a46792fc0e4d3976a3c2ac58c18.tar.gz";
        p = import (builtins.fetchTarball url) {};
      in p.nextcloud18

in my configuration for my server, and it works. Not sure whether this is a sufficient test, tho.

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 11, 2020

But I still get an error when updating my channels.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 11, 2020

@matthiasbeyer not sure why it works now, but to test this change you'd actually have to replace the Nextcloud module, not the package (e.g. with disabledModules and imports).

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 11, 2020

Yeah, it worked because I did not update the channel... after updating the channel, my nextcloud was broken again. Not sure how to build my whole system from this commit, tho.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 11, 2020

A temporary solution would be a local checkout of this branch and then you could do something like this:

{
  # Import module from this branch
  imports = [ /path/to/local/checkout/nixos/modules/services/web-apps/nextcloud.nix ];
  # Disable default Nextcloud module from your `<nixpkgs>`
  disabledModules = [ "services/web-apps/nextcloud.nix" ];
}
@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 20, 2020

ping @matthiasbeyer any updates?

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 21, 2020

I am on vacation (currently on my way home). I hope I can get down to test this during the week.

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 25, 2020

Not quite sure how to integrate this PR into my krops deployment so that it builds nextcloud from this PR instead of nixos stable...

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 25, 2020

Do you want to build from this PR's rev or do you just want to apply the changes made here in your deployment? In the latter case you can work with disabledModules as described above.

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 25, 2020

The latter. I have to add this PR to the environment to build nextcloud and nginx from it...

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 25, 2020

Okay, I guess I found a way, but now I get

error: attribute 'buildEnv' missing, at [...]nixos/modules/services/web-apps/nextcloud.nix:13:7

Edit: maybe I have to update stable nixos as well...
Edit2: Nope, that didn't do the trick...

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 25, 2020

Oh right, the php package has been refactored and this wasn't part of 20.03.
You should(tm) be able to fix this by adding sth. like this to your NixOS configuration:

{
  nixpkgs.overlays = [
    (self: super: {
      php74 = (import /path/to/checkout/of/this/branch {}).php74;
    })
  ];
}
@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 26, 2020

@matthiasbeyer did you try out the overlay?

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Sep 27, 2020

In fact I did it a bit differently, so I will try to use it with an overlay. Unfortunatly I'm exepecting people any minute now and cannot do it right now. I hope I can get to try it tomorrow.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 27, 2020

That's totally fine, have a nice sunday! :)

Just FYI, to make sure we don't have any further issues I just confirmed that a configuration like this evaluates when having <nixpkgs> point to release-20.03:

# ~/Projects/nixpkgs-backports/nextcloud-vm.nix
{
  vm = { pkgs, ... }: {
    disabledModules = [ "services/web-apps/nextcloud.nix" ];
    imports = [ ~/Projects/nixpkgs/nixos/modules/services/web-apps/nextcloud.nix ];
    nixpkgs.overlays = [
      (self: super: {
        php74 = (import ~/Projects/nixpkgs { }).php74;
      })
    ];
    services.nextcloud = {
      enable = true;
      package = pkgs.nextcloud19;
      hostName = "foobar";
      config.adminpass = "verysafe";
    };
  };
}
@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 2, 2020

@worldofpeace @jonringer this should land in 20.09 as well btw.
@matthiasbeyer do you need sth. else from me for testing? :)

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Oct 2, 2020

What's the timeframe? This weekend won't be possible as I'm completely occupied and during next week,... I don't know. 😢

@ajs124
ajs124 approved these changes Oct 2, 2020
Copy link
Member

@ajs124 ajs124 left a comment

Can confirm that LDAP settings don't work without this, but do work with it on 19.x

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 3, 2020

I'd prefer to merge this now as it fixes an actual bug for several people.
If the problem @matthiasbeyer reported isn't fixed by that I'm happy to help out investigating and bugfixes are usually backportable :)

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Oct 4, 2020

Agreed, as I probably won't get to it in the next few days.

@Ma27 Ma27 merged commit 08cc63b into NixOS:master Oct 4, 2020
16 checks passed
16 checks passed
tests tests
Details
action
Details
Evaluation Performance Report Evaluator Performance Report
Details
Wait for ofborg
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="8d8871c"; rev="8d8871c565373a46792fc0e4d3976a3c2ac58c18"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@Ma27 Ma27 deleted the Ma27:nextcloud-nginx branch Oct 4, 2020
@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 4, 2020

20.09: ec20ddd
20.03: adc7650

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Oct 4, 2020
See NixOS#97666 for further context.
@ajs124
Copy link
Member

@ajs124 ajs124 commented Oct 5, 2020

I think this broke some other stuff, e.g. right click -> copy or move (which might be a plugin?).
To fix that, I applied a patch that looks something like this:

@@ -568,20 +568,19 @@ in {
               fastcgi_read_timeout 120s;
             '';
           };
-          "~ \\.(?:css|js|svg|gif|map)$".extraConfig = ''
+          "~ \\.(?:css|js|woff2?|svg|gif|map)$".extraConfig = ''
             try_files $uri /index.php$request_uri;
             expires 6M;
             access_log off;
           '';
-          "~ \\.woff2?$".extraConfig = ''
-            try_files $uri /index.php$request_uri;
-            expires 7d;
-            access_log off;
-          '';
           "~ ^\\/(?:updater|ocs-provider|ocm-provider)(?:$|\\/)".extraConfig = ''
             try_files $uri/ =404;
             index index.php;
           '';
+          "~ \\.(?:png|html|ttf|ico|jpg|jpeg|bcmap|mp4|webm)$".extraConfig = ''
+            try_files $uri /index.php$request_uri;
+            access_log off;
+          '';
         };
         extraConfig = ''
           index index.php index.html /index.php$request_uri;

The first hunk is probably irrelevant, the second is basically copied from the upstream config as linked in this PR.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 5, 2020

Which nextcloud version are you using and is there anything in the logs? Also, what do you observe exactly? Just tested copy operations in my Nextcloud without any issues.

@ajs124
Copy link
Member

@ajs124 ajs124 commented Oct 5, 2020

I'm running 19.0.x. I didn't see anything in the server logs, just some thing 404ing in my browsers devtools.
I'm not talking about normal copying stuff, but right click -> copy or move. There's a dialog that pops up and asks you where to move stuff.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 5, 2020

There's a dialog that pops up and asks you where to move stuff.

That's what I talked about as well :)

Will try to reproduce it in a VM as soon as I have time to.

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Oct 5, 2020

Reproducible with 19.0.3, fixed in #99658

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Oct 6, 2020
See NixOS#97666 for further context.
dasJ added a commit to helsinki-systems/nixpkgs that referenced this pull request Oct 7, 2020
See NixOS#97666 for further context.
Ma27 added a commit that referenced this pull request Oct 8, 2020
See #97666 for further context.

(cherry picked from commit 227ba90)
Emantor added a commit to Emantor/nixpkgs that referenced this pull request Oct 9, 2020
dawidsowa added a commit to dawidsowa/nixpkgs that referenced this pull request Oct 11, 2020
@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Oct 30, 2020

So I updated my nextcloud(18) to 504f993 but still it is not working, as you can see for yourself: https://cloud.beyermatthi.as

Do I have to add something in my configuration as well? Unfortunately, I cannot downgrade anymore as nextcloud refuses to start then.

Edit: neither the nginx nor the nextcloud logs tell me anything about what's going on!

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Oct 30, 2020

Oh well... updating to nextcloud19 did not fix my issue... 🙈 I'm on nixos 20.03 btw.

@matthiasbeyer
Copy link
Contributor

@matthiasbeyer matthiasbeyer commented Oct 30, 2020

services.nextcloud.nginx.enable = true; fixed my issue. Go away, there's just a dumb idiot to see here (myself of course - took me three hours of debugging and one minute of help from others to figure this out).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.