Copilot commented on code in PR #13675:
URL: https://github.com/apache/cloudstack/pull/13675#discussion_r3644874143
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8891,7 +8891,7 @@ public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {EnableDynamicallyScaleVm,
AllowDiskOfferingChangeDuringScaleVm, AllowUserExpungeRecoverVm,
VmIpFetchWaitInterval, VmIpFetchTrialMax,
VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers,
AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
KvmAdditionalConfigAllowList,
XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList,
DestroyRootVolumeOnVmDestruction,
- EnforceStrictResourceLimitHostTagCheck, StrictHostTags,
AllowUserForceStopVm};
+ EnforceStrictResourceLimitHostTagCheck, StrictHostTags,
AllowUserForceStopVm, AllowExposingVmAssignFailureDetails};
Review Comment:
`AllowExposingVmAssignFailureDetails` is referenced unqualified here, but
`UserVmManagerImpl` does not implement `UserVmService`, so this constant is not
in scope and will not compile. Qualify it with `UserVmService.` (same package)
or add a static import.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java:
##########
@@ -128,6 +129,9 @@ public void execute() {
ApiErrorCode errorCode = e instanceof
InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR :
ApiErrorCode.INTERNAL_ERROR;
String msg = String.format("Failed to move Instance due to [%s].",
getVmId());
logger.error(msg, e);
+ if (e instanceof InvalidParameterValueException &&
UserVmService.AllowExposingVmAssignFailureDetails.value()) {
+ msg = e.getMessage();
+ }
Review Comment:
The generic error message reads as if the VM ID were the *cause* ("due to
[<id>]"). Also, when detailed messages are enabled the code replaces the
message with `e.getMessage()`, losing the context that this error happened
during an Instance move. Prefer a consistent message that includes the VM ID
and appends the validation details when allowed.
##########
api/src/main/java/com/cloud/vm/UserVmService.java:
##########
@@ -64,6 +65,11 @@
public interface UserVmService {
+ ConfigKey<Boolean> AllowExposingVmAssignFailureDetails = new
ConfigKey<>("Advanced", Boolean.class,
+ "vm.assign.failure.detailed.message.enabled", "true",
Review Comment:
The new global setting enables returning raw
`InvalidParameterValueException` messages to API callers by default (`"true"`).
This changes behavior and may unintentionally expose internal details (object
IDs, resource names, etc.) unless operators explicitly opt out. Consider
defaulting this config to `false` (opt-in) to preserve the prior, less verbose
error behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
