cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix notifying handlers by using an exact match (#55624)

Browse source 
* Fix notifying handlers by using an exact match rather than a string subset if listen is text rather than a list

* Enforce better type checking for listeners

* Share code for validating handler listeners

* Add test for handlers without names

* Add test for templating in handlers

* Add test for include_role

* Add a couple notes about 'listen' for handlers

* changelog

* Add a test for handlers without names

* Test templating in handlers

* changelog

* Add some tests for include_role

* Add a couple notes about 'listen' for handlers

* make more sense

* move local function into a class method
This commit is contained in:
Sloane Hertel 2019-06-27 17:29:20 -04:00 committed by GitHub
parent ee52b60d7d
commit ec1287ca7e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 153 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
changelogs/fragments/55575_notify_handlers_by_exact_match_only.yaml Normal file
View file

@ -0,0 +1,3 @@
bugfixes:
- handlers - Only notify a handler if the handler is an exact match by ensuring `listen` is a list of strings.
(https://github.com/ansible/ansible/issues/55575)

2
docs/docsite/keyword_desc.yml
View file

@ -34,7 +34,7 @@ force_handlers: Will force notified handler execution for hosts even if they fai
gather_facts: "A boolean that controls if the play will automatically run the 'setup' task to gather facts for the hosts."
gather_subset: Allows you to pass subset options to the fact gathering plugin controlled by :term:`gather_facts`.
gather_timeout: Allows you to set the timeout for the fact gathering plugin controlled by :term:`gather_facts`.
handlers: "A section with tasks that are treated as handlers, these won't get executed normally, only when notified after each section of tasks is complete."
handlers: "A section with tasks that are treated as handlers, these won't get executed normally, only when notified after each section of tasks is complete. A handler's `listen` field is not templatable."
hosts: "A list of groups, hosts or host pattern that translates into a list of hosts that are the play's target."
ignore_errors: Boolean that allows you to ignore task failures and continue with play. It does not affect connection errors.
ignore_unreachable: Boolean that allows you to ignore unreachable hosts and continue with play. This does not affect other task errors (see :term:`ignore_errors`) but is useful for groups of volatile/ephemeral hosts.

1
docs/docsite/rst/user_guide/playbooks_intro.rst
View file

@ -455,6 +455,7 @@ a shared source like Galaxy).
.. note::
* Notify handlers are always run in the same order they are defined, `not` in the order listed in the notify-statement. This is also the case for handlers using `listen`.
* Handler names and `listen` topics live in a global namespace.
* Handler names are templatable and `listen` topics are not.
* Use unique handler names. If you trigger more than one handler with the same name, the first one(s) get overwritten. Only the last one defined will run.
* You cannot notify a handler that is defined inside of an include. As of Ansible 2.1, this does work, however the include must be `static`.

100
lib/ansible/playbook/base.py
View file

@ -334,6 +334,57 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)):
return new_me
def get_validated_value(self, name, attribute, value, templar):
if attribute.isa == 'string':
value = to_text(value)
elif attribute.isa == 'int':
value = int(value)
elif attribute.isa == 'float':
value = float(value)
elif attribute.isa == 'bool':
value = boolean(value, strict=False)
elif attribute.isa == 'percent':
# special value, which may be an integer or float
# with an optional '%' at the end
if isinstance(value, string_types) and '%' in value:
value = value.replace('%', '')
value = float(value)
elif attribute.isa == 'list':
if value is None:
value = []
elif not isinstance(value, list):
value = [value]
if attribute.listof is not None:
for item in value:
if not isinstance(item, attribute.listof):
raise AnsibleParserError("the field '%s' should be a list of %s, "
"but the item '%s' is a %s" % (name, attribute.listof, item, type(item)), obj=self.get_ds())
elif attribute.required and attribute.listof == string_types:
if item is None or item.strip() == "":
raise AnsibleParserError("the field '%s' is required, and cannot have empty values" % (name,), obj=self.get_ds())
elif attribute.isa == 'set':
if value is None:
value = set()
elif not isinstance(value, (list, set)):
if isinstance(value, string_types):
value = value.split(',')
else:
# Making a list like this handles strings of
# text and bytes properly
value = [value]
if not isinstance(value, set):
value = set(value)
elif attribute.isa == 'dict':
if value is None:
value = dict()
elif not isinstance(value, dict):
raise TypeError("%s is not a dictionary" % value)
elif attribute.isa == 'class':
if not isinstance(value, attribute.class_type):
raise TypeError("%s is not a valid %s (got a %s instead)" % (name, attribute.class_type, type(value)))
value.post_validate(templar=templar)
return value
def post_validate(self, templar):
'''
we can't tell that everything is of the right type until we have
@ -389,54 +440,7 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)):
# and make sure the attribute is of the type it should be
if value is not None:
if attribute.isa == 'string':
value = to_text(value)
elif attribute.isa == 'int':
value = int(value)
elif attribute.isa == 'float':
value = float(value)
elif attribute.isa == 'bool':
value = boolean(value, strict=False)
elif attribute.isa == 'percent':
# special value, which may be an integer or float
# with an optional '%' at the end
if isinstance(value, string_types) and '%' in value:
value = value.replace('%', '')
value = float(value)
elif attribute.isa == 'list':
if value is None:
value = []
elif not isinstance(value, list):
value = [value]
if attribute.listof is not None:
for item in value:
if not isinstance(item, attribute.listof):
raise AnsibleParserError("the field '%s' should be a list of %s, "
"but the item '%s' is a %s" % (name, attribute.listof, item, type(item)), obj=self.get_ds())
elif attribute.required and attribute.listof == string_types:
if item is None or item.strip() == "":
raise AnsibleParserError("the field '%s' is required, and cannot have empty values" % (name,), obj=self.get_ds())
elif attribute.isa == 'set':
if value is None:
value = set()
elif not isinstance(value, (list, set)):
if isinstance(value, string_types):
value = value.split(',')
else:
# Making a list like this handles strings of
# text and bytes properly
value = [value]
if not isinstance(value, set):
value = set(value)
elif attribute.isa == 'dict':
if value is None:
value = dict()
elif not isinstance(value, dict):
raise TypeError("%s is not a dictionary" % value)
elif attribute.isa == 'class':
if not isinstance(value, attribute.class_type):
raise TypeError("%s is not a valid %s (got a %s instead)" % (name, attribute.class_type, type(value)))
value.post_validate(templar=templar)
value = self.get_validated_value(name, attribute, value, templar)
# and assign the massaged value back to the attribute field
setattr(self, name, value)

3
lib/ansible/playbook/handler.py
View file

@ -21,11 +21,12 @@ __metaclass__ = type
from ansible.playbook.attribute import FieldAttribute
from ansible.playbook.task import Task
from ansible.module_utils.six import string_types
class Handler(Task):
_listen = FieldAttribute(isa='list', default=list)
_listen = FieldAttribute(isa='list', default=list, listof=string_types, static=True)
def __init__(self, block=None, role=None, task_include=None):
self.notified_hosts = []

11
lib/ansible/plugins/strategy/__init__.py
View file

@ -350,6 +350,10 @@ class StrategyBase:
return [actual_host]
def get_handler_templar(self, handler_task, iterator):
handler_vars = self._variable_manager.get_vars(play=iterator._play, task=handler_task)
return Templar(loader=self._loader, variables=handler_vars)
@debug_closure
def _process_pending_results(self, iterator, one_pass=False, max_passes=None):
'''
@ -373,8 +377,7 @@ class StrategyBase:
for handler_task in handler_block.block:
if handler_task.name:
if not handler_task.cached_name:
handler_vars = self._variable_manager.get_vars(play=iterator._play, task=handler_task)
templar = Templar(loader=self._loader, variables=handler_vars)
templar = self.get_handler_templar(handler_task, iterator)
handler_task.name = templar.template(handler_task.name)
handler_task.cached_name = True
@ -530,6 +533,10 @@ class StrategyBase:
for listening_handler_block in iterator._play.handlers:
for listening_handler in listening_handler_block.block:
listeners = getattr(listening_handler, 'listen', []) or []
templar = self.get_handler_templar(listening_handler, iterator)
listeners = listening_handler.get_validated_value(
'listen', listening_handler._valid_attrs['listen'], listeners, templar
)
if handler_name not in listeners:
continue
else:

15
test/integration/targets/handlers/roles/test_templating_in_handlers/handlers/main.yml Normal file
View file

@ -0,0 +1,15 @@
---
- name: name1
set_fact:
role_non_templated_name: True
- name: "{{ handler2 }}"
set_fact:
role_templated_name: True
- name: testlistener1
set_fact:
role_non_templated_listener: True
listen: name3
- name: testlistener2
set_fact:
role_templated_listener: True
listen: "{{ handler4 }}"

16
test/integration/targets/handlers/roles/test_templating_in_handlers/tasks/main.yml Normal file
View file

@ -0,0 +1,16 @@
---
- command: echo Hello World
notify:
- "{{ handler1 }}"
- "{{ handler2 }}"
- "{{ handler3 }}"
- "{{ handler4 }}"
- meta: flush_handlers
- assert:
that:
- role_non_templated_name is defined
- role_templated_name is defined
- role_non_templated_listener is defined
- role_templated_listener is undefined

2
test/integration/targets/handlers/runme.sh
View file

@ -66,6 +66,8 @@ grep -q "ERROR! The requested handler 'notify_inexistent_handler' was not found
# Notify inexistent handlers without errors when ANSIBLE_ERROR_ON_MISSING_HANDLER=false
ANSIBLE_ERROR_ON_MISSING_HANDLER=false ansible-playbook test_handlers_inexistent_notify.yml -i inventory.handlers -v "$@"
ANSIBLE_ERROR_ON_MISSING_HANDLER=false ansible-playbook test_templating_in_handlers.yml -v "$@"
# https://github.com/ansible/ansible/issues/36649
output_dir=/tmp
set +e

9
test/integration/targets/handlers/test_handlers_listen.yml
View file

@ -13,6 +13,7 @@
that:
- "notify_listen_ran_1_1 is defined"
- "notify_listen_ran_1_2 is defined"
- "notify_listen_ran_1_3 is undefined"
handlers:
- name: notify_handler_ran_1_1
set_fact:
@ -22,6 +23,10 @@
set_fact:
notify_listen_ran_1_2: True
listen: notify_listen
- name: notify_handler_ran_1_3
set_fact:
notify_handler_ran_1_3: True
listen: notify_listen2
- name: test listen unnamed handlers
hosts: localhost
@ -38,6 +43,7 @@
that:
- "notify_listen_ran_1 is defined"
- "notify_listen_ran_2 is defined"
- "notify_listen_ran_3 is undefined"
handlers:
- set_fact:
notify_listen_ran_1: True
@ -45,6 +51,9 @@
- set_fact:
notify_listen_ran_2: True
listen: notify_listen
- set_fact:
notify_handler_ran_3: True
listen: notify_listen2
- name: test with mixed notify by name and listen
hosts: localhost

43
test/integration/targets/handlers/test_templating_in_handlers.yml Normal file
View file

@ -0,0 +1,43 @@
- name: test templated values in handlers
hosts: localhost
gather_facts: no
vars:
handler1: name1
handler2: name2
handler3: name3
handler4: name4
handlers:
- name: name1
set_fact:
non_templated_name: True
- name: "{{ handler2 }}"
set_fact:
templated_name: True
- name: testlistener1
set_fact:
non_templated_listener: True
listen: name3
- name: testlistener2
set_fact:
templated_listener: True
listen: "{{ handler4 }}"
tasks:
- command: echo Hello World
notify:
- "{{ handler1 }}"
- "{{ handler2 }}"
- "{{ handler3 }}"
- "{{ handler4 }}"
- meta: flush_handlers
- assert:
that:
- non_templated_name is defined
- templated_name is defined
- non_templated_listener is defined
- templated_listener is undefined
- include_role: name=test_templating_in_handlers