Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-32273] Unified an issue handling in CLI
Re-factored all existed CLI code to follow the new proposed scheme for
raising an exception if issue occurs, handling and reporting it.
Unified CLIRegisterer as well.
Fixed unit tests to follow the new scheme.
  • Loading branch information
pjanouse committed Feb 12, 2016
1 parent 49d9f0e commit d5725e6
Show file tree
Hide file tree
Showing 51 changed files with 739 additions and 380 deletions.
6 changes: 2 additions & 4 deletions core/src/main/java/hudson/cli/AddJobToViewCommand.java
Expand Up @@ -31,7 +31,6 @@
import hudson.model.View;

import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;

/**
* @author ogondza
Expand All @@ -55,9 +54,8 @@ public String getShortDescription() {
protected int run() throws Exception {
view.checkPermission(View.CONFIGURE);

if (!(view instanceof DirectlyModifiableView)) throw new CmdLineException(
null, "'" + view.getDisplayName() + "' view can not be modified directly"
);
if (!(view instanceof DirectlyModifiableView)) throw new IllegalStateException(
"'" + view.getDisplayName() + "' view can not be modified directly");

for (TopLevelItem job: jobs) {
((DirectlyModifiableView) view).add(job);
Expand Down
33 changes: 19 additions & 14 deletions core/src/main/java/hudson/cli/BuildCommand.java
Expand Up @@ -42,6 +42,7 @@
import hudson.util.EditDistance;
import hudson.util.StreamTaskListener;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.Option;

import java.util.Map;
Expand Down Expand Up @@ -90,7 +91,7 @@ public String getShortDescription() {
@Option(name="-r") @Deprecated
public int retryCnt = 10;

protected static final String BUILD_SCHEDULING_REFUSED = "Build scheduling Refused by an extension, hence not in Queue";
protected static final String BUILD_SCHEDULING_REFUSED = "Build scheduling Refused by an extension, hence not in Queue.";

protected int run() throws Exception {
job.checkPermission(Item.BUILD);
Expand All @@ -99,7 +100,7 @@ protected int run() throws Exception {
if (!parameters.isEmpty()) {
ParametersDefinitionProperty pdp = job.getProperty(ParametersDefinitionProperty.class);
if (pdp==null)
throw new AbortException(job.getFullDisplayName()+" is not parameterized but the -p option was specified");
throw new IllegalStateException(job.getFullDisplayName()+" is not parameterized but the -p option was specified.");

//TODO: switch to type annotations after the migration to Java 1.8
List<ParameterValue> values = new ArrayList<ParameterValue>();
Expand All @@ -108,12 +109,14 @@ protected int run() throws Exception {
String name = e.getKey();
ParameterDefinition pd = pdp.getParameterDefinition(name);
if (pd==null) {
throw new AbortException(String.format("\'%s\' is not a valid parameter. Did you mean %s?",
name, EditDistance.findNearest(name, pdp.getParameterDefinitionNames())));
String nearest = EditDistance.findNearest(name, pdp.getParameterDefinitionNames());
throw new CmdLineException(null, nearest == null ?
String.format("'%s' is not a valid parameter.", name) :
String.format("'%s' is not a valid parameter. Did you mean %s?", name, nearest));
}
ParameterValue val = pd.createValue(this, Util.fixNull(e.getValue()));
if (val == null) {
throw new AbortException(String.format("Cannot resolve the value for the parameter \'%s\'.",name));
throw new CmdLineException(null, String.format("Cannot resolve the value for the parameter '%s'.",name));
}
values.add(val);
}
Expand All @@ -126,7 +129,7 @@ protected int run() throws Exception {
// not passed in use default
ParameterValue defaultValue = pd.getDefaultParameterValue();
if (defaultValue == null) {
throw new AbortException(String.format("No default value for the parameter \'%s\'.",pd.getName()));
throw new CmdLineException(null, String.format("No default value for the parameter '%s'.",pd.getName()));
}
values.add(defaultValue);
}
Expand All @@ -147,16 +150,14 @@ protected int run() throws Exception {
} else if (job.isHoldOffBuildUntilSave()){
msg = Messages.BuildCommand_CLICause_CannotBuildConfigNotSaved(job.getFullDisplayName());
}
stderr.println(msg);
return -1;
throw new IllegalStateException(msg);
}

QueueTaskFuture<? extends AbstractBuild> f = job.scheduleBuild2(0, new CLICause(Jenkins.getAuthentication().getName()), a);

if (wait || sync || follow) {
if (f == null) {
stderr.println(BUILD_SCHEDULING_REFUSED);
return -1;
throw new IllegalStateException(BUILD_SCHEDULING_REFUSED);
}
AbstractBuild b = f.waitForStart(); // wait for the start
stdout.println("Started "+b.getFullDisplayName());
Expand All @@ -177,7 +178,9 @@ protected int run() throws Exception {
}
catch (FileNotFoundException e) {
if ( i == retryCnt ) {
throw e;
Exception myException = new AbortException();
myException.initCause(e);
throw myException;
}
i++;
Thread.sleep(retryInterval);
Expand All @@ -193,7 +196,9 @@ protected int run() throws Exception {
} else {
// if the CLI is aborted, try to abort the build as well
f.cancel(true);
throw e;
Exception myException = new AbortException();
myException.initCause(e);
throw myException;
}
}
}
Expand All @@ -214,9 +219,9 @@ protected void printUsageSummary(PrintStream stderr) {
"With the -f option, this command changes the exit code based on\n" +
"the outcome of the build (exit code 0 indicates a success)\n" +
"however, unlike -s, interrupting the command will not interrupt\n" +
"the job (exit code 125 indicates the command was interrupted)\n" +
"the job (exit code 125 indicates the command was interrupted).\n" +
"With the -c option, a build will only run if there has been\n" +
"an SCM change"
"an SCM change."
);
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/cli/CLIAction.java
Expand Up @@ -71,7 +71,7 @@ public String getUrlName() {
}

public void doCommand(StaplerRequest req, StaplerResponse rsp) throws ServletException, IOException {
final Jenkins jenkins = Jenkins.getInstance();
final Jenkins jenkins = Jenkins.getActiveInstance();
jenkins.checkPermission(Jenkins.READ);

// Strip trailing slash
Expand Down Expand Up @@ -112,7 +112,7 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod

FullDuplexHttpChannel server;
if(req.getHeader("Side").equals("download")) {
duplexChannels.put(uuid,server=new FullDuplexHttpChannel(uuid, !Jenkins.getInstance().hasPermission(Jenkins.ADMINISTER)) {
duplexChannels.put(uuid,server=new FullDuplexHttpChannel(uuid, !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER)) {
@Override
protected void main(Channel channel) throws IOException, InterruptedException {
// capture the identity given by the transport, since this can be useful for SecurityRealm.createCliAuthenticator()
Expand Down
87 changes: 61 additions & 26 deletions core/src/main/java/hudson/cli/CLICommand.java
Expand Up @@ -195,7 +195,7 @@ public String getName() {
* to an empty method.
* You would however then have to consider {@link CliAuthenticator} and {@link #getTransportAuthentication},
* so this is not really recommended.
*
*
* @param args
* Arguments to the sub command. For example, if the CLI is invoked like "java -jar cli.jar foo bar zot",
* then "foo" is the sub-command and the argument list is ["bar","zot"].
Expand All @@ -209,7 +209,23 @@ public String getName() {
* @param stderr
* Connected to the stderr of the CLI client.
* @return
* Exit code from the command.
* Exit code from the CLI command execution
*
* <p>
* Jenkins standard exit codes from CLI:
* 0 means everything went well.
* 1 means further unspecified exception is thrown while performing the command.
* 2 means CmdLineException is thrown while performing the command.
* 3 means IllegalArgumentException is thrown while performing the command.
* 4 mean IllegalStateException is thrown while performing the command.
* 5 means AbortException is thrown while performing the command.
* 6 means AccessDeniedException is thrown while performing the command.
* 7 means BadCredentialsException is thrown while performing the command.
* 8-15 are reserved for future usage
* 16+ mean a custom CLI exit error code (meaning defined by the CLI command itself)
*
* <p>
* Note: For details - see JENKINS-32273
*/
public int main(List<String> args, Locale locale, InputStream stdin, PrintStream stdout, PrintStream stderr) {
this.stdin = new BufferedInputStream(stdin);
Expand All @@ -220,55 +236,65 @@ public int main(List<String> args, Locale locale, InputStream stdin, PrintStream
CmdLineParser p = getCmdLineParser();

// add options from the authenticator
SecurityContext sc = SecurityContextHolder.getContext();
Authentication old = sc.getAuthentication();
SecurityContext sc = null;
Authentication old = null;
try {
sc = SecurityContextHolder.getContext();
old = sc.getAuthentication();

CliAuthenticator authenticator = Jenkins.getInstance().getSecurityRealm().createCliAuthenticator(this);
sc.setAuthentication(getTransportAuthentication());
new ClassParser().parse(authenticator,p);
CliAuthenticator authenticator = Jenkins.getActiveInstance().getSecurityRealm().createCliAuthenticator(this);
sc.setAuthentication(getTransportAuthentication());
new ClassParser().parse(authenticator,p);

try {
p.parseArgument(args.toArray(new String[args.size()]));
Authentication auth = authenticator.authenticate();
if (auth==Jenkins.ANONYMOUS)
auth = loadStoredAuthentication();
sc.setAuthentication(auth); // run the CLI with the right credential
if (!(this instanceof LoginCommand || this instanceof HelpCommand))
Jenkins.getInstance().checkPermission(Jenkins.READ);
Jenkins.getActiveInstance().checkPermission(Jenkins.READ);
return run();
} catch (CmdLineException e) {
stderr.println("\nERROR: " + e.getMessage());
stderr.println("");
stderr.println("ERROR: " + e.getMessage());
printUsage(stderr, p);
return 2;
} catch (IllegalStateException e) {
stderr.println("\nERROR: " + e.getMessage());
stderr.println("");
stderr.println("ERROR: " + e.getMessage());
return 4;
} catch (IllegalArgumentException e) {
stderr.println("\nERROR: " + e.getMessage());
stderr.println("");
stderr.println("ERROR: " + e.getMessage());
return 3;
} catch (AbortException e) {
// signals an error without stack trace
stderr.println("\nERROR: " + e.getMessage());
stderr.println("");
stderr.println("ERROR: " + e.getMessage());
return 5;
} catch (AccessDeniedException e) {
stderr.println("\nERROR: " + e.getMessage());
stderr.println("");
stderr.println("ERROR: " + e.getMessage());
return 6;
} catch (BadCredentialsException e) {
// to the caller, we can't reveal whether the user didn't exist or the password didn't match.
// do that to the server log instead
String id = UUID.randomUUID().toString();
LOGGER.log(Level.INFO, "CLI login attempt failed: " + id, e);
stderr.println("\nERROR: Bad Credentials. Search the server log for " + id + " for more details.");
stderr.println("");
stderr.println("ERROR: Bad Credentials. Search the server log for " + id + " for more details.");
return 7;
} catch (Exception e) {
} catch (Throwable e) {
final String errorMsg = String.format("Unexpected exception occurred while performing %s command.",
getName());
stderr.println(errorMsg);
stderr.println("");
stderr.println("ERROR: " + errorMsg);
LOGGER.log(Level.WARNING, errorMsg, e);
e.printStackTrace(stderr);
return 1;
} finally {
sc.setAuthentication(old); // restore
if(sc != null)
sc.setAuthentication(old); // restore
}
}

Expand Down Expand Up @@ -357,13 +383,22 @@ public void setTransportAuth(Authentication transportAuth) {
* To execute CLI method from outside, use {@link #main(List, Locale, InputStream, PrintStream, PrintStream)}
*
* @return
* 0 to indicate a success, otherwise an error code.
* @throws AbortException
* If the processing should be aborted. Hudson will report the error message
* without stack trace, and then exits this command.
* 0 to indicate a success, otherwise a custom error code.
* Error codes 1-15 shouldn;t be used in {@link #run()} as a custom error code.
* @throws Exception
* All the other exceptions cause the stack trace to be dumped, and then
* the command exits with an error code.
* If a further unspecified exception is thrown; means: Unknown and/or unexpected issue occurred
* @throws CmdLineException
* If a wrong parameter specified, input value can't be decoded etc.
* @throws IllegalArgumentException
* If the execution can't continue due to wrong input parameter (job doesn't exist etc.)
* @throws IllegalStateException
* If the execution can't continue due to an incorect state of Jenkins, job, build etc.
* @throws AbortException
* If the execution can't continue due to an other (rare, but foreseeable) issue
* @throws AccessDeniedException
* If the caller doesn't have sufficent rights for requested action
* @throws BadCredentialsException
* If bad credentials were provided to CLI
*/
protected abstract int run() throws Exception;

Expand Down Expand Up @@ -500,7 +535,7 @@ protected CLICommand createClone() {
*/
protected void registerOptionHandlers() {
try {
for (Class c : Index.list(OptionHandlerExtension.class, Jenkins.getInstance().pluginManager.uberClassLoader,Class.class)) {
for (Class c : Index.list(OptionHandlerExtension.class, Jenkins.getActiveInstance().pluginManager.uberClassLoader,Class.class)) {
Type t = Types.getBaseClass(c, OptionHandler.class);
CmdLineParser.registerHandler(Types.erasure(Types.getTypeArgument(t,0)), c);
}
Expand Down Expand Up @@ -552,7 +587,7 @@ public static CLICommand getCurrent() {
static {
// register option handlers that are defined
ClassLoaders cls = new ClassLoaders();
Jenkins j = Jenkins.getInstance();
Jenkins j = Jenkins.getActiveInstance();
if (j!=null) {// only when running on the master
cls.put(j.getPluginManager().uberClassLoader);

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/cli/CliProtocol.java
Expand Up @@ -77,7 +77,7 @@ protected void runCli(Connection c) throws IOException, InterruptedException {
Channel channel = cb
.withMode(Mode.BINARY)
.withRestricted(true)
.withBaseLoader(Jenkins.getInstance().pluginManager.uberClassLoader)
.withBaseLoader(Jenkins.getActiveInstance().pluginManager.uberClassLoader)
.build(new BufferedInputStream(c.in), new BufferedOutputStream(c.out));

channel.setProperty(CliEntryPoint.class.getName(),new CliManagerImpl(channel));
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/cli/CliProtocol2.java
Expand Up @@ -60,7 +60,7 @@ public void run() throws IOException, InterruptedException {

try {
// HACK: TODO: move the transport support into modules
Class<?> cls = Jenkins.getInstance().pluginManager.uberClassLoader.loadClass("org.jenkinsci.main.modules.instance_identity.InstanceIdentity");
Class<?> cls = Jenkins.getActiveInstance().pluginManager.uberClassLoader.loadClass("org.jenkinsci.main.modules.instance_identity.InstanceIdentity");
Object iid = cls.getDeclaredMethod("get").invoke(null);
PrivateKey instanceId = (PrivateKey)cls.getDeclaredMethod("getPrivate").invoke(iid);

Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/hudson/cli/ClientAuthenticationCache.java
Expand Up @@ -66,7 +66,7 @@ public FilePath call() throws IOException {
* @return {@link jenkins.model.Jenkins#ANONYMOUS} if no such credential is found, or if the stored credential is invalid.
*/
public Authentication get() {
Jenkins h = Jenkins.getInstance();
Jenkins h = Jenkins.getActiveInstance();
Secret userName = Secret.decrypt(props.getProperty(getPropertyKey()));
if (userName==null) return Jenkins.ANONYMOUS; // failed to decrypt
try {
Expand All @@ -83,7 +83,7 @@ public Authentication get() {
* Computes the key that identifies this Hudson among other Hudsons that the user has a credential for.
*/
private String getPropertyKey() {
String url = Jenkins.getInstance().getRootUrl();
String url = Jenkins.getActiveInstance().getRootUrl();
if (url!=null) return url;
return Secret.fromString("key").toString();
}
Expand All @@ -92,7 +92,7 @@ private String getPropertyKey() {
* Persists the specified authentication.
*/
public void set(Authentication a) throws IOException, InterruptedException {
Jenkins h = Jenkins.getInstance();
Jenkins h = Jenkins.getActiveInstance();

// make sure that this security realm is capable of retrieving the authentication by name,
// as it's not required.
Expand Down

0 comments on commit d5725e6

Please sign in to comment.