undo removal of accessLogger and deal with some warnings (#10567)

* undo removval of accessLogger and deal with some warnings

* Apply suggestions from code review

* add accesslog to servlet
This commit is contained in:
dahn 2025-04-14 10:17:56 +02:00 committed by GitHub
parent 53700809ed
commit dd1c32cd26
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 65 additions and 77 deletions

View File

@ -193,6 +193,7 @@ import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordRe
@Component
public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable {
private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServer.class.getName());
private static final String SANITIZATION_REGEX = "[\n\r]";
@ -235,9 +236,9 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
private EventDistributor eventDistributor = null;
private static int s_workerCount = 0;
private static Map<String, List<Class<?>>> s_apiNameCmdClassMap = new HashMap<String, List<Class<?>>>();
private static Map<String, List<Class<?>>> s_apiNameCmdClassMap = new HashMap<>();
private static ExecutorService s_executor = new ThreadPoolExecutor(10, 150, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(), new NamedThreadFactory(
private static ExecutorService s_executor = new ThreadPoolExecutor(10, 150, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new NamedThreadFactory(
"ApiServer"));
@Inject
@ -353,7 +354,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
String jobEvent = eventInfo.second();
if (logger.isTraceEnabled())
logger.trace("Handle asyjob publish event " + jobEvent);
logger.trace("Handle asyjob publish event {}", jobEvent);
if (eventDistributor == null) {
setEventDistributor(ComponentContext.getComponent(EventDistributor.class));
}
@ -378,7 +379,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
cmdEventType = eventTypeObj;
if (logger.isDebugEnabled())
logger.debug("Retrieved cmdEventType from job info: " + cmdEventType);
logger.debug("Retrieved cmdEventType from job info: {}", cmdEventType);
} else {
if (logger.isDebugEnabled())
logger.debug("Unable to locate cmdEventType marker in job info. publish as unknown event");
@ -422,11 +423,11 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
protected void setupIntegrationPortListener(Integer apiPort) {
if (apiPort == null || apiPort <= 0) {
logger.trace(String.format("Skipping setting up listener for integration port as %s is set to %d",
IntegrationAPIPort.key(), apiPort));
logger.trace("Skipping setting up listener for integration port as {} is set to {}",
IntegrationAPIPort.key(), apiPort);
return;
}
logger.debug(String.format("Setting up integration API service listener on port: %d", apiPort));
logger.debug("Setting up integration API service listener on port: {}", apiPort);
final ListenerThread listenerThread = new ListenerThread(this, apiPort);
listenerThread.start();
}
@ -437,24 +438,24 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
Integer apiPort = IntegrationAPIPort.value(); // api port, null by default
final Long snapshotLimit = ConcurrentSnapshotsThresholdPerHost.value();
if (snapshotLimit == null || snapshotLimit.longValue() <= 0) {
if (snapshotLimit == null || snapshotLimit <= 0) {
logger.debug("Global concurrent snapshot config parameter " + ConcurrentSnapshotsThresholdPerHost.value() + " is less or equal 0; defaulting to unlimited");
} else {
dispatcher.setCreateSnapshotQueueSizeLimit(snapshotLimit);
}
final Long migrationLimit = VolumeApiService.ConcurrentMigrationsThresholdPerDatastore.value();
if (migrationLimit == null || migrationLimit.longValue() <= 0) {
if (migrationLimit == null || migrationLimit <= 0) {
logger.debug("Global concurrent migration config parameter " + VolumeApiService.ConcurrentMigrationsThresholdPerDatastore.value() + " is less or equal 0; defaulting to unlimited");
} else {
dispatcher.setMigrateQueueSizeLimit(migrationLimit);
}
final Set<Class<?>> cmdClasses = new HashSet<Class<?>>();
final Set<Class<?>> cmdClasses = new HashSet<>();
for (final PluggableService pluggableService : pluggableServices) {
cmdClasses.addAll(pluggableService.getCommands());
if (logger.isDebugEnabled()) {
logger.debug("Discovered plugin " + pluggableService.getClass().getSimpleName());
logger.debug("Discovered plugin {}", pluggableService.getClass().getSimpleName());
}
}
@ -467,7 +468,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
String apiName = at.name();
List<Class<?>> apiCmdList = s_apiNameCmdClassMap.get(apiName);
if (apiCmdList == null) {
apiCmdList = new ArrayList<Class<?>>();
apiCmdList = new ArrayList<>();
s_apiNameCmdClassMap.put(apiName, apiCmdList);
}
apiCmdList.add(cmdClass);
@ -570,14 +571,14 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
throw e;
}
} finally {
logger.info(sb.toString());
ACCESSLOGGER.info(sb.toString());
CallContext.unregister();
}
}
@SuppressWarnings({"unchecked", "rawtypes"})
public void checkCharacterInkParams(final Map params) {
final Map<String, String> stringMap = new HashMap<String, String>();
final Map<String, String> stringMap = new HashMap<>();
final Set keys = params.keySet();
final Iterator keysIter = keys.iterator();
while (keysIter.hasNext()) {
@ -600,7 +601,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
public String handleRequest(final Map params, final String responseType, final StringBuilder auditTrailSb) throws ServerApiException {
checkCharacterInkParams(params);
String response = null;
String response;
String[] command = null;
try {
@ -621,7 +622,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (authManager.getAPIAuthenticator(command[0]) != null) {
return null;
}
final Map<String, String> paramMap = new HashMap<String, String>();
final Map<String, String> paramMap = new HashMap<>();
final Set keys = params.keySet();
final Iterator keysIter = keys.iterator();
while (keysIter.hasNext()) {
@ -637,16 +638,16 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (cmdClass != null) {
APICommand annotation = cmdClass.getAnnotation(APICommand.class);
if (annotation == null) {
logger.error("No APICommand annotation found for class " + cmdClass.getCanonicalName());
logger.error("No APICommand annotation found for class {}", cmdClass.getCanonicalName());
throw new CloudRuntimeException("No APICommand annotation found for class " + cmdClass.getCanonicalName());
}
BaseCmd cmdObj = (BaseCmd)cmdClass.newInstance();
BaseCmd cmdObj = (BaseCmd)cmdClass.getDeclaredConstructor().newInstance();
cmdObj = ComponentContext.inject(cmdObj);
cmdObj.configure();
cmdObj.setFullUrlParams(paramMap);
cmdObj.setResponseType(responseType);
cmdObj.setHttpMethod(paramMap.get(ApiConstants.HTTPMETHOD).toString());
cmdObj.setHttpMethod(paramMap.get(ApiConstants.HTTPMETHOD));
// This is where the command is either serialized, or directly dispatched
StringBuilder log = new StringBuilder();
@ -655,14 +656,11 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
} else {
final String errorString = "Unknown API command: " + command[0];
logger.warn(errorString);
auditTrailSb.append(" " + errorString);
auditTrailSb.append(" ").append(errorString);
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, errorString);
}
}
} catch (final InvalidParameterValueException ex) {
logger.info(ex.getMessage());
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, ex.getMessage(), ex);
} catch (final IllegalArgumentException ex) {
} catch (final InvalidParameterValueException | IllegalArgumentException ex) {
logger.info(ex.getMessage());
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, ex.getMessage(), ex);
} catch (final PermissionDeniedException ex) {
@ -675,9 +673,9 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
buf.append(obj.getUuid());
buf.append(" ");
}
logger.info("PermissionDenied: " + ex.getMessage() + " on objs: [" + buf.toString() + "]");
logger.info("PermissionDenied: " + ex.getMessage() + " on objs: [" + buf + "]");
} else {
logger.info("PermissionDenied: " + ex.getMessage());
logger.info("PermissionDenied: {}", ex.getMessage());
}
throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, ex.getMessage(), ex);
} catch (final AccountLimitException ex) {
@ -747,7 +745,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
// BaseAsyncCmd: cmd is processed and submitted as an AsyncJob, job related info is serialized and returned.
if (cmdObj instanceof BaseAsyncCmd) {
Long objectId = null;
String objectUuid = null;
String objectUuid;
if (cmdObj instanceof BaseAsyncCreateCmd) {
final BaseAsyncCreateCmd createCmd = (BaseAsyncCreateCmd)cmdObj;
dispatcher.dispatchCreateCmd(createCmd, params);
@ -788,7 +786,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
}
params.put("ctxStartEventId", String.valueOf(startEventId));
params.put("cmdEventType", asyncCmd.getEventType().toString());
params.put("cmdEventType", asyncCmd.getEventType());
params.put("ctxDetails", ApiGsonHelper.getBuilder().create().toJson(ctx.getContextParameters()));
if (asyncCmd.getHttpMethod() != null) {
params.put(ApiConstants.HTTPMETHOD, asyncCmd.getHttpMethod().toString());
@ -851,9 +849,9 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
@SuppressWarnings("unchecked")
private void buildAsyncListResponse(final BaseListCmd command, final Account account) {
final List<ResponseObject> responses = ((ListResponse)command.getResponseObject()).getResponses();
if (responses != null && responses.size() > 0) {
List<? extends AsyncJob> jobs = null;
final List<ResponseObject> responses = ((ListResponse<ResponseObject>)command.getResponseObject()).getResponses();
if (responses != null && !responses.isEmpty()) {
List<? extends AsyncJob> jobs;
// list all jobs for ROOT admin
if (accountMgr.isRootAdmin(account.getId())) {
@ -862,11 +860,11 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
jobs = asyncMgr.findInstancePendingAsyncJobs(command.getApiResourceType().toString(), account.getId());
}
if (jobs.size() == 0) {
if (jobs.isEmpty()) {
return;
}
final Map<String, AsyncJob> objectJobMap = new HashMap<String, AsyncJob>();
final Map<String, AsyncJob> objectJobMap = new HashMap<>();
for (final AsyncJob job : jobs) {
if (job.getInstanceId() == null) {
continue;
@ -903,7 +901,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (Boolean.TRUE.equals(apiKeyAccessEnabled)) {
return true;
} else {
logger.info("Api-Key access is disabled for the User " + user.toString());
logger.info("Api-Key access is disabled for the User {}", user);
return false;
}
}
@ -912,7 +910,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (Boolean.TRUE.equals(apiKeyAccessEnabled)) {
return true;
} else {
logger.info("Api-Key access is disabled for the Account " + account.toString());
logger.info("Api-Key access is disabled for the Account {}", account);
return false;
}
}
@ -929,7 +927,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
public boolean verifyRequest(final Map<String, Object[]> requestParameters, final Long userId, InetAddress remoteAddress) throws ServerApiException {
try {
String apiKey = null;
String secretKey = null;
String secretKey;
String signature = null;
String unsignedRequest = null;
@ -957,11 +955,9 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
// - build a request string with sorted params, make sure it's all lowercase
// - sign the request, verify the signature is the same
final List<String> parameterNames = new ArrayList<String>();
for (final Object paramNameObj : requestParameters.keySet()) {
parameterNames.add((String)paramNameObj); // put the name in a list that we'll sort later
}
// put the name in a list that we'll sort later
final List<String> parameterNames = new ArrayList<>(requestParameters.keySet());
Collections.sort(parameterNames);
@ -997,7 +993,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
return false; // no signature, bad request
}
Date expiresTS = null;
Date expiresTS;
// FIXME: Hard coded signature, why not have an enum
if ("3".equals(signatureVersion)) {
// New signature authentication. Check for expire parameter and its validity
@ -1017,18 +1013,18 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (expiresTS.before(now)) {
signature = signature.replaceAll(SANITIZATION_REGEX, "_");
apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_");
logger.debug(String.format("Request expired -- ignoring ...sig [%s], apiKey [%s].", signature, apiKey));
logger.debug("Request expired -- ignoring ...sig [{}], apiKey [{}].", signature, apiKey);
return false;
}
}
final TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB);
txn.close();
User user = null;
User user;
// verify there is a user with this api key
final Pair<User, Account> userAcctPair = accountMgr.findUserByApiKey(apiKey);
if (userAcctPair == null) {
logger.debug("apiKey does not map to a valid user -- ignoring request, apiKey: " + apiKey);
logger.debug("apiKey does not map to a valid user -- ignoring request, apiKey: {}", apiKey);
return false;
}
@ -1069,7 +1065,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
if (!equalSig) {
signature = signature.replaceAll(SANITIZATION_REGEX, "_");
logger.info(String.format("User signature [%s] is not equaled to computed signature [%s].", signature, computedSignature));
logger.info("User signature [{}] is not equaled to computed signature [{}].", signature, computedSignature);
} else {
CallContext.register(user, account);
}
@ -1128,10 +1124,10 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
session.removeAttribute("domain_UUID");
}
final Enumeration attrNames = session.getAttributeNames();
final Enumeration<String> attrNames = session.getAttributeNames();
if (attrNames != null) {
while (attrNames.hasMoreElements()) {
final String attrName = (String) attrNames.nextElement();
final String attrName = attrNames.nextElement();
final Object attrObj = session.getAttribute(attrName);
if (ApiConstants.USERNAME.equalsIgnoreCase(attrName)) {
response.setUsername(attrObj.toString());
@ -1202,7 +1198,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
final long longDate = date.getTime();
final float offsetInMs = (t.getOffset(longDate));
offsetInHrs = offsetInMs / (1000 * 60 * 60);
logger.info("Timezone offset from UTC is: " + offsetInHrs);
logger.info("Timezone offset from UTC is: {}", offsetInHrs);
}
final Account account = accountMgr.getAccount(userAcct.getAccountId());
@ -1253,7 +1249,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
// (bug 5483) generate a session key that the user must submit on every request to prevent CSRF, add that
// to the login response so that session-based authenticators know to send the key back
final SecureRandom sesssionKeyRandom = new SecureRandom();
final byte sessionKeyBytes[] = new byte[20];
final byte[] sessionKeyBytes = new byte[20];
sesssionKeyRandom.nextBytes(sessionKeyBytes);
final String sessionKey = Base64.encodeBase64URLSafeString(sessionKeyBytes);
session.setAttribute(ApiConstants.SESSIONKEY, sessionKey);
@ -1266,7 +1262,6 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
@Override
public void logoutUser(final long userId) {
accountMgr.logoutUser(userId);
return;
}
@Override
@ -1294,30 +1289,26 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
throw new CloudRuntimeException(errorMessage);
}
if (StringUtils.isBlank(userAccount.getEmail())) {
logger.error(String.format(
"Email is not set. username: %s account id: %d domain id: %d",
userAccount.getUsername(), userAccount.getAccountId(), userAccount.getDomainId()));
logger.error("Email is not set. username: {} account id: {} domain id: {}",
userAccount.getUsername(), userAccount.getAccountId(), userAccount.getDomainId());
throw new CloudRuntimeException("Email is not set for the user.");
}
if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getState()).equals(Account.State.ENABLED)) {
logger.error(String.format(
"User is not enabled. username: %s account id: %d domain id: %s",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid()));
logger.error("User is not enabled. username: {} account id: {} domain id: {}",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid());
throw new CloudRuntimeException("User is not enabled.");
}
if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getAccountState()).equals(Account.State.ENABLED)) {
logger.error(String.format(
"Account is not enabled. username: %s account id: %d domain id: %s",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid()));
logger.error("Account is not enabled. username: {} account id: {} domain id: {}",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid());
throw new CloudRuntimeException("Account is not enabled.");
}
if (!domain.getState().equals(Domain.State.Active)) {
logger.error(String.format(
"Domain is not active. username: %s account id: %d domain id: %s",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid()));
logger.error("Domain is not active. username: {} account id: {} domain id: {}",
userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid());
throw new CloudRuntimeException("Domain is not active.");
}
@ -1425,7 +1416,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
// code to be very specific to our needs
static class ListenerThread extends Thread {
private static Logger LOGGER = LogManager.getLogger(ListenerThread.class);
private static final Logger LOGGER = LogManager.getLogger(ListenerThread.class);
private HttpService _httpService = null;
private ServerSocket _serverSocket = null;
private HttpParams _params = null;
@ -1464,7 +1455,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
@Override
public void run() {
LOGGER.info("ApiServer listening on port " + _serverSocket.getLocalPort());
LOGGER.info("ApiServer listening on port {}", _serverSocket.getLocalPort());
while (!Thread.interrupted()) {
try {
// Set up HTTP connection
@ -1507,10 +1498,10 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
}
} catch (final IOException ex) {
if (logger.isTraceEnabled()) {
logger.trace("ApiServer: IOException - " + ex);
logger.trace("ApiServer: IOException - {}", ex.toString());
}
} catch (final HttpException ex) {
logger.warn("ApiServer: Unrecoverable HTTP protocol violation" + ex);
logger.warn("ApiServer: Unrecoverable HTTP protocol violation {}", ex.toString());
} finally {
try {
_conn.shutdown();
@ -1523,7 +1514,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
@Override
public String getSerializedApiError(final int errorCode, final String errorText, final Map<String, Object[]> apiCommandParams, final String responseType) {
String responseName = null;
Class<?> cmdClass = null;
Class<?> cmdClass;
String responseText = null;
try {
@ -1536,7 +1527,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
final String cmdName = ((String[])cmdObj)[0];
cmdClass = getCmdClass(cmdName);
if (cmdClass != null) {
responseName = ((BaseCmd)cmdClass.newInstance()).getCommandName();
responseName = ((BaseCmd)cmdClass.getDeclaredConstructor().newInstance()).getCommandName();
} else {
responseName = "errorresponse";
}
@ -1558,7 +1549,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
@Override
public String getSerializedApiError(final ServerApiException ex, final Map<String, Object[]> apiCommandParams, final String responseType) {
String responseName = null;
Class<?> cmdClass = null;
Class<?> cmdClass;
String responseText = null;
if (ex == null) {
@ -1576,7 +1567,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
final String cmdName = ((String[])cmdObj)[0];
cmdClass = getCmdClass(cmdName);
if (cmdClass != null) {
responseName = ((BaseCmd)cmdClass.newInstance()).getCommandName();
responseName = ((BaseCmd)cmdClass.getDeclaredConstructor().newInstance()).getCommandName();
} else {
responseName = "errorresponse";
}
@ -1588,8 +1579,8 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
apiResponse.setResponseName(responseName);
final ArrayList<ExceptionProxyObject> idList = ex.getIdProxyList();
if (idList != null) {
for (int i = 0; i < idList.size(); i++) {
apiResponse.addProxyObject(idList.get(i));
for (ExceptionProxyObject exceptionProxyObject : idList) {
apiResponse.addProxyObject(exceptionProxyObject);
}
}
// Also copy over the cserror code and the function/layer in which

View File

@ -21,7 +21,6 @@ import java.net.InetAddress;
import java.net.URLDecoder;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -76,9 +75,7 @@ import com.cloud.utils.net.NetUtils;
@Component("apiServlet")
public class ApiServlet extends HttpServlet {
protected static Logger LOGGER = LogManager.getLogger(ApiServlet.class);
private final static List<String> s_clientAddressHeaders = Collections
.unmodifiableList(Arrays.asList("X-Forwarded-For",
"HTTP_CLIENT_IP", "HTTP_X_FORWARDED_FOR", "Remote_Addr"));
private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName());
private static final String REPLACEMENT = "_";
private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
@ -374,7 +371,7 @@ public class ApiServlet extends HttpServlet {
LOGGER.error("unknown exception writing api response", ex);
auditTrailSb.append(" unknown exception writing api response");
} finally {
LOGGER.info(auditTrailSb.toString());
ACCESSLOGGER.info(auditTrailSb.toString());
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("===END=== " + reqStr);
}