Merge pull request #1456 from dchiquito/users-ordering

Use set comparison for updating permissions, groups, actions, and object types

Author: sc68cal

changelogs/fragments/users-ordering.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - user.groups, user.permissions, user_group.permissions, permission.actions, and permission.object_types are now treated as unordered sets for update comparison purposes.

plugins/module_utils/netbox_users.py

@@ -10,12 +10,14 @@
     ENDPOINT_NAME_MAPPING,
 )
 
-NB_CONFIG = "config"
 NB_GROUPS = "groups"
 NB_PERMISSIONS = "permissions"
 NB_TOKENS = "tokens"
 NB_USERS = "users"
 
+# These suboptions are lists, but need to be modeled as sets for comparison purposes.
+LIST_AS_SET_KEYS = set(["permissions", "groups", "actions", "object_types"])
+
 
 class NetboxUsersModule(NetboxModule):
     def __init__(self, module, endpoint):
@@ -26,7 +28,6 @@ def run(self):
         This function should have all necessary code for endpoints within the
         application to create/update/delete the endpoint objects
         Supported endpoints:
-        - config
         - groups
         - permissions
         - tokens
@@ -72,18 +73,27 @@ def run(self):
         self.module.exit_json(**self.result)
 
     def _update_netbox_object(self, data):
-        if self.endpoint == "users":
-            return self._update_netbox_user(data)
+        if self.endpoint == NB_TOKENS:
+            return self._update_netbox_token(data)
         else:
-            if self.endpoint == "tokens" and "key" in data:
-                del data["key"]
-            return super()._update_netbox_object(data)
+            return self.__update_netbox_object__(data)
+
+    def _update_netbox_token(self, data):
+        if "key" in data:
+            del data["key"]
+        return self.__update_netbox_object__(data)
 
-    def _update_netbox_user(self, data):
+    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:
+            for key in LIST_AS_SET_KEYS:
+                if serialized_nb_obj.get(key) and data.get(key):
+                    serialized_nb_obj[key] = set(serialized_nb_obj[key])
+                    updated_obj[key] = set(data[key])
+
         if serialized_nb_obj == updated_obj:
             return serialized_nb_obj, None
         else:

tests/integration/targets/v4.0/tasks/netbox_permission.yml

@@ -115,15 +115,15 @@
   ansible.builtin.assert:
     that:
       - test_five is changed
-      - test_five['user_group']['permissions'] == [1]
+      - test_five['user_group']['permissions'] == [test_one['permission']['id']]
 
 - name: "PERMISSION 6: Add permission to user"
   netbox.netbox.netbox_user:
     netbox_url: http://localhost:32768
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
     data:
       username: TestUser
-      password: TestPassword
+      password: TestPassword6
       permissions:
         - Test Permission 2
     state: present
@@ -133,7 +133,7 @@
   ansible.builtin.assert:
     that:
       - test_six is changed
-      - test_six['user']['permissions'] == [2]
+      - test_six['user']['permissions'] == [test_four['permission']['id']]
 
 - name: "PERMISSION 7: Delete"
   netbox.netbox.netbox_permission:
@@ -167,3 +167,48 @@
       - not test_eight['changed']
       - test_eight['permission'] == None
       - test_eight['msg'] == "permission Test Permission already absent"
+
+- name: "PERMISSION 9: Necessary permission"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission
+      description: The test permission
+      enabled: true
+      actions:
+        - view
+        - add
+        - change
+        - delete
+        - extreme_administration
+      object_types:
+        - vpn.tunneltermination
+        - wireless.wirelesslan
+    state: present
+
+- name: "PERMISSION 9: Re-create permission with lists in wrong order"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission
+      description: The test permission
+      enabled: true
+      actions:
+        - extreme_administration
+        - delete
+        - change
+        - add
+        - view
+      object_types:
+        - wireless.wirelesslan
+        - vpn.tunneltermination
+    state: present
+  register: test_nine
+
+- name: "PERMISSION 9: ASSERT - The same lists in a new order do not update the permission"
+  ansible.builtin.assert:
+    that:
+      - not test_nine['changed']
+      # actions and object_types seem to be ordered randomly so we cannot test them here

tests/integration/targets/v4.0/tasks/netbox_token.yml

@@ -10,8 +10,9 @@
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
     data:
       username: TestUser
-      password: TestPassword
+      password: TestPassword1
     state: present
+  register: test_user
 
 - name: "TOKEN 1: Necessary info creation"
   netbox.netbox.netbox_token:
@@ -29,7 +30,7 @@
       - test_one is changed
       - test_one['diff']['before']['state'] == "absent"
       - test_one['diff']['after']['state'] == "present"
-      - test_one['token']['user'] == 3
+      - test_one['token']['user'] == test_user['user']['id']
       - test_one['msg'] == "token ******** created"
 
 - name: "TOKEN 2: Create duplicate"

tests/integration/targets/v4.0/tasks/netbox_user.yml

@@ -10,11 +10,11 @@
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
     data:
       username: TestUser
-      password: TestPassword
+      password: TestPassword1
     state: present
   register: test_one
 
-- name: "USESR 1: ASSERT - Necessary info creation"
+- name: "USER 1: ASSERT - Necessary info creation"
   ansible.builtin.assert:
     that:
       - test_one is changed
@@ -48,7 +48,7 @@
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
     data:
       username: TestUser
-      password: TestPassword
+      password: TestPassword3
       email: [email protected]
       first_name: Test
       last_name: User
@@ -88,7 +88,7 @@
       - test_four['diff']['after']['state'] == "absent"
       - test_four['msg'] == "user TestUser deleted"
 
-- name: "USER 5: ASSERT - Delete non existing"
+- name: "USER 5: Delete non existing"
   netbox.netbox.netbox_user:
     netbox_url: http://localhost:32768
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
@@ -103,3 +103,100 @@
       - not test_five['changed']
       - test_five['user'] == None
       - test_five['msg'] == "user TestUser already absent"
+
+- name: "USER 6: Necessary group 1"
+  netbox.netbox.netbox_user_group:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test User Group Alpha
+    state: present
+  register: user_group_alpha
+
+- name: "USER 6: Necessary group 2"
+  netbox.netbox.netbox_user_group:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test User Group Beta
+    state: present
+  register: user_group_beta
+
+- name: "User 6: Necessary permission 1"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission Foo
+      actions:
+        - view
+      object_types: []
+    state: present
+  register: permission_foo
+
+- name: "User 6: Necessary permission 2"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission Bar
+      actions:
+        - view
+      object_types: []
+    state: present
+  register: permission_bar
+
+- name: "User 6: Necessary permission 3"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission Baz
+      actions:
+        - view
+      object_types: []
+    state: present
+  register: permission_baz
+
+- name: "USER 6: Set up user with multiple groups and permissions"
+  netbox.netbox.netbox_user:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      username: TestUser2
+      password: TestPassword2
+      permissions:
+        - Test Permission Foo
+        - Test Permission Bar
+        - Test Permission Baz
+      groups:
+        - Test User Group Alpha
+        - Test User Group Beta
+    state: present
+
+- name: "USER 6: Re-create user with lists in wrong order"
+  netbox.netbox.netbox_user:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      username: TestUser2
+      permissions:
+        - Test Permission Bar
+        - Test Permission Baz
+        - Test Permission Foo
+      groups:
+        - Test User Group Beta
+        - Test User Group Alpha
+    state: present
+  register: test_six
+
+- name: "USER 6: ASSERT - The same lists in a new order do not update the user"
+  ansible.builtin.assert:
+    that:
+      - not test_six['changed']
+      - test_six['msg'] == "user TestUser2 already exists"
+      - test_six['user']['groups'][0] == user_group_alpha['user_group']['id']
+      - test_six['user']['groups'][1] == user_group_beta['user_group']['id']
+      - test_six['user']['permissions'][0] == permission_foo['permission']['id']
+      - test_six['user']['permissions'][1] == permission_bar['permission']['id']
+      - test_six['user']['permissions'][2] == permission_baz['permission']['id']

tests/integration/targets/v4.0/tasks/netbox_user_group.yml

@@ -81,7 +81,7 @@
     netbox_token: "0123456789abcdef0123456789abcdef01234567"
     data:
       username: TestUser
-      password: TestPassword
+      password: TestPassword5
       groups:
         - Test User Group
     state: present
@@ -91,7 +91,7 @@
   ansible.builtin.assert:
     that:
       - test_five is changed
-      - test_five['user']['groups'] == [1]
+      - test_five['user']['groups'] == [test_one['user_group']['id']]
 
 - name: "USER_GROUP 6: Delete"
   netbox.netbox.netbox_user_group:
@@ -125,3 +125,57 @@
       - not test_seven['changed']
       - test_seven['user_group'] == None
       - test_seven['msg'] == "user_group Test User Group already absent"
+
+- name: "USER_GROUP 8: Necessary permission 1"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission Foo
+      actions:
+        - view
+      object_types: []
+    state: present
+  register: permission_foo
+
+- name: "USER_GROUP 8: Necessary permission 2"
+  netbox.netbox.netbox_permission:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test Permission Bar
+      actions:
+        - view
+      object_types: []
+    state: present
+  register: permission_bar
+
+- name: "USER_GROUP 8: Necessary info creation"
+  netbox.netbox.netbox_user_group:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test User Group
+      permissions:
+        - Test Permission Foo
+        - Test Permission Bar
+    state: present
+
+- name: "USER_GROUP 8: Re-create user group with permissions in wrong order"
+  netbox.netbox.netbox_user_group:
+    netbox_url: http://localhost:32768
+    netbox_token: "0123456789abcdef0123456789abcdef01234567"
+    data:
+      name: Test User Group
+      permissions:
+        - Test Permission Bar
+        - Test Permission Foo
+    state: present
+  register: test_eight
+
+- name: "USER_GROUP 8: ASSERT - The same permissions in a new order do not update the group"
+  ansible.builtin.assert:
+    that:
+      - not test_eight is changed
+      - test_eight['user_group']['permissions'][0] == permission_foo['permission']['id']
+      - test_eight['user_group']['permissions'][1] == permission_bar['permission']['id']