Modernize Debian and RPM packaging (systemd, dedicated user, CI tests) - #663
Modernize Debian and RPM packaging (systemd, dedicated user, CI tests)#663vharseko wants to merge 18 commits into
Conversation
Bring the opendj-deb/opendj-rpm packages up to current Linux packaging practice. The packaging had drifted from the ForgeRock era: jdeb 1.3 (2016), SysV-init only with direct update-rc.d/chkconfig calls, no Standards-Version, a 2015 changelog, and the server running as root. Service management: - Ship a native systemd unit (resources/systemd/opendj.service, Type=simple, start-ds --nodetach), kept alongside the SysV init script as a fallback for non-systemd hosts. - Register/enable/start via deb-systemd-helper / systemctl with an update-rc.d / chkconfig fallback; stop the service on removal. Dedicated service account: - Create an "opendj" system user/group and chown /opt/opendj to it; this also migrates previously root-owned installs on upgrade. - Run start-ds/stop-ds/upgrade as "opendj" from systemd, the maintainer scripts, and the SysV init script (new run_as helper, falls back to the current user when the account is absent). Build tooling and metadata: - jdeb 1.3 -> 1.14; ship the systemd unit from both deb and rpm. - control: add Standards-Version 4.7.3, Section net, a Debian revision in Version, Pre-Depends: adduser, and a newest-first JRE fallback list (default-jre-headless | ... | java25 | java21 | java17 | java11). - rpm: add Requires java-headless >= 1:11 and Requires(pre) shadow-utils. - Maintainer scripts hardened: set -e, fix invalid "exit -1", /var/run -> /run, idempotent guards. Changelog: - Generate the deb and rpm changelogs from the GitHub Releases via a new release-time helper (resources/generate-changelog.sh); committed output keeps the Maven build offline and reproducible. CI: - Add test-deb and test-rpm jobs (needs: build-maven) that install the built package, assert the opendj user and ownership, run setup, and start/stop the service (systemd on the runner; SysV in a Rocky Linux 9 container) with an ldapsearch liveness check.
91ada0f to
552a762
Compare
…e CI The dedicated `opendj` service user (PR OpenIdentityPlatform#663) runs setup/start-ds/stop-ds with a clean environment: no inherited JAVA_HOME, and `which` may be absent (e.g. minimal containers). OpenDJ's Java lookup then fell through to an error, because the package shipped config/java.properties with an unsubstituted placeholder (`default.java-home=$JAVA_HOME`) and the PATH fallback relied on the external `which`. Result: setup/start as `opendj` could not find Java. Fix Java discovery at the package level, using STABLE references so a JRE upgrade/reinstall does not break the service: - _script-util.sh: replace `which java` with `command -v java` (POSIX builtin, no dependency on the `which` package; resolves the stable /usr/bin/java alternatives symlink). This is the root-cause fix for the PATH fallback. - deb postinst / rpm %post: substitute `default.java-home` in config/java.properties with a stable symlink (/usr/lib/jvm/default-java, else the grandparent of `command -v java`, typically /usr) -- never a version- specific readlink path. Only the shipped placeholder is touched, so admin edits are preserved. - Ship an EnvironmentFile for admin overrides (OPENDJ_JAVA_HOME / OPENDJ_JAVA_BIN / OPENDJ_JAVA_ARGS): /etc/default/opendj on deb (conffile), /etc/sysconfig/ opendj on rpm (%config(noreplace)). The systemd unit reads it via EnvironmentFile=, and the SysV init script sources and exports it so the values survive the runuser switch to the service account. CI (.github/workflows/build.yml): - test-deb: run in a clean debian:12 container (install + SysV start/stop) plus a live `systemctl enable --now` on the runner. The container has no JAVA_HOME and no `which`, so it actually verifies the package configures Java itself. - test-rpm: drop the OPENDJ_JAVA_HOME band-aid; Java now comes from the package.
…privileged ports The service runs as the dedicated opendj user and so cannot bind privileged ports (LDAP 389, LDAPS 636) by default. Grant CAP_NET_BIND_SERVICE via AmbientCapabilities (and restrict CapabilityBoundingSet to it) so the non-root service can listen on those ports without running as root or applying setcap to the java binary.
…d opendj user The install guide described the old behavior (init.d, root-owned files, setup as root). Update the Debian and RPM sections to match the current packages: dedicated opendj system user, systemd service (systemctl) with a SysV fallback, JRE installed automatically via the package dependency, run setup as the opendj user, override Java via /etc/default or /etc/sysconfig/opendj, and CAP_NET_BIND_SERVICE for privileged ports.
New test-deb-upgrade (debian:12) and test-rpm-upgrade (rockylinux:9) jobs: install the released 5.1.1 package (root-owned tree, SysV only), configure an instance and start it, then upgrade to the newly built package while the server is RUNNING. The package must stop the server, create the opendj user, migrate file ownership, run the upgrade tool and restart the server; the jobs assert the pre-upgrade data is served again and the process now runs as opendj.
The released 5.1.1 scripts locate java via "which java", and the rockylinux:9 container does not ship the which utility, so the 5.1.1 setup failed with "Please set OPENDJ_JAVA_HOME". The new package does not need it (command -v + config/java.properties pinning - the very fix this PR delivers).
…anual installs from the deb/rpm tests - rpm: add Requires: util-linux - postinstall runs the upgrade tool and the server via runuser unconditionally. - test-rpm: no manual dependency installs; dnf must resolve everything from the package Requires (java-headless, util-linux, shadow-utils). - test-deb-upgrade: drop the manual JRE (the released 5.1.1 deb declares a JRE dependency itself) and procps (the process-owner check now uses /proc stat). - test-rpm-upgrade: keep only java + which, needed by the RELEASED 5.1.1 rpm which declared no dependencies at all (declarations added by OpenIdentityPlatform#677); drop them when the upgrade-source artifact link points to a release containing OpenIdentityPlatform#677.
/etc/redhat-release exists on every RHEL-family system, but the sourced /etc/init.d/functions only exists when the optional initscripts package is installed - so the init script aborted with "No such file or directory" on minimal systems. Check for the function-library files themselves and fall back to a self-contained default (the script does not depend on them). test-rpm-upgrade: the RELEASED 5.1.1 init script has the same defect and cannot be fixed, so install initscripts for the old package (alongside java and which), to be dropped when the upgrade-source artifact points to a release containing OpenIdentityPlatform#677.
Resolve build.yml conflict: keep master's build-docker-alpine benchmark steps and append the deb/rpm install and upgrade CI jobs.
…ribution # Conflicts: # .github/workflows/build.yml # opendj-packages/opendj-deb/resources/control/control # opendj-packages/opendj-rpm/pom.xml
…he build workflow CodeQL flagged the packaging test jobs for running with the default token permissions. The build workflow never uses GITHUB_TOKEN or any secret, so restrict it to contents: read at the workflow level.
…kages as the upgrade source The 5.1.2 rpm declares its runtime dependencies (OpenIdentityPlatform#677), so the manual dnf install of java/which/initscripts is no longer needed.
maximthomas
left a comment
There was a problem hiding this comment.
Good direction — a real systemd unit, a dedicated service account, command -v instead of which, and actual package tests in CI are all overdue, and the unit's mechanics (Type=simple + --nodetach, idempotent ExecStop, AmbientCapabilities) are correct.
But several of the new pieces don't do what their comments claim. The headline Java pin never executes on any code path; /etc/default/opendj is not actually a conffile; a plain apt install now leaves a service that fails on every boot; and the new opendj→root boundary is not enforced by the maintainer scripts. Most are one-to-three-line fixes.
The Java pin never runs, on any path (blocker)
opendj-packages/opendj-deb/resources/control/postinst:41-46 (identically opendj-packages/opendj-rpm/resources/specs/postinstall.sh:34-39):
JAVA_PROPS=${deb.prefix}/config/java.properties
...
if [ -n "$JH" ] && [ -x "$JH/bin/java" ] && grep -q '^default.java-home=\$JAVA_HOME' "$JAVA_PROPS" 2>/dev/null ; then
sed -i "s|^default.java-home=.*|default.java-home=$JH|" "$JAVA_PROPS"
fiTwo independent reasons it can never fire:
- Fresh install — the payload has no top-level
config/.opendj-server-legacy/src/main/assembly/opendj-archive-component.xml:217-224mapsresource/configtotemplate/config; the built tree (opendj-packages/opendj-deb/opendj-deb-standard/target/opendj/) isbin bat lib legal-notices snmp template— noconfig. Thegrepfails, thesednever runs. - After
setup—setuprewrites the file and drops the key entirely.InstallerHelper.java:807-810only re-addsdefault.java-homewhen the template lacks it, and the template has it; the writer atInstallerHelper.java:953-980truncates and rebuilds from comments +otherProperties+ args. Verified against a real post-setup artifact:opendj-server-legacy/build/unit-tests/quicksetup/OpenDS/config/java.propertiescontains nodefault.java-homeat all.
So Java is always resolved by command -v java (opendj-server-legacy/resource/bin/_script-util.sh:66) — exactly the fragility the PR set out to remove. opendj-packages/resources/env/opendj:4-6 states the opposite as fact, and the CI comments at .github/workflows/build.yml:638-639,700 assert it is tested; nothing in the workflow ever greps default.java-home out of the installed instance.
/etc/default/opendj is not a conffile (blocker)
opendj-packages/opendj-deb/pom.xml:184-193 relies on a comment that isn't true:
<!-- Service environment override (jdeb auto-marks files under /etc as conffiles) -->
<data>
<src>${env.file.location}</src>
...
<prefix>/etc/default</prefix>jdeb 1.14 populates conffileProducers only from Data.getConffile() — DebMojo.setData() calls getConffile() and nothing anywhere inspects the destination path. Confirmed empirically: the already-built opendj_*_all.deb in opendj-deb-standard/target/ ships /etc/init.d/opendj and its control.tar.gz contains no conffiles member.
Consequences: admin edits are overwritten on every upgrade with no .dpkg-dist, and the file is deleted on plain apt remove. The RPM side does this correctly — opendj-packages/opendj-rpm/pom.xml:268 <configuration>noreplace</configuration> — which makes the asymmetry clearly unintended. lintian would flag it, but .github/workflows/build.yml:632 runs it as lintian … || true.
<data>
<src>${env.file.location}</src>
<type>file</type>
<conffile>true</conffile>
<mapper>…</mapper>
</data>A plain install now leaves a service that fails on every boot (blocker)
opendj-packages/opendj-deb/resources/control/postinst:49-54 and opendj-packages/opendj-rpm/resources/specs/postinstall.sh:42-47 enable the unit unconditionally, but setup has not run and there is no config/ (see above).
sudo apt-get install ./opendj_5.2.0-1_all.deb
sudo reboot
systemctl status opendj → failed (Result: exit-code)
systemctl is-system-running → degraded
journalctl -u opendj → 5 failed starts 5s apart, "Start request repeated too quickly"
ExecStart=start-ds --nodetach --quiet cannot succeed without config/config.ldif, and Restart=on-failure/RestartSec=5 retries to the burst limit. The SysV path was benign here — instance_configured() (opendj-packages/resources/sysv/opendj:79-84) prints "Instance is not configured. Please run /opt/opendj/setup" and exits. The unit needs the equivalent:
ConditionPathExists=/opt/opendj/config/config.ldifpostinst aborts the dpkg transaction after unpacking (major)
The PR removed preinst's pre-flight abort but left postinst hard-failing, and the two now use different guards:
preinst |
postinst |
|
|---|---|---|
| master | buildinfo and non-empty archived-configs → stop; else exit -1 |
buildinfo |
| pr663 | same condition, no else |
buildinfo only (postinst:59) |
sudo apt-get install ./opendj_5.1.2-1_all.deb # never run setup
sudo apt-get install ./opendj_5.2.0-1_all.deb
→ preinst no-ops, dpkg unpacks the whole new tree, postinst:81 prints "Invalid installation…" and exit 1 → package left half-configured with the new files on disk; every subsequent apt run re-fails. Also reached by apt-get install --reinstall opendj. The RPM in the same state just prints "Instance is not configured." and succeeds (opendj-packages/opendj-rpm/resources/specs/postinstall.sh:69-71) — the deb should match.
The second divergence (empty archived-configs, e.g. after setup --doNotStart) is milder than it looks: upgrade self-defends via checkIfServerIsRunning (opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/Upgrade.java:939-958) and aborts. But postinst:73-78 then prints and exit 0, so dpkg reports success over an un-migrated instance carrying new jars.
Root executes opendj-writable scripts (major)
postinst:36 (chown -R opendj:opendj ${deb.prefix}) and postinstall.sh:29 cover the whole tree, including bin/ and lib/ shipped at mode 755 (opendj-packages/opendj-deb/pom.xml:265-281). But four call sites still run tree scripts as root:
opendj-packages/opendj-deb/resources/control/preinst:32 ${deb.prefix}/bin/stop-ds
opendj-packages/opendj-deb/resources/control/prerm:28 ${deb.prefix}/bin/stop-ds
opendj-packages/opendj-rpm/resources/specs/preinstall.sh:39 "%{_prefix}"/bin/stop-ds
opendj-packages/opendj-rpm/resources/specs/preuninstall.sh:33 "%{_prefix}"/bin/stop-ds
To be fair on severity: this is not a regression — on master the server ran as root outright (master:opendj-packages/resources/sysv/opendj:101) and the docs recommended it. It is incomplete hardening: the PR erects a boundary (User=opendj, CapabilityBoundingSet=, nologin) and leaves a root-execution hole through it. preinst/%pre run before the new payload lands, so the attacker-owned old script is what root executes.
The PR already has the fix pattern — postinst:61,69 use runuser -u opendj --. Apply it to those four lines, or better, declare ownership in package metadata (rpm <username>/<groupname> per mapping, jdeb <user>/<group>) and drop the chown — which also silences the rpm -V mismatch the chown currently produces on every packaged file.
Side effect worth noting: root-run stop-ds calls set_temp_dir (_script-util.sh:133-146), which can leave /opt/opendj/tmp root-owned and unwritable by the service.
<requiresPre> is not a plugin parameter (major)
opendj-packages/opendj-rpm/pom.xml:167-170. rpm-maven-plugin 2.1.5's mojo parameters are autoProvides, autoRequires, conflicts, obsoletes, prereqs, provides, requires, triggers — no requiresPre. Maven logs
[WARNING] Parameter 'requiresPre' is unknown for plugin 'rpm-maven-plugin:2.1.5:attached-rpm'
and proceeds, so CI stays green while the dependency is dropped entirely — shadow-utils appears nowhere else in the packaging. On a minimal image %pre's groupadd/useradd fail and User=opendj makes the unit unstartable. CI can't catch it: rockylinux:9 already ships shadow-utils.
The plugin's prereqs emits PreReq:, which rpm ≥4.8 rewrites to Requires(pre,preun):
<prereqs>
<prereq>shadow-utils</prereq>
</prereqs>The documented OPENDJ_JAVA_HOME override doesn't apply to the package's own upgrade (major)
chap-install.adoc tells admins to pin Java via /etc/default/opendj. The SysV script sources it (opendj-packages/resources/sysv/opendj:68-69) and the unit reads it via EnvironmentFile=, but no maintainer script does, and runuser hands over a clean environment:
runuser -u opendj -- ${deb.prefix}/upgrade -n --force --acceptLicense # postinst:61
runuser -u opendj -- ${deb.prefix}/bin/start-ds # postinst:69Host java is 8, admin pins 17 in /etc/default/opendj, systemctl start works. Next apt-get install ./opendj_<new>.deb → upgrade runs on Java 8 → fails → postinst:73-78 prints and exit 0, package marked installed, instance un-upgraded. On the non-systemd branch the server is then restarted on the wrong JVM. Same on RPM. Amplified by the dead Java pin above: command -v java is the only selector, and this file is the only documented way to change it.
Failed restart is silent and unretryable (major)
opendj-packages/opendj-deb/resources/control/postinst:66-71, same at opendj-packages/opendj-rpm/resources/specs/postinstall.sh:57-62:
deb-systemd-invoke start opendj.service || true
...
rm -f ${deb.prefix}/logs/statusMaster checked the return code and printed start-ds failed with return code …, removing logs/status only on success. Now a failed restart reports full success, prints nothing, and drops the "it was running before" flag — so dpkg --configure opendj won't retry either.
SysV start loses CAP_NET_BIND_SERVICE (major)
opendj-packages/resources/sysv/opendj:128 starts via run_as (line 86-98), which uses runuser/su. A root process's ambient set is empty to begin with, and the kernel clears it on the root→non-root transition anyway (cap_emulate_setxuid), so the JVM ends up with no capabilities. Only opendj.service:27 grants AmbientCapabilities=CAP_NET_BIND_SERVICE.
Upgrade a 5.1.x install serving 389/636 on a non-systemd host: postinst:69 restarts it with runuser -u opendj -- start-ds, LDAPConnectionHandler.java:636-645 hard-fails on EACCES, || true swallows it, directory down. The new CI upgrade jobs use port 1389 and can't catch this.
Two corrections to how this reads today: service opendj start and systemctl start opendj are not different on systemd hosts (Debian's service redirects to systemctl), and chap-install.adoc:447,536 does document the authbind/iptables workaround. The gap is that chap-upgrade.adoc:94 — rewritten by this PR, and the one place an upgrading admin looks — omits it.
postrm never disables the unit on remove (minor)
opendj-packages/opendj-deb/resources/control/postrm:21-35 calls deb-systemd-helper purge/unmask only under purge; the remove branch just does daemon-reload. dh_installsystemd masks on remove.
sudo apt-get install ./opendj_5.2.0-1_all.deb
sudo apt-get remove opendj
ls -l /etc/systemd/system/multi-user.target.wants/opendj.service # dangling
Every daemon-reload and boot then logs Unit file … is a dangling symlink, forever, because the admin never purges. The RPM (preuninstall.sh:27-30) leaves no residue.
prerm stops the server on deconfigure and nothing restarts it (minor)
opendj-packages/opendj-deb/resources/control/prerm:22 acts on remove or deconfigure; master acted on remove only. deconfigure is temporary — dpkg is unpacking something that Breaks opendj — and the restart at postinst:64 is gated on logs/status, which only preinst writes and only for $1 = upgrade. Net: dpkg --unpack of an unrelated package takes the directory down permanently.
Unquoted OPENDJ_JAVA_ARGS example breaks the init script (minor)
opendj-packages/resources/env/opendj:17 is consumed both by systemd EnvironmentFile= (space-tolerant) and by . /etc/default/opendj in the init script. Uncommenting
#OPENDJ_JAVA_ARGS=-server -Xmx2gparses as assignment-prefix + command. Verified on bash 3.2, dash, bash --posix and zsh: all print -Xmx2g: command not found and leave $OPENDJ_JAVA_ARGS unset — so export on line 70 exports nothing and the heap setting silently evaporates (the script has no set -e, and line 68 is [ -r … ] && . …, so the 127 is swallowed).
#OPENDJ_JAVA_ARGS="-server -Xmx2g"Also: the header says "Extra JVM args", but set_java_home_and_args (opendj-server-legacy/resource/bin/_script-util.sh:151-169) consults java.properties only when OPENDJ_JAVA_ARGS is empty — setting it replaces start-ds.java-args=-server.
RPM %post re-enables the service on every upgrade (minor)
opendj-packages/opendj-rpm/resources/specs/postinstall.sh:42-47 runs systemctl enable before $1 is ever tested (first tested at line 49). preuninstall.sh:24 gates its disable on $1 = 0, so the upgrade sequence never disables. An admin's systemctl disable opendj is undone by every dnf update. The deb is correct here — deb-systemd-helper enable records first-enable state and is a no-op afterwards. Gate on [ "$1" = "1" ] or use %systemd_post.
Docs: the documented install sequence can't produce the documented output (minor)
opendj-doc-generated-ref/src/main/asciidoc/install-guide/chap-install.adoc:456,467-468 (and :545,556-557 for RPM):
$ sudo -u opendj /opt/opendj/setup --cli
$ systemctl is-active opendj
active
setup starts the server itself unless --doNotStart is passed (InstallDS.java:792), outside systemd — so is-active prints inactive. The obvious recovery, systemctl start opendj, then exits 98 (DirectoryServer.java:222 SERVER_ALREADY_STARTED, returned from checkStartability) and Restart=on-failure drives the unit to failed. Every CI job passes --doNotStart; the docs should too, followed by systemctl start opendj.
chap-install.adoc:530 also still shows Pre Install - initial install, which the rewritten preinstall.sh no longer prints on $1=1.
CI proves less than it claims (minor)
.github/workflows/build.yml:667runssudo systemctl enable --now opendjby hand before assertingis-activeat:673— so deletingpostinst:51entirely leaves the only real-systemd job green. This is what hides the registration issues.:634dpkg-deb -c "$DEB" | grep -E "lib/systemd/system/opendj.service|etc/init.d/opendj"passes if only one is present — dropping the systemd<data>block still matches. Two separate greps.test-rpm(:681-716) runs only inrockylinux:9, where/run/systemd/systemdoesn't exist — so the RPM's systemd scriptlets, the unit, and%config(noreplace)on/etc/sysconfig/opendjare never exercised. Both*-upgradejobs are containers too, sopreinst:31deb-systemd-invoke stopandpostinst:67deb-systemd-invoke starthave zero coverage.:730,782curl -fsSL …/releases/download/5.1.2/opendj_5.1.2-1_all.deb— unpinned, unchecksummed release asset; a retag breaks both upgrade jobs for unrelated reasons.
Nits
- Undeclared deb dependencies:
opendj-packages/opendj-deb/resources/control/control:8declares onlyPre-Depends: adduser, whilepostinst:61,69needrunuser(util-linux) andpreinst:31/prerm:24/postinst:51/postrm:31-32needdeb-systemd-invoke/deb-systemd-helper(init-system-helpers). The RPM declaresutil-linuxexplicitly (opendj-rpm/pom.xml:161). Sincepreinstuses one, it needsPre-Depends. Every call is|| true-guarded, so a missing helper fails silently. - Stale
Requires: which:opendj-packages/opendj-rpm/pom.xml:158-159still requireswhichwith a comment saying_script-util.shuseswhich java— it now usescommand -v java. No shipped.shunderopendj-server-legacy/resourceuseswhichany more. - Stale changelogs:
opendj-packages/opendj-deb/resources/changelog:1tops out at5.1.1whilecontrolemits5.2.0-1, andtest-deb-upgradeinstalls 5.1.2, which has no stanza.generate-changelog.shis correct (I validated the generated output mechanically) but is wired into nothing. - Dead
INSTALL_ROOT:opendj-packages/resources/systemd/opendj.service:29—start-ds:29doesINSTALL_ROOT=\pwd`; export INSTALL_ROOT` before anything reads the inherited value. - Inert
TimeoutStartSec: line 39 withType=simple— the unit is "started" the moment the fork succeeds. Related:is-activecan reportactivebefore any listener is bound;Type=notifywould make both real. /run/systemd/systemgate is too broad:enable/disableare pure symlink operations and work fine without a booted systemd; onlydaemon-reload/start/stopneed the gate. Narrow in practice — the else branch registers a working SysV script, and no in-repo Dockerfile installs the deb/rpm.- Duplicated user creation:
opendj-packages/opendj-rpm/resources/specs/preinstall.sh:25-27and again verbatim inpostinstall.sh:26-28. chownerror handling diverges: debpostinsthasset -eand no guard (a failing chown aborts mid-configure); rpm has|| true.sufallback mangles arguments:opendj-packages/resources/sysv/opendj:96joins"$@"into one string thatsu -cre-parses. Latent (only--quietis passed today). Safe form:su -s /bin/sh "$RUNASUSER" -c 'exec "$0" "$@"' "$@".command -von legacy Bournesh:_script-util.sh:66—commandis absent from Solaris 10/bin/sh, and the repo still ships SMF handling (resource/bin/start-ds:44-58). Probably acceptable, worth a conscious decision.prermwon't stop an instance with emptyarchived-configson non-systemd hosts (prerm:26-27), so dpkg deletes/opt/opendjunder a live JVM. Pre-existing, not introduced — but it shares a root cause with the guard mismatch above and is worth fixing in the same pass.env/opendj:4-6asserts the package pointsjava.propertiesat the system JRE — false as shipped (see the first finding).chap-upgrade.adoc:94omits the privileged-port caveat thatchap-install.adoc:447,536states.- Duplicated CI wait loop: the same
for i in $(seq 1 20) … ldapsearch … sleep 3block appears four times in.github/workflows/build.yml. - Purge message overstates:
postrm:29-34prints "OpenDJ successfully removed" whileconfig/keystore.pin,config/config.ldif(Directory Manager hash),db/,changelogDb/and theopendjaccount all remain.chap-uninstall.adocdocuments this as intentional, so just soften the message.
… conditions, CI assertions - Drop the dead default.java-home pin (unreachable: the payload has no config/, and setup rewrites java.properties without the key); the service uses the PATH java from the package JRE dependency, and maintainer scripts now source /etc/default/opendj (/etc/sysconfig/opendj) so the documented override also applies to package upgrade and restart - deb: mark /etc/default/opendj as a real conffile (jdeb never infers them from /etc); Pre-Depends: init-system-helpers - systemd unit: ConditionPathExists=config/config.ldif stops restart bursts on unconfigured installs; drop dead INSTALL_ROOT and inert TimeoutStartSec - deb postinst: do not fail dpkg after unpack on an unconfigured instance (match rpm); check the restart return code and keep logs/status on failure; register via deb-systemd-helper unmask/was-enabled + update-rc.d without the booted-systemd gate - preinst/prerm/%pre/%preun: stop keyed on logs/server.pid and run stop-ds as the tree owner, never root; prerm acts on remove only - postrm: mask the unit on remove; truthful purge message - rpm: prereqs shadow-utils (requiresPre is not a plugin parameter); drop stale Requires: which; enable the unit only on initial install; drop the duplicated user creation in %post - sysv: su fallback preserves argv; env file: quote OPENDJ_JAVA_ARGS example - CI: assert conffiles and both service files; exercise postinst enable (is-enabled + pre-setup start) instead of manual enable --now; sha256-pin the 5.1.2 upgrade sources; dedup wait loops into wait-for-ldap.sh - docs: setup --doNotStart + systemctl start; privileged-port caveat in the upgrade chapter; regenerate package changelogs (5.1.2) + 5.2.0 stanza
|
Thanks for the thorough review — every finding checked out. All of them are addressed in 794154a; point-by-point below. Blockers
Majors
Minors / nits — all applied: Consciously unchanged
|
maximthomas
left a comment
There was a problem hiding this comment.
The systemd migration is correct for a fresh install with the default all-in-one layout. The upgrade and non-default paths have gaps — two of them end with a server that silently never starts, and CI cannot see either because every upgrade job runs in a container with no booted systemd.
RPM upgrade never enables the unit (blocker)
opendj-packages/opendj-rpm/resources/specs/postinstall.sh:42
if [ "$1" = "1" ] && command -v systemctl >/dev/null 2>&1 ; then
systemctl enable opendj.service >/dev/null 2>&1 || true
fiUpgrading from any pre-systemd OpenDJ RPM runs %post with $1=2, so enable is skipped. The newly shipped opendj.service now shadows the systemd-sysv-generator unit, so the existing chkconfig rc-links stop enabling anything and no multi-user.target.wants/opendj.service symlink is ever created → systemctl is-enabled opendj = disabled, and after the next reboot the server does not come up.
The deb side is safe only by accident: deb-systemd-helper was-enabled defaults to enabled when no state file exists (postinst:54). The intent — "an admin's systemctl disable survives upgrades" — needs the same state tracking on rpm, e.g. systemctl preset opendj.service on $1 = 1 and a [ ! -e /etc/systemd/system/multi-user.target.wants/opendj.service ]-style first-time check on upgrade.
ConditionPathExists breaks the split instance layout (blocker)
opendj-packages/resources/systemd/opendj.service:23
ConditionPathExists=/opt/opendj/config/config.ldifThe split layout (instance.loc → e.g. /var/opendj, read by _script-util.sh:321,345 and UpgradeUtils.java:166) is supported and documented. With it, /opt/opendj/config/config.ldif never exists, so the condition is permanently false: systemctl start opendj exits 0, the journal says Condition check resulted in ... being skipped, and nothing runs — a silent no-start with a success exit code.
Same assumption elsewhere in this PR:
postinst:67/postinstall.sh:53—[ -f .../config/buildinfo ]is false →Instance is not configured., soupgradeis never run on a genuinely configured instance.chown -R opendj:opendj ${deb.prefix}(postinst:37,postinstall.sh:27) leaves the real instance root owned by root, so the non-root service cannot write it.
Resolve instance.loc first (as the shipped scripts already do) and key all three on the resolved instance root.
stop-ds runs as the tree owner, not the process owner (major)
opendj-packages/opendj-deb/resources/control/preinst:35 (and prerm:31, preinstall.sh:42, preuninstall.sh:35)
OWNER=$(stat -c %U ${deb.prefix}/bin/stop-ds 2>/dev/null || echo root)
if [ "$OWNER" != root ] && command -v runuser >/dev/null 2>&1 ; then
runuser -u "$OWNER" -- ${deb.prefix}/bin/stop-ds || trueThe owner of a file is not the owner of the process. After a previous configure chowned the tree to opendj, an admin who started the server manually as root gets OWNER=opendj, so stop-ds issues kill <root-owned-pid> → EPERM. || true swallows it, logs/status was already touched, and dpkg proceeds to unpack the new tree on top of a live JVM. The mirror case (root-owned tree, server running as opendj) fails identically.
OWNER=$(stat -c %U /proc/$(cat ${deb.prefix}/logs/server.pid) 2>/dev/null || echo root)…and verify the stop actually happened (pid file gone) before letting the unpack continue.
deb-systemd-invoke exit code is not proof of start (major)
opendj-packages/opendj-deb/resources/control/postinst:76
deb-systemd-invoke start opendj.service && STARTED=1 || truedeb-systemd-invoke exits 0 without doing anything when policy-rc.d denies the action (the standard 101-returning policy-rc.d in image/chroot builds, with /run/systemd/system present). Result: STARTED=1, logs/status is deleted, no failure message — the operator is told the upgrade restarted the server while it is down and the retry marker has been discarded. Check systemctl is-active opendj (or the pid file) after the invoke instead of trusting the exit code.
logs/status is never cleared, so a later upgrade starts a deliberately stopped server (major)
opendj-packages/opendj-deb/resources/control/preinst:25-27 (and preinstall.sh:32-35)
The flag is only ever created. postinst:82-86 / postinstall.sh:67 now deliberately keep it after a failed restart, which makes it sticky across transactions:
- Upgrade #1, server running →
touch logs/status; the restart fails → flag kept. - Admin fixes the problem, starts the server, later stops it for maintenance.
- Upgrade #2:
[ -f logs/server.pid ]is false, nothing is touched — but the stale flag is still there, sopostinst:72starts a server the admin intentionally left down.
Add rm -f ${deb.prefix}/logs/status to the else branch of the running check (and in prerm / %preun).
ExecStop is redundant and has no stop timeout (major)
opendj-packages/resources/systemd/opendj.service:38
ExecStop=/opt/opendj/bin/stop-ds --quietThe server already handles SIGTERM via its shutdown hook and systemd's default KillSignal is SIGTERM, so this only adds work: stop-ds spawns one JVM for StopDS --checkStoppability, then kill <MAINPID>, then another JVM for WaitForFileDelete. On a large JE instance two cold JVM starts plus the backend close can exceed DefaultTimeoutStopSec (90 s) — systemd then SIGKILLs the cgroup mid-flush, forcing JE recovery on the next start. Drop ExecStop, and set an explicit TimeoutStopSec= sized for the backend either way.
--quiet on ExecStart empties the journal (major)
opendj-packages/resources/systemd/opendj.service:37
ExecStart=/opt/opendj/bin/start-ds --nodetach --quiet--nodetach --quiet selects START_AS_NON_DETACH_QUIET (DirectoryServer.java:5259, exit 104); bin/start-ds then execs the JVM with > /dev/null, and DirectoryServer.java:5020-5030 re-points System.out/System.err at logs/server.out. A start failure (bad Java, port in use, corrupt config) therefore leaves the unit failed with zero journal output and no hint where the real message is. Dropping --quiet (exit code 100) keeps stdout on the terminal so systemd captures it.
SysV instance_configured() was left on archived-configs (major)
opendj-packages/resources/sysv/opendj:108
instance_configured() {
if [ -f $INSTALL_ROOT/config/buildinfo ] && [ "$(ls -A $INSTALL_ROOT/config/archived-configs)" ] ; then
return 1
fi
return 0
}This PR deliberately moved every other gate off archived-configs (it can be empty on a freshly set-up instance) — the init script was missed. If archived-configs is empty or absent (pruned, PROPERTY_MAINTAIN_CONFIG_ARCHIVE off, restore from a backup without it), start|stop|status|restart all print Instance is not configured. and return 1. stop returning 1 means the runlevel-6 K-script never stops a running server at shutdown.
The inverted convention (return 1 = configured) with four if [ $? = 0 ] call sites is worth fixing at the same time.
update-rc.d opendj remove is in the wrong branch (minor)
opendj-packages/opendj-deb/resources/control/postrm:30-36
if [ -d /run/systemd/system ] ; then
systemctl --system daemon-reload >/dev/null 2>&1 || true
else
update-rc.d opendj remove >/dev/null 2>&1 || true
fipostinst:60 runs update-rc.d opendj defaults unconditionally, so every host has rc-links — but they are only removed when systemd is not booted. On a normal Debian/Ubuntu box apt-get remove opendj deletes /etc/init.d/opendj and leaves /etc/rc[0-6S].d/[SK]??opendj dangling. The two actions are orthogonal; update-rc.d ... remove must run in both branches.
CI does not exercise the new behaviour (minor)
.github/workflows/build.yml
test-rpm(690),test-deb-upgrade(731),test-rpm-upgrade(778) run in containers with no booted systemd, so thedeb-systemd-invoke start/systemctl startrestart paths and the enable gating never execute — which is precisely why the blocker above is invisible.- The only live-systemd job (658) binds port 1389, so
AmbientCapabilities=CAP_NET_BIND_SERVICE— the reason the service can stop running as root — is never tested.
--ldapPort 389 in the live-systemd job plus a systemctl is-enabled opendj assertion after a systemd-hosted upgrade would close both.
Nits
- rpm enable/disable gate asymmetry:
postinstall.sh:42enables without a booted-systemd gate (by design), butpreuninstall.sh:27only disables inside[ -d /run/systemd/system ].rpm -ein the same chroot leaves a danglingmulti-user.target.wantssymlink that systemd logs on every boot of the resulting image. Requires: initscripts/Requires: chkconfig(opendj-packages/opendj-rpm/pom.xml:163): c9b670d made the init script pick its function library by file existence with an explicit "No init function library: the script is self-contained anyway" fallback, so the hard deps now only block installs on ubi-micro/minimal and the RHEL-10 SysV-removal track. Make them soft or drop them./etc/default/opendjhas two parsers: systemd'sEnvironmentFile=and plain.sourcing (sysv/opendj:68,postinst:43,postinstall.sh:33). The header comment asks for quotes, but a systemd-legalOPENDJ_JAVA_ARGS=-server -Xmx4gtruncates to-serverand tries to run-Xmx4gas a command — hidden entirely by|| truein the maintainer scripts, so the package-driven restart silently uses different JVM args than systemd. Grep/export the three known keys instead of sourcing.- Stale uninstall transcript:
opendj-doc-generated-ref/src/main/asciidoc/install-guide/chap-uninstall.adoc:120,127still shows*Stopping OpenDJ server...and*OpenDJ successfully removed. The newprermechoes nothing andpostrmprints only in thepurgebranch —dpkg -ris now silent, so users following the guide will think it failed. (This chapter is touched by the PR at line 132.) - Version metadata:
opendj-packages/opendj-deb/resources/control/control:2declares5.2.0-1whileopendj-packages/opendj-deb/resources/changelog:1isopendj (5.2.0) UNRELEASED(lintianunreleased-changelog-distribution, currently masked bylintian ... || true), andopendj-packages/opendj-rpm/resources/changeloghas no 5.2.0 stanza at all (rpmlintincoherent-version-in-changelog).Standards-Version:(control:6) is a source stanza field — in a binary control file dpkg just carries it verbatim.
…split layout, live-systemd CI - rpm: migrate the chkconfig enable state to the native unit on the first upgrade from a pre-systemd package (%pre marker + rc-link check) - resolve instance.loc in the unit (ExecCondition + ConditionPathExists OR-group) and in every maintainer script: config gates, chown, status and pid files now use the real instance root - stop the server as the owner of the process, not of the tree; key "running" on a live /proc pid and verify the stop before dpkg/rpm may touch the files; clear a stale restart flag when the server was down - deb postinst: verify the restart via systemctl is-active (policy-rc.d can deny with exit 0); rpm does the same - unit: drop ExecStop (SIGTERM shutdown is graceful) and --quiet (start failures must reach the journal); TimeoutStopSec=300 for JE close - sysv: instance_configured() on buildinfo+config.ldif with the natural return convention; %preun disables without the booted-systemd gate; postrm removes rc links on every host; drop the hard chkconfig and initscripts Requires; parse /etc/default|sysconfig/opendj for the three OPENDJ_* keys instead of sourcing it - versions: 5.2.0-1 stanzas (deb unstable + rpm), -1 revision everywhere incl. generate-changelog.sh; drop Standards-Version from binary control - docs: truthful dpkg -r / rpm -e transcripts; softer %postun message - CI: live-systemd job binds port 389 (CAP_NET_BIND_SERVICE), deb upgrade runs on the runner's systemd with is-enabled/is-active asserts, rpm jobs assert the enable registration and migration
|
@maximthomas Thanks — every point of the second review is addressed in 7d46ecf. RPM upgrade never enables the unit (blocker) —
stop-ds runs as the tree owner (major) — all four scriptlets now key on a live
Sticky
SysV
CI (minor) — the live-systemd deb job binds port 389, so Nits — Verified locally: shell syntax on all scriptlets; sandbox simulations of the preinst stop/verify/stale-flag scenarios (including the refuses-to-stop abort) and of the |
…ribution # Conflicts: # .github/workflows/build.yml
Why
The
opendj-deb/opendj-rpmpackaging had drifted from current Linux practice: jdeb 1.3 (2016), SysV-init only with directupdate-rc.d/chkconfigcalls, a 2015 changelog, the server running as root, and the install guide never reflecting any of this. CI also built the.deb/.rpmbut never installed or tested them.What changed
Service management
resources/systemd/opendj.service,Type=simple,start-ds --nodetach, graceful SIGTERM stop withTimeoutStopSec=300), SysV init kept as a fallback for non-systemd hosts.setupdoes not burst-fail at boot: anExecCondition(backed by aConditionPathExists=|OR-group) resolves the instance rootinstance.loc-aware, exactly as_script-util.shdoes, so the split instance layout works too.deb-systemd-helper/systemctl(state-preserving: an admin'ssystemctl disablesurvives upgrades, and the first upgrade from a pre-systemd rpm migrates thechkconfigenable state to the native unit) with anupdate-rc.d/chkconfigfallback; stop on removal, mask on debremove.AmbientCapabilities=CAP_NET_BIND_SERVICE(+CapabilityBoundingSet) so the non-root service can still bind privileged ports (LDAP 389, LDAPS 636) without running as root.Dedicated service account
opendjsystem user/group,chown /opt/opendj— and a relocatedinstance.locroot — to it (migrates root-owned installs on upgrade); runstart-ds/upgrade/the service asopendj, and stop a running server as the owner of its process, verified stopped before dpkg/rpm may touch the files.Java discovery (robust to JRE upgrade/reinstall)
_script-util.sh:which java→command -v java(no dependency on thewhichpackage; resolves the stable/usr/bin/javaalternatives symlink that the package's JRE dependency guarantees).OPENDJ_JAVA_HOME/OPENDJ_JAVA_BIN/OPENDJ_JAVA_ARGS) live in/etc/default/opendj(deb, conffile) //etc/sysconfig/opendj(rpm,%config(noreplace)), read by the systemd unit and parsed (the three known keys, optional quotes stripped) by the init script and the maintainer scripts — so a pinned Java also applies to the package-drivenupgradeand restart.Build tooling & metadata
Section: net; Debian revision inVersionand coherent5.2.0-1changelog stanzas;Pre-Depends: adduser, init-system-helpers; JRE fallback throughjava25; rpmRequires: java-headless >= 1:11+Requires(pre): shadow-utils(viaprereqs);set -e, fixedexit -1,/run; changelogs regenerated from GitHub Releases (generate-changelog.sh, run at release time).Documentation (
install-guide/)opendjuser, systemd (systemctl) with SysV fallback, JRE installed automatically via the package dependency, runsetupasopendj, override Java via/etc/default|/etc/sysconfig/opendj,CAP_NET_BIND_SERVICEfor privileged ports.CI
test-deb(cleandebian:12container + livesystemctlon the runner) andtest-rpm(rockylinux:9container) jobs: install, assert theopendjuser + ownership,setup, service start/stop,ldapsearchliveness, cleanup. Package metadata is asserted too (conffiles member, both service files,shadow-utilsin Requires), and the live-systemd job exercises thepostinstregistration (is-enabled, start-before-setup must not fail the unit) and binds privileged port 389 to proveCAP_NET_BIND_SERVICE.test-deb-upgrade(the runner's live systemd) /test-rpm-upgrade(rockylinux:9container) jobs: install the released, sha256-pinned 5.1.2 package (root-owned, SysV-only),setup, leave the server running, upgrade to the freshly built package and assert the old data is served again — now by the dedicated user, withis-enabled/is-activeasserted after the deb upgrade and thechkconfig→systemd enable-state migration asserted on rpm.Reviewer notes
chown -R /opt/opendjon upgrade migrates existing root-owned installs.test-debinspects withdpkg-deb.build-docker-alpinefailures on some runs are an unrelated pre-existing flake; and a flaky macOS JMX test (JmxPrivilegeTestCase) can failbuild-mavenand skip thetest-deb/test-rpmdependents — re-run failed jobs if that happens.