Repository: ansible/ansible
Pull request: fix for nxos_acl_interface idempotency
##### SUMMARY
Fixes https://github.com/ansible/ansible/issues/26155

- The nxos_acl_interface component is using "show ip access-list summary" for checking if the acl_interface is existing already. This is not correct. 

- Whenever an ip access-group is configured, it will be listed in "show running-config aclmgr" under the interface on which it is configured like:
```
interface Ethernet1/2
  ip access-group ANSIBLE_ACL in
  ip access-group ANSIBLE_ACL out
```
- The change proposed always looks under "show running-config aclmgr" and checks if the access-group is already configured and if it, exits without any changes which resolves the idempotent issue.

- Since the change involves checking a different command, the uni test filename and output are also changed.

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
nxos_acl_interface

##### ANSIBLE VERSION
```ansible 2.4.0 (devel 1e8c58519e) last updated 2017/06/26 16:21:06 (GMT -400)                   
  config file = None                                                                           
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /root/agents-ci/ansible/lib/ansible                                      
  executable location = /root/agents-ci/ansible/bin/ansible                                                 
  python version = 2.7.6 (default, Oct 26 2016, 20:30:19) [GCC 4.8.4]

```
##### ADDITIONAL INFORMATION
- Unit tests results:
platform linux2 -- Python 2.7.6, pytest-3.1.2, py-1.4.34, pluggy-0.4.0
rootdir: /root/p2/Projects/nxos_ansible/fix_ansible, inifile:
collected 2 items

test_nxos_acl_interface.py ..

- Integration tests run against N3k, N7k, N9k Targets/Releases using the latest integration tests for nxos_acl_interface which are referenced at: https://github.com/ansible/ansible/pull/26142

```
```


Problem statement:
nxos_acl_interface not idempotent
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
nxos_acl_interface

##### ANSIBLE VERSION
```ansible 2.4.0 (devel 1e8c58519e) last updated 2017/06/26 16:21:06 (GMT -400)
  config file = None
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /root/agents-ci/ansible/lib/ansible
  executable location = /root/agents-ci/ansible/bin/ansible
  python version = 2.7.6 (default, Oct 26 2016, 20:30:19) [GCC 4.8.4]
```

##### OS / ENVIRONMENT
Ansible Server : Ubuntu 14.04
Device: Nexus9000 C9396PX running 7.0(3)I2(2e)

##### SUMMARY
When we run nxos_acl_interface playbook, it is not idempotent. The same playbook applied twice shows the result as 'changed'.

##### STEPS TO REPRODUCE
Playbook:
- hosts: nxos
  connection: local
  gather_facts: no

  tasks:
  - name: Configure acl interface
    nxos_acl_interface:
      name: ANSIBLE_ACL
      interface: ethernet1/2
      direction: ingress
      state: present
      provider: "{{ cli }}"

##### EXPECTED RESULTS
After running second time, the recap should be:
```
PLAY RECAP **********************************************************************************************************************************************
n9k.example.com          : ok=1    changed=0    unreachable=0    failed=0
```

##### ACTUAL RESULTS
```
PLAY RECAP **********************************************************************************************************************************************
n9k.example.com          : ok=1    changed=1    unreachable=0    failed=0
```

Hints from issue discussion:
cc @jedelman8 @GGabriele @rahushen @privateip @rcarrillocruz @mikewiebe @trishnaguha
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: Review comment fixes

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_acl_interface.py b/lib/ansible/modules/network/nxos/nxos_acl_interface.py
index b47d796b877dd2..bea4e9d2905cd5 100644
--- a/lib/ansible/modules/network/nxos/nxos_acl_interface.py
+++ b/lib/ansible/modules/network/nxos/nxos_acl_interface.py
@@ -85,83 +85,19 @@
 from ansible.module_utils.basic import AnsibleModule
 
 
-NAME = r'.*IP?\s+access list\s+(?P<name>\S+).*'
-INTERFACE = r'.*\s+(?P<interface>\w+(\d+)?\/?(\d+)?)\s-\s(?P<direction>\w+)\s+\W(?P<acl_type>\w+\s\w+)\W.*'
-
+def check_for_acl_int_present(module, name, intf, direction):
+    # Need to Captitalize the interface name as the nxos
+    # output has capitalization
+    command = ['show running-config aclmgr | section {0}'.format(intf.title())]
+    body = run_commands(module, command)
 
-def get_acl_interface(module, acl):
-    command = ['show ip access-list summary']
-    acl_list = []
+    if direction == 'ingress':
+        mdir = 'in'
+    elif direction == 'egress':
+        mdir = 'out'
 
-    body = run_commands(module, command)
-    body_split = body[0].split('Active on interfaces:')
-
-    for each_acl in body_split:
-        temp = {}
-        try:
-            match_name = re.match(NAME, each_acl, re.DOTALL)
-            name_dict = match_name.groupdict()
-            name = name_dict['name']
-        except AttributeError:
-            name = ''
-
-        temp['interfaces'] = []
-        for line in each_acl.split('\n'):
-            try:
-                match_interface = re.match(INTERFACE, line, re.DOTALL)
-                interface_dict = match_interface.groupdict()
-                interface = interface_dict['interface']
-                direction = interface_dict['direction']
-                acl_type = interface_dict['acl_type']
-            except AttributeError:
-                interface = ''
-                direction = ''
-                acl_type = ''
-
-            intf_temp = {}
-            if interface:
-                intf_temp['interface'] = interface
-            if acl_type:
-                intf_temp['acl_type'] = acl_type
-            if direction:
-                intf_temp['direction'] = direction
-            if intf_temp:
-                temp['interfaces'].append(intf_temp)
-        if name:
-            temp['name'] = name
-
-        if temp:
-            acl_list.append(temp)
-
-    existing_no_null = []
-    for each in acl_list:
-        if each.get('name') == acl:
-            interfaces = each.get('interfaces')
-            for interface in interfaces:
-                new_temp = {}
-                new_temp['name'] = acl
-                new_temp.update(interface)
-                existing_no_null.append(new_temp)
-    return existing_no_null
-
-
-def other_existing_acl(get_existing, interface, direction):
-    # now we'll just get the interface in question
-    # needs to be a list since same acl could be applied in both dirs
-    acls_interface = []
-    this = {}
-
-    if get_existing:
-        for each in get_existing:
-            if each.get('interface').lower() == interface:
-                acls_interface.append(each)
-
-        if acls_interface:
-            for each in acls_interface:
-                if each.get('direction') == direction:
-                    this = each
-
-    return acls_interface, this
+    match = re.search('ip access-group {0} {1}'.format(name, mdir), str(body[0]))
+    return True if match else False
 
 
 def apply_acl(proposed):
@@ -227,14 +163,7 @@ def main():
 
     proposed = dict(name=name, interface=interface, direction=direction)
 
-    # includes all interfaces the ACL is applied to (list)
-    get_existing = get_acl_interface(module, name)
-
-    # interface_acls = includes entries of this ACL on the interface (list)
-    # this_dir_acl_intf = dict - not null if it already exists
-    interfaces_acls, existing = other_existing_acl(get_existing, interface, direction)
-
-    end_state_acls = get_existing
+    existing = check_for_acl_int_present(module, name, interface, direction)
 
     cmds = []
     commands = []
@@ -258,15 +187,12 @@ def main():
             else:
                 load_config(module, cmds)
                 results['changed'] = True
-                end_state_acls = get_acl_interface(module, name)
-                interfaces_acls, this_dir_acl_intf = other_existing_acl(end_state_acls, interface, direction)
                 if 'configure' in cmds:
                     cmds.pop(0)
     else:
         cmds = []
 
     results['commands'] = cmds
-    results['acl_applied_to'] = end_state_acls
 
     module.exit_json(**results)
 
diff --git a/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_ip_access-list_summary.txt b/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_ip_access-list_summary.txt
deleted file mode 100644
index 7c8b16a8b2d14b..00000000000000
--- a/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_ip_access-list_summary.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-IP access list __urpf_v4_acl__
-        Total ACEs Configured: 1
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-bgp
-        Total ACEs Configured: 2
-        Configured on interfaces:
-                ethernet1/41 - egress (Router ACL)
-        Active on interfaces:
-                ethernet1/41 - egress (Router ACL)
-IP access list copp-system-p-acl-cts
-        Total ACEs Configured: 2
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-dhcp
-        Total ACEs Configured: 2
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-dhcp-relay-response
-        Total ACEs Configured: 2
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-eigrp
-        Total ACEs Configured: 1
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-ftp
-        Total ACEs Configured: 4
-        Configured on interfaces:
-        Active on interfaces:
-IP access list copp-system-p-acl-glbp
-        Total ACEs Configured: 1
-        Configured on interfaces:
-        Active on interfaces:
diff --git a/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_running-config_aclmgr.txt b/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_running-config_aclmgr.txt
new file mode 100644
index 00000000000000..097b89a4582077
--- /dev/null
+++ b/test/units/modules/network/nxos/fixtures/nxos_acl_interface/show_running-config_aclmgr.txt
@@ -0,0 +1,4 @@
+ip access-list ANSIBLE_OUT
+  10 permit tcp 1.1.1.1/24 any
+interface Ethernet1/41
+  ip access-group copp-system-p-acl-bgp out


Code review comments:
diff_hunk: @@ -86,29 +86,18 @@
 
 
 def check_for_acl_int_present(module, name, intf, direction):
-    command = ['show running-config aclmgr']
+    # Need to Captitalize the interface name as the nxos
+    # output has capitalization
+    command = ['show running-config aclmgr | section {0}'.format(intf.title())]
     body = run_commands(module, command)
-    body = str(body[0])
-
-    seq = re.compile(r"^interface " + re.escape(intf) + r"\n  ip access-group (.*) (.*)", re.M | re.I)
-    existing = False
-    while True:
-        match = re.search(seq, body)
-        if match:
-            lname = match.group(1)
-            ldir = match.group(2)
-            if ldir == 'in':
-                mdir = 'ingress'
-            else:
-                mdir = 'egress'
-            if (match.group(1) == name and mdir == direction):
-                existing = True
-                break
-            else:
-                body = re.sub('  ip access-group ' + lname + ' ' + ldir + '\n?', '', body)
-        else:
-            break
-    return existing
+
+    if direction == 'ingress':
+        mdir = 'in'
+    elif direction == 'egress':
+        mdir = 'out'
+
+    match = re.search('ip access-group {0} {1}'.format(name, mdir), str(body[0]))
+    return True if match else False
path: lib/ansible/modules/network/nxos/nxos_acl_interface.py
text: @user1:
I would `return match is not None` or `return bool(match)` rather than this