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

added password options to unarchive module issue Fixes #30100 #38252

Open
wants to merge 22 commits into
base: devel
Choose a base branch
from

Conversation

Alexander198961
Copy link
Contributor

@Alexander198961 Alexander198961 commented Apr 4, 2018

SUMMARY

added password to unarchive module

Fix #30100

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

unarchive.py

ANSIBLE VERSION
ansible 2.5.0

@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2018

@ansibot ansibot added core_review feature module needs_triage support:core test m:unarchive labels Apr 4, 2018
@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2018

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

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

lib/ansible/modules/files/unarchive.py:584:29: E203 whitespace before ','
lib/ansible/modules/files/unarchive.py:800:53: E203 whitespace before ','

The test ansible-test sanity --test validate-modules [explain] failed with 8 errors:

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (True) for "validate_certs" does not match the documentation (False)
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "keep_newer" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "list_files" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "remote_src" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "validate_certs" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:83:7: E302 DOCUMENTATION is not valid YAML
test/sanity/validate-modules/ignore.txt:789:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E322" test
test/sanity/validate-modules/ignore.txt:790:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E323" test

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

lib/ansible/modules/files/unarchive.py:83:7: error DOCUMENTATION: syntax error: expected <block end>, but found '?'

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Apr 4, 2018
@samdoran samdoran removed the needs_triage label Apr 5, 2018
@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2018

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test validate-modules [explain] failed with 8 errors:

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (True) for "validate_certs" does not match the documentation (False)
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "keep_newer" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "list_files" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "remote_src" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "validate_certs" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:83:7: E302 DOCUMENTATION is not valid YAML
test/sanity/validate-modules/ignore.txt:789:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E322" test
test/sanity/validate-modules/ignore.txt:790:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E323" test

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

lib/ansible/modules/files/unarchive.py:83:7: error DOCUMENTATION: syntax error: expected <block end>, but found '?'

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2018

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test validate-modules [explain] failed with 8 errors:

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (True) for "validate_certs" does not match the documentation (False)
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "keep_newer" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "list_files" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "remote_src" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:0:0: E325 argument_spec for "validate_certs" defines type="bool" but documentation does not
lib/ansible/modules/files/unarchive.py:83:7: E302 DOCUMENTATION is not valid YAML
test/sanity/validate-modules/ignore.txt:789:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E322" test
test/sanity/validate-modules/ignore.txt:790:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E323" test

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

lib/ansible/modules/files/unarchive.py:83:7: error DOCUMENTATION: syntax error: expected <block end>, but found '?'

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2018

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

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (None) for "password" does not match the documentation ('none')

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2018

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

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (None) for "password" does not match the documentation ('none')

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2018

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

lib/ansible/modules/files/unarchive.py:0:0: E324 Value for "default" from the argument_spec (None) for "password" does not match the documentation ('no')

click here for bot help

@ansibot ansibot removed the needs_revision label Apr 17, 2018
@ansibot ansibot added the stale_review label Jun 5, 2018
try:
(out, rc) = pexpect.run(" ".join(cmd), withexitstatus=1, events={'(?i)password': self.password + '\n'})
except:
return dict(cmd=cmd, rc=82, out=out, err='failed to unzip package')
Copy link
Member

@mattclay mattclay Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is special about rc=82 here?

try:
import pexpect
except ImportError:
self.module.fail_json(msg="The pexpect python module is required for password option")
Copy link
Member

@sivel sivel Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This try/except should be moved to top level imports, and restructured to look like:

try:
    import pexpect
    HAS_PEXPECT = True
except ImportError:
    HAS_PEXPECT = False

And then in this block, use if not HAS_PEXPECT:

@ansibot ansibot removed stale_ci stale_review labels Jun 15, 2018
@Alexander198961
Copy link
Contributor Author

Alexander198961 commented Jun 17, 2018

@sivel @mattclay fixed

try:
(out, rc) = pexpect.run(" ".join(cmd), withexitstatus=1, events={'(?i)password': self.password + '\n'})
except OSError as e:
return dict(cmd=cmd, rc=e.errno, out=out, err='failed to unzip package')
Copy link
Member

@mattclay mattclay Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use out='' instead of out=out since out will always be empty.

if not HAS_PEXPECT:
self.module.fail_json(msg="The pexpect python module is required for password option")
out = ""
rc = 0
Copy link
Member

@mattclay mattclay Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the default values for out and rc since they're unused (when using out='' below).

ignore_errors: yes

- name: check that pexpect is installed
shell: python -c "import pexpect"
Copy link
Member

@mattclay mattclay Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the pip module to install pexpect instead of detecting it, otherwise the tests will be skipped in CI.

# Run tests only in case we have unzip -P (password modifier) and zip is installed
- name: Run tests with protected zip
include: protected_zip_tests.yml
when: pexpect_is_installed.failed != true and 'zip_is_installed.failed != true' and "'must give decryption password with -P' in unzip_has_modifier_P.stderir"
Copy link
Member

@mattclay mattclay Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use zip_is_installed is not failed instead of zip_is_installed.failed != true.

There is typo in unzip_has_modifier_P.stderr.

@ansibot ansibot added stale_ci stale_review labels Jun 26, 2018
@ansibot ansibot removed stale_ci stale_review labels Sep 27, 2018
@ansibot
Copy link
Contributor

ansibot commented Sep 27, 2018

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

lib/ansible/modules/files/unarchive.py:0:0: E309 version_added for new option (password) should be 2.8. Currently 2.7

click here for bot help

@Alexander198961
Copy link
Contributor Author

Alexander198961 commented Sep 27, 2018

@mattclay @sivel could you pls review ?

@ansibot ansibot added stale_ci stale_review labels Oct 5, 2018
@ansibot ansibot added the files label Mar 5, 2019
@jctanner jctanner added this to TODO: Backlog, anything can go here. Anyone can work on this after their approved work is done. in Ansible 2.9 Apr 22, 2019
@ansibot ansibot removed the stale_review label Jun 25, 2019
@ansibot ansibot added the needs_rebase label Jul 3, 2019
@jimi-c jimi-c added affects_2.10 P3 and removed affects_2.6 labels May 8, 2020
@ansibot ansibot added pre_azp and removed stale_ci labels Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 feature files m:unarchive module needs_rebase needs_revision P3 pre_azp support:core test
Projects
No open projects
Ansible 2.9
TODO: Backlog, anything can go here. ...
Development

Successfully merging this pull request may close these issues.

7 participants