docs: copy/template rule needs better reasoning

[greg-hellings]

Summary

template-instead-of-copy rule finds false positive when content argument is used that has template values in it.

Issue Type

• Bug Report

OS / ENVIRONMENT

Linux/NixOS

STEPS TO REPRODUCE

Sample play

- name: Write file contents from variable
  ansible.builtin.copy:
    dest: /some/dest/path
    content: "{{ some_var.subfield }}"

Desired Behavior

The above code should be perfectly acceptable, as all templating will be handled locally within the file.

Actual Behavior

Error template-instead-of-copy is thrown.

[MarkusTeufelberger]

I thought this was exactly the reason this rule was created? It now tries to make you use ansible.builtin.template on a template file containing only {{ some_var.subfield }} instead.

From looking through the code, a false positive might happen if you try to write two literal {{ characters as file content, but the case you describe seems like a true positive.

Maybe @GhostLyrics can give a better explanation since he authored this rule.

[ssbarnea]

I also think that this is not a false positive but I will keep the bug open because we need to fix the documentation for this rule to explain exactly why this needs to be done, current docs page is not enough. I do remember reading a blog post about the risks but I am unable to find it now.

[GhostLyrics]

This works as (I) intended it to. It is even a little bit more lenient than I wrote it. The original version disallowed the content key entirely due to formatting also not always being applied perfectly. Maintainers changed it to only trigger on Jinja in the content key.

[MarkusTeufelberger]

I'm not a big fan of that rule personally by the way, since it is useful if a "template" is just a single line of Jinja to have it right there in the task, but maybe that's something that IDEs should solve instead (e.g. by allowing to view a referenced template directly from the tasks file).

The upstream docs state however "For advanced formatting or if content contains a variable, use the ansible.builtin.template module." (https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html) so I guess the current way this rule is written reflects this recommendation. I don't see how the documentation change in #2512 explains this though, since no variable is actually used.

[ssbarnea]

@MarkusTeufelberger ping me on matrix/irc if you are online, maybe we can work on getting the documentation updated to make it clear. I could really use your help here. My main goal is to make it clear why this rule exists, so we might need to update docs, examples and maybe even the message. I was wondering myself about this and after chatting for an hour with @bcoca I was kinda sold. What you put inside the content is not necessarily what you get into the file, even if you do not use jinja in it. That is because it acts as implicit jinja field!

[GhostLyrics]

One reason why the initial draft was more strict is that when you use content it doesn't implicitly add a newline at the end of the file, while what you'd be used to from writing Python is an implicit newline by default.

This together with the warning from the docs caused me to write the rule in the first place.

[apatard]

@MarkusTeufelberger ping me on matrix/irc if you are online, maybe we can work on getting the documentation updated to make it clear. I could really use your help here. My main goal is to make it clear why this rule exists, so we might need to update docs, examples and maybe even the message. I was wondering myself about this and after chatting for an hour with @bcoca I was kinda sold. What you put inside the content is not necessarily what you get into the file, even if you do not use jinja in it. That is because it acts as implicit jinja field!

Well, I'm probably going to make some "friends" but if someone runs an ansible task without first making sure that it will do what's expected, it's not of the fault of the content: argument and not sure that's ansible-lint role. Moreover, the problem with the newline behavior vs Python, I believe that a bunch of ansible users know nothing about Python.

fwiw, I've put this rule in skip_list. While for complex cases, it's clearly a better idea to use a template but for one-liners like the example (or with few filters), I fail to understand the advantage.

[greg-hellings]

I'm with you, @apatard. I don't think this rule is the right purview for a linter, if it is working as expected. Especially if the purported problem is a present or absent trailing newline on the file. The warning from the docs cited above as "For advanced formatting or if content contains a variable, use the [ansible.builtin.template](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/template_module.html#ansible-collections-ansible-builtin-template-module) module."talks about when content contains a variable. Not when content IS a variable, which is my case. The content of the file is a field garnished from a web call (a signed SSL certificate from ACME in this case). Writing an entire template file whose entire content is {{ ssl_content_here }} is silly, especially when that value is pulled from the results of the previous call.

Even then, it's disingenuous to users to suddenly assume they're going to have a tougher time understanding that a string in the YAML is going to be templated. That is an inherent feature of Ansible. So is using the content keyword for this.

If we're talking about lint, the seemingly proper way to lint the content argument to the copy module would be to limit its length or complexity in some way. Not to flat ban any templating from inside of it.

[ssbarnea]

I had a very long meeting @cidrblock where we investigated all the reported problems and the result will a total rewrite of this rule, one that will trigger errors only when content: is given a non string value. Basically, jinja will be allowed.

[ssbarnea]

Please check the new implementation at #2512 -- basically a total rewrite.