From 8742299e986b672ef1da0ea05d1acce84ed95362 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20=C3=81lvaro?= <carlos@cdalvaro.io>
Date: Thu, 27 Jun 2024 17:04:48 +0200
Subject: [PATCH] feat(macdefaults): catch write exceptions to return safely

---
 salt/modules/macdefaults.py | 24 +++++++++++++++++++-----
 salt/states/macdefaults.py  | 21 ++++++++++++++++-----
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/salt/modules/macdefaults.py b/salt/modules/macdefaults.py
index 31688a484a2..91318214b90 100644
--- a/salt/modules/macdefaults.py
+++ b/salt/modules/macdefaults.py
@@ -254,18 +254,32 @@ def delete(domain, key, user=None):
 def cast_value_to_vtype(value, vtype):
     """
     Convert the value to the specified vtype.
-    If the value cannot be converted, it will be returned as is.
+    If the value cannot be converted, a ValueError is raised.
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' macdefaults.cast_value_to_vtype "1.35" "float"
+
+        salt '*' macdefaults.cast_value_to_vtype "2024-06-23T09:33:44Z" "date"
+
+        salt '*' macdefaults.cast_value_to_vtype "737720347.089987" "date"
 
     value
         The value to be converted
 
     vtype
-        The type to convert the value to
+        The type to convert the value to.
+        Valid types are string, int[eger], float, bool[ean], date, data,
+        array, array-add, dict, dict-add
+
+    Raises:
+        ValueError: When the value cannot be converted to the specified type
 
     Returns:
         The converted value
 
-
     """
     # Boolean
     if vtype in ("bool", "boolean"):
@@ -301,11 +315,11 @@ def cast_value_to_vtype(value, vtype):
         if not isinstance(value, datetime):
             try:
                 value = datetime.fromtimestamp(float(value))
-            except ValueError:
+            except ValueError as e:
                 if re.match(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", value):
                     value = datetime.strptime(value, "%Y-%m-%dT%H:%M:%SZ")
                 else:
-                    raise ValueError(f"Invalid date format: '{value}'")
+                    raise ValueError(f"Invalid date format: '{value}'") from e
     # Data
     elif vtype == "data":
         if isinstance(value, str):
diff --git a/salt/states/macdefaults.py b/salt/states/macdefaults.py
index cd0e6df633e..2aa5701df0d 100644
--- a/salt/states/macdefaults.py
+++ b/salt/states/macdefaults.py
@@ -33,7 +33,7 @@ def write(name, domain, value, vtype=None, user=None):
         The value to write to the given key
 
     vtype
-        The type of value to be written, valid types are string, data, int[eger],
+        The type of value to be written. Valid types are string, data, int[eger],
         float, bool[ean], date, array, array-add, dict, dict-add
 
     user
@@ -45,17 +45,28 @@ def write(name, domain, value, vtype=None, user=None):
     current_value = __salt__["macdefaults.read"](domain, name, user)
 
     if vtype is not None:
-        value = __salt__["macdefaults.cast_value_to_vtype"](value, vtype)
+        try:
+            value = __salt__["macdefaults.cast_value_to_vtype"](value, vtype)
+        except ValueError as e:
+            ret["result"] = False
+            ret["comment"] = f"Failed to cast value {value} to {vtype}. {e}"
+            return ret
 
     if _compare_values(value, current_value, vtype):
-        ret["comment"] += f"{domain} {name} is already set to {value}"
-    else:
+        ret["comment"] = f"{domain} {name} is already set to {value}"
+        return ret
+
+    try:
         out = __salt__["macdefaults.write"](domain, name, value, vtype, user)
         if out["retcode"] != 0:
             ret["result"] = False
-            ret["comment"] = f"Failed to write default. {out['stdout']}"
+            ret["comment"] = f"Failed to write default. {out['stderr']}"
         else:
             ret["changes"]["written"] = f"{domain} {name} is set to {value}"
+    # pylint: disable-next=broad-exception-caught
+    except Exception as e:
+        ret["result"] = False
+        ret["comment"] = f"Failed to write default. {e}"
 
     return ret