Skip to content

Commit

Permalink
[FIXED JENKINS-42999] Allow bindings to specify their required context
Browse files Browse the repository at this point in the history 
Not all bindings require a workspace, and those that don't should be
able to be used in `withCredentials` outside of a `node` block. This
adds `BindingDescriptor.getRequiredContext()`, defaulting to the four
contexts that were previously required by `BindingStep`, but allowing
override. `BindingStep` will now throw a
`MissingContextVariableException` if any of the `MultiBinding`s used
have a required context that cannot be satisfied, as well as the
normal potential `MissingContextVariableException` for `BindingStep.DescriptorImpl.getRequiredContext()`.

In addition, bumped `workflow-step-api` to 2.9 and moved `BindingStep`
and friends to non-deprecated code paths while I was here.
  • Loading branch information
@abayer
abayer committed Mar 21, 2017
1 parent 252b1d5 commit 44d70a8
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 23 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Expand Up @@ -61,7 +61,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<version>2.4</version>
<version>2.9</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/org/jenkinsci/plugins/credentialsbinding/BindingDescriptor.java
View file
Expand Up @@ -29,11 +29,21 @@
import com.cloudbees.plugins.credentials.common.AbstractIdCredentialsListBoxModel;
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.domains.DomainRequirement;
import hudson.FilePath;
import hudson.Launcher;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.security.ACL;
import hudson.util.ListBoxModel;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.kohsuke.stapler.AncestorInPath;

/**
Expand All @@ -44,6 +54,19 @@ public abstract class BindingDescriptor<C extends StandardCredentials> extends D

protected abstract Class<C> type();

/**
* Returns the context {@link MultiBinding} needs to access. Defaults to {@link Run}, {@link FilePath},
* {@link Launcher} and {@link TaskListener}.
*
* This allows the system to statically infer which steps are applicable in which context
* (say in freestyle or in workflow).
* @see StepContext#get(Class)
*/
public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<Class<?>>(Arrays.asList(Run.class, FilePath.class, Launcher.class,
TaskListener.class)));
}

public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item owner) {
if (owner == null || !owner.hasPermission(Item.CONFIGURE)) {
return new ListBoxModel();
Expand Down
68 changes: 50 additions & 18 deletions src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java
View file
Expand Up @@ -24,7 +24,6 @@

package org.jenkinsci.plugins.credentialsbinding.impl;

import com.google.inject.Inject;
import hudson.EnvVars;
import hudson.Extension;
import hudson.FilePath;
Expand All @@ -40,30 +39,36 @@
import java.io.OutputStream;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.codec.Charsets;
import org.jenkinsci.plugins.credentialsbinding.MultiBinding;
import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
import org.jenkinsci.plugins.workflow.steps.BodyInvoker;
import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
import org.jenkinsci.plugins.workflow.steps.MissingContextVariableException;
import org.jenkinsci.plugins.workflow.steps.Step;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
import org.jenkinsci.plugins.workflow.steps.StepExecution;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;

/**
* Workflow step to bind credentials.
*/
@SuppressWarnings("rawtypes") // TODO DescribableHelper does not yet seem to handle List<? extends MultiBinding<?>> or even List<MultiBinding<?>>
public final class BindingStep extends AbstractStepImpl {
public final class BindingStep extends Step {

private final List<MultiBinding> bindings;

Expand All @@ -75,20 +80,41 @@ public List<MultiBinding> getBindings() {
return bindings;
}

public static final class Execution extends AbstractStepExecutionImpl {
@Override
public StepExecution start(StepContext context) throws Exception {
return new Execution(this, context);
}

public static final class Execution extends StepExecution {

private static final long serialVersionUID = 1;

@Inject(optional=true) private transient BindingStep step;
@StepContextParameter private transient Run<?,?> run;
@StepContextParameter private transient FilePath workspace;
@StepContextParameter private transient Launcher launcher;
@StepContextParameter private transient TaskListener listener;
private transient BindingStep step;

public Execution(@Nonnull BindingStep step, StepContext context) {
super(context);
this.step = step;
}

@Override public boolean start() throws Exception {
Run<?,?> run = getContext().get(Run.class);
if (run == null) {
throw new MissingContextVariableException(Run.class);
}

FilePath workspace = getContext().get(FilePath.class);
Launcher launcher = getContext().get(Launcher.class);
TaskListener listener = getContext().get(TaskListener.class);

Map<String,String> overrides = new HashMap<String,String>();
List<MultiBinding.Unbinder> unbinders = new ArrayList<MultiBinding.Unbinder>();
for (MultiBinding<?> binding : step.bindings) {
for (Class<?> requiredContext : binding.getDescriptor().getRequiredContext()) {
Object v = getContext().get(requiredContext);
if (v == null) {
throw new MissingContextVariableException(requiredContext);
}
}
MultiBinding.MultiEnvironment environment = binding.bind(run, workspace, launcher, listener);
unbinders.add(environment.getUnbinder());
overrides.putAll(environment.getValues());
Expand Down Expand Up @@ -177,9 +203,14 @@ private static final class Callback extends BodyExecutionCallback.TailCall {

@Override protected void finished(StepContext context) throws Exception {
Exception xx = null;
Run<?,?> run = context.get(Run.class);
if (run == null) {
throw new MissingContextVariableException(Run.class);
}

for (MultiBinding.Unbinder unbinder : unbinders) {
try {
unbinder.unbind(context.get(Run.class), context.get(FilePath.class), context.get(Launcher.class), context.get(TaskListener.class));
unbinder.unbind(run, context.get(FilePath.class), context.get(Launcher.class), context.get(TaskListener.class));
} catch (Exception x) {
if (xx == null) {
xx = x;
Expand All @@ -195,11 +226,7 @@ private static final class Callback extends BodyExecutionCallback.TailCall {

}

@Extension public static final class DescriptorImpl extends AbstractStepDescriptorImpl {

public DescriptorImpl() {
super(Execution.class);
}
@Extension public static final class DescriptorImpl extends StepDescriptor {

@Override public String getFunctionName() {
return "withCredentials";
Expand All @@ -213,6 +240,11 @@ public DescriptorImpl() {
return true;
}

@Override
public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(TaskListener.class, Run.class)));
}

}

}
4 changes: 3 additions & 1 deletion src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding.java
View file
Expand Up @@ -39,6 +39,8 @@
import org.jenkinsci.plugins.plaincredentials.FileCredentials;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;

public class FileBinding extends Binding<FileCredentials> {

@DataBoundConstructor public FileBinding(String variable, String credentialsId) {
Expand Down Expand Up @@ -79,7 +81,7 @@ private static class UnbinderImpl implements Unbinder {
this.dirName = dirName;
}

@Override public void unbind(Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
@Override public void unbind(@Nonnull Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
secretsDir(workspace).child(dirName).deleteRecursive();
}

Expand Down
13 changes: 12 additions & 1 deletion src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding.java
View file
Expand Up @@ -30,13 +30,20 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.jenkinsci.Symbol;

import org.jenkinsci.plugins.credentialsbinding.Binding;
import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;

public class StringBinding extends Binding<StringCredentials> {

@DataBoundConstructor public StringBinding(String variable, String credentialsId) {
Expand All @@ -47,13 +54,17 @@ public class StringBinding extends Binding<StringCredentials> {
return StringCredentials.class;
}

@Override public SingleEnvironment bindSingle(Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
@Override public SingleEnvironment bindSingle(@Nonnull Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
return new SingleEnvironment(getCredentials(build).getSecret().getPlainText());
}

@Symbol("string")
@Extension public static class DescriptorImpl extends BindingDescriptor<StringCredentials> {

@Override public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, TaskListener.class)));
}

@Override protected Class<StringCredentials> type() {
return StringCredentials.class;
}
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding.java
View file
Expand Up @@ -31,12 +31,19 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.jenkinsci.Symbol;

import org.jenkinsci.plugins.credentialsbinding.Binding;
import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;

public class UsernamePasswordBinding extends Binding<StandardUsernamePasswordCredentials> {

@DataBoundConstructor public UsernamePasswordBinding(String variable, String credentialsId) {
Expand All @@ -47,7 +54,7 @@ public class UsernamePasswordBinding extends Binding<StandardUsernamePasswordCre
return StandardUsernamePasswordCredentials.class;
}

@Override public SingleEnvironment bindSingle(Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
@Override public SingleEnvironment bindSingle(@Nonnull Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
StandardUsernamePasswordCredentials credentials = getCredentials(build);
return new SingleEnvironment(credentials.getUsername() + ':' + credentials.getPassword().getPlainText());
}
Expand All @@ -63,6 +70,9 @@ public class UsernamePasswordBinding extends Binding<StandardUsernamePasswordCre
return Messages.UsernamePasswordBinding_username_and_password();
}

@Override public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, TaskListener.class)));
}
}

}
8 changes: 7 additions & 1 deletion ...main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBinding.java
View file
Expand Up @@ -32,6 +32,7 @@
import hudson.model.TaskListener;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand All @@ -42,6 +43,8 @@
import org.jenkinsci.plugins.credentialsbinding.MultiBinding;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;

public class UsernamePasswordMultiBinding extends MultiBinding<StandardUsernamePasswordCredentials> {

private final String usernameVariable;
Expand All @@ -65,7 +68,7 @@ public String getPasswordVariable() {
return StandardUsernamePasswordCredentials.class;
}

@Override public MultiEnvironment bind(Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
@Override public MultiEnvironment bind(@Nonnull Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
StandardUsernamePasswordCredentials credentials = getCredentials(build);
Map<String,String> m = new HashMap<String,String>();
m.put(usernameVariable, credentials.getUsername());
Expand All @@ -88,6 +91,9 @@ public String getPasswordVariable() {
return Messages.UsernamePasswordMultiBinding_username_and_password();
}

@Override public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, TaskListener.class)));
}
}

}
69 changes: 69 additions & 0 deletions src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java
View file
Expand Up @@ -165,6 +165,75 @@ public static class Execution extends AbstractSynchronousStepExecution<Void> {
});
}

@Issue("JENKINS-42999")
@Test
public void limitedRequiredContext() throws Exception {
final String credentialsId = "creds";
final String username = "bob";
final String password = "s3cr3t";
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
UsernamePasswordCredentialsImpl c = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, credentialsId, "sample", username, password);
CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c);
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(""
+ "withCredentials([usernamePassword(usernameVariable: 'USERNAME', passwordVariable: 'PASSWORD', credentialsId: '" + credentialsId + "')]) {\n"
+ " node {\n" // We still need to enter a node to get the actual creds via the file.
+ " semaphore 'basics'\n"
+ " sh '''\n"
+ " set +x\n"
+ " echo curl -u $USERNAME:$PASSWORD server > script.sh\n"
+ " '''\n"
+ " }\n"
+ "}", true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
SemaphoreStep.waitForStart("basics/1", b);
}
});
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
WorkflowJob p = story.j.jenkins.getItemByFullName("p", WorkflowJob.class);
assertNotNull(p);
WorkflowRun b = p.getBuildByNumber(1);
assertNotNull(b);
assertEquals(Collections.<String>emptySet(), grep(b.getRootDir(), password));
SemaphoreStep.success("basics/1", null);
story.j.waitForCompletion(b);
story.j.assertBuildStatusSuccess(b);
story.j.assertLogNotContains(password, b);
FilePath script = story.j.jenkins.getWorkspaceFor(p).child("script.sh");
assertTrue(script.exists());
assertEquals("curl -u " + username + ":" + password + " server", script.readToString().trim());
assertEquals(Collections.<String>emptySet(), grep(b.getRootDir(), password));
}
});
}

@Issue("JENKINS-42999")
@Test
public void widerRequiredContext() throws Exception {
final String credentialsId = "creds";
final String credsFile = "credsFile";
final String credsContent = "s3cr3t";
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
FileCredentialsImpl c = new FileCredentialsImpl(CredentialsScope.GLOBAL, credentialsId, "sample", credsFile, SecretBytes.fromBytes(credsContent.getBytes()));
CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c);
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(""
+ "withCredentials([file(variable: 'targetFile', credentialsId: '" + credentialsId + "')]) {\n"
+ " echo 'We should fail before getting here'\n"
+ "}", true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
story.j.assertBuildStatus(Result.FAILURE, story.j.waitForCompletion(b));
story.j.assertLogNotContains("We should fail before getting here", b);
// We can't guarantee whether this will fail due to missing FilePath.class or Launcher.class.
// So look for the follow-up error instead.
story.j.assertLogContains("Perhaps you forgot to surround the code with a step that provides this, such as: node", b);
}
});
}

@Inject
StringCredentialsImpl.DescriptorImpl stringCredentialsDescriptor;

Expand Down

0 comments on commit 44d70a8

Please sign in to comment.