Respect enforce URL configuration option for Maven builds (#395)

* Respect enforce URL configuration option for Maven builds * Use a library call to find diff between sets
jenkinsci · Feb 12, 2024 · f6d815d · f6d815d
1 parent 54b3bad
commit f6d815d
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 18 deletions.
diff --git a/src/main/java/hudson/plugins/gradle/injection/GitScmListener.java b/src/main/java/hudson/plugins/gradle/injection/GitScmListener.java
@@ -1,6 +1,7 @@
 package hudson.plugins.gradle.injection;
 
 import com.google.common.base.Strings;
+import com.google.common.collect.Sets;
 import hudson.EnvVars;
 import hudson.Extension;
 import hudson.FilePath;
@@ -21,7 +22,9 @@
 import javax.annotation.Nonnull;
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 import static hudson.plugins.gradle.injection.GradleInjectionAware.JENKINSGRADLEPLUGIN_GRADLE_ENTERPRISE_GRADLE_INJECTION_ENABLED;
 import static hudson.plugins.gradle.injection.MavenInjectionAware.JENKINSGRADLEPLUGIN_MAVEN_PLUGIN_CONFIG_EXT_CLASSPATH;
@@ -59,7 +62,7 @@ public void onCheckout(
 
             // Check .mvn/extensions.xml for already applied Develocity extension for maven injection only
             if (mavenExtensionAlreadyApplied(config, workspace)) {
-                disableAutoInjection(build, workspace, config, listener);
+                disableMavenAutoInjection(build, workspace, config, listener);
             }
         } catch (Exception e) {
             LOGGER.error("Error occurred when processing onCheckout notification", e);
@@ -93,6 +96,31 @@ private static void disableAutoInjection(
         }
     }
 
+    private static void disableMavenAutoInjection(
+            Run<?, ?> build,
+            FilePath workspace,
+            InjectionConfig config,
+            TaskListener listener
+    ) throws Exception {
+        Computer computer = workspace.toComputer();
+        if (computer == null) {
+            return;
+        }
+
+        EnvVars envVars = computer.buildEnvironment(listener);
+
+        String currentMavenOpts = envVars.get(MavenOptsHandler.MAVEN_OPTS);
+        if (currentMavenOpts != null) {
+            Set<String> keepUrl = config.isEnforceUrl()
+                    ? Sets.newHashSet(MavenInjectionAware.GRADLE_ENTERPRISE_URL_PROPERTY_KEY.name)
+                    : Collections.emptySet();
+
+            String mavenOpts = Strings.nullToEmpty(MAVEN_OPTS_HANDLER.removeIfNeeded(currentMavenOpts, keepUrl));
+
+            build.addAction(new MavenInjectionDisabledAction(mavenOpts));
+        }
+    }
+
     private static boolean isInjectionGloballyDisabled(InjectionConfig config) {
         return config.isDisabled() || InjectionUtil.isInvalid(InjectionConfig.checkRequiredUrl(config.getServer()));
     }

diff --git a/src/main/java/hudson/plugins/gradle/injection/MavenOptsHandler.java b/src/main/java/hudson/plugins/gradle/injection/MavenOptsHandler.java
@@ -1,5 +1,6 @@
 package hudson.plugins.gradle.injection;
 
+import com.google.common.collect.Sets;
 import hudson.model.Node;
 
 import javax.annotation.Nullable;
@@ -41,7 +42,7 @@ String merge(Node node, List<SystemProperty> systemProperties) {
                 .map(SystemProperty::asString)
                 .collect(Collectors.joining(SPACE));
 
-        return filtered(getCurrentMavenOpts(node))
+        return filtered(getCurrentMavenOpts(node), Collections.emptySet())
                 .map(current -> String.join(SPACE, current, additionalProperties))
                 .orElse(additionalProperties);
     }
@@ -57,11 +58,15 @@ void removeIfNeeded(Node node) throws IOException, InterruptedException {
             return;
         }
 
-        String mavenOpts = filtered(currentMavenOpts).orElse(null);
+        String mavenOpts = filtered(currentMavenOpts, Collections.emptySet()).orElse(null);
         EnvUtil.setEnvVar(node, MAVEN_OPTS, mavenOpts);
     }
 
     public String removeIfNeeded(String currentMavenOpts) {
+        return removeIfNeeded(currentMavenOpts, Collections.emptySet());
+    }
+
+    public String removeIfNeeded(String currentMavenOpts, Set<String> keepKeys) {
         if (currentMavenOpts == null || currentMavenOpts.isEmpty()) {
             return null;
         }
@@ -71,12 +76,12 @@ public String removeIfNeeded(String currentMavenOpts) {
             return null;
         }
 
-        return filtered(currentMavenOpts).orElse(null);
+        return filtered(currentMavenOpts, keepKeys).orElse(null);
     }
 
-    private Optional<String> filtered(@Nullable String mavenOpts) throws RuntimeException {
+    private Optional<String> filtered(@Nullable String mavenOpts, Set<String> keepKeys) throws RuntimeException {
         return Optional.ofNullable(mavenOpts)
-            .map(this::filterMavenOpts);
+            .map(it -> filterMavenOpts(it, keepKeys));
     }
 
     @Nullable
@@ -89,10 +94,10 @@ private static String getCurrentMavenOpts(Node node) {
      * that were added by the auto-injection.
      */
     @Nullable
-    private String filterMavenOpts(String mavenOpts) {
+    private String filterMavenOpts(String mavenOpts, Set<String> keepKeys) {
         String filtered =
             Arrays.stream(mavenOpts.split(SPACE))
-                .filter(this::shouldBeKept)
+                .filter(systemProperty -> shouldBeKept(systemProperty, keepKeys))
                 .collect(Collectors.joining(SPACE));
 
         if (filtered.isEmpty()) {
@@ -102,7 +107,7 @@ private String filterMavenOpts(String mavenOpts) {
         return filtered;
     }
 
-    private boolean shouldBeKept(String systemProperty) {
-        return keys.stream().noneMatch(systemProperty::contains);
+    private boolean shouldBeKept(String systemProperty, Set<String> keepKeys) {
+        return Sets.difference(keys, keepKeys).stream().noneMatch(systemProperty::contains);
     }
 }
diff --git a/...test/groovy/hudson/plugins/gradle/injection/BuildScanInjectionMavenIntegrationTest.groovy b/...test/groovy/hudson/plugins/gradle/injection/BuildScanInjectionMavenIntegrationTest.groovy
@@ -703,9 +703,13 @@ node {
     }
 
     @SuppressWarnings("GStringExpressionWithinString")
-    def 'extension already applied in pipeline project and build scan attempted to publish to project configured host - #mavenSetup'(String mavenSetup) {
+    def 'extension already applied in pipeline project and build scan attempted to publish to project configured host - #mavenSetup #isUrlEnforced'(String mavenSetup, boolean isUrlEnforced) {
         given:
+        withInjectionConfig {
+            enforceUrl = isUrlEnforced
+        }
         createSlaveAndTurnOnInjection()
+
         def mavenInstallationName = setupMavenInstallation()
         def replacedMavenSetup = mavenSetup.replaceAll("mavenInstallationName", mavenInstallationName)
 
@@ -731,17 +735,28 @@ node {
         def build = j.buildAndAssertSuccess(pipelineJob)
 
         then:
-        // Project has localhost:8080 configured
-        j.assertLogContains("[WARNING] No build scan will be published: Gradle Enterprise features were not enabled due to an unexpected error while contacting Gradle Enterprise.", build)
+        if (isUrlEnforced) {
+            j.assertLogContains("Publishing build scan...", build)
+        } else {
+            // Project has localhost:8080 configured
+            j.assertLogContains("[WARNING] No build scan will be published: Gradle Enterprise features were not enabled due to an unexpected error while contacting Gradle Enterprise", build)
+        }
 
         where:
-        mavenSetup << ["withEnv([\"PATH+MAVEN=\${tool 'mavenInstallationName'}/bin\"]) {" , "withMaven(maven: 'mavenInstallationName') {"]
+        mavenSetup                                                          | isUrlEnforced
+        "withEnv([\"PATH+MAVEN=\${tool 'mavenInstallationName'}/bin\"]) {"  | false
+        "withMaven(maven: 'mavenInstallationName') {"                       | false
+        "withEnv([\"PATH+MAVEN=\${tool 'mavenInstallationName'}/bin\"]) {"  | true
+        "withMaven(maven: 'mavenInstallationName') {"                       | true
     }
 
-    def 'extension already applied in freestyle project and build scan attempted to publish to project configured host'() {
+    def 'extension already applied in freestyle project and build scan attempted to publish to project configured host - #isUrlEnforced'(boolean isUrlEnforced) {
         given:
-        mavenInstallationRule.mavenVersion = '3.9.2'
+        mavenInstallationRule.mavenVersion = '3.9.6'
         mavenInstallationRule.addInstallation()
+        withInjectionConfig {
+            enforceUrl = isUrlEnforced
+        }
         createSlaveAndTurnOnInjection()
 
         def project = j.createFreeStyleProject()
@@ -762,8 +777,15 @@ node {
         def build = j.buildAndAssertSuccess(project)
 
         then:
-        // Project has localhost:8080 configured
-        j.assertLogContains("[WARNING] No build scan will be published: Gradle Enterprise features were not enabled due to an unexpected error while contacting Gradle Enterprise.", build)
+        if (isUrlEnforced) {
+            j.assertLogContains("Publishing build scan...", build)
+        } else {
+            // Project has localhost:8080 configured
+            j.assertLogContains("[WARNING] No build scan will be published: Gradle Enterprise features were not enabled due to an unexpected error while contacting Gradle Enterprise", build)
+        }
+
+        where:
+        isUrlEnforced << [false, true]
     }
 
     private static void assertMavenConfigClasspathJars(DumbSlave slave, String... jars) {