Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core module arguments are not validated #4311

Open
TheSpy0 opened this issue Sep 3, 2024 · 6 comments
Open

Core module arguments are not validated #4311

TheSpy0 opened this issue Sep 3, 2024 · 6 comments
Assignees
Labels

Comments

@TheSpy0
Copy link

TheSpy0 commented Sep 3, 2024

Summary

Incorrect module arguments no longer trigger a args[module] error for ansible.builtin modules when using any ansible-lint that is version 24.5.0 or higher, including the current version 24.7.0. This is not the case with the versions I've tested including 24.2.3, 6.22.2, and 6.14.3.

Issue Type
  • Bug Report
OS / ENVIRONMENT
Fedora release 39 (Thirty Nine)
ansible-lint 24.7.0 using ansible-core:2.16.4 ansible-compat:24.8.0 ruamel-yaml:0.18.6 ruamel-yaml-clib:0.2.8
  • ansible installation method: pip
  • ansible-lint installation method: pip
STEPS TO REPRODUCE

Run ansible-lint <insert_playbook_name>.yml when an incorrect module argument is present in a playbook and the ansible-lint version is >=24.5.0. The example below uses the ansible.builtin.dnf module, but this has also been tested with the ansible.builtin.package module. Running with a non-builtin module such as community.general.flatpak_remote did trigger the error correctly.

- name: Test using package module
  ansible.builtin.package:
    name: "{{ install_common_packages }}"
    hello:
    state: present
 
- name: Test using dnf module
  ansible.builtin.dnf:
     name: "{{ install_redhat_packages }}"
     durp: # <--- occurs with or without a value
     update_cache: true
     state: present

- name: Test using flatpak_remote module
  community.general.flatpak_remote:
    name: flathub
    hurp:
    flatpakrepo_url: https://flathub.org/repo/flathub.flatpakrepo
    enabled: true
    state: present
Desired Behavior

Output similar to the following should occur for ansible.builtin modules.

args[module]: Unsupported parameters for (basic.py) module: hello. Supported parameters include: allow_downgrade, allowerasing, autoremove, bugfix, cacheonly, conf_file, disable_excludes, disable_gpg_check, disable_plugin, disablerepo, download_dir, download_only, enable_plugin, enablerepo, exclude, install_repoquery, install_weak_deps, installroot, list, lock_timeout, name, nobest, releasever, security, skip_broken, sslverify, state, update_cache, update_only, use_backend, validate_certs (expire-cache, pkg). (warning)
desktop_packages.yml:91 Task/Handler: Install Red Hat Packages | Red Hat repositories

Read documentation for instructions on how to ignore specific rule violations.

                  Rule Violation Summary                   
 count tag          profile rule associated tags           
     1 args[module]         syntax, experimental (warning) 

Passed: 0 failure(s), 1 warning(s) on 1 files. Last profile that met the validation criteria was 'production'. Rating: 5/5 star
Actual Behavior

Please give some details of what is happening. Include a [minimum complete
verifiable example] with:

  • minimized playbook to reproduce the error
  • the output of running ansible-lint including the command line used
  • if you're getting a stack trace, also the output of
    ansible-playbook --syntax-check playbook
[the-spy@the-spy-desktop Ansible]$ ansible-lint desktop/mod_arg_test.yml 
WARNING  Listing 1 violation(s) that are fatal
args[module]: Unsupported parameters for (basic.py) module: hurp. Supported parameters include: enabled, executable, flatpakrepo_url, method, name, state. (warning)
desktop/mod_arg_test.yml:21 Task/Handler: Ensure flathub repository is enabled

Read documentation for instructions on how to ignore specific rule violations.

                  Rule Violation Summary                   
 count tag          profile rule associated tags           
     1 args[module]         syntax, experimental (warning) 

Passed: 0 failure(s), 1 warning(s) on 1 files. Last profile that met the validation criteria was 'production'. Rating: 5/5 star

See Gist for minimal playbook
https://gist.github.com/TheSpy0/c698fbe4089723c853cb4844b3592940

@TheSpy0 TheSpy0 added bug new Triage required labels Sep 3, 2024
@audgirka audgirka removed the new Triage required label Sep 18, 2024
@cavcrosby
Copy link
Contributor

cavcrosby commented Sep 22, 2024

I believe the issue you're seeing, at least for the ansible.builtin.dnf module, is due to the changes from #4131. The relevant change in the PR can be seen here, where an action plugin of the same name as the module is attempted to be loaded first. Modules that have a corresponding action plugin do not have their arguments validated. I'm basing this on the following code.

try:
if not hasattr(module, "main"):
# skip validation for module options that are implemented as action plugin
# as the option values can be changed in action plugin and are not passed
# through `ArgumentSpecValidator` class as in case of modules.
return []

@tanwigeetika1618 tanwigeetika1618 self-assigned this Sep 24, 2024
@TheSpy0
Copy link
Author

TheSpy0 commented Sep 30, 2024

It appears you are correct. I found some modules that did not have action plugins with the same name (ansible.builtin.apt and ansible.builtin.cron were a couple of them) and received warnings (see below) when executing ansible-lint that there were options set that the modules did not support.

This is the intended behavior then?

Below is an example of the ansible.builtin.apt module.

    - name: Test task
      ansible.builtin.apt:
        name: test
        sdf: stuff
        state: present
WARNING  Listing 2 violation(s) that are fatal
args[module]: Unsupported parameters for (basic.py) module: sdf. Supported parameters include: allow_change_held_packages, allow_downgrade, allow_unauthenticated, autoclean, autoremove, cache_valid_time, clean, deb, default_release, dpkg_options, fail_on_autoremove, force, force_apt_get, install_recommends, lock_timeout, only_upgrade, package, policy_rc_d, purge, state, update_cache, update_cache_retries, update_cache_retry_max_delay, upgrade (allow-downgrade, allow-downgrades, allow-unauthenticated, allow_downgrades, default-release, install-recommends, name, pkg, update-cache). (warning)

@cavcrosby
Copy link
Contributor

It's probably what's intended. Unless that is now open to change. @tanwigeetika1618 I see you assigned yourself to the issue, do you have any thoughts?

@ssbarnea
Copy link
Member

ssbarnea commented Oct 7, 2024

@cavcrosby We kinda got stuck on this one. Do you have any idea on how we can address it?

I still consider that we should be able to validate the arguments, especially for core plugins. Sadly neither ansible-playbook --syntax-check is able to detect these. I wonder if we should consider this a core bug :p

@ssbarnea ssbarnea changed the title Incorrect module arguments are not caught Core module arguments are not validated Oct 7, 2024
@ssbarnea
Copy link
Member

ssbarnea commented Oct 7, 2024

It seems that ansible/ansible#79720 might address this.

@cavcrosby
Copy link
Contributor

I think it's worth waiting to see ansible/ansible#79720 get merged in. Kicking the tires this morning (see cavcrosby@a02914e), I see a much easier path forward utilizing the changes that should come from that PR instead of trying an alternative solution. We might even be able to simplify the argument validation for modules too with these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

5 participants