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

Facts: Do not fail if Ansible can not run local facts due to user rights #52429

Open
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@ACampesinin
Copy link

ACampesinin commented Feb 17, 2019

… user rights

SUMMARY

Fixes #52427

I decided to check the rights over each facts file in order to know if it is runable by the user running the playbook. In order to do that it considers:

  1. If file is owned by the user
  2. If file is owned by any of the user groups.
  3. If the file can be executed by anyone
  4. If the file can be read by anyone
  5. If none of the above, the local fact output is a message saying that fact is not readable by the user.
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

setup module

ADDITIONAL INFORMATION

Check the bug report: #52427

Before my change

<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'echo ~ && sleep 0'"'"''
<localhost> (0, b'/home/acampesino\n', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'( umask 77 && mkdir -p "` echo /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936 `" && echo ansible-tmp-1550432459.9194748-143150622851936="` echo /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936 `" ) && sleep 0'"'"''
<localhost> (0, b'ansible-tmp-1550432459.9194748-143150622851936=/home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936\n', b'')
Using module file /home/acampesino/ansible-orginal/lib/python3.6/site-packages/ansible-2.8.0.dev0-py3.6.egg/ansible/modules/system/setup.py
<localhost> PUT /home/acampesino/.ansible/tmp/ansible-local-12948grjgfuah/tmp6eeysray TO /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/AnsiballZ_setup.py
<localhost> SSH: EXEC sshpass -d10 sftp -o BatchMode=no -b - -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 '[localhost]'
<localhost> (0, b'sftp> put /home/acampesino/.ansible/tmp/ansible-local-12948grjgfuah/tmp6eeysray /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/AnsiballZ_setup.py\n', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'chmod u+x /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/ /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/AnsiballZ_setup.py && sleep 0'"'"''
<localhost> (0, b'', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 -tt localhost '/bin/sh -c '"'"'/usr/bin/python /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/AnsiballZ_setup.py && sleep 0'"'"''
<localhost> (1, b'\r\n{"exception": "WARNING: The below traceback may *not* be related to the actual failure.\\n  File \\"/tmp/ansible_setup_payload_lkDXkZ/ansible_setup_payload.zip/ansible/module_utils/basic.py\\", line 2845, in run_command\\n    cmd = subprocess.Popen(args, **kwargs)\\n  File \\"/usr/lib/python2.7/subprocess.py\\", line 394, in __init__\\n    errread, errwrite)\\n  File \\"/usr/lib/python2.7/subprocess.py\\", line 1047, in _execute_child\\n    raise child_exception\\n", "cmd": "/etc/ansible/facts.d/znew.fact", "failed": true, "rc": 13, "invocation": {"module_args": {"filter": "ansible_local", "gather_subset": ["all"], "fact_path": "/etc/ansible/facts.d", "gather_timeout": 10}}, "msg": "[Errno 13] Permission denied"}\r\n', b'Shared connection to localhost closed.\r\n')
<localhost> Failed to connect to the host via ssh: Shared connection to localhost closed.
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'rm -f -r /home/acampesino/.ansible/tmp/ansible-tmp-1550432459.9194748-143150622851936/ > /dev/null 2>&1 && sleep 0'"'"''
<localhost> (0, b'', b'')
The full traceback is:
WARNING: The below traceback may *not* be related to the actual failure.
  File "/tmp/ansible_setup_payload_lkDXkZ/ansible_setup_payload.zip/ansible/module_utils/basic.py", line 2845, in run_command
    cmd = subprocess.Popen(args, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 394, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1047, in _execute_child
    raise child_exception

localhost | FAILED! => {
    "changed": false,
    "cmd": "/etc/ansible/facts.d/znew.fact",
    "invocation": {
        "module_args": {
            "fact_path": "/etc/ansible/facts.d",
            "filter": "ansible_local",
            "gather_subset": [
                "all"
            ],
            "gather_timeout": 10
        }
    },
    "msg": "[Errno 13] Permission denied",
    "rc": 13
}```

**After my change **
```ansible -m setup -i 'localhost,' -a 'filter=ansible_local' all -k -vvv
ansible 2.8.0.dev0
  config file = None
  configured module search path = ['/home/acampesino/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/acampesino/ansibleenv/lib/python3.6/site-packages/ansible-2.8.0.dev0-py3.6.egg/ansible
  executable location = ../ansibleenv/bin/ansible
  python version = 3.6.7 (default, Oct 22 2018, 11:32:17) [GCC 8.2.0]
No config file found; using defaults
SSH password: 
Parsed localhost, inventory source with host_list plugin
META: ran handlers
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'echo ~ && sleep 0'"'"''
<localhost> (0, b'/home/acampesino\n', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'( umask 77 && mkdir -p "` echo /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568 `" && echo ansible-tmp-1550434706.6798608-248529026743568="` echo /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568 `" ) && sleep 0'"'"''
<localhost> (0, b'ansible-tmp-1550434706.6798608-248529026743568=/home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568\n', b'')
Using module file /home/acampesino/ansibleenv/lib/python3.6/site-packages/ansible-2.8.0.dev0-py3.6.egg/ansible/modules/system/setup.py
<localhost> PUT /home/acampesino/.ansible/tmp/ansible-local-14805rku9912w/tmpil9v8wxp TO /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/AnsiballZ_setup.py
<localhost> SSH: EXEC sshpass -d10 sftp -o BatchMode=no -b - -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 '[localhost]'
<localhost> (0, b'sftp> put /home/acampesino/.ansible/tmp/ansible-local-14805rku9912w/tmpil9v8wxp /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/AnsiballZ_setup.py\n', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'chmod u+x /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/ /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/AnsiballZ_setup.py && sleep 0'"'"''
<localhost> (0, b'', b'')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 -tt localhost '/bin/sh -c '"'"'/usr/bin/python /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/AnsiballZ_setup.py && sleep 0'"'"''
<localhost> (0, b'\r\n{"invocation": {"module_args": {"filter": "ansible_local", "gather_subset": ["all"], "fact_path": "/etc/ansible/facts.d", "gather_timeout": 10}}, "ansible_facts": {"ansible_local": {"test": {"empty": "test"}, "other": {"run_error": "facts non readable by running user"}, "znew": {"run_error": "facts non readable by running user"}}}}\r\n', b'Shared connection to localhost closed.\r\n')
<localhost> ESTABLISH SSH CONNECTION FOR USER: None
<localhost> SSH: EXEC sshpass -d10 ssh -C -o ControlMaster=auto -o ControlPersist=60s -o ConnectTimeout=10 -o ControlPath=/home/acampesino/.ansible/cp/8a5a4c6a60 localhost '/bin/sh -c '"'"'rm -f -r /home/acampesino/.ansible/tmp/ansible-tmp-1550434706.6798608-248529026743568/ > /dev/null 2>&1 && sleep 0'"'"''
<localhost> (0, b'', b'')
localhost | SUCCESS => {
    "ansible_facts": {
        "ansible_local": {
            "other": {
                "run_error": "facts non readable by running user"
            },
            "test": {
                "empty": "test"
            },
            "znew": {
                "run_error": "facts non readable by running user"
            }
        }
    },
    "changed": false,
    "invocation": {
        "module_args": {
            "fact_path": "/etc/ansible/facts.d",
            "filter": "ansible_local",
            "gather_subset": [
                "all"
            ],
            "gather_timeout": 10
        }
    }
}
META: ran handlers
META: ran handlers

@ACampesinin ACampesinin changed the title now ansible playbook does not fail if it can`t run local facts due to… now ansible playbook does not fail if it can not run local facts due to… Feb 17, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 17, 2019

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/module_utils/facts/system/local.py:98:20: bad-whitespace Exactly one space required after assignment                 out =  '{"run_error": "facts non readable by running user"}'                      ^
lib/ansible/module_utils/facts/system/local.py:98:76: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/module_utils/facts/system/local.py:98:22: E222 multiple spaces after operator
lib/ansible/module_utils/facts/system/local.py:98:77: W291 trailing whitespace

click here for bot help

@Akasurde Akasurde changed the title now ansible playbook does not fail if it can not run local facts due to… Facts: Do not fail if Ansible can not run local facts due to user rights Feb 18, 2019

Acampesinin

@ansibot ansibot added core_review and removed needs_revision labels Feb 18, 2019

@bcoca
Copy link
Member

bcoca left a comment

a much simpler fix is to catch the os/io error and add a module.warn about the file and continue processing

@ansibot ansibot added needs_revision and removed core_review labels Feb 18, 2019

@samdoran samdoran removed the needs_triage label Feb 19, 2019

@bcoca
Copy link
Member

bcoca left a comment

change is too complex, just capture ioerror/oserror from get_file_content and emit a module.warn

Acampesinin and others added some commits Feb 23, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 23, 2019

@ACampesinin this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ACampesinin

This comment has been minimized.

Copy link
Author

ACampesinin commented Feb 23, 2019

I tried to makeit simplier but deciding first if it can be executed or read by user and then just run or execute, returnging warnings using the warn module when issues or problems to read appear.

This approac allows the file to be read or executed even if the user does not own it and the file has no execution rights for the owner.

Hope this approach suits better

@ACampesinin
Copy link
Author

ACampesinin left a comment

Changed the code in mi latest commit, with a section for checks and a single section for running.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 23, 2019

@ACampesinin this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

Acampesinin

@ansibot ansibot added the stale_ci label Mar 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.