Repository: ansible/ansible
Pull request: fix nxos_interface_ospf issues
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->
Fixes #35987 
<!---
If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bugfix Pull Request

##### COMPONENT NAME
<!--- Name of the module, plugin, module or task -->
nxos_interface_ospf

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0 (devel 788010d0f0) last updated 2018/01/08 11:49:21 (GMT -400)
  config file = /etc/ansible/ansible.cfg
  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
* This PR fixes issues in #35987
* Documentation correction
* Integration test cases are enhanced for nxos_interface_ospf module to include all parameters and combinations.
* Tested on all possible platforms and versions.
<!---
Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful.
  -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```

```


Problem statement:
nxos_interface_ospf issues
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and devel branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
 - Bug Report

##### COMPONENT NAME
<!---
Name of the module, plugin, task or feature
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path
-->
nxos_interface_ospf

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0 (devel 788010d0f0) last updated 2018/01/08 11:49:21 (GMT -400)
  config file = /etc/ansible/ansible.cfg
  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
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
Ansible Server : Ubuntu 14.04
Device: N9K running 7.0(3)I5(2)

##### SUMMARY
<!--- Explain the problem briefly -->

There are few issues with nxos_interface_ospf module.
1. The area parameter can be an ipv4 address or the integer equivalent of the ipv4 address. So the range of values is 0-4294967295. However, the code is assuming 0 to 256 only which is wrong.
2. The code also assumes that the current area configuration on the device is already in ipv4 address format, which is not correct. It can be in integer format if the user has provided it in that way before. Some newer platforms do convert the int to ipv4 address and display that but most of the platforms do not, so this cannot be assumed this way.
3. The default value of hello_interval is 10 and this is not handled in the current code.
4. The integration tests do not cover all the parameters (like message-digest etc.).

Note: I will open a PR shortly to address these above issues

Hints from issue discussion:

Commit message: fix nxos_interface_ospf issues

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_interface_ospf.py b/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
index 8de0fbac67f0d8..952cfc0c363997 100644
--- a/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
+++ b/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
@@ -95,14 +95,14 @@
   message_digest_algorithm_type:
     description:
       - Algorithm used for authentication among neighboring routers
-        within an area. Valid values is 'md5'.
+        within an area. Valid values are 'md5' and 'default'.
     required: false
     choices: ['md5']
     default: null
   message_digest_encryption_type:
     description:
       - Specifies the scheme used for encrypting message_digest_password.
-        Valid values are '3des' or 'cisco_type_7' encryption.
+        Valid values are '3des' or 'cisco_type_7' encryption or 'default'.
     required: false
     choices: ['cisco_type_7','3des']
     default: null
@@ -137,6 +137,8 @@
 
 
 import re
+import struct
+import socket
 from ansible.module_utils.network.nxos.nxos import get_config, load_config
 from ansible.module_utils.network.nxos.nxos import nxos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
@@ -175,6 +177,7 @@ def get_value(arg, config, module):
                 value = value_list[0]
             elif arg == 'area':
                 value = value_list[2]
+                value = normalize_area(value, module)
     elif command == 'ip ospf message-digest-key':
         value = ''
         if has_command_val:
@@ -359,7 +362,7 @@ def state_absent(module, existing, proposed, candidate):
 def normalize_area(area, module):
     try:
         area = int(area)
-        area = '0.0.0.{0}'.format(area)
+        area = socket.inet_ntoa(struct.pack('!L', area))
     except ValueError:
         splitted_area = area.split('.')
         if len(splitted_area) != 4:
@@ -378,8 +381,8 @@ def main():
         passive_interface=dict(required=False, type='bool'),
         message_digest=dict(required=False, type='bool'),
         message_digest_key_id=dict(required=False, type='str'),
-        message_digest_algorithm_type=dict(required=False, type='str', choices=['md5']),
-        message_digest_encryption_type=dict(required=False, type='str', choices=['cisco_type_7', '3des']),
+        message_digest_algorithm_type=dict(required=False, type='str', choices=['md5', 'default']),
+        message_digest_encryption_type=dict(required=False, type='str', choices=['cisco_type_7', '3des', 'default']),
         message_digest_password=dict(required=False, type='str', no_log=True),
         state=dict(choices=['present', 'absent'], default='present', required=False)
     )
@@ -411,7 +414,7 @@ def main():
     for param in ['message_digest_encryption_type',
                   'message_digest_algorithm_type',
                   'message_digest_password']:
-        if module.params[param] == 'default':
+        if module.params[param] == 'default' and module.params['message_digest_key_id'] != 'default':
             module.exit_json(msg='Use message_digest_key_id=default to remove an existing authentication configuration')
 
     state = module.params['state']
@@ -434,6 +437,8 @@ def main():
                 proposed[key] = value
 
     proposed['area'] = normalize_area(proposed['area'], module)
+    if 'hello_interval' in proposed and proposed['hello_interval'] == '10':
+        proposed['hello_interval'] = 'default'
 
     candidate = CustomNetworkConfig(indent=3)
     if state == 'present':
diff --git a/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml b/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
index 3581c8e4d22eff..014683fb343558 100644
--- a/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
+++ b/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
@@ -50,7 +50,7 @@
     nxos_interface_ospf: &configure
       interface: "{{ nxos_int1|upper }}"
       ospf: 1
-      area: 1
+      area: 12345678
       cost: 55
       passive_interface: true
       hello_interval: 15
@@ -75,7 +75,7 @@
     nxos_interface_ospf: &modify
       interface: "{{ testint }}"
       ospf: 1
-      area: 1
+      area: 12345678
       cost: 66
       passive_interface: false
       hello_interval: 17
@@ -92,6 +92,70 @@
 
   - assert: *false
 
+  - name: default properties
+    nxos_interface_ospf: &def
+      interface: "{{ testint }}"
+      ospf: 1
+      area: 12345678
+      cost: default
+      hello_interval: 10
+      dead_interval: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_interface_ospf: *def
+    register: result
+
+  - assert: *false
+
+  - name: Message_digest properties
+    nxos_interface_ospf: &md
+      interface: "{{ testint }}"
+      ospf: 1
+      area: 12345678
+      message_digest: True
+      message_digest_key_id: 10
+      message_digest_algorithm_type: md5
+      message_digest_encryption_type: 3des
+      message_digest_password: b69f7bc54725b1bfd1ea93afa7b09400
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_interface_ospf: *md
+    register: result
+
+  - assert: *false
+
+  - name: Message_digest defaults
+    nxos_interface_ospf: &mdd
+      interface: "{{ testint }}"
+      ospf: 1
+      area: 12345678
+      message_digest: False
+      message_digest_key_id: default
+      message_digest_algorithm_type: default
+      message_digest_encryption_type: default
+      message_digest_password: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_interface_ospf: *mdd
+    register: result
+
+  - assert: *false
+
   - name: create port-channel and loopback interfaces
     nxos_config:
       commands:
@@ -117,7 +181,7 @@
     nxos_interface_ospf: &configurepc
       interface: Port-channel10
       ospf: 1
-      area: 1
+      area: 429496729
       cost: 55
       passive_interface: true
       hello_interval: 15
@@ -138,7 +202,7 @@
     nxos_interface_ospf: &configurepclower
       interface: port-channel11
       ospf: 1
-      area: 1
+      area: 42949672
       cost: 55
       passive_interface: true
       hello_interval: 15
@@ -159,7 +223,7 @@
     nxos_interface_ospf: &configurelb
       interface: LOOPBACK55
       ospf: 1
-      area: 1
+      area: 4.4.4.4
       cost: 55
       hello_interval: 15
       dead_interval: 75
@@ -179,7 +243,7 @@
     nxos_interface_ospf: &configurelblower
       interface: loopback77
       ospf: 1
-      area: 1
+      area: 429496
       cost: 77
       hello_interval: 45
       dead_interval: 75
@@ -199,7 +263,7 @@
     nxos_interface_ospf: &removeconfig
       interface: "{{ testint }}"
       ospf: 1
-      area: 1
+      area: 12345678
       cost: 55
       passive_interface: true
       hello_interval: 15


Code review comments:
diff_hunk: @@ -95,14 +95,14 @@
   message_digest_algorithm_type:
     description:
       - Algorithm used for authentication among neighboring routers
-        within an area. Valid values is 'md5'.
+        within an area. Valid values are 'md5' and 'default'.
     required: false
     choices: ['md5']
     default: null
   message_digest_encryption_type:
     description:
       - Specifies the scheme used for encrypting message_digest_password.
-        Valid values are '3des' or 'cisco_type_7' encryption.
+        Valid values are '3des' or 'cisco_type_7' encryption or 'default'.
     required: false
     choices: ['cisco_type_7','3des']
path: lib/ansible/modules/network/nxos/nxos_interface_ospf.py
text: @user1:
Please add `default` in choices also.

diff_hunk: @@ -95,14 +95,14 @@
   message_digest_algorithm_type:
     description:
       - Algorithm used for authentication among neighboring routers
-        within an area. Valid values is 'md5'.
+        within an area. Valid values are 'md5' and 'default'.
     required: false
     choices: ['md5']
path: lib/ansible/modules/network/nxos/nxos_interface_ospf.py
text: @user1:
Please add `default` in choices also.