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

preventing zero-length string value for option --limit #49261

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@viky600833

viky600833 commented Nov 28, 2018

SUMMARY

When running ansible-playbook with an zero length string value for the option --limit , the playbook will be applied on whole inventory or whole group specified by - hosts
It could result irreparable harm. It should be strictly forbidden.

The empty argument could occur when running playbook inside a shell script where the shell variable is not valued.

ansible-playbook test.yaml --limit $testing_group ( $test_group is empty or not setted)

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

function ansible.cli.CLI.parse(self)

ADDITIONAL INFORMATION
  • The testing playbook:
---
- hosts: all
  gather_facts: no
  tasks:
  - ping:
  • run playbook as:
$ ansible-playbook -i 'centos2,centos3,' test.yaml  --limit ""

PLAY [all] *******************************************************************************************

TASK [ping] ******************************************************************************************
ok: [centos3]
ok: [centos2]

PLAY RECAP *******************************************************************************************
centos2                    : ok=1    changed=0    unreachable=0    failed=0    skipped=0
centos3                    : ok=1    changed=0    unreachable=0    failed=0    skipped=0
  • in order to prevent it, non-zero length string check is done in ansible.cli.CLI.parse

  • re-run playbook

$ ansible-playbook  -i 'centos2,centos3' test.yaml --limit ""
ERROR! Unexpected Exception, this is probably a bug: ansible-playbook: error: --limit option requires non-zero length string
to see the full traceback, use -vvv
@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 28, 2018

Hi @viky600833, thank you for submitting this pull-request!

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 28, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/cli/__init__.py:579:102: bad-whitespace No space allowed after bracket         if hasattr(self.options, 'subset') and  self.options.subset != C.DEFAULT_SUBSET and isinstance( self.options.subset, string_types) and len(self.options.subset) == 0:                                                                                                       ^

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

lib/ansible/cli/__init__.py:579:47: E271 multiple spaces after keyword
lib/ansible/cli/__init__.py:579:104: E201 whitespace after '('
lib/ansible/cli/__init__.py:579:161: E501 line too long (173 > 160 characters)

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Nov 28, 2018

@ansibot ansibot added core_review and removed needs_revision labels Nov 29, 2018

600833 added some commits Dec 1, 2018

@sivel sivel self-assigned this Dec 4, 2018

@sivel sivel removed the needs_triage label Dec 4, 2018

@sivel sivel self-requested a review Dec 4, 2018

@sivel

This also needs a changelog fragment added in the changelog/fragments directory. See other files for an example.

Show resolved Hide resolved lib/ansible/cli/__init__.py Outdated
Show resolved Hide resolved lib/ansible/cli/__init__.py Outdated
@viky600833

This comment has been minimized.

viky600833 commented Dec 5, 2018

It seems something going wrong with osx pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment