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

Optimize default nginx config #1040

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

sjberman
Copy link
Contributor

@sjberman sjberman commented Sep 6, 2023

Problem: We want to ensure that the default nginx configuration uses the best possible options for performance and stability.

Solution: Using NGINX Ingress Controller as a guide, added options to increase performance, set the common X-Forwarded-For header, and allow standard media types.

Closes #557

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Problem: We want to ensure that the default nginx configuration uses the best possible options for performance and stability.

Solution: Using NGINX Ingress Controller as a guide, added options to increase performance, set the common X-Forwarded-For header, and allow standard media types.
@sjberman sjberman requested a review from a team as a code owner September 6, 2023 18:40
@github-actions github-actions bot added the bug Something isn't working label Sep 6, 2023
@kate-osborn
Copy link
Contributor

Part of the AC for #1040 is to review the config with NGINX experts. Has that been done? Should we tag an expert on this PR?

@sjberman
Copy link
Contributor Author

sjberman commented Sep 6, 2023

Part of the AC for #1040 is to review the config with NGINX experts. Has that been done? Should we tag an expert on this PR?

I haven't done that specifically. These changes were based on NIC's default configuration, module docs, multiple online guides for optimizing NGINX, and a default configuration as provided by nginx docs (using what is relevant to NKG). All of those resources influenced these minimal changes. There really aren't that many options for us to work with for a default configuration, so I felt that asking someone specifically may be overkill (and overhead) that wouldn't offer much more value that the research hasn't already offered.

@sjberman sjberman enabled auto-merge (squash) September 7, 2023 17:42
@sjberman sjberman merged commit 901b400 into nginxinc:main Sep 7, 2023
23 checks passed
@sjberman sjberman deleted the fix/default-nginx-config branch September 7, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Define Recommended NGINX Default Configuration
3 participants