Skip to content

Commit

Permalink
Replace template-instead-of-copy with avoid-implicit[copy-content] ru…
Browse files Browse the repository at this point in the history
…le (ansible#2512)

Fixes: ansible#2501
  • Loading branch information
ssbarnea authored Sep 29, 2022
1 parent c335b57 commit 0e6cdef
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
hosts: localhost
tasks:
- name: Write file content
ansible.builtin.template:
src: foo.txt.j2
ansible.builtin.copy:
content: { "foo": "bar" } # <-- avoid-implicit[copy-content]
dest: /tmp/foo.txt
mode: 0600
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
tasks:
- name: Write file content
ansible.builtin.copy:
content: "Some {{ foo }}" # <-- this is the problem
content: "Some {{ foo }}"
dest: /tmp/foo.txt
mode: 0600
5 changes: 3 additions & 2 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
from ansiblelint.loaders import yaml_from_file

DEFAULT_WARN_LIST = [
"avoid-implicit",
"experimental",
"fqcn[action]",
"fqcn[redirect]",
"jinja[spacing]", # warning until we resolve all reported false-positives
"name[casing]",
"name[play]",
"role-name",
"fqcn[action]",
"fqcn[redirect]",
]

DEFAULT_KINDS = [
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/data/profiles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ safety:
a non-determinant outcomes and cause security issues in some cases.
extends: moderate
rules:
avoid-implicit:
latest:
package-latest:
risky-file-permissions:
risky-octal:
risky-shell-pipe:
template-instead-of-copy:
shared:
extends: safety
description: >
Expand Down
37 changes: 37 additions & 0 deletions src/ansiblelint/rules/avoid_implicit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# avoid-implicit

This rule identifies the use of dangerous implicit behaviors, often also
undocumented.

This rule will produce the following type of error messages:

- `avoid-implicit[copy-content]` is not a string as [copy](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html#synopsis)
modules also accept these, but without documenting them.

## Problematic Code

```yaml
---
- name: Example playbook
hosts: localhost
tasks:
- name: Write file content
ansible.builtin.copy:
content: { "foo": "bar" } # <-- should use explicit jinja template
dest: /tmp/foo.txt
```
## Correct Code
```yaml
---
- name: Example playbook
hosts: localhost
tasks:
- name: Write file content
vars:
content: { "foo": "bar" }
ansible.builtin.copy:
content: "{{ content | to_json }}" # explicit better than implicit!
dest: /tmp/foo.txt
```
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
"""Implementation of template-instead-of-copy rule."""
# cspell:disable-next-line
# Copyright (c) 2022, Alexander Skiba
# references
# - github discussion of issue: https://github.com/ansible/ansible/issues/50580
# - ansible docs with warning: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html#synopsis
"""Implementation of avoid-implicit rule."""
# https://github.com/ansible/ansible-lint/issues/2501
from __future__ import annotations

import sys
Expand All @@ -15,26 +11,26 @@
from ansiblelint.file_utils import Lintable


class UseTemplateInsteadOfCopyRule(AnsibleLintRule):
"""Rule that identifies improper use copy instead of template."""
class AvoidImplicitRule(AnsibleLintRule):
"""Rule that identifies use of undocumented or discouraged implicit behaviors."""

id = "template-instead-of-copy"
shortdesc = "Templated files should use template instead of copy"
id = "avoid-implicit"
shortdesc = "Avoid implicit behaviors"
description = (
"Items which are templated should use ``template`` instead of "
"``copy`` with ``content`` to ensure correctness."
)
severity = "MEDIUM"
tags = ["unpredictability"]
version_added = "custom"
tags = ["unpredictability", "experimental"]
version_added = "v6.8.0"

def matchtask(
self, task: dict[str, Any], file: Lintable | None = None
) -> bool | str:
"""Confirm if current rule is matching a specific task."""
if task["action"]["__ansible_module__"] == "copy":
content = task["action"].get("content", "")
if "{{" in content:
if not isinstance(content, str):
return True
return False

Expand All @@ -46,18 +42,18 @@ def matchtask(
from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

def test_template_instead_of_copy_positive() -> None:
"""Positive test for partial-become."""
"""Positive test for avoid-implicit."""
collection = RulesCollection()
collection.register(UseTemplateInsteadOfCopyRule())
success = "examples/playbooks/rule-template-instead-of-copy-success.yml"
collection.register(AvoidImplicitRule())
success = "examples/playbooks/rule-avoid-implicit-pass.yml"
good_runner = Runner(success, rules=collection)
assert [] == good_runner.run()

def test_template_instead_of_copy_negative() -> None:
"""Negative test for partial-become."""
"""Negative test for avoid-implicit."""
collection = RulesCollection()
collection.register(UseTemplateInsteadOfCopyRule())
failure = "examples/playbooks/rule-template-instead-of-copy-failure.yml"
collection.register(AvoidImplicitRule())
failure = "examples/playbooks/rule-avoid-implicit-fail.yml"
bad_runner = Runner(failure, rules=collection)
errs = bad_runner.run()
assert len(errs) == 1
34 changes: 0 additions & 34 deletions src/ansiblelint/rules/template-instead-of-copy.md

This file was deleted.

0 comments on commit 0e6cdef

Please sign in to comment.