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

tags: fix unexpected exception for tags as dict #53529

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@resmo
Copy link
Member

resmo commented Mar 8, 2019

SUMMARY

I accidentally created tags as dict which failed badly in a way I didn't see where I did wrong.

- hosts: test
  gather_facts: yes
  tags:
    - { role: foo, tags: foo }

error is more helpful after the fix

ERROR! tags must be specified as a list or string
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION
$ ansible-playbook playbook.yml -vvv   
ansible-playbook 2.7.8
 config file = /home/user/Projects/ansible-project/ansible.cfg
 configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
 ansible python module location = /home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible
 executable location = /home/user/.local/share/virtualenvs/ansible-bjC9T5IV/bin/ansible-playbook
 python version = 3.6.7 (default, Oct 22 2018, 11:32:17) [GCC 8.2.0]
Using /home/user/Projects/ansible-project/ansible.cfg as config file
/home/user/Projects/ansible-project/inventories/hosts did not meet host_list requirements, check plugin documentation if this is unexpected
/home/user/Projects/ansible-project/inventories/hosts did not meet script requirements, check plugin documentation if this is unexpected
Parsed /home/user/Projects/ansible-project/inventories/hosts inventory source with ini plugin
statically imported: /home/user/Projects/ansible-project/roles/cloudscale/tasks/vm_up.yml
statically imported: /home/user/Projects/ansible-project/roles/cloudscale/tasks/remove_cloud_init.yml
ERROR! Unexpected Exception, this is probably a bug: unhashable type: 'AnsibleMapping'
the full traceback was:

Traceback (most recent call last):
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/bin/ansible-playbook", line 118, in <module>
   exit_code = cli.run()
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/cli/playbook.py", line 122, in run
   results = pbex.run()
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/executor/playbook_executor.py", line 81, in run
   pb = Playbook.load(playbook_path, variable_manager=self._variable_manager, loader=self._loader)
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/playbook/__init__.py", line 54, in load
   pb._load_playbook_data(file_name=file_name, variable_manager=variable_manager)
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/playbook/__init__.py", line 99, in _load_playbook_data
   pb = PlaybookInclude.load(entry, basedir=self._basedir, variable_manager=variable_manager, loader=self._loader)
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/playbook/playbook_include.py", line 42, in load
   return PlaybookInclude().load_data(ds=data, basedir=basedir, variable_manager=variable_manager, loader=loader)
 File "/home/user/.local/share/virtualenvs/ansible-bjC9T5IV/lib/python3.6/site-packages/ansible/playbook/playbook_include.py", line 88, in load_data
   entry.tags = list(set(entry.tags).union(new_obj.tags))

resmo added some commits Mar 8, 2019

@resmo

This comment has been minimized.

Copy link
Member Author

resmo commented Mar 8, 2019

ready_for_review

@@ -53,8 +53,11 @@ def evaluate_tags(self, only_tags, skip_tags, all_vars):
for tag in tags:
if isinstance(tag, list):
_temp_tags.update(tag)
else:
elif isinstance(tag, string_types):

This comment has been minimized.

@bcoca

bcoca Mar 8, 2019

Member

it should really always be a list by the time it gets here ... I'm thinking alternative patch:


diff --git a/lib/ansible/playbook/playbook_include.py b/lib/ansible/playbook/playbook_include.py
index ec3909e38e..461e84dc34 100644
--- a/lib/ansible/playbook/playbook_include.py
+++ b/lib/ansible/playbook/playbook_include.py
@@ -20,8 +20,8 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type

 import os
-
 from ansible.errors import AnsibleParserError, AnsibleAssertionError
+from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.six import iteritems, string_types
 from ansible.parsing.splitter import split_args, parse_kv
 from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleMapping
@@ -83,7 +83,12 @@ class PlaybookInclude(Base, Conditional, Taggable):
             temp_vars.update(new_obj.vars)
             param_tags = temp_vars.pop('tags', None)
             if param_tags is not None:
-                entry.tags.extend(param_tags.split(','))
+                if isinstance(param_tags, string_types):
+                    entry.tags.extend(param_tags.split(','))
+                elif is_sequence(param_tags):
+                    entry.tags.extend(param_tags)
+                else:
+                    raise AnsibleParserError('Invalid type supplied for tags keyword, expected list but got %s' % type(param_tags))
             entry.vars = temp_vars
             entry.tags = list(set(entry.tags).union(new_obj.tags))
             if entry._included_path is None:```

This comment has been minimized.

@resmo

resmo Mar 9, 2019

Author Member

tags is a list but, the tags list item may be a list as well

- debug: msg=test
  tags:
  - [ one, two, three ]

This comment has been minimized.

@bcoca

bcoca Mar 14, 2019

Member

so the check should really be 'list of strings' ... with a flatten for backwards compat

@ansibot ansibot added core_review and removed needs_revision labels Mar 8, 2019

@ansibot ansibot added needs_revision and removed core_review labels Mar 9, 2019

resmo added some commits Mar 14, 2019

@ansibot ansibot removed the ci_verified label Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.