RPM packages abuse scriptlets in violation of fedora/redhat packaging guidance

[drawks]

Overview of the Issue

When using the rpm packages as provided by this project during install or upgrade scriptlets present in the package

During installation/upgrade of the rpm packages provided by this project several scriptlets are run which do the following:

• conditionally create a consul system user and group
• create /opt/consul
• recursively change owner/group of /etc/consul.d and /opt/consul
• restart the consul service

While these actions are made with good intent they are in some cases fully in violation of the fedora/redhat packaging guidelines and in other cases implemented in a way which can create subtle and disruptive problems for users.

• User and group creation should probably be handled via the soft allocation strategy as is documented by fedora or if compatibility with older or more divergent redhat/fedora-alike distros should use the older example also provided by the Fedora project. Either methods prevents a number of corner cases related to user and group creation which have been well thought out and battle tested over many years and versions of Fedora/RedHat.
• The creation of /opt/consul in the rpm scriptlets prevents this directory from being owned by the package and also sidesteps the normal creation of the system user home directory. There is no hard requirement for this directory to exist for consul to run aside from the fact that this is the default directory used by the default consul configuration as shipped. As such it would be more appropriate for creation of thsi directory to happen in the systemd service unit as an ExecStartPre action which uses an Environment to set the desired path. This would allow operators to override the location of this directory. Something like:

  Environment=CONSUL_DATA_DIR=/opt/consul
  EnvironmentFile=-/etc/consul.d/consul.env
  User=consul
  Group=consul
  ExecStartPre=/usr/bin/mkdir -p ${CONSUL_DATA_DIR} #  or use `install` to be able to pass mode and owner/group
  ExecStart=/usr/bin/consul agent -config-dir=/etc/consul.d/

• Recursively changing ownership/group of /opt/consul and /etc/consul.d is actually VERY problematic because any file can be present in either of these directories and there should be a general expectation that file with a given owner/group and mode will retain that owner group and mode until an operator explicitly changes them (manually or config management etc). The current rpm can make files which were previously root owned and only readable by their owner spontaneously become readable by the consul user. The rpm package should only be managing ownership/group of its own packaged files. The spec file from which the rpm is generated does not appear to be present in this repository, but looking at the metadata using rpm --query we can see which config files are shipped by the package and marked as config files and that they are owned by root as opposed to consul:

  [root@39f66c0b1377 /]# rpm --dump -qp ./consul-1.15.2-1.x86_64.rpm
  /etc/consul.d/consul.env 0 1680198862 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 0100644 root root 1 0 1 X
  /etc/consul.d/consul.hcl 5154 1680198862 83a4925759f50cef5cae0b1f0fd8da542c95dd4a159be6e0e9ef0bd9af816a7d 0100644 root root 1 0 1 X
  /usr/bin/consul 139929233 1680198679 5c5fed218247eaf43c3b54008b6a4c5d5cfa1b38539d6e7bfc09ac04623389fc 0100755 root root 0 0 1 X
  /usr/lib/systemd/system/consul.service 499 1680198862 6e8f0cf14a554e6c634e90163f49430c21298841c779a9a2738773aa343ff092 0100644 root root 1 0 1 X
  [root@39f66c0b1377 /]# rpm -c -qp ./consul-1.15.2-1.x86_64.rpm
  /etc/consul.d/consul.env
  /etc/consul.d/consul.hcl
  /usr/lib/systemd/system/consul.service

  • (continued) it seems like it is probably reasonable to leave these as they are instead of chowning them since they are already readable by the consul user nor do they in their default state contain any secrets. Additionally since they are marked as config files, should an operator replace these files, change their owner/group/mode or modify the contents the rpm/yum/dnf package manager will not replace the locally modified files with their packaged defaults. This behavior is expected and safe as opposed to the current rpm's behavior.
  • Restarting consul on upgrade is actively harmful as it circumvents what should be a thoughtful and intentional decision. There are several places in the hashicorp documentation where it is explicitly stated that the order of restarts is meaningful during upgrades. Having the rpm/yum/dnf automatically restarting consul does not fit well into any of the documented upgrade procedures. Since it is possible to run consul in configurations where things like agent tokens aren't persisted to disk it is entirely possible that a working and usable consul daemon cannot be successfully restarted automatically without operator assistance AND no validation is done to the consul configuration on disk ahead of the restart, either of which situations will lead the existing package to stop a running consul daemon and then silently fail to start it. Actually the script actively blackholes the output from the restart and consumes/discards the returncode:

    
    case "$1" in
    purge | 0)
    userdel consul
    ;;

  upgrade | [1-9]*) if [ -d "/run/systemd/system" ]; then systemctl try-restart consul.service >/dev/null || : fi ;; esac

  
  ---

Reproduction Steps

1. Follow the installations steps as documented for any of the supported rpm based distributions (RHEL, CentOS, Fedora, Amazon Linux).
2. Run rpm -q --scripts consul and observe that all of the above details of how the rpm package scriptlets behave is true.

Operating system and Environment details

Any currently supported rpm based distribution as detailed in Hasicorp's packaging guide, currently:

Fedora	33, 34, 35, 36, 37
RHEL/CentOS	7, 8, 9
AmazonLinux	2, latest

[drawks]

Just after hitting "create" on this issue I also noticed that the scriptlets delete the consul user in the postuninstall scriptlet if they receive purge as the first argument to the script. Deleting users/groups in scriptlets is very expressly forbidden in the Fedora packaging guidelines, but this code path doesn't actually seem reachable as rpm/yum/dnf do not use the purge argument; it appears that this is perhaps an artifact of deb/apt packaging and/or the use of a tool like alien or fpm to generate multiple platform specific packages simultaneously. I'd advise against using such tools as they tend to result in the types of issues as I've detailed here.

[drawks]

I just spent a little while crawling through the github actions that are used to for building the release artifacts and see now that nfpm is being used to generate both deb and rpm packages. So naturally these scripts should be compliant with the policy requirements and expectations of both flavors of linux distro. I'll see if I can float a PR that addresses my above issues with the rpm packaging and also make similar changes where needed for debian policy compliance.