Revert to 1376.v18876d10ce9c detectWithContainer logic (#788)

* Revert to 1376.v18876d10ce9c detectWithContainer logic Restores with container detection handling back to 1376.v18876d10ce9c. This version contained special handling for kubernetes-plugin as well as naming convention for other plugins to trigger special with container handling. * Add tests for detectWithContainer * source format --------- Co-authored-by: Benoit GUERIN <[email protected]>
jenkinsci · May 29, 2024 · b57d3dd · b57d3dd
1 parent c11858d
commit b57d3dd
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 10 deletions.
diff --git a/...ine-maven/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java b/...ine-maven/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java
@@ -111,6 +111,8 @@
 import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 import org.jenkinsci.plugins.workflow.steps.GeneralNonBlockingStepExecution;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.springframework.util.ClassUtils;
 
 @SuppressFBWarnings(
@@ -257,23 +259,56 @@ private boolean detectWithContainer() throws IOException {
         Launcher launcher1 = launcher;
         while (launcher1 instanceof Launcher.DecoratedLauncher) {
             String launcherClassName = launcher1.getClass().getName();
-            // ToDo: add support for container() step (ContainerExecDecorator) from Kubernetes plugin
-            if (launcherClassName.contains("WithContainerStep")) {
-                LOGGER.log(Level.FINE, "Step running within docker.image(): {0}", launcherClassName);
+            Optional<Boolean> withContainer = isWithContainerLauncher(launcherClassName);
+            if (withContainer.isPresent()) {
+                boolean result = withContainer.get();
+                if (result) {
+                    console.trace(
+                            "[withMaven] IMPORTANT \"withMaven(){...}\" step running within a Docker container. See ");
+                    console.traceHyperlink(
+                            "https://github.com/jenkinsci/pipeline-maven-plugin/blob/master/FAQ.adoc#how-to-use-the-pipeline-maven-plugin-with-docker",
+                            "Pipeline Maven Plugin FAQ");
+                    console.trace(" in case of problem.");
+                }
 
-                console.trace(
-                        "[withMaven] IMPORTANT \"withMaven(){...}\" step running within a Docker container. See ");
-                console.traceHyperlink(
-                        "https://github.com/jenkinsci/pipeline-maven-plugin/blob/master/FAQ.adoc#how-to-use-the-pipeline-maven-plugin-with-docker",
-                        "Pipeline Maven Plugin FAQ");
-                console.trace(" in case of problem.");
-                return true;
+                return result;
             }
+
             launcher1 = ((Launcher.DecoratedLauncher) launcher1).getInner();
         }
         return false;
     }
 
+    /**
+     * Check if the launcher class name is a known container launcher.
+     * @param launcherClassName launcher class name
+     * @return empty if unknown and should keep checking, true if it is a container launcher, false if it is not.
+     */
+    @Restricted(NoExternalUse.class)
+    protected static Optional<Boolean> isWithContainerLauncher(String launcherClassName) {
+        // kubernetes-plugin container step execution does not require special container handling
+        if (launcherClassName.contains("org.csanchez.jenkins.plugins.kubernetes.pipeline.ContainerExecDecorator")) {
+            LOGGER.log(Level.FINE, "Step running within Kubernetes withContainer(): {1}", launcherClassName);
+            return Optional.of(false);
+        }
+
+        // for plugins that require special container handling should include this name launcher naming convention
+        // since there is no common interface to detect if a step is running within a container
+        if (launcherClassName.contains("ContainerExecDecorator")) {
+            LOGGER.log(Level.FINE, "Step running within container exec decorator: {0}", launcherClassName);
+            return Optional.of(true);
+        }
+
+        // detect docker.image().inside {} or withDockerContainer step from docker-workflow-plugin, which has the
+        // launcher name org.jenkinsci.plugins.docker.workflow.WithContainerStep.Decorator
+        if (launcherClassName.contains("WithContainerStep")) {
+            LOGGER.log(Level.FINE, "Step running within docker.image(): {0}", launcherClassName);
+            return Optional.of(true);
+        }
+
+        return Optional.empty();
+    }
+
     /**
      * Setup the selected JDK. If none is provided nothing is done.
      */

diff --git a/...maven/src/test/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2Test.java b/...maven/src/test/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2Test.java
@@ -31,4 +31,35 @@ public void testEscapeWindowsBatchChars() {
 
         assertThat(actualEscapedMavenConfig).isEqualTo(expectedEscapedMavenConfig);
     }
+
+    @Test
+    public void testIsWithContainerLauncherUnknown() {
+        assertThat(WithMavenStepExecution2.isWithContainerLauncher("hudson.SomeUnknownLauncher"))
+                .isEmpty();
+    }
+
+    @Test
+    public void testIsWithContainerLauncherWithContainerStep() {
+        assertThat(WithMavenStepExecution2.isWithContainerLauncher(
+                        "org.jenkinsci.plugins.docker.workflow.WithContainerStep.Decorator"))
+                .contains(true);
+    }
+
+    @Test
+    public void testIsWithContainerLauncherKubernetesPluginContainerExecDecorator() {
+        assertThat(WithMavenStepExecution2.isWithContainerLauncher(
+                        "org.csanchez.jenkins.plugins.kubernetes.pipeline.ContainerExecDecorator"))
+                .contains(false);
+    }
+
+    @Test
+    public void testIsWithContainerLauncherCustomContainerExecDecorator() {
+        // for plugins that supporting launching their own containerized steps
+        assertThat(WithMavenStepExecution2.isWithContainerLauncher("com.jenkins.plugins.custom.ContainerExecDecorator"))
+                .contains(true);
+        assertThat(
+                        WithMavenStepExecution2.isWithContainerLauncher(
+                                "com.jenkins.plugins.kubernetes.pipeline.MyContainerExecDecorator.MyContainerDecoratedLauncher"))
+                .contains(true);
+    }
 }