From de095ba70d2a2261ffb2db7f3b8f29544939028e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 15 Dec 2023 16:25:28 +0530 Subject: [PATCH 1/6] server: fix url check for storages without a valid url (#8353) Fixes #8352 Some managed storages may not need a valid URL to be passed. We can skip check and extraction of host or path from url of such storages. --- .../com/cloud/storage/StorageManagerImpl.java | 103 ++++++++++++------ .../cloud/storage/StorageManagerImplTest.java | 49 +++++++++ 2 files changed, 116 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 4bc471c5084..04889d0b2a8 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -18,10 +18,12 @@ package com.cloud.storage; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; +import java.io.UnsupportedEncodingException; import java.math.BigDecimal; import java.math.BigInteger; import java.net.URI; import java.net.URISyntaxException; +import java.net.URLDecoder; import java.net.UnknownHostException; import java.nio.file.Files; import java.sql.PreparedStatement; @@ -132,8 +134,9 @@ import org.apache.cloudstack.storage.object.ObjectStore; import org.apache.cloudstack.storage.object.ObjectStoreEntity; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.lang3.EnumUtils; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.time.DateUtils; +import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -254,8 +257,6 @@ import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.VMInstanceDao; import com.google.common.collect.Sets; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; @Component @@ -684,12 +685,21 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return true; } - private DataStore createLocalStorage(Map poolInfos) throws ConnectionException{ + protected String getValidatedPareForLocalStorage(Object obj, String paramName) { + String result = obj == null ? null : obj.toString(); + if (StringUtils.isEmpty(result)) { + throw new InvalidParameterValueException(String.format("Invalid %s provided", paramName)); + } + return result; + } + + protected DataStore createLocalStorage(Map poolInfos) throws ConnectionException{ Object existingUuid = poolInfos.get("uuid"); if( existingUuid == null ){ poolInfos.put("uuid", UUID.randomUUID().toString()); } - String hostAddress = poolInfos.get("host").toString(); + String hostAddress = getValidatedPareForLocalStorage(poolInfos.get("host"), "host"); + String hostPath = getValidatedPareForLocalStorage(poolInfos.get("hostPath"), "path"); Host host = _hostDao.findByName(hostAddress); if( host == null ) { @@ -708,8 +718,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C StoragePoolInfo pInfo = new StoragePoolInfo(poolInfos.get("uuid").toString(), host.getPrivateIpAddress(), - poolInfos.get("hostPath").toString(), - poolInfos.get("hostPath").toString(), + hostPath, + hostPath, StoragePoolType.Filesystem, capacityBytes, 0, @@ -809,6 +819,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException { String providerName = cmd.getStorageProviderName(); Map uriParams = extractUriParamsAsMap(cmd.getUrl()); + boolean isFileScheme = "file".equals(uriParams.get("scheme")); DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(providerName); if (storeProvider == null) { @@ -822,7 +833,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C Long podId = cmd.getPodId(); Long zoneId = cmd.getZoneId(); - ScopeType scopeType = uriParams.get("scheme").toString().equals("file") ? ScopeType.HOST : ScopeType.CLUSTER; + ScopeType scopeType = ScopeType.CLUSTER; + if (isFileScheme) { + scopeType = ScopeType.HOST; + } String scope = cmd.getScope(); if (scope != null) { try { @@ -889,12 +903,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C params.put("managed", cmd.isManaged()); params.put("capacityBytes", cmd.getCapacityBytes()); params.put("capacityIops", cmd.getCapacityIops()); - params.putAll(uriParams); + if (MapUtils.isNotEmpty(uriParams)) { + params.putAll(uriParams); + } DataStoreLifeCycle lifeCycle = storeProvider.getDataStoreLifeCycle(); DataStore store = null; try { - if (params.get("scheme").toString().equals("file")) { + if (isFileScheme) { store = createLocalStorage(params); } else { store = lifeCycle.initialize(params); @@ -923,42 +939,55 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(store.getId(), DataStoreRole.Primary); } - private Map extractUriParamsAsMap(String url){ + protected Map extractUriParamsAsMap(String url) { Map uriParams = new HashMap<>(); - UriUtils.UriInfo uriInfo = UriUtils.getUriInfo(url); + UriUtils.UriInfo uriInfo; + try { + uriInfo = UriUtils.getUriInfo(url); + } catch (CloudRuntimeException cre) { + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("URI validation for url: %s failed, returning empty uri params", url)); + } + return uriParams; + } String scheme = uriInfo.getScheme(); String storageHost = uriInfo.getStorageHost(); String storagePath = uriInfo.getStoragePath(); - try { - if (scheme == null) { - throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// (or cifs://) as a prefix"); - } else if (scheme.equalsIgnoreCase("nfs")) { - if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { - throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path"); - } - } else if (scheme.equalsIgnoreCase("cifs")) { - // Don't validate against a URI encoded URI. + if (scheme == null) { + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("Scheme for url: %s is not found, returning empty uri params", url)); + } + return uriParams; + } + boolean isHostOrPathBlank = StringUtils.isAnyBlank(storagePath, storageHost); + if (scheme.equalsIgnoreCase("nfs")) { + if (isHostOrPathBlank) { + throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path"); + } + } else if (scheme.equalsIgnoreCase("cifs")) { + // Don't validate against a URI encoded URI. + try { URI cifsUri = new URI(url); String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri); if (warnMsg != null) { throw new InvalidParameterValueException(warnMsg); } - } else if (scheme.equalsIgnoreCase("sharedMountPoint")) { - if (storagePath == null) { - throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path"); - } - } else if (scheme.equalsIgnoreCase("rbd")) { - if (storagePath == null) { - throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool"); - } - } else if (scheme.equalsIgnoreCase("gluster")) { - if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { - throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume"); - } + } catch (URISyntaxException e) { + throw new InvalidParameterValueException(url + " is not a valid uri"); + } + } else if (scheme.equalsIgnoreCase("sharedMountPoint")) { + if (storagePath == null) { + throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path"); + } + } else if (scheme.equalsIgnoreCase("rbd")) { + if (storagePath == null) { + throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool"); + } + } else if (scheme.equalsIgnoreCase("gluster")) { + if (isHostOrPathBlank) { + throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume"); } - } catch (URISyntaxException e) { - throw new InvalidParameterValueException(url + " is not a valid uri"); } String hostPath = null; @@ -975,7 +1004,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C uriParams.put("host", storageHost); uriParams.put("hostPath", hostPath); uriParams.put("userInfo", uriInfo.getUserInfo()); - uriParams.put("port", uriInfo.getPort() + ""); + if (uriInfo.getPort() > 0) { + uriParams.put("port", uriInfo.getPort() + ""); + } return uriParams; } diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 478547bff1e..ba5f2baf932 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -17,12 +17,15 @@ package com.cloud.storage; import com.cloud.agent.api.StoragePoolInfo; +import com.cloud.exception.ConnectionException; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.storage.dao.VolumeDao; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.commons.collections.MapUtils; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,7 +36,9 @@ import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; @RunWith(MockitoJUnitRunner.class) public class StorageManagerImplTest { @@ -157,4 +162,48 @@ public class StorageManagerImplTest { } + @Test + public void testExtractUriParamsAsMapWithSolidFireUrl() { + String sfUrl = "MVIP=1.2.3.4;SVIP=6.7.8.9;clusterAdminUsername=admin;" + + "clusterAdminPassword=password;clusterDefaultMinIops=1000;" + + "clusterDefaultMaxIops=2000;clusterDefaultBurstIopsPercentOfMaxIops=2"; + Map uriParams = storageManagerImpl.extractUriParamsAsMap(sfUrl); + Assert.assertTrue(MapUtils.isEmpty(uriParams)); + } + + @Test + public void testExtractUriParamsAsMapWithNFSUrl() { + String scheme = "nfs"; + String host = "HOST"; + String path = "/PATH"; + String sfUrl = String.format("%s://%s%s", scheme, host, path); + Map uriParams = storageManagerImpl.extractUriParamsAsMap(sfUrl); + Assert.assertTrue(MapUtils.isNotEmpty(uriParams)); + Assert.assertEquals(scheme, uriParams.get("scheme")); + Assert.assertEquals(host, uriParams.get("host")); + Assert.assertEquals(path, uriParams.get("hostPath")); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCreateLocalStorageHostFailure() { + Map test = new HashMap<>(); + test.put("host", null); + try { + storageManagerImpl.createLocalStorage(test); + } catch (ConnectionException e) { + throw new RuntimeException(e); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void testCreateLocalStoragePathFailure() { + Map test = new HashMap<>(); + test.put("host", "HOST"); + test.put("hostPath", ""); + try { + storageManagerImpl.createLocalStorage(test); + } catch (ConnectionException e) { + throw new RuntimeException(e); + } + } } From dda672503f250233ad827d714333bd62f6bfe172 Mon Sep 17 00:00:00 2001 From: John Bampton Date: Fri, 15 Dec 2023 21:43:32 +1000 Subject: [PATCH 2/6] Remove unneeded duplicate words (#8358) This PR removes some unneeded duplicate words. --- .../engine/orchestration/NetworkOrchestrator.java | 2 +- .../api/commands/ConfigureSimulatorHAProviderState.java | 4 ++-- .../cloud/hypervisor/vmware/resource/VmwareResource.java | 2 +- .../com/cloud/storage/resource/VmwareStorageProcessor.java | 2 +- test/integration/component/test_escalations_instances.py | 6 +++--- usage/src/main/java/com/cloud/usage/UsageManagerImpl.java | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 30fd35e5f37..c6396dbdede 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -3248,7 +3248,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra // get updated state for the network network = _networksDao.findById(networkId); if (network.getState() != Network.State.Allocated && network.getState() != Network.State.Setup && !forced) { - s_logger.debug("Network is not not in the correct state to be destroyed: " + network.getState()); + s_logger.debug("Network is not in the correct state to be destroyed: " + network.getState()); return false; } diff --git a/plugins/hypervisors/simulator/src/main/java/com/cloud/api/commands/ConfigureSimulatorHAProviderState.java b/plugins/hypervisors/simulator/src/main/java/com/cloud/api/commands/ConfigureSimulatorHAProviderState.java index 1d68a184a5a..ef6f2db3391 100644 --- a/plugins/hypervisors/simulator/src/main/java/com/cloud/api/commands/ConfigureSimulatorHAProviderState.java +++ b/plugins/hypervisors/simulator/src/main/java/com/cloud/api/commands/ConfigureSimulatorHAProviderState.java @@ -69,12 +69,12 @@ public final class ConfigureSimulatorHAProviderState extends BaseCmd { private Boolean activity; @Parameter(name = ApiConstants.RECOVER, type = CommandType.BOOLEAN, - description = "Set true is haprovider for simulator host should be be recoverable", + description = "Set true is haprovider for simulator host should be recoverable", required = true) private Boolean recovery; @Parameter(name = ApiConstants.FENCE, type = CommandType.BOOLEAN, - description = "Set true is haprovider for simulator host should be be fence-able", + description = "Set true is haprovider for simulator host should be fence-able", required = true) private Boolean fenceable; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 6c457273bc3..a01ddcc81ce 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -3542,7 +3542,7 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes if (diskInfo == null) { diskInfo = diskInfoBuilder.getDiskInfoByDeviceBusName(infoInChain.getDiskDeviceBusName()); if (diskInfo != null) { - s_logger.info("Found existing disk from from chain device bus information: " + infoInChain.getDiskDeviceBusName()); + s_logger.info("Found existing disk from chain device bus information: " + infoInChain.getDiskDeviceBusName()); return diskInfo; } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 2cf1663e677..be37c16a0ca 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -2209,7 +2209,7 @@ public class VmwareStorageProcessor implements StorageProcessor { if (diskInfo == null) { diskInfo = diskInfoBuilder.getDiskInfoByDeviceBusName(infoInChain.getDiskDeviceBusName()); if (diskInfo != null) { - s_logger.info("Found existing disk from from chain device bus information: " + infoInChain.getDiskDeviceBusName()); + s_logger.info("Found existing disk from chain device bus information: " + infoInChain.getDiskDeviceBusName()); return diskInfo; } } diff --git a/test/integration/component/test_escalations_instances.py b/test/integration/component/test_escalations_instances.py index 0004cce0b35..89c4f4ce2a6 100644 --- a/test/integration/component/test_escalations_instances.py +++ b/test/integration/component/test_escalations_instances.py @@ -1244,7 +1244,7 @@ class TestListInstances(cloudstackTestCase): status[0], "Listing VM's by name and zone failed" ) - # Verifying Verifying that the size of the list is 1 + # Verifying that the size of the list is 1 self.assertEqual( 1, len(list_vms), @@ -1404,7 +1404,7 @@ class TestListInstances(cloudstackTestCase): status[0], "Listing VM's by name and zone failed" ) - # Verifying Verifying that the size of the list is 1 + # Verifying that the size of the list is 1 self.assertEqual( 1, len(list_vms), @@ -1474,7 +1474,7 @@ class TestListInstances(cloudstackTestCase): status[0], "Listing VM's by name, account and zone failed" ) - # Verifying Verifying that the size of the list is 1 + # Verifying that the size of the list is 1 self.assertEqual( 1, len(list_vms), diff --git a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java index 37906496059..c027b7074fb 100644 --- a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java +++ b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java @@ -1899,7 +1899,7 @@ public class UsageManagerImpl extends ManagerBase implements UsageManager, Runna if (usageVpnUsers.size() > 0) { s_logger.debug(String.format("We do not need to create the usage VPN user [%s] assigned to account [%s] because it already exists.", userId, accountId)); } else { - s_logger.debug(String.format("Creating VPN user user [%s] assigned to account [%s] domain [%s], zone [%s], and created at [%s]", userId, accountId, domainId, zoneId, + s_logger.debug(String.format("Creating VPN user [%s] assigned to account [%s] domain [%s], zone [%s], and created at [%s]", userId, accountId, domainId, zoneId, event.getCreateDate())); UsageVPNUserVO vpnUser = new UsageVPNUserVO(zoneId, accountId, domainId, userId, event.getResourceName(), event.getCreateDate(), null); _usageVPNUserDao.persist(vpnUser); From 127fd9d2f06ebb2938b482e95914a32ef788e57e Mon Sep 17 00:00:00 2001 From: sato03 Date: Fri, 15 Dec 2023 09:12:24 -0300 Subject: [PATCH 3/6] UI: Project column in Default View (#8287) The Default View has a projects toggle button that allows users to see which resources belong to projects, but there is nothing to indicate which project they belong to. For this reason, a project column was added if the projects button is enabled, indicating the name of the project to which the resource belongs. --------- Co-authored-by: Henrique Sato --- ui/src/components/view/ListView.vue | 4 +++ ui/src/config/section/compute.js | 44 +++++++++++++++++++++++++---- ui/src/config/section/event.js | 11 +++++++- ui/src/config/section/image.js | 6 ++++ ui/src/config/section/network.js | 41 ++++++++++++++++++++++++--- ui/src/config/section/storage.js | 11 ++++++-- ui/src/views/AutogenView.vue | 3 ++ 7 files changed, 108 insertions(+), 12 deletions(-) diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index 53c6efb321d..a61f1930f71 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -330,6 +330,10 @@ {{ text }} {{ text }} + diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index b56c8eeead9..4cb9ed8e2ba 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -61,16 +61,18 @@ export default { if (store.getters.metrics) { fields.push(...metricsFields) } - if (store.getters.userInfo.roletype === 'Admin') { fields.splice(2, 0, 'instancename') - fields.push('account') fields.push('hostname') + fields.push('account') } else if (store.getters.userInfo.roletype === 'DomainAdmin') { fields.push('account') } else { fields.push('serviceofferingname') } + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('zonename') return fields }, @@ -464,8 +466,13 @@ export default { columns: () => { const fields = ['displayname', 'state', 'name', 'type', 'current', 'parentName', 'created'] if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { - fields.push('domain') fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push('domain') + } else if (store.getters.listAllProjects) { + fields.push('project') } return fields }, @@ -536,6 +543,9 @@ export default { if (['Admin', 'DomainAdmin'].includes(store.userInfo.roletype)) { fields.push('account') } + if (store.listAllProjects) { + fields.push('project') + } if (store.apis.scaleKubernetesCluster.params.filter(x => x.name === 'autoscalingenabled').length > 0) { fields.splice(2, 0, 'autoscalingenabled') } @@ -633,7 +643,13 @@ export default { docHelp: 'adminguide/autoscale_without_netscaler.html', resourceType: 'AutoScaleVmGroup', permission: ['listAutoScaleVmGroups'], - columns: ['name', 'state', 'associatednetworkname', 'publicip', 'publicport', 'privateport', 'minmembers', 'maxmembers', 'availablevirtualmachinecount', 'account'], + columns: (store) => { + var fields = ['name', 'state', 'associatednetworkname', 'publicip', 'publicport', 'privateport', 'minmembers', 'maxmembers', 'availablevirtualmachinecount', 'account'] + if (store.listAllProjects) { + fields.push('project') + } + return fields + }, details: ['name', 'id', 'account', 'domain', 'associatednetworkname', 'associatednetworkid', 'lbruleid', 'lbprovider', 'publicip', 'publicipid', 'publicport', 'privateport', 'minmembers', 'maxmembers', 'availablevirtualmachinecount', 'interval', 'state', 'created'], related: [{ name: 'vm', @@ -737,7 +753,15 @@ export default { docHelp: 'adminguide/virtual_machines.html#changing-the-vm-name-os-or-group', resourceType: 'VMInstanceGroup', permission: ['listInstanceGroups'], - columns: ['name', 'account', 'domain'], + + columns: (store) => { + var fields = ['name', 'account'] + if (store.listAllProjects) { + fields.push('project') + } + fields.push('domain') + return fields + }, details: ['name', 'id', 'account', 'domain', 'created'], related: [{ name: 'vm', @@ -791,7 +815,12 @@ export default { var fields = ['name', 'fingerprint'] if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('domain') + } else if (store.getters.listAllProjects) { + fields.push('project') } return fields }, @@ -941,7 +970,12 @@ export default { var fields = ['name', 'type', 'description'] if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('domain') + } else if (store.getters.listAllProjects) { + fields.push('project') } return fields }, diff --git a/ui/src/config/section/event.js b/ui/src/config/section/event.js index 5f4b27b88ed..5ab87e86964 100644 --- a/ui/src/config/section/event.js +++ b/ui/src/config/section/event.js @@ -15,13 +15,22 @@ // specific language governing permissions and limitations // under the License. +import store from '@/store' + export default { name: 'event', title: 'label.events', icon: 'ScheduleOutlined', docHelp: 'adminguide/events.html', permission: ['listEvents'], - columns: ['level', 'type', 'state', 'description', 'resource', 'username', 'account', 'domain', 'created'], + columns: () => { + var fields = ['level', 'type', 'state', 'description', 'resource', 'username', 'account'] + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push(...['domain', 'created']) + return fields + }, details: ['username', 'id', 'description', 'resourcetype', 'resourceid', 'state', 'level', 'type', 'account', 'domain', 'created'], searchFilters: ['level', 'domainid', 'account', 'keyword', 'resourcetype'], related: [{ diff --git a/ui/src/config/section/image.js b/ui/src/config/section/image.js index 792e47f021f..7a5d52d1b89 100644 --- a/ui/src/config/section/image.js +++ b/ui/src/config/section/image.js @@ -47,6 +47,9 @@ export default { fields.push('size') fields.push('account') } + if (store.getters.listAllProjects) { + fields.push('project') + } if (['Admin'].includes(store.getters.userInfo.roletype)) { fields.push('templatetype') fields.push('order') @@ -220,6 +223,9 @@ export default { fields.push('size') fields.push('account') } + if (store.getters.listAllProjects) { + fields.push('project') + } if (['Admin'].includes(store.getters.userInfo.roletype)) { fields.push('order') } diff --git a/ui/src/config/section/network.js b/ui/src/config/section/network.js index 7d7313ffb25..e1a0e69b57e 100644 --- a/ui/src/config/section/network.js +++ b/ui/src/config/section/network.js @@ -34,10 +34,16 @@ export default { permission: ['listNetworks'], resourceType: 'Network', columns: () => { - var fields = ['name', 'state', 'type', 'vpcname', 'cidr', 'ip6cidr', 'broadcasturi', 'domainpath', 'account', 'zonename'] + var fields = ['name', 'state', 'type', 'vpcname', 'cidr', 'ip6cidr', 'broadcasturi', 'domainpath'] if (!isAdmin()) { fields = fields.filter(function (e) { return e !== 'broadcasturi' }) } + if (store.getters.listAllProjects) { + fields.push('project') + } else { + fields.push('account') + } + fields.push('zonename') return fields }, details: () => { @@ -197,7 +203,14 @@ export default { docHelp: 'adminguide/networking_and_traffic.html#configuring-a-virtual-private-cloud', permission: ['listVPCs'], resourceType: 'Vpc', - columns: ['name', 'state', 'displaytext', 'cidr', 'account', 'domain', 'zonename'], + columns: () => { + var fields = ['name', 'state', 'displaytext', 'cidr', 'account'] + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push(...['domain', 'zonename']) + return fields + }, details: ['name', 'id', 'displaytext', 'cidr', 'networkdomain', 'ip6routes', 'ispersistent', 'redundantvpcrouter', 'restartrequired', 'zonename', 'account', 'domain', 'dns1', 'dns2', 'ip6dns1', 'ip6dns2', 'publicmtu'], searchFilters: ['name', 'zoneid', 'domainid', 'account', 'tags'], related: [{ @@ -334,10 +347,16 @@ export default { if (store.getters.userInfo.roletype === 'Admin') { fields.splice(2, 0, 'instancename') fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('domain') fields.push('hostname') } else if (store.getters.userInfo.roletype === 'DomainAdmin') { fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } } else { fields.push('serviceofferingname') } @@ -730,7 +749,14 @@ export default { docHelp: 'adminguide/networking_and_traffic.html#reserving-public-ip-addresses-and-vlans-for-accounts', permission: ['listPublicIpAddresses'], resourceType: 'PublicIpAddress', - columns: ['ipaddress', 'state', 'associatednetworkname', 'vpcname', 'virtualmachinename', 'allocated', 'account', 'domain', 'zonename'], + columns: () => { + var fields = ['ipaddress', 'state', 'associatednetworkname', 'vpcname', 'virtualmachinename', 'allocated', 'account'] + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push(...['domain', 'zonename']) + return fields + }, details: ['ipaddress', 'id', 'associatednetworkname', 'virtualmachinename', 'networkid', 'issourcenat', 'isstaticnat', 'virtualmachinename', 'vmipaddress', 'vlan', 'allocated', 'account', 'domain', 'zonename'], filters: ['allocated', 'reserved', 'free'], component: shallowRef(() => import('@/views/network/PublicIpResource.vue')), @@ -1120,7 +1146,14 @@ export default { title: 'label.vpncustomergatewayid', icon: 'lock-outlined', permission: ['listVpnCustomerGateways'], - columns: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'account', 'domain'], + columns: () => { + var fields = ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'account'] + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push('domain') + return fields + }, details: ['name', 'id', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap', 'account', 'domain'], searchFilters: ['keyword', 'domainid', 'account'], resourceType: 'VPNCustomerGateway', diff --git a/ui/src/config/section/storage.js b/ui/src/config/section/storage.js index 0fbb930e750..a096067b135 100644 --- a/ui/src/config/section/storage.js +++ b/ui/src/config/section/storage.js @@ -49,13 +49,15 @@ export default { if (store.getters.metrics) { fields.push(...metricsFields) } - if (store.getters.userInfo.roletype === 'Admin') { - fields.push('account') fields.push('storage') + fields.push('account') } else if (store.getters.userInfo.roletype === 'DomainAdmin') { fields.push('account') } + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('zonename') return fields @@ -320,7 +322,12 @@ export default { var fields = ['name', 'state', 'volumename', 'intervaltype', 'physicalsize', 'created'] if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { fields.push('account') + if (store.getters.listAllProjects) { + fields.push('project') + } fields.push('domain') + } else if (store.getters.listAllProjects) { + fields.push('project') } fields.push('zonename') return fields diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index 6c9c13dbf77..bf1c42d4c05 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -879,6 +879,9 @@ export default { this.$store.getters.customColumns[this.$store.getters.userInfo.id][this.$route.path] = this.selectedColumns } else { this.selectedColumns = this.$store.getters.customColumns[this.$store.getters.userInfo.id][this.$route.path] || this.selectedColumns + if (this.$store.getters.listAllProjects && !this.projectView) { + this.selectedColumns.push('project') + } this.updateSelectedColumns() } } From af872224d6a4cfff16486f09ab522e5dfeb1c9dc Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Sat, 16 Dec 2023 16:18:20 +0530 Subject: [PATCH 4/6] =?UTF-8?q?README:=20that=20time=20of=20the=20year!=20?= =?UTF-8?q?=F0=9F=8E=84=20(#8365)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rohit Yadav --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e193913612f..7e91acb3348 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Apache CloudStack [![Build Status](https://github.com/apache/cloudstack/actions/workflows/build.yml/badge.svg?branch=main)](https://github.com/apache/cloudstack/actions/workflows/build.yml) [![UI Build](https://github.com/apache/cloudstack/actions/workflows/ui.yml/badge.svg)](https://github.com/apache/cloudstack/actions/workflows/ui.yml) [![License Check](https://github.com/apache/cloudstack/actions/workflows/rat.yml/badge.svg?branch=main)](https://github.com/apache/cloudstack/actions/workflows/rat.yml) [![Simulator CI](https://github.com/apache/cloudstack/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/apache/cloudstack/actions/workflows/ci.yml) [![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=apache_cloudstack&metric=alert_status)](https://sonarcloud.io/dashboard?id=apache_cloudstack) [![codecov](https://codecov.io/gh/apache/cloudstack/branch/main/graph/badge.svg)](https://codecov.io/gh/apache/cloudstack) -[![Apache CloudStack](tools/logo/apache_cloudstack.png)](https://cloudstack.apache.org/) +[![Apache CloudStack](tools/logo/acsxmas.jpg)](https://cloudstack.apache.org/) Apache CloudStack is open source software designed to deploy and manage large networks of virtual machines, as a highly available, highly scalable From 16d45f731d7ae7d49287d33f21c2eb7ae141707e Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Mon, 18 Dec 2023 03:36:31 -0300 Subject: [PATCH 5/6] Save the account which deliberately removed a public IP from quarantine (#8339) When a public IP gets removed from quarantine, the removal reason gets saved to the database; however, it may also be useful for operators to know who removed the public IP from quarantine. For that reason, this PR extends the public IP quarantine feature so that the account that deliberately removed an IP from quarantine also gets saved to the database. --- .../com/cloud/network/PublicIpQuarantine.java | 2 + .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/response/IpQuarantineResponse.java | 12 ++++ .../network/vo/PublicIpQuarantineVO.java | 12 ++++ .../upgrade/dao/Upgrade41810to41900.java | 4 ++ .../META-INF/db/schema-41810to41900.sql | 3 + .../java/com/cloud/api/ApiResponseHelper.java | 4 ++ .../cloud/network/IpAddressManagerImpl.java | 2 + .../com/cloud/api/ApiResponseHelperTest.java | 56 +++++++++++++++++++ .../cloud/network/IpAddressManagerTest.java | 27 +++++++++ 10 files changed, 123 insertions(+) diff --git a/api/src/main/java/com/cloud/network/PublicIpQuarantine.java b/api/src/main/java/com/cloud/network/PublicIpQuarantine.java index d1ec98afe46..625a1bbbe50 100644 --- a/api/src/main/java/com/cloud/network/PublicIpQuarantine.java +++ b/api/src/main/java/com/cloud/network/PublicIpQuarantine.java @@ -30,6 +30,8 @@ public interface PublicIpQuarantine extends InternalIdentity, Identity { String getRemovalReason(); + Long getRemoverAccountId(); + Date getRemoved(); Date getCreated(); diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index d7767721667..3ae0f319189 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -803,6 +803,7 @@ public class ApiConstants { public static final String IPSEC_PSK = "ipsecpsk"; public static final String GUEST_IP = "guestip"; public static final String REMOVED = "removed"; + public static final String REMOVER_ACCOUNT_ID = "removeraccountid"; public static final String REMOVAL_REASON = "removalreason"; public static final String COMPLETED = "completed"; public static final String IKE_VERSION = "ikeversion"; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java index 55720296315..c93f259382c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java @@ -60,6 +60,10 @@ public class IpQuarantineResponse extends BaseResponse { @Param(description = "The reason for removing the IP from quarantine prematurely.") private String removalReason; + @SerializedName(ApiConstants.REMOVER_ACCOUNT_ID) + @Param(description = "ID of the account that removed the IP from quarantine.") + private String removerAccountId; + public IpQuarantineResponse() { super("quarantinedips"); } @@ -127,4 +131,12 @@ public class IpQuarantineResponse extends BaseResponse { public void setRemovalReason(String removalReason) { this.removalReason = removalReason; } + + public String getRemoverAccountId() { + return removerAccountId; + } + + public void setRemoverAccountId(String removerAccountId) { + this.removerAccountId = removerAccountId; + } } diff --git a/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java b/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java index 56d167a0060..89e02610bd2 100644 --- a/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java +++ b/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java @@ -63,6 +63,9 @@ public class PublicIpQuarantineVO implements PublicIpQuarantine { @Column(name = "removal_reason") private String removalReason = null; + @Column(name = "remover_account_id") + private Long removerAccountId = null; + public PublicIpQuarantineVO() { } @@ -98,6 +101,11 @@ public class PublicIpQuarantineVO implements PublicIpQuarantine { return removalReason; } + @Override + public Long getRemoverAccountId() { + return this.removerAccountId; + } + @Override public String getUuid() { return uuid; @@ -111,6 +119,10 @@ public class PublicIpQuarantineVO implements PublicIpQuarantine { this.removalReason = removalReason; } + public void setRemoverAccountId(Long removerAccountId) { + this.removerAccountId = removerAccountId; + } + @Override public Date getRemoved() { return removed; diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java index bdfe58cbf89..13e30c0f6e2 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java @@ -77,6 +77,7 @@ public class Upgrade41810to41900 implements DbUpgrade, DbUpgradeSystemVmTemplate decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn); migrateBackupDates(conn); addIndexes(conn); + addRemoverAccountIdForeignKeyToQuarantinedIps(conn); } @Override @@ -262,4 +263,7 @@ public class Upgrade41810to41900 implements DbUpgrade, DbUpgradeSystemVmTemplate DbUpgradeUtils.addIndexIfNeeded(conn, "event", "resource_type", "resource_id"); } + private void addRemoverAccountIdForeignKeyToQuarantinedIps(Connection conn) { + DbUpgradeUtils.addForeignKey(conn, "quarantined_ips", "remover_account_id", "account", "id"); + } } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql index 27170fcac14..308d48a311c 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql @@ -314,3 +314,6 @@ CREATE TABLE `cloud_usage`.`bucket_statistics` ( `size` bigint unsigned COMMENT 'total size of bucket objects', PRIMARY KEY(`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +-- Add remover account ID to quarantined IPs table. +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.quarantined_ips', 'remover_account_id', 'bigint(20) unsigned DEFAULT NULL COMMENT "ID of the account that removed the IP from quarantine, foreign key to `account` table"'); diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 5424a36cdd8..e2b72f6175c 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -5200,6 +5200,10 @@ public class ApiResponseHelper implements ResponseGenerator { quarantinedIpsResponse.setRemoved(quarantinedIp.getRemoved()); quarantinedIpsResponse.setEndDate(quarantinedIp.getEndDate()); quarantinedIpsResponse.setRemovalReason(quarantinedIp.getRemovalReason()); + if (quarantinedIp.getRemoverAccountId() != null) { + Account removerAccount = _accountMgr.getAccount(quarantinedIp.getRemoverAccountId()); + quarantinedIpsResponse.setRemoverAccountId(removerAccount.getUuid()); + } quarantinedIpsResponse.setResponseName("quarantinedip"); return quarantinedIpsResponse; diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 75ea572491e..b5908935350 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -2471,9 +2471,11 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findById(quarantineProcessId); Ip ipAddress = _ipAddressDao.findById(publicIpQuarantineVO.getPublicIpAddressId()).getAddress(); Date removedDate = new Date(); + Long removerAccountId = CallContext.current().getCallingAccountId(); publicIpQuarantineVO.setRemoved(removedDate); publicIpQuarantineVO.setRemovalReason(removalReason); + publicIpQuarantineVO.setRemoverAccountId(removerAccountId); s_logger.debug(String.format("Removing public IP Address [%s] from quarantine by updating the removed date to [%s].", ipAddress, removedDate)); publicIpQuarantineDao.persist(publicIpQuarantineVO); diff --git a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java index f681669c789..1bea3ac78f3 100644 --- a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java +++ b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java @@ -17,10 +17,12 @@ package com.cloud.api; import com.cloud.domain.DomainVO; +import com.cloud.network.PublicIpQuarantine; import com.cloud.network.as.AutoScaleVmGroup; import com.cloud.network.as.AutoScaleVmGroupVO; import com.cloud.network.as.AutoScaleVmProfileVO; import com.cloud.network.as.dao.AutoScaleVmGroupVmMapDao; +import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerVO; import com.cloud.network.dao.NetworkServiceMapDao; @@ -41,6 +43,7 @@ import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.response.AutoScaleVmGroupResponse; import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse; import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse; +import org.apache.cloudstack.api.response.IpQuarantineResponse; import org.apache.cloudstack.api.response.NicSecondaryIpResponse; import org.apache.cloudstack.api.response.UnmanagedInstanceResponse; import org.apache.cloudstack.api.response.UsageRecordResponse; @@ -97,6 +100,9 @@ public class ApiResponseHelperTest { @Mock UserDataDao userDataDaoMock; + @Mock + IPAddressDao ipAddressDaoMock; + @Spy @InjectMocks ApiResponseHelper apiResponseHelper = new ApiResponseHelper(); @@ -396,4 +402,54 @@ public class ApiResponseHelperTest { Assert.assertEquals(1, response.getDisks().size()); Assert.assertEquals(1, response.getNics().size()); } + + @Test + public void createQuarantinedIpsResponseTestReturnsObject() { + String quarantinedIpUuid = "quarantined_ip_uuid"; + Long previousOwnerId = 300L; + String previousOwnerUuid = "previous_owner_uuid"; + String previousOwnerName = "previous_owner_name"; + Long removerAccountId = 400L; + String removerAccountUuid = "remover_account_uuid"; + Long publicIpAddressId = 500L; + String publicIpAddress = "1.2.3.4"; + Date created = new Date(599L); + Date removed = new Date(600L); + Date endDate = new Date(601L); + String removalReason = "removalReason"; + + PublicIpQuarantine quarantinedIpMock = Mockito.mock(PublicIpQuarantine.class); + IPAddressVO ipAddressVoMock = Mockito.mock(IPAddressVO.class); + Account previousOwner = Mockito.mock(Account.class); + Account removerAccount = Mockito.mock(Account.class); + + Mockito.when(quarantinedIpMock.getUuid()).thenReturn(quarantinedIpUuid); + Mockito.when(quarantinedIpMock.getPreviousOwnerId()).thenReturn(previousOwnerId); + Mockito.when(quarantinedIpMock.getPublicIpAddressId()).thenReturn(publicIpAddressId); + Mockito.doReturn(ipAddressVoMock).when(ipAddressDaoMock).findById(publicIpAddressId); + Mockito.when(ipAddressVoMock.getAddress()).thenReturn(new Ip(publicIpAddress)); + Mockito.doReturn(previousOwner).when(accountManagerMock).getAccount(previousOwnerId); + Mockito.when(previousOwner.getUuid()).thenReturn(previousOwnerUuid); + Mockito.when(previousOwner.getName()).thenReturn(previousOwnerName); + Mockito.when(quarantinedIpMock.getCreated()).thenReturn(created); + Mockito.when(quarantinedIpMock.getRemoved()).thenReturn(removed); + Mockito.when(quarantinedIpMock.getEndDate()).thenReturn(endDate); + Mockito.when(quarantinedIpMock.getRemovalReason()).thenReturn(removalReason); + Mockito.when(quarantinedIpMock.getRemoverAccountId()).thenReturn(removerAccountId); + Mockito.when(removerAccount.getUuid()).thenReturn(removerAccountUuid); + Mockito.doReturn(removerAccount).when(accountManagerMock).getAccount(removerAccountId); + + IpQuarantineResponse result = apiResponseHelper.createQuarantinedIpsResponse(quarantinedIpMock); + + Assert.assertEquals(quarantinedIpUuid, result.getId()); + Assert.assertEquals(publicIpAddress, result.getPublicIpAddress()); + Assert.assertEquals(previousOwnerUuid, result.getPreviousOwnerId()); + Assert.assertEquals(previousOwnerName, result.getPreviousOwnerName()); + Assert.assertEquals(created, result.getCreated()); + Assert.assertEquals(removed, result.getRemoved()); + Assert.assertEquals(endDate, result.getEndDate()); + Assert.assertEquals(removalReason, result.getRemovalReason()); + Assert.assertEquals(removerAccountUuid, result.getRemoverAccountId()); + Assert.assertEquals("quarantinedip", result.getResponseName()); + } } diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index 935fb4e8c3b..5b8399ad000 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -36,12 +36,14 @@ import com.cloud.network.dao.PublicIpQuarantineDao; import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.user.Account; import com.cloud.user.AccountManager; +import org.apache.cloudstack.context.CallContext; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; @@ -397,6 +399,31 @@ public class IpAddressManagerTest { Assert.assertFalse(result); } + @Test + public void removePublicIpAddressFromQuarantineTestPersistsObject() { + Long quarantineProcessId = 100L; + Long publicAddressId = 200L; + Long callingAccountId = 300L; + String removalReason = "removalReason"; + + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + Mockito.doReturn(publicIpQuarantineVOMock).when(publicIpQuarantineDaoMock).findById(quarantineProcessId); + Mockito.when(publicIpQuarantineVOMock.getPublicIpAddressId()).thenReturn(publicAddressId); + Mockito.doReturn(ipAddressVO).when(ipAddressDao).findById(publicAddressId); + + CallContext callContextMock = Mockito.mock(CallContext.class); + Mockito.when(callContextMock.getCallingAccountId()).thenReturn(callingAccountId); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + + ipAddressManager.removePublicIpAddressFromQuarantine(quarantineProcessId, removalReason); + + Mockito.verify(publicIpQuarantineVOMock).setRemoved(Mockito.any()); + Mockito.verify(publicIpQuarantineVOMock).setRemovalReason(removalReason); + Mockito.verify(publicIpQuarantineVOMock).setRemoverAccountId(callingAccountId); + Mockito.verify(publicIpQuarantineDaoMock).persist(publicIpQuarantineVOMock); + } + } + @Test public void updateSourceNatIpAddress() throws Exception { IPAddressVO requestedIp = Mockito.mock(IPAddressVO.class); From 33e2a4dd6635798f98d4726406ed1af4c00a4cc5 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 18 Dec 2023 07:38:51 +0100 Subject: [PATCH 6/6] VPC: update default network offering for vpc tier to conserve_mode=1 (#8309) This PR updates the conserve mode of default vpc tier offering to conserve_mode=1 so we can create both port forwarding and load balancing rules on a public IP in vpc tiers. This fixes #8313 --- .../main/resources/META-INF/db/schema-41810to41900.sql | 2 ++ .../java/com/cloud/network/vpc/VpcManagerImpl.java | 4 ++-- test/integration/component/test_vpc_network.py | 10 +++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql index 308d48a311c..15307353c3f 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql @@ -21,6 +21,8 @@ ALTER TABLE `cloud`.`mshost` MODIFY COLUMN `state` varchar(25); +UPDATE `cloud`.`network_offerings` SET conserve_mode=1 WHERE name='DefaultIsolatedNetworkOfferingForVpcNetworks'; + -- Invalidate existing console_session records UPDATE `cloud`.`console_session` SET removed=now(); -- Modify acquired column in console_session to datetime type diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 1f99d164625..341a3b81b42 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1809,9 +1809,9 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis * ("No redunant router support when network belnogs to VPC"); } */ - // 4) Conserve mode should be off + // 4) Conserve mode should be off in older versions if (guestNtwkOff.isConserveMode()) { - throw new InvalidParameterValueException("Only networks with conserve mode Off can belong to VPC"); + s_logger.info("Creating a network with conserve mode in VPC"); } // 5) If Netscaler is LB provider make sure it is in dedicated mode diff --git a/test/integration/component/test_vpc_network.py b/test/integration/component/test_vpc_network.py index 9a313e23094..db436bbd398 100644 --- a/test/integration/component/test_vpc_network.py +++ b/test/integration/component/test_vpc_network.py @@ -1001,23 +1001,23 @@ class TestVPCNetwork(cloudstackTestCase): # 1. Create a network offering with guest type=Isolated that has all # supported Services(Vpn,dhcpdns,UserData, SourceNat,Static NAT,LB # and PF,LB,NetworkAcl ) provided by VPCVR and conserve mode is ON - # 2. Create offering fails since Conserve mode ON isn't allowed within - # VPC + # 2. Create offering should succeed since Conserve mode ON is allowed within + # VPC since https://github.com/apache/cloudstack/pull/8309 # 3. Repeat test for offering which has Netscaler as external LB # provider """ self.debug("Creating network offering with conserve mode = ON") - with self.assertRaises(Exception): + try: nw = NetworkOffering.create( self.apiclient, self.services[value], conservemode=True ) self.cleanup.append(nw) - self.debug( - "Network creation failed as VPC support nw with conserve mode OFF") + except Exception as e: + self.warn("Network creation failed in VPC with conserve mode ON") return