Do not remove the old masters when setting the attribute fails

If the setting of server attribute fails (e.g. due to master not having the associated role enabled) the error would pop up *after* the old values were cleared from LDAP. Fix this behavior so that all checks are performed before manipulating any data. https://pagure.io/freeipa/issue/7029 Reviewed-By: Martin Basti <[email protected]>
martbab · Jul 4, 2017 · e2e380e · e2e380e
1 parent f4d7753
commit e2e380e
Showing 1 changed file with 12 additions and 7 deletions.
diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py
@@ -371,11 +371,18 @@ def _add(self, api_instance, masters):
                    on the master
         """
 
+        ldap = api_instance.Backend.ldap2
+
+        master_dns = self._get_master_dns(api_instance, masters)
+        service_entries = self._get_masters_service_entries(ldap, master_dns)
+        for service_entry in service_entries:
+            self._add_attribute_to_svc_entry(ldap, service_entry)
+
+    def _check_receiving_masters_having_associated_role(self, api_instance,
+                                                      masters):
         assoc_role_providers = set(
             self._get_assoc_role_providers(api_instance))
         masters_set = set(masters)
-        ldap = api_instance.Backend.ldap2
-
         masters_without_role = masters_set - assoc_role_providers
 
         if masters_without_role:
@@ -385,11 +392,6 @@ def _add(self, api_instance, masters):
                         {'role': self.associated_role.name})
             )
 
-        master_dns = self._get_master_dns(api_instance, masters)
-        service_entries = self._get_masters_service_entries(ldap, master_dns)
-        for service_entry in service_entries:
-            self._add_attribute_to_svc_entry(ldap, service_entry)
-
     def set(self, api_instance, masters):
         """
         set the attribute on masters
@@ -407,6 +409,9 @@ def set(self, api_instance, masters):
         if sorted(old_masters) == sorted(masters):
             raise errors.EmptyModlist
 
+        self._check_receiving_masters_having_associated_role(
+            api_instance, masters)
+
         if old_masters:
             self._remove(api_instance, old_masters)