Repository: ansible/ansible
Pull request: Fix --diff to produce output when creating a new file
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
https://github.com/ansible/ansible/pull/51350 removed apending `state: absent` for missing files in the module's result. However `_get_diff_data` method was specifically checking for `absent` in the result. This changes that to check for missing `state` key instead.

This needs to be backported to `2.8` if merged.

Fixes #57618
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
`lib/ansible/plugins/action/__init__.py`

Problem statement:
Regression: --diff does not produce output when template creates new file
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Explain the problem briefly below -->

##### ISSUE TYPE
- Bug Report
- Regression

##### COMPONENT NAME
template module

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.8.1
  config file = None
```
ansible 2.8.0 is also affected

##### CONFIGURATION

##### OS / ENVIRONMENT

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->

<!--- Paste example playbooks or commands between quotes below -->
Create two files: template and playbook to generate file based on template

template: ./template_test.j2
```text
{{ ansible_managed }}
```

playbook: ./template_test.yaml
```yaml
- hosts: localhost
  connection: local
  tasks:
    - name: create new file from template
      template:
        src:  template_test.j2
        dest: /path/to/file
```

Then run playbook: `ansible-playbook ./template_test.yaml --check --diff`

##### EXPECTED RESULTS
```
TASK [create new file from template] *************************
--- before
+++ after: <tmp_path>/template_test.j2
@@ -0,0 +1 @@
+Ansible managed

changed: [localhost]
```

##### ACTUAL RESULTS
```paste below
TASK [create new file from template] *************************
changed: [localhost]
```

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/files/template.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/files/template.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
This was introduced in cc9c72d6f845710b24e952670b534a57f6948513

Commit message: Fix --diff to produce output when creating a new file

Fixes #57618

Patch to review:
diff --git a/changelogs/fragments/57618-fix-diff-on-absent-files.yaml b/changelogs/fragments/57618-fix-diff-on-absent-files.yaml
new file mode 100644
index 00000000000000..a45790319a0520
--- /dev/null
+++ b/changelogs/fragments/57618-fix-diff-on-absent-files.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - "Fix --diff to produce output when creating a new file (https://github.com/ansible/ansible/issues/57618)"
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 2f6237d007cb08..04cb29d8d70984 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -1104,7 +1104,7 @@ def _get_diff_data(self, destination, source, task_vars, source_file=True):
 
         if not peek_result.get('failed', False) or peek_result.get('rc', 0) == 0:
 
-            if peek_result.get('state') == 'absent':
+            if peek_result.get('state') is None:
                 diff['before'] = u''
             elif peek_result.get('appears_binary'):
                 diff['dst_binary'] = 1
diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml
index c0a48e314ba178..023d872183d4f1 100644
--- a/test/integration/targets/copy/tasks/main.yml
+++ b/test/integration/targets/copy/tasks/main.yml
@@ -61,6 +61,19 @@
     - import_tasks: acls.yml
       when: ansible_system == 'Linux'
 
+    # https://github.com/ansible/ansible/issues/57618
+    - name: Test diff contents
+      copy:
+        content: 'Ansible managed\n'
+        dest: "{{ local_temp_dir }}/file.txt"
+      diff: yes
+      register: diff_output
+
+    - assert:
+        that:
+          - 'diff_output.diff[0].before == ""'
+          - '"Ansible managed" in diff_output.diff[0].after'
+
   always:
     - name: Cleaning
       file:


Code review comments:
diff_hunk: @@ -1104,7 +1104,7 @@ def _get_diff_data(self, destination, source, task_vars, source_file=True):
 
         if not peek_result.get('failed', False) or peek_result.get('rc', 0) == 0:
 
-            if peek_result.get('state') == 'absent':
+            if peek_result.get('state') is None:
path: lib/ansible/plugins/action/__init__.py
text: @user1:
i would do both `in ('absent', None)` as there are cases in which 'absent' would be a correct return.

@author:
I don't think there are. `peek_result` is a result of the file module being executed with `_diff_peek=True` which just checks if the file is binary and calls `exit_json`. From there `add_path_info` is only method that modifies `state` in the result and no longer sets `absent`.

Having said that doing `in ('absent', None)` might be more defensive, so I'd be +1 for that.

@user1:
you also have to take into account actions that have 'remote_src=true' as they might not rely on peek