False positive no-handler with and in when

[MarcinWieczorek]

Summary

no-handler rule recommends to use handlers when when is an and condition.
Example: when: r1.changed and r2.changed.
Afaik handlers can only be used for single conditions and using notify on multiple tasks acts as a logical or, but here logical and is needed.
I have discussed this issue some time ago on IRC and we came to the conclusion that this is a bug. Forgive me if I missed something.

Issue Type

• Bug Report

Ansible and Ansible Lint details

$ ansible --version
ansible 2.10.7
  config file = /home/censored/.config/ansible/ansible.cfg
  configured module search path = ['/home/censored/.local/share/ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  executable location = /sbin/ansible
  python version = 3.9.2 (default, Feb 20 2021, 18:40:11) [GCC 10.2.0]

$ ansible-lint --version
ansible-lint 5.0.7 using ansible 2.10.7

• ansible installation method: OS package
• ansible-lint installation method: source (pip install . in venv)

OS / ENVIRONMENT

Linux samluks 5.11.11-arch1-1 SMP PREEMPT Tue, 30 Mar 2021 14:10:17 +0000 x86_64 GNU/Linux

STEPS TO REPRODUCE

• use provided playbook (when: r1.changed and r2.changed)
• run ansible-lint on it
• get no-handler rule violation

Playbook at examples/playbooks/rule-no-handler-and.yml

- name: no-handler testcase
  hosts: managed
  tasks:
    - name: registering task 1
      command: echo Hello
      register: r1
      changed_when: true
    
    - name: registering task 2
      command: echo Hello
      register: r2
      changed_when: true
  
    - name: when task
      command: echo Hello
      when: r1.changed and r2.changed

Desired Behaviour

No warning given as this case seems to be impossible to be solved with handlers.

Actual Behaviour

$ ansible-lint examples/playbooks/rule-no-handler-and.yml
WARNING  Listing 1 violation(s) that are fatal
no-handler: Tasks that run when changed should likely be handlers
examples/playbooks/rule-no-handler-and.yml:14 Task/Handler: when task

You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - no-handler  # Tasks that run when changed should likely be handlers
Finished with 1 failure(s), 0 warning(s) on 1 files.

Testcase

Assumes playbook given above (that should not show warnings)

diff --git a/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py b/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
index ae564aa..89d65e4 100644
--- a/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
+++ b/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
@@ -87,3 +87,13 @@ if 'pytest' in sys.modules:
         assert results[1].linenumber == 14
         assert results[2].filename == 'examples/playbooks/rule-no-handler.yml'
         assert results[2].linenumber == 18
+
+    def test_rule_no_handler_and() -> None:
+        """Verify rule when using 'and'. Should not fail"""
+        collection = RulesCollection()
+        collection.register(UseHandlerRatherThanWhenChangedRule())
+
+        lintable = Lintable('examples/playbooks/rule-no-handler-and.yml')
+        results = Runner(lintable, rules=collection).run()
+
+        assert len(results) == 0

Possible fix

Searching for string and would be bad for variable names with that string inside (legal).
Searching for and (surrounded with spaces) seems to be a better idea, but I'm not sure what are the other options. There could be a line break or something.

diff --git a/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py b/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
index ae564aa..89d65e4 100644
--- a/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
+++ b/src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
@@ -27,7 +27,7 @@ from ansiblelint.rules import AnsibleLintRule
 def _changed_in_when(item: str) -> bool:
     if not isinstance(item, str):
         return False
-    return any(
+    return ' and ' not in item and any(
         changed in item
         for changed in [
             '.changed',

[konstruktoid]

Hi @MarcinWieczorek, can you supply a playbook with an example of when logical and is used but ansible treats it as a logical or and the use of handlers isn't possible?

Using when: r1.changed and r2.changed with ansible [core 2.11.0] and the below playbook seems to work fine.

- name: handler 'and' testcase
  hosts: localhost
  tasks:
    - name: registering task 1
      command: echo "Task 1"
      register: r1
      changed_when: true
    
    - name: registering task 2
      command: echo "Task 2"
      register: r2
      changed_when: false
  
    - name: registering task 3
      command: echo "Task 3"
      register: r3
      changed_when: true
  
    - name: registering handler task 1
      command: echo "Handler task 1"
      register: r4
      when: r1.changed and (not r2.changed and r3.changed)
      notify:
        - Debug handler task

  handlers:
    - name: Debug handler task
      debug:
        msg: "r1: {{ r1.stdout_lines }} r2: {{ r2.stdout_lines }} r3: {{ r3.stdout_lines }} r4: {{ r4.stdout_lines }}"

PLAY [handler 'and' testcase] *******************************************************

TASK [Gathering Facts] **************************************************************
ok: [127.0.0.1]

TASK [registering task 1] *************************************************************
changed: [127.0.0.1]

TASK [registering task 2] ************************************************************
ok: [127.0.0.1]

TASK [registering task 3] ************************************************************
changed: [127.0.0.1]

TASK [registering handler task 1] ****************************************************
changed: [127.0.0.1]

RUNNING HANDLER [Debug handler task] ******************************************
ok: [127.0.0.1] => {
    "msg": "r1: ['Task 1'] r2: ['Task 2'] r3: ['Task 3'] r4: ['Handler task 1']"
}

PLAY RECAP ************************************************************************
127.0.0.1                  : ok=6    changed=3

[MarcinWieczorek]

Your example works properly, but doesn't pass the lint:

WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: handler_and_testcase.yml
WARNING  Listing 1 violation(s) that are fatal
no-handler: Tasks that run when changed should likely be handlers
handler_and_testcase.yml:19 Task/Handler: registering handler task 1

You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - no-handler  # Tasks that run when changed should likely be handlers

Finished with 1 failure(s), 0 warning(s) on 1 files.

[konstruktoid]

Ah, I thought the issue somehow was related handlers not accepting when: and.

[MarcinWieczorek]

Thank you, I'll make sure to return with some feedback when I get a chance to test this fix.