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

Configure timeouts & some improvements #10

Merged
merged 33 commits into from
Mar 3, 2021
Merged

Configure timeouts & some improvements #10

merged 33 commits into from
Mar 3, 2021

Conversation

zacblazic
Copy link
Member

The main goal of this change is to configure timeouts that work well with the default idle timeout of an upstream load balancer (i.e. 60 seconds).

Additionally brings in some other changes, all of which were inspired by ingress-nginx.

@zacblazic
Align client_body_timeout with client_header_timeout
72e63e0
@zacblazic
Reduce client_body_buffer_size to 16k
6464697 
To prevent denial of service attachs, such as http slow attack.

* https://blog.qualys.com/vulnerabilities-research/2011/11/02/how-to-protect-against-slow-http-attacks
* https://www.acunetix.com/blog/web-security-zone/hardening-nginx
* https://www.upguard.com/blog/how-to-build-a-tough-nginx-server-in-15-steps
@zacblazic
Disable daemon mode explicitly
6a36f8e 
* http://nginx.org/en/docs/ngx_core_module.html#daemon
@zacblazic
Automatically detect optimal number of worker processes
ae60679 
* http://nginx.org/en/docs/ngx_core_module.html#worker_processes
@zacblazic
Set worker_shutdown_timeout to 240 seconds
5b1404c 
* http://nginx.org/en/docs/ngx_core_module.html#worker_shutdown_timeout
@zacblazic
Use optimal events configuration
ac4ec4a 
* http://nginx.org/en/docs/ngx_core_module.html#multi_accept
* http://nginx.org/en/docs/ngx_core_module.html#worker_connections
* http://nginx.org/en/docs/ngx_core_module.html#use
@zacblazic
Enable asynchronous file i/o
0eb3e27 
* http://nginx.org/en/docs/http/ngx_http_core_module.html#aio
* http://nginx.org/en/docs/http/ngx_http_core_module.html#aio_write
@zacblazic
Enable tcp nodelay
775a5a4 
* http://nginx.org/en/docs/http/ngx_http_core_module.html#tcp_nodelay
@zacblazic
Reduce client timeouts to 60 seconds
3018617 
* http://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout
* http://nginx.org/en/docs/http/ngx_http_core_module.html#client_header_timeout
@zacblazic
Reduce keepalive_timeout to 75 seconds
b4c66cb 
* http://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout
@zacblazic
Reduce proxy_connect_timeout to 5 seconds
c8556f2 
* http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout
@zacblazic
Reduce proxy_read_timeout to 60 seconds
541d2ae 
* http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout
@zacblazic
Mitigate httpoxy vulnerability
5bcf72b 
* https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/
@zacblazic
Move access_log to server block for healthcheck server
4a58a23
@zacblazic
Fix indentation for healtcheck server block
26fa61c
@zacblazic
Disable keep-alive for healtcheck server
b02d004
@zacblazic zacblazic requested a review from a team as a code owner March 1, 2021 14:08
zacblazic
zacblazic commented Mar 1, 2021
View reviewed changes
Copy link
Member Author

@zacblazic zacblazic left a comment

Choose a reason for hiding this comment

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

Some comments...

config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
config/http.conf Show resolved Hide resolved
config/http.conf Show resolved Hide resolved
zacblazic
zacblazic commented Mar 1, 2021
View reviewed changes
config/http.conf Outdated Show resolved Hide resolved
@itskingori
Copy link
Member

@zacblazic I think you're missing a CHANGELOG entry.

itskingori
itskingori previously approved these changes Mar 2, 2021
View reviewed changes
Copy link
Member

@itskingori itskingori left a comment

Choose a reason for hiding this comment

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

Great additions! 🎖️ + 🏆

zacblazic added 4 commits March 2, 2021 13:15
@zacblazic
Disable sendfile
5f0adb4 
Because this only applies when serving static content, which we'll
not be doing directly from the proxy, only from upstream servers.
@zacblazic
Enable reset_timedout_connection
c9015b5 
Resets timed out connections which avoids keeping an already closed
socket with filled buffers in a closing (FIN_WAIT1) state for a long
time.

* http://nginx.org/en/docs/http/ngx_http_core_module.html#reset_timedout_connection
@zacblazic
Separate client/proxy/keepalive sections
7e9c1b5
@zacblazic
Remove setting of client_body_buffer_size
d927ddb 
Prefer to let it be the default, we don't really need to have this
set here.
itskingori
itskingori reviewed Mar 3, 2021
View reviewed changes
Copy link
Member

@itskingori itskingori left a comment

Choose a reason for hiding this comment

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

Some comments ... 💬

config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
config/http.conf Outdated Show resolved Hide resolved
@zacblazic @itskingori
Rename connection_upgrade to proxy_connection
4de0f86 
Co-authored-by: King'ori Maina <j@kingori.co>
zacblazic and others added 2 commits March 3, 2021 12:19
@zacblazic @itskingori
Update changelog
05cd0bd 
Co-authored-by: King'ori Maina <j@kingori.co>
itskingori
itskingori previously approved these changes Mar 3, 2021
View reviewed changes
Copy link
Member

@itskingori itskingori left a comment

Choose a reason for hiding this comment

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

Let's give this a spin. 🌪️

itskingori
itskingori reviewed Mar 3, 2021
View reviewed changes
config/log.conf Outdated
@@ -8,10 +8,12 @@ log_format main_json escape=json
'"body_bytes_sent":"$body_bytes_sent",'
'"host":"$host",'
'"http_connection":"$http_connection",'
'"http_upgrade":"$http_upgrade",'
Copy link
Member

@itskingori itskingori Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Oops. Not alphabetical. 😅 Nit-pick.

itskingori
itskingori previously approved these changes Mar 3, 2021
View reviewed changes
@zacblazic
Set command in dockerfile explicitly
ac09a63 
Overwrites the command in the base image which specifies "daemon off".
Ensure we're not passing "daemon off" twice, which resultes in an error.
@zacblazic zacblazic merged commit ca36161 into master Mar 3, 2021
@zacblazic zacblazic deleted the update-configs branch March 3, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itskingori itskingori itskingori approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zacblazic @itskingori