diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9bb01cdeb1..f4fa75a4b5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -241,8 +241,17 @@ jobs: --baseDN dc=example,dc=com --sampleData 5000 --cli --acceptLicense --no-prompt \ --propertiesFilePath /tmp/opendj-setup.properties.bcfks --doNotStart + # start-ds sweeps the native libraries the BC FIPS loader leaves in tmp/ and nothing + # else: the rest of tmp/ belongs to tools that may still be running, setup among them + # (issue #1030). Without these two probes a sweep back to "tmp/*" passes every cell. + mkdir -p opendj-server-legacy/target/package/opendj/tmp/bc-fips-jni_123 + touch opendj-server-legacy/target/package/opendj/tmp/keep.me + opendj-server-legacy/target/package/opendj/bin/start-ds + test ! -e opendj-server-legacy/target/package/opendj/tmp/bc-fips-jni_123 + test -e opendj-server-legacy/target/package/opendj/tmp/keep.me + opendj-server-legacy/target/package/opendj/bin/status --hostname localhost --bindDN "cn=Directory Manager" --bindPassword password --trustAll opendj-server-legacy/target/package/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "dc=example,dc=com" --searchScope base "(objectClass=*)" 1.1 opendj-server-legacy/target/package/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "ou=people,dc=example,dc=com" --searchScope sub "(uid=user.*)" dn | grep ^dn: | wc -l | grep -q 5000 @@ -394,7 +403,14 @@ jobs: opendj-server-legacy\target\package\opendj\upgrade.bat opendj-server-legacy\target\package\opendj\bat\import-ldif.bat --offline --ldifFile test.ldif --backendID=example2 opendj-server-legacy\target\package\opendj\bat\rebuild-index.bat --offline --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll + # The same probe as the Unix FIPS step, for start-ds.bat: the sweep takes the BC FIPS + # directories and leaves everything else in tmp\ alone (issue #1030). The runner's + # install path has no space in it, so this does not settle the quoting of the for /D line. + New-Item -ItemType Directory -Force opendj-server-legacy\target\package\opendj\tmp\bc-fips-jni_123 | Out-Null + New-Item -ItemType File -Force opendj-server-legacy\target\package\opendj\tmp\keep.me | Out-Null opendj-server-legacy\target\package\opendj\bat\start-ds.bat + if (Test-Path opendj-server-legacy\target\package\opendj\tmp\bc-fips-jni_123) { throw "start-ds.bat left a bc-fips-jni_* directory behind" } + if (-not (Test-Path opendj-server-legacy\target\package\opendj\tmp\keep.me)) { throw "start-ds.bat removed a tmp file that is not its own" } opendj-server-legacy\target\package\opendj\bat\rebuild-index.bat --bindDN "cn=Directory Manager" --bindPassword password --baseDN "dc=example2,dc=com" --rebuildAll --trustAll opendj-server-legacy\target\package\opendj\bat\ldapsearch.bat --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword password --useSsl --trustAll --baseDN "dc=example2,dc=com" --searchScope sub "(uid=user.*)" dn | find /c '"dn:"' | findstr "10000" opendj-server-legacy\target\package\opendj\bat\stop-ds.bat @@ -409,6 +425,19 @@ jobs: if ($LASTEXITCODE -ne 0) { throw "net stop 'OpenDJ Server' failed with exit code $LASTEXITCODE" } opendj-server-legacy\target\package\opendj\bat\windows-service.bat --disableService + # A test step that fails leaves its instances behind. The server-side story of a + # failed start lives in logs/server.out and logs/errors, and nothing else prints it + # (setup only has the client-side view, see issue #1030). + - name: Dump the server logs of a failed test + if: failure() + shell: bash + run: | + for f in opendj-server-legacy/target/package/opendj*/logs/server.out opendj-server-legacy/target/package/opendj*/logs/errors; do + [ -f "$f" ] || continue + echo "===== $f" + cat "$f" + done + - name: Upload artifacts OpenDJ Server uses: actions/upload-artifact@v7 with: diff --git a/opendj-server-legacy/resource/bin/start-ds b/opendj-server-legacy/resource/bin/start-ds index 0c965b6ad0..91805e8376 100644 --- a/opendj-server-legacy/resource/bin/start-ds +++ b/opendj-server-legacy/resource/bin/start-ds @@ -14,7 +14,7 @@ # # Copyright 2006-2009 Sun Microsystems, Inc. # Portions Copyright 2011-2014 ForgeRock AS. -# Portions Copyright 2025 3A Systems LLC. +# Portions Copyright 2025-2026 3A Systems LLC. # Capture the current working directory so that we can change to it later. @@ -73,10 +73,14 @@ PID_FILE=${INSTANCE_ROOT}/logs/server.pid LOG_FILE=${INSTANCE_ROOT}/logs/server.out STARTING_FILE=${INSTANCE_ROOT}/logs/server.starting -# Cleanup the tmp directory +# Remove the native libraries the BC FIPS loader extracts into the tmp directory +# on every start, so that they do not pile up. Nothing else in there is ours to +# remove: the tmp directory is java.io.tmpdir for every tool, and a tool may +# still be running - setup starts the server through this script and keeps its +# own log open (issue #1030). OPENDJ_TMP_DIR="${INSTANCE_ROOT}/tmp" if [ -d "${OPENDJ_TMP_DIR}" ]; then - rm -rf ${OPENDJ_TMP_DIR}/* + rm -rf "${OPENDJ_TMP_DIR}"/bc-fips-jni_* fi # See if the provided set of arguments were sufficient for us to be able to diff --git a/opendj-server-legacy/resource/bin/start-ds.bat b/opendj-server-legacy/resource/bin/start-ds.bat index f591502ba2..46c5f31f95 100644 --- a/opendj-server-legacy/resource/bin/start-ds.bat +++ b/opendj-server-legacy/resource/bin/start-ds.bat @@ -58,16 +58,15 @@ echo %SCRIPT%: CLASSPATH=%CLASSPATH% >> %LOG% echo %SCRIPT%: PATH=%PATH% >> %LOG% -rem cleanup the tmp directory -set CUR_DIR=%CD% +rem Remove the native libraries the BC FIPS loader extracts into the tmp +rem directory on every start, so that they do not pile up. Nothing else in +rem there is ours to remove: the tmp directory is java.io.tmpdir for every +rem tool, and a tool may still be running - setup starts the server through +rem this script and keeps its own log open (issue #1030). set OPENDJ_TMP_DIR=%INSTANCE_ROOT%\tmp -rem The paths must be quoted: an unquoted parenthesis (e.g. from -rem "C:\Program Files (x86)") terminates the ( ) block at parse time. -dir /b /s /a "%OPENDJ_TMP_DIR%" | findstr .>nul && ( - cd /d "%OPENDJ_TMP_DIR%" - for /F "delims=" %%i in ('dir /b') do (rmdir "%%i" /s/q>NUL 2>&1 || del "%%i" /s/q>NUL 2>&1) - cd /d "%CUR_DIR%" -) +rem %%~i drops the quotes cmd would hand back if it ever kept them: an install path with a +rem space would otherwise split the rmdir argument, and the redirect would hide it. +for /D %%i in ("%OPENDJ_TMP_DIR%\bc-fips-jni_*") do rmdir "%%~i" /s/q>NUL 2>&1 "%OPENDJ_JAVA_BIN%" -client %SCRIPT_NAME_ARG% org.opends.server.core.DirectoryServer --configFile "%INSTANCE_ROOT%\config\config.ldif" --checkStartability %* diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java index 0f714af2f6..301b48b3a8 100644 --- a/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/guitools/uninstaller/UninstallLauncher.java @@ -13,6 +13,7 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.guitools.uninstaller; @@ -129,8 +130,8 @@ protected void initializeParser() @Override protected void guiLaunchFailed() { System.err.println( - tempLogFile.isEnabled() ? ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(tempLogFile.getPath()) - : ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED.get()); + hasTempLogFile() ? ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(getTempLogFile().getPath()) + : ERR_UNINSTALL_LAUNCHER_GUI_LAUNCHED_FAILED.get()); } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java index dbf983fa1a..0c54f02822 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Installation.java @@ -64,7 +64,7 @@ public final class Installation /** The relative path where the database files are. */ private static final String DATABASES_PATH_RELATIVE = "db"; /** The relative path where the log files are. */ - private static final String LOGS_PATH_RELATIVE = "logs"; + public static final String LOGS_PATH_RELATIVE = "logs"; /** The relative path where the LDIF files are. */ private static final String LDIFS_PATH_RELATIVE = "ldif"; /** The relative path where the backup files are. */ diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java index 1fc34e76f5..53171a6ea9 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Launcher.java @@ -13,6 +13,7 @@ * * Copyright 2008-2009 Sun Microsystems, Inc. * Portions Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.quicksetup; @@ -21,6 +22,7 @@ import static org.opends.messages.QuickSetupMessages.*; import static org.opends.server.util.DynamicConstants.*; +import java.io.File; import java.io.PrintStream; import org.forgerock.i18n.LocalizableMessage; @@ -40,8 +42,14 @@ public abstract class Launcher { /** Arguments with which this launcher was invoked. */ protected final String[] args; - /** The temporary log file which will be kept if an error occurs. */ - protected final TempLogFile tempLogFile; + private final String tempLogFilePrefix; + private final File tempLogFileDirectory; + /** + * The temporary log file which will be kept if an error occurs; see {@link #getTempLogFile()}. + * Volatile because the GUI road asks for it from the thread that runs the splash screen and + * the roads after it read it from the main thread. + */ + private volatile TempLogFile tempLogFile; /** * Creates a Launcher. @@ -52,11 +60,55 @@ public abstract class Launcher { * temporary log file path where messages will be logged */ public Launcher(final String[] args, final String tempLogFilePrefix) { + this(args, tempLogFilePrefix, null); + } + + /** + * Creates a Launcher whose temporary log file lives in the given directory. + * + * @param args + * String[] of argument passes from the command line + * @param tempLogFilePrefix + * temporary log file path where messages will be logged + * @param tempLogFileDirectory + * the directory to create the temporary log file in, or {@code null} for the OS + * temporary directory + */ + public Launcher(final String[] args, final String tempLogFilePrefix, final File tempLogFileDirectory) { if (args == null) { throw new IllegalArgumentException("args cannot be null"); } this.args = args; - this.tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix); + this.tempLogFilePrefix = tempLogFilePrefix; + this.tempLogFileDirectory = tempLogFileDirectory; + } + + /** + * The temporary log file of this launcher, created the first time it is asked for. + *

+ * Creating it costs a file - and, with a directory of the caller's choosing, the directory + * as well - that nothing removes afterwards unless the operation succeeds. So it is created + * on the first road that can fail an operation and not before: {@code --help}, + * {@code --version}, a usage error and the other roads that attempt nothing leave no log + * behind (issue #1030). + * + * @return the temporary log file, creating it if this is the first call. + */ + protected synchronized TempLogFile getTempLogFile() { + if (tempLogFile == null) { + tempLogFile = TempLogFile.newTempLogFile(tempLogFilePrefix, tempLogFileDirectory); + } + return tempLogFile; + } + + /** + * Whether there is a log to name, without creating one to answer. + * + * @return {@code true} if a temporary log file has been created and can be used to log + * messages. + */ + protected boolean hasTempLogFile() { + return tempLogFile != null && tempLogFile.isEnabled(); } /** @@ -194,12 +246,12 @@ public void run() { try { - SplashScreen.main(tempLogFile, args); + SplashScreen.main(getTempLogFile(), args); returnValue[0] = 0; } catch (Throwable t) { - if (tempLogFile.isEnabled()) + if (hasTempLogFile()) { logger.warn(LocalizableMessage.raw("Error launching GUI: "+t)); StringBuilder buf = new StringBuilder(); @@ -337,6 +389,9 @@ else if (shouldPrintUsage()) { } System.exit(ReturnCode.SUCCESSFUL.getReturnCode()); } else if (isCli()) { + // An operation is about to run: from here on there is something worth logging, and + // preExit() names the file. The roads above attempt nothing and leave no log behind. + getTempLogFile(); CliApplication cliApp = createCliApplication(); int exitCode = launchCli(cliApp); preExit(cliApp); @@ -361,8 +416,8 @@ private void preExit(CliApplication cliApp) { // Add an extra space systematically System.out.println(); - if (tempLogFile.isEnabled()) { - System.out.println(INFO_GENERAL_SEE_FOR_DETAILS.get(tempLogFile.getPath())); + if (hasTempLogFile()) { + System.out.println(INFO_GENERAL_SEE_FOR_DETAILS.get(getTempLogFile().getPath())); } } } diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java index ccb1c9e887..6f193b8803 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/TempLogFile.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.nio.charset.Charset; import java.nio.file.Files; import java.util.Date; import java.text.DateFormat; @@ -51,9 +52,51 @@ public class TempLogFile */ public static TempLogFile newTempLogFile(final String prefix) { + return newTempLogFile(prefix, null); + } + + /** + * Creates a new temporary log file in the given directory. + *

+ * The directory is created if it does not exist yet. When it is {@code null} or cannot be + * used, the log file goes to the OS temporary directory instead, as with + * {@link #newTempLogFile(String)}. The name of the file follows the pattern + * prefix-[RANDOM_NUMBER_STRING].log either way. + * + * @param prefix + * log file prefix to which log messages will be written. + * @param directory + * the directory to create the log file in, or {@code null} for the OS temporary + * directory. + * @return a new temporary log file. + */ + public static TempLogFile newTempLogFile(final String prefix, final File directory) + { + IOException fallbackReason = null; + if (directory != null) + { + try + { + Files.createDirectories(directory.toPath()); + return new TempLogFile(Files.createTempFile(directory.toPath(), prefix, ".log").toFile()); + } + catch (final IOException e) + { + // Nothing can be logged yet: the first publisher is the one the constructor installs + // below, so the warning has to wait until there is a log to write it to. + fallbackReason = e; + } + } try { - return new TempLogFile(Files.createTempFile(prefix, ".log").toFile()); + final TempLogFile tempLogFile = new TempLogFile(Files.createTempFile(prefix, ".log").toFile()); + if (fallbackReason != null) + { + localizedLogger.warn(LocalizableMessage.raw("Unable to create temp log file in " + directory + + " because: " + fallbackReason.getMessage() + ", falling back to the temporary directory"), + fallbackReason); + } + return tempLogFile; } catch (final IOException e) { @@ -68,9 +111,14 @@ private TempLogFile() { this.logFile = null; this.writer=null; + this.startupErrorLogPublisher = null; + this.startupDebugLogPublisher = null; } final TextWriter writer; + /** Kept so that they can be taken off the logger singletons again, see {@link #deleteLogFileAfterSuccess()}. */ + private final ErrorLogPublisher startupErrorLogPublisher; + private final DebugLogPublisher startupDebugLogPublisher; private TempLogFile(final File file) throws IOException { @@ -83,9 +131,9 @@ private TempLogFile(final File file) throws IOException }else { writer=new TextWriter.STREAM(new FileOutputStream(file)); } - ErrorLogPublisher startupErrorLogPublisher = TextErrorLogPublisher.getServerStartupTextErrorPublisher(writer); + startupErrorLogPublisher = TextErrorLogPublisher.getServerStartupTextErrorPublisher(writer); ErrorLogger.getInstance().addLogPublisher(startupErrorLogPublisher); - DebugLogger.getInstance().addPublisherIfRequired(writer); + startupDebugLogPublisher = DebugLogger.getInstance().addPublisherIfRequired(writer); localizedLogger.info(LocalizableMessage.raw("QuickSetup application launched " + DateFormat.getDateTimeInstance(DateFormat.LONG, DateFormat.LONG).format(new Date()), null)); } @@ -100,11 +148,23 @@ public File getLogFile() return logFile; } - /** Closes the log file handler and delete the temp log file . */ + /** + * Closes the log file handler and delete the temp log file . + *

+ * The publishers installed by the constructor go with it: they are held by the logger + * singletons, which outlive this object, and once the writer is shut everything they are + * handed is written to a closed stream and swallowed. + */ public void deleteLogFileAfterSuccess() { if (isEnabled()) { + if (startupErrorLogPublisher != null) { + ErrorLogger.getInstance().removeLogPublisher(startupErrorLogPublisher); + } + if (startupDebugLogPublisher != null) { + DebugLogger.getInstance().removeLogPublisher(startupDebugLogPublisher); + } if (writer!=null) { writer.shutdown(); } @@ -121,6 +181,40 @@ public boolean isEnabled() return logFile != null; } + /** + * Return {@code true} if the temp log file is still on disk and can be read. + *

+ * Unlike {@link #isEnabled()} this is about the file, not the logger: something else may have + * removed the file while the logger still writes to it (see issue #1030), and then there is + * nothing to hand over to whoever needs the log. + * + * @return {@code true} if the temp log file is there and readable. + */ + public boolean isReadable() + { + return logFile != null && Files.isReadable(logFile.toPath()) && Files.isRegularFile(logFile.toPath()); + } + + /** + * Reads the whole temp log file. + *

+ * The file is decoded with the default charset of the JVM, which is the one + * {@link TextWriter.STREAM} wrote it with: reader and writer are the same JVM, so a + * non-ASCII path or base DN in a report comes back as it was logged. + * + * @return the contents of the temp log file. + * @throws IOException + * if the file cannot be read, for instance because it is no longer there. + */ + public String readContents() throws IOException + { + if (logFile == null) + { + throw new IOException("No temp log file"); + } + return new String(Files.readAllBytes(logFile.toPath()), Charset.defaultCharset()); + } + /** * Return the absolute path of the temp log file. * @return the absolute path of the temp log file. diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java index 661af28dd3..cae7929085 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/Installer.java @@ -42,9 +42,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.PrintStream; -import java.io.UnsupportedEncodingException; -import java.nio.file.Files; -import java.nio.file.Paths; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; @@ -317,6 +314,10 @@ public void run() uninstall(); setCurrentProgressStep(InstallProgressStep.FINISHED_CANCELED); notifyListeners(null); + // Nothing names this log on this road - notifyListenersOfExistingLogFile() belongs to + // handleInstallationError() below - and uninstall() has just taken the installation + // back, so keeping the file would leave a report nobody is told about (issue #1030). + tempLogFile.deleteLogFileAfterSuccess(); } else { handleInstallationError(ex); } @@ -617,23 +618,34 @@ public String getInstancePath() return Utils.getInstancePathFromInstallPath(installPath); } - private void notifyListenersOfExistingLogFile() + /** Package-private so that {@code InstallerTest} can drive every road of this report. */ + void notifyListenersOfExistingLogFile() { - if (tempLogFile.isEnabled()) + if (!tempLogFile.isEnabled()) { - final String tempLogFilePath = tempLogFile.getPath(); - notifyListeners(getFormattedProgress(INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(tempLogFilePath))); - //write log - try { - notifyListeners(getLineBreak()); - notifyListeners(LocalizableMessage.valueOf(new String(Files.readAllBytes(Paths.get(tempLogFilePath)),"UTF-8"))); - } catch (UnsupportedEncodingException e) { - e.printStackTrace(); - } catch (IOException e) { - e.printStackTrace(); - } + return; + } + final String tempLogFilePath = tempLogFile.getPath(); + if (!tempLogFile.isReadable()) + { + // Something removed the log while it was being written (issue #1030): say so + // rather than ask for a file that is not there. + notifyListeners(getFormattedWarning(INFO_GENERAL_LOG_IN_ERROR_MISSING.get(tempLogFilePath))); notifyListeners(getLineBreak()); + return; + } + notifyListeners(getFormattedProgress(INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(tempLogFilePath))); + notifyListeners(getLineBreak()); + // Write the log out as well, so that a report has it even when the file is not attached. + try + { + notifyListeners(LocalizableMessage.raw(tempLogFile.readContents())); } + catch (final IOException e) + { + notifyListeners(getFormattedWarning(INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(tempLogFilePath, e))); + } + notifyListeners(getLineBreak()); } /** Creates a default instance. */ diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java index ff6820bb20..99c0a981c6 100644 --- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java +++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/SetupLauncher.java @@ -13,6 +13,7 @@ * * Copyright 2008-2010 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.quicksetup.installer; @@ -22,6 +23,8 @@ import static com.forgerock.opendj.util.StaticUtils.registerBcProvider; +import java.io.File; + import org.forgerock.i18n.LocalizableMessage; import org.opends.quicksetup.CliApplication; import org.opends.quicksetup.Installation; @@ -65,7 +68,7 @@ public static void main(String[] args) { * @param args the arguments passed by the command lines. */ public SetupLauncher(String[] args) { - super(args, LOG_FILE_PREFIX); + super(args, LOG_FILE_PREFIX, instanceLogsDirectory()); if (System.getProperty(PROPERTY_SCRIPT_NAME) == null) { System.setProperty(PROPERTY_SCRIPT_NAME, Installation.getSetupFileName()); @@ -73,6 +76,32 @@ public SetupLauncher(String[] args) { initializeParser(); } + /** + * The {@code logs/} directory of the instance being set up, where the setup log is kept. + *

+ * The launcher scripts point {@code java.io.tmpdir} at {@code /tmp}, the scratch + * space of every tool, which {@code start-ds} - run by setup itself to start the server - + * used to sweep clean (issue #1030). The log of a failed setup belongs next to the server's + * own logs instead, where {@code server.out} tells the other half of the story. + *

+ * Neither the directory nor the log is created here: {@link Launcher#getTempLogFile()} + * creates both when an install is about to run, so that a road which installs nothing - + * {@code setup --help} on a package whose instance directory is not laid down yet, for one - + * leaves nothing behind. + * + * @return the logs directory of the instance, or {@code null} when the launcher is not + * running from an installation and the OS temporary directory has to do. + */ + private static File instanceLogsDirectory() + { + final String installPath = Utils.getInstallPathFromClasspath(); + if (installPath == null) + { + return null; + } + return new File(Utils.getInstancePathFromInstallPath(installPath), Installation.LOGS_PATH_RELATIVE); + } + /** Initialize the contents of the argument parser. */ protected void initializeParser() { @@ -109,7 +138,7 @@ else if (argParser.isUsageArgumentPresent() || else if (isCli()) { Utils.checkJavaVersion(); - System.exit(InstallDS.mainCLI(args, tempLogFile)); + System.exit(InstallDS.mainCLI(args, this::getTempLogFile)); } else { @@ -121,7 +150,7 @@ else if (isCli()) if (exitCode != 0) { guiLaunchFailed(); Utils.checkJavaVersion(); - System.exit(InstallDS.mainCLI(args, tempLogFile)); + System.exit(InstallDS.mainCLI(args, this::getTempLogFile)); } } } @@ -145,8 +174,8 @@ public ArgumentParser getArgumentParser() { @Override protected void guiLaunchFailed() { System.err.println( - tempLogFile.isEnabled() ? INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(tempLogFile.getPath()) - : INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); + hasTempLogFile() ? INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED_DETAILS.get(getTempLogFile().getPath()) + : INFO_SETUP_LAUNCHER_GUI_LAUNCHED_FAILED.get()); } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java index ee1de64591..fd9f54ac42 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/tools/InstallDS.java @@ -40,6 +40,7 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.function.Supplier; import com.forgerock.opendj.util.FipsStaticUtils; import org.forgerock.i18n.LocalizableMessage; @@ -202,7 +203,11 @@ public int getReturnCode() private Integer lastResetAdminConnectorPort; private Integer lastResetJmxPort; - private final TempLogFile tempLogFile; + /** + * The temporary log file where messages will be logged, asked for only once an install is + * about to run: the roads which return before that one leave no log behind (issue #1030). + */ + private final Supplier tempLogFile; private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass(); @@ -216,7 +221,7 @@ public int getReturnCode() * @param tempLogFile * the temporary log file where messages will be logged. */ - private InstallDS(PrintStream out, PrintStream err, TempLogFile tempLogFile) + private InstallDS(PrintStream out, PrintStream err, Supplier tempLogFile) { super(out, err); this.tempLogFile = tempLogFile; @@ -229,10 +234,12 @@ private InstallDS(PrintStream out, PrintStream err, TempLogFile tempLogFile) * @param args * the command-line arguments provided to this program. * @param tempLogFile - * the temporary log file where messages will be logged. + * supplies the temporary log file where messages will be logged. It is called on + * the road that runs the install and not on the roads which return before it, so + * that a run which installs nothing creates no log. * @return The error code. */ - public static int mainCLI(String[] args, final TempLogFile tempLogFile) + public static int mainCLI(String[] args, final Supplier tempLogFile) { return mainCLI(args, System.out, System.err, tempLogFile); } @@ -259,7 +266,29 @@ public static int mainCLI( // // *NOTE* this method has been kept public because it is used by OpenAM. // + return mainCLI(args, outStream, errStream, () -> tempLogFile); + } + /** + * Parses the provided command-line arguments and uses that information to run + * the setup tool. + * + * @param args + * The command-line arguments provided to this program. + * @param outStream + * The output stream to use for standard output, or null + * if standard output is not needed. + * @param errStream + * The output stream to use for standard error, or null + * if standard error is not needed. + * @param tempLogFile + * supplies the temporary log file where messages will be logged, see + * {@link #mainCLI(String[], Supplier)}. + * @return The error code. + */ + public static int mainCLI( + String[] args, OutputStream outStream, OutputStream errStream, Supplier tempLogFile) + { final PrintStream out = NullOutputStream.wrapOrNullStream(outStream); System.setProperty(Constants.CLI_JAVA_PROPERTY, "true"); @@ -349,7 +378,9 @@ private int execute(String[] args) System.setProperty(Constants.CLI_JAVA_PROPERTY, "true"); final Installer installer = new Installer(); - installer.setTempLogFile(tempLogFile); + // The first point where an install can fail: from here on there is a log to keep, and + // this is where it gets created. + installer.setTempLogFile(tempLogFile.get()); installer.setUserData(uData); installer.setProgressMessageFormatter(formatter); installer.addProgressUpdateListener( diff --git a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties index 464522010e..e0f708c6bc 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/quickSetup.properties @@ -12,6 +12,7 @@ # # Copyright 2006-2010 Sun Microsystems, Inc. # Portions Copyright 2010-2016 ForgeRock AS. +# Portions Copyright 2026 3A Systems, LLC. @@ -346,6 +347,10 @@ INFO_GENERAL_LOADING=Loading... INFO_GENERAL_SEE_FOR_DETAILS=See %s for a detailed log of this operation. INFO_GENERAL_PROVIDE_LOG_IN_ERROR=See %s for a detailed log of the failed operation.\ \nPlease report this error and provide the log file mentioned above. +INFO_GENERAL_LOG_IN_ERROR_MISSING=The log file %s of the failed operation is no \ + longer there, so it cannot be provided with the error report. +INFO_GENERAL_LOG_IN_ERROR_UNREADABLE=The log file %s of the failed operation \ + could not be read: %s INFO_GENERAL_SERVER_STARTED=started INFO_GENERAL_SERVER_STOPPED=stopped INFO_GENERAL_WARNING=Warning diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java new file mode 100644 index 0000000000..e429c84503 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/LauncherTest.java @@ -0,0 +1,133 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; + +import org.forgerock.i18n.LocalizableMessage; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import com.forgerock.opendj.cli.ArgumentParser; + +/** + * Tests when a launcher creates its temporary log file. + *

+ * Nothing removes that file unless the operation it belongs to succeeds, so a road which + * attempts nothing - {@code setup --help}, {@code --version}, a usage error - must not create + * one, nor the directory it would live in (issue #1030). + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class LauncherTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private TempLogFile created; + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("launcherTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + if (created != null) + { + created.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + @Test + public void testBuildingALauncherLeavesNothingOnDisk() throws Exception + { + final File instance = new File(tempDir, "not-yet-laid-down"); + final File logs = new File(instance, "logs"); + + final TestLauncher launcher = new TestLauncher(logs); + + assertFalse(launcher.hasTempLogFile(), "the log must wait for a road that can fail"); + assertFalse(logs.exists(), logs.getPath()); + assertFalse(instance.exists(), instance.getPath()); + } + + @Test + public void testAskingForTheLogCreatesItOnce() throws Exception + { + final File logs = new File(tempDir, "asked-for/logs"); + final TestLauncher launcher = new TestLauncher(logs); + + final TempLogFile logFile = launcher.getTempLogFile(); + created = logFile; + + assertTrue(logFile.isReadable(), logFile.getPath()); + assertEquals(logFile.getLogFile().getCanonicalFile().getParentFile(), logs.getCanonicalFile()); + assertTrue(launcher.hasTempLogFile()); + assertSame(launcher.getTempLogFile(), logFile, "a second ask must not create a second log"); + } + + /** A launcher with nothing in it but the log file behaviour under test. */ + private static final class TestLauncher extends Launcher + { + TestLauncher(final File tempLogFileDirectory) + { + super(new String[0], PREFIX, tempLogFileDirectory); + } + + @Override + public ArgumentParser getArgumentParser() + { + return null; + } + + @Override + protected LocalizableMessage getFrameTitle() + { + return LocalizableMessage.raw("test"); + } + + @Override + protected CliApplication createCliApplication() + { + return null; + } + + @Override + protected void willLaunchGui() + { + // nothing is launched here + } + + @Override + protected void guiLaunchFailed() + { + // nothing is launched here + } + } +} diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java new file mode 100644 index 0000000000..c3add16ebc --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/TempLogFileTest.java @@ -0,0 +1,170 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup; + +import static java.nio.charset.Charset.defaultCharset; +import static java.nio.file.StandardOpenOption.APPEND; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; + +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Tests that a {@link TempLogFile} can be placed in a directory of the caller's choosing (the + * instance {@code logs/} directory for setup, so that {@code start-ds} does not sweep it away + * with the rest of {@code tmp/}, see issue #1030), and that it tells whether the file is still + * there to be read. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class TempLogFileTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private final List created = new ArrayList<>(); + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("tempLogFileTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + for (TempLogFile logFile : created) + { + logFile.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + private TempLogFile track(TempLogFile logFile) + { + created.add(logFile); + return logFile; + } + + private static File parentOf(TempLogFile logFile) throws IOException + { + return logFile.getLogFile().getCanonicalFile().getParentFile(); + } + + /** The directory does not exist before setup lays the instance down: it has to be created. */ + @Test + public void testLogFileIsCreatedInTheRequestedDirectory() throws Exception + { + final File logs = new File(tempDir, "not-yet-laid-down/logs"); + assertFalse(logs.exists()); + + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, logs)); + + assertTrue(logFile.isEnabled()); + assertTrue(logFile.isReadable()); + assertEquals(parentOf(logFile), logs.getCanonicalFile()); + assertTrue(logFile.getLogFile().getName().startsWith(PREFIX), logFile.getPath()); + assertTrue(logFile.getLogFile().getName().endsWith(".log"), logFile.getPath()); + } + + @Test + public void testReadContentsReturnsWhatIsInTheFile() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + // The log's own stream is not in append mode and sits at the end of its own bytes, so a + // record written after the marker would be written over it: shut the writer first, and + // the marker is the last thing in the file whatever else the JVM logs. + logFile.writer.shutdown(); + final String marker = "the last line written before the failure"; + Files.write(logFile.getLogFile().toPath(), (marker + "\n").getBytes(defaultCharset()), APPEND); + + assertTrue(logFile.readContents().endsWith(marker + "\n")); + } + + /** Being enabled means messages are logged; being readable means the file is there to hand over. */ + @Test + public void testIsReadableFollowsTheFileNotTheLogger() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + assertTrue(logFile.isReadable()); + + // Not File.delete(): the writer still holds the file, and Windows does not delete a file + // that is open. deleteLogFileAfterSuccess() shuts the writer first, as setup does. + logFile.deleteLogFileAfterSuccess(); + assertFalse(logFile.getLogFile().exists()); + + assertTrue(logFile.isEnabled()); + assertFalse(logFile.isReadable()); + try + { + logFile.readContents(); + fail("reading a deleted log must fail"); + } + catch (IOException expected) + { + // the caller reports it instead of promising the file + } + } + + /** A directory where the log was is not a log: there is nothing to hand over either. */ + @Test + public void testADirectoryAtTheLogPathIsNotReadable() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs"))); + logFile.deleteLogFileAfterSuccess(); + assertTrue(logFile.getLogFile().mkdir()); + + assertTrue(Files.isReadable(logFile.getLogFile().toPath())); + assertFalse(logFile.isReadable()); + } + + @Test + public void testNoDirectoryMeansTheTemporaryDirectory() throws Exception + { + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, null)); + + assertTrue(logFile.isEnabled()); + assertEquals(parentOf(logFile), new File(System.getProperty("java.io.tmpdir")).getCanonicalFile()); + } + + /** A directory that cannot be used must not cost the log: fall back to the temporary directory. */ + @Test + public void testUnusableDirectoryFallsBackToTheTemporaryDirectory() throws Exception + { + final File notADirectory = new File(tempDir, "not-a-directory"); + assertTrue(notADirectory.createNewFile()); + + final TempLogFile logFile = track(TempLogFile.newTempLogFile(PREFIX, notADirectory)); + + assertTrue(logFile.isEnabled()); + assertTrue(logFile.isReadable()); + assertNotEquals(parentOf(logFile), notADirectory.getCanonicalFile()); + assertEquals(parentOf(logFile), new File(System.getProperty("java.io.tmpdir")).getCanonicalFile()); + } +} diff --git a/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java new file mode 100644 index 0000000000..6e9f462188 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/InstallerTest.java @@ -0,0 +1,168 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.quicksetup.installer; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.opends.messages.QuickSetupMessages.*; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.forgerock.i18n.LocalizableMessage; +import org.opends.quicksetup.TempLogFile; +import org.opends.quicksetup.event.ProgressUpdateEvent; +import org.opends.quicksetup.event.ProgressUpdateListener; +import org.opends.quicksetup.util.PlainTextProgressMessageFormatter; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Tests what a failed installation tells its listeners about the log file. + *

+ * This report is the one place the diagnosis of a failed setup lives, and it used to promise + * the file without looking at it - printing a {@code NoSuchFileException} stack when the file + * was gone (issue #1030). The roads below are the whole of that decision. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "quicksetup" }, sequential = true) +public class InstallerTest extends DirectoryServerTestCase +{ + private static final String PREFIX = "opendj-setup-"; + + private File tempDir; + private final List created = new ArrayList<>(); + + @BeforeClass + public void setUp() throws IOException + { + tempDir = TestCaseUtils.createTemporaryDirectory("installerTest"); + } + + @AfterClass + public void tearDown() throws IOException + { + for (TempLogFile logFile : created) + { + logFile.deleteLogFileAfterSuccess(); + } + TestCaseUtils.deleteDirectory(tempDir); + } + + /** A log that is there is named and written out, as before. */ + @Test + public void testAReadableLogIsHandedOver() throws Exception + { + final TempLogFile logFile = newLogFile(); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(logFile.getPath())); + // The line the constructor logs: the contents of the file reach the report. + assertTrue(report.contains("QuickSetup application launched"), report); + assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(logFile.getPath())); + } + + /** A log something else removed is reported as gone, instead of being asked for. */ + @Test + public void testAMissingLogIsReportedAsMissing() throws Exception + { + final TempLogFile logFile = newLogFile(); + final String path = logFile.getPath(); + logFile.deleteLogFileAfterSuccess(); + assertFalse(logFile.isReadable()); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); + // The line that asks for the file must not go out when there is no file to provide. + assertDoesNotContain(report, INFO_GENERAL_PROVIDE_LOG_IN_ERROR.get(path)); + } + + /** A log which is there but cannot be read costs the report its contents, not its diagnosis. */ + @Test + public void testAnUnreadableLogIsReportedAsUnreadable() throws Exception + { + final String path = new File(tempDir, "unreadable.log").getAbsolutePath(); + final IOException failure = new IOException("Input/output error"); + final TempLogFile logFile = mock(TempLogFile.class); + when(logFile.isEnabled()).thenReturn(true); + when(logFile.isReadable()).thenReturn(true); + when(logFile.getPath()).thenReturn(path); + when(logFile.readContents()).thenThrow(failure); + + final String report = reportOf(logFile); + + assertContains(report, INFO_GENERAL_LOG_IN_ERROR_UNREADABLE.get(path, failure)); + assertDoesNotContain(report, INFO_GENERAL_LOG_IN_ERROR_MISSING.get(path)); + } + + /** Nothing is said about a log that was never created. */ + @Test + public void testNoLogMeansNoReport() throws Exception + { + final TempLogFile logFile = mock(TempLogFile.class); + when(logFile.isEnabled()).thenReturn(false); + + assertTrue(reportOf(logFile).isEmpty(), "a launcher without a log has nothing to report"); + } + + private TempLogFile newLogFile() + { + final TempLogFile logFile = TempLogFile.newTempLogFile(PREFIX, new File(tempDir, "logs")); + created.add(logFile); + return logFile; + } + + /** What the listeners of a failed installation are told about the log file. */ + private static String reportOf(final TempLogFile logFile) + { + final Installer installer = new Installer(); + installer.setProgressMessageFormatter(new PlainTextProgressMessageFormatter()); + final StringBuilder report = new StringBuilder(); + installer.addProgressUpdateListener(new ProgressUpdateListener() + { + @Override + public void progressUpdate(final ProgressUpdateEvent ev) + { + if (ev.getNewLogs() != null) + { + report.append(ev.getNewLogs()); + } + } + }); + installer.setTempLogFile(logFile); + installer.notifyListenersOfExistingLogFile(); + return report.toString(); + } + + private static void assertContains(final String report, final LocalizableMessage expected) + { + assertTrue(report.contains(expected.toString()), "expected <" + expected + "> in <" + report + ">"); + } + + private static void assertDoesNotContain(final String report, final LocalizableMessage unexpected) + { + assertFalse(report.contains(unexpected.toString()), "unexpected <" + unexpected + "> in <" + report + ">"); + } +}