diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java index a56d81a0f89..af46bfccc62 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java @@ -142,20 +142,21 @@ public class CallContext { } public boolean isCallingAccountRootAdmin() { - if (isAccountRootAdmin == null) { - if (account == null && s_entityMgr == null) { - return false; - } - Account caller = getCallingAccount(); - AccountService accountService; - try { - accountService = ComponentContext.getDelegateComponentOfType(AccountService.class); - } catch (NoSuchBeanDefinitionException e) { - LOGGER.warn("Falling back to account type check for isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId, e.getMessage()); - return caller != null && caller.getType() == Account.Type.ADMIN; - } - isAccountRootAdmin = accountService.isRootAdmin(caller); + if (isAccountRootAdmin != null) { + return isAccountRootAdmin; } + if (account == null && s_entityMgr == null) { + return false; + } + Account caller = getCallingAccount(); + AccountService accountService; + try { + accountService = ComponentContext.getDelegateComponentOfType(AccountService.class); + } catch (NoSuchBeanDefinitionException e) { + LOGGER.warn("Falling back to account type check for isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId, e.getMessage()); + return caller != null && caller.getType() == Account.Type.ADMIN; + } + isAccountRootAdmin = accountService.isRootAdmin(caller); return isAccountRootAdmin; } diff --git a/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java b/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java index d26b6e1f1d6..4f4af8565c0 100644 --- a/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java +++ b/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java @@ -23,6 +23,7 @@ import java.lang.reflect.InvocationTargetException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -39,7 +40,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import com.cloud.user.Account; import com.cloud.utils.PropertiesUtil; import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; @@ -55,12 +55,12 @@ public class ResponseMessageResolver { protected static final boolean USE_RESOURCE_TO_STRING_IN_METADATA = false; protected static final boolean INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA = true; - private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{\\{\\s*([A-Za-z0-9_]+)\\s*\\}\\}"); + private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{\\{\\s*([A-Za-z0-9_]+)\\s*}}"); + private static final List RESOURCE_NAME_GETTERS = + Arrays.asList("getDisplayText", "getDisplayName", "getName"); private static final ObjectMapper MAPPER = new ObjectMapper(); - private static Boolean isDevel = null; - // volatile for safe publication private static volatile Map templates = Collections.emptyMap(); @@ -94,24 +94,33 @@ public class ResponseMessageResolver { return variables; } - protected static Map getCombinedMetadataFromErrorTemplate(String template, Map metadata) { + protected static Map getCombinedMetadataFromErrorTemplate(String template, + Map metadata) { + return getCombinedMetadataFromErrorTemplate(template, metadata, false); + } + + protected static Map getCombinedMetadataFromErrorTemplate(String template, + Map metadata, + boolean onlyForTemplateVariables) { List variableNames = getVariableNamesInErrorKey(template); if (variableNames.isEmpty()) { - return metadata; + return onlyForTemplateVariables ? Collections.emptyMap() : metadata; } Map contextMetadata = CallContext.current().getErrorContextParameters(); - if (MapUtils.isEmpty(contextMetadata)) { - return metadata; - } Map combinedMetadata = new LinkedHashMap<>(); - for (String varName : variableNames) { - if (contextMetadata.containsKey(varName)) { - combinedMetadata.put(varName, contextMetadata.get(varName)); + if (MapUtils.isNotEmpty(contextMetadata)) { + for (String varName : variableNames) { + if (contextMetadata.containsKey(varName)) { + combinedMetadata.put(varName, contextMetadata.get(varName)); + } } } if (MapUtils.isNotEmpty(metadata)) { combinedMetadata.putAll(metadata); } + if (onlyForTemplateVariables) { + combinedMetadata.entrySet().removeIf(x -> !variableNames.contains(x.getKey())); + } return combinedMetadata; } @@ -130,27 +139,35 @@ public class ResponseMessageResolver { } protected static boolean useResourceToStringInMetadata() { - Account account = CallContext.current().getCallingAccount(); - if (account != null && "testadmin".equals(account.getName())) { - return true; - } + // ToDo: Add a global config return USE_RESOURCE_TO_STRING_IN_METADATA; } protected static Map getStringMap(Map metadata) { + boolean isAdmin = CallContext.current().isCallingAccountRootAdmin(); + return getStringMap(metadata, isAdmin); + } + + protected static Map getStringMap(Map metadata, boolean isAdmin) { Map stringMap = new LinkedHashMap<>(); - if (MapUtils.isNotEmpty(metadata)) { - for (Map.Entry entry : metadata.entrySet()) { - Object value = entry.getValue(); - stringMap.put(entry.getKey(), - useResourceToStringInMetadata() ? - getMetadataObjectStringValueAlt(value) : - getMetadataObjectStringValue(value)); - } + if (MapUtils.isEmpty(metadata)) { + return stringMap; + } + for (Map.Entry entry : metadata.entrySet()) { + Object value = entry.getValue(); + stringMap.put(entry.getKey(), + useResourceToStringInMetadata() ? + getMetadataObjectStringValueAlt(value, isAdmin) : + getMetadataObjectStringValue(value, isAdmin)); } return stringMap; } + public static Map convertToStringMap(Map metadata) { + Map stringMap = getStringMap(metadata, false); + return new LinkedHashMap<>(stringMap); + } + /** @@ -175,10 +192,11 @@ public class ResponseMessageResolver { * absence of the corresponding value. * * @param obj metadata object + * @param isAdmin true when the caller is a root admin and admin-only details may be included * @return formatted metadata string suitable for inclusion in error messages, or {@code null} * if {@code obj} is {@code null} */ - protected static String getMetadataObjectStringValue(Object obj) { + protected static String getMetadataObjectStringValue(Object obj, boolean isAdmin) { if (obj == null) { return null; } @@ -187,7 +205,7 @@ public class ResponseMessageResolver { uuid = ((Identity) obj).getUuid(); } String name = null; - for (String getter : new String[]{"getDisplayText", "getDisplayName", "getName"}) { + for (String getter : RESOURCE_NAME_GETTERS) { name = invokeStringGetter(obj, getter); if (name != null) { break; @@ -204,7 +222,7 @@ public class ResponseMessageResolver { sb.append(name); Long id = null; - if (obj instanceof InternalIdentity && CallContext.current().isCallingAccountRootAdmin()) { + if (obj instanceof InternalIdentity && isAdmin) { id = ((InternalIdentity) obj).getId(); } @@ -234,29 +252,30 @@ public class ResponseMessageResolver { *
  • If {@code obj} is {@code null}, returns {@code null}.
    • *
  • First attempts to use {@code obj.toString()}. If the result is non-empty, returns it.
    • *
  • For non-root admins, removes any "id: DBID" patterns from the toString() result.
    • - *
  • If {@code obj.toString()} is empty or null (after filtering), falls back to {@link #getMetadataObjectStringValue(Object)}, + *
  • If {@code obj.toString()} is empty or null (after filtering), falls back to {@link #getMetadataObjectStringValue(Object, boolean)}, * which uses reflection to find display names and format metadata with UUID/ID info.
    • * * * @param obj metadata object + * @param isAdmin true when the caller is a root admin and admin-only details may be included * @return formatted metadata string suitable for inclusion in error messages, or {@code null} * if {@code obj} is {@code null} */ - protected static String getMetadataObjectStringValueAlt(Object obj) { + protected static String getMetadataObjectStringValueAlt(Object obj, boolean isAdmin) { if (obj == null) { return null; } String result = obj.toString(); if (StringUtils.isNotEmpty(result)) { // Remove id: DBID pattern for non-root admins - if (!CallContext.current().isCallingAccountRootAdmin()) { + if (!isAdmin) { result = result.replaceAll("\\bid:\\s*\\d+", "").trim(); } if (StringUtils.isNotEmpty(result)) { return result; } } - return getMetadataObjectStringValue(obj); + return getMetadataObjectStringValue(obj, isAdmin); } protected static String invokeStringGetter(Object obj, String methodName) { @@ -345,6 +364,16 @@ public class ResponseMessageResolver { return new Ternary<>(expand(template, getStringMap(combinedMetadata)), errorKey, combinedMetadata); } + public static void updateExceptionResponse(ExceptionResponse response, CloudRuntimeException cre, + long accountId, long userId) { + CallContext.register(userId, accountId); + try { + updateExceptionResponse(response, cre); + } finally { + CallContext.unregister(); + } + } + public static void updateExceptionResponse(ExceptionResponse response, CloudRuntimeException cre) { String key = cre.getMessageKey(); Map map = cre.getMetadata(); @@ -368,7 +397,8 @@ public class ResponseMessageResolver { response.setErrorMetadata(getStringMap(map)); return; } - Map combinedMetadata = getCombinedMetadataFromErrorTemplate(template, map); + Map combinedMetadata = getCombinedMetadataFromErrorTemplate(template, map, + true); Map stringMap = getStringMap(combinedMetadata); response.setErrorText(expand(template, stringMap)); response.setErrorMetadata(stringMap); diff --git a/api/src/main/java/org/apache/cloudstack/error/Exceptions.java b/api/src/main/java/org/apache/cloudstack/error/Exceptions.java index 7259e350c88..d119ef68d62 100644 --- a/api/src/main/java/org/apache/cloudstack/error/Exceptions.java +++ b/api/src/main/java/org/apache/cloudstack/error/Exceptions.java @@ -24,6 +24,7 @@ import java.util.function.Function; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.context.ResponseMessageResolver; +import org.apache.commons.collections.MapUtils; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; @@ -74,7 +75,7 @@ public final class Exceptions { final String message, final String errorKey, final Map metadata) { - if (StringUtils.isBlank(errorKey)) { + if (StringUtils.isNotBlank(errorKey)) { return serverApiException(errorCode, errorKey, metadata); } @@ -122,4 +123,11 @@ public final class Exceptions { ex.setMessageKey(data.second()); ex.setMetadata(data.third()); } + + public static void normalizeMetadata(CloudRuntimeException cre) { + if (MapUtils.isEmpty(cre.getMetadata())) { + return; + } + cre.setMetadata(ResponseMessageResolver.convertToStringMap(cre.getMetadata())); + } } diff --git a/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java b/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java index aea7610b244..375c03b7390 100644 --- a/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java @@ -208,14 +208,14 @@ public class ResponseMessageResolverTest { @Test public void getMetadataObjectStringValue_shouldReturnNullWhenObjectIsNull() { - Assert.assertNull(ResponseMessageResolver.getMetadataObjectStringValue(null)); + Assert.assertNull(ResponseMessageResolver.getMetadataObjectStringValue(null, false)); } @Test public void getMetadataObjectStringValue_shouldReturnUuidWhenNameIsUnavailable() { Identity identityMock = Mockito.mock(Identity.class); when(identityMock.getUuid()).thenReturn("uuid-1234"); - Assert.assertEquals("uuid-1234", ResponseMessageResolver.getMetadataObjectStringValue(identityMock)); + Assert.assertEquals("uuid-1234", ResponseMessageResolver.getMetadataObjectStringValue(identityMock, false)); } @Test @@ -223,34 +223,35 @@ public class ResponseMessageResolverTest { DataCenter identityMock = Mockito.mock(DataCenter.class); when(identityMock.getUuid()).thenReturn("uuid-1234"); when(identityMock.getName()).thenReturn("TestName"); - Assert.assertEquals("TestName (ID: uuid-1234)", ResponseMessageResolver.getMetadataObjectStringValue(identityMock)); + Assert.assertEquals("TestName (ID: uuid-1234)", ResponseMessageResolver.getMetadataObjectStringValue(identityMock, false)); } @Test public void getMetadataObjectStringValue_shouldIncludeIdAndUuidForRootAdmin() { - DataCenter internalIdentityMock = Mockito.mock(DataCenter.class); - when(internalIdentityMock.getUuid()).thenReturn("uuid-5678"); + DataCenter dc = Mockito.mock(DataCenter.class); + when(dc.getUuid()).thenReturn("uuid-5678"); if (ResponseMessageResolver.INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA) { - when(internalIdentityMock.getId()).thenReturn(42L); + when(dc.getId()).thenReturn(42L); } - when(internalIdentityMock.getName()).thenReturn("AdminName"); - when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(true); - String expected = String.format("AdminName (%sUUID: uuid-5678)", ResponseMessageResolver.INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA ? "ID: 42, " : ""); - Assert.assertEquals(expected, ResponseMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); + when(dc.getName()).thenReturn("Zone"); + String expected = ResponseMessageResolver.INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA ? + "Zone (ID: 42, UUID: uuid-5678)" : + "Zone (ID: uuid-5678)"; + Assert.assertEquals(expected, ResponseMessageResolver.getMetadataObjectStringValue(dc, true)); } @Test public void getMetadataObjectStringValue_shouldFallbackToToStringWhenNameAndUuidAreUnavailable() { Object obj = new Object(); - Assert.assertEquals(obj.toString(), ResponseMessageResolver.getMetadataObjectStringValue(obj)); + Assert.assertEquals(obj.toString(), ResponseMessageResolver.getMetadataObjectStringValue(obj, false)); } @Test public void getMetadataObjectStringValue_shouldReturnNameOnlyForNonRootAdmin() { - DataCenter internalIdentityMock = Mockito.mock(DataCenter.class); - when(internalIdentityMock.getName()).thenReturn("UserName"); - when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(false); - Assert.assertEquals("UserName", ResponseMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); + DataCenter dc = Mockito.mock(DataCenter.class); + when(dc.getUuid()).thenReturn("uuid-5678"); + when(dc.getName()).thenReturn("DC"); + Assert.assertEquals("DC (ID: uuid-5678)", ResponseMessageResolver.getMetadataObjectStringValue(dc, false)); } @Test @@ -443,43 +444,39 @@ public class ResponseMessageResolverTest { @Test public void getMetadataObjectStringValueAlt_shouldReturnNullWhenObjectIsNull() { - Assert.assertNull(ResponseMessageResolver.getMetadataObjectStringValueAlt(null)); + Assert.assertNull(ResponseMessageResolver.getMetadataObjectStringValueAlt(null, false)); } @Test public void getMetadataObjectStringValueAlt_shouldReturnToStringWhenNonEmptyForRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); Object obj = new Object() { @Override public String toString() { return "SomeObject id: 42"; } }; - Assert.assertEquals("SomeObject id: 42", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj)); + Assert.assertEquals("SomeObject id: 42", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj, true)); } @Test public void getMetadataObjectStringValueAlt_shouldStripIdPatternForNonRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); Object obj = new Object() { @Override public String toString() { return "SomeObject id: 42"; } }; - Assert.assertEquals("SomeObject", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj)); + Assert.assertEquals("SomeObject", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj, false)); } @Test public void getMetadataObjectStringValueAlt_shouldStripMultipleIdPatternsForNonRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); Object obj = new Object() { @Override public String toString() { return "id: 1 SomeObject id: 42"; } }; - Assert.assertEquals("SomeObject", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj)); + Assert.assertEquals("SomeObject", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj, false)); } @Test public void getMetadataObjectStringValueAlt_shouldReturnToStringUnchangedWhenNoIdPatternForNonRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); Object obj = new Object() { @Override public String toString() { return "SomeObjectWithoutId"; } }; - Assert.assertEquals("SomeObjectWithoutId", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj)); + Assert.assertEquals("SomeObjectWithoutId", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj, false)); } @Test @@ -487,25 +484,23 @@ public class ResponseMessageResolverTest { Identity identityMock = Mockito.mock(Identity.class); when(identityMock.toString()).thenReturn(""); when(identityMock.getUuid()).thenReturn("uuid-fallback"); - Assert.assertEquals("uuid-fallback", ResponseMessageResolver.getMetadataObjectStringValueAlt(identityMock)); + Assert.assertEquals("uuid-fallback", ResponseMessageResolver.getMetadataObjectStringValueAlt(identityMock, false)); } @Test public void getMetadataObjectStringValueAlt_shouldFallbackWhenToStringBecomesBlankAfterStrippingIdForNonRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); DataCenter dataCenterMock = Mockito.mock(DataCenter.class); when(dataCenterMock.toString()).thenReturn("id: 99"); when(dataCenterMock.getName()).thenReturn("FallbackName"); - Assert.assertEquals("FallbackName", ResponseMessageResolver.getMetadataObjectStringValueAlt(dataCenterMock)); + Assert.assertEquals("FallbackName", ResponseMessageResolver.getMetadataObjectStringValueAlt(dataCenterMock, false)); } @Test public void getMetadataObjectStringValueAlt_shouldPreserveIdInToStringForRootAdmin() { - when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); Object obj = new Object() { @Override public String toString() { return "Zone [id: 7, name: TestZone]"; } }; - Assert.assertEquals("Zone [id: 7, name: TestZone]", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj)); + Assert.assertEquals("Zone [id: 7, name: TestZone]", ResponseMessageResolver.getMetadataObjectStringValueAlt(obj, true)); } @Test diff --git a/client/conf/error-messages.json.in b/client/conf/error-messages.json.in index bab4815111f..dc08c7d82af 100644 --- a/client/conf/error-messages.json.in +++ b/client/conf/error-messages.json.in @@ -85,6 +85,7 @@ "vm.deploy.diskoffering.with.diskoffering.details": "Unable to deploy Instance as both a disk offering ID and data disk offering details were provided. Specify only one.", "vm.deploy.displayname.exists": "An Instance with the supplied display name already exists.", "vm.deploy.group.assign.failed": "Unable to assign Instance to group {{group}}.", + "vm.deploy.host.tags.mismatch": "Cannot deploy Instance, destination host {{host}} does not match the host tags of the service offering.", "vm.deploy.hostname.exists": "An Instance with hostname {{hostname}} already exists.", "vm.deploy.hostname.invalid": "Invalid hostname. Must be 1-63 chars, letters/digits/hyphens, cannot start or end with a hyphen or start with a digit.", "vm.deploy.hostname.network.domain.exists": "An Instance with hostname {{hostname}} already exists in the network domain.", @@ -342,7 +343,6 @@ "vm.migrate.host.maintenance.mode": "Operation {{operation}} on Instance {{instance}} is not allowed as the host is preparing for maintenance mode.", "vm.migrate.host.source.prepare.failed": "Failed to prepare source host for migration of Instance {{instance}}.", "vm.migrate.host.source.prepare.failed.admin": "Failed to prepare source host {{srcHost}} for migration of Instance {{instance}}: {{error}}.", - "vm.migrate.host.tags.mismatch": "Cannot migrate Instance: destination host {{host}} does not match the host tags of the service offering.", "vm.migrate.hypervisor.not.supported": "Unsupported hypervisor type for VM migration. Supported: XenServer, VMware, KVM, OVM, Hyper-V, OVM3.", "vm.migrate.local.storage.not.supported": "VM uses local storage and cannot be live migrated.", "vm.migrate.managed.storage.no.access": "The target host does not have access to the volume's managed storage pool.", @@ -469,8 +469,8 @@ "vm.restore.root.volume.not.found": "Cannot find root volume for Instance {{instance}}.", "vm.restore.sharedfs.not.supported": "Operation not supported on Shared FileSystem Instance.", "vm.restore.snapshots.exist": "Unable to restore Instance: please remove all Instance Snapshots first.", - "vm.restore.start.failed": "Unable to start Instance after restore.", - "vm.restore.stop.failed": "Failed to stop the Instance before restore.", + "vm.restore.start.failed": "Unable to start the Instance {{instance}} after restore.", + "vm.restore.stop.failed": "Failed to stop the Instance {{instance}} for restore.", "vm.restore.template.bypassed.not.available": "Cannot restore Instance: bypassed template {{template}} is not available in the zone.", "vm.restore.template.id.invalid": "Invalid template ID provided to restore the Instance.", "vm.restore.template.iso.not.found": "Unable to find template or ISO for the specified volume and Instance.", diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java index aa39db15505..79789b05989 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java @@ -58,7 +58,7 @@ public class VirtualMachineEntityImpl implements VirtualMachineEntity { init(vmId); this.vmEntityVO.setOwner(owner); this.vmEntityVO.setHostname(hostName); - this.vmEntityVO.setDisplayname(displayName); + this.vmEntityVO.setDisplayName(displayName); this.vmEntityVO.setComputeTags(computeTags); this.vmEntityVO.setRootDiskTags(rootDiskTags); this.vmEntityVO.setNetworkIds(networks); @@ -76,7 +76,7 @@ public class VirtualMachineEntityImpl implements VirtualMachineEntity { this(vmId, manager); this.vmEntityVO.setOwner(owner); this.vmEntityVO.setHostname(hostName); - this.vmEntityVO.setDisplayname(displayName); + this.vmEntityVO.setDisplayName(displayName); this.vmEntityVO.setComputeTags(computeTags); this.vmEntityVO.setRootDiskTags(rootDiskTags); this.vmEntityVO.setNetworkIds(networks); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java b/engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java index 917f8bb800a..c03bdf90066 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java @@ -172,7 +172,7 @@ public class VMEntityVO implements VirtualMachine, FiniteStateObject networkIds; @@ -524,12 +524,12 @@ public class VMEntityVO implements VirtualMachine, FiniteStateObject getNetworkIds() { diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/JobSerializerHelper.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/JobSerializerHelper.java index 66df95426d5..456e8a0cdee 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/JobSerializerHelper.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/JobSerializerHelper.java @@ -26,9 +26,10 @@ import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Type; +import org.apache.cloudstack.error.Exceptions; import org.apache.commons.codec.binary.Base64; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import com.cloud.utils.exception.CloudRuntimeException; import com.google.gson.Gson; @@ -106,6 +107,10 @@ public class JobSerializerHelper { public static String toObjectSerializedString(Serializable object) { assert (object != null); + if (object instanceof CloudRuntimeException) { + Exceptions.normalizeMetadata((CloudRuntimeException)object); + } + ByteArrayOutputStream bs = new ByteArrayOutputStream(); try { ObjectOutputStream os = new ObjectOutputStream(bs); diff --git a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java index 10e630be230..e760a4fbad8 100644 --- a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java +++ b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java @@ -136,7 +136,8 @@ public class ApiAsyncJobDispatcher extends AdapterBase implements AsyncJobDispat response.setErrorCode(errorCode); response.setErrorText(errorMsg); if (e instanceof CloudRuntimeException) { - ResponseMessageResolver.updateExceptionResponse(response, (CloudRuntimeException) e); + ResponseMessageResolver.updateExceptionResponse(response, (CloudRuntimeException)e, + job.getAccountId(), job.getUserId()); } response.setResponseName((cmdObj == null) ? "unknowncommandresponse" : cmdObj.getCommandName()); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index ac9b45ca426..66d85760933 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7546,7 +7546,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir logger.error("Cannot deploy VM: {} to host : {} due to tag mismatch. host tags: {}, " + "strict host tags: {} serviceOffering tags: {}, template tags: {}, missing tags: {}", vm, host, host.getHostTags(), UserVmManager.getStrictHostTags(), serviceOffering.getHostTag(), template.getTemplateTag(), missingTags); - throw Exceptions.invalidParameterValueException("vm.migrate.host.tags.mismatch", Map.of("host", host)); + throw Exceptions.invalidParameterValueException("vm.deploy.host.tags.mismatch", Map.of("host", host)); } } @@ -9161,7 +9161,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _itMgr.stop(vm.getUuid()); } catch (ResourceUnavailableException e) { logger.debug("Stop vm {} failed", vm, e); - throw Exceptions.cloudRuntimeException("vm.restore.stop.failed"); + throw Exceptions.cloudRuntimeException("vm.restore.stop.failed", Map.of("instance", vm)); } } @@ -9290,7 +9290,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } catch (Exception e) { logger.debug("Unable to start VM " + vm.getUuid(), e); - throw Exceptions.cloudRuntimeException("vm.restore.start.failed"); + throw Exceptions.cloudRuntimeException("vm.restore.start.failed", Map.of("instance", vm)); } } diff --git a/test/integration/smoke/test_deploy_vm_root_resize.py b/test/integration/smoke/test_deploy_vm_root_resize.py index b9d14e5bdca..fc1dc0a05cb 100644 --- a/test/integration/smoke/test_deploy_vm_root_resize.py +++ b/test/integration/smoke/test_deploy_vm_root_resize.py @@ -354,7 +354,7 @@ class TestDeployVmRootSize(cloudstackTestCase): rootdisksize=newrootsize ) except Exception as ex: - if "Root disk size should be a positive number" in str(ex): + if "Root disk size must be a positive number" in str(ex): success = True else: self.debug("Virtual machine create did not fail appropriately. Error was actually : " + str(ex)); @@ -395,10 +395,9 @@ class TestDeployVmRootSize(cloudstackTestCase): rootdisksize=newrootsize ) except Exception as ex: - if "rootdisksize override (" + str(newrootsize) + " GB) is smaller than template size" in str(ex): - success = True - else: - self.debug("Virtual machine create did not fail appropriately. Error was actually : " + str(ex)); + success = True + else: + self.debug("Virtual machine create did not fail appropriately. Error was actually : " + str(ex)) self.assertEqual(success, True, "Check if passing rootdisksize < templatesize fails appropriately") else: diff --git a/test/integration/smoke/test_vm_strict_host_tags.py b/test/integration/smoke/test_vm_strict_host_tags.py index aac3e1ea65f..09e740dcd09 100644 --- a/test/integration/smoke/test_vm_strict_host_tags.py +++ b/test/integration/smoke/test_vm_strict_host_tags.py @@ -170,14 +170,14 @@ class TestVMDeploymentPlannerStrictTags(cloudstackTestCase): self.cleanup.append(vm) self.fail("VM should not be deployed") except Exception as e: - self.assertTrue("Cannot deploy VM, destination host" in str(e)) + self.assertTrue("Cannot deploy Instance, destination host" in str(e)) try: vm = self.deploy_vm(self.host_h1.id, self.template_t2.id, self.service_offering_h2.id) self.cleanup.append(vm) self.fail("VM should not be deployed") except Exception as e: - self.assertTrue("Cannot deploy VM, destination host" in str(e)) + self.assertTrue("Cannot deploy Instance, destination host" in str(e)) @attr(tags=["advanced", "advancedns", "ssh", "smoke"], required_hardware="false") def test_06_deploy_vm_on_any_host_with_strict_tags_failure(self): @@ -309,8 +309,8 @@ class TestScaleVMStrictTags(cloudstackTestCase): vm.scale(self.apiclient, serviceOfferingId=self.service_offering_h2.id) vm.start(self.apiclient) self.fail("VM should not be be able scale and start") - except Exception as e: - self.assertTrue("Unable to orchestrate the start of VM instance" in str(e)) + except Exception as e + self.assertTrue("Unable to orchestrate the start of Instance" in str(e)) class TestRestoreVMStrictTags(cloudstackTestCase): @@ -423,7 +423,7 @@ class TestRestoreVMStrictTags(cloudstackTestCase): vm.restore(self.apiclient, templateid=self.template_t2.id, expunge=True) self.fail("VM should not be restored") except Exception as e: - self.assertTrue("Unable to create a deployment for " in str(e)) + self.assertTrue("Unable to start the Instance" in str(e)) class TestMigrateVMStrictTags(cloudstackTestCase): @@ -549,4 +549,4 @@ class TestMigrateVMStrictTags(cloudstackTestCase): VirtualMachine.list(self.apiclient, id=vm.id, listall=True)[0] self.fail("VM should not be migrated") except Exception as e: - self.assertTrue("Cannot deploy VM, destination host:" in str(e)) + self.assertTrue("Cannot deploy Instance, destination host" in str(e))