Skip to content

Commit

Permalink
template-instead-of-copy: reduce number of false positives
Browse files Browse the repository at this point in the history
- adds extra conditions for triggering the rule, basically eliminating
  the most simple strings from triggering it
- update documentation

Fixes: #2501
  • Loading branch information
ssbarnea committed Sep 29, 2022
1 parent b546673 commit 6f96ac2
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 35 deletions.
34 changes: 0 additions & 34 deletions src/ansiblelint/rules/template-instead-of-copy.md

This file was deleted.

67 changes: 67 additions & 0 deletions src/ansiblelint/rules/template_instead_of_copy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# template-instead-of-copy

This rule identifies the presence of variable interpolation inside `copy`
module and recommends instead the use of the `template` module. This is based on
the official [recommendation](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html#synopsis)
from Ansible documentation.

This rule will be triggered if any of the following conditions are met:

- `content` is not a string
- `content` contains any Unicode character (not being pure ASCII)
- `content` is it contains newline chars in any place other than the end of file
- `content` contains at least one of `{{`, `{%`, `{#` substrings

```{warning}
Unless you explicitly include a `\n` at the end of your string, the final
file will not have a trailing newline. This is a very common error, as many
tools fail to work properly if the last line is not terminated by a newline.
```

## Problematic Code

```yaml
---
- name: Example playbook
hosts: localhost
tasks:
- name: Write file content
ansible.builtin.copy:
content: { "foo": "bar" } # <-- should use template instead
dest: /tmp/foo.txt
```
Please note that in this case, the content of the file will be
`{ "foo", "bar" }`, which is different from the original content (see that
the single quotation marks were modified by Jinja into double ones). The file
will also not have an ending newline, something that is usually expected by
people knowing Python language.

Several other conversions can happen while using `copy`, including up to 4
levels of templating. On the other hand, the `template` module will only
perform 1 level of templating.

## Correct Code

```yaml
---
- name: Example playbook (1st solution, using `template` module)
hosts: localhost
tasks:
- name: Write file content
ansible.builtin.template:
# Add `templates/foo.txt.j2` with what you need inside
src: foo.txt.j2
dest: /tmp/foo.txt
```
```yaml
---
- name: Example playbook (2nd solution, use only a very simple string)
hosts: localhost
tasks:
- name: Write file content
ansible.builtin.copy:
content: "{ 'foo': 'bar' }"
dest: /tmp/foo.txt
```
11 changes: 10 additions & 1 deletion src/ansiblelint/rules/template_instead_of_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,16 @@ def matchtask(
"""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
for seq in ("{{", "{#", "{%"):
if seq in content:
return True
try:
content.encode("ascii")
except UnicodeEncodeError:
return True
if "\n" in content.rstrip("\n"):
return True
return False

Expand Down

0 comments on commit 6f96ac2

Please sign in to comment.