Changing how associations work in awx collection (#13626)

Co-authored-by: Alan Rominger <arominge@redhat.com>
Co-authored-by: Jessica Steurer <70719005+jay-steurer@users.noreply.github.com>

awx_collection/plugins/module_utils/controller_api.py

@@ -56,6 +56,8 @@ class ControllerModule(AnsibleModule):
         ),
         controller_config_file=dict(type='path', aliases=['tower_config_file'], required=False, default=None),
     )
+    # Associations of these types are ordered and have special consideration in the modified associations function
+    ordered_associations = ['instance_groups', 'galaxy_credentials']
     short_params = {
         'host': 'controller_host',
         'username': 'controller_username',
@@ -680,17 +682,26 @@ class ControllerAPIModule(ControllerModule):
         response = self.get_all_endpoint(association_endpoint)
         existing_associated_ids = [association['id'] for association in response['json']['results']]
 
-        # Disassociate anything that is in existing_associated_ids but not in new_association_list
+        # Some associations can be ordered (like galaxy credentials)
-        ids_to_remove = list(set(existing_associated_ids) - set(new_association_list))
+        if association_endpoint.strip('/').split('/')[-1] in self.ordered_associations:
-        for an_id in ids_to_remove:
+            if existing_associated_ids == new_association_list:
+                return  # If the current associations EXACTLY match the desired associations then we can return
+            removal_list = existing_associated_ids  # because of ordering, we have to remove everything
+            addition_list = new_association_list  # re-add everything back in-order
+        else:
+            if set(existing_associated_ids) == set(new_association_list):
+                return
+            removal_list = set(existing_associated_ids) - set(new_association_list)
+            addition_list = set(new_association_list) - set(existing_associated_ids)
+
+        for an_id in removal_list:
             response = self.post_endpoint(association_endpoint, **{'data': {'id': int(an_id), 'disassociate': True}})
             if response['status_code'] == 204:
                 self.json_output['changed'] = True
             else:
                 self.fail_json(msg="Failed to disassociate item {0}".format(response['json'].get('detail', response['json'])))
 
-        # Associate anything that is in new_association_list but not in `association`
+        for an_id in addition_list:
-        for an_id in list(set(new_association_list) - set(existing_associated_ids)):
             response = self.post_endpoint(association_endpoint, **{'data': {'id': int(an_id)}})
             if response['status_code'] == 204:
                 self.json_output['changed'] = True

awx_collection/test/awx/test_job_template.py

@@ -2,9 +2,11 @@ from __future__ import absolute_import, division, print_function
 
 __metaclass__ = type
 
+import random
+
 import pytest
 
-from awx.main.models import ActivityStream, JobTemplate, Job, NotificationTemplate
+from awx.main.models import ActivityStream, JobTemplate, Job, NotificationTemplate, Label
 
 
 @pytest.mark.django_db
@@ -243,6 +245,42 @@ def test_job_template_with_survey_encrypted_default(run_module, admin_user, proj
     )
 
 
+@pytest.mark.django_db
+def test_associate_changed_status(run_module, admin_user, organization, project):
+    # create JT and labels
+    jt = JobTemplate.objects.create(name='foo', project=project, playbook='helloworld.yml')
+    labels = [Label.objects.create(name=f'foo{i}', organization=organization) for i in range(10)]
+
+    # sanity: no-op without labels involved
+    result = run_module('job_template', dict(name=jt.name, playbook='helloworld.yml'), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result['changed'] is False
+
+    # first time adding labels, this should make the label list equal to what was specified
+    result = run_module('job_template', dict(name=jt.name, playbook='helloworld.yml', labels=[l.name for l in labels]), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result['changed']
+    assert set(l.id for l in jt.labels.all()) == set(l.id for l in labels)
+
+    # shuffling the labels should not result in any change
+    random.shuffle(labels)
+    result = run_module('job_template', dict(name=jt.name, playbook='helloworld.yml', labels=[l.name for l in labels]), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result['changed'] is False
+
+    # not specifying labels should not change labels
+    result = run_module('job_template', dict(name=jt.name, playbook='helloworld.yml'), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result['changed'] is False
+
+    # should be able to remove only some labels
+    fewer_labels = labels[:7]
+    result = run_module('job_template', dict(name=jt.name, playbook='helloworld.yml', labels=[l.name for l in fewer_labels]), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result['changed']
+    assert set(l.id for l in jt.labels.all()) == set(l.id for l in fewer_labels)
+
+
 @pytest.mark.django_db
 def test_associate_only_on_success(run_module, admin_user, organization, project):
     jt = JobTemplate.objects.create(

awx_collection/test/awx/test_organization.py

@@ -3,8 +3,9 @@ from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
 import pytest
+import random
 
-from awx.main.models import Organization
+from awx.main.models import Organization, Credential, CredentialType
 
 
 @pytest.mark.django_db
@@ -30,3 +31,63 @@ def test_create_organization(run_module, admin_user):
     assert result == {"name": "foo", "changed": True, "id": org.id, "invocation": {"module_args": module_args}}
 
     assert org.description == 'barfoo'
+
+
+@pytest.mark.django_db
+def test_galaxy_credential_order(run_module, admin_user):
+    org = Organization.objects.create(name='foo')
+    cred_type = CredentialType.defaults['galaxy_api_token']()
+    cred_type.save()
+
+    cred_ids = []
+    for number in range(1, 10):
+        new_cred = Credential.objects.create(name=f"Galaxy Credential {number}", credential_type=cred_type, organization=org, inputs={'url': 'www.redhat.com'})
+        cred_ids.append(new_cred.id)
+
+    random.shuffle(cred_ids)
+
+    module_args = {
+        'name': 'foo',
+        'state': 'present',
+        'controller_host': None,
+        'controller_username': None,
+        'controller_password': None,
+        'validate_certs': None,
+        'controller_oauthtoken': None,
+        'controller_config_file': None,
+        'galaxy_credentials': cred_ids,
+    }
+
+    result = run_module('organization', module_args, admin_user)
+    print(result)
+    assert result['changed'] is True
+
+    cred_order_in_org = []
+    for a_cred in org.galaxy_credentials.all():
+        cred_order_in_org.append(a_cred.id)
+
+    assert cred_order_in_org == cred_ids
+
+    # Shuffle them up and try again to make sure a new order is honored
+    random.shuffle(cred_ids)
+
+    module_args = {
+        'name': 'foo',
+        'state': 'present',
+        'controller_host': None,
+        'controller_username': None,
+        'controller_password': None,
+        'validate_certs': None,
+        'controller_oauthtoken': None,
+        'controller_config_file': None,
+        'galaxy_credentials': cred_ids,
+    }
+
+    result = run_module('organization', module_args, admin_user)
+    assert result['changed'] is True
+
+    cred_order_in_org = []
+    for a_cred in org.galaxy_credentials.all():
+        cred_order_in_org.append(a_cred.id)
+
+    assert cred_order_in_org == cred_ids