From a5b80464df92c2ebdc7ca73159763d10d4ed7068 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 1 Feb 2018 16:30:08 -0800 Subject: [PATCH] Fix packaging/os modules for wildcard imports and get_exception --- lib/ansible/modules/packaging/os/apt.py | 4 ++-- lib/ansible/modules/packaging/os/apt_repository.py | 8 +++++++- lib/ansible/modules/packaging/os/dpkg_selections.py | 4 ++-- lib/ansible/modules/packaging/os/homebrew_tap.py | 4 ++-- lib/ansible/modules/packaging/os/layman.py | 8 +++++--- lib/ansible/modules/packaging/os/macports.py | 9 ++++----- lib/ansible/modules/packaging/os/opkg.py | 7 +++---- lib/ansible/modules/packaging/os/pkg5_publisher.py | 4 ++-- lib/ansible/modules/packaging/os/pkgin.py | 4 ++-- lib/ansible/modules/packaging/os/pkgutil.py | 5 +---- lib/ansible/modules/packaging/os/portage.py | 6 ++---- lib/ansible/modules/packaging/os/slackpkg.py | 4 ++-- lib/ansible/modules/packaging/os/svr4pkg.py | 4 ++-- lib/ansible/modules/packaging/os/swdepot.py | 10 +++++----- lib/ansible/modules/packaging/os/yum_repository.py | 3 ++- lib/ansible/modules/packaging/os/zypper_repository.py | 9 +++++---- test/sanity/code-smell/no-wildcard-import.sh | 1 - 17 files changed, 48 insertions(+), 46 deletions(-) diff --git a/lib/ansible/modules/packaging/os/apt.py b/lib/ansible/modules/packaging/os/apt.py index b865c4921d..327b42ee60 100644 --- a/lib/ansible/modules/packaging/os/apt.py +++ b/lib/ansible/modules/packaging/os/apt.py @@ -841,7 +841,7 @@ def get_cache(module): try: cache = apt.Cache() except SystemError as e: - if '/var/lib/apt/lists/' in str(e).lower(): + if '/var/lib/apt/lists/' in to_native(e).lower(): # update cache until files are fixed or retries exceeded retries = 0 while retries < 2: @@ -850,7 +850,7 @@ def get_cache(module): if rc == 0: break if rc != 0: - module.fail_json(msg='Updating the cache to correct corrupt package lists failed:\n%s\n%s' % (str(e), str(so) + str(se)), rc=rc) + module.fail_json(msg='Updating the cache to correct corrupt package lists failed:\n%s\n%s' % (to_native(e), so + se), rc=rc) # try again cache = apt.Cache() else: diff --git a/lib/ansible/modules/packaging/os/apt_repository.py b/lib/ansible/modules/packaging/os/apt_repository.py index e09289f432..7c2ca9f896 100644 --- a/lib/ansible/modules/packaging/os/apt_repository.py +++ b/lib/ansible/modules/packaging/os/apt_repository.py @@ -103,6 +103,7 @@ EXAMPLES = ''' ''' import glob +import json import os import re import sys @@ -122,12 +123,17 @@ except ImportError: distro = None HAVE_PYTHON_APT = False +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils._text import to_native +from ansible.module_utils.urls import fetch_url + + if sys.version_info[0] < 3: PYTHON_APT = 'python-apt' else: PYTHON_APT = 'python3-apt' -DEFAULT_SOURCES_PERM = int('0644', 8) +DEFAULT_SOURCES_PERM = 0o0644 VALID_SOURCE_TYPES = ('deb', 'deb-src') diff --git a/lib/ansible/modules/packaging/os/dpkg_selections.py b/lib/ansible/modules/packaging/os/dpkg_selections.py index b4f1f4f893..a4a91a2795 100644 --- a/lib/ansible/modules/packaging/os/dpkg_selections.py +++ b/lib/ansible/modules/packaging/os/dpkg_selections.py @@ -41,6 +41,8 @@ EXAMPLES = ''' selection: hold ''' +from ansible.module_utils.basic import AnsibleModule + def main(): module = AnsibleModule( @@ -72,7 +74,5 @@ def main(): module.exit_json(changed=changed, before=current, after=selection) -from ansible.module_utils.basic import * - if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/homebrew_tap.py b/lib/ansible/modules/packaging/os/homebrew_tap.py index 0cda078c1e..51c5acabcd 100644 --- a/lib/ansible/modules/packaging/os/homebrew_tap.py +++ b/lib/ansible/modules/packaging/os/homebrew_tap.py @@ -70,6 +70,8 @@ EXAMPLES = ''' import re +from ansible.module_utils.basic import AnsibleModule + def a_valid_tap(tap): '''Returns True if the tap is valid.''' @@ -248,8 +250,6 @@ def main(): else: module.exit_json(changed=changed, msg=msg) -# this is magic, see lib/ansible/module_common.py -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/layman.py b/lib/ansible/modules/packaging/os/layman.py index 7cdf1833e7..fe3e102bab 100644 --- a/lib/ansible/modules/packaging/os/layman.py +++ b/lib/ansible/modules/packaging/os/layman.py @@ -83,6 +83,7 @@ EXAMPLES = ''' ''' import shutil + from os import path try: @@ -92,6 +93,10 @@ try: except ImportError: HAS_LAYMAN_API = False +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.urls import fetch_url + + USERAGENT = 'ansible-httpget' @@ -262,8 +267,5 @@ def main(): module.exit_json(changed=changed, name=name) -# import module snippets -from ansible.module_utils.basic import * -from ansible.module_utils.urls import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/macports.py b/lib/ansible/modules/packaging/os/macports.py index fc21ac5ff9..4d2b6f18d1 100644 --- a/lib/ansible/modules/packaging/os/macports.py +++ b/lib/ansible/modules/packaging/os/macports.py @@ -66,7 +66,8 @@ EXAMPLES = ''' state: inactive ''' -import pipes +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.six.moves import shlex_quote def update_package_db(module, port_path): @@ -83,7 +84,7 @@ def query_package(module, port_path, name, state="present"): if state == "present": - rc, out, err = module.run_command("%s installed | grep -q ^.*%s" % (pipes.quote(port_path), pipes.quote(name)), use_unsafe_shell=True) + rc, out, err = module.run_command("%s installed | grep -q ^.*%s" % (shlex_quote(port_path), shlex_quote(name)), use_unsafe_shell=True) if rc == 0: return True @@ -91,7 +92,7 @@ def query_package(module, port_path, name, state="present"): elif state == "active": - rc, out, err = module.run_command("%s installed %s | grep -q active" % (pipes.quote(port_path), pipes.quote(name)), use_unsafe_shell=True) + rc, out, err = module.run_command("%s installed %s | grep -q active" % (shlex_quote(port_path), shlex_quote(name)), use_unsafe_shell=True) if rc == 0: return True @@ -225,8 +226,6 @@ def main(): elif p["state"] == "inactive": deactivate_packages(module, port_path, pkgs) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/opkg.py b/lib/ansible/modules/packaging/os/opkg.py index 7fbe78702c..8a4863d721 100644 --- a/lib/ansible/modules/packaging/os/opkg.py +++ b/lib/ansible/modules/packaging/os/opkg.py @@ -87,7 +87,8 @@ EXAMPLES = ''' force: overwrite ''' -import pipes +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.six.moves import shlex_quote def update_package_db(module, opkg_path): @@ -104,7 +105,7 @@ def query_package(module, opkg_path, name, state="present"): if state == "present": - rc, out, err = module.run_command("%s list-installed | grep -q \"^%s \"" % (pipes.quote(opkg_path), pipes.quote(name)), use_unsafe_shell=True) + rc, out, err = module.run_command("%s list-installed | grep -q \"^%s \"" % (shlex_quote(opkg_path), shlex_quote(name)), use_unsafe_shell=True) if rc == 0: return True @@ -193,8 +194,6 @@ def main(): elif p["state"] in ["absent", "removed"]: remove_packages(module, opkg_path, pkgs) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/pkg5_publisher.py b/lib/ansible/modules/packaging/os/pkg5_publisher.py index e8a310353d..387303affd 100644 --- a/lib/ansible/modules/packaging/os/pkg5_publisher.py +++ b/lib/ansible/modules/packaging/os/pkg5_publisher.py @@ -75,6 +75,8 @@ EXAMPLES = ''' origin: 'https://pkg.example.com/site/' ''' +from ansible.module_utils.basic import AnsibleModule + def main(): module = AnsibleModule( @@ -205,7 +207,5 @@ def unstringify(val): return val -from ansible.module_utils.basic import * - if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/pkgin.py b/lib/ansible/modules/packaging/os/pkgin.py index 3bccaeb677..d7cf546794 100644 --- a/lib/ansible/modules/packaging/os/pkgin.py +++ b/lib/ansible/modules/packaging/os/pkgin.py @@ -132,6 +132,8 @@ EXAMPLES = ''' import re +from ansible.module_utils.basic import AnsibleModule + def query_package(module, name): """Search for the package by name. @@ -382,8 +384,6 @@ def main(): elif p["state"] == "absent": remove_packages(module, pkgs) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/pkgutil.py b/lib/ansible/modules/packaging/os/pkgutil.py index e6fbeb16a7..a0f29ea0cb 100644 --- a/lib/ansible/modules/packaging/os/pkgutil.py +++ b/lib/ansible/modules/packaging/os/pkgutil.py @@ -66,8 +66,7 @@ EXAMPLES = ''' state: latest ''' -import os -import pipes +from ansible.module_utils.basic import AnsibleModule def package_installed(module, name): @@ -225,8 +224,6 @@ def main(): module.exit_json(**result) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/portage.py b/lib/ansible/modules/packaging/os/portage.py index 0cded3b76a..e765cfdc93 100644 --- a/lib/ansible/modules/packaging/os/portage.py +++ b/lib/ansible/modules/packaging/os/portage.py @@ -220,11 +220,11 @@ EXAMPLES = ''' depclean: yes ''' - import os -import pipes import re +from ansible.module_utils.basic import AnsibleModule + def query_package(module, package, action): if package.startswith('@'): @@ -508,8 +508,6 @@ def main(): elif p['state'] in portage_absent_states: unmerge_packages(module, packages) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/slackpkg.py b/lib/ansible/modules/packaging/os/slackpkg.py index 20a22843be..386275b461 100644 --- a/lib/ansible/modules/packaging/os/slackpkg.py +++ b/lib/ansible/modules/packaging/os/slackpkg.py @@ -68,6 +68,8 @@ EXAMPLES = ''' state: latest ''' +from ansible.module_utils.basic import AnsibleModule + def query_package(module, slackpkg_path, name): @@ -195,8 +197,6 @@ def main(): elif p["state"] in ['removed', 'absent']: remove_packages(module, slackpkg_path, pkgs) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/svr4pkg.py b/lib/ansible/modules/packaging/os/svr4pkg.py index e4b7d31630..5709a16b54 100644 --- a/lib/ansible/modules/packaging/os/svr4pkg.py +++ b/lib/ansible/modules/packaging/os/svr4pkg.py @@ -105,6 +105,8 @@ EXAMPLES = ''' import os import tempfile +from ansible.module_utils.basic import AnsibleModule + def package_installed(module, name, category): cmd = [module.get_bin_path('pkginfo', True)] @@ -257,8 +259,6 @@ def main(): module.exit_json(**result) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/swdepot.py b/lib/ansible/modules/packaging/os/swdepot.py index f32e43c7e5..9783790d03 100644 --- a/lib/ansible/modules/packaging/os/swdepot.py +++ b/lib/ansible/modules/packaging/os/swdepot.py @@ -69,7 +69,9 @@ EXAMPLES = ''' ''' import re -import pipes + +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.six.moves import shlex_quote def compare_package(version1, version2): @@ -97,10 +99,10 @@ def query_package(module, name, depot=None): cmd_list = '/usr/sbin/swlist -a revision -l product' if depot: - rc, stdout, stderr = module.run_command("%s -s %s %s | grep %s" % (cmd_list, pipes.quote(depot), pipes.quote(name), pipes.quote(name)), + rc, stdout, stderr = module.run_command("%s -s %s %s | grep %s" % (cmd_list, shlex_quote(depot), shlex_quote(name), shlex_quote(name)), use_unsafe_shell=True) else: - rc, stdout, stderr = module.run_command("%s %s | grep %s" % (cmd_list, pipes.quote(name), pipes.quote(name)), use_unsafe_shell=True) + rc, stdout, stderr = module.run_command("%s %s | grep %s" % (cmd_list, shlex_quote(name), shlex_quote(name)), use_unsafe_shell=True) if rc == 0: version = re.sub(r"\s\s+|\t", " ", stdout).strip().split()[1] else: @@ -210,8 +212,6 @@ def main(): module.exit_json(changed=changed, name=name, state=state, msg=msg) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/lib/ansible/modules/packaging/os/yum_repository.py b/lib/ansible/modules/packaging/os/yum_repository.py index b0235a5b11..0baf6bfb09 100644 --- a/lib/ansible/modules/packaging/os/yum_repository.py +++ b/lib/ansible/modules/packaging/os/yum_repository.py @@ -447,9 +447,10 @@ state: ''' import os + from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils._text import to_native from ansible.module_utils.six.moves import configparser +from ansible.module_utils._text import to_native class YumRepo(object): diff --git a/lib/ansible/modules/packaging/os/zypper_repository.py b/lib/ansible/modules/packaging/os/zypper_repository.py index 92d9fe303c..9826ace8f0 100644 --- a/lib/ansible/modules/packaging/os/zypper_repository.py +++ b/lib/ansible/modules/packaging/os/zypper_repository.py @@ -143,10 +143,13 @@ EXAMPLES = ''' runrefresh: yes ''' -REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck'] - from distutils.version import LooseVersion +from ansible.module_utils.basic import AnsibleModule + + +REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck'] + def _get_cmd(*args): """Combines the non-interactive zypper command with arguments/subcommands""" @@ -403,8 +406,6 @@ def main(): else: module.fail_json(msg="Zypper failed with rc %s" % rc, rc=rc, stdout=stdout, stderr=stderr, repodata=repodata, state=state, warnings=warnings) -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() diff --git a/test/sanity/code-smell/no-wildcard-import.sh b/test/sanity/code-smell/no-wildcard-import.sh index aae43541af..d52a81f3bb 100755 --- a/test/sanity/code-smell/no-wildcard-import.sh +++ b/test/sanity/code-smell/no-wildcard-import.sh @@ -17,7 +17,6 @@ wildcard_imports=$(find . -path ./test/runner/.tox -prune \ -o -path ./test/units/plugins/action/test_action.py \ -o -path ./lib/ansible/compat/tests/mock.py -prune \ -o -path ./lib/ansible/compat/tests/unittest.py \ - -o -path ./lib/ansible/modules/packaging/os -prune \ -o -name '*.py' -type f -exec grep -H 'import \*' '{}' '+')