nxos_nxapi fix pick-2.5 (#42490)

* Enforcing NXAPI default HTTP behavior (#41817)

* nxos_nxapi http default behavior

* Use nxos_nxapi module in prepare_nxos_tests

* Refactor nxos_nxapi configure test to use yaml block

* Extend nxos_nxapi https & http test cases

* Removed NXOS internal release naming

* Resolved ansibot sanity errors

* Fix typo in prepare_nxos_tests

* Address PR comments

* Shippable indicates this is no longer needed

* Add port change logic and testing

(cherry picked from commit db7300904d)

* add changelog

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add get_capabilities in nxapi module_utils (#42688)

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
(cherry picked from commit d5e9653c96)

changelogs/fragments/nxos_module_utils_nxapi_get_capabilities.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- get_capabilities in nxapi module_utils should not return empty dictionary (https://github.com/ansible/ansible/pull/42688).

changelogs/fragments/nxos_nxapi_fix_2.5.7.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- Enforcing NXAPI default HTTP behavior (https://github.com/ansible/ansible/pull/41817).

lib/ansible/module_utils/network/nxos/nxos.py

@@ -418,8 +418,34 @@ class Nxapi:
         else:
             return []
 
+    def get_device_info(self):
+        device_info = {}
+
+        device_info['network_os'] = 'nxos'
+        reply = self.run_commands({'command': 'show version', 'output': 'json'})
+        data = reply[0]
+
+        platform_reply = self.run_commands({'command': 'show inventory', 'output': 'json'})
+        platform_info = platform_reply[0]
+
+        device_info['network_os_version'] = data.get('sys_ver_str') or data.get('kickstart_ver_str')
+        device_info['network_os_model'] = data['chassis_id']
+        device_info['network_os_hostname'] = data['host_name']
+        device_info['network_os_image'] = data.get('isan_file_name') or data.get('kick_file_name')
+
+        if platform_info:
+            inventory_table = platform_info['TABLE_inv']['ROW_inv']
+            for info in inventory_table:
+                if 'Chassis' in info['name']:
+                    device_info['network_os_platform'] = info['productid']
+
+        return device_info
+
     def get_capabilities(self):
-        return {}
+        result = {}
+        result['device_info'] = self.get_device_info()
+        result['network_api'] = 'nxapi'
+        return result
 
 
 def is_json(cmd):

lib/ansible/modules/network/nxos/nxos_config.py

@@ -403,7 +403,7 @@ def main():
 
     try:
         info = get_capabilities(module)
-        api = info.get('network_api', 'nxapi')
+        api = info.get('network_api')
         device_info = info.get('device_info', {})
         os_platform = device_info.get('network_os_platform', '')
     except ConnectionError:

lib/ansible/modules/network/nxos/nxos_nxapi.py

@@ -162,7 +162,8 @@ def check_args(module, warnings):
 
 
 def map_obj_to_commands(want, have, module):
-    commands = list()
+    send_commands = list()
+    commands = dict()
 
     def needs_update(x):
         return want.get(x) is not None and (want.get(x) != have.get(x))
@@ -170,29 +171,30 @@ def map_obj_to_commands(want, have, module):
     if needs_update('state'):
         if want['state'] == 'absent':
             return ['no feature nxapi']
-        commands.append('feature nxapi')
-
-    if needs_update('http') or (have.get('http') and needs_update('http_port')):
-        if want['http'] is True or (want['http'] is None and have['http'] is True):
-            port = want['http_port'] or 80
-            commands.append('nxapi http port %s' % port)
-        elif want['http'] is False:
-            commands.append('no nxapi http')
-
-    if needs_update('https') or (have.get('https') and needs_update('https_port')):
-        if want['https'] is True or (want['https'] is None and have['https'] is True):
-            port = want['https_port'] or 443
-            commands.append('nxapi https port %s' % port)
-        elif want['https'] is False:
-            commands.append('no nxapi https')
+        send_commands.append('feature nxapi')
+    elif want['state'] == 'absent':
+        return send_commands
+
+    for parameter in ['http', 'https']:
+        port_param = parameter + '_port'
+        if needs_update(parameter):
+            if want.get(parameter) is False:
+                commands[parameter] = 'no nxapi %s' % parameter
+            else:
+                commands[parameter] = 'nxapi %s port %s' % (parameter, want.get(port_param))
+
+        if needs_update(port_param) and want.get(parameter) is True:
+            commands[parameter] = 'nxapi %s port %s' % (parameter, want.get(port_param))
 
     if needs_update('sandbox'):
-        cmd = 'nxapi sandbox'
+        commands['sandbox'] = 'nxapi sandbox'
         if not want['sandbox']:
-            cmd = 'no %s' % cmd
-        commands.append(cmd)
+            commands['sandbox'] = 'no %s' % commands['sandbox']
 
-    return commands
+    for parameter in commands.keys():
+        send_commands.append(commands[parameter])
+
+    return send_commands
 
 
 def parse_http(data):
@@ -265,10 +267,10 @@ def main():
     """ main entry point for module execution
     """
     argument_spec = dict(
-        http=dict(aliases=['enable_http'], type='bool'),
-        http_port=dict(type='int'),
-        https=dict(aliases=['enable_https'], type='bool'),
-        https_port=dict(type='int'),
+        http=dict(aliases=['enable_http'], type='bool', default=True),
+        http_port=dict(type='int', default=80),
+        https=dict(aliases=['enable_https'], type='bool', default=False),
+        https_port=dict(type='int', default=443),
         sandbox=dict(aliases=['enable_sandbox'], type='bool'),
         state=dict(default='present', choices=['started', 'stopped', 'present', 'absent'])
     )
@@ -279,6 +281,11 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
+    warning_msg = "Module nxos_nxapi currently defaults to configure 'http port 80'. "
+    warning_msg += "Default behavior is changing to configure 'https port 443'"
+    warning_msg += " when params 'http, http_port, https, https_port' are not set in the playbook"
+    module.deprecate(msg=warning_msg, version="2.11")
+
     check_args(module, warnings)
 
     result = {'changed': False, 'warnings': warnings}

test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes.yaml

@@ -1,7 +0,0 @@
----
-- name: Assert configuration changes
-  assert:
-    that:
-      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port
-      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port|string is search("9443")
-      - result.stdout[0]['operation_status'].o_status == 'nxapi enabled'

test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes_http.yaml

@@ -0,0 +1,16 @@
+---
+- name: Assert HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port|string is search("80")
+      - result.stdout[0]['operation_status'].o_status == 'nxapi enabled'
+  when: major_version is version('9.2', '<')
+
+- name: Assert HTTP configuration changes 9.2 or greater
+  assert:
+    that:
+      - result.stdout[0]['http_port']
+      - result.stdout[0]['http_port']|string is search("80")
+      - result.stdout[0]['nxapi_status'] == 'nxapi enabled'
+  when: major_version is version('9.2', '>=')

test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes_https.yaml

@@ -0,0 +1,16 @@
+---
+- name: Assert HTTPS configuration changes
+  assert:
+    that:
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'].l_port|string is search("9443")
+      - result.stdout[0]['operation_status'].o_status == 'nxapi enabled'
+  when: major_version is version('9.2', '<')
+
+- name: Assert HTTPS configuration changes 9.2 or greater
+  assert:
+    that:
+      - result.stdout[0]['https_port']
+      - result.stdout[0]['https_port']|string is search("9443")
+      - result.stdout[0]['nxapi_status'] == 'nxapi enabled'
+  when: major_version is version('9.2', '>=')

test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes_https_http.yaml

@@ -0,0 +1,20 @@
+---
+- name: Assert HTTPS & HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][1].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][1].l_port|string is search("9443")
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][0].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][0].l_port|string is search("80")
+      - result.stdout[0]['operation_status'].o_status == 'nxapi enabled'
+  when: major_version is version('9.2', '<')
+
+- name: Assert HTTPS & HTTP configuration changes 9.2 or greater
+  assert:
+    that:
+      - result.stdout[0]['https_port']
+      - result.stdout[0]['https_port']|string is search("9443")
+      - result.stdout[0]['http_port']
+      - result.stdout[0]['http_port']|string is search("80")
+      - result.stdout[0]['nxapi_status'] == 'nxapi enabled'
+  when: major_version is version('9.2', '>=')

test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes_https_http_ports.yaml

@@ -0,0 +1,20 @@
+---
+- name: Assert HTTPS & HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][1].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][1].l_port|string is search("500")
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][0].l_port
+      - result.stdout[0]['TABLE_listen_on_port']['ROW_listen_on_port'][0].l_port|string is search("99")
+      - result.stdout[0]['operation_status'].o_status == 'nxapi enabled'
+  when: major_version is version('9.2', '<')
+
+- name: Assert HTTPS & HTTP configuration changes 9.2 or greater
+  assert:
+    that:
+      - result.stdout[0]['https_port']
+      - result.stdout[0]['https_port']|string is search("500")
+      - result.stdout[0]['http_port']
+      - result.stdout[0]['http_port']|string is search("99")
+      - result.stdout[0]['nxapi_status'] == 'nxapi enabled'
+  when: major_version is version('9.2', '>=')

test/integration/targets/nxos_nxapi/tasks/platform/n5k/assert_changes_http.yaml

@@ -0,0 +1,6 @@
+---
+- name: Assert HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is not defined
+      - result.stdout[0].http_port|string is search("80")

test/integration/targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https.yaml

@@ -1,5 +1,5 @@
 ---
-- name: Assert configuration changes
+- name: Assert HTTPS configuration changes
   assert:
     that:
       - result.stdout[0].http_port is not defined

test/integration/targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https_http.yaml

@@ -0,0 +1,8 @@
+---
+- name: Assert HTTPS && HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is defined
+      - result.stdout[0].http_port is defined
+      - result.stdout[0].https_port|string is search("9443")
+      - result.stdout[0].http_port|string is search("80")

test/integration/targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https_http_ports.yaml

@@ -0,0 +1,8 @@
+---
+- name: Assert HTTPS && HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is defined
+      - result.stdout[0].http_port is defined
+      - result.stdout[0].https_port|string is search("500")
+      - result.stdout[0].http_port|string is search("99")

test/integration/targets/nxos_nxapi/tasks/platform/n7k/assert_changes_http.yaml

@@ -0,0 +1,7 @@
+---
+- name: Assert HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is not defined
+      - result.stdout[0].http_port|string is search("80")
+      - result.stdout[0].sandbox_status == 'Enabled'

test/integration/targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https.yaml

@@ -1,5 +1,5 @@
 ---
-- name: Assert configuration changes
+- name: Assert HTTPS configuration changes
   assert:
     that:
       - result.stdout[0].http_port is not defined

test/integration/targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https_http.yaml

@@ -0,0 +1,9 @@
+---
+- name: Assert HTTPS & HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is defined
+      - result.stdout[0].http_port is defined
+      - result.stdout[0].https_port|string is search("9443")
+      - result.stdout[0].http_port|string is search("80")
+      - result.stdout[0].sandbox_status == 'Enabled'

test/integration/targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https_http_ports.yaml

@@ -0,0 +1,9 @@
+---
+- name: Assert HTTPS & HTTP configuration changes
+  assert:
+    that:
+      - result.stdout[0].https_port is defined
+      - result.stdout[0].http_port is defined
+      - result.stdout[0].https_port|string is search("500")
+      - result.stdout[0].http_port|string is search("99")
+      - result.stdout[0].sandbox_status == 'Enabled'

test/integration/targets/nxos_nxapi/tests/cli/configure.yaml

@@ -9,42 +9,145 @@
     state: absent
     provider: "{{ cli }}"
 
-- name: Configure NXAPI
-  nxos_nxapi:
-    enable_http: no
-    enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
-    enable_https: yes
-    https_port: 9443
-    provider: "{{ cli }}"
-  register: result
+- block:
+  - name: Configure NXAPI HTTPS
+    nxos_nxapi: &configure_https
+      enable_http: no
+      enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
+      enable_https: yes
+      https_port: 9443
+      provider: "{{ cli }}"
+    register: result
 
-- nxos_command:
-    commands:
-      - show nxapi | json
-    provider: "{{ cli }}"
-  register: result
+  - nxos_command:
+      commands:
+        - show nxapi | json
+      provider: "{{ cli }}"
+    register: result
 
-- include: targets/nxos_nxapi/tasks/platform/n7k/assert_changes.yaml
-  when: platform is match('N7K')
+  - include: targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https.yaml
+    when: platform is match('N7K')
 
-- include: targets/nxos_nxapi/tasks/platform/n5k/assert_changes.yaml
-  when: platform is match('N5K')
+  - include: targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https.yaml
+    when: platform is match('N5K')
 
-- include: targets/nxos_nxapi/tasks/platform/default/assert_changes.yaml
-  when: not ( platform is search('N7K')) and not (platform is search('N5K')) and not (platform is search('N35'))
+  - include: targets/nxos_nxapi/tasks/platform/default/assert_changes_https.yaml
+    when: not ( platform is search('N7K')) and not (platform is search('N5K')) and not (platform is search('N35'))
 
-- name: Configure NXAPI again
-  nxos_nxapi:
-    enable_http: no
-    enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
-    enable_https: yes
-    https_port: 9443
-    provider: "{{ cli }}"
-  register: result
+  - name: Configure NXAPI HTTPS again
+    nxos_nxapi: *configure_https
+    register: result
+
+  - name: Assert configuration is idempotent
+    assert: &assert_false
+      that:
+        - result.changed == false
+
+
+  - name: Configure NXAPI HTTPS & HTTP
+    nxos_nxapi: &configure_https_http
+      enable_http: yes
+      enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
+      enable_https: yes
+      https_port: 9443
+      provider: "{{ cli }}"
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+      provider: "{{ cli }}"
+    register: result
+
+  - include: targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https_http.yaml
+    when: platform is match('N7K')
+
+  - include: targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https_http.yaml
+    when: platform is match('N5K')
+
+  - include: targets/nxos_nxapi/tasks/platform/default/assert_changes_https_http.yaml
+    when: not ( platform is search('N7K')) and not (platform is search('N5K')) and not (platform is search('N35'))
+
+  - name: Configure NXAPI HTTPS & HTTP again
+    nxos_nxapi: *configure_https_http
+    register: result
+
+  - name: Assert configuration is idempotent
+    assert: *assert_false
+
+  - name: Configure different NXAPI HTTPS & HTTP ports
+    nxos_nxapi: &configure_https_http_ports
+      enable_http: yes
+      enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
+      enable_https: yes
+      http_port: 99
+      https_port: 500
+      provider: "{{ cli }}"
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+      provider: "{{ cli }}"
+    register: result
+
+  - include: targets/nxos_nxapi/tasks/platform/n7k/assert_changes_https_http_ports.yaml
+    when: platform is match('N7K')
+
+  - include: targets/nxos_nxapi/tasks/platform/n5k/assert_changes_https_http_ports.yaml
+    when: platform is match('N5K')
+
+  - include: targets/nxos_nxapi/tasks/platform/default/assert_changes_https_http_ports.yaml
+    when: not ( platform is search('N7K')) and not (platform is search('N5K')) and not (platform is search('N35'))
+
+  - name: Configure different NXAPI HTTPS & HTTP ports again
+    nxos_nxapi: *configure_https_http_ports
+    register: result
+
+  - name: Assert configuration is idempotent
+    assert: *assert_false
+
+  - name: Configure NXAPI HTTP
+    nxos_nxapi: &configure_http
+      enable_http: yes
+      enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
+      enable_https: no
+      provider: "{{ cli }}"
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+      provider: "{{ cli }}"
+    register: result
+
+  - include: targets/nxos_nxapi/tasks/platform/n7k/assert_changes_http.yaml
+    when: platform is match('N7K')
+
+  - include: targets/nxos_nxapi/tasks/platform/n5k/assert_changes_http.yaml
+    when: platform is match('N5K')
+
+  - include: targets/nxos_nxapi/tasks/platform/default/assert_changes_http.yaml
+    when: not ( platform is search('N7K')) and not (platform is search('N5K')) and not (platform is search('N35'))
+
+  - name: Configure NXAPI HTTP again
+    nxos_nxapi: *configure_http
+    register: result
+
+  - name: Assert configuration is idempotent
+    assert: *assert_false
+
+  always:
+  - name: Cleanup - Disable NXAPI
+    nxos_nxapi:
+      state: absent
+      provider: "{{ cli }}"
+    register: result
 
-- name: Assert configuration is idempotent
-  assert:
-    that:
-      - result.changed == false
+  - name: Cleanup - Re-enable NXAPI
+    nxos_nxapi:
+      state: present
+      provider: "{{ cli }}"
+    register: result
 
-- debug: msg="END cli/configure.yaml"
+  - debug: msg="END cli/configure.yaml"

test/integration/targets/prepare_nxos_tests/tasks/main.yml

@@ -1,14 +1,14 @@
 ---
-- name: Toggle feature nxapi - Enable
+- name: Enable Feature Privilage
   nxos_config:
     lines:
-      - feature nxapi
       - feature privilege
   connection: network_cli
   ignore_errors: yes
 
-- name: Set nxapi to default state
+- name: Enable Feature NXAPI
   nxos_nxapi:
+    state: present
   connection: network_cli
 
 # Gather the list of interfaces on this device and make the list
@@ -93,6 +93,7 @@
 # 8.0(1)
 # 7.3(0)D1(1)
 # 7.0(3)IHD8(1)
+- set_fact: major_version="{{ image_version[0:3] }}"
 - set_fact: imagetag="{{ image_version[0:3] }}"
   when: image_version is search("\d\.\d\(\d\)")
 - set_fact: imagetag="{{ image_version[6:8] }}"

test/sanity/validate-modules/ignore.txt

@@ -1733,7 +1733,6 @@ lib/ansible/modules/network/nxos/nxos_lldp.py E326
 lib/ansible/modules/network/nxos/nxos_ntp_auth.py E324
 lib/ansible/modules/network/nxos/nxos_ntp_options.py E325
 lib/ansible/modules/network/nxos/nxos_ntp_options.py E326
-lib/ansible/modules/network/nxos/nxos_nxapi.py E324
 lib/ansible/modules/network/nxos/nxos_nxapi.py E325
 lib/ansible/modules/network/nxos/nxos_nxapi.py E326
 lib/ansible/modules/network/nxos/nxos_ospf_vrf.py E325