From 150e7e0b16a3d01c62303187d3428f4f3bd6c44a Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 6 Aug 2024 12:54:18 -0300 Subject: [PATCH 1/3] Fix imports --- .../cloud/user/AccountManagerImplTest.java | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 5ec453af485..f0a5af2bd87 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -61,34 +61,6 @@ import com.cloud.vm.UserVmManagerImpl; import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.snapshot.VMSnapshotVO; -import org.apache.cloudstack.acl.ControlledEntity; -import org.apache.cloudstack.acl.SecurityChecker.AccessType; -import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; -import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; -import org.apache.cloudstack.api.response.UserTwoFactorAuthenticationSetupResponse; -import org.apache.cloudstack.auth.UserAuthenticator; -import org.apache.cloudstack.auth.UserAuthenticator.ActionOnFailedAuthentication; -import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.InOrder; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.mockito.ArgumentMatchers.nullable; @RunWith(MockitoJUnitRunner.class) public class AccountManagerImplTest extends AccountManagetImplTestBase { From 21f3fde7b404f8195b54e622b84c17d33f54639e Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Wed, 7 Aug 2024 09:53:35 +0200 Subject: [PATCH 2/3] libvirtstorageadaptor: better handle failed libvirt storagepool destroy (#9390) If the libvirt mount point is still busy and can't be unmounted right now, it was waited 5 seconds and an plain unmount was tried, without cleaning up the libvirt storagepool. This kept libvirt thinking the storagepool is active and mounted (which it wasn't). Now after the plain unmount call, also the libvirt storagepool will be destroyed. --- .../kvm/storage/LibvirtStorageAdaptor.java | 56 +++++++++++++------ 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index e26b2c51790..df6c047e7e2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -766,26 +766,53 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } } + private boolean destroyStoragePool(Connect conn, String uuid) throws LibvirtException { + StoragePool sp; + try { + sp = conn.storagePoolLookupByUUIDString(uuid); + } catch (LibvirtException exc) { + s_logger.warn("Storage pool " + uuid + " doesn't exist in libvirt. Assuming it is already removed"); + s_logger.warn(exc.getStackTrace()); + return true; + } + + if (sp != null) { + if (sp.isPersistent() == 1) { + sp.destroy(); + sp.undefine(); + } else { + sp.destroy(); + } + sp.free(); + + return true; + } else { + s_logger.warn("Storage pool " + uuid + " doesn't exist in libvirt. Assuming it is already removed"); + return false; + } + } + + private boolean destroyStoragePoolHandleException(Connect conn, String uuid) + { + try { + return destroyStoragePool(conn, uuid); + } catch (LibvirtException e) { + s_logger.error(String.format("Failed to destroy libvirt pool %s: %s", uuid, e)); + } + return false; + } + @Override public boolean deleteStoragePool(String uuid) { s_logger.info("Attempting to remove storage pool " + uuid + " from libvirt"); - Connect conn = null; + Connect conn; try { conn = LibvirtConnection.getConnection(); } catch (LibvirtException e) { throw new CloudRuntimeException(e.toString()); } - StoragePool sp = null; Secret s = null; - - try { - sp = conn.storagePoolLookupByUUIDString(uuid); - } catch (LibvirtException e) { - s_logger.warn("Storage pool " + uuid + " doesn't exist in libvirt. Assuming it is already removed"); - return true; - } - /* * Some storage pools, like RBD also have 'secret' information stored in libvirt * Destroy them if they exist @@ -797,13 +824,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } try { - if (sp.isPersistent() == 1) { - sp.destroy(); - sp.undefine(); - } else { - sp.destroy(); - } - sp.free(); + destroyStoragePool(conn, uuid); if (s != null) { s.undefine(); s.free(); @@ -821,6 +842,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { String result = Script.runSimpleBashScript("sleep 5 && umount " + targetPath); if (result == null) { s_logger.info("Succeeded in unmounting " + targetPath); + destroyStoragePoolHandleException(conn, uuid); return true; } s_logger.error("Failed to unmount " + targetPath); From bf1167627886fbb2241e0c5b6bb97e0aa7e259ed Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 7 Aug 2024 18:40:50 +0200 Subject: [PATCH 3/3] test: fix component test test_acl_sharednetwork_deployVM-impersonation.py (#9499) --- ...t_acl_sharednetwork_deployVM-impersonation.py | 16 ++++++++-------- tools/marvin/marvin/cloudstackException.py | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test/integration/component/test_acl_sharednetwork_deployVM-impersonation.py b/test/integration/component/test_acl_sharednetwork_deployVM-impersonation.py index 36b71defadb..84de263bdd3 100644 --- a/test/integration/component/test_acl_sharednetwork_deployVM-impersonation.py +++ b/test/integration/component/test_acl_sharednetwork_deployVM-impersonation.py @@ -1171,7 +1171,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin is NOT able to deploy a VM for user in ROOT domain in a shared network with scope=all") except Exception as e: self.debug("When a Domain admin user deploys a VM for ROOT user in a shared network with scope=all %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail("Error message validation failed when Domain admin is NOT able to deploy a VM for user in ROOT domain in a shared network with scope=all") @attr("simulator_only", tags=["advanced"], required_hardware="false") @@ -1199,7 +1199,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin user is able to Deploy VM for a domain user, but there is no access to in a shared network with scope=domain with no subdomain access ") except Exception as e: self.debug("When a Domain admin user deploys a VM for a domain user, but there is no access to in a shared network with scope=domain with no subdomain access %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail( "Error mesage validation failed when Domain admin user tries to Deploy VM for a domain user, but there is no access to in a shared network with scope=domain with no subdomain access ") @@ -1405,7 +1405,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin is able to deploy a VM for user in ROOT domain in a shared network with scope=Domain and no subdomain access") except Exception as e: self.debug("When a regular user from ROOT domain deploys a VM in a shared network with scope=domain with no subdomain access %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail( "Error message validation failed when Domain admin tries to deploy a VM for user in ROOT domain in a shared network with scope=Domain and no subdomain access") @@ -1601,7 +1601,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin is able to deploy a VM for user in ROOT domain in a shared network with scope=Domain and subdomain access") except Exception as e: self.debug("When a user from ROOT domain deploys a VM in a shared network with scope=domain with subdomain access %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail( "Error message validation failed when Domain admin tries to deploy a VM for user in ROOT domain in a shared network with scope=Domain and subdomain access") @@ -1717,7 +1717,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin is able to deploy a VM for an regular user from a differnt domain in a shared network with scope=account") except Exception as e: self.debug("When a user from different domain deploys a VM in a shared network with scope=account %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail( "Error message validation failed when Domain admin tries to deploy a VM for an regular user from a differnt domain in a shared network with scope=account") @@ -1746,7 +1746,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Domain admin is able to deploy a VM for an regular user in ROOT domain in a shared network with scope=account") except Exception as e: self.debug("When a user from ROOT domain deploys a VM in a shared network with scope=account %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_DOMAIN): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail("Error message validation failed when Domain admin tries to deploy a VM for an regular user in ROOT domain in a shared network with scope=account") ## Test cases relating to deploying Virtual Machine as Regular user for other users in shared network with scope=all @@ -1776,7 +1776,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Regular user is allowed to deploy a VM for another user in the same domain in a shared network with scope=all") except Exception as e: self.debug("When a regular user deploys a VM for another user in the same domain in a shared network with scope=all %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_ACCOUNT): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail("Error message validation failed when Regular user tries to deploy a VM for another user in the same domain in a shared network with scope=all") @attr("simulator_only", tags=["advanced"], required_hardware="false") @@ -1804,7 +1804,7 @@ class TestSharedNetworkImpersonation(cloudstackTestCase): self.fail("Regular user is allowed to deploy a VM for another user in the same domain in a shared network with scope=all") except Exception as e: self.debug("When a regular user deploys a VM for another user in the same domain in a shared network with scope=all %s" % e) - if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_ACCOUNT): + if not CloudstackAclException.verifyMsginException(e, CloudstackAclException.NO_PERMISSION_TO_OPERATE_SOURCE): self.fail("Error message validation failed when Regular user tries to deploy a VM for another user in the same domain in a shared network with scope=all") @staticmethod diff --git a/tools/marvin/marvin/cloudstackException.py b/tools/marvin/marvin/cloudstackException.py index 5a2f72d8c59..610cf15a0ba 100644 --- a/tools/marvin/marvin/cloudstackException.py +++ b/tools/marvin/marvin/cloudstackException.py @@ -77,6 +77,7 @@ class CloudstackAclException(): UNABLE_TO_LIST_NETWORK_ACCOUNT = "Can't create/list resources for account" NO_PERMISSION_TO_ACCESS_ACCOUNT = "does not have permission to access resource Acct" NOT_AVAILABLE_IN_DOMAIN = "not available in domain" + NO_PERMISSION_TO_OPERATE_SOURCE = "does not have permission to operate with provided resource" @staticmethod def verifyMsginException(e,message):