[#1087] Copy the secret volume on every start of the Docker image

[vharseko]

Problem

• Docker image: keystores from SECRET_VOLUME are copied only on the first start, so a renewed certificate never reaches the server #1087: keystores and truststores from SECRET_VOLUME were copied only on the first start, and only after setup.sh had already started the server. The LDAPS handler had built its SSL context from the self-signed keystore by then, so the mounted certificate was served only from the first restart on. A renewed certificate never reached an instance on a persistent volume, because later starts execed the server before the copy.
• Docker image: a freshly bootstrapped container ignores SIGTERM and is killed without stopping the server #1085 (after the bootstrap run.sh ended in tail -f /dev/null with bash as PID 1, so the first container lifetime always ended with a kill) was fixed by [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098 while this PR was open. This PR started out fixing it too; since the rebase onto [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098 it relies on [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098's stop and only adds the copy of the secret volume before the server starts.

Change

opendj-packages/opendj-docker/run.sh:

• copy_secrets copies the key* / trust* files of SECRET_VOLUME that differ from those in ./data/config. Each file is written next to its target, set to mode 0600 and renamed over it, so the server never reads a half copied file. It succeeds only when it copied something, and sync_secrets repeats it until a pass finds nothing left to copy (at most 5 passes): a Secret updated between the copy of keystore and that of keystore.pin would otherwise leave a keystore of one version next to the password of the other.
• start_server copies the volume, starts watch_secrets in the background when SECRET_VOLUME_REFRESH (default 60 s, 0 disables) is set, logs that the volume is copied on start only when the value is not digits above 0 (1m, 30.5, "60 "), and execs start-ds --nodetach. Both the first start and every later start end there.
• watch_secrets runs sync_secrets every SECRET_VOLUME_REFRESH seconds, .pin files included. The server loads a key or trust store again, together with its PIN file, on the first handshake after either changed (A renewed key store or trust store file is not used until the server is restarted #1095, on master), so a renewed certificate is served without a restart, a new password included. A handshake that lands between the copy of a store and that of its .pin keeps the store loaded last and logs msgID 653 once; the next handshake after the second copy loads the new pair. (Before the last round the watcher held every store back while a password on the volume differed from the one the server started with, because the server kept that password and a rejected dsconfig change disabled the LDAPS handler, A rejected change to the LDAPS connection handler stops the handler listening until a later change is accepted or the server restarts #1109. [#1095] Load a file based key store or trust store again when the file changes #1101 and [#1109] Keep a connection handler listening when a change to it is rejected #1110 removed both reasons before this PR was merged.)
• On the first start, after the bootstrap, run.sh stops the server setup started before the .bootstrap-complete marker is written, as [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098 made it do on master. This PR ends that road in start_server rather than in a bare exec start-ds, so the first start copies the secret volume before the server starts, as every later start does, and the server is PID 1 on both roads. What happens when stop-ds fails is [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098's: the failure is logged and the start is tried anyway. (This PR had stop-ds || { …; exit 1; } from round 5; at the rebase it gave way to [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098's handling of the same case.) The line "The instance is bootstrapped, the health check may probe it" follows the marker.

README.md gets a "Certificates" section: what the volume holds, that with the default OPENDJ_SSL_OPTIONS the key entry may have any alias (with --generateSelfSignedCertificate setup binds no handler to one, even with a --certNickname; only a keystore of its own, --useJavaKeystore or --usePkcs12keyStore, with a --certNickname binds them), when it is copied, that a renewed store is served on the next handshake, a new password included, as long as the keystore keeps an alias the previous one used (a keystore with only new aliases is served from the next restart, as #1101 takes it only then), and a cert-manager setup (spec.keystores.pkcs12 plus a projected volume mapping keystore.p12 → keystore and the password → keystore.pin). The "Health check" section recommends docker run --init: the server is PID 1, and a JVM does not reap the processes a timed-out probe leaves behind. A PEM kubernetes.io/tls Secret cannot be used as is: OpenDJ reads keystores, and the Alpine image has no openssl for a conversion. SECRET_VOLUME_REFRESH is added to the variables table and to both Dockerfiles.

.github/workflows/build.yml gets a "Docker test secret volume" step in both docker jobs, between "Docker test replication" and "Docker test bootstrap LDIFs". It runs with set -E, so that its ERR trap prints the container logs for a failure inside its functions too. It mounts a keystore, whose key is under an alias other than setup's own server-cert, and a truststore at SECRET_VOLUME, and checks that:

• LDAPS serves the mounted certificate from the first start;
• PID 1 is java;
• the copied keystore.pin has mode 0600, and the truststore is copied;
• stop-ds ran before the instance was marked bootstrapped (the order of the two lines in the log);
• a renewed keystore (v2, same alias) reaches the data volume within a minute while the server runs and is served without a restart, and the keystore is copied twice in all (on the start and for the renewal), not on every pass of the watcher;
• a keystore renewed with a new password (v3, same alias) is copied together with its .pin (copied twice in all) and served without a restart;
• docker stop takes under 30 s and exits with 143, and after docker start v3 is still served;
• a keystore renewed with another password and a new alias while no container runs (v4) is served by a new container on the same data volume with SECRET_VOLUME_REFRESH=0, so it can only have been copied by the start over the existing instance;
• with SECRET_VOLUME_REFRESH=0, no watcher runs (no sleep process in the container), and a keystore renewed while the server runs is not copied.

The existing "Docker test" step is left as #1098 made it (online import, docker stop under 50 s and not with exit code 137, restart); this PR no longer changes it.

A certificate copied while the server runs is served without a restart thanks to #1095 (PR #1101, on master). #1101 takes a keystore while the server runs only if it holds a key under an alias the previous one did: checked on an image carrying #1101, v2 under a new alias stayed unserved (msgID 653, "holds no private key under any of the aliases"), while v3 under the first alias, and v4 with a new password as well, were served at once.

Verification

Images built from openidentityplatform/opendj:latest and :alpine with this run.sh (and the master health check), a JKS keystore + keystore.pin mounted read-only at /var/secrets/opendj, and SECRET_VOLUME_REFRESH=5:

check	before (latest)	after (latest and alpine)
first start, secret v1	self-signed certificate	CN=secret-v1
PID 1 after the bootstrap	bash	java (since #1098 on master as well)
docker stop of the first start	whole timeout, exit 137	3-5 s, exit 143 (since #1098 on master as well)
secret replaced with v2 while running	not copied	copied to the volume by the watcher
restart after v2	still CN=secret-v1	CN=secret-v2
v2 while running, on a server carrying #1101 (same alias)	-	CN=secret-v2 without a restart
no secret volume	self-signed	self-signed, clean stop
PKCS12 (openssl legacy and default encoding) named keystore, alias certificate	-	served
mode of the copied keystore / keystore.pin	-	0600 (BusyBox cp recreates the temp file, hence the explicit chmod)

openssl x509 -subject is called with -nameopt RFC2253: the runner's OpenSSL 3.0.13 prints CN = secret-v1 by default, 3.2+ prints CN=secret-v1.

The CI step was extracted from the workflow and run locally the way Actions runs it (bash -eo pipefail). The images were built from this branch's Dockerfiles around a server zip carrying #1101, whose extensions code and messages match master. The step passes on both images with this run.sh (132 s on latest, 199 s on alpine), and goes red against each of these run.sh mutants (run on latest):

mutant	red at
restart branch execs start-ds without copying	CN=secret-v4
the watcher leaves the .pin files alone (the hold of the earlier rounds)	the wait for keystore.pin of v3
watcher guard -ge 0 (a busy loop for 0)	the last ! cmp -s
0 taken for the default interval	! grep -sqx sleep
unchanged files copied again	the copy count (40 instead of 2)
trust* dropped from the glob	cmp of the truststore
the marker written before stop-ds	the order of the two log lines

In an earlier round, the LDAPS handler bound to server-cert during the bootstrap went red at the first healthy (LDAPS disabled, "The key with alias '[server-cert]' was not found"); v1 is still under an alias of its own.

--certNickname server-cert added to the default OPENDJ_SSL_OPTIONS is not a regression: for a self-signed certificate setup writes no nickname to the handlers, and the step stays green.

Against the published openidentityplatform/opendj:latest with the old run.sh the step fails at the first check (the self-signed certificate is served). A keystore without a friendlyName (alias 1, as cert-manager's go-pkcs12 writes it) is served as well: setup binds no connection handler to an alias.

All three docker steps of the workflow ("Docker test", "Docker test custom password", "Docker test secret volume"), extracted and run the same way against both images with the run.sh of the previous round, pass (the secret volume step has had two more checks since, with the times above; the other two steps do not mount a secret volume):

image	Docker test	custom password	secret volume
latest	97 s (docker stop 9 s, exit 143)	43 s	63 s
alpine	113 s (docker stop 9 s, exit 143)	54 s	75 s

Found while evaluating the Helm chart proposal (discussion #1079).

Fixes #1087

[vharseko]

@maximthomas, a round before your review: the first CI run of this PR failed in build-docker and build-docker-alpine, and the failure was caused by the PR. Fixed in 481572a.

What failed. The existing "Docker test" step runs stop-ds, an offline import-ldif / rebuild-index and start-ds inside the container. With this PR the server is PID 1 from the first start on, so stop-ds stopped the whole container and the next docker exec failed with container ... is not running. The step only ever worked because PID 1 was tail -f /dev/null (#1085).

What changed.

• .github/workflows/build.yml: "Docker test" now imports online, checks that docker stop -t 60 takes under 50 s and does not exit with 137, restarts the container, rebuilds the indexes online and checks the 10000 entries. The hunks are word for word those of [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098 (the same Docker image: a freshly bootstrapped container ignores SIGTERM and is killed without stopping the server #1085 change), so the workflow merges cleanly whichever of the two lands first. run.sh will still need a rebase for the second one.
• run.sh: stop-ds now runs before the .bootstrap-complete marker is written, not after it. Stopped after the marker, the health check could report the bootstrap's server healthy just before it went down (the race [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098 ran into: the next dsconfig fails with "Server Connection Closed").

Verified. All three docker steps of the workflow, run locally the way Actions runs them (bash -eo pipefail) against latest and alpine with this run.sh, pass: "Docker test" 97 s / 113 s (docker stop 9 s, exit 143), "custom password" 43 s / 54 s, "secret volume" 63 s / 75 s.

The PR description is updated to match.

[vharseko]

@maximthomas, rebased onto the current master (2ba918a) as 631d048, with no change to the fix itself.

The merge of #1091 made the second commit (Test the secret volume of the Docker image in CI) conflict in build.yml: both add a step after Docker test custom password in each docker job. Both steps are kept, Docker test arbitrary uid first and then Docker test secret volume. The first and the third commit applied cleanly and are unchanged.

What #1091 changes for this PR: its Docker test arbitrary uid step restarts a container under an arbitrary uid in group 0, so it also runs the start_server path of the new run.sh. copy_secrets writes into data/config, which that uid can write since #1091 gives /opt/opendj to group 0 with g=u.

[maximthomas]

praise: the start sequence is right where #1087 and #1085 broke it.

• ./bin/stop-ds runs before .bootstrap-complete is written (run.sh:134-140), so the health check never reports the bootstrap's server healthy just before it goes down.
• copy_secrets writes each file next to its target with mktemp + chmod 600 + mv -f (run.sh:56): the server never reads a half-copied keystore, and the key is readable by its user only.
• Both roads end in one start_server (run.sh:73-83), so the first start and every later one copy the volume the same way.

---

issue (blocking): the secret-volume step greps CN=secret-v1, but the runner's openssl x509 -subject prints CN = secret-v1.

.github/workflows/build.yml:588, :594, :605, :803, :809, :820

Both docker jobs run on ubuntu-latest (ubuntu-24.04), whose /usr/bin/openssl is 3.0.13. There get_nameopt() defaults to XN_FLAG_ONELINE (apps/lib/apps.c:198-200 at openssl-3.0.13), which includes XN_FLAG_SPC_EQ, so served() returns subject=CN = secret-v1. grep -q "CN=secret-v1" exits 1 under bash -eo pipefail, and build-docker and build-docker-alpine go red at the first certificate check, whatever LDAPS serves. The no-space form is the 3.2+ default (openssl-3.2.0 apps.c:189-192), which is what a Homebrew openssl prints locally. The step has not run in CI yet: run 36013203998 stopped at "Docker test".

          served() { echo | openssl s_client -connect 127.0.0.1:1637 2>/dev/null | openssl x509 -noout -subject -nameopt RFC2253; }

Or: grep -Eq 'CN ?= ?secret-v1' at each check. Same change in both jobs.

---

issue (blocking): the cert-manager recipe gives a keystore whose key has alias 1, not server-cert, so the LDAPS handler disables itself.

opendj-packages/opendj-docker/README.md:43, :66-83

Setup's self-signed options set the LDAPS handler's ssl-cert-nickname to server-cert (SecurityOptions.java:37, :104). cert-manager builds keystore.p12 with go-pkcs12 Encode, which puts only a localKeyID on the key bag and no friendlyName, and the JDK loads such a store (as PKCS12 or as JKS) with aliases=[1]. Once run.sh copies it over config/keystore, LDAPConnectionHandler.createSSLContext (:1330-1344) logs ERR_KEYSTORE_DOES_NOT_CONTAIN_ALIAS, drops the alias and disables the handler: a pod that follows the recipe serves no LDAPS, and the image's HEALTHCHECK, which probes LDAPS, never passes. CI misses this because its keystore() uses -alias server-cert. The same gap is in the prose at :43: nothing says which alias the key entry needs.

# the Certificate
spec:
  secretName: opendj-tls
  keystores:
    jks:
      create: true
      alias: server-cert
      passwordSecretRef: { name: opendj-keystore-password, key: password }
---
# the pod template of the StatefulSet
volumes:
  - name: secrets
    projected:
      sources:
        - secret:
            name: opendj-tls
            items: [{ key: keystore.jks, path: keystore }]
        - secret:
            name: opendj-keystore-password
            items: [{ key: password, path: keystore.pin }]

Also state at :43 that the key must be under alias server-cert. keystores.jks.alias needs cert-manager 1.15+. The other option is to set the LDAPS handler's ssl-cert-nickname to the alias the keystore actually has.

---

issue (non-blocking): nothing tests the copy on the restart road: the watcher has already copied v2 before docker stop.

.github/workflows/build.yml:598-605, :813-820; opendj-packages/opendj-docker/run.sh:92-100

The step waits at :598 until the watcher (SECRET_VOLUME_REFRESH=5) has copied keystore v2 into data/config, then stops and starts the container. At docker start, copy_secrets finds the files equal and copies nothing. A mutant whose [ -d ./data/config ] branch runs BASE's bare exec ./bin/start-ds --nodetach still serves CN=secret-v2 and stays green in both jobs. That is the restart-road half of #1087 ("later starts exec'ed the server before the copy"), and this step is its only test. Pin: recreate the container on the same data volume with the watcher off and a keystore renewed while it was down (after the -nameopt fix above):

          docker rm -f test_secret
          keystore secret-v3
          docker run -d --memory="512m" -e SECRET_VOLUME_REFRESH=0 -p 127.0.0.1:1637:1636 --name=test_secret \
            -v "$SECRETS":/var/secrets/opendj:ro -v test_secret_data:/opt/opendj/data "$IMAGE"
          healthy
          grep -q "CN=secret-v3" <<< "$(served)"

Pin: the restart branch without copy_secrets serves v2, and grep -q "CN=secret-v3" goes red.

---

suggestion (non-blocking): no test runs with SECRET_VOLUME_REFRESH=0, so nothing pins the -gt 0 guard.

opendj-packages/opendj-docker/run.sh:77; .github/workflows/build.yml:591, :806

SECRET_VOLUME_REFRESH appears in CI only as =5. If the guard is dropped, or turned into -ge, 0 becomes while sleep 0; do copy_secrets; done: a busy loop, where the README says 0 means "copies them on start only". Continue in the REFRESH=0 container from the previous comment. The negation goes inside sh -c because bash -e ignores a failing ! cmd:

          keystore secret-v4
          sleep 15
          docker exec test_secret sh -c '! cmp -s /var/secrets/opendj/keystore /opt/opendj/data/config/keystore'

Pin: with the guard dropped or -ge 0, the busy loop copies v4 within the 15 s and the docker exec goes red.

---

suggestion (non-blocking): a SECRET_VOLUME_REFRESH that is not a whole number turns the watcher off and logs nothing.

opendj-packages/opendj-docker/run.sh:75-79

[ "$SECRET_VOLUME_REFRESH" -gt 0 ] 2>/dev/null returns 2 for 1m, 60s and 30.5, all of which sleep would accept, and the error goes to /dev/null. The log line at :75 is printed either way. An operator who writes 1m gets no refresh and no hint of it.

    if [ "$SECRET_VOLUME_REFRESH" -gt 0 ] 2>/dev/null; then
      watch_secrets &
    elif [ "$SECRET_VOLUME_REFRESH" != 0 ]; then
      echo "SECRET_VOLUME_REFRESH=$SECRET_VOLUME_REFRESH is not a whole number of seconds, the secret volume is copied on start only"
    fi

---

suggestion (non-blocking): if the keystore password changes while the server runs, the server keeps the old PIN in memory, and it no longer opens the copied keystore.

opendj-packages/opendj-docker/run.sh:65-68

FileBasedKeyManagerProvider sets keyStorePIN only at init or on a config change (:90), but getKeystore() reads the keystore file again on every call (:129-137). After the watcher copies a keystore with a new password, a dsconfig change to the LDAPS, StartTLS or HTTPS handler rebuilds the SSL context from keystore v2 with PIN v1. The change is rejected (ERR_FILE_KEYMANAGER_CANNOT_LOAD) until the next restart. Serving is not affected. This cannot happen with the recipe's fixed passwordSecretRef. Two options: the watcher skips a pass in which keystore.pin differs from the one in data/config (the copy at the next start applies it, and that is when it takes effect anyway), or the README says that a new keystore password needs a restart.

---

suggestion (non-blocking): on the first start the JVM is now PID 1, and it does not reap orphans left by a health probe that timed out.

opendj-packages/opendj-docker/run.sh:142, :82; opendj-packages/opendj-docker/Dockerfile:83

The HEALTHCHECK is in shell form and ends in || exit 1, so sh forks ldapsearch, and _client-script.sh forks java. When a probe runs past --timeout=30s, Docker sends SIGKILL to the exec'd sh only. The rest of the chain is reparented to PID 1, and a JVM never calls waitpid, so each orphan becomes a zombie. At BASE, bash on tail -f /dev/null reaped them on the first lifetime. The restart road already had java as PID 1. Each zombie costs one pid slot, and Kubernetes does not use this HEALTHCHECK. One README line would do: recommend docker run --init, which puts a PID 1 there that reaps and forwards SIGTERM.

---

suggestion (non-blocking): keystore and keystore.pin are compared and renamed one at a time, so one pass can mix two versions of the Secret.

opendj-packages/opendj-docker/run.sh:52-56

If the projected volume's ..data swap lands between the two cmp calls during a password rotation, data/config holds keystore v1 with pin v2 until the next pass. Today this can bite only a start that straddles the swap. It matters once #1101 makes an in-run copy live: compare the whole key*/trust* set first, and copy only when a second compare agrees.

[vharseko]

@maximthomas, thanks for the review. Round 4 is in c099b70. Seven of the eight points are taken; the cert-manager alias one did not reproduce, details below.

CN=secret-v1 vs CN = secret-v1 (blocking): fixed. Confirmed in ubuntu:24.04: OpenSSL 3.0.13 prints subject=CN = secret-v1, and with -nameopt RFC2253 it prints subject=CN=secret-v1. My local runs used OpenSSL 3.6.3, which is why the step passed here. served() now passes -nameopt RFC2253 in both jobs.

The cert-manager keystore with alias 1 (blocking): not taken, the LDAPS handler is not bound to server-cert in this image. The image runs setup with OPENDJ_SSL_OPTIONS="--generateSelfSignedCertificate". For SELF_SIGNED_CERTIFICATE, Installer.addCertificateArguments (Installer.java:1589-1594) passes only -k and -t to ConfigureDS, with no -c. So the LDAPS and LDAP handlers get no ssl-cert-nickname at all. SELF_SIGNED_CERT_ALIAS is the alias setup gives the generated key, not a value written to a handler. With an empty nickname set, LDAPConnectionHandler.createSSLContext takes the aliases.isEmpty() branch (:1324-1327) and uses the key manager as it is. ERR_KEYSTORE_DOES_NOT_CONTAIN_ALIAS is never reached.

Measured: I built a PKCS12 with openssl pkcs12 -export and no -name, so there was no friendlyName and only a localKeyID, as go-pkcs12 writes it. keytool -list shows it as 1, PrivateKeyEntry. I mounted it at SECRET_VOLUME with this run.sh. The container turned healthy, LDAPS served subject=CN=noalias-v1, and the error log had no alias message. In a fresh instance of the image, config.ldif has ds-cfg-ssl-cert-nickname only on the Administration Connector (admin-cert) and the Crypto Manager (ads-certificate), not on the LDAPS or LDAP handler. The earlier check in the PR description also stands: a PKCS12 with alias certificate is served.

What I did take from it is the gap in the prose. README.md now says that with the default options the key entry may have any alias, and that if OPENDJ_SSL_OPTIONS sets --certNickname, the key has to be under that alias. The recipe keeps keystores.pkcs12, so it does not need cert-manager 1.15+.

The restart road was not tested (non-blocking): taken. After the restart with v2, the step now stops and removes the container. It then renews the keystore to v3 while no container runs, creates a new container on the same data volume with SECRET_VOLUME_REFRESH=0, and checks that LDAPS serves CN=secret-v3. With the watcher off, only the copy on the restart road can bring v3 in.

SECRET_VOLUME_REFRESH=0 (non-blocking): taken, in the same container: keystore v4 is written, and 15 s later docker exec test_secret sh -c '! cmp -s …' checks that it was not copied.

Both pins were checked against mutants, running the step locally the way Actions does (bash -eo pipefail) on latest and alpine:

run.sh	latest	alpine
this round	passes, 211 s	passes, 247 s
restart branch exec ./bin/start-ds --nodetach without the copy	red at grep -q "CN=secret-v3"	red at grep -q "CN=secret-v3"
guard -ge 0	red at ! cmp -s	red at ! cmp -s

SECRET_VOLUME_REFRESH that is not a whole number (non-blocking): taken as you suggested, with the message "is not a whole number of seconds above 0", so that -5 is covered too. I checked it on 60, 5, 0, 1m, 30.5 and -5: only 0 stays silent.

A new keystore password while the server runs (non-blocking): taken as a README sentence. "The same holds for a new keystore password: the server keeps the one it started with." With #1101, a handshake after the change reads the PIN again (currentKeyManager()), so the new password is picked up there. Only getKeyManagers(), which a dsconfig change of a handler calls, still loads the store with the PIN in memory until the next handshake. I noted that on #1101: #1101 (comment)

PID 1 and orphaned probes (non-blocking): taken. The Health check section of the README now recommends docker run --init (init: true in Compose).

keystore and keystore.pin of two versions in one pass (non-blocking): taken. copy_secrets now succeeds only when it copied a file. A new sync_secrets repeats passes until one finds nothing left to copy (at most 5), and both the start and the watcher use it. A pass that straddles the ..data swap copies keystore v1 and pin v2. The next pass sees that the keystore differs and copies v2, and the pass after that is empty. I checked this outside Docker with a cp that swaps the source files right after the keystore is copied: one copy_secrets leaves keystore=v1 pin=v2, and sync_secrets leaves keystore=v2 pin=v2.

The PR description is updated to match.

Edited: the first version said that #1101 reloads the store with the same keyStorePIN. It does not: it reads the PIN again. Only the narrower getKeyManagers() case above remains.

[vharseko]

@maximthomas, rebased onto the current master (67eb249) as 4abcc52, with no change to the fix itself.

The merge of #1094 made two commits conflict:

• run.sh (first commit, Copy the secret volume on every start…): [#1084] Keep the root password out of the log and off the command line when a Docker container joins replication #1094 adds a removal of the password file of replicate.sh from /dev/shm right where this commit adds the secret volume functions. Both are kept, the removal first. It still runs before anything else, and replicate.sh still runs after the bootstrap and before the stop-ds that hands over to start_server.
• build.yml (second commit, Test the secret volume of the Docker image in CI): both add a step after Docker test arbitrary uid in each docker job. Both steps are kept, Docker test replication first and then Docker test secret volume.

The third and the fourth commit applied cleanly and are unchanged.

[maximthomas]

praise: Round 1's blocking points are settled, and the new REFRESH=0 run pins the copy on an existing instance.

• served() now pipes through openssl x509 -noout -subject -nameopt RFC2253, so CN=secret-vN matches on the runner's OpenSSL 3.0 too.
• The REFRESH=0 container, created again on test_secret_data after keystore secret-v3, pins the start-time copy over an existing instance (your mutant table: red at CN=secret-v3).
• The README's alias paragraph is right, and my round-1 issue on the cert-manager alias is withdrawn: Installer.addCertificateArguments passes no nickname for SELF_SIGNED_CERTIFICATE, and ConfigureDS then removes ds-cfg-ssl-cert-nickname from the LDAP/LDAPS/HTTP/JMX handlers, so a key under alias 1 is served.

---

issue (non-blocking): The ERR trap of "Docker test secret volume" does not run for a failure inside healthy() or keystore().

.github/workflows/build.yml:627, :630-638, :901, :904-912

Actions runs the step as bash -eo pipefail without -E. A function does not inherit the ERR trap, and errexit ends the shell from inside the function. So a container that never turns healthy (timeout exits 124), or a failed keytool/mv, fails the step without the docker logs test_secret the trap is there to print. I checked this locally: bash -eo pipefail -c 'trap "echo ERR-TRAP-RAN" ERR; h() { bash -c "exit 124"; }; h' exits 124 and prints nothing, and with set -E the trap prints. Your other docker steps call timeout at top level, so their traps do run.

set -E
trap 'code=$?; echo "::group::container logs (test_secret)"; docker logs test_secret 2>&1 || true; echo "::endgroup::"; exit $code' ERR

---

issue (non-blocking): If the keystore password changes while the server runs, the next dsconfig change to the LDAPS handler shuts LDAPS down. The new README sentence does not warn about this.

opendj-packages/opendj-docker/run.sh:79-83, opendj-packages/opendj-docker/README.md:64-65

watch_secrets is on by default and copies keystore and keystore.pin into data/config while the server runs. FileBasedKeyManagerProvider reads the PIN only at init (:90), but it reloads the keystore file on every getKeystore() (:129-138). A dsconfig modify of the LDAPS handler goes through LDAPConnectionHandler2.isConfigurationAcceptable:584 → createSSLContext. There containsAtLeastOneKey swallows the wrong-password failure, and disableAndWarnIfUseSSL sets enabled = false on the live handler before the change is refused. run() then stops the listener, and LDAPS stays closed until a restart. The flag flip during the dry run is an older server bug, but before this PR nothing in the image rewrote data/config under a running server. You can fix this in either of two ways. One: the watcher leaves a changed password to the next start, which is when it takes effect anyway. Two: the README says to restart before any change to a TLS connection handler.

pins_unchanged() {
  local src
  for src in "$SECRET_VOLUME"/*.pin; do
    [ -f "$src" ] || continue
    cmp -s "$src" "./data/config/$(basename -- "$src")" || return 1
  done
}

watch_secrets() {
  while sleep "$SECRET_VOLUME_REFRESH"; do
    pins_unchanged && sync_secrets
  done
}

---

suggestion (non-blocking): The secret-volume step never changes keystore.pin and never mounts a truststore. So the sync_secrets repeat, the trust* glob and the skip of unchanged files are not pinned.

.github/workflows/build.yml:630-636, :904-910; opendj-packages/opendj-docker/run.sh:56-76

Three run.sh mutants pass both images' steps:

• sync_secrets() { copy_secrets; }. One pass copies both changed files.
• Dropping "$SECRET_VOLUME"/trust* from :56.
• Deleting cmp -s "$src" "$dst" && continue at :59. Every start and every 5 s tick then copies identical bytes five times, and no check looks at that.

A truststore in the volume pins the glob. Counting the copy log line pins the skip. The repeat needs a Secret that changes in the middle of a pass, which CI cannot produce on demand. Not run here (no Docker daemon).

keystore() {
  # ... as now, then:
  docker run --rm --entrypoint keytool -v "$SECRETS":/secrets "$IMAGE" -exportcert -rfc -alias server-cert \
    -keystore /secrets/keystore -storepass changeit -file /secrets/cert.pem
  rm -f "$SECRETS/truststore.new"
  docker run --rm --entrypoint keytool -v "$SECRETS":/secrets "$IMAGE" -importcert -noprompt -alias ca \
    -file /secrets/cert.pem -keystore /secrets/truststore.new -storetype JKS -storepass changeit
  mv -f "$SECRETS/truststore.new" "$SECRETS/truststore"
  printf changeit > "$SECRETS/truststore.pin"
  rm -f "$SECRETS/cert.pem"
}
# after the first `healthy`:
docker exec test_secret cmp -s /var/secrets/opendj/truststore /opt/opendj/data/config/truststore
sleep 12
test "$(docker logs test_secret 2>&1 | grep -c '^Copied keystore from the secret volume$')" -eq 1

Pin: the trust* mutant goes red at cmp, and the mutant without the skip goes red at the count (5 or more instead of 1).

---

suggestion (non-blocking): Every CI keystore puts its key under alias server-cert, so nothing in CI pins the README's "the key entry may have any alias".

.github/workflows/build.yml:631, :905; opendj-packages/opendj-docker/README.md:50-52

server-cert is also the name setup gives its own key. Suppose a regression binds the handlers to that name, for example a -a server-cert in the SELF_SIGNED_CERTIFICATE case or a --certNickname server-cert in the default OPENDJ_SSL_OPTIONS. CI would still serve CN=secret-v1..v3. A cert-manager keystore.p12 (alias 1) would lose LDAPS. The "alias 1 is served" result in the PR body came from a local run.

docker run --rm --entrypoint keytool -v "$SECRETS":/secrets "$IMAGE" -genkeypair -alias "$1" \

Pin: with a per-version alias, the handler-binding mutant fails at CN=secret-v1. If you also take the truststore pin above, use -alias "$1" in its -exportcert.

---

suggestion (non-blocking): The SECRET_VOLUME_REFRESH=0 check waits 15 s, and the default interval is 60 s. A mutant that treats 0 as "use the default" passes.

.github/workflows/build.yml:663-664, :937-938

Take [ "$SECRET_VOLUME_REFRESH" -gt 0 ] 2>/dev/null || SECRET_VOLUME_REFRESH=60 added after run.sh:47. Its watcher ticks at t0+60 s, and the ! cmp runs about 35-45 s after t0. So only the busy-loop mutant from your table (-ge 0) is caught. Asking the container whether a watcher exists answers this at once: with 0 no sleep process runs, and with any watcher one nearly always does. Keep the ! cmp for the busy loop. Not run here.

docker exec test_secret sh -c '! grep -sqx sleep /proc/[0-9]*/comm'

---

suggestion (non-blocking): Only a health probe landing in a window of about 1 s pins the rule that the bootstrap's server stops before .bootstrap-complete is written.

opendj-packages/opendj-docker/run.sh:150-156; .github/workflows/build.yml:527-528

Swap the two and the HEALTHCHECK (test -f marker && ldapsearch, every 5 s during the start period) reports healthy only if it starts after the touch and finishes before stop-ds sends SIGTERM. Even then, "Docker test" goes red only if its 10 s poll reaches dsconfig create-backend while the foreground server is still starting. So a regression would come back as the same flake as in #1098.

Pin: check the order on the instance rather than through a race. For example, assert that .bootstrap-complete is not older than the first "The Directory Server is now stopped" line in logs/errors. Or log a line after the touch and assert that it follows stop-ds's output in docker logs.

---

suggestion (non-blocking): The exit status of ./bin/stop-ds is ignored. If the bootstrap's server takes more than 200 s to stop, exec start-ds fails with "already running" and ends the container.

opendj-packages/opendj-docker/run.sh:150

stop-ds waits for logs/server.pid in WaitForFileDelete, and that wait gives up after DirectoryServer.DEFAULT_TIMEOUT (200 s). The file is deleted only when the JVM exits. After a timeout, start_server execs start-ds, whose start check sees the lock still held and exits, and PID 1 goes with it. Not run: nothing I read bounds the stop time of a freshly bootstrapped server, and this machine has no Docker daemon. Failing with the reason says what happened:

./bin/stop-ds || { echo "The server the bootstrap started did not stop (stop-ds exited $?)"; exit 1; }

---

nitpick (non-blocking): A SECRET_VOLUME_REFRESH with surrounding whitespace passes the -gt 0 guard, but the first sleep then fails. The values 00 and +0 get the "not a whole number" message.

opendj-packages/opendj-docker/run.sh:80, :91-94

[ "60 " -gt 0 ] and [ " 60" -gt 0 ] are true, and sleep "1 " exits 1 with "invalid time interval". So the loop ends at once, and the only trace is sleep's error line. 00 and +0 do switch the watcher off, but the log calls them invalid.

if [[ $SECRET_VOLUME_REFRESH =~ ^[0-9]+$ ]] && [ "$SECRET_VOLUME_REFRESH" -gt 0 ]; then
  watch_secrets &
elif ! [[ $SECRET_VOLUME_REFRESH =~ ^0+$ ]]; then
  echo "SECRET_VOLUME_REFRESH=$SECRET_VOLUME_REFRESH is not a whole number of seconds above 0, the secret volume is copied on start only"
fi

[vharseko]

@maximthomas, thanks for the review. Round 5 is in 83b6119, rebased onto the current master (d30ff78, no conflicts, the earlier commits unchanged). All eight points are taken. Two of them came out a little differently from your snippets, see points 2 and 4.

The ERR trap inside functions (issue): taken. The step now starts with set -E.

A new keystore password while the server runs (issue): taken, the watcher leaves it to the next start. You are right that this is worse than the rejected change I described in round 4. Checked in the code: containsAtLeastOneKey() returns false for the load failure, and disableAndWarnIfUseSSL clears enabled on the live handler during isConfigurationAcceptable. The flag comes back only with a later successful change to the handler or a restart. I reproduced it on the published 5.1.2 image and filed the server side as #1109. Two changes to your pins_unchanged snippet:

• It looks at key*.pin and trust*.pin only. With *.pin, an unrelated .pin file on the volume, which is never copied, would stop the watcher for good.
• The watcher no longer copies .pin files at all (sync_secrets stores). Otherwise a pass that starts just before the new .pin lands would copy it. From then on pins_unchanged compares the volume with that copy and not with the password the server holds, and the next pass copies the keystore with the new password. So while the server runs, data/config/*.pin stay the ones it started with.

The watcher logs "A password on the secret volume changed, the secret volume is copied again on the next start" once per change. The README sentence says the same. Once #1101 lands, the server reads the PIN again on every load, and this hold can be relaxed to allow a hot rotation. That belongs in a follow-up, not in this PR.

The sync_secrets repeat, the trust* glob, the skip of unchanged files (suggestion): taken for the last two. keystore() now also writes a JKS truststore holding the certificate, plus truststore.pin. After the first healthy, the step checks that the truststore was copied. After the watcher has copied v2, it counts the Copied keystore lines, which must be exactly 2 (the start and v2). The count needs only one extra tick, not 12 s. As you say, the repeat needs a Secret that changes in the middle of a pass, and that stays covered only by the check outside Docker from round 4.

The alias (suggestion): taken, and it turned up a wrong sentence in the README. Each keystore now holds its key under the alias secret-vN. The regression you name as a second example, --certNickname server-cert added to the default OPENDJ_SSL_OPTIONS, is not one. I ran it as a mutant, and the step stays green: for SELF_SIGNED_CERTIFICATE, Installer.addCertificateArguments (:1589-1594) passes no -a at all. Only the keystore cases pass one (:1653-1657). So the README's "if OPENDJ_SSL_OPTIONS sets a --certNickname, the key has to be under that alias" was too broad. It now says that with --generateSelfSignedCertificate the handlers are bound to no alias even when a --certNickname is given, and that only a keystore of its own (--useJavaKeystore, --usePkcs12keyStore) with a --certNickname binds them. The first regression, a handler bound to server-cert, does go red now. The mutant G2 below sets ssl-cert-nickname:server-cert on the LDAPS handler during the bootstrap. With the old -alias server-cert it would have passed.

SECRET_VOLUME_REFRESH=0 treated as the default (suggestion): taken. After the REFRESH=0 container turns healthy, docker exec test_secret sh -c '! grep -sqx sleep /proc/[0-9]*/comm' runs before the ! cmp. BusyBox's sleep shows as sleep in comm too.

The order of stop-ds and the marker (suggestion): taken, through the log. run.sh prints "The instance is bootstrapped, the health check may probe it" right after the touch. The step checks that the two lines Stopping Server... (which stop-ds prints) and that one occur in this order and only once each. setup.sh does not run stop-ds itself.

The exit status of stop-ds (suggestion): taken as you wrote it: ./bin/stop-ds || { echo "The server the bootstrap started did not stop (stop-ds exited $?)"; exit 1; }. Not tested: I have no way to make stop-ds time out on demand.

Whitespace, 00, +0 in SECRET_VOLUME_REFRESH (nitpick): taken as you wrote it. On both images, 60, 5 and 08 start the watcher. 0 and 00 turn it off without a message. "60 ", " 60", +0, 1m, 30.5 and -5 get the message.

The step, extracted from the workflow and run the way Actions runs it (bash -eo pipefail), against images carrying this run.sh, with each mutant on latest:

run.sh	result
this round, latest	passes, 187 s
this round, alpine	passes, 190 s
A: the restart road execs start-ds without copying	red at grep -q "CN=secret-v3"
B: watcher guard -ge 0 (a busy loop for 0)	red at the last ! cmp -s (v4)
C: 0 taken for the default 60 s	red at ! grep -sqx sleep
D: the watcher copies a keystore whose password changed	red at the ! cmp -s after v3 with a new password
E: cmp -s "$src" "$dst" && continue removed	red at the count (30 instead of 2)
F: "$SECRET_VOLUME"/trust* dropped from the glob	red at cmp of the truststore
G: --certNickname server-cert added to OPENDJ_SSL_OPTIONS	passes: setup writes no nickname for a self-signed certificate
G2: the LDAPS handler bound to server-cert during the bootstrap	red at the first healthy: "The key with alias '[server-cert]' was not found", LDAPS is disabled, and the health check probes LDAPS
H: the marker written before stop-ds	red at the order of the two log lines

The PR description is updated to match.

[maximthomas]

praise: Everything round 2 raised is fixed or pinned, and the watcher no longer hands a running server a keystore under a new password on the normal road.

• watch_secrets copies only the stores and holds while a key*.pin / trust*.pin on the volume differs from data/config, logging that once (opendj-packages/opendj-docker/run.sh:95-106).
• set -E (.github/workflows/build.yml:627, :927), ./bin/stop-ds || { …; exit 1; } (run.sh:173) and the ^[0-9]+$ guard (run.sh:114-117) fix round 2's trap issue, the ignored stop-ds status and the padded refresh value.
• CI now pins the truststore copy (build.yml:660), a per-version alias (:636) and the absence of a watcher under SECRET_VOLUME_REFRESH=0 (! grep -sqx sleep /proc/[0-9]*/comm, :687).

[vharseko]

@maximthomas, thanks for the review. The merge of #1098 and #1099 made this PR conflict, so it is rebased onto the current master (ea06fd6) as d872a09. One thing you approved in 83b6119 is gone in the process, described first.

stop-ds after the bootstrap is now #1098's. #1098 made the same change for #1085: stop the server setup started before the marker is written, then exec start-ds. On master, a failing stop-ds is logged ("Could not stop the server started by the bootstrap, starting OpenDJ may fail") and the start is tried anyway, and the comment there says why. Your round 5 point (the exit status was ignored) is covered that way, though not the way you suggested. I kept master's block rather than change what #1098 settled, and this PR now only ends that road in start_server, so the first start copies the secret volume before the server starts. The comment above the block now also names the certificate. So ./bin/stop-ds || { …; exit 1; } (run.sh:173 in your review) is no longer in the PR. If you would rather have the exit 1, it is one line on top of master, and I can add it here.

What else the rebase changed:

• 6f2e5e5c00 (Stop the bootstrap's server before the health check may probe it, and import online in the Docker test) is dropped. Its step was word for word [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098's, and its run.sh part is what master does now, so it was empty after the rebase.
• README.md: the ROOT_PASSWORD row keeps [#1093] Hand the root password to the Docker bootstrap tools in a file #1099's note about line breaks, next to this PR's SECRET_VOLUME and SECRET_VOLUME_REFRESH rows.
• build.yml: Docker test secret volume now sits between Docker test replication and Docker test bootstrap LDIFs in both docker jobs, not after the latter. [#1092] Probe the Docker container's health without binding as the root user #1102 adds its step after Docker test bootstrap LDIFs, so the two PRs no longer insert at the same line and merge cleanly in either order. The step itself is unchanged. Its check of the log order (Stopping Server... before The instance is bootstrapped, …) still holds, because master's own line reads Stopping the server started by the bootstrap.
• Two commit messages follow suit. The first no longer says the commit stops the server or Fixes #1085 (Docker image: a freshly bootstrapped container ignores SIGTERM and is killed without stopping the server #1085 is closed by [#1085] Stop the server cleanly on SIGTERM in a container that bootstrapped its instance #1098). The last no longer describes the exit 1.

The PR description is updated to match. Checked: bash -n run.sh, the workflow parses, each commit's build.yml holds the same lines as before the move, and the branch merges cleanly with master and with #1102. Docker was not run locally for the rebase; CI is the gate.

[maximthomas]

praise: The rebase onto #1098 keeps what master already settled and changes only what this PR is about.

• The first start still ends in start_server (opendj-packages/opendj-docker/run.sh:196), after master's if ! ./bin/stop-ds block (run.sh:185-187), so the secret volume is copied before the server starts. The comment above the block now names the certificate (run.sh:177-179).
• "Docker test secret volume" now comes before "Docker test bootstrap LDIFs", so #1102 merges in either order. Run 36308826934 at this head runs the step green in both build-docker and build-docker-alpine.

---

issue (non-blocking): Round 3's point still stands. watch_secrets reads the pins before it reads the stores, so a password change landing between the two reads still copies a keystore under the new password next to the old keystore.pin.

opendj-packages/opendj-docker/run.sh:105, :107, :69

The rebase did not touch this block. The fix from round 3 still applies as written: read the pins again after the store is copied to its temp file, and rename it only if they are unchanged.

    if tmp=$(mktemp "$dst.XXXXXX") && cp "$src" "$tmp" && chmod 600 "$tmp" &&
        # a password may have changed since watch_secrets compared them: the store just read
        # may then be of the new version, and is left to the next start
        { [ "$1" != stores ] || pins_unchanged || { rm -f "$tmp"; return 1; }; } &&
        mv -f "$tmp" "$dst"; then

---

note (non-blocking): The PR title and the subject of 607cf4f still say "and run the server as PID 1 after the bootstrap".

• Since the rebase, that part is #1098's change on master, as the description says. After a squash merge, the subject would credit it to #1087. [#1087] Copy the secret volume on every start of the Docker image says what the PR does now.

[vharseko]

@maximthomas, thanks for the review. This round is c54c480, on the current master (5ced345). The four earlier commits are unchanged by the rebase apart from the subject of the first one. Both points are taken, the first differently from your snippet, because master has moved under it.

The passwords read before the stores (issue): taken, by removing the hold instead of checking the passwords a second time. You are right about the window. watch_secrets read the passwords in pins_unchanged, and only then did cp read the store. A Kubernetes Secret swaps all its files at once, so a swap in between got a store of the new password copied next to the old .pin, and the second pass of sync_secrets compared only the stores. One note: I don't find this point in round 3. Review 5324754665 has the praise only, with no inline comments and no edits, so I take it as new here.

When I rebased, master already had #1101 and #1110, and together they remove the reason for the hold:

• [#1095] Load a file based key store or trust store again when the file changes #1101 (A renewed key store or trust store file is not used until the server is restarted #1095): FileBasedKeyManagerProvider and FileBasedTrustManagerProvider read the PIN again with every load (currentPIN()). They load the store again on the first handshake after the store or its PIN file changed. A pair that cannot be opened keeps the store loaded last in use and logs msgID 653, and it is tried again once either file changes.
• [#1109] Keep a connection handler listening when a change to it is rejected #1110 (A rejected change to the LDAPS connection handler stops the handler listening until a later change is accepted or the server restarts #1109): a rejected change no longer disables a running handler.

The hold was only there because the server kept its first password and because of #1109. So the watcher now copies the .pin files along with the stores. pins_unchanged, the stores mode and the "copied again on the next start" message are gone, and there is no pair of reads left to race. A handshake that lands between the copy of keystore and that of keystore.pin keeps the previous certificate and logs 653 once. The next handshake after the second copy loads the new pair. I said in round 5 that the hold could be relaxed once #1101 landed. It landed before this PR, so the relaxation goes in here rather than in a follow-up.

The first local run turned up something the CI step now has to respect. #1101 takes a keystore while the server runs only if it still holds a key under one of the aliases the previous one had (ERR_FILE_KEYMANAGER_NO_KNOWN_KEY_ALIAS). With the per-version alias from round 5, v2 was copied but not served. Checked on an image carrying #1101:

keystore copied while the server runs	served
v2 under a new alias	still v1, msgID 653 "holds no private key under any of the aliases"
v3 under v1's alias	v3
v4 under v1's alias, new password	v4

So the step keeps v1's alias for the renewals made while the container runs, and uses a new alias for the renewal made while no container runs. That keeps your round-5 pin that the key may have any alias: v1 is still under an alias other than setup's own server-cert, which is what the G2 mutant of round 5 relied on (not run again this round). The README now says that a renewed keystore is served without a restart if it keeps an alias the previous one used, which cert-manager does. A keystore with only new aliases is served from the next restart.

The step now checks that:

• v2 is served while the container runs;
• v3, with a new password, is copied together with its .pin (copies keystore.pin = 2) and served without a restart;
• after docker stop / docker start, v3 is still served;
• v4, with another password and a new alias, renewed while no container runs, is copied by the start of the SECRET_VOLUME_REFRESH=0 container.

The PR title and the subject of the first commit (note): taken. Both now read [#1087] Copy the secret volume on every start of the Docker image. The PID 1 part is #1098's on master, as the description says.

The step was run locally the way Actions runs it (bash -eo pipefail), on images built from this branch's Dockerfiles. The server zip came from #1101's last round, and its extensions code and messages are identical to the squash on master:

run.sh	result
this round, latest	passes, 132 s
this round, alpine	passes, 199 s
A: the restart road execs start-ds without copying	red at CN=secret-v4
P: the watcher leaves the .pin files alone (the hold of round 5)	red at the wait for keystore.pin of v3
B: watcher guard -ge 0 (a busy loop for 0)	red at the last ! cmp -s
C: 0 taken for the default 60 s	red at ! grep -sqx sleep
E: unchanged files copied again	red at the count (40 instead of 2)
F: trust* dropped from the glob	red at cmp of the truststore
H: the marker written before stop-ds	red at the order of the two log lines

The machine was heavily loaded, so each mutant ran only when the load was below 16. The first runs of A and P, under a load of about 560, stalled in the bootstrap before the first healthy, and are not counted.

The PR description is updated to match.

[maximthomas]

praise: Dropping the pin gate once the server reloads the store together with its password makes the watcher simpler and removes the round-3 race.

• watch_secrets runs a plain sync_secrets (run.sh:92-96), with no pins_unchanged gate left to race against a Secret swap.
• The CI step now checks that the reload happens rather than that the copy is held back: v2 under the kept alias is served without a restart (build.yml:699-702), v3 with a new password is too (:708-711), and v4 with a new password and a new alias is served after a new start (:723-727).
• keystore() takes the alias as a third argument (alias=${3:-$1}, build.yml:670), so the renewal keeps secret-v1 the way cert-manager keeps its alias.