reduce diff with upstream and/or Debian package

[anarcat]

I had to do a significant amount of work to get this module to not diverge too much from the upstream defaults or, more specifically, the settings shipped with the nginx-light Debian pakage. I figured it would be better to leave those defaults alone by default or at least document why we need to change those. And if we do need to change those, why not contribute this back upstream to the Debian package or the Nginx upstream?

This ticket is an attempt at at least documenting those divergences.

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 5.5
• Ruby: N/A
• Distribution: Debian buster 10
• Module version: 1.0

How to reproduce (e.g Puppet code you use)

include nginx

What are you seeing

Lots of diff.

What behaviour did you expect instead

Minimal diff. :)

Output log

I lost the original output of the entire diff, unfortunately...

Any additional information you'd like to impart

... but here's what I ended up hacking together to fix it as much as I could:

# common nginx configuration
#
# @param client_max_body_size max upload size on this server. upstream
#                             default is 1m, see:
#                             https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size
class profile::nginx(
  Optional[String] $client_max_body_size = '1m',
) {
  class { 'nginx':
    confd_purge           => true,
    server_purge          => true,
    manage_repo           => false,
    http2                 => 'on',
    server_tokens         => 'off',
    package_flavor        => 'light',
    # XXX: doesn't work because a default is specified in the
    # class. doesn't matter much because the puppet module reuses
    # upstream default.
    worker_rlimit_nofile  => undef,
    accept_mutex          => 'off',
    # XXX: doesn't work because a default is specified in the
    # class. but that doesn't matter because accept_mutex is off so
    # this has no effect
    accept_mutex_delay    => undef,
    http_tcp_nopush       => 'on',
    gzip                  => 'on',
    client_max_body_size  => $client_max_body_size,
    run_dir               => '/run/nginx',
    client_body_temp_path => '/run/nginx/client_body_temp',
    proxy_temp_path       => '/run/nginx/proxy_temp',
    proxy_connect_timeout => '60s',
    proxy_read_timeout    => '60s',
    proxy_send_timeout    => '60s',
    # XXX: hardcoded, should just let nginx figure it out
    proxy_cache_max_size  => '15g',
    # XXX: hardcoded, should just let nginx figure it out
    proxy_cache_max_size  => '15g',
    proxy_cache_inactive  => '24h',
  }
  # recreate the default vhost
  nginx::resource::server { 'default':
    server_name         => ['_'],
    www_root            => "/srv/www/${webserver::defaultpage::defaultdomain}/htdocs/",
    listen_options      => 'default_server',
    ipv6_enable         => true,
    ipv6_listen_options => 'default_server',
    ssl                 => true,
    ssl_redirect        => true,
    ssl_cert            => '/etc/ssl/torproject-auto/servercerts/thishost.crt',
    ssl_key             => '/etc/ssl/torproject-auto/serverkeys/thishost.key';
  }
}

And here are tables of the things that were changed through parameters, things that were left unchanged even though they differ, and things that were changed but are problematic because there's no way to disable that

Changed

Reduce diff with Debian package in those settings:

setting	Debian	Upstream	Puppet	Note
server_tokens	off	?	on	more reasonable to stay silent about our identity
worker_rlimit_nofile	absent	absent	?	1024 is the default system ulimit
accept_mutex	absent	off	on?	off since 1.11, not necessary w/ EPOLLEXCLUSIVE
accept_mutex_delay	absent	absent	?	active only if the above is on
gzip	on	?	off	worthwhile optimization
tcp_nopush	on	?	off	same
client_max_body_size	1m	1m	10m	keep low to reduce DDOS attack surface
run_dir	/run/nginx	?	/var/nginx	fixed in #1352
client_body_temp_path	?	?	?	doesn't respect run_dir
proxy_temp_path	?	?	?	doesn't respect run_dir
proxy_connect_timeout	absent	90s	60s	no reason to change from default
proxy_read_timeout	absent	90s	60s	no reason to change from default
proxy_send_timeout	absent	90s	60s	no reason to change from default

Unchanged

Those were deemed acceptable

setting	Debian	Puppet	Note
worker_connections	768	1024	matches the default ulimit
types_hash_max_size	2048	1024	current upstream default, only 78 entries in mime.types
types_hash_bucket_size	absent	512	upstream default
keepalive_requests	absent	100	upstream default
server_names_hash_bucket_size	absent	64	not great: upstream default is automatic
server_names_hash_max_size	absent	512	upstream default
client_body_timeout	absent	60s	upstream default
send_timeout	absent	60s	upstream default
lingering_timeout	absent	5s	upstream default
gzip_comp_level	absent	1	upstream default
gzip_min_length	absent	20	upstream default
gzip_http_version	absent	1.1	upstream default
gzip_proxied	absent	off	upstream default
gzip_vary	absent	off	upstream default
gzip_disable	absent	msie6	upstream default is also absent, but harmless?
proxy_set_header	absent	Host $host
proxy_set_header	absent	X-Real-IP $remote_addr
proxy_set_header	absent	X-Forwarded-For $proxy_add_x_forwarded_for
proxy_set_header	absent	Proxy ""
proxy_headers_hash_bucket_size	absent	64	upstream default

Remaining problems

Those are problematic and still need to be fixed:

Setting	Upstream	Puppet	Note
proxy_buffers	8 4k or 8k	32 4k	should be based on page size
proxy_buffer_size	8k or 16k	8k	should be based on page size
client_body_buffer_size	8 or 16k	128k	much larger than default?
proxy_cache_max_size	unset	500m	upstream default unclear?
proxy_cache_inactive	10m	20m	needless change

I would particularly like to see the latter settings changed, because it's not possible to let nginx autodetect those otherwise, and there's no way to tell the nginx module to not set those in the configuration.