netcmd: domain: add model exceptions and error handling

* Only handle what we know, otherwise raise the existing LdbError
* Cutom messages added in the model layer so we don't have to do it in
  the commands themselves

Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>

python/samba/netcmd/domain/models/auth_silo.py

@@ -20,8 +20,9 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE, Message, MessageElement
+from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE, LdbError, Message, MessageElement
 
+from .exceptions import AddMemberError, RemoveMemberError
 from .fields import DnField, BooleanField, StringField
 from .model import Model
 
@@ -65,7 +66,10 @@ class AuthenticationSilo(Model):
                                    "msDS-AuthNPolicySiloMembers"))
 
         # Update authentication silo.
-        ldb.modify(message)
+        try:
+            ldb.modify(message)
+        except LdbError as e:
+            raise AddMemberError(f"Failed to add silo member: {e}")
 
         # If the modify operation was successful refresh members field.
         self.refresh(ldb, fields=["members"])
@@ -85,7 +89,10 @@ class AuthenticationSilo(Model):
                                    "msDS-AuthNPolicySiloMembers"))
 
         # Update authentication silo.
-        ldb.modify(message)
+        try:
+            ldb.modify(message)
+        except LdbError as e:
+            raise RemoveMemberError(f"Failed to remove silo member: {e}")
 
         # If the modify operation was successful refresh members field.
         self.refresh(ldb, fields=["members"])

python/samba/netcmd/domain/models/exceptions.py

@@ -20,5 +20,33 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-class MultipleObjectsReturned(Exception):
+class ModelError(Exception):
+    pass
+
+
+class MultipleObjectsReturned(ModelError):
+    pass
+
+
+class DoesNotExist(ModelError):
+    pass
+
+
+class AddMemberError(ModelError):
+    pass
+
+
+class RemoveMemberError(ModelError):
+    pass
+
+
+class ProtectError(ModelError):
+    pass
+
+
+class UnprotectError(ModelError):
+    pass
+
+
+class DeleteError(ModelError):
     pass

python/samba/netcmd/domain/models/model.py

@@ -27,7 +27,8 @@ from ldb import ERR_NO_SUCH_OBJECT, FLAG_MOD_ADD, FLAG_MOD_REPLACE, LdbError,\
     Message, MessageElement, SCOPE_BASE, SCOPE_SUBTREE, binary_encode
 from samba.sd_utils import SDUtils
 
-from .exceptions import MultipleObjectsReturned
+from .exceptions import DeleteError, DoesNotExist, MultipleObjectsReturned,\
+    ProtectError, UnprotectError
 from .fields import DateTimeField, DnField, Field, GUIDField, IntegerField,\
     StringField
 
@@ -173,7 +174,15 @@ class Model(metaclass=ModelMeta):
         :param fields: Optional list of field names to refresh
         """
         attrs = [self.fields[f].name for f in fields] if fields else None
-        res = ldb.search(self.dn, scope=SCOPE_BASE, attrs=attrs)
+
+        # This shouldn't normally happen but in case the object refresh fails.
+        try:
+            res = ldb.search(self.dn, scope=SCOPE_BASE, attrs=attrs)
+        except LdbError as e:
+            if e.args[0] == ERR_NO_SUCH_OBJECT:
+                raise DoesNotExist(f"Refresh failed, object gone: {self.dn}")
+            raise
+
         self._apply(ldb, res[0])
 
     def as_dict(self, include_hidden=False):
@@ -225,9 +234,17 @@ class Model(metaclass=ModelMeta):
         :param ldb: Ldb connection
         :param kwargs: Search criteria as keyword args
         """
-        result = ldb.search(cls.get_search_dn(ldb),
-                            scope=SCOPE_SUBTREE,
-                            expression=cls.build_expression(**kwargs))
+        base_dn = cls.get_search_dn(ldb)
+
+        # If the container does not exist produce a friendly error message.
+        try:
+            result = ldb.search(base_dn,
+                                scope=SCOPE_SUBTREE,
+                                expression=cls.build_expression(**kwargs))
+        except LdbError as e:
+            if e.args[0] == ERR_NO_SUCH_OBJECT:
+                raise DoesNotExist(f"Container does not exist: {base_dn}")
+            raise
 
         # For now this returns a simple generator of model instances.
         # This could eventually become a QuerySet class if we need to add
@@ -262,9 +279,17 @@ class Model(metaclass=ModelMeta):
                 else:
                     raise
         else:
-            res = ldb.search(cls.get_search_dn(ldb),
-                             scope=SCOPE_SUBTREE,
-                             expression=cls.build_expression(**kwargs))
+            base_dn = cls.get_search_dn(ldb)
+
+            # If the container does not exist produce a friendly error message.
+            try:
+                res = ldb.search(base_dn,
+                                 scope=SCOPE_SUBTREE,
+                                 expression=cls.build_expression(**kwargs))
+            except LdbError as e:
+                if e.args[0] == ERR_NO_SUCH_OBJECT:
+                    raise DoesNotExist(f"Container does not exist: {base_dn}")
+                raise
 
         # Expect to get one object back or raise MultipleObjectsReturned.
         # For multiple records, please call .query() instead.
@@ -382,8 +407,13 @@ class Model(metaclass=ModelMeta):
 
         :param ldb: Ldb connection
         """
-        if self.dn is not None:
+        if self.dn is None:
+            raise DeleteError("Cannot delete object that doesn't have a dn.")
+
+        try:
             ldb.delete(self.dn)
+        except LdbError as e:
+            raise DeleteError(f"Delete failed: {e}")
 
     def protect(self, ldb):
         """Protect object from accidental deletion.
@@ -391,7 +421,11 @@ class Model(metaclass=ModelMeta):
         :param ldb: Ldb connection
         """
         utils = SDUtils(ldb)
-        utils.dacl_add_ace(self.dn, "(D;;DTSD;;;WD)")
+
+        try:
+            utils.dacl_add_ace(self.dn, "(D;;DTSD;;;WD)")
+        except LdbError as e:
+            raise ProtectError(f"Failed to protect object: {e}")
 
     def unprotect(self, ldb):
         """Unprotect object from accidental deletion.
@@ -399,4 +433,8 @@ class Model(metaclass=ModelMeta):
         :param ldb: Ldb connection
         """
         utils = SDUtils(ldb)
-        utils.dacl_delete_aces(self.dn, "(D;;DTSD;;;WD)")
+
+        try:
+            utils.dacl_delete_aces(self.dn, "(D;;DTSD;;;WD)")
+        except LdbError as e:
+            raise UnprotectError(f"Failed to unprotect object: {e}")