Ubuntu
tomcat9 package

Merge ~ahasenack/ubuntu/+source/tomcat9:kinetic-tomcat9-logging-fix into ubuntu/+source/tomcat9:ubuntu/devel

  1. Git
  2. lp:~ahasenack/ubuntu/+source/tomcat9
  3. kinetic-tomcat9-logging-fix
  4. Merge into ubuntu/devel
Proposed by Andreas Hasenack
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merge reported by: Andreas Hasenack
Merged at revision: 165edec1c2d4dced29ab2a01c4d45ccade3f68b6
Proposed branch: ~ahasenack/ubuntu/+source/tomcat9:kinetic-tomcat9-logging-fix
Merge into: ubuntu/+source/tomcat9:ubuntu/devel
Diff against target: 74 lines (+21/-4)
5 files modified
debian/changelog (+13/-0)
debian/control (+2/-1)
debian/logrotate.template (+2/-2)
debian/rsyslog/tomcat9.conf (+1/-1)
debian/tomcat9.postinst (+3/-0)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Christian Ehrhardt (community) Approve
Canonical Server Reporter Pending
Canonical Server Pending
Review via email: mp+425340@code.qastaging.launchpad.net

Description of the change

This fixes tomcat9's logging via ubuntu's unprivileged rsyslog.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/tomcat9-logging/

# Context

The package sets /var/log/tomcat9 to be 2770 tomcat:adm, so newly created files will inherit the adm group ownership.

There are 4 entities writing to /var/log/tomcat9 and they all need to agree on permissions and ownership:
- rsyslog: catalina.out only, runs as syslog:adm. It can write to /var/log/tomcat9 due to the adm group permission
- logrotate: catalina.out and its rotated logs only. Runs as root, can set any permission/ownership it wants
- java (tomcat9 itself): localhost*, catalina.<date>.log: runs as tomcat:tomcat, manages its own rotation of these logs. The log files end up being owned by group adm because of the group sticky bit in the /var/log/tomcat9 directory
- tomcat9's maintainer scripts (postinst): chown -Rh tomcat:adm /var/log/tomcat9

With the above in mind, consider that there are only two groups of files in /var/log/tomcat9:
- logs produced by the java process directly, which runs as tomcat:tomcat
- single log file written to by rsyslogd, which runs as syslog:adm

Our main problem is with the rsyslogd-produced log file: /var/log/tomcat9/catalina.out

I chose to alter 3 of these writers:
- rsyslog: drop the ownership setting of catalina.out. We run unprivileged and cannot set the owner to tomcat, which is what debian does
- logrotate: switch to syslog:adm. Without this, rsyslog won't be able to write to catalina.out. Here I could have just dropped the "create" and "su" lines, because logrotate will keep the permissions and ownership of the file it is rotating in the new empty file it creates. I also don't think we need to use "copytruncate", this seems to be old history from when catalina.out was produced by tomcat itself, and not via rsyslog. But one thing at a time.
- postinst: add another chown, specific to catalina.out*, to adjust the permissions back to syslog:adm. I could have replaced debian's chown -R with something more elaborate, that would loop over the logs and skip catalina.out*, or chown it differently, but here I wanted to keep our delta simple.

The above changes survive logrotation events ("logrotate -f /etc/logrotate.conf" can be used to test), package upgrade/reinstall events, and even a full rm -rf /var/log/tomcat9/* (i.e., it doesn't rely on pre-creating of log files with correct permissions before a daemon (rsyslog or java) can write to them). It also "fixes" previous installations which will start working wrt logging.

For testing, an easy way to trigger writing to /var/log/tomcat9/catalina.out (which is via rsyslogd) is to install or purge the tomcat9-admin package.

To post a comment you must log in.
Revision history for this message
Christian Ehrhardt (paelzer) wrote :

Wow there are really a lot of combinations of different actions and their order to consider here.
AFAICS you covered them all well and I approve +1

I have one minimal comment to enhance an explanation that you have made, but that is up to you if you agree.

For Kinetic this really is step forward.

I'm a bit scared of SRUing this despite seeming absolutely correct.
Changing permissions/ownership always has an odd feel of "what if someone manually adapted for this problem".
We have had this in the past and depending on the uncertainty level we guarded the changes by evaluating the situation if it matches the expected one (in this case perm/ownership) but back then all changes were in maintainer scripts, here with conf files and such this would not work.
Did you discuss the SRUability of this with anyone on the SRU team already?

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

Approvers: ahasenack, paelzer
Uploaders: ahasenack, paelzer
MP auto-approved

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

> Changing permissions/ownership always has an odd feel of "what if someone manually adapted for
> this problem".

I had no discussion yet about an SRU, and I share this concern. The biggest issue I think is the maintainer script, which even without our changes (pristine debian) is already forcing a bunch of chown/chmod everytime. Even if an admin changed config files, the maintainer script would still be running chmod/chown on /var/log/tomcat9. I don't really know how someone would have adapted and fixed their system in the meantime, unless they changed the logging directory to be somewhere else entirely. Then all the remaining changes are in config files, which a package upgrade wouldn't override.

I could refrain from my chown in postinst if I detect something changed, but then I would have to include debian's chown/chmod in that if/then/else clause.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

> back then all changes were in maintainer scripts, here with conf files and such this would not
> work.

Indeed, I was thinking of ways to check the config files, maybe a simple grep looking for the settings I'm changing (fileOwner in rsyslog snippet, and "create" in logrotate snippet), or even md5sums, but the biggest problem is that this are config file *snippets*, i.e., they could be named anything in a .d directory, or the user could have changed the main config file even (logrotate.conf or rsyslog.conf).

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Uploaded:

Uploading tomcat9_9.0.64-2ubuntu1.dsc
Uploading tomcat9_9.0.64-2ubuntu1.debian.tar.xz
Uploading tomcat9_9.0.64-2ubuntu1_source.buildinfo
Uploading tomcat9_9.0.64-2ubuntu1_source.changes

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

This migrated. It wasn't merged automatically because of the empty directory problem:

ERROR: Empty directories exist and will disappear on commit, causing
extraneous unintended changes:

webapps/examples/WEB-INF/lib

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches