From a961da9145e2bc6177960d34e38e39eaa5591af5 Mon Sep 17 00:00:00 2001 From: Sagar Paul Date: Wed, 31 Jan 2024 14:56:09 +0530 Subject: [PATCH 1/3] [GHA] Update reference for GitHub actions (#989) * [GHA] Update reference for GitHub actions * fix tests * test how it goes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * reference sep workflow * fix tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix tests * hold source-req file * honour name change * fix name * Temp add unit source * Make PR review ready * fix update * revert codecov --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .ansible-lint | 6 +- .../workflows/{ack.yml => check_label.yml} | 10 +- .github/workflows/codecoverage.yml | 3 +- .github/workflows/draft_release.yml | 18 +++ .github/workflows/lint.yml | 13 -- .github/workflows/push.yml | 38 ----- .github/workflows/release.yml | 4 +- .github/workflows/tests.yml | 18 +-- .../unit/modules/network/ios/test_ios_acls.py | 131 ++++++++++++++---- tox-ansible.ini | 10 ++ 10 files changed, 154 insertions(+), 97 deletions(-) rename .github/workflows/{ack.yml => check_label.yml} (60%) create mode 100644 .github/workflows/draft_release.yml delete mode 100644 .github/workflows/lint.yml delete mode 100644 .github/workflows/push.yml create mode 100644 tox-ansible.ini diff --git a/.ansible-lint b/.ansible-lint index 8d9bb70b8..d2869106c 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -1,5 +1,7 @@ --- + profile: production -exclude_paths: - - changelogs/changelog.yaml +# exclude_paths: + +# - changelogs/changelog.yaml diff --git a/.github/workflows/ack.yml b/.github/workflows/check_label.yml similarity index 60% rename from .github/workflows/ack.yml rename to .github/workflows/check_label.yml index fda595dc5..b120bfa32 100644 --- a/.github/workflows/ack.yml +++ b/.github/workflows/check_label.yml @@ -1,15 +1,11 @@ --- -# See https://github.com/ansible-community/devtools/blob/main/.github/workflows/ack.yml -name: ack - +name: "Check label" concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true - on: # yamllint disable-line rule:truthy pull_request_target: types: [opened, labeled, unlabeled, synchronize] - jobs: - ack: - uses: ansible/devtools/.github/workflows/ack.yml@main + check_label: + uses: ansible/ansible-content-actions/.github/workflows/check_label.yaml@main diff --git a/.github/workflows/codecoverage.yml b/.github/workflows/codecoverage.yml index c2a7ad60d..3e0f17919 100644 --- a/.github/workflows/codecoverage.yml +++ b/.github/workflows/codecoverage.yml @@ -1,8 +1,9 @@ --- -name: code_coverage +name: "Code coverage" on: # yamllint disable-line rule:truthy push: + branches: [main] pull_request: branches: [ main ] diff --git a/.github/workflows/draft_release.yml b/.github/workflows/draft_release.yml new file mode 100644 index 000000000..33a890a57 --- /dev/null +++ b/.github/workflows/draft_release.yml @@ -0,0 +1,18 @@ +--- +name: "Draft release" +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true +on: # yamllint disable-line rule:truthy + workflow_dispatch: +env: + NAMESPACE: ${{ github.repository_owner }} + COLLECTION_NAME: ios + ANSIBLE_COLLECTIONS_PATHS: ./ +jobs: + update_release_draft: + uses: ansible/ansible-content-actions/.github/workflows/draft_release.yaml@main + with: + repo: ${{ github.event.pull_request.head.repo.full_name }} + secrets: + BOT_PAT: ${{ secrets.BOT_PAT }} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml deleted file mode 100644 index fbac38cbf..000000000 --- a/.github/workflows/lint.yml +++ /dev/null @@ -1,13 +0,0 @@ ---- -name: ansible-lint -on: # yamllint disable-line rule:truthy - pull_request: - branches: ["main"] -jobs: - build: - name: Ansible Lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Run ansible-lint - uses: ansible/ansible-lint@main diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml deleted file mode 100644 index 2e144f623..000000000 --- a/.github/workflows/push.yml +++ /dev/null @@ -1,38 +0,0 @@ ---- -# push workflow is shared and expected to perform actions after a merge happens -# on a maintenance branch (default or release). For example updating the -# draft release-notes. -# based on great work from -# https://github.com/T-Systems-MMS/ansible-collection-icinga-director -name: push - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -on: # yamllint disable-line rule:truthy - # # auto generate changelog and add PR by ansibuddy - # push: - # # branches to consider in the event; optional, defaults to all - # branches: - # - main - # - 'releases/**' - # - 'stable/**' - # # Prevent a 2nd run after the changelog is updated - # paths-ignore: - # - CHANGELOG.rst - # - changelogs/changelog.yaml - workflow_dispatch: - -env: - NAMESPACE: cisco - COLLECTION_NAME: ios - ANSIBLE_COLLECTIONS_PATHS: ./ - -jobs: - update_release_draft: - uses: ansible/devtools/.github/workflows/push_network.yml@main - with: - repo: ansible-collections/cisco.ios - secrets: - BOT_PAT: ${{ secrets.BOT_PAT }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index eb04259d1..6dbb1aa39 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,12 +1,12 @@ --- -name: release +name: "Release collection" on: # yamllint disable-line rule:truthy release: types: [published] jobs: release: - uses: ansible/devtools/.github/workflows/release_collection.yml@main + uses: ansible/ansible-content-actions/.github/workflows/release.yaml@main with: environment: release secrets: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2c1d85055..27291dc81 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,5 +1,5 @@ --- -name: CI +name: "CI" concurrency: group: ${{ github.head_ref || github.run_id }} @@ -12,15 +12,16 @@ on: # yamllint disable-line rule:truthy schedule: - cron: '0 0 * * *' - jobs: changelog: - uses: ansible-network/github_actions/.github/workflows/changelog.yml@main - if: github.event_name != 'schedule' + uses: ansible/ansible-content-actions/.github/workflows/changelog.yaml@main + if: github.event_name == 'pull_request' + ansible-lint: + uses: ansible/ansible-content-actions/.github/workflows/ansible_lint.yaml@main sanity: - uses: ansible-network/github_actions/.github/workflows/sanity.yml@main + uses: ansible/ansible-content-actions/.github/workflows/sanity.yaml@main unit-galaxy: - uses: ansible-network/github_actions/.github/workflows/unit_galaxy.yml@main + uses: ansible/ansible-content-actions/.github/workflows/unit.yaml@main unit-source: uses: ansible-network/github_actions/.github/workflows/unit_source.yml@main with: @@ -33,7 +34,7 @@ jobs: - changelog - sanity - unit-galaxy - - unit-source + - ansible-lint runs-on: ubuntu-latest steps: - run: >- @@ -41,6 +42,7 @@ jobs: set([ '${{ needs.changelog.result }}', '${{ needs.sanity.result }}', - '${{ needs.unit-galaxy.result }}', + '${{ needs.unit-galaxy.result }}' + '${{ needs.ansible-lint.result }}' '${{ needs.unit-source.result }}' ])" diff --git a/tests/unit/modules/network/ios/test_ios_acls.py b/tests/unit/modules/network/ios/test_ios_acls.py index 33c4a1ddb..83f256ffb 100644 --- a/tests/unit/modules/network/ios/test_ios_acls.py +++ b/tests/unit/modules/network/ios/test_ios_acls.py @@ -91,7 +91,10 @@ def test_ios_acls_merged(self): aces=[ dict( grant="deny", - source=dict(address="192.0.2.0", wildcard_bits="0.0.0.255"), + source=dict( + address="192.0.2.0", + wildcard_bits="0.0.0.255", + ), ), ], ), @@ -137,7 +140,9 @@ def test_ios_acls_merged(self): protocol_options=dict(tcp=dict(ack="True")), sequence="200", source=dict(object_group="test_network_og"), - destination=dict(object_group="test_network_og"), + destination=dict( + object_group="test_network_og", + ), dscp="ef", ttl=dict(eq=10), ), @@ -223,7 +228,10 @@ def test_ios_acls_merged_remarks_positional(self): { "aces": [ { - "destination": {"any": True, "port_protocol": {"eq": "22"}}, + "destination": { + "any": True, + "port_protocol": {"eq": "22"}, + }, "grant": "permit", "protocol": "tcp", "sequence": 10, @@ -233,14 +241,20 @@ def test_ios_acls_merged_remarks_positional(self): }, }, { - "destination": {"any": True, "port_protocol": {"eq": "22"}}, + "destination": { + "any": True, + "port_protocol": {"eq": "22"}, + }, "grant": "permit", "protocol": "tcp", "sequence": 20, "source": {"host": "10.160.114.111"}, }, { - "destination": {"any": True, "port_protocol": {"eq": "22"}}, + "destination": { + "any": True, + "port_protocol": {"eq": "22"}, + }, "grant": "permit", "protocol": "tcp", "sequence": 30, @@ -361,7 +375,9 @@ def test_ios_acls_merged_remarks_positional(self): "grant": "permit", "protocol": "ipv6", "sequence": 10, - "source": {"address": "2001:ABAD:BEEF:1221::/64"}, + "source": { + "address": "2001:ABAD:BEEF:1221::/64", + }, }, { "destination": { @@ -380,7 +396,10 @@ def test_ios_acls_merged_remarks_positional(self): "aces": [ {"remarks": ["empty remark 1"], "sequence": 10}, {"remarks": ["empty remark 2"], "sequence": 20}, - {"remarks": ["empty remark never ends"], "sequence": 30}, + { + "remarks": ["empty remark never ends"], + "sequence": 30, + }, ], "name": "empty_ipv6_acl", }, @@ -402,7 +421,10 @@ def test_ios_acls_merged_remarks_positional(self): "sequence": 40, "source": {"any": True}, }, - {"remarks": ["I am new set of ipv6 ace"], "sequence": 50}, + { + "remarks": ["I am new set of ipv6 ace"], + "sequence": 50, + }, { "destination": {"any": True}, "grant": "permit", @@ -518,7 +540,10 @@ def test_ios_acls_merged_idempotent(self): "sequence": 10, "grant": "deny", "protocol": "tcp", - "source": {"any": True, "port_protocol": {"eq": "www"}}, + "source": { + "any": True, + "port_protocol": {"eq": "www"}, + }, "destination": { "any": True, "port_protocol": {"eq": "telnet"}, @@ -641,7 +666,10 @@ def test_ios_acls_replaced_idempotent(self): "address": "198.51.100.0", "wildcard_bits": "0.0.0.255", }, - "destination": {"any": True, "port_protocol": {"eq": "22"}}, + "destination": { + "any": True, + "port_protocol": {"eq": "22"}, + }, "log": {"set": True, "user_cookie": "testLog"}, }, { @@ -718,7 +746,10 @@ def test_ios_acls_replaced_idempotent(self): "sequence": 10, "grant": "deny", "protocol": "tcp", - "source": {"any": True, "port_protocol": {"eq": "www"}}, + "source": { + "any": True, + "port_protocol": {"eq": "www"}, + }, "destination": { "any": True, "port_protocol": {"eq": "telnet"}, @@ -831,7 +862,10 @@ def test_ios_acls_overridden_idempotent(self): "address": "198.51.100.0", "wildcard_bits": "0.0.0.255", }, - "destination": {"any": True, "port_protocol": {"eq": "22"}}, + "destination": { + "any": True, + "port_protocol": {"eq": "22"}, + }, "log": {"set": True, "user_cookie": "testLog"}, }, { @@ -874,7 +908,10 @@ def test_ios_acls_overridden_idempotent(self): "sequence": 10, "grant": "deny", "protocol": "tcp", - "source": {"any": True, "port_protocol": {"eq": "www"}}, + "source": { + "any": True, + "port_protocol": {"eq": "www"}, + }, "destination": { "any": True, "port_protocol": {"eq": "telnet"}, @@ -909,7 +946,10 @@ def test_ios_acls_deleted_afi_based(self): self.execute_show_command_name.return_value = dedent("") set_module_args(dict(config=[dict(afi="ipv4")], state="deleted")) result = self.execute_module(changed=True) - commands = ["no ip access-list extended 110", "no ip access-list standard test_acl"] + commands = [ + "no ip access-list extended 110", + "no ip access-list standard test_acl", + ] self.assertEqual(sorted(result["commands"]), sorted(commands)) def test_ios_acls_deleted_acl_based(self): @@ -938,7 +978,10 @@ def test_ios_acls_deleted_acl_based(self): grant="deny", protocol_options=dict(icmp=dict(echo="True")), sequence="10", - source=dict(address="192.0.2.0", wildcard_bits="0.0.0.255"), + source=dict( + address="192.0.2.0", + wildcard_bits="0.0.0.255", + ), destination=dict( address="192.0.3.0", wildcard_bits="0.0.0.255", @@ -960,7 +1003,10 @@ def test_ios_acls_deleted_acl_based(self): grant="deny", protocol_options=dict(tcp=dict(ack="True")), sequence="10", - source=dict(any="True", port_protocol=dict(eq="www")), + source=dict( + any="True", + port_protocol=dict(eq="www"), + ), destination=dict( any="True", port_protocol=dict(eq="telnet"), @@ -994,9 +1040,15 @@ def test_ios_acls_rendered(self): dict( grant="deny", sequence="10", - remarks=["check for remark", "remark for acl 110"], + remarks=[ + "check for remark", + "remark for acl 110", + ], protocol_options=dict(tcp=dict(syn="True")), - source=dict(address="192.0.2.0", wildcard_bits="0.0.0.255"), + source=dict( + address="192.0.2.0", + wildcard_bits="0.0.0.255", + ), destination=dict( address="192.0.3.0", wildcard_bits="0.0.0.255", @@ -1046,7 +1098,9 @@ def test_ios_acls_parsed(self): "source": {"any": True, "port_protocol": {"eq": "www"}}, "destination": { "any": True, - "port_protocol": {"range": {"start": 10, "end": 20}}, + "port_protocol": { + "range": {"start": 10, "end": 20}, + }, }, "dscp": "af11", "protocol_options": {"tcp": {"ack": True}}, @@ -1092,7 +1146,11 @@ def test_ios_acls_parsed_matches(self): "name": "R1_TRAFFIC", "acl_type": "standard", "aces": [ - {"sequence": 10, "grant": "permit", "source": {"host": "10.11.12.13"}}, + { + "sequence": 10, + "grant": "permit", + "source": {"host": "10.11.12.13"}, + }, { "sequence": 40, "grant": "permit", @@ -1543,7 +1601,10 @@ def test_ios_acls_parsed_multioption(self): "sequence": 70, "grant": "permit", "protocol": "tcp", - "source": {"address": "10.1.1.0", "wildcard_bits": "0.0.0.255"}, + "source": { + "address": "10.1.1.0", + "wildcard_bits": "0.0.0.255", + }, "destination": { "address": "172.16.1.0", "port_protocol": {"eq": "telnet"}, @@ -1557,9 +1618,21 @@ def test_ios_acls_parsed_multioption(self): "name": "2", "acl_type": "standard", "aces": [ - {"sequence": 30, "grant": "permit", "source": {"host": "172.16.1.11"}}, - {"sequence": 20, "grant": "permit", "source": {"host": "172.16.1.10"}}, - {"sequence": 10, "grant": "permit", "source": {"host": "172.16.1.2"}}, + { + "sequence": 30, + "grant": "permit", + "source": {"host": "172.16.1.11"}, + }, + { + "sequence": 20, + "grant": "permit", + "source": {"host": "172.16.1.10"}, + }, + { + "sequence": 10, + "grant": "permit", + "source": {"host": "172.16.1.2"}, + }, ], }, { @@ -1570,7 +1643,10 @@ def test_ios_acls_parsed_multioption(self): "sequence": 10, "grant": "permit", "protocol": "icmp", - "source": {"address": "10.1.1.0", "wildcard_bits": "0.0.0.255"}, + "source": { + "address": "10.1.1.0", + "wildcard_bits": "0.0.0.255", + }, "destination": { "address": "172.16.1.0", "wildcard_bits": "0.0.0.255", @@ -1594,7 +1670,10 @@ def test_ios_acls_parsed_multioption(self): "grant": "permit", "protocol": "tcp", "source": {"host": "10.1.1.1"}, - "destination": {"host": "10.5.5.5", "port_protocol": {"eq": "www"}}, + "destination": { + "host": "10.5.5.5", + "port_protocol": {"eq": "www"}, + }, }, { "sequence": 30, diff --git a/tox-ansible.ini b/tox-ansible.ini new file mode 100644 index 000000000..5e1f4b36a --- /dev/null +++ b/tox-ansible.ini @@ -0,0 +1,10 @@ +[ansible] + +skip = + py3.7 + py3.8 + 2.9 + 2.10 + 2.11 + 2.12 + 2.13 From d629cc66a0564fc877824b9ddead743b0a7810ad Mon Sep 17 00:00:00 2001 From: Vinay M <63404819+roverflow@users.noreply.github.com> Date: Wed, 31 Jan 2024 19:03:33 +0530 Subject: [PATCH 2/3] Resolve idempotency issue in SNMP Server user attribute handling (#1014) * Fix snmp facts not rendering correctly * cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed changelog * Fix unnecessary negation of user * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * cleanup --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Sagar Paul --- changelogs/fragments/snmp_idempotancy_fix.yml | 3 ++ .../ios/config/snmp_server/snmp_server.py | 45 ++++++------------- .../network/ios/test_ios_snmp_server.py | 3 +- 3 files changed, 18 insertions(+), 33 deletions(-) create mode 100644 changelogs/fragments/snmp_idempotancy_fix.yml diff --git a/changelogs/fragments/snmp_idempotancy_fix.yml b/changelogs/fragments/snmp_idempotancy_fix.yml new file mode 100644 index 000000000..ce420963e --- /dev/null +++ b/changelogs/fragments/snmp_idempotancy_fix.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - ios_snmp_server - fixed config issue with snmp user password update being idempotent on consecutive runs. diff --git a/plugins/module_utils/network/ios/config/snmp_server/snmp_server.py b/plugins/module_utils/network/ios/config/snmp_server/snmp_server.py index 187d0779d..d61a70a10 100644 --- a/plugins/module_utils/network/ios/config/snmp_server/snmp_server.py +++ b/plugins/module_utils/network/ios/config/snmp_server/snmp_server.py @@ -237,39 +237,20 @@ def _compare(self, want, have): def _compare_lists_attrs(self, want, have): """Compare list of dict""" for _parser in self.list_parsers: - if _parser == "users": - i_want = want.get(_parser, {}) - i_have = have.get(_parser, {}) - for key, wanting in iteritems(i_want): - wanting_compare = deepcopy(wanting) - if ( - "authentication" in wanting_compare - and "password" in wanting_compare["authentication"] - ): - wanting_compare["authentication"].pop("password") - if ( - "encryption" in wanting_compare - and "password" in wanting_compare["encryption"] - ): - wanting_compare["encryption"].pop("password") - haveing = i_have.pop(key, {}) - if wanting_compare != haveing: - if haveing and self.state in ["overridden", "replaced"]: + i_want = want.get(_parser, {}) + i_have = have.get(_parser, {}) + for key, wanting in iteritems(i_want): + haveing = i_have.pop(key, {}) + if wanting != haveing: + if haveing and self.state in ["overridden", "replaced"]: + if not ( + _parser == "users" + and wanting.get("username") == haveing.get("username") + ): self.addcmd(haveing, _parser, negate=True) - self.addcmd(wanting, _parser) - for key, haveing in iteritems(i_have): - self.addcmd(haveing, _parser, negate=True) - else: - i_want = want.get(_parser, {}) - i_have = have.get(_parser, {}) - for key, wanting in iteritems(i_want): - haveing = i_have.pop(key, {}) - if wanting != haveing: - if haveing and self.state in ["overridden", "replaced"]: - self.addcmd(haveing, _parser, negate=True) - self.addcmd(wanting, _parser) - for key, haveing in iteritems(i_have): - self.addcmd(haveing, _parser, negate=True) + self.addcmd(wanting, _parser) + for key, haveing in iteritems(i_have): + self.addcmd(haveing, _parser, negate=True) def _snmp_list_to_dict(self, data): """Convert all list of dicts to dicts of dicts""" diff --git a/tests/unit/modules/network/ios/test_ios_snmp_server.py b/tests/unit/modules/network/ios/test_ios_snmp_server.py index 905371b03..28a11fd7c 100644 --- a/tests/unit/modules/network/ios/test_ios_snmp_server.py +++ b/tests/unit/modules/network/ios/test_ios_snmp_server.py @@ -1933,10 +1933,11 @@ def test_ios_snmpv3_user_server_overridden(self): }, } overridden = [ - "no snmp-server user flow mfamily v3 access 27", + "snmp-server user replaceUser replaceUser v3 auth md5 replaceUser access 22", "snmp-server user flow mfamily v3 access 27", "no snmp-server user newuser newfamily v1 access 24", ] + playbook["state"] = "overridden" set_module_args(playbook) result = self.execute_module(changed=True) From d4b3c0189779c0d03f82ebcf6cafdba997d72e4f Mon Sep 17 00:00:00 2001 From: Sagar Paul Date: Wed, 31 Jan 2024 19:09:11 +0530 Subject: [PATCH 3/3] Update test-requirements.txt (#1025) --- test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-requirements.txt b/test-requirements.txt index 108ac0d0e..5a90586f2 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,5 +1,5 @@ # For ansible-tox-linters -black==23.3.0 ; python_version >= '3.7' +black==23.3.0 flake8 yamllint