ansible/ansible — pull request #4480 by joemaller (2013-10-13 05:35:13+00:00)
Title: ignore src when file module state=absent

This pull request contains one small change: tweaking a conditional to stop changing `path` if `state` is 'absent' or 'link'.

This works, but I can't see why the original statement exists at all nor any reason to keep it. After reviewing the documentation and file module source, this block of code should never be needed. [The file module documentation][docs] is quite explicit about the `src` parameter:

> src: path of the file to link to (***applies only to state=link***).

Here's the code in question, lines 181-182:

if src is not None and os.path.isdir(path) and state != "link":
params['path'] = path = os.path.join(path, os.path.basename(src))

So, contrary to the documentation, path will be modified with src *only if state != link*.

After walking through `main`, the only times this might come into play is when `prev_state` is a directory (lines 209-220). *Except* that we never get there because `path` was modified. Consider how the following task would be executed:

file: path=/remove/this/dir src=/foo/bar state=absent

1. `basename(src)` is joined to path, becoming `/remove/this/dir/bar` (lines 181-182)
2. `os.path.lexists(path)` (line 195) isn't asked to find the original path, looking instead for the modified path, so `prev_state` ends up as 'absent'
3. Since `prev_state` and `state` are both 'absent', the module exits with `changed=False` (lines 232-233)

This is wrong.

Yes, it's an edge case and `src` shouldn't be there. But *if* `src` is there, it should be ignored by all states except 'link'.

Here's the output when trying to remove a path with src specified, note the different paths between the REMOTE_MODULE log and the task's exit status:

TASK: [Remove tip of fake_path (this tries to remove the wrong path)] *********
<127.0.0.1> REMOTE_MODULE file path=/this/path/is/fake src=/var/log state=absent
ok: [default] => {"changed": false, "path": "/this/path/is/fake/log", "state": "absent"}

I found this by trying to condense the three steps necessary to create a link in a non-existent directory, having only the link's path to work with:

vars:
fake_path: /some/fake/path
tasks:
- name: Create link-placeholder to fill in parent dirs
file: path={{ fake_path }} state=directory

- name: Remove tip of placeholder leaving parent dirs
file: path={{ fake_path }} state=absent

- name: Create link from fake_path to /usr/log
file: path={{ fake_path }} src=/usr/log state=link

...became this looped task:

vars:
fake_path: /some/fake/path
tasks:
- name: Create and link fake_path to /var/log
file: path={{ fake_path }} src=/var/log state={{ item }}
with_items:
- directory
- absent
- link

Looping state feels too clever, but having three independent steps felt too fragile. My presumption was that `src` would be ignored by all non-link states.

[docs]: http://www.ansibleworks.com/docs/modules.html#file