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

refresh nginx config and optimize delivery #3313

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

rigelk
Copy link
Collaborator

@rigelk rigelk commented Nov 16, 2020

Description

This PR refactors the Nginx configuration for the following points:

  • update tls version to include 1.3 by default. so far it was not included by default to make room for previous versions of Nginx, but since 2018 Debian stable has included Nginx in version 1.14.1, and tls 1.3 is available since Nginx 1.13.0.
  • clearly indicate that new minimum required version.
  • update outdated ssl_ciphers to remove cipher required to support android 4.4, since that version is unsupported since March 2020.
  • reordered configuration in sections for easier maintenance: performance optimizations are separated from the vital application/websocket parts.
  • move parts that always require manual configuration at the top: peertube host and server name, use server_name
  • move peertube host to a more flexible upstream block: it allows to configure it in one place instead of 3, and is future-proof regarding load-balancing.
  • simplified port 80 block: Let’s Encrypt supports 301 redirects.
  • group certificate-related config together.
  • remove reslover config: it defaults to /etc/resolv.conf which is more than enough.
  • align values with their neighbors for easier reading
  • always specify units
  • always specify default values when they differ from the values set
  • use ’m’ for minutes, ’M’ for megabytes
  • add consensual optimizations wrt file serving:
    • add timeout optimizations
    • add file descriptor cache optimizations
    • enable sendfile with chunk size > rate limit
    • enable threading
    • tcp optimizations
    • point to further, more system-specific optimizations in the section description
  • CDN configuration reduced to one line change

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 👍 yes, light tests as follows are enough
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

I checked the configuration validates/passes the nginx config checker.

I checked sections reordering doesn’t impact response (locations match not in order but in best match order, and we don’t use return in the server context, so this doesn’t change the current behaviour).

I checked the configuration is working via the docker-compose setup and the production setup.

README.md Show resolved Hide resolved
support/nginx/peertube Show resolved Hide resolved
support/nginx/peertube Outdated Show resolved Hide resolved
@rigelk
Copy link
Collaborator Author

rigelk commented Nov 16, 2020

@Chocobozzz tested on sherri, I modified a few things further and left the config on the server.

@rigelk rigelk merged commit 5f59cf0 into Chocobozzz:develop Nov 16, 2020
@rigelk rigelk deleted the nginx-refresh branch November 16, 2020 18:16
Chocobozzz pushed a commit that referenced this pull request Jan 13, 2021
5f59cf0 introduced requirements on additional nginx modules:

nginx: [emerg] "aio threads" is unsupported on this platform in /etc/nginx/sites-enabled/peertube:247
https://nginx.org/en/docs/http/ngx_http_core_module.html#aio

nginx: [emerg] unknown directive "deny" in /etc/nginx/sites-enabled/peertube:83
https://nginx.org/en/docs/http/ngx_http_access_module.html
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.

None yet

2 participants