[MNG-8406] Proper IT isolation (#1968)

Implement proper IT isolation, in a way, that user running ITs should not have it's own env "mixed in" into IT runs. In essence, use "alternate" `user.home` for ITs. Also use proper means to set user home. This will also stop fork 4 ITs (as they were forked due presence of env variable). Note: yes, this causes longer IT execution time (as can be seen), as the ITs user home (core-it-suite/target/user-home) starts as empty (tail is set to "outer" local repo), so ITs do download stuff. --- https://issues.apache.org/jira/browse/MNG-8406
apache · Dec 12, 2024 · 79d7739 · 79d7739
1 parent 0f15ad7
commit 79d7739
Show file tree

Hide file tree

Showing 9 changed files with 122 additions and 108 deletions.
diff --git a/its/core-it-suite/pom.xml b/its/core-it-suite/pom.xml
@@ -518,6 +518,8 @@ under the License.
           <useSystemClassLoader>false</useSystemClassLoader>
           <promoteUserPropertiesToSystemProperties>false</promoteUserPropertiesToSystemProperties>
           <systemPropertyVariables>
+            <maven.test.user.home>${project.build.directory}/user-home</maven.test.user.home>
+            <maven.test.repo.local>${settings.localRepository}</maven.test.repo.local>
             <maven.test.tmpdir>${project.build.testOutputDirectory}</maven.test.tmpdir>
             <maven.version>${maven.version}</maven.version>
             <maven.home>${preparedMavenHome}</maven.home>
@@ -595,28 +597,6 @@ under the License.
         </plugins>
       </build>
     </profile>
-    <profile>
-      <id>maven-repo-local</id>
-      <activation>
-        <property>
-          <name>maven.repo.local</name>
-        </property>
-      </activation>
-      <build>
-        <plugins>
-          <plugin>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <configuration>
-              <systemPropertyVariables>
-                <!-- Pass this through to the tests (if set!) to have them
-                  pick the right repository -->
-                <maven.repo.local>${maven.repo.local}</maven.repo.local>
-              </systemPropertyVariables>
-            </configuration>
-          </plugin>
-        </plugins>
-      </build>
-    </profile>
     <profile>
       <id>maven-repo-local-tail</id>
       <activation>

diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/ItUtils.java b/its/core-it-suite/src/test/java/org/apache/maven/it/ItUtils.java
@@ -62,24 +62,19 @@ public static String calcHash(File file, String algo) throws Exception {
     }
 
     /**
-     * @see ItUtils#setUserHome(Verifier, Path)
+     * @deprecated Use {@link Verifier#setUserHomeDirectory(Path)} instead.
      */
+    @Deprecated
     public static void setUserHome(Verifier verifier, File file) {
         setUserHome(verifier, file.toPath());
     }
 
     /**
-     * Note that this only has effect when fork mode is set to true.
-     * Please make sure to call {@link Verifier#setForkJvm(boolean)} and set it to true
+     * @deprecated Use {@link Verifier#setUserHomeDirectory(Path)} instead.
      */
+    @Deprecated
     public static void setUserHome(Verifier verifier, Path home) {
-        // NOTE: We set the user.home directory instead of say settings.security to reflect Maven's normal behavior
-        String path = home.toAbsolutePath().toString();
-        if (path.indexOf(' ') < 0) {
-            verifier.setEnvironmentVariable("MAVEN_OPTS", "-Duser.home=" + path);
-        } else {
-            verifier.setEnvironmentVariable("MAVEN_OPTS", "\"-Duser.home=" + path + "\"");
-        }
+        verifier.setUserHomeDirectory(home);
     }
 
     public static void assertCanonicalFileEquals(File expected, File actual) throws IOException {

diff --git a/...it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java b/...it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java
@@ -122,7 +122,7 @@ public void testitBasic() throws Exception {
         verifier.deleteArtifacts("org.apache.maven.its.mng0553");
         verifier.verifyArtifactNotPresent("org.apache.maven.its.mng0553", "a", "0.1-SNAPSHOT", "jar");
         verifier.filterFile("settings-template.xml", "settings.xml", filterProps);
-        ItUtils.setUserHome(verifier, new File(testDir, "userhome"));
+        verifier.setUserHomeDirectory(new File(testDir, "userhome").toPath());
         verifier.addCliArgument("--settings");
         verifier.addCliArgument("settings.xml");
         verifier.addCliArgument("validate");
@@ -185,7 +185,7 @@ public void testitEncryption() throws Exception {
 
         Verifier verifier = newVerifier(testDir.getAbsolutePath());
         verifier.setAutoclean(false);
-        ItUtils.setUserHome(verifier, new File(testDir, "userhome"));
+        verifier.setUserHomeDirectory(new File(testDir, "userhome").toPath());
         verifier.addCliArgument("--encrypt-master-password");
         verifier.addCliArgument("test");
         verifier.setLogFileName("log-emp.txt");
@@ -198,7 +198,7 @@ public void testitEncryption() throws Exception {
 
         verifier = newVerifier(testDir.getAbsolutePath());
         verifier.setAutoclean(false);
-        ItUtils.setUserHome(verifier, new File(testDir, "userhome"));
+        verifier.setUserHomeDirectory(new File(testDir, "userhome").toPath());
         verifier.addCliArgument("--encrypt-password");
         verifier.addCliArgument("testpass");
         verifier.setLogFileName("log-ep.txt");

diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3955EffectiveSettingsTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3955EffectiveSettingsTest.java
@@ -48,7 +48,9 @@ public void testitMNG3955() throws Exception {
         File testDir = extractResources("/mng-3955");
 
         Verifier verifier = newVerifier(testDir.getAbsolutePath());
+        String localRepo = verifier.getLocalRepository();
         verifier.setAutoclean(false);
+        verifier.addCliArgument("-Dmaven.repo.local.tail=" + localRepo);
         verifier.addCliArgument("--settings");
         verifier.addCliArgument("settings.xml");
         verifier.addCliArgument("--offline");

diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5669ReadPomsOnce.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5669ReadPomsOnce.java
@@ -50,7 +50,7 @@ public void testWithoutBuildConsumer() throws Exception {
         Verifier verifier = newVerifier(testDir.getAbsolutePath(), false);
         Map<String, String> filterProperties = Collections.singletonMap(
                 "${javaAgentJar}",
-                verifier.getArtifactPath("org.apache.maven.its", "core-it-javaagent", "2.1-SNAPSHOT", "jar"));
+                verifier.getSupportArtifactPath("org.apache.maven.its", "core-it-javaagent", "2.1-SNAPSHOT", "jar"));
         verifier.filterFile(".mvn/jvm.config", ".mvn/jvm.config", null, filterProperties);
 
         verifier.setForkJvm(true); // pick up agent

diff --git a/...core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java b/...core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java
@@ -25,7 +25,7 @@
 
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 public class MavenITmng7772CoreExtensionFoundTest extends AbstractMavenIntegrationTestCase {
     public MavenITmng7772CoreExtensionFoundTest() {
@@ -37,12 +37,15 @@ public void testWithExtensionsXmlCoreExtensionsFound() throws Exception {
         File testDir = extractResources("/mng-7772-core-extensions-found");
 
         Verifier verifier = newVerifier(new File(testDir, "extension").getAbsolutePath());
+        verifier.setLogFileName("extension-install.txt");
         verifier.addCliArgument("install");
         verifier.execute();
         verifier.verifyErrorFreeLog();
+        String installedToLocalRepo = verifier.getLocalRepository();
 
         verifier = newVerifier(testDir.getAbsolutePath());
-        ItUtils.setUserHome(verifier, Paths.get(testDir.toPath().toString(), "home-extensions-xml"));
+        verifier.setUserHomeDirectory(Paths.get(testDir.toPath().toString(), "home-extensions-xml"));
+        verifier.addCliArgument("-Dmaven.repo.local=" + installedToLocalRepo);
 
         verifier.addCliArgument("validate");
         verifier.execute();
@@ -54,27 +57,22 @@ public void testWithExtensionsXmlCoreExtensionsFound() throws Exception {
     public void testWithLibExtCoreExtensionsFound() throws Exception {
         File testDir = extractResources("/mng-7772-core-extensions-found");
 
-        Verifier verifier = newVerifier(new File(testDir, "extension").getAbsolutePath());
+        Path extensionBasedir = new File(testDir, "extension").getAbsoluteFile().toPath();
+        Verifier verifier = newVerifier(extensionBasedir.toString());
+        verifier.setLogFileName("extension-package.txt");
         verifier.addCliArgument("package");
         verifier.execute();
         verifier.verifyErrorFreeLog();
 
-        Path jarPath = Paths.get(verifier.getArtifactPath(
-                "org.apache.maven.its.7772-core-extensions-scopes", "maven-it-core-extensions", "0.1", "jar", ""));
-
-        assertNotNull(jarPath, "Jar output path was not found in the log");
+        Path jarPath = extensionBasedir.resolve("target").resolve("maven-it-core-extensions-0.1.jar");
 
-        Path jarToPath = Paths.get(testDir.toString(), "home-lib-ext", ".m2", "ext", "extension.jar");
-        try {
-            Files.copy(jarPath, jarToPath);
+        assertTrue("Jar output path was not built", Files.isRegularFile(jarPath));
 
-            verifier = newVerifier(testDir.getAbsolutePath());
-            ItUtils.setUserHome(verifier, Paths.get(testDir.toPath().toString(), "home-lib-ext"));
-            verifier.addCliArgument("validate");
-            verifier.execute();
-            verifier.verifyTextInLog("[INFO] Extension loaded!");
-        } finally {
-            Files.deleteIfExists(jarToPath);
-        }
+        verifier = newVerifier(testDir.getAbsolutePath());
+        verifier.setUserHomeDirectory(Paths.get(testDir.toPath().toString(), "home-lib-ext"));
+        verifier.addCliArgument("-Dmaven.ext.class.path=" + jarPath);
+        verifier.addCliArgument("validate");
+        verifier.execute();
+        verifier.verifyTextInLog("[INFO] Extension loaded!");
     }
 }
diff --git a/...-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java b/...-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java
@@ -38,7 +38,7 @@ public void testCoreExtensionsNotFound() throws Exception {
         File testDir = extractResources("/mng-7772-core-extensions-not-found");
 
         Verifier verifier = newVerifier(testDir.getAbsolutePath());
-        ItUtils.setUserHome(verifier, Paths.get(testDir.toPath().toString(), "home"));
+        verifier.setUserHomeDirectory(Paths.get(testDir.toPath().toString(), "home"));
 
         try {
             verifier.addCliArgument("validate");

diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java
@@ -40,7 +40,7 @@ void testLegacy() throws Exception {
 
         Verifier verifier = newVerifier(testDir.getAbsolutePath());
         verifier.setLogFileName("log-legacy.txt");
-        ItUtils.setUserHome(verifier, new File(testDir, "legacyhome"));
+        verifier.setUserHomeDirectory(new File(testDir, "legacyhome").toPath());
         verifier.addCliArgument("org.apache.maven.plugins:maven-help-plugin:3.3.0:effective-settings");
         verifier.addCliArgument("-DshowPasswords");
         verifier.execute();
@@ -62,7 +62,7 @@ void testModern() throws Exception {
         Verifier verifier = newVerifier(testDir.getAbsolutePath());
         verifier.setLogFileName("log-modern.txt");
         verifier.setEnvironmentVariable("MAVEN_MASTER_PASSWORD", "master");
-        ItUtils.setUserHome(verifier, new File(testDir, "home"));
+        verifier.setUserHomeDirectory(new File(testDir, "home").toPath());
         verifier.addCliArgument("org.apache.maven.plugins:maven-help-plugin:3.3.0:effective-settings");
         verifier.addCliArgument("-DshowPasswords");
         verifier.execute();

diff --git a/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java b/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java
@@ -92,7 +92,7 @@ public class Verifier {
 
     private final Path tempBasedir; // empty basedir for queries
 
-    private Path userHomeDirectory;
+    private final Path outerLocalRepository; // this is the "outer" build effective local repo
 
     private final List<String> defaultCliArguments;
 
@@ -102,13 +102,15 @@ public class Verifier {
 
     private final List<String> cliArguments = new ArrayList<>();
 
+    private Path userHomeDirectory; // the user home
+
     private String executable = ExecutorRequest.MVN;
 
     private boolean autoClean = true;
 
     private boolean forkJvm = false;
 
-    private boolean handleLocalRepoTail = true;
+    private boolean handleLocalRepoTail = true; // if false: IT will become fully isolated
 
     private String logFileName = "log.txt";
 
@@ -132,7 +134,9 @@ public Verifier(String basedir, List<String> defaultCliArguments) throws Verific
         try {
             this.basedir = Paths.get(basedir).toAbsolutePath();
             this.tempBasedir = Files.createTempDirectory("verifier");
-            this.userHomeDirectory = Paths.get(System.getProperty("user.home"));
+            this.userHomeDirectory = Paths.get(System.getProperty("maven.test.user.home", "user.home"));
+            Files.createDirectories(this.userHomeDirectory);
+            this.outerLocalRepository = Paths.get(System.getProperty("maven.test.repo.local", ".m2/repository"));
             this.executorHelper = new HelperImpl(
                     VERIFIER_FORK_MODE,
                     Paths.get(System.getProperty("maven.home")),
@@ -147,7 +151,7 @@ public Verifier(String basedir, List<String> defaultCliArguments) throws Verific
     }
 
     public void setUserHomeDirectory(Path userHomeDirectory) {
-        this.userHomeDirectory = requireNonNull(userHomeDirectory);
+        this.userHomeDirectory = requireNonNull(userHomeDirectory, "userHomeDirectory");
     }
 
     public String getExecutable() {
@@ -167,55 +171,29 @@ public void execute() throws VerificationException {
         if (handleLocalRepoTail) {
             // note: all used Strings are non-null/empty if "not present" for simpler handling
             // "outer" build pass these in, check are they present or not
+            // Important: here we do "string ops" only, and no path ops, as it will be Maven
+            // (based on user.home and other) that will unravel these strings to paths!
             String outerTail = System.getProperty("maven.repo.local.tail", "").trim();
-            String outerHead = System.getProperty("maven.repo.local", "").trim();
-
-            // if none of the outer thing is set, we have nothing to do
-            if (!outerTail.isEmpty() || !outerHead.isEmpty()) {
-                String itTail = args.stream()
-                        .filter(s -> s.startsWith("-Dmaven.repo.local.tail="))
-                        .map(s -> s.substring(24).trim())
-                        .findFirst()
-                        .orElse("");
-                if (!itTail.isEmpty()) {
-                    // remove it
-                    args = args.stream()
-                            .filter(s -> !s.startsWith("-Dmaven.repo.local.tail="))
-                            .collect(Collectors.toList());
-                }
-                String itHead = args.stream()
-                        .filter(s -> s.startsWith("-Dmaven.repo.local="))
-                        .map(s -> s.substring(19).trim())
-                        .findFirst()
-                        .orElse("");
-                if (!itHead.isEmpty()) {
-                    // remove it
-                    args = args.stream()
-                            .filter(s -> !s.startsWith("-Dmaven.repo.local="))
-                            .collect(Collectors.toList());
-                }
+            String outerHead = outerLocalRepository.toString();
+
+            String itTail = args.stream()
+                    .filter(s -> s.startsWith("-Dmaven.repo.local.tail="))
+                    .map(s -> s.substring(24).trim())
+                    .findFirst()
+                    .orElse("");
+            if (!itTail.isEmpty()) {
+                // remove it
+                args = args.stream()
+                        .filter(s -> !s.startsWith("-Dmaven.repo.local.tail="))
+                        .collect(Collectors.toList());
+            }
 
-                if (!itHead.isEmpty()) {
-                    // itHead present: itHead left as is, push all to itTail
-                    args.add("-Dmaven.repo.local=" + itHead);
-                    itTail = Stream.of(itTail, outerHead, outerTail)
-                            .filter(s -> !s.isEmpty())
-                            .collect(Collectors.joining(","));
-                    if (!itTail.isEmpty()) {
-                        args.add("-Dmaven.repo.local.tail=" + itTail);
-                    }
-                } else {
-                    // itHead not present: if outerHead present, make it itHead; join itTail and outerTail as tail
-                    if (!outerHead.isEmpty()) {
-                        args.add("-Dmaven.repo.local=" + outerHead);
-                    }
-                    itTail = Stream.of(itTail, outerTail)
-                            .filter(s -> !s.isEmpty())
-                            .collect(Collectors.joining(","));
-                    if (!itTail.isEmpty()) {
-                        args.add("-Dmaven.repo.local.tail=" + itTail);
-                    }
-                }
+            // push things to tail
+            itTail = Stream.of(itTail, outerHead, outerTail)
+                    .filter(s -> !s.isEmpty())
+                    .collect(Collectors.joining(","));
+            if (!itTail.isEmpty()) {
+                args.add("-Dmaven.repo.local.tail=" + itTail);
             }
         }
 
@@ -657,6 +635,67 @@ public String getArtifactPath(String gid, String aid, String version, String ext
                 + executorHelper.artifactPath(executorHelper.executorRequest(), gav, null);
     }
 
+    private String getSupportArtifactPath(String artifact) {
+        StringTokenizer tok = new StringTokenizer(artifact, ":");
+        if (tok.countTokens() != 4) {
+            throw new IllegalArgumentException("Artifact must have 4 tokens: '" + artifact + "'");
+        }
+
+        String[] a = new String[4];
+        for (int i = 0; i < 4; i++) {
+            a[i] = tok.nextToken();
+        }
+
+        String groupId = a[0];
+        String artifactId = a[1];
+        String version = a[2];
+        String ext = a[3];
+        return getSupportArtifactPath(groupId, artifactId, version, ext);
+    }
+
+    public String getSupportArtifactPath(String groupId, String artifactId, String version, String ext) {
+        return getSupportArtifactPath(groupId, artifactId, version, ext, null);
+    }
+
+    /**
+     * Returns the absolute path to the artifact denoted by groupId, artifactId, version, extension and classifier.
+     *
+     * @param gid        The groupId, must not be null.
+     * @param aid        The artifactId, must not be null.
+     * @param version    The version, must not be null.
+     * @param ext        The extension, must not be null.
+     * @param classifier The classifier, may be null to be omitted.
+     * @return the absolute path to the artifact denoted by groupId, artifactId, version, extension and classifier,
+     *         never null.
+     */
+    public String getSupportArtifactPath(String gid, String aid, String version, String ext, String classifier) {
+        if (classifier != null && classifier.isEmpty()) {
+            classifier = null;
+        }
+        if ("maven-plugin".equals(ext)) {
+            ext = "jar";
+        } else if ("coreit-artifact".equals(ext)) {
+            ext = "jar";
+            classifier = "it";
+        } else if ("test-jar".equals(ext)) {
+            ext = "jar";
+            classifier = "tests";
+        }
+
+        String gav;
+        if (classifier != null) {
+            gav = gid + ":" + aid + ":" + ext + ":" + classifier + ":" + version;
+        } else {
+            gav = gid + ":" + aid + ":" + ext + ":" + version;
+        }
+        return outerLocalRepository
+                .resolve(executorHelper.artifactPath(
+                        executorHelper.executorRequest().argument("-Dmaven.repo.local=" + outerLocalRepository),
+                        gav,
+                        null))
+                .toString();
+    }
+
     public List<String> getArtifactFileNameList(String org, String name, String version, String ext) {
         List<String> files = new ArrayList<>();
         String artifactPath = getArtifactPath(org, name, version, ext);