This is an automated email from the ASF dual-hosted git repository. weizhou pushed a commit to branch 4.18-vm-autoscaling in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 1e4aaa538cd71ad7ebec2bc8e86828e528bf73f4 Author: Wei Zhou <[email protected]> AuthorDate: Tue Nov 8 10:43:05 2022 +0100 AS: check AS group name (a-zA-Z0-9 and hyphen only) --- .../com/cloud/network/as/AutoScaleManagerImpl.java | 28 ++++++++-- .../cloud/network/as/AutoScaleManagerImplTest.java | 60 +++++++++++++++++++--- ui/public/locales/en.json | 1 + ui/src/views/compute/CreateAutoScaleVmGroup.vue | 9 ++++ 4 files changed, 87 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index 477b16e5836..6c6a1fb07d6 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -282,7 +282,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage private static final List<String> supportedDeployParams = Arrays.asList(PARAM_ROOT_DISK_SIZE, PARAM_DISK_OFFERING_ID, PARAM_DISK_SIZE, PARAM_SECURITY_GROUP_IDS, PARAM_OVERRIDE_DISK_OFFERING_ID, PARAM_SSH_KEYPAIRS, PARAM_AFFINITY_GROUP_IDS, PARAM_NETWORK_IDS); - private static final String VM_HOSTNAME_PREFIX = "autoScaleVm"; + protected static final String VM_HOSTNAME_PREFIX = "autoScaleVm-"; + protected static final int VM_HOSTNAME_RANDOM_SUFFIX_LENGTH = 6; private static final Long DEFAULT_HOST_ID = -1L; @@ -1168,6 +1169,9 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage @DB protected AutoScaleVmGroupVO checkValidityAndPersist(final AutoScaleVmGroupVO vmGroup, final List<Long> passedScaleUpPolicyIds, final List<Long> passedScaleDownPolicyIds) { + + checkAutoScaleVmGroupName(vmGroup.getName()); + int minMembers = vmGroup.getMinMembers(); int maxMembers = vmGroup.getMaxMembers(); int interval = vmGroup.getInterval(); @@ -1882,8 +1886,26 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage } private String getNextVmHostName(AutoScaleVmGroupVO asGroup) { - return VM_HOSTNAME_PREFIX + "-" + asGroup.getName() + "-" + asGroup.getNextVmSeq() + "-" + - RandomStringUtils.random(6, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); + String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + + RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); + // Truncate vm group name because max length of vm name is 63 + int subStringLength = Math.min(asGroup.getName().length(), 63 - VM_HOSTNAME_PREFIX.length() - vmHostNameSuffix.length()); + return VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; + } + + private void checkAutoScaleVmGroupName(String groupName) { + String errorMessage = ""; + if (groupName == null || groupName.length() > 255 || groupName.length() < 1) { + errorMessage = "AutoScale Vm Group name must be between 1 and 255 characters long"; + } else if (!groupName.toLowerCase().matches("[a-z0-9-]*")) { + errorMessage = "AutoScale Vm Group name may contain only the ASCII letters 'a' through 'z' (in a case-insensitive manner), " + + "the digits '0' through '9' and the hyphen ('-')"; + } + if (StringUtils.isNotBlank(errorMessage)) { + s_logger.warn(errorMessage); + throw new InvalidParameterValueException("Invalid AutoScale VM group name. It can contain the ASCII letters 'a' through 'z', " + + "'A' through 'Z', the digits '0' through '9' and the hyphen ('-'), must be between 1 and 255 characters long."); + } } private boolean startNewVM(long vmId) { diff --git a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java index b494c02fe98..ba58c7b5157 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -153,8 +153,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.matches; import static org.mockito.ArgumentMatchers.nullable; - import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; @@ -294,6 +294,9 @@ public class AutoScaleManagerImplTest { private static final Long vmGroupId = 22L; private static final String vmGroupName = "test-vmgroup"; + private static final String vmGroupNameWithMaxLength = "12345678901234567890123456789012345678901234567890" + + "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345"; private static final String vmGroupUuid = "2222-2222-1111"; private static final int minMembers = 2; private static final int maxMembers = 3; @@ -952,12 +955,13 @@ public class AutoScaleManagerImplTest { UpdateAutoScaleVmGroupCmd cmd = new UpdateAutoScaleVmGroupCmd(); ReflectionTestUtils.setField(cmd, "id", vmGroupId); - ReflectionTestUtils.setField(cmd, "name", vmGroupName + "-new"); + ReflectionTestUtils.setField(cmd, "name", vmGroupNameWithMaxLength); ReflectionTestUtils.setField(cmd, "minMembers", minMembers + 1); ReflectionTestUtils.setField(cmd, "maxMembers", maxMembers + 1); ReflectionTestUtils.setField(cmd, "interval", interval); when(autoScaleVmGroupDao.findById(vmGroupId)).thenReturn(asVmGroupMock); + when(asVmGroupMock.getName()).thenReturn(vmGroupNameWithMaxLength); when(asVmGroupMock.getInterval()).thenReturn(interval); when(asVmGroupMock.getMaxMembers()).thenReturn(maxMembers); when(asVmGroupMock.getMinMembers()).thenReturn(minMembers); @@ -992,7 +996,7 @@ public class AutoScaleManagerImplTest { Assert.assertEquals(asVmGroupMock, vmGroup); - Mockito.verify(asVmGroupMock).setName(vmGroupName + "-new"); + Mockito.verify(asVmGroupMock).setName(vmGroupNameWithMaxLength); Mockito.verify(asVmGroupMock).setMinMembers(minMembers + 1); Mockito.verify(asVmGroupMock).setMaxMembers(maxMembers + 1); Mockito.verify(asVmGroupMock).setInterval(interval); @@ -1003,7 +1007,7 @@ public class AutoScaleManagerImplTest { UpdateAutoScaleVmGroupCmd cmd = new UpdateAutoScaleVmGroupCmd(); ReflectionTestUtils.setField(cmd, "id", vmGroupId); - ReflectionTestUtils.setField(cmd, "name", vmGroupName + "new"); + ReflectionTestUtils.setField(cmd, "name", vmGroupName + "-new"); ReflectionTestUtils.setField(cmd, "minMembers", minMembers + 1); ReflectionTestUtils.setField(cmd, "maxMembers", maxMembers + 1); ReflectionTestUtils.setField(cmd, "interval", interval); @@ -1018,6 +1022,34 @@ public class AutoScaleManagerImplTest { AutoScaleVmGroup vmGroup = autoScaleManagerImplSpy.updateAutoScaleVmGroup(cmd); } + @Test(expected = InvalidParameterValueException.class) + public void testUpdateAutoScaleVmGroupFail2() { + UpdateAutoScaleVmGroupCmd cmd = new UpdateAutoScaleVmGroupCmd(); + + ReflectionTestUtils.setField(cmd, "id", vmGroupId); + String newName = vmGroupName + "!"; + ReflectionTestUtils.setField(cmd, "name", newName); + when(asVmGroupMock.getName()).thenReturn(newName); + + when(autoScaleVmGroupDao.findById(vmGroupId)).thenReturn(asVmGroupMock); + + AutoScaleVmGroup vmGroup = autoScaleManagerImplSpy.updateAutoScaleVmGroup(cmd); + } + + @Test(expected = InvalidParameterValueException.class) + public void testUpdateAutoScaleVmGroupFail3() { + UpdateAutoScaleVmGroupCmd cmd = new UpdateAutoScaleVmGroupCmd(); + + ReflectionTestUtils.setField(cmd, "id", vmGroupId); + String newName = vmGroupNameWithMaxLength + "6"; + ReflectionTestUtils.setField(cmd, "name", newName); + when(asVmGroupMock.getName()).thenReturn(newName); + + when(autoScaleVmGroupDao.findById(vmGroupId)).thenReturn(asVmGroupMock); + + AutoScaleVmGroup vmGroup = autoScaleManagerImplSpy.updateAutoScaleVmGroup(cmd); + } + @Test public void testEnableAutoScaleVmGroupInEnabledState() { when(autoScaleVmGroupDao.findById(vmGroupId)).thenReturn(asVmGroupMock); @@ -1144,6 +1176,7 @@ public class AutoScaleManagerImplTest { when(asVmGroupMock.getProfileId()).thenReturn(vmProfileId); when(asVmGroupMock.getLoadBalancerId()).thenReturn(loadBalancerId); when(asVmGroupMock.getNextVmSeq()).thenReturn(nextVmSeq); + when(asVmGroupMock.getName()).thenReturn(vmGroupName); when(autoScaleVmProfileDao.findById(vmProfileId)).thenReturn(asVmProfileMock); when(asVmProfileMock.getTemplateId()).thenReturn(templateId); @@ -1174,7 +1207,10 @@ public class AutoScaleManagerImplTest { Assert.assertEquals((long) virtualMachineId, result); - Mockito.verify(userVmService).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(), + String vmHostNamePattern = autoScaleManagerImplSpy.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Mockito.verify(userVmService).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), + matches(vmHostNamePattern), matches(vmHostNamePattern), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), eq(true), any(), any(), any(), any(), any(), any(), any(), eq(true), any()); Mockito.verify(asVmGroupMock).setNextVmSeq(nextVmSeq + 1); @@ -1185,6 +1221,7 @@ public class AutoScaleManagerImplTest { when(asVmGroupMock.getProfileId()).thenReturn(vmProfileId); when(asVmGroupMock.getLoadBalancerId()).thenReturn(loadBalancerId); when(asVmGroupMock.getNextVmSeq()).thenReturn(nextVmSeq + 1); + when(asVmGroupMock.getName()).thenReturn(vmGroupName); when(autoScaleVmProfileDao.findById(vmProfileId)).thenReturn(asVmProfileMock); when(asVmProfileMock.getTemplateId()).thenReturn(templateId); @@ -1216,8 +1253,11 @@ public class AutoScaleManagerImplTest { Assert.assertEquals((long) virtualMachineId, result); - Mockito.verify(userVmService).createAdvancedSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(), - any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), + String vmHostNamePattern = autoScaleManagerImplSpy.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Mockito.verify(userVmService).createAdvancedSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), + matches(vmHostNamePattern), matches(vmHostNamePattern), + any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), eq(true), any(), any()); Mockito.verify(asVmGroupMock).setNextVmSeq(nextVmSeq + 2); } @@ -1227,6 +1267,7 @@ public class AutoScaleManagerImplTest { when(asVmGroupMock.getProfileId()).thenReturn(vmProfileId); when(asVmGroupMock.getLoadBalancerId()).thenReturn(loadBalancerId); when(asVmGroupMock.getNextVmSeq()).thenReturn(nextVmSeq + 2); + when(asVmGroupMock.getName()).thenReturn(vmGroupNameWithMaxLength); when(autoScaleVmProfileDao.findById(vmProfileId)).thenReturn(asVmProfileMock); when(asVmProfileMock.getTemplateId()).thenReturn(templateId); @@ -1258,7 +1299,10 @@ public class AutoScaleManagerImplTest { Assert.assertEquals((long) virtualMachineId, result); - Mockito.verify(userVmService).createAdvancedVirtualMachine(any(), any(), any(), any(), any(), any(), any(), + String vmHostNamePattern = autoScaleManagerImplSpy.VM_HOSTNAME_PREFIX + vmGroupNameWithMaxLength.substring(0, 41) + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Mockito.verify(userVmService).createAdvancedVirtualMachine(any(), any(), any(), any(), any(), + matches(vmHostNamePattern), matches(vmHostNamePattern), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), eq(true), any(), any(), any(), any(), any(), any(), any(), eq(true), any(), any()); Mockito.verify(asVmGroupMock).setNextVmSeq(nextVmSeq + 3); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 1742e7c5c40..be87ccb031e 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -2314,6 +2314,7 @@ "message.error.internallb.instance.port": "Please specify a instance port.", "message.error.internallb.name": "Please specify a name for the internal LB.", "message.error.internallb.source.port": "Please specify a source port.", +"message.error.invalid.autoscale.vmgroup.name": "Invalid AutoScale VM group name. It can contain the ASCII letters 'a' through 'z', 'A' through 'Z', the digits '0' through '9' and the hyphen ('-'), must be between 1 and 255 characters long.", "message.error.ip.range": "Please enter valid range.", "message.error.ipv4.address": "Please enter a valid IPv4 address.", "message.error.ipv4.dns1": "Please enter IpV4 DNS 1.", diff --git a/ui/src/views/compute/CreateAutoScaleVmGroup.vue b/ui/src/views/compute/CreateAutoScaleVmGroup.vue index fc2cb3bca50..432a98e5466 100644 --- a/ui/src/views/compute/CreateAutoScaleVmGroup.vue +++ b/ui/src/views/compute/CreateAutoScaleVmGroup.vue @@ -2225,6 +2225,15 @@ export default { return } + const regex = /^([a-zA-Z0-9-]){0,255}$/ + if (!values.name || values.name.length === 0 || !regex.test(values.name)) { + this.$notification.error({ + message: this.$t('message.request.failed'), + description: this.$t('message.error.invalid.autoscale.vmgroup.name') + }) + return + } + if (this.scaleUpPolicies.length === 0) { this.$notification.error({ message: this.$t('message.request.failed'),
