Add experimental rule for run_once (#2626)

Co-authored-by: Ajinkya <audgirka@redhat.com>

.github/workflows/tox.yml

@@ -166,7 +166,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 708
+      PYTEST_REQPASS: 710
 
     steps:
       - name: Activate WSL1

examples/playbooks/run-once-fail.yml

@@ -0,0 +1,10 @@
+---
+- name: "Example with run_once"
+  hosts: all
+  strategy: free # <-- avoid use of strategy as free
+  gather_facts: false
+  tasks:
+    - name: Task with run_once
+      ansible.builtin.debug:
+        msg: "Test"
+      run_once: true # <-- avoid use of strategy as free at play level when using run_once at task level

examples/playbooks/run-once-pass.yml

@@ -0,0 +1,8 @@
+---
+- name: "Example without run_once"
+  hosts: all
+  gather_facts: false
+  tasks:
+    - name: Task without run_once
+      ansible.builtin.debug:
+        msg: "Test"

src/ansiblelint/rules/no_free_form.py

@@ -1,4 +1,4 @@
-"""Implementation of NoFreeFormRune."""
+"""Implementation of NoFreeFormRule."""
 from __future__ import annotations
 
 import re
@@ -14,7 +14,7 @@
     from ansiblelint.file_utils import Lintable
 
 
-class NoFreeFormRune(AnsibleLintRule):
+class NoFreeFormRule(AnsibleLintRule):
     """Rule for detecting discouraged free-form syntax for action modules."""
 
     id = "no-free-form"
@@ -106,5 +106,5 @@ def test_rule_no_free_form(
         results = Runner(file, rules=default_rules_collection).run()
 
         for result in results:
-            assert result.rule.id == NoFreeFormRune.id, result
+            assert result.rule.id == NoFreeFormRule.id, result
         assert len(results) == expected

src/ansiblelint/rules/run_once.md

@@ -0,0 +1,41 @@
+# run-once
+
+This rule warns against the use of `run_once` when `strategy` is set to `free`.
+
+This rule can produce the following messages:
+
+- `run_once[play]`: Play uses `strategy: free`.
+- `run_once[task]`: Using `run_once` may behave differently if `strategy` is set to `free`.
+
+For more information see the following topics in Ansible documentation:
+
+- [free strategy](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/free_strategy.html#free-strategy)
+- [selecting a strategy](https://docs.ansible.com/ansible/latest/user_guide/playbooks_strategies.html#selecting-a-strategy)
+- [run_once(playbook keyword) more info](https://docs.ansible.com/ansible/latest/reference_appendices/playbooks_keywords.html)
+
+## Problematic Code
+
+```yaml
+---
+- name: "Example with run_once"
+  hosts: all
+  strategy: free # <-- avoid use of strategy as free
+  gather_facts: false
+  tasks:
+    - name: Task with run_once
+      ansible.builtin.debug:
+        msg: "Test"
+      run_once: true # <-- avoid use of strategy as free at play level when using run_once at task level
+```
+
+## Correct Code
+
+```yaml
+- name: "Example without run_once"
+  hosts: all
+  gather_facts: false
+  tasks:
+    - name: Task without run_once
+      ansible.builtin.debug:
+        msg: "Test"
+```

src/ansiblelint/rules/run_once.py

@@ -0,0 +1,84 @@
+"""Optional Ansible-lint rule to warn use of run_once with strategy free."""
+from __future__ import annotations
+
+import sys
+from typing import TYPE_CHECKING, Any
+
+from ansiblelint.errors import MatchError
+from ansiblelint.rules import AnsibleLintRule
+
+if TYPE_CHECKING:
+    from ansiblelint.file_utils import Lintable
+
+
+class RunOnce(AnsibleLintRule):
+    """Run once should use strategy other than free."""
+
+    id = "run-once"
+    link = "https://docs.ansible.com/ansible/latest/reference_appendices/playbooks_keywords.html"
+    description = "When using run_once, we should avoid using strategy as free."
+
+    tags = ["idiom", "experimental"]
+    severity = "MEDIUM"
+
+    def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
+        """Return matches found for a specific playbook."""
+        # If the Play uses the 'strategy' and it's value is set to free
+
+        if not file or file.kind != "playbook" or not data:
+            return []
+
+        strategy = data.get("strategy", None)
+        run_once = data.get("run_once", False)
+        if (not strategy and not run_once) or strategy != "free":
+            return []
+        return [
+            self.create_matcherror(
+                message="Play uses strategy: free",
+                filename=file,
+                tag="run_once[play]",
+            )
+        ]
+
+    def matchtask(
+        self, task: dict[str, Any], file: Lintable | None = None
+    ) -> list[MatchError]:
+        """Return matches for a task."""
+        if not file or file.kind != "playbook":
+            return []
+
+        run_once = task.get("run_once", False)
+        if not run_once:
+            return []
+        return [
+            self.create_matcherror(
+                message="Using run_once may behave differently if strategy is set to free.",
+                filename=file,
+                tag="run_once[task]",
+            )
+        ]
+
+
+# testing code to be loaded only with pytest or when executed the rule file
+if "pytest" in sys.modules:
+
+    import pytest
+
+    from ansiblelint.rules import RulesCollection  # pylint: disable=ungrouped-imports
+    from ansiblelint.runner import Runner  # pylint: disable=ungrouped-imports
+
+    @pytest.mark.parametrize(
+        ("test_file", "failure"),
+        (
+            pytest.param("examples/playbooks/run-once-pass.yml", 0, id="pass"),
+            pytest.param("examples/playbooks/run-once-fail.yml", 2, id="fail"),
+        ),
+    )
+    def test_run_once(
+        default_rules_collection: RulesCollection, test_file: str, failure: int
+    ) -> None:
+        """Test rule matches."""
+        results = Runner(test_file, rules=default_rules_collection).run()
+        for result in results:
+            assert result.rule.id == RunOnce().id
+        assert len(results) == failure

test/test_rules_collection.py

@@ -166,5 +166,5 @@ def test_rules_id_format() -> None:
             rule.help != "" or rule.description or rule.__doc__
         ), f"Rule {rule.id} must have at least one of:  .help, .description, .__doc__"
     assert "yaml" in keys, "yaml rule is missing"
-    assert len(rules) == 47  # update this number when adding new rules!
+    assert len(rules) == 48  # update this number when adding new rules!
     assert len(keys) == len(rules), "Duplicate rule ids?"