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.

Sign up for GitHub

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

Jump to bottom

nixos/nextcloud: update nginx config #95508

Merged
merged 1 commit into from Aug 17, 2020
Merged

nixos/nextcloud: update nginx config #95508

merged 1 commit into from Aug 17, 2020

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Aug 15, 2020

Motivation for this change

This patch ensures that latest Nextcloud works flawlessly again on our
nginx. The new config is mostly based on upstream recommendations
(again)[1]:

  • Trying to access internals now results in a 404.
  • All .php-routes get properly resolved now.
  • Removed 404/403 handling from nginx as the app itself takes care of
    this. Also, this breaks the /ocs-API.
  • .woff2?-files expire later than other assets like images.

Closes #95293

[1] https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html

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.

@Ma27
nixos/nextcloud: update nginx config
42f6244 
This patch ensures that latest Nextcloud works flawlessly again on our
`nginx`. The new config is mostly based on upstream recommendations
(again)[1]:

* Trying to access internals now results in a 404.
* All `.php`-routes get properly resolved now.
* Removed 404/403 handling from `nginx` as the app itself takes care of
  this. Also, this breaks the `/ocs`-API.
* `.woff2?`-files expire later than other assets like images.

Closes NixOS#95293

[1] https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html
@Ma27 Ma27 added the 9.needs: port to stable A PR needs a backport to the stable release. label Aug 15, 2020
@Ma27 Ma27 added this to the 20.09 milestone Aug 15, 2020
@Ma27
Copy link
Member Author

Ma27 commented Aug 15, 2020

Already deployed this to my own instance successfully btw :)

@mweinelt
Copy link
Member

Yay, thanks! My Nextcloud instance is still on 20.03, so I need to find a few minutes tonight to port those changes for testing.

@Ma27
Copy link
Member Author

Ma27 commented Aug 15, 2020

@mweinelt AFAIK the merge-conflict should only be caused by indentation changes that were needed after the removal of cfg.nginx.enable. It should be possible to just copy the nginx-block from my branch to your module.

@mweinelt
Copy link
Member

Noted, will give it a try after dinner.

mweinelt added a commit to mweinelt/nixpkgs that referenced this pull request Aug 15, 2020
@mweinelt
nixos/nextcloud: update nginx config NixOS#95508
64131cc
@mweinelt
Copy link
Member

mweinelt commented Aug 15, 2020 •
edited

That does look alot better already!

One error in the Nextcloud logs is still there:

Aug 15 21:21:58 files Nextcloud[850]: {"reqId":"U3w4qFpXZ6KPGrtfx9uO","level":3,"time":"2020-08-15T19:21:58+00:00","remoteAddr":"2003:e5:3f37:103:c88e:901b:54a5:5463","user":"hexa","app":"lib","method":"GET","url":"/apps/maps/","message":"ResourceLocator can not find a web root (root: /var/lib/nextcloud/store-apps/maps, file: css/maps/4d3b-14bb-style.css?v=966826b4e0c8f3f79aed96cd8fa5a4a3, webRoot: , throw: true)","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0","version":"19.0.1.1"}

That files is located at /var/lib/nextcloud/data/appdata_ocrt1n1yccv6/css/maps/4d3b-14bb-style.css.

That being said, I don't see the related error in the Firefox networking tab.

@Ma27
Copy link
Member Author

Ma27 commented Aug 15, 2020

Will take a look tomorrow :)

@aanderse
Copy link
Member

@Ma27 unfortunately I'm a httpd guy, not an nginx person... so these changes seem a bit too specific for me to provide value in reviewing. Sorry 😞

@aanderse aanderse removed their request for review August 15, 2020 21:07
@Ma27
Copy link
Member Author

Ma27 commented Aug 16, 2020

@mweinelt it seems as I don't have any maps/-directory on my Nextcloud 19. Do you use an older version I can use to reproduce or do you have some special config declared?

@mweinelt
Copy link
Member

mweinelt commented Aug 16, 2020 •
edited

It's a separate app that needs to be installed. https://apps.nextcloud.com/apps/maps

I'm on 19.0.1, so pretty recent.

And as I noted in the issue this is a generic issue with apps.

{
   "reqId":"rcySgiTQDl5IDxC64Oin",
   "level":3,
   "time":"2020-08-16T12:30:27+00:00",
   "remoteAddr":"80.135.0.0",
   "user":"hexa",
   "app":"lib",
   "method":"GET",
   "url":"/apps/gpxedit/",
   "message":"ResourceLocator can not find a web root (root: /var/lib/nextcloud/store-apps/gpxedit, file: css/gpxedit/fab1-14bb-Leaflet.LinearMeasurement.css?v=966826b4e0c8f3f79aed96cd8fa5a4a3, webRoot: , throw: true)",
   "userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0",
   "version":"19.0.1.1"
}

The file in question is located at /var/lib/nextcloud/data/appdata_ocrt1n1yccv6/css/gpxedit/fab1-14bb-Leaflet.LinearMeasurement.css.

[root@files:/var/lib/nextcloud]# rg ocrt1n1yccv6
config/config.php
39:  'instanceid' => 'ocrt1n1yccv6',

@Ma27
Copy link
Member Author

Ma27 commented Aug 16, 2020

I don't understand though why the CSS file lands in the datadirectory and not in the (writable) /store-apps-prefix where the app itself is installed as well.

This seems to be an internal Nextcloud problem though (that doesn't mean that it isn't NixOS-related) which would also explain why you don't have any errors in your browser console. One symptom I see btw is that the maps are just white without any additional info, maybe that's caused by this error.

Unfortunately I don't know the app-installer well enough. Do you want to take a look? Otherwise I can investigate further at some point in the next week(s).

@mweinelt
Copy link
Member

I don't understand though why the CSS file lands in the datadirectory and not in the (writable) /store-apps-prefix where the app itself is installed as well.

Maybe it's being generated during install, rendered from `store-apps/maps/css/Leaflet.LinearMeasurement.scss and auto-generated stuff lands in the appdata directory? It also has this weird name that looks like they want to prevent caching. Not sure where it comes from.

I've scrolled through the source code of both nextcloud/server and nextcloud/maps but I'm not sure where any of this is happening.

@flokli
Copy link
Contributor

flokli commented Aug 17, 2020

I propose merging this PR, and debugging the issue with maps at a later time. This already improves things quite a bunch. WDYT?

mweinelt
mweinelt approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

Major QOL improvement. Thanks!

@mweinelt mweinelt merged commit a153452 into NixOS:master Aug 17, 2020
@mweinelt
Copy link
Member

@Ma27 Will you take care of the backport?

@Ma27 Ma27 deleted the nextcloud-nginx branch August 17, 2020 13:35
@Ma27 Ma27 mentioned this pull request Aug 17, 2020
Closed
@Ma27
Copy link
Member Author

Ma27 commented Aug 17, 2020

@Ma27 Will you take care of the backport?

Yes.

I propose merging this PR, and debugging the issue with maps at a later time

Agreed. Opened #95704.

@Ma27
Copy link
Member Author

Ma27 commented Aug 17, 2020

Ported to stable as eb8080d. Tests are passing and @mweinelt and I are already using this patch on 20.03 %)

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mweinelt mweinelt mweinelt approved these changes

@fpletz fpletz Awaiting requested review from fpletz

@lheckemann lheckemann Awaiting requested review from lheckemann

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
20.09
Development

Successfully merging this pull request may close these issues.

Nextcloud: Missing routes
4 participants
@Ma27 @mweinelt @aanderse @flokli