From c7159c5fa48e217b163be6516a2b8c8fe61e284f Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Fri, 29 Sep 2023 14:41:01 -0400 Subject: [PATCH 1/5] Fix Boolean values defaulting to False in collection --- awx_collection/plugins/module_utils/controller_api.py | 7 +++++++ .../tests/integration/targets/host/tasks/main.yml | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/awx_collection/plugins/module_utils/controller_api.py b/awx_collection/plugins/module_utils/controller_api.py index 0c5c19165585..789974eae643 100644 --- a/awx_collection/plugins/module_utils/controller_api.py +++ b/awx_collection/plugins/module_utils/controller_api.py @@ -809,6 +809,13 @@ def create_if_needed(self, existing_item, new_item, endpoint, on_create=None, au # We will pull the item_name out from the new_item, if it exists item_name = self.get_item_name(new_item, allow_unknown=True) + # Remove null values - this assumes there are no nullable fields with non-null defaults + # the None value is used to indicate not-provided by Ansible and argparse + # this is needed so that boolean fields will not get a false value when not provided + for key in list(new_item.keys()): + if new_item[key] is None: + new_item.pop(key) + response = self.post_endpoint(endpoint, **{'data': new_item}) # 200 is response from approval node creation on tower 3.7.3 or awx 15.0.0 or earlier. diff --git a/awx_collection/tests/integration/targets/host/tasks/main.yml b/awx_collection/tests/integration/targets/host/tasks/main.yml index d7752ca6617a..244c1e3f93c4 100644 --- a/awx_collection/tests/integration/targets/host/tasks/main.yml +++ b/awx_collection/tests/integration/targets/host/tasks/main.yml @@ -68,6 +68,11 @@ that: - "result is changed" +- name: Newly created host should have API default value for enabled + assert: + that: + - "lookup('awx.awx.controller_api', 'hosts/{{result.id}}/').enabled" + - name: Delete a Host host: name: "{{ result.id }}" From b71f6ad421336e0b9047b2dca3fbafa88fc41748 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Mon, 2 Oct 2023 15:30:22 -0400 Subject: [PATCH 2/5] Remove null values in other cases, fix null handling for WFJT nodes --- .../plugins/module_utils/controller_api.py | 14 +++++++------- .../plugins/modules/workflow_job_template_node.py | 2 +- .../tests/integration/targets/host/tasks/main.yml | 10 ++++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/awx_collection/plugins/module_utils/controller_api.py b/awx_collection/plugins/module_utils/controller_api.py index 789974eae643..2193adad577c 100644 --- a/awx_collection/plugins/module_utils/controller_api.py +++ b/awx_collection/plugins/module_utils/controller_api.py @@ -809,13 +809,6 @@ def create_if_needed(self, existing_item, new_item, endpoint, on_create=None, au # We will pull the item_name out from the new_item, if it exists item_name = self.get_item_name(new_item, allow_unknown=True) - # Remove null values - this assumes there are no nullable fields with non-null defaults - # the None value is used to indicate not-provided by Ansible and argparse - # this is needed so that boolean fields will not get a false value when not provided - for key in list(new_item.keys()): - if new_item[key] is None: - new_item.pop(key) - response = self.post_endpoint(endpoint, **{'data': new_item}) # 200 is response from approval node creation on tower 3.7.3 or awx 15.0.0 or earlier. @@ -987,6 +980,13 @@ def update_if_needed(self, existing_item, new_item, on_update=None, auto_exit=Tr def create_or_update_if_needed( self, existing_item, new_item, endpoint=None, item_type='unknown', on_create=None, on_update=None, auto_exit=True, associations=None ): + # Remove null values - this assumes there are no nullable fields with non-null defaults + # the None value is used to indicate not-provided by Ansible and argparse + # this is needed so that boolean fields will not get a false value when not provided + for key in list(new_item.keys()): + if new_item[key] is None: + new_item.pop(key) + if existing_item: return self.update_if_needed(existing_item, new_item, on_update=on_update, auto_exit=auto_exit, associations=associations) else: diff --git a/awx_collection/plugins/modules/workflow_job_template_node.py b/awx_collection/plugins/modules/workflow_job_template_node.py index 71ca1c140075..39537ddc549c 100644 --- a/awx_collection/plugins/modules/workflow_job_template_node.py +++ b/awx_collection/plugins/modules/workflow_job_template_node.py @@ -362,7 +362,7 @@ def main(): 'timeout', ): field_val = module.params.get(field_name) - if field_val: + if field_val is not None: new_fields[field_name] = field_val association_fields = {} diff --git a/awx_collection/tests/integration/targets/host/tasks/main.yml b/awx_collection/tests/integration/targets/host/tasks/main.yml index 244c1e3f93c4..7431aaeb3d1e 100644 --- a/awx_collection/tests/integration/targets/host/tasks/main.yml +++ b/awx_collection/tests/integration/targets/host/tasks/main.yml @@ -42,6 +42,16 @@ that: - "result is not changed" +- name: Modify the host as a no-op + host: + name: "{{ host_name }}" + inventory: "{{ inv_name }}" + register: result + +- assert: + that: + - "result is not changed" + - name: Delete a Host host: name: "{{ host_name }}" From 79cc73ef9d4572c042b75a8cad4ec48623cd7a99 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Tue, 3 Oct 2023 10:33:06 -0400 Subject: [PATCH 3/5] Only remove null values if it is a boolean field --- awx_collection/plugins/module_utils/controller_api.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/awx_collection/plugins/module_utils/controller_api.py b/awx_collection/plugins/module_utils/controller_api.py index 2193adad577c..166d43c49e41 100644 --- a/awx_collection/plugins/module_utils/controller_api.py +++ b/awx_collection/plugins/module_utils/controller_api.py @@ -980,12 +980,14 @@ def update_if_needed(self, existing_item, new_item, on_update=None, auto_exit=Tr def create_or_update_if_needed( self, existing_item, new_item, endpoint=None, item_type='unknown', on_create=None, on_update=None, auto_exit=True, associations=None ): - # Remove null values - this assumes there are no nullable fields with non-null defaults - # the None value is used to indicate not-provided by Ansible and argparse + # Remove boolean values of certain specific types # this is needed so that boolean fields will not get a false value when not provided for key in list(new_item.keys()): - if new_item[key] is None: - new_item.pop(key) + if key in self.argument_spec: + param_spec = self.argument_spec[key] + if 'type' in param_spec and param_spec['type'] == 'bool': + if new_item[key] is None: + new_item.pop(key) if existing_item: return self.update_if_needed(existing_item, new_item, on_update=on_update, auto_exit=auto_exit, associations=associations) From 0ef14a9a5d0537cabe0ba3141f43c9b78a515a2c Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Tue, 3 Oct 2023 10:34:31 -0400 Subject: [PATCH 4/5] Reset changes to WFJT node field processing --- awx_collection/plugins/modules/workflow_job_template_node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx_collection/plugins/modules/workflow_job_template_node.py b/awx_collection/plugins/modules/workflow_job_template_node.py index 39537ddc549c..71ca1c140075 100644 --- a/awx_collection/plugins/modules/workflow_job_template_node.py +++ b/awx_collection/plugins/modules/workflow_job_template_node.py @@ -362,7 +362,7 @@ def main(): 'timeout', ): field_val = module.params.get(field_name) - if field_val is not None: + if field_val: new_fields[field_name] = field_val association_fields = {} From c2edea30689b82333a24ae66c9eba55e361916fb Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 5 Oct 2023 08:40:15 -0400 Subject: [PATCH 5/5] Use test content from sean-m-sullivan to fix lookups in assert --- .../tests/integration/targets/host/tasks/main.yml | 6 +++++- .../tests/integration/targets/schedule/tasks/main.yml | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/awx_collection/tests/integration/targets/host/tasks/main.yml b/awx_collection/tests/integration/targets/host/tasks/main.yml index 7431aaeb3d1e..2b5d66ff8b43 100644 --- a/awx_collection/tests/integration/targets/host/tasks/main.yml +++ b/awx_collection/tests/integration/targets/host/tasks/main.yml @@ -78,10 +78,14 @@ that: - "result is changed" +- name: Use lookup to check that host was enabled + ansible.builtin.set_fact: + host_enabled_test: "lookup('awx.awx.controller_api', 'hosts/{{result.id}}/').enabled" + - name: Newly created host should have API default value for enabled assert: that: - - "lookup('awx.awx.controller_api', 'hosts/{{result.id}}/').enabled" + - host_enabled_test - name: Delete a Host host: diff --git a/awx_collection/tests/integration/targets/schedule/tasks/main.yml b/awx_collection/tests/integration/targets/schedule/tasks/main.yml index df7e2d88e3c1..6f9eca1b33b6 100644 --- a/awx_collection/tests/integration/targets/schedule/tasks/main.yml +++ b/awx_collection/tests/integration/targets/schedule/tasks/main.yml @@ -76,6 +76,15 @@ that: - result is changed + - name: Use lookup to check that schedules was enabled + ansible.builtin.set_fact: + schedules_enabled_test: "lookup('awx.awx.controller_api', 'schedules/{{result.id}}/').enabled" + + - name: Newly created schedules should have API default value for enabled + assert: + that: + - schedules_enabled_test + - name: Build a real schedule with exists schedule: name: "{{ sched1 }}"