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

nginx: added serverName option for virtualHosts #21931

Merged

Conversation

bobvanderlinden
Copy link
Member

@bobvanderlinden bobvanderlinden commented Jan 16, 2017

Motivation for this change

This allows overriding the server_name attribute of virtual
hosts. By doing so it is possible to have multiple virtualHost
definitions that share the same server_name. This is useful in
particular when you need a HTTP as well as a HTTPS virtualhost: same
server_name, different port.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Tested using: https://gist.github.com/bobvanderlinden/939cd64a8290a7388cce2cf4303b0995

@mention-bot
Copy link

@bobvanderlinden, 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.

@danbst
Copy link
Contributor

danbst commented Jan 16, 2017

Why not using serverAliases?

@bobvanderlinden
Copy link
Member Author

bobvanderlinden commented Jan 17, 2017

I can imagine it being modeled like this:

          services.nginx = {
            enable = true;
            virtualHosts = {
              "example_http" = {
                serverAliases = [ "example.org" ];
                root = ./wwwroot;
              };
              "example_https" = {
                serverAliases = [ "example.org" ];
                enableSSL = true;
                enableACME = true;
                root = ./wwwroot;
              };
            };
          };

But in that case wouldn't it also be available on the domain-names example_http and example_https? The nginx configuration will have server_name example_https example.org;. I can also imagine ACME not handling this correctly, since it would try to retrieve a certificate for example_https. I haven't tried this yet. I'll give that a shot.

EDIT:
In the above example, when ACME runs, I get the following error:

ACME server returned an error: urn:acme:error:malformed :: The request message was malformed :: Invalid character in DNS name

I think that is because of the _ in the name, choosing a name without _ causes the following error:

Failed to establish a new connection: [Errno -2] Name or service not known

Which makes sense, but it seems the serverAlias is not being used.

I did notice that this PR doesn't handle ACME correctly yet.

@bobvanderlinden
Copy link
Member Author

I've updated the PR to correctly handle ACME.

This allows overriding the `server_name` attribute of virtual
hosts. By doing so it is possible to have multiple virtualHost
definitions that share the same `server_name`. This is useful in
particular when you need a HTTP as well as a HTTPS virtualhost: same
server_name, different port.
@globin globin merged commit d9987f3 into NixOS:master Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants