Why is using args from variables considered unsave?

[Michał Czeraszkiewicz]

I'm trying out this playbook:

---

- name: Test
  hosts: all
  vars:
    link_list:
      - src: /etc/hosts
        dest: /tmp/2
        state: link
      - src: /etc/fstab
        dest: /tmp/1
        state: link
      - src: /etc/hostname
        dest: /tmp/3
        state: link
      - src: /etc/resolv.conf
        dest: /tmp/4
        state: link
  tasks:
    - file:
      args: "{{ item }}"
      with_items: "{{ link_list }}"

I get the following message:

[DEPRECATION WARNING]: Using variables for task params is unsafe, especially if the variables come from an external source like facts.
This feature will be removed in a future release. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.

Why is that so?

I think the usage of args is more elegant and feature proof (I don't need to extend my role when parameters change in the module) than specifying the arguments by hand. See example here.

[Parag Doke]

I am also looking forward to understand why this is being deprecated. Also, if anyone knows an alternative, please do share.

It looks like this deprecation warning was first introduced via commit https://github.com/ansible/ansible/commit/e526743b4f91a571f48bff4280ce5e441f72a292#diff-76ffc0551f8cf3d6255500316568e60bR242 by jimi-c. The commit message/comment refers to a debate on whether to have this added to 2.0. Hopefully, if that discussion is captured in text somewhere, a link to that will help understand the reasoning behind.

[James Cammarata]

We consider this unsafe due to the fact that, if the variable is based on a fact of some kind, your tasks could start doing unexpected or malicious things (in the event that the remote system is compromised in some way).

In your example, you could still simplify your task by simply restructuring it a bit:

- file:

    src: "{{item.src}}"

    dest: "{{item.dest}}"

    state: link

  with_items:

  <your var list here>

James Cammarata

Ansible Lead/Sr. Principal Software Engineer

Ansible by Red Hat

twitter: @thejimic, github: jimi-c

--
You received this message because you are subscribed to the Google Groups "Ansible Project" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ansible-project+unsubscribe@googlegroups.com.
To post to this group, send email to ansible-project@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-project/8de82682-1289-467d-907d-b2b5272045a9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Olivier Cervello]

I'm sorry but your solution is very limited in scope, and not flexible at all (something we expect from modern automation software).

Example with file resource:

Let's say I want 2 symlinks and a file to be removed, and don't want to bother declaring multiple variables / tasks for it (after all, Ansible has one resource valid for both those actions, so I should be able to define a single variable, and a single task).

---

- name: Test
  hosts: all
  vars:

    files:

      - src: /etc/hosts
        dest: /tmp/2
        state: link
      - src: /etc/fstab
        dest: /tmp/1
        state: link
      - src: /etc/hostname
        dest: /tmp/3
        state:

 link
      - path: /etc/nginx/nginx.conf
        state: absent

Only the following syntax can allow to run multiple tasks that have mixed params (and null params in some case):

- file:
    args: "{{ item }}"
    with_items: "{{ files }}"

If I used the syntax you offered above, I would need to specify defaults to None for all the arguments:

- file:
    src: "{{item.src | default(None) }}"
    dest: "{{item.dest | default(None) }}"
    path: "{{ item.path | default(None) }}"
    state: "{{ item.state }}"
  with_items: "{{ files }}"

The last syntax is worse because:

• it removes the original task defaults and you have specify them AGAIN by looking up documentations and setting every task args to their original defaults.
  
• if I need to use one of the other resource arguments (say `mode`, I have to modify my task to add it, when the first syntax gives you a task that work with any argument defined in the `file` resource...)

Another example with EC2 provisioning:

After deprecation, I am again forced to explicitly define all the `ec2` task variables (if I want automation flexibility in `group_vars`):

- name: Provision EC2 instances
  ec2:
    key_name: "{{ item.key_name }}"
    instance_type: "{{ item.instance_type }}"
    image: "{{ item.image}}"
    wait: "{{ item.wait }}"
    exact_count: "{{ item.exact_count }}"
    count_tag: "{{ item.count_tag }}"
    vpc_subnet_id: "{{ item.vpc_subnet_id }}"
    assign_public_ip: "{{ item.assign_public_ip }}"
    region: "{{ item.region }}"
    group: "{{ item.group }}"
    instance_tags: "{{ item.instance_tags }}"
    instance_profile_name: "{{ item.instance_profile_name }}"
    volumes: "{{ item.volumes }}"
  with_items: "{{ ec2_instances }}"

Before the deprecation, we could have much simpler tasks:

- name: Provision EC2 instances
  ec2: "{{ item }}"
  with_items: "{{ ec2_instances }}"

Summary

The Ansible core team has not specified the reason it consider this practice unsafe. If an attacker can temper with the `group_vars`, he can do it in both case (this syntax enabled or not). Deprecating this also violates Python principle of simplicity. IMHO, this is a step backwards for Ansible.

The DevOps engineer or the developer automating the solution should worry if it's safe / unsafe, should own the deployment code / deployment variables and has to make sure those are stored in a safe place. I don't think it's the responsibility of Ansible to prevent this behaviour.

Please explain how you consider this last syntax as more unsafe than the first syntax, and in what conditions.

Thanks,

Olivier Cervello

Cloud DevOps Infrastructure Engineer

Google Inc.

To unsubscribe from this group and stop receiving emails from it, send an email to ansible-proje...@googlegroups.com.
To post to this group, send email to ansible...@googlegroups.com.

[Matt Martz]

Instead of `default(None)` you should be using `default(omit)` it was designed exactly for this purpose, and will not override the default.  In instructs ansible to not pass that provided argument on to the module.

To unsubscribe from this group and stop receiving emails from it, send an email to ansible-project+unsubscribe@googlegroups.com.
To post to this group, send email to ansible-project@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-project/24177474-ff43-4f01-9f7c-ed1951b13665%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Matt Martz
@sivel
sivel.net

[Olivier Cervello]

Ok, but I consider this as a workaround and not a solution (cf. comment I wrote above).

[Jim Ficarra]

 

with_items is acceptable in the situations where you intend to loop over items, not pass in variable that aren't meant to be iterated over.  This deprecation has implications for include_role and import_role where the task parameters certainly will come from variables.

An additional 2 cents - there doesn't seem to be much offered in the way of alternatives - honestly with_items is not an appropriate alternative for use cases not involving iteration.  What is Red Hat offering for an alternative other than with_items?

[Geoffroy Desvernay]

On Thursday, January 25, 2018 at 9:17:31 PM UTC+1, Jim Ficarra wrote:

 

with_items is acceptable in the situations where you intend to loop over items, not pass in variable that aren't meant to be iterated over.  This deprecation has implications for include_role and import_role where the task parameters certainly will come from variables.

An additional 2 cents - there doesn't seem to be much offered in the way of alternatives - honestly with_items is not an appropriate alternative for use cases not involving iteration.  What is Red Hat offering for an alternative other than with_items?

 

2 cts more here:

I'm in the same kind of headache with zfs module: the list of possible zfs properties depends on the zfs implementation, and is very long (seems like a bas idea to fix this list anywhere).
Most people need to set 1 or 2 properties, but some uses need much more…

And I really think that if an attacker can write in ansible's inventory… I'd consider *all* systems manageable from ansible as corrupted. I think about it like any program's config: the program should reject invalid items if possible.

In the zfs module's case, it should implement thousands lines of code (with associated bugs) to be able to do this correctly (or not correctly…). The other option is to limit it's functionnality to a little subset of what it can do today.

Same thing with cron module (a bit mode "workaround-able").

The people (or program) responsible for inventory's variable *have to* understand what they do, please don't try to make ansible too much intelligent (KISS please ;)

[Geoffroy Desvernay]

Forgot a killer-feature of zfs: you can define your own properties… no one is able to list possibilities here.

[vas...@gmail.com]

Hello,

I agree that this is a needed feature that simplifies code and enhances readability. I opened a feature request to make this a configuration option rather than removing it, if they insist on deprecating it.

https://github.com/ansible/ansible/issues/42192

V.G.