Issue #6017 - Start Property Overrides (#7118)

* Issue #6017 - jetty-start Property override behavior

+ All *.mod based configuration defaults are default
  properties are not forced values and can be overridden.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

jetty-start/src/main/java/org/eclipse/jetty/start/Module.java

@@ -408,8 +408,30 @@ public void process(BaseHome basehome) throws FileNotFoundException, IOException
                                 break;
                             case "DEFAULTS": // old name introduced in 9.2.x
                             case "INI": // new name for 9.3+
-                                _defaultConfig.add(line);
+                            {
+                                // All default properties are to be treated as `<k>?=<v>` by the configuration system
+                                // even if they are present as `<k>=<v>`
+                                int idx = line.indexOf('=');
+                                if (idx > 0)
+                                {
+                                    String key = line.substring(0, idx);
+                                    String value = line.substring(idx + 1);
+                                    if (key.endsWith("?"))
+                                    {
+                                        // already the correct way
+                                        _defaultConfig.add(line);
+                                    }
+                                    else
+                                    {
+                                        _defaultConfig.add(String.format("%s?=%s", key, value));
+                                    }
+                                }
+                                else
+                                {
+                                    _defaultConfig.add(line);
+                                }
                                 break;
+                            }
                             case "INI-TEMPLATE":
                                 _iniTemplate.add(line);
                                 break;

jetty-start/src/test/java/org/eclipse/jetty/start/usecases/PropertyOverrideTest.java

@@ -0,0 +1,180 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.start.usecases;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.eclipse.jetty.start.FS;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.eclipse.jetty.toolchain.test.ExtraMatchers.ordered;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class PropertyOverrideTest extends AbstractUseCase
+{
+    /**
+     * Ensure that a property specified in a `*.mod` file can be overridden by the command line.
+     */
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "jetty.sslContext.keyStorePassword=invalid",
+        "jetty.sslContext.keyStorePassword?=invalid"
+    })
+    public void testBasicModuleIniPropertyOverrideWithCommandLine(String propRef) throws Exception
+    {
+        setupStandardHomeDir();
+
+        Files.write(homeDir.resolve("modules/ssl.mod"),
+            List.of(
+                "[depend]",
+                "main",
+                "[ini-template]",
+                "# jetty.sslContext.keyStorePassword=default"
+            ),
+            StandardCharsets.UTF_8);
+
+        FS.ensureDirectoryExists(baseDir.resolve("modules"));
+
+        Files.write(baseDir.resolve("modules/ssl-ini.mod"),
+            List.of(
+                "[depend]",
+                "ssl",
+                "[ini]",
+                propRef
+            ),
+            StandardCharsets.UTF_8);
+
+        FS.ensureDirectoryExists(baseDir.resolve("start.d"));
+        Files.write(baseDir.resolve("start.d/main.ini"),
+            List.of(
+                "--module=ssl-ini"
+            ),
+            StandardCharsets.UTF_8);
+
+        // === Execute Main
+        List<String> commandLine = List.of(
+            // this "command line" property value should win
+            "jetty.sslContext.keyStorePassword=storepwd"
+        );
+        ExecResults results = exec(commandLine, false);
+
+        // === Validate Resulting XMLs
+        List<String> expectedXmls = Arrays.asList(
+            "${jetty.home}/etc/base.xml",
+            "${jetty.home}/etc/main.xml"
+        );
+        List<String> actualXmls = results.getXmls();
+        assertThat("XML Resolution Order", actualXmls, ordered(expectedXmls));
+
+        // === Validate Resulting LIBs
+        List<String> expectedLibs = Arrays.asList(
+            "${jetty.home}/lib/base.jar",
+            "${jetty.home}/lib/main.jar",
+            "${jetty.home}/lib/other.jar"
+        );
+        List<String> actualLibs = results.getLibs();
+        assertThat("Libs", actualLibs, ordered(expectedLibs));
+
+        // === Validate Resulting Properties
+        List<String> expectedProperties = new ArrayList<>();
+        // we should see value from command line
+        expectedProperties.add("jetty.sslContext.keyStorePassword=storepwd");
+        expectedProperties.add("main.prop=value0");
+        List<String> actualProperties = results.getProperties();
+
+        Collections.sort(expectedProperties);
+        Collections.sort(actualProperties);
+        assertThat("Properties", actualProperties, ordered(expectedProperties));
+    }
+
+    /**
+     * Ensure that a property specified in a `*.mod` file can be overridden by the ini configuration.
+     */
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "jetty.sslContext.keyStorePassword=invalid",
+        "jetty.sslContext.keyStorePassword?=invalid"
+    })
+    public void testBasicModuleIniPropertyOverrideWithStartdIni(String propRef) throws Exception
+    {
+        setupStandardHomeDir();
+
+        Files.write(homeDir.resolve("modules/ssl.mod"),
+            List.of(
+                "[depend]",
+                "main",
+                "[ini-template]",
+                "# jetty.sslContext.keyStorePassword=default"
+            ),
+            StandardCharsets.UTF_8);
+
+        FS.ensureDirectoryExists(baseDir.resolve("modules"));
+
+        Files.write(baseDir.resolve("modules/ssl-ini.mod"),
+            List.of(
+                "[depend]",
+                "ssl",
+                "[ini]",
+                propRef
+            ),
+            StandardCharsets.UTF_8);
+
+        FS.ensureDirectoryExists(baseDir.resolve("start.d"));
+        Files.write(baseDir.resolve("start.d/main.ini"),
+            List.of(
+                "--module=ssl-ini",
+                // this should override mod default
+                "jetty.sslContext.keyStorePassword=storepwd"
+            ),
+            StandardCharsets.UTF_8);
+
+        // === Execute Main
+        List<String> commandLine = List.of();
+        ExecResults results = exec(commandLine, false);
+
+        // === Validate Resulting XMLs
+        List<String> expectedXmls = Arrays.asList(
+            "${jetty.home}/etc/base.xml",
+            "${jetty.home}/etc/main.xml"
+        );
+        List<String> actualXmls = results.getXmls();
+        assertThat("XML Resolution Order", actualXmls, ordered(expectedXmls));
+
+        // === Validate Resulting LIBs
+        List<String> expectedLibs = Arrays.asList(
+            "${jetty.home}/lib/base.jar",
+            "${jetty.home}/lib/main.jar",
+            "${jetty.home}/lib/other.jar"
+        );
+        List<String> actualLibs = results.getLibs();
+        assertThat("Libs", actualLibs, ordered(expectedLibs));
+
+        // === Validate Resulting Properties
+        List<String> expectedProperties = new ArrayList<>();
+        // we should see value from command line
+        expectedProperties.add("jetty.sslContext.keyStorePassword=storepwd");
+        expectedProperties.add("main.prop=value0");
+        List<String> actualProperties = results.getProperties();
+
+        Collections.sort(expectedProperties);
+        Collections.sort(actualProperties);
+        assertThat("Properties", actualProperties, ordered(expectedProperties));
+    }
+}

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

@@ -38,6 +38,7 @@
 import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
 import org.eclipse.jetty.io.ClientConnector;
 import org.eclipse.jetty.start.FS;
+import org.eclipse.jetty.toolchain.test.PathAssert;
 import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets;
 import org.eclipse.jetty.unixsocket.server.UnixSocketConnector;
 import org.eclipse.jetty.util.BlockingArrayQueue;
@@ -968,6 +969,45 @@ public void testModuleWithExecEmitsWarning() throws Exception
         }
     }
 
+    @Test
+    public void testIniSectionPropertyOverriddenByCommandLine() throws Exception
+    {
+        String jettyVersion = System.getProperty("jettyVersion");
+        JettyHomeTester distribution = JettyHomeTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        Path jettyBase = distribution.getJettyBase();
+        Path jettyBaseModules = jettyBase.resolve("modules");
+        Files.createDirectories(jettyBaseModules);
+        String pathProperty = "jetty.sslContext.keyStorePath";
+        // Create module with an [ini] section with an invalid password,
+        // which should be overridden on the command line at startup.
+        String module = "" +
+            "[depends]\n" +
+            "ssl\n" +
+            "\n" +
+            "[ini]\n" +
+            "" + pathProperty + "=modbased\n";
+        String moduleName = "ssl-ini";
+        Files.write(jettyBaseModules.resolve(moduleName + ".mod"), module.getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE);
+
+        try (JettyHomeTester.Run run1 = distribution.start("--add-module=https,test-keystore,ssl-ini"))
+        {
+            assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, run1.getExitValue());
+
+            // Override the property on the command line with the correct password.
+            try (JettyHomeTester.Run run2 = distribution.start(pathProperty + "=cmdline"))
+            {
+                assertTrue(run2.awaitConsoleLogsFor("Started Server@", 5, TimeUnit.SECONDS));
+                PathAssert.assertFileExists("${jetty.base}/cmdline", jettyBase.resolve("cmdline"));
+                PathAssert.assertNotPathExists("${jetty.base}/modbased", jettyBase.resolve("modbased"));
+            }
+        }
+    }
+
     @Test
     public void testWellKnownModule() throws Exception
     {