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

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

@@ -8,39 +8,135 @@
   nxos_nxapi:
     state: absent
 
-- name: Configure NXAPI
-  nxos_nxapi:
-    enable_http: no
-    enable_sandbox: "{{nxapi_sandbox_option|default(omit)}}"
-    enable_https: yes
-    https_port: 9443
-  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
+    register: result
 
-- nxos_command:
-    commands:
-      - show nxapi | json
-  register: result
+  - nxos_command:
+      commands:
+        - show nxapi | json
+    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
-  register: result
-
-- name: Assert configuration is idempotent
-  assert:
-    that:
-      - result.changed == false
-
-- debug: msg="END cli/configure.yaml"
+  - 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
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+    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
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+    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
+    register: result
+
+  - nxos_command:
+      commands:
+        - show nxapi | json
+    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
+    register: result
+
+  - name: Cleanup - Re-enable NXAPI
+    nxos_nxapi:
+      state: present
+    register: result
+
+  - 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

@@ -944,7 +944,6 @@ lib/ansible/modules/network/nxos/nxos_gir.py E326
 lib/ansible/modules/network/nxos/nxos_igmp_interface.py E326
 lib/ansible/modules/network/nxos/nxos_interface.py E324
 lib/ansible/modules/network/nxos/nxos_lldp.py E326
-lib/ansible/modules/network/nxos/nxos_nxapi.py E324
 lib/ansible/modules/network/nxos/nxos_nxapi.py E326
 lib/ansible/modules/network/nxos/nxos_pim_interface.py E326
 lib/ansible/modules/network/nxos/nxos_pim_rp_address.py E326