Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule 208 not possible with unarchive or directory with recurse #1064

Closed
nerrehmit opened this issue Oct 1, 2020 · 3 comments · Fixed by #1089
Closed

Rule 208 not possible with unarchive or directory with recurse #1064

nerrehmit opened this issue Oct 1, 2020 · 3 comments · Fixed by #1089
Assignees
Labels

Comments

@nerrehmit
Copy link
Contributor

Summary

How can one set a mode on the "unarchive" module and on the file module with state:directory and recurse: yes so that rule 208 is ok?

Issue Type
  • Bug Report
Ansible and Ansible Lint details
ansible --version
ansible 2.9.13

ansible-lint --version
ansible-lint 4.3.5
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

Linux

STEPS TO REPRODUCE

unarchive example from the ansible documentation

file module example with state: directory and recurse: yes from documentation

command

ansible-lint example/test.yml

example playbook

---
- hosts: somehosts

  tasks:
    - name: Extract foo.tgz into /var/lib/foo
      unarchive:
        src: foo.tgz
        dest: /var/lib/foo

    - name: Recursively change ownership of a directory
      file:
        path: /etc/foo
        state: directory
        recurse: yes
        owner: foo
        group: foo
Desired Behaviour

ansible-lint reports no problems.

Actual Behaviour

ansible-lint rule 208 requires a mode for the ansible module unarchive https://docs.ansible.com/ansible/latest/collections/ansible/builtin/unarchive_module.html
However the archive might contain files with different permissions (eg. an executable and a README, License, etc.) and therefore there is no single mode that's fitting for all unarchived contents.

ansible-lint rule 208 also requires that a mode is set for the file module https://docs.ansible.com/ansible/latest/collections/ansible/builtin/file_module.html
This is problematic in case I apply the module towards a directory with recurse: yes (eg. to change the owner recursively) as directories mainly have 0755 and the containing files are not necessarily executable.

[208] File permissions unset or incorrect
example/test.yml:5
Task/Handler: Extract foo.tgz into /var/lib/foo

[208] File permissions unset or incorrect
example/test.yml:10
Task/Handler: Recursively change ownership of a directory
@nerrehmit nerrehmit added priority/medium new Triage required labels Oct 1, 2020
@abdennour
Copy link

I've same issue with unarchive module:
If the rule 208 instructs to set the mode, i will lose default mode of each file.
For now, i made it as permissive quality gate :

# .ansible-lint file

warn_list:
  - experimental

@ssbarnea ssbarnea self-assigned this Nov 3, 2020
@ssbarnea ssbarnea added bug and removed priority/medium new Triage required labels Nov 3, 2020
ssbarnea added a commit that referenced this issue Nov 3, 2020
As archives can also contain permissions (like tar ones), we should
avoid triggering 208 rule when unarchive module is used.

Fixes: #1064
albinvass pushed a commit that referenced this issue Nov 3, 2020
As archives can also contain permissions (like tar ones), we should
avoid triggering 208 rule when unarchive module is used.

Fixes: #1064
@nerrehmit
Copy link
Contributor Author

Thank you for the pull request to fix the issue with the unarchive module.

I described a second issue in my post about the file module with recursive. For example to change the ownership. Then I have a clash between directory permissions (usually 0755) and the files within those directories (usually 0644 or 0660).

Do you have any idea how that might be resolved? Or maybe a workaround. Happy to test things.

@ssbarnea
Copy link
Member

ssbarnea commented Nov 3, 2020

If I remember well if recursive is true file module should not trigger the rule. If not you have the opportunity to make a PR to also fix this one.

Cheap workaround would be to add # noqa 208 to silence each occurrence, but a solution from which all users can benefit is much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants