search

ansible-collections / cisco.ios

Ansible Network Collection for Cisco IOS
GNU General Public License v3.0
262 stars 161 forks source link

Fix AttributeError on dictionary check #516

Closed vrelk closed 2 years ago

vrelk commented 2 years ago

Check if proto_options is actually a dictionary instead of simply checking if it exists.

SUMMARY

This fixes the error AttributeError: 'bool' object has no attribute 'keys', which is a result of simply checking if proto_options exists, rather than making sure it is actually a dictionary.

Adding protocol_options to every entry isn't an option as that results in Unsupported attribute for standard ACL - protocol_options.

ISSUE TYPE
COMPONENT NAME

ios_acls

ADDITIONAL INFORMATION

An example task that results in the error would be

- name: CISCO | IOS | Deploying ACL template
  cisco.ios.ios_acls:
    state: overridden
    config:
      - afi: ipv4
        acls:
          - name: TEST_ACL
            acl_type: extended
            aces:
              - sequence: 10
                grant: permit
                protocol_options:
                  udp: 'yes'
                source:
                  any: 'yes'
                  port_protocol:
                    eq: 68
                destination:
                  any: 'yes'
                  port_protocol:
                    eq: 69
softwarefactory-project-zuul[bot] commented 2 years ago

Build succeeded.

KB-perByte commented 2 years ago

Hey @vrelk you might want to add a changelog and a test case to check the change a unit test case should be fine, and if it fixes any open issues then you can tag it with the PR. Regards

softwarefactory-project-zuul[bot] commented 2 years ago

Build succeeded.

softwarefactory-project-zuul[bot] commented 2 years ago

Build succeeded.

softwarefactory-project-zuul[bot] commented 2 years ago

Unable to freeze job graph: Job network-ee-integration-tests-ios depends on network-ee-build-container-image which was not run.

goneri commented 2 years ago

recheck

vrelk commented 2 years ago

Yeah, I'm not sure what changed. Everything passed on be3ad82 except a missing blank line at the end of the changelog, but after fixing that, all hell broke loose.

softwarefactory-project-zuul[bot] commented 2 years ago

Build failed.

KB-perByte commented 2 years ago

Hey @vrelk there are some significant optimizations around ACLs module, you might need to fetch the changes as per this https://github.com/ansible-collections/cisco.ios/pull/540 PR before we merge this one. Regards

KB-perByte commented 2 years ago

and about the CI failures, there are some changes on our CI pipeline, I am tracking the PRs to be merged.

softwarefactory-project-zuul[bot] commented 2 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

softwarefactory-project-zuul[bot] commented 2 years ago

Build succeeded.

vrelk commented 2 years ago

Err, yeah. Don't think I did that right.... 146 files changed, but at least it passed. I'm just gonna start over. I will reference this pull request in the new one for reference, and put that one here once I have it.