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

Add a rule to validate module options #2749

Merged
merged 1 commit into from Dec 8, 2022

Conversation

ganeshrn
Copy link
Member

The new rule will validate the correctness
of module options for tasks. The validation
will check if the option name is valid and
has correct value along with conditionals like

  • mutually_exclusive
  • required_together
  • required_one_of
  • required_if

Note:

  • For template values for options the data type validation will be ignored
  • For modules that are implemented as action plugins the check includes
    if the option is correct and passes the data type check or not.

@ganeshrn ganeshrn requested review from a team as code owners November 28, 2022 13:37
@ganeshrn ganeshrn added the new rule A request for a new rule label Nov 28, 2022
@ganeshrn ganeshrn marked this pull request as draft November 28, 2022 14:32
@ssbarnea
Copy link
Member

ssbarnea commented Dec 4, 2022

@ganeshrn How far are you from making it ready for review?

@ganeshrn
Copy link
Member Author

ganeshrn commented Dec 5, 2022

@ganeshrn I will get the PR ready for review soon most likely by end of this week.

@ganeshrn ganeshrn marked this pull request as ready for review December 5, 2022 04:19
@ganeshrn
Copy link
Member Author

ganeshrn commented Dec 5, 2022

The PR is ready for review.

@ganeshrn ganeshrn force-pushed the validate_module_options branch 4 times, most recently from 09ae5aa to 475d501 Compare December 8, 2022 11:35
ssbarnea added a commit to ssbarnea/ansible-lint that referenced this pull request Dec 8, 2022
This should make it easier to reuse these utility functions.

Related: ansible#2749
ssbarnea added a commit that referenced this pull request Dec 8, 2022
This should make it easier to reuse these utility functions.

Related: #2749
The new rule will validate the correctness
of module options for tasks. The validation
will check if the option name is valid and
has correct value along with conditionals like
* mutually_exclusive
* required_together
* required_one_of
* required_if

Note:
*  For template values for options the data type validation will be ignored
*  For modules that are implemented as action plugins the check includes
   if the option is correct and passes the data type check or not.
@ssbarnea ssbarnea added this to the 6.10.x milestone Dec 8, 2022
@hswong3i
Copy link
Contributor

This new rule is not such friendly, which forcing me to disable it with ansible-lint 6.10.0 :-(

My problematic code (https://github.com/alvistack/ansible-role-ansible/blob/272ce69153b7ddda0a36f8e05e026feaba2cf86e/tasks/debian.yml#L17-L25):

- name: apt-key add
  ansible.builtin.apt_key:
    keyring: "{{ item.keyring }}"
    url: "{{ item.url }}"
    id: "{{ item.id }}"
    state: "{{ item.state }}"
  loop: "{{ _apt_key }}"
  register: result
  until: result is succeeded

For multiple OS support, apt_key is referenced from https://github.com/alvistack/ansible-role-ansible/blob/272ce69153b7ddda0a36f8e05e026feaba2cf86e/vars/ubuntu-22.04.yml#L19-L23:

_apt_key:
  - keyring: "/etc/apt/trusted.gpg.d/home_alvistack.gpg"
    url: "http://downloadcontent.opensuse.org/repositories/home:/alvistack/xUbuntu_22.04/Release.key"
    id: "789CFFDE0295B8A1F4E5690C4BECC97550D0B1FD"
    state: "present"

With ansible-lint 6.10.0 now it generate follow WARNING Listing 9 violation(s) that are fatal for me:

$ ansible-lint 
WARNING  Listing 9 violation(s) that are fatal
args[module]: value of state must be one of: absent, present, got: {{ item.state }} (warning)
tasks/debian.yml:17 Task/Handler: apt-key add

args[module]: value of state must be one of: absent, present, got: {{ item.state }} (warning)
tasks/debian.yml:27 Task/Handler: apt-add-repository

args[module]: value of state must be one of: absent, build-dep, fixed, latest, present, got: {{ item.state }} (warning)
tasks/debian.yml:46 Task/Handler: apt-get install

args[module]: value of state must be one of: absent, directory, file, hard, link, touch, got: {{ item.state | default('directory') }} (warning)
tasks/main.yml:56 Task/Handler: prepare directories

args[module]: value of state must be one of: absent, directory, file, hard, link, touch, got: {{ item.state | default('file') }} (warning)
tasks/main.yml:76 Task/Handler: prepare files

args[module]: value of state must be one of: absent, present, got: {{ item.state }} (warning)
tasks/redhat.yml:17 Task/Handler: rpm --import

args[module]: value of state must be one of: absent, installed, latest, present, removed, got: {{ item.state }} (warning)
tasks/redhat.yml:49 Task/Handler: yum install

args[module]: value of state must be one of: absent, present, got: {{ item.state }} (warning)
tasks/suse.yml:17 Task/Handler: rpm --import

args[module]: value of state must be one of: absent, installed, latest, present, removed, dist-upgrade, got: {{ item.state }} (warning)
tasks/suse.yml:51 Task/Handler: zypper install

You can skip specific rules or tags by adding them to your configuration file:
# .config/ansible-lint.yml
warn_list:  # or 'skip_list' to silence them completely
  - experimental  # all rules tagged as experimental

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

Passed with production profile, 5/5 star rating: 0 failure(s), 9 warning(s) on 81 files.

May I have more information why value of state must be one of: ... ? Why it shouldn't / couldn't pass as variable?

hswong3i added a commit to alvistack/docker-devel that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-debian that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-confluence that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-dnsmasq that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-fedora that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-jira that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-fisheye that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-mitmproxy that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-gitlab-ce that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-httpd that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-rhel that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-php-fpm that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-mariadb that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-postfix that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-gitlab-ee that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-centos that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-opensuse that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-ceph that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-postgres that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/docker-ubuntu that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-debian that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-fedora that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-rhel that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-devel that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-kubernetes that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-ubuntu that referenced this pull request Dec 15, 2022
hswong3i added a commit to alvistack/vagrant-opensuse that referenced this pull request Dec 15, 2022
@gone-for-coding
Copy link

See this bug report as well: #3200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new rule A request for a new rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants