mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
2 days ago 006982fba689b10a553f4327fa52c9461c392307
[#1229] Look for java on the PATH of the caller in upgrade, before it narrows PATH to /bin:/usr/bin (#1230)

Fixes #1229

### Problem

`upgrade` was the only launcher that sourced `lib/_script-util.sh` with
`set-environment-vars` (`PATH=/bin:/usr/bin`) before
`lib/_server-script.sh` looked for java. The `command -v java` lookup
(`_script-util.sh:66`) therefore missed a java that is on the caller's
`PATH` only from another directory. With no `OPENDJ_JAVA_HOME`,
`JAVA_HOME` or `java-home` property to fall back on, `upgrade` exited
with `Please set OPENDJ_JAVA_HOME ...`. `start-ds`, `ldapsearch` and the
other tools started from the same shell found java.

Where `JAVA_HOME` let the lookup succeed, as in the Docker image built
on `eclipse-temurin`, dash still printed `test: -a: unexpected operator`
on every run, because line 67 tested the empty result without quotes.

### Fix

- `upgrade` now sources `_script-util.sh` with `set-java-home-and-args`
first, and only then with `set-environment-vars`. `set_opendj_java_bin`
exports `OPENDJ_JAVA_BIN`, and `_server-script.sh` takes it as it is, in
the first branch of its lookup. So java is resolved from the caller's
environment, as for every other tool, and the rest of the script still
runs with the narrowed `PATH`.
- `_script-util.sh:67` now quotes `TEST_JAVA_PATH`. An empty lookup
result fails the test without a message, and the function goes on to
`JAVA_BIN`, `JAVA_HOME` and, if neither is set, the error message.
- `set_environment_vars` appends `--add-exports`/`--add-opens` and
`--enable-native-access` only where `OPENDJ_JAVA_ARGS` does not hold
them yet, and runs the probe of each only then. `upgrade` now runs the
function with `OPENDJ_JAVA_BIN` known, and `_server-script.sh` runs it
again in the child. Without the guard, the three module flags reached
the upgrade JVM twice, and every run started two more `java --version`
JVMs. The tools that run the function once get the same arguments as
before. On JDK 11, which refuses `--enable-native-access`, the child
still probes that flag, so `upgrade` starts one `java --version` JVM
more than at master there; the flags it passes are the same.

The Windows launchers do not narrow `PATH` and are not changed.

### Test

Three checks in the `Docker test` step of `build.yml`, on the running
container:

- `upgrade -n --force` with `JAVA_HOME` unset must succeed. In the
default image, java is on the `PATH` only from `/opt/java/openjdk/bin`.
- Two passes of `set-environment-vars` in one shell, as `upgrade` and
`_server-script.sh` run it, must leave one `--add-opens` in
`OPENDJ_JAVA_ARGS`. This pins the guard: the JVM takes the doubled flags
without a warning, so nothing else fails without it.
- `ldapsearch --version` with `PATH=/bin:/usr/bin`, so with no java
anywhere, must not print `unexpected operator`.

On alpine the first and the last check pass with or without the fix,
because `/usr/bin/java` is there. The default image is the one that
catches a regression of the lookup. The check of the guard fails without
it in both images.

Local RED/GREEN: the lines of the step were taken from `build.yml` and
run in an `openidentityplatform/opendj:5.2.0` container, whose scripts
are the same as on master, with the scripts of this branch copied in:

| Scripts | Result |
|---|---|
| 5.2.0 (same as master) | fails at the first check: `Please set
OPENDJ_JAVA_HOME ...`, exit 1 |
| `upgrade` fixed only | first check passes, second fails:
`_client-script.sh: 67: test: -a: unexpected operator` |
| this branch | pass |

The check of the guard, cut from `build.yml` and run with `docker exec`
in running 5.2.0 (dash) and 5.2.0-alpine (busybox ash) containers: with
`_script-util.sh` of `upgrade` fixed and no guard it exits 1 in both (2x
`--add-opens`, 4x `--add-exports`), with this branch it exits 0 (1x /
2x). It fails at master as well, which has no guard; there `upgrade`
itself does not double the flags, because its first pass runs without a
java.

The arguments of the upgrade JVM, with a `java` on the `PATH` that logs
its argv and runs the real one, in the 5.2.0 container (dash) and the
5.2.0-alpine container (busybox ash):

| Scripts | `java --version` probes | `--add-exports` / `--add-opens` |
|---|---|---|
| master | 2 | 2 / 1 |
| `upgrade` fixed, no guard | 4 | 4 / 2 |
| this branch | 2 | 2 / 1 |

The master row is from the default image only: on alpine, master finds
`/usr/bin/java` under the narrowed `PATH` and does not run the logging
java. At this branch the upgrade argv is the same as at master, and so
is the argv of `ldapsearch`, `export-ldif` and `status`, in both images.

Both images run JDK 25, which accepts `--enable-native-access`. JDK 11
refuses it, and it is the minimum in the pom and the java of the
`linux/386` alpine image. With the same logging java, over two passes of
`set-environment-vars` run as `upgrade` runs them (its own, master
without `OPENDJ_JAVA_BIN`, then one in a child shell), in
`eclipse-temurin:11-jre` (11.0.32) and `eclipse-temurin:21-jdk`:

| Scripts | probes, JDK 11 | probes, JDK 21 |
|---|---|---|
| master | 2 | 2 |
| `upgrade` fixed, no guard | 4 | 4 |
| this branch | 3 | 2 |

On JDK 11 the child pass probes `--enable-native-access` again, as the
first pass appended nothing. After both passes `OPENDJ_JAVA_ARGS` holds
2x `--add-exports` / 1x `--add-opens` with this branch on both JDKs, as
at master.

Before the guard was added, images 5.2.0 and 5.2.0-alpine with the
scripts of this branch also passed the whole `Docker test` step and the
`Docker test upgrade from a released image` step, with 5.1.2 as the
release. The latter is a real upgrade through `run.sh`, including the
`.upgrade-pending` cases.
3 files modified
39 ■■■■■ changed files
.github/workflows/build.yml 9 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/resource/bin/_script-util.sh 18 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/resource/upgrade 12 ●●●●● patch | view | raw | blame | history
.github/workflows/build.yml
@@ -711,6 +711,15 @@
          test "$(docker exec test grep -c "The Directory Server is now stopped" /opt/opendj/data/logs/errors)" -gt "$stopped"
          docker exec test 'sh' '-c' '/opt/opendj/bin/rebuild-index --hostname localhost --port 4444 --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll --trustAll'
          docker exec test 'sh' '-c' '/opt/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "ou=people,dc=example2,dc=com" --searchScope sub "(uid=user.*)" dn | grep ^dn: | wc -l | grep -q 10000'
          # upgrade looks for java before it narrows PATH to /bin:/usr/bin, as the other tools do:
          # the java of the default image is on the PATH from /opt/java/openjdk/bin alone (alpine has
          # /usr/bin/java, so only the default row can fail), and run.sh runs upgrade on every start of a volume (#1229)
          docker exec test sh -c 'unset JAVA_HOME; sh /opt/opendj/upgrade -n --force'
          # upgrade runs set_environment_vars twice: the module flags reach the JVM once
          docker exec test sh -c 'OPENDJ_JAVA_BIN=$(command -v java) INSTALL_ROOT=/opt/opendj INSTANCE_ROOT=/opt/opendj SCRIPT_NAME=upgrade; export OPENDJ_JAVA_BIN INSTALL_ROOT INSTANCE_ROOT SCRIPT_NAME; SCRIPT_UTIL_CMD=set-environment-vars; . /opt/opendj/lib/_script-util.sh; . /opt/opendj/lib/_script-util.sh; test "$(echo " $OPENDJ_JAVA_ARGS " | tr " " "\n" | grep -c -- "^--add-opens$")" -eq 1'
          # a java found nowhere is reported without a shell error about the empty lookup
          out=$(docker exec test sh -c 'unset JAVA_HOME; PATH=/bin:/usr/bin /opt/opendj/bin/ldapsearch --version' 2>&1 || true)
          if grep -F "unexpected operator" <<<"$out"; then echo "::error::The lookup of java on the PATH fails with a shell error where it finds no java"; false; fi
          docker rm -f test
      - name: Docker test custom password
        shell: bash
opendj-server-legacy/resource/bin/_script-util.sh
@@ -64,7 +64,7 @@
        OPENDJ_JAVA_BIN=${PROPERTY_VALUE}/bin/java
      else
        TEST_JAVA_PATH=`command -v java 2> /dev/null`
        if test ! -z ${TEST_JAVA_PATH} -a -f ${TEST_JAVA_PATH}
        if test ! -z "${TEST_JAVA_PATH}" -a -f "${TEST_JAVA_PATH}"
        then
          OPENDJ_JAVA_BIN=${TEST_JAVA_PATH}
        elif test ! -z "${JAVA_BIN}" -a -f "${JAVA_BIN}"
@@ -248,23 +248,31 @@
  SCRIPT_NAME_ARG=-Dorg.opends.server.scriptName=${SCRIPT_NAME}
    export SCRIPT_NAME_ARG
    
  # upgrade runs this function again in _server-script.sh: flags already there are neither probed nor added twice
  case " ${OPENDJ_JAVA_ARGS} " in
    *" --add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.tools.keytool=ALL-UNNAMED --add-opens java.base/jdk.internal.loader=ALL-UNNAMED "*) ;;
    *)
  "${OPENDJ_JAVA_BIN}" --add-opens java.base/jdk.internal.loader=ALL-UNNAMED --version > /dev/null 2>&1
  RESULT_CODE=${?}
  if test ${RESULT_CODE} -eq 0
  then
    export OPENDJ_JAVA_ARGS="$OPENDJ_JAVA_ARGS --add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.tools.keytool=ALL-UNNAMED --add-opens java.base/jdk.internal.loader=ALL-UNNAMED"
  fi
      ;;
  esac
  case " ${OPENDJ_JAVA_ARGS} " in
    *" --enable-native-access=ALL-UNNAMED "*) ;;
    *)
  "${OPENDJ_JAVA_BIN}" --enable-native-access=ALL-UNNAMED --version > /dev/null 2>&1
  RESULT_CODE=${?}
  if test ${RESULT_CODE} -eq 0
  then
    case " ${OPENDJ_JAVA_ARGS} " in
      *" --enable-native-access=ALL-UNNAMED "*) ;;
      *) OPENDJ_JAVA_ARGS="${OPENDJ_JAVA_ARGS} --enable-native-access=ALL-UNNAMED" ;;
    esac
        OPENDJ_JAVA_ARGS="${OPENDJ_JAVA_ARGS} --enable-native-access=ALL-UNNAMED"
    export OPENDJ_JAVA_ARGS
  fi
      ;;
  esac
}
# Configure the appropriate CLASSPATH for server, using Opend DJ logger.
opendj-server-legacy/resource/upgrade
@@ -13,6 +13,7 @@
# information: "Portions Copyright [year] [name of copyright owner]".
#
# Copyright 2013-2015 ForgeRock AS.
# Portions Copyright 2026 3A Systems, LLC.
# This script may be used to perform a backup of a Directory Server backend.
@@ -27,6 +28,17 @@
INSTALL_ROOT=${SCRIPT_DIR}
export INSTALL_ROOT
# Look for java on the PATH of the caller, as the other tools do, before set-environment-vars
# narrows it to /bin:/usr/bin: _server-script.sh takes the OPENDJ_JAVA_BIN found here as it is
SCRIPT_UTIL_CMD=set-java-home-and-args
export SCRIPT_UTIL_CMD
.  "${INSTALL_ROOT}/lib/_script-util.sh"
RETURN_CODE=$?
if test ${RETURN_CODE} -ne 0
then
  exit ${RETURN_CODE}
fi
# Set environment variables
SCRIPT_UTIL_CMD=set-environment-vars
export SCRIPT_UTIL_CMD