Merge branch 'master' into logging-exMTL

Author: jimklimov

lib/src/main/java/com/cloudbees/groovy/cps/impl/CallEnv.java

@@ -132,4 +132,10 @@ public void buildStackTraceElements(List<StackTraceElement> stack, int depth) {
     public int getDepth() {
         return depth;
     }
+
+    @Override
+    public String toString() {
+        return callSiteLoc != null ? super.toString() + " @" + callSiteLoc : super.toString();
+    }
+
 }

plugin/pom.xml

@@ -41,6 +41,8 @@
         </license>
     </licenses>
     <properties>
+        <!-- TODO until in parent -->
+        <jenkins-test-harness.version>2403.v256947ecb_c8a_</jenkins-test-harness.version>
         <no-test-jar>false</no-test-jar>
         <node.version>18.15.0</node.version>
         <yarn.version>1.22.19</yarn.version>
@@ -54,6 +56,17 @@
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
+            <!-- TODO until in BOM -->
+            <dependency>
+                <groupId>org.jenkins-ci.plugins</groupId>
+                <artifactId>support-core</artifactId>
+                <version>1575.v49b_01a_460e3b_</version>
+            </dependency>
+            <dependency>
+                <groupId>org.jenkins-ci.plugins.workflow</groupId>
+                <artifactId>workflow-api</artifactId>
+                <version>1366.vf1fb_e1a_f6b_22</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
     <dependencies>
@@ -226,7 +239,7 @@
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>testcontainers</artifactId>
-            <version>1.20.4</version>
+            <version>1.20.6</version>
             <scope>test</scope>
             <exclusions>
                 <!-- Provided by Jenkins core -->
@@ -239,7 +252,7 @@
         <dependency>
             <groupId>org.awaitility</groupId>
             <artifactId>awaitility</artifactId>
-            <version>4.2.2</version>
+            <version>4.3.0</version>
             <scope>test</scope>
         </dependency>
         <dependency>

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java

@@ -145,8 +145,6 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import net.jcip.annotations.GuardedBy;
 
-import org.acegisecurity.Authentication;
-import org.acegisecurity.userdetails.UsernameNotFoundException;
 import java.nio.charset.StandardCharsets;
 import jenkins.util.SystemProperties;
 import org.codehaus.groovy.GroovyBugError;
@@ -158,6 +156,8 @@
 import org.jenkinsci.plugins.workflow.flow.FlowExecutionList;
 import org.jenkinsci.plugins.workflow.graph.FlowGraphWalker;
 import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
 
 /**
  * {@link FlowExecution} implemented with Groovy CPS.
@@ -425,8 +425,8 @@ public CpsFlowExecution(@NonNull String script, boolean sandbox, @NonNull  FlowE
         this.script = script;
         this.sandbox = sandbox;
         this.durabilityHint = durabilityHint;
-        Authentication auth = Jenkins.getAuthentication();
-        this.user = auth.equals(ACL.SYSTEM) ? null : auth.getName();
+        Authentication auth = Jenkins.getAuthentication2();
+        this.user = auth.equals(ACL.SYSTEM2) ? null : auth.getName();
         this.storage = createStorage();
         this.storage.setAvoidAtomicWrite(!this.getDurabilityHint().isAtomicWrite());
     }
@@ -1872,21 +1872,21 @@ void notifyListeners(List<FlowNode> nodes, boolean synchronous) {
         }
     }
 
-    @Override public Authentication getAuthentication() {
+    @Override public Authentication getAuthentication2() {
         if (user == null) {
-            return ACL.SYSTEM;
+            return ACL.SYSTEM2;
         }
         try {
             User u = User.getById(user, true);
             if (u == null) {
-                return Jenkins.ANONYMOUS;
+                return Jenkins.ANONYMOUS2;
             } else {
-                return u.impersonate();
+                return u.impersonate2();
             }
         } catch (UsernameNotFoundException x) {
             LOGGER.log(Level.WARNING, "could not restore authentication", x);
             // Should not expose this to callers.
-            return Jenkins.ANONYMOUS;
+            return Jenkins.ANONYMOUS2;
         }
     }
 

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java

@@ -169,7 +169,7 @@ void restore() {
 
     private ThreadContext setUp() {
         CpsFlowExecution execution = cpsThreadGroup.getExecution();
-        ACL.impersonate(execution.getAuthentication());
+        ACL.impersonate2(execution.getAuthentication2());
         CURRENT.set(cpsThreadGroup);
         cpsThreadGroup.busy = true;
         Thread t = Thread.currentThread();

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/LoggingInvoker.java

@@ -30,6 +30,7 @@
 import com.cloudbees.groovy.cps.sandbox.SandboxInvoker;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import groovy.lang.Closure;
 import groovy.lang.GroovyClassLoader;
 import hudson.ExtensionList;
 import java.util.Set;
@@ -133,10 +134,55 @@ private void maybeRecord(Class<?> clazz, Supplier<String> message) {
         return delegate.getProperty(lhs, name);
     }
 
+    private static final Set<String> CLOSURE_METAPROPS = Set.of("delegate", "directive", "resolveStrategy");
+
     @Override public void setProperty(Object lhs, String name, Object value) throws Throwable {
         Class<?> clazz = classOf(lhs);
         maybeRecord(clazz, () -> clazz.getName() + "." + name);
         delegate.setProperty(lhs, name, value);
+        if (SystemProperties.getBoolean(LoggingInvoker.class.getName() + ".fieldSetWarning", true)) {
+            if (value != null && !CLOSURE_METAPROPS.contains(name)) {
+                var receiver = findReceiver(lhs);
+                if (receiver instanceof CpsScript) {
+                    try {
+                        receiver.getClass().getDeclaredField(name);
+                        // OK, @Field, ignore
+                    } catch (NoSuchFieldException x) {
+                        var g = CpsThreadGroup.current();
+                        if (g != null) {
+                            var e = g.getExecution();
+                            if (e != null) {
+                                e.getOwner().getListener().getLogger().println(Messages.LoggingInvoker_field_set(receiver.getClass().getSimpleName(), name, value.getClass().getSimpleName()));
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    private Object findReceiver(Object o) {
+        if (o instanceof Closure<?> c) {
+            // c.f. https://github.com/apache/groovy/blob/41b990d0a20e442f29247f0e04cbed900f3dcad4/src/main/groovy/lang/Closure.java#L344
+            return switch (c.getResolveStrategy()) {
+                case Closure.DELEGATE_ONLY -> findReceiver(c.getDelegate());
+                case Closure.OWNER_ONLY -> findReceiver(c.getOwner());
+                case Closure.TO_SELF -> c;
+                case Closure.DELEGATE_FIRST -> {
+                    var delegate = c.getDelegate();
+                    if (delegate == null) {
+                        yield findReceiver(c.getOwner());
+                    }
+                    // TODO: Groovy will actually try the delegate first and then the owner if needed, but it is
+                    // difficult for us to know what will happen in advance.
+                    yield findReceiver(delegate);
+                }
+                default ->
+                    // TODO: Groovy will actually try the owner first and then the delegate if needed.
+                    findReceiver(c.getOwner());
+            };
+        }
+        return o;
     }
 
     @Override public Object getAttribute(Object lhs, String name) throws Throwable {

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/Messages.properties

@@ -6,3 +6,8 @@ SnippetizerLink.OnlineDocsLink.displayName=Online Documentation
 SnippetizerLink.ExamplesLink.displayName=Examples Reference
 SnippetizerLink.GDSLLink.displayName=IntelliJ IDEA GDSL
 Pipeline.Syntax=Pipeline Syntax
+# TODO perhaps create a https://jenkins.io/redirect/pipeline-field-set/
+LoggingInvoker.field_set=\
+  Did you forget the `def` keyword? \
+  {0} seems to be setting a field named {1} (to a value of type {2}) \
+  which could lead to memory leaks or other issues.

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/Snippetizer/globals.groovy

@@ -30,7 +30,7 @@ l.layout(title:_("Pipeline Syntax: Global Variable Reference"), norefresh: true)
                 }
               }
               dd{
-                def rd = request.getView(v, "help");
+                def rd = request2.getView(v, "help");
                 div(class:"help", style:"display: block") {
                   if (rd != null) {
                     st.include(page: "help", it: v)

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/Snippetizer/index.jelly

@@ -40,7 +40,7 @@ THE SOFTWARE.
                 </f:block>
                 <f:section title="${%Steps}"/>
                 <!-- Similar to f:dropdownDescriptorSelector, but adds fallback content to block, and JENKINS-25130 adds per-selection help: -->
-                <j:set var="item" value="${it.getItem(request)}"/>
+                <j:set var="item" value="${it.getItem(request2)}"/>
                 <d:taglib uri="local">
                     <d:tag name="listSteps">
                         <j:forEach var="quasiDescriptor" items="${quasiDescriptors}">

plugin/src/test/java/org/jenkinsci/plugins/workflow/Workflow2Test.java

@@ -88,6 +88,9 @@ private static void stage2(JenkinsRule r) throws Throwable {
         inputStepExecution.proceed(null);
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         var b2 = await().until(() -> p.getBuildByNumber(2), Matchers.notNullValue());
-        assertThat("Expecting the replayed build to use sandbox", ((CpsFlowExecution) b2.getExecution()).isSandbox(), is(true));
+        var execution = await().until(b2::getExecution, Matchers.notNullValue());
+        assertThat("Expecting the replayed build to use sandbox", ((CpsFlowExecution) execution).isSandbox(), is(true));
+        b2.doStop();
+        r.waitForCompletion(b2);
     }
 }