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

nixos/httpd: rewrite module to support overridable virtual hosts #72626

Closed
wants to merge 1 commit into from

Conversation

@aanderse
Copy link
Contributor

aanderse commented Nov 2, 2019

Motivation for this change

The NixOS httpd module has fallen behind the nginx module in every possible way. The module has received no contributions of substance aside from myself in the past several years now - partly because the code base is overly complicated, but also partly because the module is so limited in functionality compared to nginx. Few people have motivation to use this module as is.

The biggest problems with the httpd module, in contrast with nginx, which cannot be fixed without breaking backwards compatibility are:

  • the virtualHosts option is of type listOf instead of attrsOf which doesn't allow users to modify a virtualHost once created
  • each virtualHost defined is specific to either http or https which makes it extremely difficult to implement integration with security.acme.certs
  • the servedDirs and servedFiles options suffer the same problem of being a listOf instead of attrsOf which means once defined no modifications can be made
Things not yet done
  • Rewrite the servedDirs and servedFiles options to use attrsOf instead of listOf
  • Continue to cleanup the code
  • Write comprehensive documentation for the new module including a wide variety of examples
  • Write a migration guide for any existing users, with explanation of how to (temporarily) opt out of these changes
  • Adjust any other modules in NixOS which use services.httpd, and update existing modules to take advantage of new features
  • Extensive testing, including updates to nixos/tests/
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Nov 2, 2019

Example of current test configuration, keeping in mind I haven't rewritten the servedDirs or servedFiles options yet, so subject to change:

{
  imports = [ /home/aaron/nixpkgs/nixos/modules/services/web-servers/apache-httpd/default.nix ];
  disabledModules = [
    "services/logging/awstats.nix"
    "services/web-servers/apache-httpd/default.nix"
  ];

  # run httpd as a non root user for enhanced security
  systemd.services.httpd.serviceConfig = {
    User = "wwwrun";
    AmbientCapabilities = [ "CAP_NET_BIND_SERVICE" "CAP_SETGID CAP_SETUID" ];
    CapabilityBoundingSet = [ "CAP_NET_BIND_SERVICE" "CAP_SETGID CAP_SETUID" ];
  };

  services.httpd = {
    enable = true;
    enablePHP = true;
    adminAddr = "webmaster@example.org";
    logPerVirtualHost = true;
    virtualHosts = {
      "test1.example.org" = {
        documentRoot = "/var/www/test1.example.org";
        addSSL = true;
        enableACME = true;
      };
      "test2.example.org" = {
        documentRoot = "/var/www/test2.example.org";
        forceSSL = true;
        useACMEHost = "test2.example.org";
      };
    };
  };
  
  security.acme.certs."test1.example.org" = {
    user = "wwwrun";
    group = "wwwrun";
    webroot = "/var/lib/acme/acme-challenges";
  };
}

cc @Izorkin as potentially interested party. If this PR is merged I think #56304 might no longer be required.

@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Nov 9, 2019

I'm breaking this into several smaller PRs which are more targeted.

@aanderse aanderse closed this Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.