From 28bc99565b12e0480ae2b86d8d97353696857f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Aur=C3=A8le=20Brothier?= Date: Fri, 28 Jul 2017 10:29:30 +0200 Subject: [PATCH] CLOUDSTACK-9631: API: affinitygroupids or affinitygroupnames must be given (#1798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return an exception if both parameter are missing. This fixes an NPE in AffinityGroupServiceImpl.updateVMAffinityGroups() when the list was null. Signed-off-by: Marc-Aurèle Brothier --- .../UpdateVMAffinityGroupCmd.java | 12 ++++++--- .../component/test_affinity_groups.py | 27 ++++++++++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/command/user/affinitygroup/UpdateVMAffinityGroupCmd.java b/api/src/org/apache/cloudstack/api/command/user/affinitygroup/UpdateVMAffinityGroupCmd.java index 703b051ef86..70850001cfc 100644 --- a/api/src/org/apache/cloudstack/api/command/user/affinitygroup/UpdateVMAffinityGroupCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/affinitygroup/UpdateVMAffinityGroupCmd.java @@ -92,10 +92,6 @@ public class UpdateVMAffinityGroupCmd extends BaseAsyncCmd { } public List getAffinityGroupIdList() { - if (affinityGroupNameList != null && affinityGroupIdList != null) { - throw new InvalidParameterValueException("affinitygroupids parameter is mutually exclusive with affinitygroupnames parameter"); - } - // transform group names to ids here if (affinityGroupNameList != null) { List affinityGroupIds = new ArrayList(); @@ -138,6 +134,14 @@ public class UpdateVMAffinityGroupCmd extends BaseAsyncCmd { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException { + if (affinityGroupNameList != null && affinityGroupIdList != null) { + throw new InvalidParameterValueException("affinitygroupids parameter is mutually exclusive with affinitygroupnames parameter"); + } + + if (affinityGroupNameList == null && affinityGroupIdList == null) { + throw new InvalidParameterValueException("affinitygroupids parameter or affinitygroupnames parameter must be given"); + } + CallContext.current().setEventDetails("VM ID: " + getId()); UserVm result = _affinityGroupService.updateVMAffinityGroups(getId(), getAffinityGroupIdList()); ArrayList dc = new ArrayList(); diff --git a/test/integration/component/test_affinity_groups.py b/test/integration/component/test_affinity_groups.py index c0c552973ae..1bf740361e6 100644 --- a/test/integration/component/test_affinity_groups.py +++ b/test/integration/component/test_affinity_groups.py @@ -1040,7 +1040,6 @@ class TestUpdateVMAffinityGroups(cloudstackTestCase): for aff_grp in aff_grps: aff_grp.delete(self.api_client) - @unittest.skip("Skip - Failing - work in progress") @attr(tags=["simulator", "basic", "advanced", "multihost", "NotRun"]) def test_04_update_aff_grp_remove_all(self): """ @@ -1087,6 +1086,32 @@ class TestUpdateVMAffinityGroups(cloudstackTestCase): for aff_grp in aff_grps: aff_grp.delete(self.api_client) + @attr(tags=["simulator", "basic", "advanced", "multihost", "NotRun"]) + def test_06_update_aff_grp_invalid_args(self): + """ + Update the list of Affinity Groups with either both args or none + """ + + self.create_aff_grp(aff_grp=self.services["host_anti_affinity"]) + self.create_aff_grp(aff_grp=self.services["host_anti_affinity"]) + vm1, hostid1 = self.create_vm_in_aff_grps([], account_name=self.account.name, domain_id=self.domain.id) + + aff_grps = [self.aff_grp[0], self.aff_grp[1]] + vm1.stop(self.api_client) + + with self.assertRaises(Exception): + vm1.update_affinity_group(self.api_client) + + with self.assertRaises(Exception): + vm1.update_affinity_group(self.api_client, affinitygroupids=[self.aff_grp[0].id], affinitygroupnames=[self.aff_grp[1].name]) + + vm1.update_affinity_group(self.api_client, affinitygroupids=[]) + + vm1.delete(self.api_client) + # Can cleanup affinity groups since none are set on the VM + for aff_grp in aff_grps: + aff_grp.delete(self.api_client) + class TestDeployVMAffinityGroups(cloudstackTestCase): @classmethod