Repository: ansible/ansible
Pull request: documents approaches to octal modes better
##### SUMMARY
Closes #78715.

The old documentation suggested prepending a `0` to signal octal values for `mode`. This approach fails in loops, possibly elsewhere.

This PR changes the documentation to recommend quoting octal values for `mode`, while noting that the initial 0 works in some cases (to help folks looking at pre-existing playbooks/roles/etc.). Wordsmithing welcome!

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Docs Pull Request

##### COMPONENT NAME
all files modules
docs.ansible.com

##### ADDITIONAL INFORMATION
N/A

Problem statement:
ansible.builtin.file: incorrect permissions being set when looping 
### Summary

Setting file permissions for a list of files like this
```
my_items:
  - path: dir1
    mode: 0770
  - ... further entries ..
```

using `ansible.builtin.file` results in incorrect permissions. For details see steps to reproduce below. 

### Issue Type

Documentation Report

### Component Name

file

### Ansible Version

```console
$ ansible --version
ansible [core 2.12.6]
  config file = /home/yannik/projects/xxx/ansible.cfg
  configured module search path = ['/home/yannik/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.10/site-packages/ansible
  ansible collection location = /home/yannik/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.6 (main, Aug  2 2022, 00:00:00) [GCC 11.3.1 20220421 (Red Hat 11.3.1-2)]
  jinja version = 3.0.1
  libyaml = True
```


### Configuration

```console
# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
ANSIBLE_NOCOWS(/home/yannik/projects/xxx/ansible.cfg) = True
```


### OS / Environment

fedora 35

### Steps to Reproduce

Create a host_vars file with this:
```yaml
my_items:
  - path: dir1
    mode: 0770
```

and run the following playbook:
```yaml (paste below)
- file:
    path: "/tmp/{{ item.path }}"
    state: directory
    mode: "{{ item.mode }}"
  loop: "{{ my_items }}"
```


### Expected Results

The resulting permissions of `/tmp/dir1` should be equal to the permissions being set when not using a loop:

### Specifying `mode` directly in the playbook task
```
- file:
    path: "/tmp/dir1"
    state: directory
    mode: 0770
```
**Result: permissions set to 0770**

### Specifying `mode` using a variable
`hosts_vars`: 
```
my_permissions_variable: 0770
```
playbook:
```
- file:
    path: "/tmp/dir"
    state: directory
    mode: "{{ my_permissions_variable }}"
```

**Result: permissions set to 0770**


### Actual Results

```console
File permissions are set to `0504` instead of `0770` when using a loop:

$ ansible-playbook test.yml -v         
Using /home/yannik/projects/xxx/ansible.cfg as config file

PLAY [app0] *************************************************************************************************

TASK [Gathering Facts] **************************************************************************************
ok: [app0]

TASK [file] *************************************************************************************************
ok: [app0] => (item={'path': 'dir1', 'mode': 504}) => changed=false 
  ansible_loop_var: item
  gid: 0
  group: root
  item:
    mode: 504
    path: dir1
  mode: '0504'
  owner: root
  path: /tmp/dir1
  size: 4096
  state: directory
  uid: 0

PLAY RECAP **************************************************************************************************
app0                       : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   


$ stat /tmp/dir1
  File: /tmp/dir1
  Size: 4096      	Blocks: 8          IO Block: 4096   directory
Device: fe01h/65025d	Inode: 2126076     Links: 2
Access: (0504/dr-x---r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2022-09-06 14:28:46.635247941 +0200
Modify: 2022-09-06 14:28:46.635247941 +0200
Change: 2022-09-06 14:28:46.635247941 +0200
 Birth: 2022-09-06 14:28:46.635247941 +0200
```


### Code of Conduct

- [X] I agree to follow the Ansible Code of Conduct

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

If these files are incorrect, 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/devel/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
The problem here is somewhat complex, but is effectively the expected behavior.

When you specify `mode: 0770`, pyyaml detects this as an octal, but since Python does not have a persistent means of storing an octal, the value is converted to an integer, effectively `mode: 504`.

Using the standard means of jinja2 templating in ansible, an individual templating operation can only result in one of the following python data types: `dict`, `list`, `set`, `bool`, `str`

As such, the result of `mode: "{{ item.mode }}"` is effectively `mode: !!str 504`.  The python internals of mode handling in `chmod` will assume that any string mode provided is a string representation of an octal, which means it is interpreted as `0504` and not `0770`.

The most robust solution to this problem is to make your mode a string from the start, by specifying:

```yaml
my_items:
  - path: dir1
    mode: "0770"
```

The other option is to enable [jinja2 native](https://docs.ansible.com/ansible-core/2.13/reference_appendices/config.html#default-jinja2-native)
Hi @sivel, 

thanks for the explanation!

Currently, the docs for the `mode` parameter of the file module state this:

> For those used to /usr/bin/chmod remember that modes are actually octal numbers. You must either add a leading zero so that Ansible’s YAML parser knows it is an octal number (like 0644 or 01777) or quote it (like '644' or '1777') so Ansible receives a string and can do its own conversion from string into number.

In earlier versions of ansible, the leading zero was the only option. Quoting wasn't even mentioned:
> For those used to /usr/bin/chmod remember that modes are actually octal numbers (like 0644 or 01777). Leaving off the leading zero will likely have unexpected results.

As observed in this issue, the solution of prepending a leading zero leads to results that are (for users) completely unexpected and seemingly broken in specific circumstances, e.g. when using loops.

Due to this, the `prepend a leading zero` method should IMO be removed from the docs and a warning should be thrown when an integer is provided as `mode` parameter, signaling that modes should always be quoted. Even though I have pretty extensive experience with ansible, I had to go bug-hunting for quite a while to identify this problem.

On a side note regarding your other suggestion: What are the reason that `jinja2 native` is it not enabled by default? Are there any downsides (except from backwards-compatibility woes)?

Adding a warning is not something we plan to do, as it's a completely normal use case where the author has placed an octal in mode without any templating.

We've adjusted this to a documentation report, so that we can perhaps clarify the documentation more.
Hi @sivel,

would you mind commenting on my question regarding your proposed solution of enabling jinja2 native?
> What are the reason that jinja2 native is it not enabled by default? Are there any downsides (except from backwards-compatibility woes)?
> would you mind commenting on my question regarding your proposed solution of enabling jinja2 native?
> 
> > What are the reason that jinja2 native is it not enabled by default? Are there any downsides (except from backwards-compatibility woes)?

It's largely backwards compatibility concerns.  As described above, the non-native implementation can only produce certain variable types, whereas native isn't restricted.  As such, there may be work arounds that users have accounted for that are incompatible with native.  Historically, it also required upgrading to jinja2 versions that may not be available for your OS, but that is a hard requirement now anyway.

We will likely make it the default in a future release.

Commit message: documents approaches to octal modes better

Patch to review:
diff --git a/lib/ansible/plugins/doc_fragments/files.py b/lib/ansible/plugins/doc_fragments/files.py
index b87fd11d10257f..53b3b49385e0c6 100644
--- a/lib/ansible/plugins/doc_fragments/files.py
+++ b/lib/ansible/plugins/doc_fragments/files.py
@@ -18,10 +18,11 @@ class ModuleDocFragment(object):
     description:
     - The permissions the resulting filesystem object should have.
     - For those used to I(/usr/bin/chmod) remember that modes are actually octal numbers.
-      You must either add a leading zero so that Ansible's YAML parser knows it is an octal number
-      (like C(0644) or C(01777)) or quote it (like C('644') or C('1777')) so Ansible receives
+      You must give Ansible enough information to parse them correctly.
+      For consistent results, quote octal numbers (for example, C('644') or C('1777')) so Ansible receives
       a string and can do its own conversion from string into number.
-    - Giving Ansible a number without following one of these rules will end up with a decimal
+      Adding a leading zero (for example, C(0755)) works sometimes, but fails in loops and some other circumstances.
+    - Giving Ansible a number without following either of these rules will end up with a decimal
       number which will have unexpected results.
     - As of Ansible 1.8, the mode may be specified as a symbolic mode (for example, C(u+rwx) or
       C(u=rw,g=r,o=r)).


Code review comments:
diff_hunk: @@ -18,10 +18,11 @@ class ModuleDocFragment(object):
     description:
     - The permissions the resulting filesystem object should have.
     - For those used to I(/usr/bin/chmod) remember that modes are actually octal numbers.
-      You must either add a leading zero so that Ansible's YAML parser knows it is an octal number
-      (like C(0644) or C(01777)) or quote it (like C('644') or C('1777')) so Ansible receives
+      You must give Ansible enough information to parse them correctly.
+      For consistent results, quote octal numbers (for example, C('644') or C('1777')) so Ansible receives
       a string and can do its own conversion from string into number.
-    - Giving Ansible a number without following one of these rules will end up with a decimal
+      Adding a leading zero (for example, C(0755)) works sometimes, but fails in loops and some other circumstances.
path: lib/ansible/plugins/doc_fragments/files.py
text: @user1:
Depending on how exactly it is used in loops, it might not fail, so how about:
```suggestion
      Adding a leading zero (for example, C(0755)) works sometimes, but can fail in loops and some other circumstances.
```