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: support overridable virtual hosts #73113

Merged
merged 1 commit into from Dec 26, 2019
Merged

Conversation

@aanderse
Copy link
Contributor

aanderse commented Nov 9, 2019

I have split this PR into multiple commits to make it easier to review. Ignoring white space also helps in a few places. Before merging all commits should be squashed as the commits individually break various things.

Motivation for this change
  • 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
  • httpd module code is overly complex
  • httpd module has fallen far behind the nginx module and as a result isn't used by many people on nixos
Things not yet done
  • merge a PR to rewrite of the awstats module, with offer to do so generously provided by @aristaeus
  • rewrite examples in the nixos manual under the abstractions section of the manual which reference services.httpd, something which @samueldr may have some ideas on
  • write release notes
  • squash all commits into a single before merging this PR
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 aanderse force-pushed the aanderse:httpd-vhost branch from 23f2ce2 to ed60e4e Nov 9, 2019
@aanderse aanderse requested review from bobvanderlinden and andir Nov 9, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Nov 9, 2019

Example of current test configuration:

{
  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"
  ];

  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."test2.example.org" = {
    user = "wwwrun";
    group = "wwwrun";
    webroot = "/var/lib/acme/acme-challenges";
  };
}
@aanderse aanderse force-pushed the aanderse:httpd-vhost branch 3 times, most recently from 615c975 to b378398 Nov 9, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Nov 9, 2019

ping @redvers, @FPtje, and @Izorkin as users of the httpd module. Any review, testing, and feedback is appreciated.

@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from b5ecc12 to f7cd970 Nov 10, 2019
@nixos-discourse

This comment has been minimized.

Copy link

nixos-discourse commented Nov 14, 2019

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-are-your-goals-for-20-03/4773/3

@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from f7cd970 to 2eb0299 Nov 22, 2019
@aanderse aanderse force-pushed the aanderse:httpd-vhost branch 2 times, most recently from d6a42ab to 97b8dd9 Dec 1, 2019
@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from 97b8dd9 to d6a747c Dec 4, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 6, 2019

cc @vanyaklimenko in case they have interest in testing this PR.

@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from d6a747c to c3c4efe Dec 13, 2019
@aanderse aanderse requested a review from ivan Dec 14, 2019
@aanderse aanderse force-pushed the aanderse:httpd-vhost branch 3 times, most recently from 1c2a512 to 80bc305 Dec 15, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 18, 2019

@GrahamcOfBorg test haproxy proxy upnp

@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 18, 2019

@GrahamcOfBorg test limesurvey mediawiki
@GrahamcOfBorg test moodle wordpress

Copy link
Member

ivan left a comment

I applied this to nixpkgs master, converted my configuration, and merged my http:// and https:// virtualhosts into a single virtualhost. That broke https:// because I had

        listen = [
          { ip = "1.2.3.4"; port = 80; }
          { ip = "1.2.3.4"; port = 443; }
        ];

which caused Apache to run HTTP without SSL on port 443.

From the implementation in nginx, I correctly guessed that I could do this here too:

        listen = [
          { ip = "1.2.3.4"; port = 80; }
          { ip = "1.2.3.4"; port = 443; ssl = true; }
        ];

but this is perplexingly undocumented in apache-httpd/per-server-options.nix. (Yes, it really does work, but I cannot figure out why.)

@ivan

This comment has been minimized.

Copy link
Member

ivan commented Dec 18, 2019

Sorry! I was looking at the wrong branch locally and see that it is documented.

@ivan
ivan approved these changes Dec 18, 2019
Copy link
Member

ivan left a comment

Migrating my configuration wasn't very painful and the migrated configuration appears to behave correctly.

I think this should be merged once someone else looks over it, or confirms that those six Apache-using modules in nixpkgs still work.

@aanderse aanderse marked this pull request as ready for review Dec 18, 2019
@aanderse aanderse requested a review from nbp as a code owner Dec 18, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 18, 2019

@ivan I just need to test awstats PR and then merge it, and get a few more people testing this then I'm satisfied. At this point though I'm looking for review and testing, so thank you very much!

@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from 80bc305 to 3a485a2 Dec 22, 2019
…ualHosts option type from listOf to attrsOf, add ACME integration
@aanderse aanderse force-pushed the aanderse:httpd-vhost branch from 3a485a2 to 79215f0 Dec 25, 2019
@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 25, 2019

@GrahamcOfBorg test limesurvey mediawiki
@GrahamcOfBorg test moodle wordpress
@GrahamcOfBorg test haproxy hitch proxy upnp

@aanderse

This comment has been minimized.

Copy link
Contributor Author

aanderse commented Dec 26, 2019

I've been running this on 3 machines for a while now, @ivan has given it a look over and migrated their configuration to use it, all outstanding issues/requirements addressed... so merging.

@aanderse aanderse merged commit 4d2dd15 into NixOS:master Dec 26, 2019
18 checks passed
18 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
tests.haproxy, tests.hitch, tests.proxy, tests.upnp on aarch64-linux Success
Details
tests.haproxy, tests.hitch, tests.proxy, tests.upnp on x86_64-linux Success
Details
tests.limesurvey, tests.mediawiki on aarch64-linux Success
Details
tests.limesurvey, tests.mediawiki on x86_64-linux Success
Details
tests.moodle, tests.wordpress on aarch64-linux Success
Details
tests.moodle, tests.wordpress on x86_64-linux Success
Details
@aanderse aanderse deleted the aanderse:httpd-vhost branch Dec 26, 2019
@dasJ dasJ mentioned this pull request Dec 29, 2019
6 of 361 tasks complete
@aanderse aanderse mentioned this pull request Dec 30, 2019
8 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.