Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop using Guice to inject InputStep & context parameters #69

Merged
merged 6 commits into from
Jan 19, 2022
Merged

Stop using Guice to inject InputStep & context parameters #69

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2d830f2
Stop using Guice to inject `InputStep` & context parameters
jglick Jan 3, 2022
e04bbbb
Got an incremental deployment
jglick Jan 3, 2022
0cf9ca2
Merge branch 'master' of https://github.com/jenkinsci/pipeline-input-…
jglick Jan 4, 2022
e9b5c5c
https://github.com/jenkinsci/workflow-step-api-plugin/pull/81 released
jglick Jan 4, 2022
f3b863d
Added `InputStepTest.serialForm` passing against `master`
jglick Jan 4, 2022
a060d36
Merge commit 'f3b863df05880c48407c9aa2eaa16fc7ea299738' into guice
jglick Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<version>999999-SNAPSHOT</version> <!-- TODO https://github.com/jenkinsci/workflow-step-api-plugin/pull/81 -->
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down
24 changes: 18 additions & 6 deletions src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import hudson.Util;
import hudson.model.ParameterDefinition;
import hudson.model.PasswordParameterDefinition;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.util.Secret;
import java.io.Serializable;
import java.util.Collections;
Expand All @@ -20,9 +22,12 @@
import org.jenkinsci.plugins.structs.describable.CustomDescribableModel;
import org.jenkinsci.plugins.structs.describable.DescribableModel;
import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable;
import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
import org.jenkinsci.plugins.workflow.steps.Step;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
import org.jenkinsci.plugins.workflow.steps.StepExecution;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

Expand Down Expand Up @@ -62,6 +67,7 @@ public class InputStep extends AbstractStepImpl implements Serializable {

@DataBoundConstructor
public InputStep(String message) {
super(true);
if (message==null)
message = "Pipeline has paused and needs your input before proceeding";
this.message = message;
Expand Down Expand Up @@ -151,18 +157,18 @@ public boolean canSettle(Authentication a) {
return false;
}

@Override public StepExecution start(StepContext context) throws Exception {
return new InputStepExecution(this, context);
}


@Override
public DescriptorImpl getDescriptor() {
return (DescriptorImpl)super.getDescriptor();
}

@Extension
public static class DescriptorImpl extends AbstractStepDescriptorImpl implements CustomDescribableModel {

public DescriptorImpl() {
super(InputStepExecution.class);
}
public static class DescriptorImpl extends StepDescriptor implements CustomDescribableModel {

@Override
public String getFunctionName() {
Expand All @@ -174,6 +180,12 @@ public String getDisplayName() {
return Messages.wait_for_interactive_input();
}

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

/**
* Compatibility hack for JENKINS-63516.
*/
Expand Down
76 changes: 45 additions & 31 deletions src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStepExecution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.cloudbees.plugins.credentials.CredentialsParameterValue;
import com.cloudbees.plugins.credentials.builds.CredentialsParameterBinder;
import com.google.inject.Inject;
import hudson.FilePath;
import hudson.Util;
import hudson.console.HyperlinkNote;
Expand All @@ -19,7 +18,6 @@
import hudson.security.ACL;
import hudson.security.ACLContext;
import hudson.security.SecurityRealm;
import hudson.security.Permission;
import hudson.util.HttpResponses;
import jenkins.model.IdStrategy;
import jenkins.model.Jenkins;
Expand All @@ -31,7 +29,6 @@
import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
import org.jenkinsci.plugins.workflow.support.actions.PauseAction;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.interceptor.RequirePOST;
Expand All @@ -50,6 +47,7 @@
import java.util.logging.Logger;
import jenkins.util.Timer;
import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException;
import org.jenkinsci.plugins.workflow.steps.StepContext;

/**
* @author Kohsuke Kawaguchi
Expand All @@ -58,21 +56,24 @@ public class InputStepExecution extends AbstractStepExecutionImpl implements Mod

private static final Logger LOGGER = Logger.getLogger(InputStepExecution.class.getName());

@StepContextParameter private transient Run run;

@StepContextParameter private transient TaskListener listener;

@StepContextParameter private transient FlowNode node;

/**
* Result of the input.
*/
private Outcome outcome;

@Inject(optional=true) InputStep input;
final InputStep input;

InputStepExecution(InputStep input, StepContext context) {
super(context);
this.input = input;
}

@Override
public boolean start() throws Exception {
Run<?, ?> run = getRun();
TaskListener listener = getListener();
FlowNode node = getNode();

// record this input
getPauseAction().add(this);

Expand Down Expand Up @@ -101,6 +102,8 @@ public void stop(Throwable cause) throws Exception {
@Override public void run() {
try (ACLContext context = ACL.as(ACL.SYSTEM)) {
doAbort();
} catch (IOException | InterruptedException x) {
LOGGER.log(Level.WARNING, "failed to abort " + getContext(), x);
}
}
});
Expand All @@ -114,8 +117,16 @@ public InputStep getInput() {
return input;
}

public Run getRun() {
return run;
public Run<?, ?> getRun() throws IOException, InterruptedException {
return getContext().get(Run.class);
}

private FlowNode getNode() throws InterruptedException, IOException {
return getContext().get(FlowNode.class);
}

private TaskListener getListener() throws IOException, InterruptedException {
return getContext().get(TaskListener.class);
}

/**
Expand All @@ -128,7 +139,8 @@ public boolean isSettled() {
/**
* Gets the {@link InputAction} that this step should be attached to.
*/
private InputAction getPauseAction() {
private InputAction getPauseAction() throws IOException, InterruptedException {
Run<?, ?> run = getRun();
InputAction a = run.getAction(InputAction.class);
if (a==null)
run.addAction(a=new InputAction());
Expand Down Expand Up @@ -175,15 +187,15 @@ public HttpResponse doProceed(StaplerRequest request) throws IOException, Servle
* @param params A map that represents the parameters sent in the request
* @return A HttpResponse object that represents Status code (200) indicating the request succeeded normally.
*/
public HttpResponse proceed(@CheckForNull Map<String,Object> params) {
public HttpResponse proceed(@CheckForNull Map<String,Object> params) throws IOException, InterruptedException {
User user = User.current();
String approverId = null;
if (user != null){
approverId = user.getId();
run.addAction(new ApproverAction(approverId));
listener.getLogger().println("Approved by " + hudson.console.ModelHyperlinkNote.encodeTo(user));
getRun().addAction(new ApproverAction(approverId));
getListener().getLogger().println("Approved by " + hudson.console.ModelHyperlinkNote.encodeTo(user));
}
node.addAction(new InputSubmittedAction(approverId, params));
getNode().addAction(new InputSubmittedAction(approverId, params));

Object v;
if (params != null && params.size() == 1) {
Expand All @@ -200,7 +212,7 @@ public HttpResponse proceed(@CheckForNull Map<String,Object> params) {

@Deprecated
@SuppressWarnings("unchecked")
public HttpResponse proceed(Object v) {
public HttpResponse proceed(Object v) throws IOException, InterruptedException {
if (v instanceof Map) {
return proceed(new HashMap<String,Object>((Map) v));
} else if (v == null) {
Expand All @@ -214,7 +226,7 @@ public HttpResponse proceed(Object v) {
* Used from the Proceed hyperlink when no parameters are defined.
*/
@RequirePOST
public HttpResponse doProceedEmpty() throws IOException {
public HttpResponse doProceedEmpty() throws IOException, InterruptedException {
preSubmissionCheck();

return proceed(null);
Expand All @@ -224,7 +236,7 @@ public HttpResponse doProceedEmpty() throws IOException {
* REST endpoint to abort the workflow.
*/
@RequirePOST
public HttpResponse doAbort() {
public HttpResponse doAbort() throws IOException, InterruptedException {
preAbortCheck();

FlowInterruptedException e = new FlowInterruptedException(Result.ABORTED, new Rejection(User.current()));
Expand All @@ -240,7 +252,7 @@ public HttpResponse doAbort() {
/**
* Check if the current user can abort/cancel the run from the input.
*/
private void preAbortCheck() {
private void preAbortCheck() throws IOException, InterruptedException {
if (isSettled()) {
throw new Failure("This input has been already given");
} if (!canCancel() && !canSubmit()) {
Expand All @@ -255,7 +267,7 @@ private void preAbortCheck() {
/**
* Check if the current user can submit the input.
*/
public void preSubmissionCheck() {
public void preSubmissionCheck() throws IOException, InterruptedException {
if (isSettled())
throw new Failure("This input has been already given");
if (!canSubmit()) {
Expand All @@ -267,38 +279,39 @@ public void preSubmissionCheck() {
}
}

private void postSettlement() {
private void postSettlement() throws IOException, InterruptedException {
try {
getPauseAction().remove(this);
run.save();
getRun().save();
} catch (IOException | InterruptedException | TimeoutException x) {
LOGGER.log(Level.WARNING, "failed to remove InputAction from " + run, x);
LOGGER.log(Level.WARNING, "failed to remove InputAction from " + getContext(), x);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, but here and elsewhere where you call getContext() did you mean getRun()? That would make more sense to me as a direct translation of the old run variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I see what you mean about GitHub not letting you comment on a single line without leaving a "review". I clicked "leave a single comment", yet now it appears as "reviewed 18 minutes ago.")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getRun() would be the more direct equivalent, but might throw exceptions. getContext() is good enough for logging purposes as its toString() will include an identification of the build: https://github.com/jenkinsci/workflow-cps-plugin/blob/a9433432b33c65f58d9e7b4dc4aa83249672eaf7/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsStepContext.java#L596-L605 + https://github.com/jenkinsci/workflow-job-plugin/blob/969637446d3ba7ea2c1b26cb7ee1b9c44c6e4ebc/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java#L1003-L1005

Copy link
Member

@basil basil Jan 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, sounds good then.

} finally {
FlowNode node = getNode();
if (node != null) {
try {
PauseAction.endCurrentPause(node);
} catch (IOException x) {
LOGGER.log(Level.WARNING, "failed to end PauseAction in " + run, x);
LOGGER.log(Level.WARNING, "failed to end PauseAction in " + getContext(), x);
}
} else {
LOGGER.log(Level.WARNING, "cannot set pause end time for {0} in {1}", new Object[] {getId(), run});
LOGGER.log(Level.WARNING, "cannot set pause end time for {0} in {1}", new Object[] {getId(), getContext()});
}
}
}

private boolean canCancel() {
private boolean canCancel() throws IOException, InterruptedException {
return !Jenkins.get().isUseSecurity() || getRun().getParent().hasPermission(Job.CANCEL);
}

private boolean canSubmit() {
private boolean canSubmit() throws IOException, InterruptedException {
Authentication a = Jenkins.getAuthentication();
return canSettle(a);
}

/**
* Checks if the given user can settle this input.
*/
private boolean canSettle(Authentication a) {
private boolean canSettle(Authentication a) throws IOException, InterruptedException {
String submitter = input.getSubmitter();
if (submitter==null)
return getRun().getParent().hasPermission(Job.BUILD);
Expand Down Expand Up @@ -369,6 +382,7 @@ private Map<String,Object> parseValue(StaplerRequest request) throws ServletExce
}
}

Run<?, ?> run = getRun();
CredentialsParameterBinder binder = CredentialsParameterBinder.getOrCreate(run);
String userId = Jenkins.getAuthentication().getName();
for (ParameterValue val : vals) {
Expand All @@ -394,7 +408,7 @@ private Map<String,Object> parseValue(StaplerRequest request) throws ServletExce
private Object convert(String name, ParameterValue v) throws IOException, InterruptedException {
if (v instanceof FileParameterValue) {
FileParameterValue fv = (FileParameterValue) v;
FilePath fp = new FilePath(run.getRootDir()).child(name);
FilePath fp = new FilePath(getRun().getRootDir()).child(name);
fp.copyFrom(fv.getFile());
return fp;
} else {
Expand Down