Repository: ansible/ansible
Pull request: Allow tags to be templated from a variable
##### SUMMARY
Allow tags to be templated from a variable. Fixes #49825

The problem that we run into here, is that when we call `_extend_value` from `Task._get_parent_attribute`, we turn `'{{ foo }}'` into `['{{ foo }}']`.

Later when `{{foo}}` is templated, that becomes `[['foo', 'bar']]`

Basically, I get around this problem in this PR, by flattening `tags`.

Additionally this PR removes 2 unneeded methods `Taggable.__init__` and `Taggable._load_tags`. `_load_tags` basically duplicated the code in `evaluate_tags`, but with less available functionality.

Additionally, the `itertools.groupby` was unneeded, as it was added when tags was meant to not be a list in that context (https://github.com/ansible/ansible/commit/0eb1c880ddac9547560040311739b5ca8291a642), the `set` does what is needed.



##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request
- Docs Pull Request
- Feature Pull Request
- New Module Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->

##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```

Problem statement:
Using a variable list for tags parameter results in an unhashable type: 'list' exception
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Explain the problem briefly below -->
Providing a variable list to the tags parameter results in the following error message:
```
ERROR! Unexpected Exception, this is probably a bug: unhashable type: 'list'
```
##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->
Tags parameter
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.7.4
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, May 31 2018, 09:41:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below
Nothing changed from defaults.
```

##### OS / ENVIRONMENT
<!--- Provide all relevant information below, e.g. target OS versions, network device firmware, etc. -->
Red Hat Enterprise Linux Server release 7.5 (Maipo)
##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->
Run with `ansible-playbook -vvvv foo.yml`.
<!--- Paste example playbooks or commands between quotes below -->
```yaml
- hosts: all
  remote_user: root
  gather_facts: no
  vars:
    foo: ['foo', 'bar']
  tasks:
  - debug:
      msg: "{{ foo }}"
    tags: "{{ foo }}"
```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- Describe what you expected to happen when running the steps above -->
Expect no error/exception.

##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes -->
```paste below
ansible-playbook 2.7.4
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-playbook
  python version = 2.7.5 (default, May 31 2018, 09:41:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
/etc/ansible/hosts did not meet host_list requirements, check plugin documentation if this is unexpected
/etc/ansible/hosts did not meet script requirements, check plugin documentation if this is unexpected
Set default localhost to localhost
Parsed /etc/ansible/hosts inventory source with ini plugin
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python2.7/site-packages/ansible/plugins/callback/default.pyc

PLAYBOOK: foo.yml ***************************************************************************************************************************************************************************************************************************
1 plays in foo.yml

PLAY [all] **********************************************************************************************************************************************************************************************************************************
ERROR! Unexpected Exception, this is probably a bug: unhashable type: 'list'
the full traceback was:

Traceback (most recent call last):
  File "/usr/bin/ansible-playbook", line 118, in <module>
    exit_code = cli.run()
  File "/usr/lib/python2.7/site-packages/ansible/cli/playbook.py", line 122, in run
    results = pbex.run()
  File "/usr/lib/python2.7/site-packages/ansible/executor/playbook_executor.py", line 156, in run
    result = self._tqm.run(play=play)
  File "/usr/lib/python2.7/site-packages/ansible/executor/task_queue_manager.py", line 263, in run
    start_at_done=self._start_at_done,
  File "/usr/lib/python2.7/site-packages/ansible/executor/play_iterator.py", line 197, in __init__
    new_block = block.filter_tagged_tasks(play_context, all_vars)
  File "/usr/lib/python2.7/site-packages/ansible/playbook/block.py", line 394, in filter_tagged_tasks
    return evaluate_block(self)
  File "/usr/lib/python2.7/site-packages/ansible/playbook/block.py", line 389, in evaluate_block
    new_block.block = evaluate_and_append_task(block.block)
  File "/usr/lib/python2.7/site-packages/ansible/playbook/block.py", line 383, in evaluate_and_append_task
    task.evaluate_tags(play_context.only_tags, play_context.skip_tags, all_vars=all_vars)):
  File "/usr/lib/python2.7/site-packages/ansible/playbook/taggable.py", line 63, in evaluate_tags
    tags = set([i for i, _ in itertools.groupby(tags)])
TypeError: unhashable type: 'list'
```

Hints from issue discussion:
Files identified in the description:
None

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->

Commit message: Allow tags to be templated from a variable. Fixes #49825

Patch to review:
diff --git a/changelogs/fragments/tags-var.yaml b/changelogs/fragments/tags-var.yaml
new file mode 100644
index 00000000000000..40c581cd723748
--- /dev/null
+++ b/changelogs/fragments/tags-var.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- tags - allow tags to be specified by a variable (https://github.com/ansible/ansible/issues/49825)
diff --git a/lib/ansible/playbook/taggable.py b/lib/ansible/playbook/taggable.py
index 38b43620a0034c..39971a5f7cf86e 100644
--- a/lib/ansible/playbook/taggable.py
+++ b/lib/ansible/playbook/taggable.py
@@ -19,8 +19,6 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import itertools
-
 from ansible.errors import AnsibleError
 from ansible.module_utils.six import string_types
 from ansible.playbook.attribute import FieldAttribute
@@ -32,21 +30,6 @@ class Taggable:
     untagged = frozenset(['untagged'])
     _tags = FieldAttribute(isa='list', default=list, listof=(string_types, int), extend=True)
 
-    def __init__(self):
-        super(Taggable, self).__init__()
-
-    def _load_tags(self, attr, ds):
-        if isinstance(ds, list):
-            return ds
-        elif isinstance(ds, string_types):
-            value = ds.split(',')
-            if isinstance(value, list):
-                return [x.strip() for x in value]
-            else:
-                return [ds]
-        else:
-            raise AnsibleError('tags must be specified as a list', obj=ds)
-
     def evaluate_tags(self, only_tags, skip_tags, all_vars):
         ''' this checks if the current item should be executed depending on tag options '''
 
@@ -60,7 +43,14 @@ def evaluate_tags(self, only_tags, skip_tags, all_vars):
                 else:
                     tags = set([tags])
             else:
-                tags = set([i for i, _ in itertools.groupby(tags)])
+                _temp_tags = set()
+                for tag in tags:
+                    if isinstance(tag, list):
+                        _temp_tags.update(tag)
+                    else:
+                        _temp_tags.add(tag)
+                tags = _temp_tags
+            self.tags = list(tags)
         else:
             # this makes isdisjoint work for untagged
             tags = self.untagged


Code review comments:
diff_hunk: @@ -32,21 +30,6 @@ class Taggable:
     untagged = frozenset(['untagged'])
     _tags = FieldAttribute(isa='list', default=list, listof=(string_types, int), extend=True)
 
-    def __init__(self):
-        super(Taggable, self).__init__()
-
-    def _load_tags(self, attr, ds):
-        if isinstance(ds, list):
-            return ds
-        elif isinstance(ds, string_types):
-            value = ds.split(',')
path: lib/ansible/playbook/taggable.py
text: @user1:
comma-separated lists won't work anymore

@author:
That is handled already in `evaluate_tags`.  It's just not visible in this PR since I didn't change it.

@user1:
Tags is a list though. But it's never iterated over with the string types being separated by ','. I don't actually see where that `if not isinstance(tags, list):` in evaluate_tags would be true.

@user1:
Simple test case:
```
---
- hosts: localhost
  connection: local
  gather_facts: no
  tasks:
  - debug:
        var: ansible_host
    tags: one,two,three
```
Works on devel, but not this branch.

@author:
Due to where `tags` is used, `post_validate` doesn't ensure it's a list before use.  `evaluate_tags` happens before that.  We were doing it in `_load_tags` which happened before `post_validate` too.

As of now it works to do `tags: foo,bar`.  We could go through some deprecation period to disallow using a CSV, but not really related to this PR.

@user1:
Devel:
```
(python2.7.13) 11:15:27 [ansible]$ ansible-playbook test_tag.yml --tags one
 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match
'all'


PLAY [localhost] *******************************************************************************************************

TASK [debug] ***********************************************************************************************************
ok: [localhost] => {
    "ansible_host": "127.0.0.1"
}

PLAY RECAP *************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0
```
This branch:
```
(python2.7.13) 11:16:07 [ansible]$ ansible-playbook test_tag.yml --tags one
 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match
'all'


PLAY [localhost] *******************************************************************************************************

PLAY RECAP *************************************************************************************************************

```

@author:
I think this helps.  I was misunderstanding what you were saying.  I must have overlooked something.  I'll see about deduping code but adding `_load_tags` back.

@user2:
we want to keep `tags: x,y` we allow 'string comma lists' everywhere we allow lists, not sure why we want to make exception here