tomcat: rewrite module #84377

aanderse · 2020-04-05T18:39:58Z

Motivation for this change

The current tomcat module suffers from a few shortcomings, such as

it doesn't expose configuration of server-wide components such as Engine or Connector
state, configuration, logging, and applications are all stored in the same stateful directory which then necessitates less than desirable options like purifyOnStart
the code is extremely difficult to read and reason through properly, especially for those new to nix and NixOS
how to use the module is confusing and not well understood, deviating from NixOS norms
the module would benefit from a section in the manual explaining some basic use cases and how to configure them, avoiding potential confusion
to enable ssl the user is encouraged to place a secret (keystore or certificate password) on the nix store

Things I'm proposing

A rewrite of the module which lists tomcat use cases and then provides solutions to these use cases, addressing the above mentioned shortcomings of the current module.

After identifying some common use cases of tomcat that are relevant to me I have taken a very rough (and extremely incomplete) first pass at rewriting the module. Many options description values are empty or incorrect (copy+paste from another module), but I wanted to start a conversation with interested parties to see how we might improve tomcat on NixOS.

Here is what I came up with:

local development, ssl doesn't matter, you want to serve over port 8080 as is the default expressed in server.xml from upstream

# quick and easy, up and running on port 8080, the developer can work from appBase
services.tomcat.enable = true;
services.tomcat.virtualHosts.localhost = {
  appBase = "/var/www/tomcat";
};

run tomcat as a full featured web server on ports 80 and 443, PEM certificates provisioned by Let's Encrypt, requiring no keystore password in the nix store

# full Let's Encrypt integration out of the box, easy to host multiple sites, sane defaults but power to override most settings through various module options
services.tomcat.enable = true;
services.tomcat.virtualHosts."blog.example.org" = {
  enableACME = true;
  forceSSL = true;
  appBase = localpkgs.my-blog-war-file;
};
services.tomcat.virtualHosts."forum.example.org" = {
  enableACME = true;
  onlySSL = true;
  appBase = "/var/www/forum.example.org";
  sslHostConfig = {
      disableCompression = false;
      disableSessionTickets = true;
      honorCipherOrder = false;
  };
};

serve java content through a web server like nginx or apache-httpd via mod_jk or reverse proxy, where tomcat is installed on the same machine as your web server

# I'm not so familiar with AJP so I need to research this more
services.tomcat.enable = true;
services.tomcat.connectors = [
  { port = 8080; protocol = "HTTP/1.1"; }
  { port = 9009; protocol="AJP/1.3"; }
];
services.tomcat.virtualHosts.localhost = {
  appBase = "/webapps";
};
# configure your web server to connect to tomcat over the appropriate port, depending on reverse proxy or mod_jk

run several instances of tomcat over multiple servers and use something like apache-httpd or nginx to load balance them, possibly using a self signed certificate

# example to follow

What I want from you

@danbst @pvgoran @deliciouslytyped @tomberek @abathur as potentially interested individuals
Opinions, thoughts, comments, your use cases, module requests, examples, discussion of what I'm proposing is a good path forward or not. Minor review of code so far if you feel like it, keeping in mind it outs not complete. I have tested a few workflows, but not all yet.

Things done

abathur · 2020-04-05T19:40:32Z

Responding so you know I'm not ignoring you :)

I don't think I've used Tomcat via Nix or otherwise, but guessing you found me via #17879. Not sure I'll have much to add directly (since my interest is just transitively in finding out if Guacamole is a viable Teamviewer alternative), but @Shados and @jraygauthier may have thoughts arising from #48140.

pvgoran · 2020-04-06T03:10:37Z

The tomcat module is, indeed, quite limited, and arguably flawed, and rewriting it seems like a good idea. But I see multiple challenges/problems here.

First and foremost, existing configurations should remain working as they are, for at least one release cycle. Since the module's logic is completely replaced, probably the best way to achieve this would be to leave the existing module in place, and create a new one. (I'm not sure if this fits the establishes practices of NixOS.)
The old module uses tomcat's default configuration files and modifies them. The new module rebuilds the configuration files from scratch. There is a danger here that a new version of Tomcat may introduce some important contents in the default configuration file, NixOS may not notice this and use the old contents, and NixOS's tomcat instances would then lack this (presumingly important) configuration. I know that this is highly hypothetical scenario, but still it's possible, and has a potential of causing big problems.
Minor but important: the new module must escape XML attribute names and values. (Maybe introduce a new escaping function in lib/strings.nix?)
The location of Tomcat's stateful data shouldn't change for existing installations. (It won't be a problem if the old module is left in place, as suggested in (1).) At least, Tomcat should continue to write its logs to /var/tomcat/log, and store its session files in /var/tomcat/work.
4a. It would be good to allow specifying the location of stateful data (at least logs) via attributes. At the same time, it would to read these locations. (For example, I'm using something like "${config.services.tomcat.baseDir}/logs" to determine where the logs are, and to configure my log rotation scripts; after upgrade, I would use config.services.tomcat.logsDir.)
Minor: I don't like unconditional use of tomcat-native. Maybe it doesn't hurt, but I'm used to steer away from it. (It uses special kinds of connectors with unfamiliar, and possibly undesirable, properties.)
I'm using custom catalinaOpts and commonLibs, so I would like to see them configurable. (For completeness, sharedLibs and javaOpts should be configurable as well.)

(Also, the descriptions of extraConfig and logFormat currently refer to Apache.)

I don't have much time available, but I would like to follow the development of this module rewrite, and to help (at least by commenting/reviewing/testing).

aanderse · 2020-04-06T04:14:08Z

@pvgoran thanks for taking the time to comment. A couple questions:

are you using the tomcat module in production?
are you using tomcat directly as a web server, or are you using it with a reverse proxy, etc...?
are you storing any secrets in the nix store with this module?
what connectors and ports are you using?

pvgoran · 2020-04-06T04:27:14Z

are you using the tomcat module in production?

Yes.

are you using tomcat directly as a web server, or are you using it with a reverse proxy, etc...?

With a reverse proxy which lives on another machine connected via VPN. I'm also making local requests directly to Tomcat.

are you storing any secrets in the nix store with this module?

I don't think so. (SSL is handled by the reverse proxy.)

what connectors and ports are you using?

HTTP on 8080.

nixos/modules/services/web-servers/tomcat/default.nix

aanderse · 2020-04-17T00:28:23Z

Thanks for your comments @danbst. I've had some higher priority stuff get in the way of this lately but will eventually circle back around.

aanderse · 2020-05-12T01:30:59Z

@pvgoran thanks for your feedback. I've finally come back around to this PR. While still heavily WIP I'm making some progress and can somewhat address your points now.

I understand. Fortunately NixOS makes it trivial to pin modules so you can take as long as you need to migrate. See https://nixos.org/nixos/manual/#sec-replace-modules for details if you aren't already familiar with this.
tomcat is established and stable enough that dramatic changes aren't frequent. diff is a pretty solid tool for maintainers to guard against the type of problems you mention. Building server.xml the way this new module does is much more flexible and just as robust as the use of sed (which is scary! 😱) in the current module.
That could be useful, though not strictly required. Something to keep in mind.
I'm glad you mention the logs here. I hadn't added this option into my rewrite yet, but definitely should have. I have now added a logDir option which will store tomcat access logs. I'm still looking into the implications of the work directory. The way the current module copies configuration into stateful directories is bad. The new module has separated configuration from stateful data so configuration can be truly immutable, which is not only much cleaner, but a big win.
You are definitely correct - this shouldn't be forced upon users who don't want/need it. I have fixed this now. The idea is that if you are using tomcat as an actual web server (as opposed to pairing with nginx/httpd) you want to pull in tomcat-native, otherwise you don't.
catalinaOpts and javaOpts have been added back in 👍. Upstream suggests NOT using sharedLibs and commonLibs to a point where they have removed the way this worked in tomcat versions 6 or 7. Upstream suggests including jar files in your web application, which seems to fit the nix way of doing things reproducibly (build your apps with maven/gradle and specify all dependencies exactly). There is still the ability to do this (and I'll document it, it is relatively elegant) using catalina.properties but I don't think as a distro we want to promote things upstream is suggesting users steer clear of.

More to come.

pvgoran · 2020-06-03T03:52:27Z

@aanderse I didn't find time to reply yet. :(
But I'm going to do it.

Mic92 · 2020-06-03T06:55:54Z

nixos/modules/services/web-servers/tomcat/default.nix

+      ''
+  ;
+
+  serverXmlFile = pkgs.writeText "server.xml" ''


by the way builtins.toXML does not work for cases like this?
Than escaping would come for free and it might be also faster since it is directly implemented as a native function.

I'll have to give that a try. Thanks @Mic92!

stale · 2020-12-02T20:58:55Z

I marked this as stale due to inactivity. → More info

Mic92 · 2020-12-03T07:22:22Z

There is an temporary editor file committed that should be removed as well.

stale · 2021-06-02T16:04:48Z

I marked this as stale due to inactivity. → More info

aanderse · 2022-01-17T03:34:01Z

Unfortunately I don't have reason to finish this.

ofborg bot added 6.topic: nixos 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Apr 5, 2020

aanderse changed the title ~~Tomcat~~ tomcat: rewrite module Apr 6, 2020

aanderse force-pushed the tomcat branch from d166310 to be95e9e Compare April 8, 2020 00:36

danbst reviewed Apr 17, 2020

View reviewed changes

nixos/modules/services/web-servers/tomcat/default.nix Outdated Show resolved Hide resolved

danbst reviewed Apr 17, 2020

View reviewed changes

nixos/modules/services/web-servers/tomcat/default.nix Show resolved Hide resolved

danbst reviewed Apr 17, 2020

View reviewed changes

nixos/modules/services/web-servers/tomcat/default.nix Outdated Show resolved Hide resolved

danbst reviewed Apr 17, 2020

View reviewed changes

nixos/modules/services/web-servers/tomcat/default.nix Outdated Show resolved Hide resolved

aanderse force-pushed the tomcat branch from be95e9e to 6047da8 Compare May 9, 2020 13:52

ofborg bot added the 2.status: merge conflict label May 9, 2020

aanderse force-pushed the tomcat branch 2 times, most recently from c825617 to e4dbd9f Compare May 10, 2020 01:16

ofborg bot removed the 2.status: merge conflict label May 10, 2020

aanderse force-pushed the tomcat branch from e4dbd9f to 0bef255 Compare May 10, 2020 01:23

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 10.rebuild-linux: 1 10.rebuild-linux: 1-10 labels May 10, 2020

aanderse force-pushed the tomcat branch from 0bef255 to 8bb0e9d Compare May 12, 2020 01:26

nixos/tomcat: rewrite module

0f80afb

aanderse force-pushed the tomcat branch from 8bb0e9d to 0f80afb Compare May 20, 2020 00:21

ofborg bot added 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-linux: 0 labels May 20, 2020

pvgoran mentioned this pull request Jun 3, 2020

tomcat module doesn't expose configuration of server-wide components such as Engine or Connector #61225

Open

Mic92 reviewed Jun 3, 2020

View reviewed changes

stale bot added the 2.status: stale label Dec 2, 2020

stale bot removed the 2.status: stale label Dec 3, 2020

stale bot added the 2.status: stale label Jun 2, 2021

aanderse mentioned this pull request Jun 17, 2021

tomcat updates #127167

Merged

11 tasks

aanderse closed this Jan 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tomcat: rewrite module #84377

tomcat: rewrite module #84377

aanderse commented Apr 5, 2020

abathur commented Apr 5, 2020

pvgoran commented Apr 6, 2020

aanderse commented Apr 6, 2020

pvgoran commented Apr 6, 2020

aanderse commented Apr 17, 2020

aanderse commented May 12, 2020

pvgoran commented Jun 3, 2020

Mic92 Jun 3, 2020 •

edited

Loading

aanderse Jun 5, 2020

stale bot commented Dec 2, 2020

Mic92 commented Dec 3, 2020

stale bot commented Jun 2, 2021

aanderse commented Jan 17, 2022

tomcat: rewrite module #84377

tomcat: rewrite module #84377

Conversation

aanderse commented Apr 5, 2020

Motivation for this change

Things I'm proposing

What I want from you

Things done

abathur commented Apr 5, 2020

pvgoran commented Apr 6, 2020

aanderse commented Apr 6, 2020

pvgoran commented Apr 6, 2020

aanderse commented Apr 17, 2020

aanderse commented May 12, 2020

pvgoran commented Jun 3, 2020

Mic92 Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

aanderse Jun 5, 2020

Choose a reason for hiding this comment

stale bot commented Dec 2, 2020

Mic92 commented Dec 3, 2020

stale bot commented Jun 2, 2021

aanderse commented Jan 17, 2022

Mic92 Jun 3, 2020 •

edited

Loading