User mapping rule performs operator validation for all updates to prevent NoneType errors (#188)

Co-authored-by: Bhavneet-Sharma <[email protected]>

Author: P-Cao

plugins/modules/user_mapping_rule.py

@@ -495,10 +495,11 @@ def form_rule_for_operations_update(self, update_rule, new_rule_data, do_update)
     def form_rule_for_user_update(self, key, update_rule, new_rule_data, do_update):
         """
         Check and form rule for user update
+        :param key: The key to check in the rules
         :param update_rule: Updated rule data
         :param new_rule_data: New rule data.
         :param do_update: Is update required.
-        :return: True if update is needed and and the new updated rule.
+        :return: True if update is needed and the new updated rule.
         """
         if 'domain' in new_rule_data[key] and new_rule_data[key]['domain'] != update_rule[key]['domain']:
             update_rule[key]['domain'] = new_rule_data[key]['domain']
@@ -536,6 +537,7 @@ def check_and_form_rule_for_update(self, new_rule_params, rule_details):
         do_update = False
         new_rule_data = new_rule_params['rule']
         if new_rule_data:
+            self.validate_for_specific_operators(new_rule_data)
             if 'operator' in new_rule_data and new_rule_data['operator'] is not None and new_rule_data['operator'] != rule_details['operator']:
                 update_rule['operator'] = new_rule_data['operator']
                 update_rule = self.form_rule_for_operator_update(update_rule, new_rule_data, rule_details)
@@ -544,7 +546,7 @@ def check_and_form_rule_for_update(self, new_rule_params, rule_details):
                 do_update, update_rule = self.form_rule_for_operations_update(update_rule, new_rule_data, do_update)
             if 'user1' in new_rule_data and new_rule_data['user1'] is not None:
                 do_update, update_rule = self.form_rule_for_user_update('user1', update_rule, new_rule_data, do_update)
-            if 'user2' in new_rule_data and new_rule_data['user1'] is not None:
+            if 'user2' in new_rule_data and new_rule_data['user2'] is not None:
                 do_update, update_rule = self.form_rule_for_user_update('user2', update_rule, new_rule_data, do_update)
         if 'new_order' in new_rule_params and new_rule_params['new_order'] is not None:
             do_update = self.check_new_order_update(new_rule_params['apply_order'], new_rule_params['new_order'], do_update)

tests/unit/plugins/modules/test_usermappingrules.py

@@ -227,6 +227,33 @@ def test_update_trim_usermappingrule_exception(self, powerscale_module_mock):
         self.capture_fail_json_call(MockUserMappingRuleApi.get_error_responses('trim_update_error'),
                                     powerscale_module_mock)
 
+    def test_update_trim_usermappingrule_user_exception(self, powerscale_module_mock):
+        usermappingrules_details = MockUserMappingRuleApi.GET_USERMAPPINGRULE_RESPONSE_FOR_TRIM
+        usermappingrules_details['rules']['rules'].append(usermappingrules_details['rules']['rules'][0])
+        usermappingrules_details_after_update = copy.deepcopy(usermappingrules_details)
+        new_rule = copy.deepcopy(usermappingrules_details['rules']['rules'][0])
+        new_rule['rule'] = {
+            'operator': 'trim',
+            'options': {
+                'groups': True
+            }
+        }
+        usermappingrules_details_after_update['rules']['rules'][0] = new_rule
+        self.usermappingrules_args.update({
+            'apply_order': 2,
+            'new_order': 1,
+            'rule': new_rule['rule'],
+            'state': 'present'
+        })
+        mock_none_response = MagicMock(return_value=None)
+        powerscale_module_mock.module.params = self.usermappingrules_args
+        powerscale_module_mock.auth_api.get_mapping_users_rules = MagicMock(
+            side_effect=[MockSDKResponse(usermappingrules_details),
+                         MockSDKResponse(usermappingrules_details)])
+        powerscale_module_mock.auth_api.update_mapping_users_rules = mock_none_response
+        self.capture_fail_json_call(MockUserMappingRuleApi.get_error_responses('trim_user_error'),
+                                    powerscale_module_mock)
+
     def test_update_usermappingrule_exception(self, powerscale_module_mock):
         usermappingrules_details = MockUserMappingRuleApi.GET_USERMAPPINGRULE_RESPONSE
         usermappingrules_details['rules']['rules'].append(usermappingrules_details['rules']['rules'][0])