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

ASA network/service object-group module #52925

Open
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@FedericoOlivieri
Copy link
Contributor

FedericoOlivieri commented Feb 25, 2019

SUMMARY

ASA module for maintaining network and service object groups.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

asa_og

ADDITIONAL INFORMATION

Given a list of entries, the module update the object-group indicated in 'name' field with the lines in the variable. The lines into the Ansible variable always reflect what is in the running config.

vars:
  config_list:
    - range 56832 56959
    - range 61363 65185

- name: configure UDP object-group service with port-object
  asa_og:
    name: service_object_test
    group_type: port-object
    protocol: udp
    lines: "{{ item }}"
    provider: "{{ fws }}"
  register: result
  loop:
    - "{{ config_list }}"

  commands: [
    "object-group service service_object_test udp",
    " port-object range 56832 56959",
    " port-object range 61363 65185"
    ]

federico.olivieri added some commits Feb 25, 2019

federico.olivieri
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 25, 2019

@FedericoOlivieri, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 25, 2019

The test ansible-test sanity --test pylint [explain] failed with 25 errors:

lib/ansible/modules/network/asa/asa_og.py:137:50: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:143:11: singleton-comparison Comparison to True should be just 'expr' or 'expr is True'
lib/ansible/modules/network/asa/asa_og.py:147:48: singleton-comparison Comparison to False should be 'not expr' or 'expr is False'
lib/ansible/modules/network/asa/asa_og.py:152:54: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:158:49: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:193:15: undefined-variable Undefined variable 'cmp'
lib/ansible/modules/network/asa/asa_og.py:197:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:201:75: bad-whitespace No space allowed before bracket                                     add_lines.append('network-object ' + i )                                                                            ^
lib/ansible/modules/network/asa/asa_og.py:208:81: bad-whitespace No space allowed before bracket                                     remove_lines.append('no network-object ' + i )                                                                                  ^
lib/ansible/modules/network/asa/asa_og.py:210:66: bad-whitespace No space allowed before bracket                                     remove_lines.append('no ' + i )                                                                   ^
lib/ansible/modules/network/asa/asa_og.py:213:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:217:75: bad-whitespace No space allowed before bracket                                     add_lines.append('service-object ' + i )                                                                            ^
lib/ansible/modules/network/asa/asa_og.py:224:81: bad-whitespace No space allowed before bracket                                     remove_lines.append('no service-object ' + i )                                                                                  ^
lib/ansible/modules/network/asa/asa_og.py:226:66: bad-whitespace No space allowed before bracket                                     remove_lines.append('no ' + i )                                                                   ^
lib/ansible/modules/network/asa/asa_og.py:230:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:234:72: bad-whitespace No space allowed before bracket                                     add_lines.append('port-object ' + i )                                                                         ^
lib/ansible/modules/network/asa/asa_og.py:241:78: bad-whitespace No space allowed before bracket                                     remove_lines.append('no port-object ' + i )                                                                               ^
lib/ansible/modules/network/asa/asa_og.py:254:46: bad-whitespace No space allowed before :             if 'network-object' in group_type :                                               ^
lib/ansible/modules/network/asa/asa_og.py:255:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:259:63: bad-whitespace No space allowed before bracket                         add_lines.append('network-object ' + i )                                                                ^
lib/ansible/modules/network/asa/asa_og.py:267:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:270:59: bad-whitespace No space allowed before bracket                     add_lines.append('service-object ' + i )                                                            ^
lib/ansible/modules/network/asa/asa_og.py:276:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:278:56: bad-whitespace No space allowed before bracket                     add_lines.append('port-object ' + i )                                                         ^
test/units/modules/network/asa/test_asa_og.py:23:0: relative-beyond-top-level Attempted relative import beyond top-level package

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

lib/ansible/modules/network/asa/asa_og.py:10:0: ImportError: No module named ipaddress

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

lib/ansible/modules/network/asa/asa_og.py:10:0: ImportError: No module named ipaddress

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

lib/ansible/modules/network/asa/asa_og.py:143:30: E712 comparison to True should be 'if cond is True:' or 'if cond:'
lib/ansible/modules/network/asa/asa_og.py:147:67: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
lib/ansible/modules/network/asa/asa_og.py:151:5: E303 too many blank lines (2)
lib/ansible/modules/network/asa/asa_og.py:170:5: E303 too many blank lines (2)
lib/ansible/modules/network/asa/asa_og.py:201:75: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:208:81: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:210:66: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:217:75: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:224:81: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:226:66: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:229:21: E303 too many blank lines (2)
lib/ansible/modules/network/asa/asa_og.py:234:72: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:241:78: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:252:9: E303 too many blank lines (2)
lib/ansible/modules/network/asa/asa_og.py:254:46: E203 whitespace before ':'
lib/ansible/modules/network/asa/asa_og.py:259:63: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:270:59: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:278:56: E202 whitespace before ')'
lib/ansible/modules/network/asa/asa_og.py:331:1: E305 expected 2 blank lines after class or function definition, found 1

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

lib/ansible/modules/network/asa/asa_og.py:8:0: E106 Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.
lib/ansible/modules/network/asa/asa_og.py:8:0: E107 Imports should be directly below DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.
lib/ansible/modules/network/asa/asa_og.py:58:1: E311 EXAMPLES is not valid YAML

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

lib/ansible/modules/network/asa/asa_og.py:58:1: error EXAMPLES: syntax error: expected <block end>, but found '-'

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Feb 25, 2019

federico.olivieri
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 25, 2019

The test ansible-test sanity --test pylint [explain] failed with 12 errors:

lib/ansible/modules/network/asa/asa_og.py:128:50: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:142:54: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:148:49: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:182:15: undefined-variable Undefined variable 'cmp'
lib/ansible/modules/network/asa/asa_og.py:186:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:202:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:218:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:241:46: bad-whitespace No space allowed before :             if 'network-object' in group_type :                                               ^
lib/ansible/modules/network/asa/asa_og.py:242:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:254:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:263:32: ansible-format-automatic-specification Format string contains automatic field numbering specification
test/units/modules/network/asa/test_asa_og.py:23:0: relative-beyond-top-level Attempted relative import beyond top-level package

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

lib/ansible/modules/network/asa/asa_og.py:241:46: E203 whitespace before ':'

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

lib/ansible/modules/network/asa/asa_og.py:8:0: E106 Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.
lib/ansible/modules/network/asa/asa_og.py:8:0: E107 Imports should be directly below DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.

click here for bot help

federico.olivieri
federico.olivieri
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 26, 2019

The test ansible-test sanity --test pylint [explain] failed with 6 errors:

lib/ansible/modules/network/asa/asa_og.py:67:0: misplaced-future __future__ import is not the first non docstring statement
lib/ansible/modules/network/asa/asa_og.py:178:15: undefined-variable Undefined variable 'cmp'
lib/ansible/modules/network/asa/asa_og.py:182:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:198:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
lib/ansible/modules/network/asa/asa_og.py:214:40: ansible-format-automatic-specification Format string contains automatic field numbering specification
test/units/modules/network/asa/test_asa_og.py:23:0: relative-beyond-top-level Attempted relative import beyond top-level package

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

lib/ansible/modules/network/asa/asa_og.py:67:0: SyntaxError: from __future__ imports must occur at the beginning of the file

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

lib/ansible/modules/network/asa/asa_og.py:67:0: SyntaxError: from __future__ imports must occur at the beginning of the file

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

lib/ansible/modules/network/asa/asa_og.py:67:0: SyntaxError: from __future__ imports must occur at the beginning of the file

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

lib/ansible/modules/network/asa/asa_og.py:67:0: SyntaxError: from __future__ imports must occur at the beginning of the file

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

lib/ansible/modules/network/asa/asa_og.py:67:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test import --python 3.8 [explain] failed with 1 error:

lib/ansible/modules/network/asa/asa_og.py:67:1: SyntaxError: from __future__ imports must occur at the beginning of the file

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

lib/ansible/modules/network/asa/asa_og.py:76:1: E302 expected 2 blank lines, found 1

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

lib/ansible/modules/network/asa/asa_og.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'from __future__ imports must occur at the beginning of the file (asa_og.py, line 67)'

click here for bot help

federico.olivieri
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 26, 2019

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

lib/ansible/modules/network/asa/asa_og.py:179:15: undefined-variable Undefined variable 'cmp'

click here for bot help

@FedericoOlivieri

This comment has been minimized.

Copy link
Contributor Author

FedericoOlivieri commented Feb 26, 2019

undefined-variable Undefined variable 'cmp'

cmp is a method not a variable so not sure why is complaining about it

https://www.tutorialspoint.com/python/list_cmp.htm

@NilashishC NilashishC self-requested a review Feb 27, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 27, 2019

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

lib/ansible/modules/network/asa/asa_og.py:179:15: undefined-variable Undefined variable 'cmp'

click here for bot help

@dagwieers dagwieers added the cisco label Feb 27, 2019

federico.olivieri
@NilashishC

This comment has been minimized.

Copy link
Contributor

NilashishC commented Feb 27, 2019

@FedericoOlivieri cmp is deprecated in Python 3. Please use a different approach.

@FedericoOlivieri

This comment has been minimized.

Copy link
Contributor Author

FedericoOlivieri commented Feb 27, 2019

@NilashishC I have replaced cmp() with lines != sorted(have_lines). I have tested the module with the change and is working as expected.

federico.olivieri
@FedericoOlivieri

This comment has been minimized.

Copy link
Contributor Author

FedericoOlivieri commented Feb 27, 2019

@NilashishC can you pleas help to understand why test are failing. I see AssertionError: socket_path must be a value but I am not quite sure what it means. Thank you!

@NilashishC

This comment has been minimized.

Copy link
Contributor

NilashishC commented Feb 27, 2019

@FedericoOlivieri you will need to mock the get_connection(). Try adding the following in setUp:

self.mock_get_connection = patch('ansible.module_utils.network.asa.asa.get_connection')
self.get_connection = self.mock_get_connection.start()

Quick tip: It's probably a good idea to run the unit tests locally before you push. Have a look at this :)

Run the unit tests against all the Python versions that Shippable tests. That way, you can catch issues much earlier, instead of relying on the Shippable runs, which do take a bit of time.

federico.olivieri added some commits Feb 27, 2019

federico.olivieri
@FedericoOlivieri

This comment has been minimized.

Copy link
Contributor Author

FedericoOlivieri commented Feb 28, 2019

@NilashishC thanks for your suggestion. I have problem with dependencies on python 2.6 when I try to run test locally.I need to figure that out first :)

@FedericoOlivieri

This comment has been minimized.

Copy link
Contributor Author

FedericoOlivieri commented Feb 28, 2019

@NilashishC I need for you help once again :)

Pipeline is failing on units test: AssertionError: False != True : {'commands': [], 'changed': False}
for test_asa_og_replace() and test_asa_og_update. Those functions have commands list with items so self.execute_module(changed=True, commands=commands) on both.

However, looking at the error seems that changed=True is not passed properly to asa_module (?)
Please note that I have created asa_module as there were not units test for ASA. I have copied and renamed ios_module.

@NilashishC

This comment has been minimized.

Copy link
Contributor

NilashishC commented Mar 4, 2019

@FedericoOlivieri The issue seems to be with the logic in your code. The asa_module seems to be fine. The unit test case is failing because the module is returning an empty command list and hence, changed False. The test case also seems to fail when running on an actual device. Let me give an example on how to reproduce.

Connect to your device, enter configuration mode and execute the following commands in sequence. This is exactly what you have in your fixtures.

object-group service service_object_test udp
port-object range 56832 56959
port-object range 61363 65185

Now, run the failing test case as a task in a playbook directly on the device.

- asa_og:
    name: 'service_object_test'
    group_type: 'port-object'
    protocol: 'udp'
    lines: 
      - 'range 1024 4201'

The module doesn't seem to generate any commands. Hence, the assertions in your unit test cases are failing.

@ansibot ansibot added the stale_ci label Mar 12, 2019

federico.olivieri
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.