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

wordpress: create module to replace the httpd subservice #62175

Merged
merged 3 commits into from Jul 4, 2019

Conversation

@aanderse
Copy link
Contributor

commented May 28, 2019

Motivation for this change

Ultimately to remove usage of mod_php in all nixos modules.
See #57752 for prior work, which is still pending.

NOTE:

  • I have not updated relevant documentation yet, I'm waiting to get some feedback on the actual code before I do that.
  • Certain chunks of code were copy+pasted from the httpd subservice with as little fixup as possible, because I have never run a wordpress site so don't know that much about it.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

@FPtje Do you use the wordpress subservice for httpd on NixOS? Your review and feedback on this PR would be appreciated.

@nixos-discourse

This comment has been minimized.

Copy link

commented May 29, 2019

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/3

@FPtje

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

We do use wordpress.

The code looks fine to me. My main concerns are with migration. Is it easy enough to migrate an existing wordpress installation to the config requested in this PR?

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

@FPtje I'm not a WordPress admin so I can't tell you've I've run an update yet. My plan was to install a WordPress site with with existing subservice and then migrate it, hopefully with as few manual steps as possible.

Are you in a position you could clone your database to a test environment and try this new module after I write out migration steps?

Are you able to share (relevant) bits of your httpd and wordpress config?

Do you make use of the .package option to keep your installation up to date? How about the .languages option?

Thanks for any info you can provide!

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

@FPtje I have created a site, uploaded a bunch of images, made a few posts, and then upgraded to the new service + package. Here was my config using the apache subservice:

  services.mysql.enable = true;
  services.mysql.package = pkgs.mariadb;
  services.httpd.enable = true;
  services.httpd.adminAddr = "admin@example.com";
  services.httpd.extraSubservices = [
    {
      serviceType = "wordpress";
      dbUser = "wwwrun";
      dbPassword = "password";
      dbHost = "localhost";
      themes = [
        # For shits and giggles, let's package the responsive theme
        (pkgs.stdenv.mkDerivation {
          name = "responsive-theme";
          # Download the theme from the wordpress site
          src = pkgs.fetchurl {
            url = http://wordpress.org/themes/download/responsive.1.9.7.6.zip;
            sha256 = "1g1mjvjbx7a0w8g69xbahi09y2z8wfk1pzy1wrdrdnjlynyfgzq8";
          };
          # We need unzip to build this package
          buildInputs = [ pkgs.unzip ];
          # Installing simply means copying all files to the output directory
          installPhase = "mkdir -p $out; cp -R * $out/";
        })
      ];
    }
  ];

And now my config using the new service:

  services.wordpress.enable = true;
  services.wordpress.uploadsDir = "/data/uploads";
  services.wordpress.database.user = "wwwrun";
  services.wordpress.database.passwordFile = pkgs.writeText "insecure-password-file" "password";
  services.wordpress.database.createLocally = false;
  services.wordpress.virtualHost.hostName = "localhost";
  services.wordpress.virtualHost.adminAddr = "admin@example.com";
  services.wordpress.themes = [
    # For shits and giggles, let's package the responsive theme
    (pkgs.stdenv.mkDerivation {
      name = "responsive-theme";
      # Download the theme from the wordpress site
      src = pkgs.fetchurl {
        url = http://wordpress.org/themes/download/responsive.1.9.7.6.zip;
        sha256 = "1g1mjvjbx7a0w8g69xbahi09y2z8wfk1pzy1wrdrdnjlynyfgzq8";
      };
      # We need unzip to build this package
      buildInputs = [ pkgs.unzip ];
      # Installing simply means copying all files to the output directory
      installPhase = "mkdir -p $out; cp -R * $out/";
    })
  ];

A few things worth noting.

  • There is no need to explicitly enable mysql or httpd as the module takes care of that for you, though you still can enable both these options.
  • If you never explicitly set wordpressUploads the default value has changed and you should set that. The permissions and ownership will automatically be modified.
  • I have modified the layout of the wordpress package so files are located under $out/share/wordpress instead of $out as that is standard, so you couldn't use an old version of wordpress with a new version of the module unless you modified the package to follow the layout accordingly.

Looking forward to hearing back from you about your config, etc...

@aanderse aanderse force-pushed the aanderse:wordpress branch from 820aa81 to eb4db49 May 31, 2019

@FPtje

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

Our config is scattered between some files (some files with options, some files with config for a specific machine). But basically, this is the gist of it:

let 
virtualHost = site : {
  hostName = site.hostname;
  listen = [ { inherit (cfg) port ip; } ];
  extraSubservices = [
    { serviceType = "wordpress";
      inherit (site) dbName dbPasswordFile themes plugins;
      wordpressUploads = wordpressUploads + "/" + site.hostname;
      languages = [ "en_GB" ];
      extraConfig = mkMerge [
        ''
          define('WP_SITEURL', '${site.siteUrl}');
          define('WP_HOME',    '${site.siteUrl}');
          define('WP_MEMORY_LIMIT', 'xxxM');
        ''
        (mkIf cfg.behindProxy ''
          ${optionalString site.enableSSL "define('FORCE_SSL_ADMIN', true);"}
          if ($_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https')
            $_SERVER['HTTPS']='on';
        '')
      ];
      extraHtaccess = ''
        php_value upload_max_filesize ...
        php_value post_max_size       ...
        php_value max_execution_time  ...
        php_value max_input_vars      ...
      '';
    }
  ];
};

sites = {
  "foo.example.com" = {
    dbName = "wordpress_example";
    inherit (cfg) dbPasswordFile;
    aliases = [ "foo.example.tk" ];
    themes = with pkgs.myCustom.wordpress.themes; [
      someTheme
    ];
    plugins = with pkgs.myCustom.wordpress.plugins; [
      foo 
      bar 
      baz
    ];
  };
};
in {
  services.mysql = {
    enable = true;
    package = pkgs.mysql;
  };
  services.httpd = {
    enable = true;
    listen = [{ inherit (cfg) port ip; }];
    logPerVirtualHost = true;
    adminAddr="foo@example.com";
    virtualHosts = map virtualHost (attrValues cfg.sites);
  };
}

One note here is that at certain places we do have multiple sites. In our case these sites are mapped to multiple virtualHosts. In this PR it seems that there's only one possible virtualHost.

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

@FPtje Thanks for posting this. I'll admit I'm a bit shocked someone is using this subservice so extensively. I'm glad NixOS has been providing value for you.

Regarding multiple sites I'm personally of the opinion NixOS is probably better off using containers to host multiple versions of a service instead of baking that ability right into the module. If you look over many of the web modules we have in NixOS you will notice the majority only let you create a single copy of the site.

Have you ever used NixOS containers before? Would that be an option for you? It's very important to me that we provide a reasonable transition to users of existing functionality but this change is important because extraSubservices is very old and out dated.

@FPtje

This comment has been minimized.

Copy link
Contributor

commented Jun 2, 2019

Ironically, it's the containers for our test cases that use multiple sites. Our production systems at this moment would not be affected by such limitation.

Regardless I would argue that it's a very common use case indeed. One that NixOS supports now and one that is quite easily supported on other distributions. The majority of web modules may not support running multiple sites, but is that a conscious decision? I know specifically with Wordpress there is very much a valid use case for running multiple sites. You can have different markets for different countries, running different websites with different content on www.YourCompany.<Country code>. Officially demanding the use of containers for such use cases seems remiss.

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

@FPtje You'll notice that in the link you provided there is no distribution support for running multiple sites, the tutorial simply has the user manually configure apache for multiple copies of wordpress... which is also quite easy to do in NixOS.

Let me give this some more thought. Thank you again for your feedback. I find it quite valuable.

@aanderse aanderse force-pushed the aanderse:wordpress branch from eb4db49 to e52ed8e Jun 2, 2019

@Infinisil

This comment has been minimized.

Copy link
Member

commented Jun 2, 2019

That's a problem in current NixOS in general, that all services really only support one instance, even if oftentimes multiple could be wanted. If people need this, then I think we should support it in the NixOS modules, and not make people use NixOS containers for that, especially because interacting with services in NixOS containers is much more troublesome (I think, I haven't used NixOS containers a lot).

@aanderse aanderse referenced this pull request Jun 9, 2019
6 of 10 tasks complete
@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2019

I've put together a first draft of multiple site support. Still a few things to be fixed up. Here is an example:

services.wordpress = {
  "site1.com" = {
    database.name = "site1";
    virtualHost.adminAddr = "admin@site1.com";
    virtualHost.hostName = "site1.com";
  };
  "site2.com" = {
    database.name = "site2";
    virtualHost.adminAddr = "admin@site2.com";
    virtualHost.hostName = "site2.com";
  };
};

Obvious room for improvement: default virtualHost.hostName to whatever the site is declared as. I hope to get to this as part of the cleanup over the weekend.

@aanderse aanderse force-pushed the aanderse:wordpress branch 2 times, most recently from 19d7dc2 to 7da4e90 Jun 22, 2019

nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/wordpress.nix Outdated Show resolved Hide resolved

@aanderse aanderse force-pushed the aanderse:wordpress branch 2 times, most recently from 480993d to 25680a1 Jun 22, 2019

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@FPtje Thanks to @Infinisil I have revised the code and I think it should be acceptable for your needs now. Basic example:

services.wordpress = {
  "site1.com" = {
    database.name = "site1";
    virtualHost.adminAddr = "admin@site1.com";
  };
  "site2.com" = {
    database.name = "site2";
    virtualHost.adminAddr = "admin@site2.com";
    extraConfig = ''
      define('WP_MEMORY_LIMIT', 'xxxM');
    '';
  };
};

Can you please take a look and if everything is acceptable test it out and let me know how it works for you?

@aanderse aanderse force-pushed the aanderse:wordpress branch from 25680a1 to 8ba3607 Jun 26, 2019

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

Just ran through an update from 5.1.1 to 5.2.1, then to 5.2.2 and it was quite smooth.

@FPtje is there anything else you require of this module before we merge? From my side the only thing left is to add something to the release notes.

@FPtje

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Sorry, I haven't had the chance to properly look at this. With the ability to have multiple sites, this PR looks good. I don't think I require anything else.

@aanderse aanderse force-pushed the aanderse:wordpress branch from 8ba3607 to 5ebde47 Jun 28, 2019

@aanderse aanderse force-pushed the aanderse:wordpress branch from 5ebde47 to 2f31bad Jul 3, 2019

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

@GrahamcOfBorg test wordpress

@aanderse aanderse force-pushed the aanderse:wordpress branch from 2f31bad to b9e6838 Jul 3, 2019

@aanderse aanderse merged commit c0a3236 into NixOS:master Jul 4, 2019

16 of 17 checks passed

tests.wordpress on x86_64-linux Timed out, unknown build status
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
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.wordpress on aarch64-linux Success
Details
wordpress on aarch64-linux Success
Details
wordpress on x86_64-linux Success
Details

@aanderse aanderse deleted the aanderse:wordpress branch Jul 4, 2019

@aanderse

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

@FPtje Thank you for your comments and review. I hope this module proves useful for you. Ping me if anything comes up which needs addressing during implementation.

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.