address comments

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2026-04-29 10:46:22 +05:30
parent 5710676961
commit 574f0ea40a
16 changed files with 129 additions and 91 deletions

View File

@ -261,6 +261,7 @@ public class ApiConstants {
public static final String FOR_VIRTUAL_NETWORK = "forvirtualnetwork";
public static final String FOR_SYSTEM_VMS = "forsystemvms";
public static final String FOR_PROVIDER = "forprovider";
public static final String FROM_CHECKPOINT_ID = "fromcheckpointid";
public static final String FULL_PATH = "fullpath";
public static final String GATEWAY = "gateway";
public static final String IP6_GATEWAY = "ip6gateway";
@ -610,6 +611,7 @@ public class ApiConstants {
public static final String TENANT_NAME = "tenantname";
public static final String TOTAL = "total";
public static final String TOTAL_SUBNETS = "totalsubnets";
public static final String TO_CHECKPOINT_ID = "tocheckpointid";
public static final String TYPE = "type";
public static final String TRUST_STORE = "truststore";
public static final String TRUST_STORE_PASSWORD = "truststorepass";

View File

@ -163,7 +163,8 @@ public class CreateVolumeCmd extends BaseAsyncCreateCustomIdCmd implements UserC
public Long getStorageId() {
if (snapshotId != null && storageId != null) {
throw new IllegalArgumentException("StorageId parameter cannot be specified with the SnapshotId parameter.");
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
"StorageId parameter cannot be specified with the SnapshotId parameter.");
}
return storageId;
}

View File

@ -127,16 +127,16 @@ public class BackupResponse extends BaseResponse {
@Param(description = "Indicates whether the VM from which the backup was taken is expunged or not", since = "4.22.0")
private Boolean isVmExpunged;
@SerializedName("from_checkpoint_id")
@Param(description = "Previous active checkpoint id for incremental backups", since = "4.22.0")
@SerializedName(ApiConstants.FROM_CHECKPOINT_ID)
@Param(description = "Previous active checkpoint ID for incremental backups", since = "4.23.0")
private String fromCheckpointId;
@SerializedName("to_checkpoint_id")
@Param(description = "Next checkpoint id for incremental backups", since = "4.22.0")
@SerializedName(ApiConstants.TO_CHECKPOINT_ID)
@Param(description = "Next checkpoint ID for incremental backups", since = "4.23.0")
private String toCheckpointId;
@SerializedName(ApiConstants.HOST_ID)
@Param(description = "Host ID where the backup is running", since = "4.22.0")
@Param(description = "Host ID where the backup is running", since = "4.23.0")
private String hostId;
public String getId() {

View File

@ -25,7 +25,9 @@ import java.util.Enumeration;
import java.util.List;
import javax.servlet.DispatcherType;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.cloudstack.utils.server.ServerPropertiesUtil;
import org.apache.cloudstack.veeam.api.ApiRouteHandler;
@ -155,33 +157,27 @@ public class VeeamControlServer {
// Handler for root ('/') path
final ServletContextHandler root = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
root.setContextPath("/");
root.addServlet(new ServletHolder(new javax.servlet.http.HttpServlet() {
root.addServlet(new ServletHolder(new HttpServlet() {
private static final long serialVersionUID = 1L;
@Override
protected void doGet(javax.servlet.http.HttpServletRequest req, javax.servlet.http.HttpServletResponse resp)
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws java.io.IOException {
resp.setContentType("text/plain");
resp.setStatus(javax.servlet.http.HttpServletResponse.SC_OK);
resp.setStatus(HttpServletResponse.SC_OK);
resp.getWriter().println("Veeam Control API");
}
@Override
protected void doPost(javax.servlet.http.HttpServletRequest req, javax.servlet.http.HttpServletResponse resp)
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws java.io.IOException {
doGet(req, resp);
}
}), "/*");
final RequestLog requestLog = (request, response) -> {
final String uri = request.getRequestURI() +
(request.getQueryString() != null ? "?" + request.getQueryString() : "");
LOGGER.info("Request - remoteAddr: {}, method: {}, uri: {}, headers: {}, status: {}",
request.getRemoteAddr(),
request.getMethod(),
uri,
dumpRequestHeaders(request),
response.getStatus());
LOGGER.debug("Handled request - {}",
() -> getRequestResponseMetadata(request, response));
};
final RequestLogHandler requestLogHandler = new RequestLogHandler();
@ -201,16 +197,25 @@ public class VeeamControlServer {
}
}
private static String dumpRequestHeaders(HttpServletRequest request) {
private static String getRequestResponseMetadata(HttpServletRequest request, HttpServletResponse response) {
final StringBuilder sb = new StringBuilder();
final Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
final String name = headerNames.nextElement();
final Enumeration<String> values = request.getHeaders(name);
while (values.hasMoreElements()) {
sb.append(name).append("=").append(values.nextElement()).append("; ");
sb.append("remote address: ").append(request.getRemoteAddr()).append(", ");
sb.append("method: ").append(request.getMethod()).append(", ");
sb.append("uri: ").append(request.getRequestURI())
.append(request.getQueryString() != null ? "?" + request.getQueryString() : "").append(", ");
if (VeeamControlService.DeveloperLogs.value()) {
sb.append("headers: [");
final Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
final String name = headerNames.nextElement();
final Enumeration<String> values = request.getHeaders(name);
while (values.hasMoreElements()) {
sb.append(name).append("=").append(values.nextElement()).append("; ");
}
}
sb.append("], ");
}
sb.append("status: ").append(response.getStatus());
return sb.toString();
}
}

View File

@ -57,12 +57,20 @@ public interface VeeamControlService extends PluggableService, Configurable {
"", "Comma-separated list of CIDR blocks representing clients allowed to access the API. " +
"If empty, all clients will be allowed. Example: '192.168.1.1/24,192.168.2.100/32", true);
ConfigKey<Boolean> DeveloperLogs = new ConfigKey<>("Developer", Boolean.class, "integration.veeam.control.developer.logs",
"false", "Enable verbose logging for development and troubleshooting purposes. " +
"Logs will include detailed information about API requests, responses, and internal operations.", false);
long getCurrentManagementServerHostId();
List<String> getAllowedClientCidrs();
String getInstanceId();
boolean validateCredentials(String username, String password);
String getHmacSecret();
static String getPackageVersion() {
return VeeamControlService.class.getPackage().getImplementationVersion();
}

View File

@ -17,6 +17,7 @@
package org.apache.cloudstack.veeam;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
@ -31,6 +32,8 @@ import org.apache.commons.lang3.StringUtils;
import com.cloud.cluster.ManagementServerHostVO;
import com.cloud.cluster.dao.ManagementServerHostDao;
import com.cloud.user.AccountService;
import com.cloud.utils.UuidUtils;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.net.NetUtils;
@ -39,6 +42,9 @@ public class VeeamControlServiceImpl extends ManagerBase implements VeeamControl
@Inject
ManagementServerHostDao managementServerHostDao;
@Inject
AccountService accountService;
private List<RouteHandler> routeHandlers;
private VeeamControlServer veeamControlServer;
private SingleCache<List<String>> allowedClientCidrsCache;
@ -83,12 +89,23 @@ public class VeeamControlServiceImpl extends ManagerBase implements VeeamControl
return allowedClientCidrsCache.get();
}
@Override
public String getInstanceId() {
return accountService.getSystemAccount().getUuid();
}
@Override
public boolean validateCredentials(String username, String password) {
return DataUtil.constantTimeEquals(Username.value(), username) &&
DataUtil.constantTimeEquals(Password.value(), password);
}
@Override
public String getHmacSecret() {
String base = getInstanceId() + ":" + Port.value() + ":" + Username.value() + Password.value();
return UuidUtils.nameUUIDFromBytes(base.getBytes(StandardCharsets.UTF_8)).toString();
}
@Override
public boolean start() {
allowedClientCidrsCache = new SingleCache<>(30, this::getAllowedClientCidrsInternal);
@ -132,7 +149,8 @@ public class VeeamControlServiceImpl extends ManagerBase implements VeeamControl
Password,
ServiceAccountId,
InstanceRestoreAssignOwner,
AllowedClientCidrs
AllowedClientCidrs,
DeveloperLogs
};
}
}

View File

@ -39,7 +39,6 @@ import org.apache.cloudstack.veeam.api.dto.SummaryCount;
import org.apache.cloudstack.veeam.api.dto.Version;
import org.apache.cloudstack.veeam.utils.Negotiation;
import com.cloud.user.AccountService;
import com.cloud.utils.component.ManagerBase;
public class ApiRouteHandler extends ManagerBase implements RouteHandler {
@ -49,7 +48,7 @@ public class ApiRouteHandler extends ManagerBase implements RouteHandler {
ServerAdapter serverAdapter;
@Inject
AccountService accountService;
VeeamControlService veeamControlService;
@Override
public boolean canHandle(String method, String path) {
@ -99,7 +98,7 @@ public class ApiRouteHandler extends ManagerBase implements RouteHandler {
/* ---------------- Product info ---------------- */
ProductInfo productInfo = new ProductInfo();
productInfo.setInstanceId(accountService.getSystemAccount().getUuid());
productInfo.setInstanceId(veeamControlService.getInstanceId());
productInfo.name = VeeamControlService.PLUGIN_NAME;
productInfo.version = Version.fromPackageAndCSVersion(true);

View File

@ -71,7 +71,7 @@ public class AsyncJobJoinVOToJobConverter {
protected static void fillAction(final ResourceAction action, final AsyncJobJoinVO vo) {
final String basePath = VeeamControlService.ContextPath.value();
action.setJob(Ref.of(basePath + JobsRouteHandler.BASE_ROUTE + vo.getUuid(), vo.getUuid()));
action.setJob(Ref.of(basePath + JobsRouteHandler.BASE_ROUTE + "/" + vo.getUuid(), vo.getUuid()));
action.setStatus("complete");
}

View File

@ -17,9 +17,9 @@
package org.apache.cloudstack.veeam.api.converter;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.cloudstack.veeam.VeeamControlService;
import org.apache.cloudstack.veeam.api.DataCentersRouteHandler;
@ -50,7 +50,6 @@ public class DataCenterJoinVOToDataCenterConverter {
dc.setStatus(Grouping.AllocationState.Enabled.equals(zone.getAllocationState()) ? "up" : "down");
dc.setLocal("false");
dc.setQuotaMode("disabled");
dc.setStorageFormat("v5");
// ---- Versions ----
final Version ver = Version.fromPackageAndCSVersion(false);
@ -61,11 +60,10 @@ public class DataCenterJoinVOToDataCenterConverter {
dc.setMacPool(Ref.of(basePath + "/macpools/default", "default"));
// ---- Related links ----
dc.link = Arrays.asList(
Link.of(href + "/clusters", "clusters"),
Link.of(href + "/networks", "networks"),
Link.of(href + "/storagedomains", "storagedomains")
);
dc.link = Stream.of("cluster", "networks", "storagedomains")
.map(rel -> Link.of(rel, href + "/" + rel))
.collect(Collectors.toList());
return dc;
}

View File

@ -21,7 +21,7 @@ public class ReportedDevice extends BaseDto {
private String comment;
private String description;
private NamedList<Ip> ips;
private Mac Mac;
private Mac mac;
private String name;
private String type;
private Vm vm;
@ -51,11 +51,11 @@ public class ReportedDevice extends BaseDto {
}
public Mac getMac() {
return Mac;
return mac;
}
public void setMac(Mac mac) {
Mac = mac;
this.mac = mac;
}
public String getName() {

View File

@ -64,8 +64,8 @@ public class AllowedClientCidrsFilter implements Filter {
final HttpServletResponse resp = (HttpServletResponse) response;
if (veeamControlService == null) {
LOGGER.warn("Failed to inject VeeamControlService, allowing request by default");
chain.doFilter(request, response);
LOGGER.error("Failed to inject VeeamControlService, rejecting request because allowed client CIDRs cannot be evaluated");
resp.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE, "Service Unavailable");
return;
}

View File

@ -128,7 +128,7 @@ public class BearerOrBasicAuthFilter implements Filter {
final byte[] expectedSig;
try {
expectedSig = JwtUtil.hmacSha256((headerB64 + "." + payloadB64).getBytes(StandardCharsets.UTF_8),
SsoService.HMAC_SECRET.getBytes(StandardCharsets.UTF_8));
veeamControlService.getHmacSecret().getBytes(StandardCharsets.UTF_8));
} catch (Exception e) {
return false;
}

View File

@ -40,7 +40,6 @@ public class SsoService extends ManagerBase implements RouteHandler {
private static final String BASE_ROUTE = "/sso";
private static final long DEFAULT_TTL_SECONDS = 3600;
public static final List<String> REQUIRED_SCOPES = List.of("ovirt-app-admin", "ovirt-app-portal");
public static final String HMAC_SECRET = "change-this-super-secret-key-change-this";
@Inject
VeeamControlService veeamControlService;
@ -111,7 +110,7 @@ public class SsoService extends ManagerBase implements RouteHandler {
long expMillis = nowMillis + ttl * 1000L;
final String token;
try {
token = JwtUtil.issueHs256Jwt(username, effectiveScope, ttl, HMAC_SECRET);
token = JwtUtil.issueHs256Jwt(username, effectiveScope, ttl, veeamControlService.getHmacSecret());
} catch (Exception e) {
io.getWriter().write(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
Map.of("error", "server_error",

View File

@ -23,6 +23,8 @@ import java.time.Instant;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import com.google.gson.JsonObject;
public class JwtUtil {
public static final String ALGORITHM = "HmacSHA256";
public static final String ISSUER = "veeam-control";
@ -31,15 +33,17 @@ public class JwtUtil {
long now = Instant.now().getEpochSecond();
long exp = now + ttlSeconds;
String headerJson = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}";
String payloadJson =
"{"
+ "\"iss\":\"" + DataUtil.jsonEscape(ISSUER) + "\","
+ "\"sub\":\"" + DataUtil.jsonEscape(subject) + "\","
+ "\"scope\":\"" + DataUtil.jsonEscape(scope) + "\","
+ "\"iat\":" + now + ","
+ "\"exp\":" + exp
+ "}";
JsonObject headerObject = new JsonObject();
headerObject.addProperty("alg", "HS256");
headerObject.addProperty("typ", "JWT");
String headerJson = headerObject.toString();
JsonObject payloadObject = new JsonObject();
payloadObject.addProperty("iss", ISSUER);
payloadObject.addProperty("sub", subject);
payloadObject.addProperty("scope", scope);
payloadObject.addProperty("iat", now);
payloadObject.addProperty("exp", exp);
String payloadJson = payloadObject.toString();
String header = DataUtil.b64Url(headerJson.getBytes(StandardCharsets.UTF_8));
String payload = DataUtil.b64Url(payloadJson.getBytes(StandardCharsets.UTF_8));

View File

@ -30,46 +30,46 @@ public class PathUtil {
public static List<String> extractIdAndSubPath(final String path, final String baseRoute) {
if (StringUtils.isBlank(path)) {
return null;
return null;
}
// Remove base route (be tolerant of trailing slash in baseRoute)
String rest = path;
if (StringUtils.isNotBlank(baseRoute)) {
String normalizedBase = baseRoute.endsWith("/") && baseRoute.length() > 1
? baseRoute.substring(0, baseRoute.length() - 1)
: baseRoute;
if (rest.startsWith(normalizedBase)) {
rest = rest.substring(normalizedBase.length());
}
}
// Remove base route (be tolerant of trailing slash in baseRoute)
String rest = path;
if (StringUtils.isNotBlank(baseRoute)) {
String normalizedBase = baseRoute.endsWith("/") && baseRoute.length() > 1
? baseRoute.substring(0, baseRoute.length() - 1)
: baseRoute;
if (rest.startsWith(normalizedBase)) {
rest = rest.substring(normalizedBase.length());
}
// Expect "/{id}" or "/{id}/..." (no empty segments)
if (StringUtils.isBlank(rest) || !rest.startsWith("/")) {
return null; // /api/datacenters (no id) or invalid format
}
rest = rest.substring(1); // remove leading '/'
if (StringUtils.isBlank(rest)) {
return null;
}
final String[] parts = rest.split("/", -1);
// Collect non-blank segments
List<String> validParts = new ArrayList<>();
for (String part : parts) {
if (StringUtils.isNotBlank(part)) {
validParts.add(part);
}
}
// Expect "/{id}" or "/{id}/..." (no empty segments)
if (StringUtils.isBlank(rest) || !rest.startsWith("/")) {
return null; // /api/datacenters (no id) or invalid format
}
// Validate first segment, check if it is a UUID if enabled
if (validParts.isEmpty() || (CONSIDER_ONLY_UUID_AS_ID && !UuidUtils.isUuid(validParts.get(0)))) {
return null;
}
rest = rest.substring(1); // remove leading '/'
if (StringUtils.isBlank(rest)) {
return null;
}
final String[] parts = rest.split("/", -1);
// Collect non-blank segments
List<String> validParts = new ArrayList<>();
for (String part : parts) {
if (StringUtils.isNotBlank(part)) {
validParts.add(part);
}
}
// Validate first segment is a UUID
if (validParts.isEmpty() || (CONSIDER_ONLY_UUID_AS_ID && !UuidUtils.isUuid(validParts.get(0)))) {
return null;
}
return validParts;
return validParts;
}
}

View File

@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets;
import javax.servlet.http.HttpServletResponse;
import org.apache.cloudstack.veeam.VeeamControlService;
import org.apache.cloudstack.veeam.api.dto.Fault;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@ -64,7 +65,10 @@ public final class ResponseWriter {
return;
}
LOGGER.info("Writing response: {}\n{}", status, payload);
if (VeeamControlService.DeveloperLogs.value()) {
LOGGER.debug("Writing response: status={}, contentType={}, payloadLength={}\n{}",
status, contentType, payload.length(), payload);
}
resp.setCharacterEncoding(StandardCharsets.UTF_8.name());
resp.setHeader("Content-Type", contentType);