Merge pull request #6942 from beeankha/fix_jt_module_param_override

Fix tower_job_template module so that params are not overridden (unless specified) in subsequent tasks

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx_collection/plugins/modules/tower_job_template.py

@@ -65,7 +65,6 @@ options:
       type: list
       elements: str
       version_added: 2.8
-      default: []
     vault_credential:
       description:
         - Name of the vault credential to use for the job template.
@@ -301,7 +300,7 @@ notes:
 
 
 EXAMPLES = '''
-- name: Create tower Ping job template
+- name: Create Tower Ping job template
   tower_job_template:
     name: "Ping"
     job_type: "run"
@@ -315,6 +314,20 @@ EXAMPLES = '''
     survey_enabled: yes
     survey_spec: "{{ lookup('file', 'my_survey.json') }}"
     custom_virtualenv: "/var/lib/awx/venv/custom-venv/"
+
+- name: Add start notification to Job Template
+  tower_job_template:
+    name: "Ping"
+    notification_templates_started:
+      - Notification1
+      - Notification2
+
+- name: Remove Notification1 start notification from Job Template
+  tower_job_template:
+    name: "Ping"
+    notification_templates_started:
+      - Notification2
+
 '''
 
 from ..module_utils.tower_api import TowerModule
@@ -348,7 +361,7 @@ def main():
         credential=dict(default=''),
         vault_credential=dict(default=''),
         custom_virtualenv=dict(),
-        credentials=dict(type='list', default=[], elements='str'),
+        credentials=dict(type='list', elements='str'),
         forks=dict(type='int'),
         limit=dict(default=''),
         verbosity=dict(type='int', choices=[0, 1, 2, 3, 4], default=0),
@@ -398,11 +411,11 @@ def main():
     credential = module.params.get('credential')
     vault_credential = module.params.get('vault_credential')
     credentials = module.params.get('credentials')
-    if vault_credential:
+    if vault_credential != '':
         if credentials is None:
             credentials = []
         credentials.append(vault_credential)
-    if credential:
+    if credential != '':
         if credentials is None:
             credentials = []
         credentials.append(credential)
@@ -450,36 +463,36 @@ def main():
     if webhook_credential is not None:
         new_fields['webhook_credential'] = module.resolve_name_to_id('credentials', webhook_credential)
 
-    credentials_ids = None
+    association_fields = {}
+
     if credentials is not None:
-        credentials_ids = []
+        association_fields['credentials'] = []
         for item in credentials:
-            credentials_ids.append(module.resolve_name_to_id('credentials', item))
+            association_fields['credentials'].append(module.resolve_name_to_id('credentials', item))
 
     labels = module.params.get('labels')
-    labels_ids = None
     if labels is not None:
-        labels_ids = []
+        association_fields['labels'] = []
         for item in labels:
-            labels_ids.append(module.resolve_name_to_id('labels', item))
+            association_fields['labels'].append(module.resolve_name_to_id('labels', item))
 
     notifications_start = module.params.get('notification_templates_started')
-    notification_start_ids = []
     if notifications_start is not None:
+        association_fields['notification_templates_started'] = []
         for item in notifications_start:
-            notification_start_ids.append(module.resolve_name_to_id('notification_templates', item))
+            association_fields['notification_templates_started'].append(module.resolve_name_to_id('notification_templates', item))
 
     notifications_success = module.params.get('notification_templates_success')
-    notification_success_ids = []
     if notifications_success is not None:
+        association_fields['notification_templates_success'] = []
         for item in notifications_success:
-            notification_success_ids.append(module.resolve_name_to_id('notification_templates', item))
+            association_fields['notification_templates_success'].append(module.resolve_name_to_id('notification_templates', item))
 
     notifications_error = module.params.get('notification_templates_error')
-    notification_error_ids = []
     if notifications_error is not None:
+        association_fields['notification_templates_error'] = []
         for item in notifications_error:
-            notification_error_ids.append(module.resolve_name_to_id('notification_templates', item))
+            association_fields['notification_templates_error'].append(module.resolve_name_to_id('notification_templates', item))
 
     on_change = None
     new_spec = module.params.get('survey_spec')
@@ -498,13 +511,7 @@ def main():
     module.create_or_update_if_needed(
         existing_item, new_fields,
         endpoint='job_templates', item_type='job_template',
-        associations={
-            'credentials': credentials_ids,
-            'labels': labels_ids,
-            'notification_templates_success': notification_success_ids,
-            'notification_templates_started': notification_start_ids,
-            'notification_templates_error': notification_error_ids
-        },
+        associations=association_fields,
         on_create=on_change, on_update=on_change,
     )
 

awx_collection/test/awx/test_job_template.py

@@ -3,7 +3,7 @@ __metaclass__ = type
 
 import pytest
 
-from awx.main.models import ActivityStream, JobTemplate, Job
+from awx.main.models import ActivityStream, JobTemplate, Job, NotificationTemplate
 
 
 @pytest.mark.django_db
@@ -165,3 +165,54 @@ def test_job_template_with_survey_encrypted_default(run_module, admin_user, proj
         "The field survey_spec of job_template {0} has encrypted data and "
         "may inaccurately report task is changed.".format(result['id'])
     )
+
+
+@pytest.mark.django_db
+def test_associate_only_on_success(run_module, admin_user, organization, project):
+    jt = JobTemplate.objects.create(
+        name='foo',
+        project=project,
+        playbook='helloworld.yml',
+        ask_inventory_on_launch=True,
+    )
+    create_kwargs = dict(
+        notification_configuration={
+            'url': 'http://www.example.com/hook',
+            'headers': {
+                'X-Custom-Header': 'value123'
+            },
+            'password': 'bar'
+        },
+        notification_type='webhook',
+        organization=organization
+    )
+    nt1 = NotificationTemplate.objects.create(name='nt1', **create_kwargs)
+    nt2 = NotificationTemplate.objects.create(name='nt2', **create_kwargs)
+
+    jt.notification_templates_error.add(nt1)
+
+    # test preservation of error NTs when success NTs are added
+    result = run_module('tower_job_template', dict(
+        name='foo',
+        playbook='helloworld.yml',
+        project=project.name,
+        notification_templates_success=['nt2']
+    ), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result.get('changed', True), result
+
+    assert list(jt.notification_templates_success.values_list('id', flat=True)) == [nt2.id]
+    assert list(jt.notification_templates_error.values_list('id', flat=True)) == [nt1.id]
+
+    # test removal to empty list
+    result = run_module('tower_job_template', dict(
+        name='foo',
+        playbook='helloworld.yml',
+        project=project.name,
+        notification_templates_success=[]
+    ), admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    assert result.get('changed', True), result
+
+    assert list(jt.notification_templates_success.values_list('id', flat=True)) == []
+    assert list(jt.notification_templates_error.values_list('id', flat=True)) == [nt1.id]

awx_collection/tests/integration/targets/tower_job_template/tasks/main.yml

@@ -13,6 +13,7 @@
     jt2: "AWX-Collection-tests-tower_job_template-jt2-{{ test_id }}"
     lab1: "AWX-Collection-tests-tower_job_template-lab1-{{ test_id }}"
     email_not: "AWX-Collection-tests-tower_job_template-email-not-{{ test_id }}"
+    webhook_not: "AWX-Collection-tests-tower_notification-wehbook-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}"
 
 - name: Create a Demo Project
   tower_project:
@@ -63,6 +64,21 @@
     use_ssl: false
     state: present
 
+- name: Add webhook notification
+  tower_notification:
+    name: "{{ webhook_not }}"
+    organization: Default
+    notification_type: webhook
+    url: http://www.example.com/hook
+    headers:
+      X-Custom-Header: value123
+    state: present
+  register: result
+
+- assert:
+    that:
+      - result is changed
+
 - name: Create Job Template 1
   tower_job_template:
     name: "{{ jt1 }}"
@@ -258,37 +274,54 @@
     that:
       - "result is changed"
 
-- name: Add started notification to Job Template 2
+- name: Add started notifications to Job Template 2
   tower_job_template:
     name: "{{ jt2 }}"
     notification_templates_started:
       - "{{ email_not }}"
+      - "{{ webhook_not }}"
   register: result
 
 - assert:
     that:
       - "result is changed"
 
-- name: Re Add started notification to Job Template 2
+- name: Re Add started notifications to Job Template 2
   tower_job_template:
     name: "{{ jt2 }}"
     notification_templates_started:
       - "{{ email_not }}"
+      - "{{ webhook_not }}"
   register: result
 
 - assert:
     that:
       - "result is not changed"
 
-- name: Remove started notification to Job Template 2
+- name: Add success notifications to Job Template 2
   tower_job_template:
     name: "{{ jt2 }}"
+    notification_templates_success:
+      - "{{ email_not }}"
+      - "{{ webhook_not }}"
   register: result
 
 - assert:
     that:
       - "result is changed"
 
+- name: Remove "on start" webhook notification from Job Template 2
+  tower_job_template:
+    name: "{{ jt2 }}"
+    notification_templates_started:
+      - "{{ email_not }}"
+  register: result
+
+- assert:
+    that:
+      - "result is changed"
+
+
 - name: Delete Job Template 2
   tower_job_template:
     name: "{{ jt2 }}"