-
-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
nginx service: add commonHttpConfig option #23279
Conversation
@mbbx6spp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @globin, @domenkozar and @fadenb to be potential reviewers. |
We've using Are there options that have to be defined before the |
@fpletz when I use
(when I use the virtualHosts declarative form and not managing my own Of course, if I add a global |
Just to bump this because I don't think the use case is understood.
Checking nginx config file shows this
Hopefully that illustrates the problem with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot about this PR. Thanks a lot for the explanation and the examples. And the test of course! Looks good to me.
@globin What do you think? I'm not sure if the wording could confuse anyone.
Also, this should be backported to 17.03 at least. |
How about |
Is this going to make it into 17.03? @globin if you want to change it to prependHttpConfig you should make sure it goes all the way on the top. I didn't want to mess with the ordering too much. If it changes, please provide an update here and update the docs. |
Yes, you're right. @globin and I discussed this a few minutes ago and we'll go with your suggestion. Thanks! This will be backported to 17.03. |
(cherry picked from commit 251b9ca) cc NixOS#23279
cc NixOS#23279 (cherry picked from commit 7151e74)
Motivation for this change
For users of the
nginx
module that use the declarativevirtualHosts
structure there must be a way to define common definitions likelog_format
before using them inside of theserver
orlocation
context blocks of the generated virtual hosts that is also at thehttp
context block scope. Currently the NixOS nginx module does not offer a solution that doesn't require me to manage raw config files for my virtual hosts (I get anemerg
because when I try to append the http config viaappendHttpConfig
it shows up after the virtual host generated usage).I added a
nginx
module VM test to very this works. Seenixos/tests/nginx.nix
.Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)