nixos/nginx: allow overriding fastcgi params #108277
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Your change looks nice but there is a build error on the CI steps. I am in doubt if the error is related, it seems a false failure to me. |
Yeah. Probably be fixed by #108280. |
|
the ci fix should be now in master. |
By default in Nginx, if you want to override a single fastcgi_param, you have to override all of them. This is less of a big deal if you're editing the Nginx configuration directly, but when you're generating the Nginx configuration with Nix it can be very annoying to bloat your configuration repeating the default values of FastCGI parameters every time. This patch adds a fastcgiParams option to Nginx locations. If any parameters are set through this, all the default values will be included as well, so only the ones that are changing need to be supplied. There's no way to use fastcgiParams to actually override all parameters if that's what you want, but I think that's a niche use case and it's still possible using extraConfig, which up until now was the only option Nginx allows the fastcgi_param directive in http and server scopes as well as location, but here I only support location. It would be possible to support the others, but I don't think it's worth it. It would be a possible future enhancement if somebody has a need for it.
otavio
approved these changes
Jan 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default in Nginx, if you want to override a single fastcgi_param,
you have to override all of them. This is less of a big deal if
you're editing the Nginx configuration directly, but when you're
generating the Nginx configuration with Nix it can be very annoying to
bloat your configuration repeating the default values of FastCGI
parameters every time.
This patch adds a fastcgiParams option to Nginx locations. If any
parameters are set through this, all the default values will be
included as well, so only the ones that are changing need to be
supplied. There's no way to use fastcgiParams to actually override
all parameters if that's what you want, but I think that's a niche use
case and it's still possible using extraConfig, which up until now was
the only option
Nginx allows the fastcgi_param directive in http and server scopes as
well as location, but here I only support location. It would be
possible to support the others, but I don't think it's worth it. It
would be a possible future enhancement if somebody has a need for it.
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)