Skip to content

Commit

Permalink
[JENKINS-31582] Using ArgumentsAction.getResolvedArguments to better …
Browse files Browse the repository at this point in the history 
…handle JENKINS-45109.
  • Loading branch information
@jglick
jglick committed Jun 29, 2017
1 parent 4f703cf commit 0a0e050
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 28 deletions.
4 changes: 2 additions & 2 deletions pom.xml
View file
Expand Up @@ -28,7 +28,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.30</version>
<version>2.31</version>
<relativePath />
</parent>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down Expand Up @@ -78,7 +78,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
<version>2.16</version>
<version>2.18-20170629.143731-7</version> <!-- TODO https://github.com/jenkinsci/workflow-api-plugin/pull/44 -->
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImpl.java
View file
Expand Up @@ -225,6 +225,7 @@ Object sanitizeListAndRecordMutation(@Nonnull List objects, @CheckForNull EnvVar
* (including the Step or UninstantiatedDescribable)
*/
@CheckForNull
@SuppressWarnings("unchecked")
Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull EnvVars vars) {
// Package scoped so we can test it directly
Object tempVal = o;
Expand Down Expand Up @@ -254,7 +255,13 @@ Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull Env
if (modded != tempVal) {
// Sanitization stripped out some values, so we need to record that and return modified version
this.isUnmodifiedBySanitization = false;
return modded;
if (o instanceof UninstantiatedDescribable) {
// Need to retain the symbol.
UninstantiatedDescribable ud = (UninstantiatedDescribable) o;
return new UninstantiatedDescribable(ud.getSymbol(), ud.getKlass(), (Map<String, ?>) modded);
} else {
return modded;
}
} else { // Any mutation was just from exploding step/uninstantiated describable, and we can just use the original
return o;
}
Expand Down
56 changes: 33 additions & 23 deletions src/main/java/org/jenkinsci/plugins/workflow/cps/nodes/StepAtomNode.java
View file
Expand Up @@ -25,6 +25,7 @@
package org.jenkinsci.plugins.workflow.cps.nodes;

import hudson.model.Action;
import hudson.model.Describable;
import hudson.model.Descriptor;
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
import org.jenkinsci.plugins.workflow.graph.AtomNode;
Expand All @@ -34,8 +35,10 @@

import java.io.ObjectStreamException;
import java.util.Collections;
import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.structs.SymbolLookup;
import org.jenkinsci.plugins.structs.describable.DescribableModel;
import org.jenkinsci.plugins.structs.describable.DescribableParameter;
Expand Down Expand Up @@ -84,19 +87,11 @@ protected Object readResolve() throws ObjectStreamException {
if (d == null) {
return null;
}
if (d.isMetaStep()) {
DescribableParameter p = new DescribableModel<>(d.clazz).getFirstRequiredParameter();
if (p != null) {
Object arg = ArgumentsAction.getArguments((FlowNode) node).get(p.getName());
if (arg instanceof UninstantiatedDescribable) {
String symbol = ((UninstantiatedDescribable) arg).getSymbol();
if (symbol != null) {
Descriptor<?> descriptor = SymbolLookup.get().findDescriptor(p.getErasedType(), symbol);
if (descriptor != null) {
return descriptor.getDisplayName();
}
} // TODO to support $class it might be better to go through DescribableModel.resolveClass, if it were public; cf. discussion in JENKINS-31582
}
Class<?> delegateType = getDelegateType((FlowNode) node, d);
if (delegateType != null && Describable.class.isAssignableFrom(delegateType)) {
Descriptor<?> descriptor = Jenkins.getInstance().getDescriptor(delegateType.asSubclass(Describable.class));
if (descriptor != null) {
return descriptor.getDisplayName();
}
}
return d.getDisplayName();
Expand All @@ -113,16 +108,11 @@ protected String getTypeDisplayName() {
if (d == null) {
return null;
}
if (d.isMetaStep()) {
DescribableParameter p = new DescribableModel<>(d.clazz).getFirstRequiredParameter();
if (p != null) {
Object arg = ArgumentsAction.getArguments((FlowNode) node).get(p.getName());
if (arg instanceof UninstantiatedDescribable) {
String symbol = ((UninstantiatedDescribable) arg).getSymbol();
if (symbol != null) {
return symbol;
}
}
Class<?> delegateType = getDelegateType((FlowNode) node, d);
if (delegateType != null) {
Set<String> symbols = SymbolLookup.getSymbolValue(delegateType);
if (!symbols.isEmpty()) {
return symbols.iterator().next();
}
}
return d.getFunctionName();
Expand All @@ -133,4 +123,24 @@ protected String getTypeFunctionName() {
String fn = effectiveFunctionName(this);
return fn != null ? fn : descriptorId;
}

/**
* @return for example {@code JUnitResultArchiver} given {@code junit '…'}
*/
private static @CheckForNull Class<?> getDelegateType(@Nonnull FlowNode node, @Nonnull StepDescriptor d) {
if (d.isMetaStep()) {
DescribableParameter p = new DescribableModel<>(d.clazz).getFirstRequiredParameter();
if (p != null) {
Object arg = ArgumentsAction.getResolvedArguments(node).get(p.getName());
if (arg instanceof UninstantiatedDescribable) {
DescribableModel<?> delegateModel = ((UninstantiatedDescribable) arg).getModel();
if (delegateModel != null) {
return delegateModel.getType();
}
}
}
}
return null;
}

}
26 changes: 24 additions & 2 deletions src/test/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImplTest.java
View file
Expand Up @@ -13,6 +13,7 @@
import hudson.Functions;
import hudson.XmlFile;
import hudson.model.Action;
import hudson.tasks.ArtifactArchiver;
import org.apache.commons.lang.RandomStringUtils;
import org.jenkinsci.plugins.credentialsbinding.impl.BindingStep;
import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
Expand Down Expand Up @@ -40,10 +41,10 @@
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import static org.hamcrest.Matchers.*;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.JenkinsRule;

import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
Expand Down Expand Up @@ -394,7 +395,7 @@ public void testUnusualStepInstantiations() throws Exception {
" node('master') { \n" +
" writeFile text: 'hello world', file: 'msg.out'\n" +
" step([$class: 'ArtifactArchiver', artifacts: 'msg.out', fingerprint: false])\n "+
" withEnv(['CUSTOM=val']) {\n"+ //Symbol-based, because withEnv is a metastep
" withEnv(['CUSTOM=val']) {\n"+ //Symbol-based, because withEnv is a metastep; TODO huh? no it is not
" echo env.CUSTOM\n"+
" }\n"+
"}"
Expand Down Expand Up @@ -423,4 +424,25 @@ public void testUnusualStepInstantiations() throws Exception {
Assert.assertEquals("CUSTOM=val", (String)((ArrayList) ob).get(0));
testDeserialize(run.getExecution());
}

@Test
public void testReallyUnusualStepInstantiations() throws Exception {
WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "unusualInstantiation");
job.setDefinition(new CpsFlowDefinition(
" node() {\n" +
" writeFile text: 'hello world', file: 'msg.out'\n" +
" step(new hudson.tasks.ArtifactArchiver('msg.out'))\n" + // note, not whitelisted
"}", false));
WorkflowRun run = r.buildAndAssertSuccess(job);
LinearScanner scan = new LinearScanner();

FlowNode testNode = scan.findFirstMatch(run.getExecution().getCurrentHeads().get(0), new NodeStepTypePredicate("step"));
ArgumentsAction act = testNode.getPersistentAction(ArgumentsAction.class);
Assert.assertNotNull(act);
Object delegate = act.getArgumentValue("delegate");
Assert.assertThat(delegate, instanceOf(ArtifactArchiver.class));
Assert.assertEquals("msg.out", ((ArtifactArchiver) delegate).getArtifacts());
Assert.assertFalse(((ArtifactArchiver) delegate).isFingerprint());
}

}
88 changes: 88 additions & 0 deletions src/test/java/org/jenkinsci/plugins/workflow/cps/nodes/StepNodeTest.java
View file
Expand Up @@ -29,6 +29,8 @@
import hudson.model.Result;
import hudson.tasks.junit.JUnitResultArchiver;
import java.util.List;
import java.util.logging.Level;
import org.apache.commons.lang.StringUtils;
import static org.hamcrest.Matchers.*;
import org.jenkinsci.plugins.configfiles.buildwrapper.ConfigFileBuildWrapper;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
Expand All @@ -39,16 +41,19 @@
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import static org.junit.Assert.*;
import org.junit.ClassRule;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;

public class StepNodeTest {

@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
@Rule public JenkinsRule r = new JenkinsRule();
@Rule public LoggerRule logging = new LoggerRule().record(StepAtomNode.class, Level.FINE);

@Issue("JENKINS-45109")
@Test public void metastepConsole() throws Exception {
Expand Down Expand Up @@ -78,4 +83,87 @@ public class StepNodeTest {
r.assertLogContains("[Pipeline] // configFileProvider", b);
}

@Test public void metastepConsoleShellClass() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
"node {\n" +
" wrap([$class: 'ConfigFileBuildWrapper', managedFiles: []]) {\n" +
" writeFile text: '''<testsuite name='a'><testcase name='c'><error>failed</error></testcase></testsuite>''', file: 'x.xml'\n" +
" step([$class: 'JUnitResultArchiver', testResults: 'x.xml'])\n" +
" }\n" +
"}", true));
WorkflowRun b = r.assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0));
List<FlowNode> coreStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("step"));
assertThat(coreStepNodes, hasSize(1));
assertEquals("junit", coreStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(JUnitResultArchiver.class).getDisplayName(), coreStepNodes.get(0).getDisplayName());
List<FlowNode> coreWrapperStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), Predicates.and(new NodeStepTypePredicate("wrap"), new Predicate<FlowNode>() {
@Override public boolean apply(FlowNode n) {
return n instanceof StepStartNode && !((StepStartNode) n).isBody();
}
}));
assertThat(coreWrapperStepNodes, hasSize(1));
assertEquals("configFileProvider", coreWrapperStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(ConfigFileBuildWrapper.class).getDisplayName() + " : Start", coreWrapperStepNodes.get(0).getDisplayName());
r.assertLogContains("[Pipeline] junit", b);
r.assertLogContains("[Pipeline] configFileProvider", b);
r.assertLogContains("[Pipeline] // configFileProvider", b);
}

@Test public void metastepConsoleRaw() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
"node {\n" +
" wrap(new org.jenkinsci.plugins.configfiles.buildwrapper.ConfigFileBuildWrapper([])) {\n" +
" writeFile text: '''<testsuite name='a'><testcase name='c'><error>failed</error></testcase></testsuite>''', file: 'x.xml'\n" +
" step(new hudson.tasks.junit.JUnitResultArchiver('x.xml'))\n" +
" }\n" +
"}", false));
WorkflowRun b = r.assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0));
List<FlowNode> coreStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("step"));
assertThat(coreStepNodes, hasSize(1));
assertEquals("junit", coreStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(JUnitResultArchiver.class).getDisplayName(), coreStepNodes.get(0).getDisplayName());
List<FlowNode> coreWrapperStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), Predicates.and(new NodeStepTypePredicate("wrap"), new Predicate<FlowNode>() {
@Override public boolean apply(FlowNode n) {
return n instanceof StepStartNode && !((StepStartNode) n).isBody();
}
}));
assertThat(coreWrapperStepNodes, hasSize(1));
assertEquals("configFileProvider", coreWrapperStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(ConfigFileBuildWrapper.class).getDisplayName() + " : Start", coreWrapperStepNodes.get(0).getDisplayName());
r.assertLogContains("[Pipeline] junit", b);
r.assertLogContains("[Pipeline] configFileProvider", b);
r.assertLogContains("[Pipeline] // configFileProvider", b);
}

@Ignore("TODO ArgumentsAction.getResolvedArguments does not yet handle NotStoredReason sensibly")
@Test public void metastepConsoleNotStoredArgument() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
String spaces = StringUtils.repeat(" ", 1025); // cf. ArgumentsAction.MAX_RETAINED_LENGTH
p.setDefinition(new CpsFlowDefinition(
"node {\n" +
" configFileProvider([]) {\n" +
" writeFile text: '''<testsuite name='a'><testcase name='c'><error>failed</error></testcase></testsuite>''', file: 'x.xml'\n" +
" junit 'x.xml," + spaces + "'\n" +
" }\n" +
"}", true));
WorkflowRun b = r.assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0));
List<FlowNode> coreStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("step"));
assertThat(coreStepNodes, hasSize(1));
assertEquals("junit", coreStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(JUnitResultArchiver.class).getDisplayName(), coreStepNodes.get(0).getDisplayName());
List<FlowNode> coreWrapperStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), Predicates.and(new NodeStepTypePredicate("wrap"), new Predicate<FlowNode>() {
@Override public boolean apply(FlowNode n) {
return n instanceof StepStartNode && !((StepStartNode) n).isBody();
}
}));
assertThat(coreWrapperStepNodes, hasSize(1));
assertEquals("configFileProvider", coreWrapperStepNodes.get(0).getDisplayFunctionName());
assertEquals(r.jenkins.getDescriptor(ConfigFileBuildWrapper.class).getDisplayName() + " : Start", coreWrapperStepNodes.get(0).getDisplayName());
r.assertLogContains("[Pipeline] junit", b);
r.assertLogContains("[Pipeline] configFileProvider", b);
r.assertLogContains("[Pipeline] // configFileProvider", b);
}

}

0 comments on commit 0a0e050

Please sign in to comment.