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

name[play]: now also correctly works with import_playbook blocks #2359

Merged
merged 2 commits into from Aug 30, 2022

Conversation

apatard
Copy link
Contributor

@apatard apatard commented Aug 29, 2022

When running ansible-lint on a playbook like:

---
- ansible.builtin.import_playbook: "test.yml"

ansible-lint is now "complaining":

WARNING  Ignored exception from NameRule.<bound method AnsibleLintRule.matchyaml of name: Rule for checking task and play names.>: 'name'

The test in src/ansiblelint/rules/name.py:matchplay() is trying to ignore
the import_playbook case but with current test leads to the line:

data["name"], lintable=file, linenumber=data[LINE_NUMBER_KEY]

and in this example, the key "name" doesn't exist, leading to the warning.

The proposed solution here is to split the test into two and return
no error if "name" is not set for import_playbook.

[ Not sure what's currently best practice with import_playbook:
should a name be set or not ? ]

Signed-off-by: Arnaud Patard apatard@hupstream.com

@ssbarnea
Copy link
Member

Older versions did not check for it but based on my discussions with @cidrblock i think it would be better to require it here too, for documentation purposes. At this moment ansible does not display the name for these in the UI but this may also change in the future.

When running ansible-lint on a playbook like:

```
---
- ansible.builtin.import_playbook: "test.yml"
```

ansible-lint is now "complaining":

```
WARNING  Ignored exception from NameRule.<bound method AnsibleLintRule.matchyaml of name: Rule for checking task and play names.>: 'name'
```

The test ``in src/ansiblelint/rules/name.py:matchplay()`` is trying to ignore
the import_playbook case but with current test leads to the line:

```
data["name"], lintable=file, linenumber=data[LINE_NUMBER_KEY]
```

and in this example, the key "name" doesn't exist, leading to the warning.

The proposed solution is to suppress the special casing.

Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Now that ansible-lint is giving an error for plays without name
for import_playbook, add missing name.

Signed-off-by: Arnaud Patard <apatard@hupstream.com>
@apatard apatard force-pushed the import_playbook_namecheck_fix branch from 0b9dd0c to a0ce213 Compare August 30, 2022 07:47
@ssbarnea ssbarnea changed the title Fix "Raise name[play] for plays missing a name" name[play]: now also correctly works with import_playbook blocks Aug 30, 2022
@ssbarnea ssbarnea merged commit 43584ac into ansible:main Aug 30, 2022
hswong3i added a commit to alvistack/ansible-collection-kubernetes that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/ansible-collection-ceph that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/ansible-collection-gitlab that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/ansible-collection-gnome that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-centos that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-ceph that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-debian that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-gitlab-runner that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-fedora that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-kubernetes that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-opensuse that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-ubuntu that referenced this pull request Sep 2, 2022
hswong3i added a commit to alvistack/vagrant-rhel that referenced this pull request Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants