From 79d7739dcc5c60d3a189d631690e9ad59f5595e1 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 12 Dec 2024 15:24:03 +0100 Subject: [PATCH] [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 --- its/core-it-suite/pom.xml | 24 +-- .../java/org/apache/maven/it/ItUtils.java | 15 +- ...nITmng0553SettingsAuthzEncryptionTest.java | 6 +- .../MavenITmng3955EffectiveSettingsTest.java | 2 + .../maven/it/MavenITmng5669ReadPomsOnce.java | 2 +- .../MavenITmng7772CoreExtensionFoundTest.java | 34 ++--- ...enITmng7772CoreExtensionsNotFoundTest.java | 2 +- .../it/MavenITmng8379SettingsDecryptTest.java | 4 +- .../java/org/apache/maven/it/Verifier.java | 139 +++++++++++------- 9 files changed, 121 insertions(+), 107 deletions(-) diff --git a/its/core-it-suite/pom.xml b/its/core-it-suite/pom.xml index b236580ac0..19a42780dc 100644 --- a/its/core-it-suite/pom.xml +++ b/its/core-it-suite/pom.xml @@ -518,6 +518,8 @@ under the License. false false + ${project.build.directory}/user-home + ${settings.localRepository} ${project.build.testOutputDirectory} ${maven.version} ${preparedMavenHome} @@ -595,28 +597,6 @@ under the License. - - maven-repo-local - - - maven.repo.local - - - - - - maven-surefire-plugin - - - - ${maven.repo.local} - - - - - - maven-repo-local-tail 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 index 6c0cc0b738..5e84518ccd 100644 --- 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 @@ class ItUtils { } /** - * @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/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java index c38caf896f..aaee06396b 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng0553SettingsAuthzEncryptionTest.java @@ -122,7 +122,7 @@ public class MavenITmng0553SettingsAuthzEncryptionTest extends AbstractMavenInte 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 class MavenITmng0553SettingsAuthzEncryptionTest extends AbstractMavenInte 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 class MavenITmng0553SettingsAuthzEncryptionTest extends AbstractMavenInte 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 index 8dbf0e20a8..d7d1b6b9ba 100644 --- 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 class MavenITmng3955EffectiveSettingsTest extends AbstractMavenIntegratio 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 index b5bee035c8..e020d3ff8f 100644 --- 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 class MavenITmng5669ReadPomsOnce extends AbstractMavenIntegrationTestCase Verifier verifier = newVerifier(testDir.getAbsolutePath(), false); Map 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/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java index 3e5dae4245..6b210695f1 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionFoundTest.java @@ -25,7 +25,7 @@ import java.nio.file.Paths; 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 class MavenITmng7772CoreExtensionFoundTest extends AbstractMavenIntegrati 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 class MavenITmng7772CoreExtensionFoundTest extends AbstractMavenIntegrati 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", "")); + Path jarPath = extensionBasedir.resolve("target").resolve("maven-it-core-extensions-0.1.jar"); - assertNotNull(jarPath, "Jar output path was not found in the log"); + assertTrue("Jar output path was not built", Files.isRegularFile(jarPath)); - Path jarToPath = Paths.get(testDir.toString(), "home-lib-ext", ".m2", "ext", "extension.jar"); - try { - Files.copy(jarPath, jarToPath); - - 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/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java index 81c6cc921e..a109ac9648 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng7772CoreExtensionsNotFoundTest.java @@ -38,7 +38,7 @@ public class MavenITmng7772CoreExtensionsNotFoundTest extends AbstractMavenInteg 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 index 1bb80efb4e..a615d6f2fc 100644 --- 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 @@ class MavenITmng8379SettingsDecryptTest extends AbstractMavenIntegrationTestCase 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 @@ class MavenITmng8379SettingsDecryptTest extends AbstractMavenIntegrationTestCase 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 index 7e74123d9f..4624035ce3 100644 --- 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 defaultCliArguments; @@ -102,13 +102,15 @@ public class Verifier { private final List 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 class Verifier { 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 class Verifier { } public void setUserHomeDirectory(Path userHomeDirectory) { - this.userHomeDirectory = requireNonNull(userHomeDirectory); + this.userHomeDirectory = requireNonNull(userHomeDirectory, "userHomeDirectory"); } public String getExecutable() { @@ -167,55 +171,29 @@ public class Verifier { 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(); + String outerHead = outerLocalRepository.toString(); - // 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 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 class Verifier { + 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 getArtifactFileNameList(String org, String name, String version, String ext) { List files = new ArrayList<>(); String artifactPath = getArtifactPath(org, name, version, ext);