From 97070b49a6538c5a7d302f3bc5e0210c7da0e4fa Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Fri, 3 Mar 2023 17:45:19 +0530 Subject: [PATCH] Generalize args rule exception handling (#3113) Co-authored-by: Ajinkya Udgirkar --- examples/playbooks/rule-args-module-pass.yml | 23 +++++++ src/ansiblelint/rules/args.py | 69 ++++++-------------- test/test_skip_inside_yaml.py | 2 +- 3 files changed, 44 insertions(+), 50 deletions(-) diff --git a/examples/playbooks/rule-args-module-pass.yml b/examples/playbooks/rule-args-module-pass.yml index f9ce33fce8..54964849cd 100644 --- a/examples/playbooks/rule-args-module-pass.yml +++ b/examples/playbooks/rule-args-module-pass.yml @@ -33,3 +33,26 @@ loop: "{{ repositories_keys }}" loop_control: loop_var: zj_item + + - name: Bug 2428 daemon_reload should be allowed + ansible.builtin.systemd: + name: foo + state: restarted + daemon_reload: true + + - name: Bug 2424 async_status + ansible.builtin.async_status: + jid: "{{ 999 }}" + + - name: Bug https://github.com/VSChina/vscode-ansible/issues/261 + ansible.builtin.set_fact: + dns_nameservers: "{{ var1 }}" + + - name: Bug cmd should be allowed + ansible.builtin.command: + cmd: echo "foo" + changed_when: false + + - name: Bag another allowed form of command + ansible.builtin.command: "/etc/test.sh" + changed_when: false diff --git a/src/ansiblelint/rules/args.py b/src/ansiblelint/rules/args.py index 0d7f1128f4..f369e099d3 100644 --- a/src/ansiblelint/rules/args.py +++ b/src/ansiblelint/rules/args.py @@ -38,6 +38,19 @@ flags=re.MULTILINE | re.DOTALL, ) +workarounds_drop_map = { + # https://github.com/ansible/ansible-lint/issues/3110 + "ansible.builtin.copy": ["decrypt"], + # https://github.com/ansible/ansible-lint/issues/2824#issuecomment-1354337466 + "ansible.builtin.service": ["daemon_reload"], + # Avoid: Unsupported parameters for (basic.py) module: cmd. Supported parameters include: _raw_params, _uses_shell, argv, chdir, creates, executable, removes, stdin, stdin_add_newline, strip_empty_ends. + "ansible.builtin.command": ["cmd"], +} +workarounds_inject_map = { + # https://github.com/ansible/ansible-lint/issues/2824 + "ansible.builtin.async_status": {"_async_dir": "/tmp/ansible-async"} +} + @lru_cache def load_module(module_name: str) -> loader.PluginLoadContext: @@ -85,14 +98,12 @@ def matchtask( for key, value in task["action"].items() if not key.startswith("__") } - # https://github.com/ansible/ansible-lint/issues/2824 - if loaded_module.resolved_fqcn == "ansible.builtin.async_status": - module_args["_async_dir"] = "/tmp/ansible-async" - if loaded_module.resolved_fqcn == "ansible.builtin.service": - _logger.debug( - "Skipped service module validation as not being supported yet." - ) - return [] + if loaded_module.resolved_fqcn in workarounds_inject_map: + module_args.update(workarounds_inject_map[loaded_module.resolved_fqcn]) + if loaded_module.resolved_fqcn in workarounds_drop_map: + for key in workarounds_drop_map[loaded_module.resolved_fqcn]: + if key in module_args: + del module_args[key] with mock.patch.object( mock_ansible_module, "AnsibleModule", CustomAnsibleModule @@ -148,6 +159,7 @@ def matchtask( except ValidationPassed: return [] + # pylint: disable=unused-argument def _sanitize_results( self, results: list[MatchError], module_name: str ) -> list[MatchError]: @@ -157,47 +169,6 @@ def _sanitize_results( result_msg = result.message if ignored_re.match(result_msg): continue - if result_msg.startswith("Unsupported parameters"): - # cmd option is a special case in command module and after option validation is done. - if ( - "Unsupported parameters for (basic.py) module" in result_msg - and module_name - in ["command", "ansible.builtin.command", "ansible.legacy.command"] - ): - continue - result.message = result_msg.replace("(basic.py)", f"{module_name}") - elif result_msg.startswith("missing required arguments"): - if ( - "missing required arguments: free_form" in result_msg - and module_name - in [ - "raw", - "ansible.builtin.raw", - "ansible.legacy.raw", - "meta", - "ansible.builtin.meta", - "ansible.legacy.meta", - ] - ): - # free_form option is a special case in raw module hence ignore this error. - continue - if ( - "missing required arguments: key_value" in result_msg - and module_name - in [ - "set_fact", - "ansible.builtin.set_fact", - "ansible.legacy.set_fact", - ] - ): - # handle special case for set_fact module with key and value - continue - if "Supported parameters include" in result_msg and module_name in [ - "set_fact", - "ansible.builtin.set_fact", - "ansible.legacy.set_fact", - ]: - continue sanitized_results.append(result) return sanitized_results diff --git a/test/test_skip_inside_yaml.py b/test/test_skip_inside_yaml.py index c35268d0dc..fdc553e7ce 100644 --- a/test/test_skip_inside_yaml.py +++ b/test/test_skip_inside_yaml.py @@ -55,7 +55,7 @@ def test_role_tasks_with_block(default_text_runner: RunFromText) -> None: @pytest.mark.parametrize( ("lintable", "expected"), - (pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 2, id="yaml"),), + (pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 4, id="yaml"),), ) def test_inline_skips( default_rules_collection: RulesCollection, lintable: str, expected: int