Bugfix: Change tag diff to sets to assert uniqueness (#247)

Author: FragmentedPacket

.travis.yml

@@ -30,7 +30,7 @@ jobs:
         - cd ..
         - pip install -U pip "setuptools>=46.1.3"
         - pip install pytest==4.6.5 pytest-mock pytest-xdist jinja2 PyYAML black==19.10b0 "coverage<5"
-        - pip install pynetbox cryptography codecov jmespath jsondiff ansible
+        - pip install pynetbox==4.3.3 cryptography codecov jmespath jsondiff ansible
 
     # Stick to python 3.7 instead of 3.8, as ansible-test sanity is not compatible with 3.7
     # https://github.com/ansible/ansible/issues/67118
@@ -49,7 +49,7 @@ jobs:
         - cd ..
         - pip install -U pip "setuptools>=46.1.3"
         - pip install pytest==4.6.5 pytest-mock pytest-xdist jinja2 PyYAML black==19.10b0 "coverage<5"
-        - pip install pynetbox cryptography codecov jmespath jsondiff ansible
+        - pip install pynetbox==4.3.3 cryptography codecov jmespath jsondiff ansible
 
     # Latest development versions of Netbox and Ansible, newest Python
     # This may be broken sometimes by changes in the netbox & ansible projects
@@ -67,7 +67,7 @@ jobs:
         - cd ..
         - pip install -U pip "setuptools>=46.1.3"
         - pip install pytest==4.6.5 pytest-mock pytest-xdist jinja2 PyYAML black==19.10b0 "coverage<5"
-        - pip install pynetbox cryptography jmespath jsondiff
+        - pip install pynetbox==4.3.3 cryptography jmespath jsondiff
         - git clone https://github.com/ansible/ansible.git
         - cd ansible
         - source hacking/env-setup

plugins/module_utils/netbox_utils.py

@@ -805,7 +805,12 @@ def _update_netbox_object(self, data):
         """
         serialized_nb_obj = self.nb_object.serialize()
         updated_obj = serialized_nb_obj.copy()
-        updated_obj.update(data)
+        if serialized_nb_obj.get("tags") and data.get("tags"):
+            serialized_nb_obj["tags"] = set(serialized_nb_obj["tags"])
+            updated_obj["tags"] = set(data["tags"])
+        else:
+            updated_obj.update(data)
+
         if serialized_nb_obj == updated_obj:
             return serialized_nb_obj, None
         else:

tests/integration/targets/latest/tasks/netbox_power_outlet.yml

@@ -24,7 +24,7 @@
       - test_one['diff']['before']['state'] == "absent"
       - test_one['diff']['after']['state'] == "present"
       - test_one['power_outlet']['name'] == "Power Outlet"
-      - test_one['power_outlet']['device'] == 9
+      - test_one['power_outlet']['device'] == 10
       - test_one['msg'] == "power_outlet Power Outlet created"
 
 - name: "POWER_OUTLET 2: Create duplicate"
@@ -42,7 +42,7 @@
     that:
       - not test_two['changed']
       - test_two['power_outlet']['name'] == "Power Outlet"
-      - test_two['power_outlet']['device'] == 9
+      - test_two['power_outlet']['device'] == 10
       - test_two['msg'] == "power_outlet Power Outlet already exists"
 
 - name: "POWER_OUTLET 3: Update power_outlet with other fields"
@@ -68,7 +68,7 @@
       - test_three['diff']['after']['feed_leg'] == "B"
       - test_three['diff']['after']['description'] == "test description"
       - test_three['power_outlet']['name'] == "Power Outlet"
-      - test_three['power_outlet']['device'] == 9
+      - test_three['power_outlet']['device'] == 10
       - test_three['power_outlet']['type'] == "ita-e"
       - test_three['power_outlet']['power_port'] == 1
       - test_three['power_outlet']['feed_leg'] == "B"
@@ -92,7 +92,7 @@
       - test_four['diff']['before']['state'] == "absent"
       - test_four['diff']['after']['state'] == "present"
       - test_four['power_outlet']['name'] == "Power Outlet 2"
-      - test_four['power_outlet']['device'] == 9
+      - test_four['power_outlet']['device'] == 10
       - test_four['msg'] == "power_outlet Power Outlet 2 created"
 
 - name: "POWER_OUTLET 5: Delete Power Outlet"

tests/integration/targets/latest/tasks/netbox_power_port.yml

@@ -35,7 +35,7 @@
       - test_one['diff']['before']['state'] == "absent"
       - test_one['diff']['after']['state'] == "present"
       - test_one['power_port']['name'] == "Power Port"
-      - test_one['power_port']['device'] == 9
+      - test_one['power_port']['device'] == 10
       - test_one['msg'] == "power_port Power Port created"
 
 - name: "POWER_PORT 2: Create duplicate"
@@ -53,7 +53,7 @@
     that:
       - not test_two['changed']
       - test_two['power_port']['name'] == "Power Port"
-      - test_two['power_port']['device'] == 9
+      - test_two['power_port']['device'] == 10
       - test_two['msg'] == "power_port Power Port already exists"
 
 - name: "POWER_FEED 3: Update power_port with other fields"
@@ -79,7 +79,7 @@
       - test_three['diff']['after']['maximum_draw'] == 20
       - test_three['diff']['after']['description'] == "test description"
       - test_three['power_port']['name'] == "Power Port"
-      - test_three['power_port']['device'] == 9
+      - test_three['power_port']['device'] == 10
       - test_three['power_port']['type'] == "ita-e"
       - test_three['power_port']['allocated_draw'] == 10
       - test_three['power_port']['maximum_draw'] == 20
@@ -103,7 +103,7 @@
       - test_four['diff']['before']['state'] == "absent"
       - test_four['diff']['after']['state'] == "present"
       - test_four['power_port']['name'] == "Power Port 2"
-      - test_four['power_port']['device'] == 9
+      - test_four['power_port']['device'] == 10
       - test_four['msg'] == "power_port Power Port 2 created"
 
 - name: "POWER_PORT 5: Delete Power Port"

tests/integration/targets/regression/tasks/main.yml

@@ -44,3 +44,62 @@
         that:
           - test_two is changed
           - test_two['msg'] == "New prefix created within 10.10.0.0/16"
+
+    - name: "3 - Add device with tags - Setup device to test #242"
+      netbox.netbox.netbox_device:
+        netbox_url: "http://localhost:32768"
+        netbox_token: "0123456789abcdef0123456789abcdef01234567"
+        data:
+          name: "issue-242"
+          device_type: "Cisco Test"
+          device_role: "Core Switch"
+          site: "Test Site"
+          status: "Staged"
+          tags:
+            - first
+            - second
+
+    - name: "4 - Add device with tags out of order - shouldn't change - Tests #242 is fixed"
+      netbox.netbox.netbox_device:
+        netbox_url: "http://localhost:32768"
+        netbox_token: "0123456789abcdef0123456789abcdef01234567"
+        data:
+          name: "issue-242"
+          device_type: "Cisco Test"
+          device_role: "Core Switch"
+          site: "Test Site"
+          status: "Staged"
+          tags:
+            - second
+            - first
+      register: test_four
+      diff: yes
+
+    - name: "4 - Assert not changed - Tests #242 is fixed"
+      assert:
+        that:
+          - not test_four["changed"]
+
+    - name: "5 - Add device with extra tag - Tests #242 is fixed"
+      netbox.netbox.netbox_device:
+        netbox_url: "http://localhost:32768"
+        netbox_token: "0123456789abcdef0123456789abcdef01234567"
+        data:
+          name: "issue-242"
+          device_type: "Cisco Test"
+          device_role: "Core Switch"
+          site: "Test Site"
+          status: "Staged"
+          tags:
+            - second
+            - third
+            - first
+      register: test_five
+      diff: yes
+
+    - name: "5 - Assert added tag - Tests #242 is fixed"
+      assert:
+        that:
+          - test_five is changed
+          - test_five["diff"]["after"]["tags"] is defined
+          - test_five["device"]["tags"] is defined