From 0d48da060d3f0adf76768bb72bf75076c2d86c0e Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Fri, 22 Mar 2024 11:47:09 -0300 Subject: [PATCH 1/5] lint tools: bump code verification tools versions Bump version of ansible-lint, Flake8, Pylint and yamllint to newer versions as used in Ansible tests. --- .github/workflows/lint.yml | 2 +- .pre-commit-config.yaml | 10 +++++----- requirements-dev.txt | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 849d9a1d5..fb22861e5 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,7 +16,7 @@ jobs: python-version: "3.x" - name: Run ansible-lint run: | - pip install "ansible-core>=2.16,<2.17" 'ansible-lint>=6.22' + pip install "ansible-core>=2.16,<2.17" 'ansible-lint==6.22' utils/build-galaxy-release.sh -ki cd .galaxy-build ansible-lint --profile production --exclude tests/integration/ --exclude tests/unit/ --parseable --nocolor diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 323b56ea4..82bfbcd71 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ --- repos: - repo: https://github.com/ansible/ansible-lint.git - rev: v6.22.0 + rev: v24.5.0 hooks: - id: ansible-lint always_run: false @@ -21,20 +21,20 @@ repos: --parseable --nocolor - repo: https://github.com/adrienverge/yamllint.git - rev: v1.32.0 + rev: v1.35.1 hooks: - id: yamllint files: \.(yaml|yml)$ - repo: https://github.com/pycqa/flake8 - rev: 6.0.0 + rev: 7.0.0 hooks: - id: flake8 - repo: https://github.com/pycqa/pydocstyle - rev: 6.0.0 + rev: 6.3.0 hooks: - id: pydocstyle - repo: https://github.com/pycqa/pylint - rev: v3.0.2 + rev: v3.2.2 hooks: - id: pylint args: diff --git a/requirements-dev.txt b/requirements-dev.txt index 5df0f4335..08e336621 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,10 +1,10 @@ -r requirements-tests.txt ipdb==0.13.4 pre-commit==2.20.0 -flake8==6.0.0 +flake8==7.0.0 flake8-bugbear -pylint==2.17.2 +pylint>=3.2 wrapt==1.14.1 pydocstyle==6.3.0 -yamllint==1.32.0 -ansible-lint >= 6.22 +yamllint==1.35.1 +ansible-lint>=24.5.0 From 60905ef5bfcc29c60496875b6112927805d6096f Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 26 Mar 2024 10:53:34 -0300 Subject: [PATCH 2/5] upstream ci: Update Github actions Github actions checkout v3.1.0 and setup-python v4.3.0 use deprecated Node.js 16. Bumping version to checkout v4.1.1 and setup-python v5.1.0 fixes the workflows, as both use the recommended Node.js 20. The checkout depth has been set to 1 (shallow copy) for all tasks that do not require git history to be available. --- .github/workflows/ansible-test.yml | 2 +- .github/workflows/docs.yml | 24 +++++++++++----------- .github/workflows/lint.yml | 32 +++++++++++++++--------------- .github/workflows/readme.yml | 4 ++-- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ansible-test.yml b/.github/workflows/ansible-test.yml index 24929098d..1aecc6150 100644 --- a/.github/workflows/ansible-test.yml +++ b/.github/workflows/ansible-test.yml @@ -8,7 +8,7 @@ jobs: name: Verify ansible-test sanity runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: fetch-depth: 0 - name: Run ansible-test diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c5f1719ba..f509046c7 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -8,10 +8,10 @@ jobs: name: Check Ansible Documentation with ansible-core 2.13. runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: '3.x' - name: Install Ansible 2.13 @@ -25,10 +25,10 @@ jobs: name: Check Ansible Documentation with ansible-core 2.14. runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: '3.x' - name: Install Ansible 2.14 @@ -42,10 +42,10 @@ jobs: name: Check Ansible Documentation with ansible-core 2.15. runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: '3.x' - name: Install Ansible 2.15 @@ -59,10 +59,10 @@ jobs: name: Check Ansible Documentation with latest Ansible version. runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: '3.x' - name: Install Ansible-latest diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index fb22861e5..cec6196cd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -8,10 +8,10 @@ jobs: name: Verify ansible-lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + - uses: actions/setup-python@v5.1.0 with: python-version: "3.x" - name: Run ansible-lint @@ -25,10 +25,10 @@ jobs: name: Verify yamllint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: "3.x" - name: Run yaml-lint @@ -38,10 +38,10 @@ jobs: name: Verify pydocstyle runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: "3.x" - name: Run pydocstyle @@ -53,10 +53,10 @@ jobs: name: Verify flake8 runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: "3.x" - name: Run flake8 @@ -68,10 +68,10 @@ jobs: name: Verify pylint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 - - uses: actions/setup-python@v4.3.0 + fetch-depth: 1 + - uses: actions/setup-python@v5.1.0 with: python-version: "3.x" - name: Run pylint @@ -83,8 +83,8 @@ jobs: name: Shellcheck runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 + fetch-depth: 1 - name: Run ShellCheck uses: ludeeus/action-shellcheck@master diff --git a/.github/workflows/readme.yml b/.github/workflows/readme.yml index edea5b9fe..5f821f672 100644 --- a/.github/workflows/readme.yml +++ b/.github/workflows/readme.yml @@ -8,9 +8,9 @@ jobs: name: Verify readme runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3.1.0 + - uses: actions/checkout@v4.1.1 with: - fetch-depth: 0 + fetch-depth: 1 - name: Run readme test run: | error=0 From f53ca3ad39ad7b6716d059ae5624df79474a3387 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Wed, 22 May 2024 10:04:22 -0300 Subject: [PATCH 3/5] pylint: Ignore usage of 'unicode' before assignment New versions of pylint ignore Python 2 functions and types, evaluating 'unicode' as "undefined". ansible-freeipa will always define 'unicode' when running under Python 3, and it is always defined under Python 2. This patch fixes these false positives. --- plugins/module_utils/ansible_freeipa_module.py | 5 ++++- plugins/modules/ipagroup.py | 4 +++- plugins/modules/ipahost.py | 2 +- plugins/modules/iparole.py | 2 +- plugins/modules/ipaservice.py | 2 +- plugins/modules/ipauser.py | 2 +- roles/ipareplica/library/ipareplica_add_to_ipaservers.py | 2 +- roles/ipareplica/library/ipareplica_prepare.py | 2 +- 8 files changed, 13 insertions(+), 8 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 16b4fa810..8675beee9 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -487,7 +487,10 @@ def module_params_get(module, name, allow_empty_list_item=False): # Ansible issue https://github.com/ansible/ansible/issues/77108 if isinstance(value, list): for val in value: - if isinstance(val, (str, unicode)) and not val: + if ( + isinstance(val, (str, unicode)) # pylint: disable=E0606 + and not val + ): if not allow_empty_list_item: module.fail_json( msg="Parameter '%s' contains an empty string" % diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index dcb1f3a36..1ffdef9de 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -663,7 +663,9 @@ def main(): check_parameters(ansible_module, state, action) - elif isinstance(group_name, (str, unicode)): + elif ( + isinstance(group_name, (str, unicode)) # pylint: disable=E0606 + ): name = group_name else: ansible_module.fail_json(msg="Group '%s' is not valid" % diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py index a1d214236..86336c461 100644 --- a/plugins/modules/ipahost.py +++ b/plugins/modules/ipahost.py @@ -988,7 +988,7 @@ def main(): sshpubkey = [str(normalize_sshpubkey(key)) for key in sshpubkey] - elif isinstance(host, (str, unicode)): + elif isinstance(host, (str, unicode)): # pylint: disable=E0606 name = host else: ansible_module.fail_json(msg="Host '%s' is not valid" % diff --git a/plugins/modules/iparole.py b/plugins/modules/iparole.py index 2e8762680..25017e766 100644 --- a/plugins/modules/iparole.py +++ b/plugins/modules/iparole.py @@ -293,7 +293,7 @@ def result_get_value_lowercase(res_find, key, default=None): if existing is not None: if isinstance(existing, (list, tuple)): existing = [to_text(item).lower() for item in existing] - if isinstance(existing, (str, unicode)): + if isinstance(existing, (str, unicode)): # pylint: disable=E0606 existing = existing.lower() else: existing = default diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 1a4fd7146..8326d4228 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -693,7 +693,7 @@ def main(): delete_continue = service.get("delete_continue") - elif isinstance(service, (str, unicode)): + elif isinstance(service, (str, unicode)): # pylint: disable=E0606 name = service else: ansible_module.fail_json(msg="Service '%s' is not valid" % diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index 8c8bb436c..d7c18082d 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -1382,7 +1382,7 @@ def main(): email = extend_emails(email, default_email_domain) - elif isinstance(user, (str, unicode)): + elif isinstance(user, (str, unicode)): # pylint: disable=E0606 name = user else: ansible_module.fail_json(msg="User '%s' is not valid" % diff --git a/roles/ipareplica/library/ipareplica_add_to_ipaservers.py b/roles/ipareplica/library/ipareplica_add_to_ipaservers.py index 66c1615b1..6fb8b1f92 100644 --- a/roles/ipareplica/library/ipareplica_add_to_ipaservers.py +++ b/roles/ipareplica/library/ipareplica_add_to_ipaservers.py @@ -139,7 +139,7 @@ def main(): conn.connect(ccache=installer._ccache) remote_api.Command['hostgroup_add_member']( u'ipaservers', - host=[unicode(api.env.host)], + host=[unicode(api.env.host)], # pylint: disable=E0606 ) finally: if conn.isconnected(): diff --git a/roles/ipareplica/library/ipareplica_prepare.py b/roles/ipareplica/library/ipareplica_prepare.py index 962432407..5d9fd2d54 100644 --- a/roles/ipareplica/library/ipareplica_prepare.py +++ b/roles/ipareplica/library/ipareplica_prepare.py @@ -658,7 +658,7 @@ def main(): # Check authorization result = remote_api.Command['hostgroup_find']( cn=u'ipaservers', - host=[unicode(api.env.host)] + host=[unicode(api.env.host)] # pylint: disable=E0606 )['result'] add_to_ipaservers = not result From 52241fe233addea50b9e071618fbca53cdb9b27a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Wed, 22 May 2024 10:22:15 -0300 Subject: [PATCH 4/5] pylint: ensure variables are initialized pylint doesn't know that some functions may terminate execution, like, AnsibleModule's fail_json, and assume that, depending on the code path, some variables may not be initialized when used. This change ensure that variables are always initialized independent of the code path. --- plugins/modules/ipaautomember.py | 10 ++++------ plugins/modules/ipadnsforwardzone.py | 2 ++ plugins/modules/ipadnsrecord.py | 2 ++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/modules/ipaautomember.py b/plugins/modules/ipaautomember.py index 027140100..eb5dec19c 100644 --- a/plugins/modules/ipaautomember.py +++ b/plugins/modules/ipaautomember.py @@ -450,6 +450,10 @@ def main(): commands = [] for name in names: + _type = None + inclusive_add, inclusive_del = [], [] + exclusive_add, exclusive_del = [], [] + # Make sure automember rule exists res_find = find_automember(ansible_module, name, automember_type) @@ -495,16 +499,12 @@ def main(): transform_conditions(inclusive), res_find.get("automemberinclusiveregex", []) ) - else: - inclusive_add, inclusive_del = [], [] if exclusive is not None: exclusive_add, exclusive_del = gen_add_del_lists( transform_conditions(exclusive), res_find.get("automemberexclusiveregex", []) ) - else: - exclusive_add, exclusive_del = [], [] elif action == "member": if res_find is None: @@ -512,9 +512,7 @@ def main(): msg="No automember '%s'" % name) inclusive_add = transform_conditions(inclusive or []) - inclusive_del = [] exclusive_add = transform_conditions(exclusive or []) - exclusive_del = [] for _inclusive in inclusive_add: key, regex = _inclusive.split("=", 1) diff --git a/plugins/modules/ipadnsforwardzone.py b/plugins/modules/ipadnsforwardzone.py index 93a31f5ec..1397934ee 100644 --- a/plugins/modules/ipadnsforwardzone.py +++ b/plugins/modules/ipadnsforwardzone.py @@ -250,6 +250,8 @@ def main(): operation = "add" invalid = [] + wants_enable = False + if state in ["enabled", "disabled"]: if action == "member": ansible_module.fail_json( diff --git a/plugins/modules/ipadnsrecord.py b/plugins/modules/ipadnsrecord.py index f027791c8..c8559c49e 100644 --- a/plugins/modules/ipadnsrecord.py +++ b/plugins/modules/ipadnsrecord.py @@ -1605,6 +1605,8 @@ def main(): res_find = find_dnsrecord(ansible_module, zone_name, name) + cmds = [] + if state == 'present': cmds = define_commands_for_present_state( ansible_module, zone_name, entry, res_find) From 77c1d206d36f026448dc14a61ffcae1f2052d4c0 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Wed, 22 May 2024 11:20:12 -0300 Subject: [PATCH 5/5] fixup! pylint: Ignore usage of 'unicode' before assignment --- plugins/module_utils/ansible_freeipa_module.py | 2 +- plugins/modules/ipagroup.py | 4 +++- plugins/modules/ipahost.py | 4 +++- plugins/modules/iparole.py | 2 +- plugins/modules/ipaservice.py | 6 +++++- plugins/modules/ipauser.py | 6 +++++- roles/ipareplica/library/ipareplica_add_to_ipaservers.py | 2 +- roles/ipareplica/library/ipareplica_prepare.py | 2 +- 8 files changed, 20 insertions(+), 8 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 8675beee9..f372301ff 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -488,7 +488,7 @@ def module_params_get(module, name, allow_empty_list_item=False): if isinstance(value, list): for val in value: if ( - isinstance(val, (str, unicode)) # pylint: disable=E0606 + isinstance(val, (str, unicode)) # pylint: disable=W0012,E0606 and not val ): if not allow_empty_list_item: diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index 1ffdef9de..6be6d5422 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -664,7 +664,9 @@ def main(): check_parameters(ansible_module, state, action) elif ( - isinstance(group_name, (str, unicode)) # pylint: disable=E0606 + isinstance( + group_name, (str, unicode) # pylint: disable=W0012,E0606 + ) ): name = group_name else: diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py index 86336c461..79d51f7fb 100644 --- a/plugins/modules/ipahost.py +++ b/plugins/modules/ipahost.py @@ -988,7 +988,9 @@ def main(): sshpubkey = [str(normalize_sshpubkey(key)) for key in sshpubkey] - elif isinstance(host, (str, unicode)): # pylint: disable=E0606 + elif ( + isinstance(host, (str, unicode)) # pylint: disable=W0012,E0606 + ): name = host else: ansible_module.fail_json(msg="Host '%s' is not valid" % diff --git a/plugins/modules/iparole.py b/plugins/modules/iparole.py index 25017e766..ba77db9ac 100644 --- a/plugins/modules/iparole.py +++ b/plugins/modules/iparole.py @@ -293,7 +293,7 @@ def result_get_value_lowercase(res_find, key, default=None): if existing is not None: if isinstance(existing, (list, tuple)): existing = [to_text(item).lower() for item in existing] - if isinstance(existing, (str, unicode)): # pylint: disable=E0606 + if isinstance(existing, (str, unicode)): # pylint: disable=W0012,E0606 existing = existing.lower() else: existing = default diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 8326d4228..5cf821210 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -693,7 +693,11 @@ def main(): delete_continue = service.get("delete_continue") - elif isinstance(service, (str, unicode)): # pylint: disable=E0606 + elif ( + isinstance( + service, (str, unicode) # pylint: disable=W0012,E0606 + ) + ): name = service else: ansible_module.fail_json(msg="Service '%s' is not valid" % diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index d7c18082d..37fe734a2 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -1382,7 +1382,11 @@ def main(): email = extend_emails(email, default_email_domain) - elif isinstance(user, (str, unicode)): # pylint: disable=E0606 + elif ( + isinstance( + user, (str, unicode) # pylint: disable=W0012,E0606 + ) + ): name = user else: ansible_module.fail_json(msg="User '%s' is not valid" % diff --git a/roles/ipareplica/library/ipareplica_add_to_ipaservers.py b/roles/ipareplica/library/ipareplica_add_to_ipaservers.py index 6fb8b1f92..4056f705d 100644 --- a/roles/ipareplica/library/ipareplica_add_to_ipaservers.py +++ b/roles/ipareplica/library/ipareplica_add_to_ipaservers.py @@ -139,7 +139,7 @@ def main(): conn.connect(ccache=installer._ccache) remote_api.Command['hostgroup_add_member']( u'ipaservers', - host=[unicode(api.env.host)], # pylint: disable=E0606 + host=[unicode(api.env.host)], # pylint: disable=W0012,E0606 ) finally: if conn.isconnected(): diff --git a/roles/ipareplica/library/ipareplica_prepare.py b/roles/ipareplica/library/ipareplica_prepare.py index 5d9fd2d54..63f1dcbdc 100644 --- a/roles/ipareplica/library/ipareplica_prepare.py +++ b/roles/ipareplica/library/ipareplica_prepare.py @@ -658,7 +658,7 @@ def main(): # Check authorization result = remote_api.Command['hostgroup_find']( cn=u'ipaservers', - host=[unicode(api.env.host)] # pylint: disable=E0606 + host=[unicode(api.env.host)] # pylint: disable=W0012,E0606 )['result'] add_to_ipaservers = not result