CLOUDSTACK-9631: API: affinitygroupids or affinitygroupnames must be given (#1798)

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 <m@brothier.org>

Author: marcaurele

api/src/org/apache/cloudstack/api/command/user/affinitygroup/UpdateVMAffinityGroupCmd.java

@@ -92,10 +92,6 @@ public Long getId() {
     }
 
     public List<Long> 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<Long> affinityGroupIds = new ArrayList<Long>();
@@ -138,6 +134,14 @@ public long getEntityOwnerId() {
 
     @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<VMDetails> dc = new ArrayList<VMDetails>();

test/integration/component/test_affinity_groups.py

@@ -1040,7 +1040,6 @@ def test_03_update_aff_grp_for_vm_with_no_aff_grp(self):
         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 @@ def test_05_update_aff_grp_on_running_vm(self):
         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