Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
89 changes: 46 additions & 43 deletions core/src/main/java/hudson/cli/CLICommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import hudson.Functions;
import hudson.cli.declarative.CLIMethod;
import hudson.cli.declarative.OptionHandlerExtension;
import hudson.cli.listeners.CliListener;
import hudson.remoting.Channel;
import hudson.security.SecurityRealm;
import java.io.BufferedInputStream;
Expand All @@ -49,8 +50,6 @@
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import jenkins.util.SystemProperties;
import org.jvnet.hudson.annotation_indexer.Index;
Expand Down Expand Up @@ -239,6 +238,9 @@
this.locale = locale;
CmdLineParser p = getCmdLineParser();

final String correlationId = UUID.randomUUID().toString();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: The introduction of correlationId common to all events could be highlighted in the PR description.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At this point correlationId is only used in BadCredentialsException handling. (where the original randomUUID comes form)

final int argsSize = args.size();

// add options from the authenticator
SecurityContext sc = null;
Authentication old = null;
Expand All @@ -253,56 +255,59 @@
if (!(this instanceof HelpCommand || this instanceof WhoAmICommand))
Jenkins.get().checkPermission(Jenkins.READ);
p.parseArgument(args.toArray(new String[0]));
LOGGER.log(Level.FINE, "Invoking CLI command {0}, with {1} arguments, as user {2}.",
new Object[] {getName(), args.size(), auth.getName()});

CliListener.fireExecution(correlationId, getName(), argsSize, auth);
int res = run();
LOGGER.log(Level.FINE, "Executed CLI command {0}, with {1} arguments, as user {2}, return code {3}",
new Object[] {getName(), args.size(), auth.getName(), res});
CliListener.fireCompleted(correlationId, getName(), argsSize, auth, res);

return res;
} catch (CmdLineException e) {
logFailedCommandAndPrintExceptionErrorMessage(args, e);
printUsage(stderr, p);
return 2;
} catch (IllegalStateException e) {
logFailedCommandAndPrintExceptionErrorMessage(args, e);
return 4;
} catch (IllegalArgumentException e) {
logFailedCommandAndPrintExceptionErrorMessage(args, e);
return 3;
} catch (AbortException e) {
logFailedCommandAndPrintExceptionErrorMessage(args, e);
return 5;
} catch (AccessDeniedException e) {
logFailedCommandAndPrintExceptionErrorMessage(args, e);
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();
logAndPrintError(e, "Bad Credentials. Search the server log for " + id + " for more details.",
"CLI login attempt failed: " + id, Level.INFO);
return 7;
} catch (Throwable e) {
String errorMsg = "Unexpected exception occurred while performing " + getName() + " command.";
logAndPrintError(e, errorMsg, errorMsg, Level.WARNING);
Functions.printStackTrace(e, stderr);
return 1;
int exitCode = handleException(e, correlationId, p);
CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), exitCode, e);
return exitCode;
} finally {
if (sc != null)
sc.setAuthentication(old); // restore
}
}

private void logFailedCommandAndPrintExceptionErrorMessage(List<String> args, Throwable e) {
Authentication auth = getTransportAuthentication2();
String logMessage = String.format("Failed call to CLI command %s, with %d arguments, as user %s.",
getName(), args.size(), auth != null ? auth.getName() : "<unknown>");

logAndPrintError(e, e.getMessage(), logMessage, Level.FINE);
/**
* Determines command stderr output and return the exit code as described on {@link #main(List, Locale, InputStream, PrintStream, PrintStream)}
* */
protected int handleException(Throwable e, String correlationId, CmdLineParser p) {
int exitCode;
if (e instanceof CmdLineException) {
exitCode = 2;
printError(e.getMessage());
printUsage(stderr, p);
} else if (e instanceof IllegalArgumentException) {
exitCode = 3;
printError(e.getMessage());
} else if (e instanceof IllegalStateException) {
exitCode = 4;
printError(e.getMessage());
} else if (e instanceof AbortException) {
exitCode = 5;
printError(e.getMessage());
} else if (e instanceof AccessDeniedException) {
exitCode = 6;
printError(e.getMessage());
} else if (e instanceof BadCredentialsException) {

Check warning on line 295 in core/src/main/java/hudson/cli/CLICommand.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 295 is only partially covered, one branch is missing
Copy link
Copy Markdown
Contributor Author

@apuig apuig Mar 11, 2025

Choose a reason for hiding this comment

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

Looks like BadCredentialsException is not longer possible inside CLICommand#main, note {set/get}TransportAuth2:

  • for HTTP or WebSocket the BadCredential is thrown/handled in the filters. When using command line -auth user:pass basic auth : BasicHeaderAuthenticator / AbstractUserAuth.

  • for SSH auth errors (not a BadCredentials) is also handled before executing the command (PublicKeyAuthenticatorImpl / UserAuthNamedFactory).

b1803a9 I can't find CliAuthenticationTest, may not apply anymore.

EDIT: this branch is only possible if a CLICommand implementation throws a BadCredentialsException,

exitCode = 7;
// 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
printError("Bad Credentials. Search the server log for " + correlationId + " for more details.");

Check warning on line 299 in core/src/main/java/hudson/cli/CLICommand.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 296-299 are not covered by tests
} else {
exitCode = 1;
printError("Unexpected exception occurred while performing " + getName() + " command.");
Functions.printStackTrace(e, stderr);
}
return exitCode;
}

private void logAndPrintError(Throwable e, String errorMessage, String logMessage, Level logLevel) {
LOGGER.log(logLevel, logMessage, e);


private void printError(String errorMessage) {
this.stderr.println();
this.stderr.println("ERROR: " + errorMessage);
}
Expand Down Expand Up @@ -538,8 +543,6 @@
return null;
}

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

private static final ThreadLocal<CLICommand> CURRENT_COMMAND = new ThreadLocal<>();

/*package*/ static CLICommand setCurrent(CLICommand cmd) {
Expand Down
59 changes: 12 additions & 47 deletions core/src/main/java/hudson/cli/declarative/CLIRegisterer.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@
import static java.util.logging.Level.SEVERE;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.AbortException;
import hudson.Extension;
import hudson.ExtensionComponent;
import hudson.ExtensionFinder;
import hudson.Functions;
import hudson.Util;
import hudson.cli.CLICommand;
import hudson.cli.CloneableCLICommand;
import hudson.cli.listeners.CliListener;
import hudson.model.Hudson;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -50,18 +49,14 @@
import java.util.MissingResourceException;
import java.util.Stack;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.ExtensionComponentSet;
import jenkins.ExtensionRefreshException;
import jenkins.model.Jenkins;
import org.jvnet.hudson.annotation_indexer.Index;
import org.jvnet.localizer.ResourceBundleHolder;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.ParserProperties;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
Expand Down Expand Up @@ -202,6 +197,9 @@

List<MethodBinder> binders = new ArrayList<>();

final String correlationId = UUID.randomUUID().toString();
final int argsSize = args.size();

CmdLineParser parser = bindMethod(binders);
try {
// TODO this could probably use ACL.as; why is it calling SecurityContext.setAuthentication rather than SecurityContextHolder.setContext?
Expand All @@ -215,15 +213,16 @@
sc.setAuthentication(auth); // run the CLI with the right credential
jenkins.checkPermission(Jenkins.READ);

CliListener.fireExecution(correlationId, getName(), argsSize, auth);

// resolve them
Object instance = null;
for (MethodBinder binder : binders)
instance = binder.call(instance);

if (instance instanceof Integer)
return (Integer) instance;
else
return 0;
Integer exitCode = (instance instanceof Integer) ? (Integer) instance : 0;

Check warning on line 223 in core/src/main/java/hudson/cli/declarative/CLIRegisterer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 223 is only partially covered, one branch is missing
CliListener.fireCompleted(correlationId, getName(), argsSize, auth, exitCode);
return exitCode;
} catch (InvocationTargetException e) {
Throwable t = e.getTargetException();
if (t instanceof Exception)
Expand All @@ -232,47 +231,13 @@
} finally {
sc.setAuthentication(old); // restore
}
} catch (CmdLineException e) {
printError(e.getMessage());
printUsage(stderr, parser);
return 2;
} catch (IllegalStateException e) {
printError(e.getMessage());
return 4;
} catch (IllegalArgumentException e) {
printError(e.getMessage());
return 3;
} catch (AbortException e) {
printError(e.getMessage());
return 5;
} catch (AccessDeniedException e) {
printError(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();
logAndPrintError(e, "Bad Credentials. Search the server log for " + id + " for more details.",
"CLI login attempt failed: " + id, Level.INFO);
return 7;
} catch (Throwable e) {
final String errorMsg = "Unexpected exception occurred while performing " + getName() + " command.";
logAndPrintError(e, errorMsg, errorMsg, Level.WARNING);
Functions.printStackTrace(e, stderr);
return 1;
int exitCode = handleException(e, correlationId, parser);
CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), exitCode, e);
return exitCode;
}
}

private void printError(String errorMessage) {
this.stderr.println();
this.stderr.println("ERROR: " + errorMessage);
}

private void logAndPrintError(Throwable e, String errorMessage, String logMessage, Level logLevel) {
LOGGER.log(logLevel, logMessage, e);
printError(errorMessage);
}

@Override
protected int run() throws Exception {
throw new UnsupportedOperationException();
Expand Down
145 changes: 145 additions & 0 deletions core/src/main/java/hudson/cli/listeners/CliListener.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/*
* The MIT License
*
* Copyright (c) 2025
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.cli.listeners;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.ExtensionPoint;
import hudson.cli.CLICommand;
import jenkins.util.Listeners;
import org.springframework.security.core.Authentication;

/**
* Callback around {@link CLICommand#run()}, each execution generates a new `correlationId` in order to group related events.
*
* @since TODO
*/
public interface CliListener extends ExtensionPoint {

/**
* Called before.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The command to be executed.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user performing the execution.
*
* */
default void onExecution(
@NonNull String correlationId, @NonNull String command, int argsSize, @CheckForNull Authentication auth) {}

/**
* Called after.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The executed command.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user executing the command.
* @param exitCode `run` returned exit code.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Zero, presumably?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implementations of CLICommand#run can return other values to indicate custom errors.

I try to avoid onSuccess because of this (but the javadoc comment still mention success :S), the idea is we call this if the command returned instead of throwing an exception.
I agree onCompleted vs onError its not clear enough, but can't find a better option (onException?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated javadoc and using onException

* */
default void onCompleted(
@NonNull String correlationId,
@NonNull String command,
int argsSize,
@CheckForNull Authentication auth,
int exitCode) {}

/**
* Catch exceptions.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The executed command.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user executing the command
* @param exitCode `run` returned exit code.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems unnecessary to pass this parameter to the listener. If anyone really cared it could be inferred by the same logic as in CLICommand but generally a listener is going to want to report any error at the Java level, not by using a Unix-like exit code value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ATM exit code is used to respect the current log levels. Notably unhanded exceptions and BadCredentials.
We can indeed check the same kind of Exceptions as in CLICommand#handleException, but exit codes handled by jenkins are properly documented on CLICommand#main, perhaps more maintainable ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

exit code removed in the signature with the exception

* @param t Any error during the execution of the command.
* */
default void onError(
@NonNull String correlationId,
@NonNull String command,
int argsSize,
@CheckForNull Authentication auth,
int exitCode,
@NonNull Throwable t) {}

Check warning on line 85 in core/src/main/java/hudson/cli/listeners/CliListener.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 51-85 are not covered by tests

/**
* Fires the {@link #onExecution} event.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The command to be executed.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user performing the execution.
*
* */
static void fireExecution(
@NonNull String correlationId, @NonNull String command, int argsSize, @CheckForNull Authentication auth) {
Listeners.notify(
CliListener.class, true, listener -> listener.onExecution(correlationId, command, argsSize, auth));
}

/**
* Fires the {@link #onCompleted} event.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The executed command.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user executing the command.
* @param exitCode `run` returned exit code.
* */
static void fireCompleted(
@NonNull String correlationId,
@NonNull String command,
int argsSize,
@CheckForNull Authentication auth,
int exitCode) {
Listeners.notify(
CliListener.class,
true,
listener -> listener.onCompleted(correlationId, command, argsSize, auth, exitCode));
}

/**
* Fires the {@link #onError} event.
*
* @param correlationId This value is used to correlate this command event to other, related command events.
* @param command The executed command.
* @param argsSize Number of arguments passed to the command.
* @param auth Authenticated user executing the command
* @param exitCode `run` returned exit code.
* @param t Any error during the execution of the command.
* */
static void fireError(
@NonNull String correlationId,
@NonNull String command,
int argsSize,
@CheckForNull Authentication auth,
int exitCode,
@NonNull Throwable t) {
Listeners.notify(
CliListener.class,
true,
listener -> listener.onError(correlationId, command, argsSize, auth, exitCode, t));
}
}
Loading
Loading