From cc7600a3f8dd5ba7e476b2fff9071ba874dc2a7f Mon Sep 17 00:00:00 2001 From: Donat Szabo Date: Wed, 2 Sep 2026 15:57:25 +0200 Subject: [PATCH 1/4] Argument spec implementation for postfix role --- meta/argument_specs.yml | 84 +++++++++++++++ tasks/assert_role_vars.yml | 10 ++ tasks/main.yml | 3 + tests/tests_invalid_input.yml | 196 ++++++++++++++++++++++++++++++++++ 4 files changed, 293 insertions(+) create mode 100644 meta/argument_specs.yml create mode 100644 tasks/assert_role_vars.yml create mode 100644 tests/tests_invalid_input.yml diff --git a/meta/argument_specs.yml b/meta/argument_specs.yml new file mode 100644 index 00000000..6b068ca3 --- /dev/null +++ b/meta/argument_specs.yml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: MIT +--- +argument_specs: + main: + short_description: The postfix role. + description: > + The postfix role allows you to install, configure, and start the + Postfix mail transfer agent. It can apply `main.cf` settings, + deploy additional lookup table files under `/etc/postfix`, and + optionally manage firewall and SELinux policy for SMTP-related + ports. + options: + postfix_conf: + type: dict + default: {} + description: > + Dictionary of Postfix `main.cf` parameter names and values. + Keys not supported by the installed Postfix are ignored. + Set `previous` to `replaced` to reinstall Postfix and apply + configuration on a clean installation. + postfix_files: + type: list + elements: dict + default: [] + description: > + List of additional files to place in `/etc/postfix`. Each + entry may optionally be converted to a Postfix lookup table + with `postmap`. + options: + name: + type: str + required: true + description: > + Base file name under `/etc/postfix` for the file content. + content: + type: str + required: true + description: > + File content to write under `/etc/postfix/`. + postmap: + type: bool + default: false + description: > + Whether to run `postmap` on the file after it is created + or updated. + postfix_check: + type: bool + default: true + description: > + Whether to run `postfix check` before Postfix is restarted + when the configuration has changed. + postfix_backup: + type: bool + default: false + description: > + Whether to make a single backup copy of `main.cf` before + applying changes. When `postfix_backup_multiple` is `true`, + timestamped backups are used instead. + postfix_backup_multiple: + type: bool + default: true + description: > + Whether to make timestamped backup copies of `main.cf` + before applying changes. When `true`, this overrides + `postfix_backup`. + postfix_manage_firewall: + type: bool + default: false + description: > + Whether to open SMTP-related ports (`25/tcp`, `465/tcp`, and + `587/tcp`) using the firewall role. + postfix_manage_selinux: + type: bool + default: false + description: > + Whether to assign `smtp_port_t` to SMTP-related ports using + the selinux role. + postfix_secure_logging: + type: bool + default: true + description: > + Whether to suppress potentially sensitive task output by + setting `no_log` on tasks that handle credentials and other + secrets. diff --git a/tasks/assert_role_vars.yml b/tasks/assert_role_vars.yml new file mode 100644 index 00000000..0cafc2a5 --- /dev/null +++ b/tasks/assert_role_vars.yml @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: MIT +--- +- name: Assert postfix_conf previous is replaced when set + ansible.builtin.assert: + that: + - postfix_conf.previous == 'replaced' + fail_msg: >- + postfix_conf.previous must be 'replaced' when set, + got {{ postfix_conf.previous | type_debug }} + when: postfix_conf.previous is defined diff --git a/tasks/main.yml b/tasks/main.yml index 885547a5..b65e10d3 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -1,4 +1,7 @@ --- +- name: Validate role parameters + ansible.builtin.include_tasks: assert_role_vars.yml + - name: Ensure ansible_facts required by role include_tasks: set_facts.yml diff --git a/tests/tests_invalid_input.yml b/tests/tests_invalid_input.yml new file mode 100644 index 00000000..43c2da41 --- /dev/null +++ b/tests/tests_invalid_input.yml @@ -0,0 +1,196 @@ +# SPDX-License-Identifier: MIT +--- +- name: Verify invalid parameters are rejected + hosts: all + tasks: + - name: Run invalid input tests + block: + # ==================================================== + # Section 1: Verify role works with valid defaults + # ==================================================== + - name: Run role with valid defaults + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + + # ==================================================== + # Section 2: argument_specs validation (Ansible 2.11+) + # ==================================================== + - name: Run argument specs validation tests + when: ansible_version.full is version("2.11", ">=") + block: + # --- Test: invalid bool postfix_check --- + - name: Argument specs reject non-bool postfix_check + block: + - name: Run role with non-bool postfix_check + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_check: not_a_bool + rescue: + - name: Mark invalid postfix_check type rejected + ansible.builtin.set_fact: + __invalid_input_postfix_check_type_failed: true + + - name: Assert invalid postfix_check type was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_check_type_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_check with a + non-boolean value + + # --- Test: invalid bool postfix_secure_logging --- + - name: Argument specs reject non-bool postfix_secure_logging + block: + - name: Run role with non-bool postfix_secure_logging + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_secure_logging: not_a_bool + rescue: + - name: Mark invalid postfix_secure_logging type rejected + ansible.builtin.set_fact: + __invalid_input_postfix_secure_logging_type_failed: true + + - name: Assert invalid postfix_secure_logging type was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_secure_logging_type_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_secure_logging + with a non-boolean value + + # --- Test: invalid type postfix_conf --- + - name: Argument specs reject non-dict postfix_conf + block: + - name: Run role with non-dict postfix_conf + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_conf: not_a_dict + rescue: + - name: Mark invalid postfix_conf type rejected + ansible.builtin.set_fact: + __invalid_input_postfix_conf_type_failed: true + + - name: Assert invalid postfix_conf type was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_conf_type_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_conf with a + non-dictionary value + + # --- Test: invalid type postfix_files --- + - name: Argument specs reject non-list postfix_files + block: + - name: Run role with non-list postfix_files + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_files: not_a_list + rescue: + - name: Mark invalid postfix_files type rejected + ansible.builtin.set_fact: + __invalid_input_postfix_files_type_failed: true + + - name: Assert invalid postfix_files type was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_files_type_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_files with a + non-list value + + # --- Test: missing required name in postfix_files --- + - name: Argument specs reject postfix_files missing name + block: + - name: Run role without required name in postfix_files + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_files: + - content: example content + rescue: + - name: Mark missing postfix_files name rejected + ansible.builtin.set_fact: + __invalid_input_postfix_files_missing_name_failed: true + when: >- + 'missing required arguments' in + (ansible_failed_result | default({}) | to_json) + + - name: Assert missing postfix_files name was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_files_missing_name_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_files entries + missing the required name field + + # --- Test: missing required content in postfix_files --- + - name: Argument specs reject postfix_files missing content + block: + - name: Run role without required content in postfix_files + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_files: + - name: test_file + rescue: + - name: Mark missing postfix_files content rejected + ansible.builtin.set_fact: + __invalid_input_postfix_files_missing_content_failed: true + when: >- + 'missing required arguments' in + (ansible_failed_result | default({}) | to_json) + + - name: Assert missing postfix_files content was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_files_missing_content_failed + | default(false) + fail_msg: >- + argument_specs should reject postfix_files entries + missing the required content field + + # ==================================================== + # Section 3: assert_role_vars validation (all versions) + # ==================================================== + + - name: Assert rejects invalid postfix_conf previous value + block: + - name: Run role with invalid postfix_conf previous + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_conf: + previous: not_replaced + rescue: + - name: Mark invalid postfix_conf previous rejected + ansible.builtin.set_fact: + __invalid_input_postfix_conf_previous_failed: true + + - name: Assert invalid postfix_conf previous was rejected + ansible.builtin.assert: + that: + - >- + __invalid_input_postfix_conf_previous_failed + | default(false) + fail_msg: >- + assert_role_vars should reject postfix_conf.previous + when it is not 'replaced' + + always: + - name: Clear test facts + ansible.builtin.set_fact: + __invalid_input_postfix_check_type_failed: + __invalid_input_postfix_secure_logging_type_failed: + __invalid_input_postfix_conf_type_failed: + __invalid_input_postfix_files_type_failed: + __invalid_input_postfix_files_missing_name_failed: + __invalid_input_postfix_files_missing_content_failed: + __invalid_input_postfix_conf_previous_failed: + tags: tests::cleanup From 7edc6929cf96501196a8b03db6b84ea8afbc1bd6 Mon Sep 17 00:00:00 2001 From: Donat Szabo Date: Wed, 2 Sep 2026 16:20:27 +0200 Subject: [PATCH 2/4] Made changes recommended by coderabbit --- tests/tests_invalid_input.yml | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/tests_invalid_input.yml b/tests/tests_invalid_input.yml index 43c2da41..abff03ad 100644 --- a/tests/tests_invalid_input.yml +++ b/tests/tests_invalid_input.yml @@ -11,6 +11,12 @@ - name: Run role with valid defaults ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + - name: Assert postfix service is running after valid defaults + ansible.builtin.command: systemctl is-active postfix + register: __postfix_service_active + changed_when: false + failed_when: __postfix_service_active.stdout | trim != 'active' + # ==================================================== # Section 2: argument_specs validation (Ansible 2.11+) # ==================================================== @@ -28,6 +34,11 @@ - name: Mark invalid postfix_check type rejected ansible.builtin.set_fact: __invalid_input_postfix_check_type_failed: true + when: >- + 'postfix_check' in + (ansible_failed_result | default({}) | to_json) + and 'bool' in + (ansible_failed_result | default({}) | to_json | lower) - name: Assert invalid postfix_check type was rejected ansible.builtin.assert: @@ -50,6 +61,11 @@ - name: Mark invalid postfix_secure_logging type rejected ansible.builtin.set_fact: __invalid_input_postfix_secure_logging_type_failed: true + when: >- + 'postfix_secure_logging' in + (ansible_failed_result | default({}) | to_json) + and 'bool' in + (ansible_failed_result | default({}) | to_json | lower) - name: Assert invalid postfix_secure_logging type was rejected ansible.builtin.assert: @@ -72,6 +88,11 @@ - name: Mark invalid postfix_conf type rejected ansible.builtin.set_fact: __invalid_input_postfix_conf_type_failed: true + when: >- + 'postfix_conf' in + (ansible_failed_result | default({}) | to_json) + and 'dict' in + (ansible_failed_result | default({}) | to_json | lower) - name: Assert invalid postfix_conf type was rejected ansible.builtin.assert: @@ -94,6 +115,9 @@ - name: Mark invalid postfix_files type rejected ansible.builtin.set_fact: __invalid_input_postfix_files_type_failed: true + when: >- + 'postfix_files' in + (ansible_failed_result | default({}) | to_json) - name: Assert invalid postfix_files type was rejected ansible.builtin.assert: @@ -120,6 +144,8 @@ when: >- 'missing required arguments' in (ansible_failed_result | default({}) | to_json) + and 'name found in postfix_files' in + (ansible_failed_result | default({}) | to_json) - name: Assert missing postfix_files name was rejected ansible.builtin.assert: @@ -146,6 +172,8 @@ when: >- 'missing required arguments' in (ansible_failed_result | default({}) | to_json) + and 'content found in postfix_files' in + (ansible_failed_result | default({}) | to_json) - name: Assert missing postfix_files content was rejected ansible.builtin.assert: @@ -161,6 +189,12 @@ # Section 3: assert_role_vars validation (all versions) # ==================================================== + - name: Run role with valid postfix_conf previous replaced + ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml + vars: + postfix_conf: + previous: replaced + - name: Assert rejects invalid postfix_conf previous value block: - name: Run role with invalid postfix_conf previous @@ -172,6 +206,9 @@ - name: Mark invalid postfix_conf previous rejected ansible.builtin.set_fact: __invalid_input_postfix_conf_previous_failed: true + when: >- + 'postfix_conf.previous must be' in + (ansible_failed_result | default({}) | to_json) - name: Assert invalid postfix_conf previous was rejected ansible.builtin.assert: From 2fc9b4e8a3f5e4b25e5896c9fec994107e17436b Mon Sep 17 00:00:00 2001 From: Donat Szabo Date: Thu, 3 Sep 2026 10:23:45 +0200 Subject: [PATCH 3/4] Removed type --- tasks/assert_role_vars.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/assert_role_vars.yml b/tasks/assert_role_vars.yml index 0cafc2a5..a33eef6d 100644 --- a/tasks/assert_role_vars.yml +++ b/tasks/assert_role_vars.yml @@ -6,5 +6,5 @@ - postfix_conf.previous == 'replaced' fail_msg: >- postfix_conf.previous must be 'replaced' when set, - got {{ postfix_conf.previous | type_debug }} + got {{ postfix_conf.previous }} when: postfix_conf.previous is defined From f0e909b153af52ef2686af86a87832bd39767fdb Mon Sep 17 00:00:00 2001 From: Donat Szabo Date: Mon, 7 Sep 2026 15:39:39 +0200 Subject: [PATCH 4/4] Changed from command to module use --- tests/tests_invalid_input.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/tests_invalid_input.yml b/tests/tests_invalid_input.yml index abff03ad..2ce82505 100644 --- a/tests/tests_invalid_input.yml +++ b/tests/tests_invalid_input.yml @@ -12,10 +12,11 @@ ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml - name: Assert postfix service is running after valid defaults - ansible.builtin.command: systemctl is-active postfix - register: __postfix_service_active - changed_when: false - failed_when: __postfix_service_active.stdout | trim != 'active' + ansible.builtin.service: + name: postfix + state: started + register: __postfix_service + failed_when: __postfix_service is changed # noqa no-handler # ==================================================== # Section 2: argument_specs validation (Ansible 2.11+)