Validate BGP_AS, more...

1. FabricCommon()._fixup_payloads_to_commit() - refactor for future extensibility.

2. FabricCommon(). _fixup_anycast_gw_mac() - new method to validate and fix ANYCAST_GW_MAC

3. FabricCommon(). _fixup_bgp_as() - new method to validate BGP_AS

4. ConversionUtils().__init__() - Add compiled regex for BGP_AS from the Easy_Fabric template.

5. ConversionUtils(). bgp_as_is_valid() - New method to validate BGP ASN

plugins/module_utils/common/conversion.py

@@ -17,8 +17,79 @@
 __metaclass__ = type
 __author__ = "Allen Robel"
 
+import re
+
 
 class ConversionUtils:
+    """
+    Utility methods for converting, translating, and validating values.
+
+    - bgp_asn_is_valid: Return True if value is a valid BGP ASN, False otherwise.
+    - make_boolean: Return value converted to boolean, if possible.
+    - make_int: Return value converted to int, if possible.
+    - make_none: Return None if value is a string representation of a None type.
+    - reject_boolean_string: Reject quoted boolean values e.g. "False", "true"
+    """
+
+    def __init__(self):
+        re_asn_string = "^(("
+        re_asn_string += "(\\+)?[1-9]{1}[0-9]{0,8}|"
+        re_asn_string += "(\\+)?[1-3]{1}[0-9]{1,9}|"
+        re_asn_string += "(\\+)?"
+        re_asn_string += "[4]{1}([0-1]{1}[0-9]{8}|"
+        re_asn_string += "[2]{1}([0-8]{1}[0-9]{7}|"
+        re_asn_string += "[9]{1}([0-3]{1}[0-9]{6}|"
+        re_asn_string += "[4]{1}([0-8]{1}[0-9]{5}|"
+        re_asn_string += "[9]{1}([0-5]{1}[0-9]{4}|"
+        re_asn_string += "[6]{1}([0-6]{1}[0-9]{3}|"
+        re_asn_string += "[7]{1}([0-1]{1}[0-9]{2}|"
+        re_asn_string += "[2]{1}([0-8]{1}[0-9]{1}|"
+        re_asn_string += "[9]{1}[0-5]{1})))))))))|"
+        re_asn_string += "([1-5]\\d{4}|"
+        re_asn_string += "[1-9]\\d{0,3}|"
+        re_asn_string += "6[0-4]\\d{3}|"
+        re_asn_string += "65[0-4]\\d{2}|"
+        re_asn_string += "655[0-2]\\d|"
+        re_asn_string += "6553[0-5])(\\.[1-5]\\d{4}|[1-9]\\d{0,3}|"
+        re_asn_string += "6[0-4]\\d{3}|65[0-4]\\d{2}|"
+        re_asn_string += "655[0-2]\\d|6553[0-5]|0)?)$"
+        self.re_asn = re.compile(re_asn_string)
+
+        self.bgp_as_invalid_reason = None
+
+    def bgp_as_is_valid(self, value):
+        """
+        -   Return True if value is a valid BGP ASN.
+        -   Return False, otherwise.
+        -   Set ConversionUtils().bgp_as_invalid_reason to a string with the
+            reason why the value is not a valid BGP ASN.
+
+        Usage example:
+
+        ```python
+        conversion = ConversionUtils()
+        if not conversion.bgp_as_is_valid(value):
+            print(conversion.bgp_as_invalid_reason)
+        ```
+        """
+        if isinstance(value, float):
+            msg = f"BGP ASN ({value}) cannot be type float() due to "
+            msg += "loss of trailing zeros. "
+            msg += "Use a string or integer instead."
+            self.bgp_as_invalid_reason = msg
+            return False
+        try:
+            asn = str(value)
+        except:
+            msg = f"BGP ASN ({value}) could not be converted to a string."
+            self.bgp_as_invalid_reason = msg
+            return False
+        if not self.re_asn.match(asn):
+            msg = f"BGP ASN {value} failed regex validation."
+            self.bgp_as_invalid_reason = msg
+            return False
+        return True
+
     @staticmethod
     def make_boolean(value):
         """
@@ -59,7 +130,7 @@ def make_none(value):
     def reject_boolean_string(parameter, value) -> None:
         """
         -   Reject quoted boolean values e.g. "False", "true"
-        -   raise ValueError with informative message if the value is
+        -   Raise ``ValueError`` with informative message if the value is
             a string representation of a boolean.
         """
         if isinstance(value, int):
@@ -73,3 +144,16 @@ def reject_boolean_string(parameter, value) -> None:
             msg += "(e.g. True/False or true/false, instead of "
             msg += "'True'/'False' or 'true'/'false')."
             raise ValueError(msg)
+
+    @staticmethod
+    def translate_mac_address(mac_addr):
+        """
+        -   Accept mac address with any (or no) punctuation and convert it
+            into the dotted-quad format that the controller expects.
+        -   On success, return translated mac address.
+        -   On failure, raise ``ValueError``.
+        """
+        mac_addr = re.sub(r"[\W\s_]", "", mac_addr)
+        if not re.search("^[A-Fa-f0-9]{12}$", mac_addr):
+            raise ValueError(f"Invalid MAC address: {mac_addr}")
+        return "".join((mac_addr[:4], ".", mac_addr[4:8], ".", mac_addr[8:]))

plugins/module_utils/fabric/common.py

@@ -122,44 +122,59 @@ def _fixup_payloads_to_commit(self) -> None:
 
         Modifications:
         - Translate ANYCAST_GW_MAC to a format the controller understands
+        - Validate BGP_AS
+        """
+        try:
+            self._fixup_anycast_gw_mac()
+            self._fixup_bgp_as()
+        except ValueError as error:
+            self.results.failed = True
+            self.results.changed = False
+            self.results.register_task_result()
+            raise ValueError(error) from error
+
+    def _fixup_bgp_as(self) -> None:
+        """
+        Raise ``ValueError`` if BGP_AS is not a valid BGP ASN.
+        """
+        method_name = inspect.stack()[0][3]
+        for payload in self._payloads_to_commit:
+            if "BGP_AS" not in payload:
+                continue
+            bgp_as = payload["BGP_AS"]
+            if not self.conversion.bgp_as_is_valid(bgp_as):
+                fabric_name = payload.get("FABRIC_NAME", "UNKNOWN")
+                msg = f"{self.class_name}.{method_name}: "
+                msg += f"Invalid BGP_AS {bgp_as} "
+                msg += f"for fabric {fabric_name}, "
+                msg += f"Error detail: {self.conversion.bgp_as_invalid_reason}"
+                raise ValueError(msg)
+
+    def _fixup_anycast_gw_mac(self) -> None:
+        """
+        -   Translate the ANYCAST_GW_MAC address to the format the
+            controller expects.
+        -   Raise ``ValueError`` if the translation fails.
         """
         method_name = inspect.stack()[0][3]
         for payload in self._payloads_to_commit:
             if "ANYCAST_GW_MAC" not in payload:
                 continue
             try:
-                payload["ANYCAST_GW_MAC"] = self.translate_mac_address(
+                payload["ANYCAST_GW_MAC"] = self.conversion.translate_mac_address(
                     payload["ANYCAST_GW_MAC"]
                 )
             except ValueError as error:
                 fabric_name = payload.get("FABRIC_NAME", "UNKNOWN")
                 anycast_gw_mac = payload.get("ANYCAST_GW_MAC", "UNKNOWN")
 
-                self.results.failed = True
-                self.results.changed = False
-                self.results.register_task_result()
-
                 msg = f"{self.class_name}.{method_name}: "
                 msg += "Error translating ANYCAST_GW_MAC "
                 msg += f"for fabric {fabric_name}, "
                 msg += f"ANYCAST_GW_MAC: {anycast_gw_mac}, "
                 msg += f"Error detail: {error}"
                 raise ValueError(msg) from error
 
-    @staticmethod
-    def translate_mac_address(mac_addr):
-        """
-        Accept mac address with any (or no) punctuation and convert it
-        into the dotted-quad format that the controller expects.
-
-        Return mac address formatted for the controller on success
-        Raise ValueError on failure.
-        """
-        mac_addr = re.sub(r"[\W\s_]", "", mac_addr)
-        if not re.search("^[A-Fa-f0-9]{12}$", mac_addr):
-            raise ValueError(f"Invalid MAC address: {mac_addr}")
-        return "".join((mac_addr[:4], ".", mac_addr[4:8], ".", mac_addr[8:]))
-
     def _handle_response(self, response, verb) -> Dict[str, Any]:
         """
         - Call the appropriate handler for response based on verb

plugins/module_utils/fabric/update.py

@@ -197,7 +197,7 @@ def _prepare_anycast_gw_mac_for_comparison(self, fabric_name, mac_address):
         """
         method_name = inspect.stack()[0][3]
         try:
-            mac_address = self.translate_mac_address(mac_address)
+            mac_address = self.conversion.translate_mac_address(mac_address)
         except ValueError as error:
             self.results.failed = True
             self.results.changed = False

tests/unit/modules/dcnm/dcnm_fabric/test_fabric_common.py

@@ -124,7 +124,7 @@ def test_fabric_common_00020(fabric_common) -> None:
         instance = fabric_common
         instance.results = Results()
         instance._payloads_to_commit = payloads_fabric_common(key)
-    match = r"FabricCommon\._fixup_payloads_to_commit: "
+    match = r"FabricCommon\._fixup_anycast_gw_mac: "
     match += "Error translating ANYCAST_GW_MAC for fabric f1, "
     match += r"ANYCAST_GW_MAC: 00\.54, Error detail: Invalid MAC address"
     with pytest.raises(ValueError, match=match):

tests/unit/modules/dcnm/dcnm_fabric/test_fabric_create.py

@@ -601,8 +601,10 @@ def test_fabric_create_00033(monkeypatch, fabric_create) -> None:
         which returns a dict with keys DATA == [], RETURN_CODE == 200
     -   FabricCreate()._build_payloads_to_commit() sets FabricCreate()._payloads_to_commit
         to a list containing fabric f1 payload
-    -   FabricCreate.commit() calls FabricCommon_fixup_payloads_to_commit() which
-        raises ``ValueError`` because the mac address is malformed.
+    -   FabricCreate.commit() calls FabricCommon_fixup_payloads_to_commit()
+    -   FabricCommon_fixup_payloads_to_commit() calls
+        FabricCommon()._fixup_anycast_gw_mac() which raises ``ValueError``
+        because the mac address is malformed.
     """
     method_name = inspect.stack()[0][3]
     key = f"{method_name}a"
@@ -634,7 +636,7 @@ def mock_dcnm_send(*args, **kwargs):
 
     monkeypatch.setattr(PATCH_DCNM_SEND, mock_dcnm_send)
 
-    match = r"FabricCreate\._fixup_payloads_to_commit: "
+    match = r"FabricCreate\._fixup_anycast_gw_mac: "
     match += "Error translating ANYCAST_GW_MAC for fabric f1, "
     match += "ANYCAST_GW_MAC: 00:12:34:56:78:9, "
     match += "Error detail: Invalid MAC address: 00123456789"

tests/unit/modules/dcnm/dcnm_fabric/test_fabric_create_bulk.py

@@ -606,13 +606,17 @@ def test_fabric_create_bulk_00033(monkeypatch, fabric_create_bulk) -> None:
     -   FabricCreateBulk.payloads is set to contain one payload for a fabric (f1)
         that does not exist on the controller.  ``ANYCAST_GW_MAC`` in this payload
         has a malformed mac address.
-    -   FabricCreateBulk.commit() calls FabricCreate()._build_payloads_to_commit()
-    -   FabricCreate()._build_payloads_to_commit() calls FabricDetails().refresh()
-        which returns a dict with keys DATA == [], RETURN_CODE == 200
-    -   FabricCreate()._build_payloads_to_commit() sets FabricCreate()._payloads_to_commit
-        to a list containing fabric f1 payload
-    -   FabricCreateBulk.commit() calls FabricCommon_fixup_payloads_to_commit() which
-        raises ``ValueError`` because the mac address is malformed.
+    -   FabricCreateBulk().commit() calls
+        FabricCreate()._build_payloads_to_commit()
+    -   FabricCreate()._build_payloads_to_commit() calls
+        FabricDetails().refresh() which returns a dict with keys
+        DATA == [], RETURN_CODE == 200
+    -   FabricCreate()._build_payloads_to_commit() sets
+        FabricCreate()._payloads_to_commit to a list containing fabric f1 payload
+    -   FabricCreateBulk().commit() calls FabricCommon()._fixup_payloads_to_commit()
+    -   FabricCommon()._fixup_payloads_to_commit() calls
+        FabricCommon()._fixup_anycast_gw_mac() which raises ``ValueError``
+        because the mac address is malformed.
     """
     method_name = inspect.stack()[0][3]
     key = f"{method_name}a"
@@ -644,7 +648,7 @@ def mock_dcnm_send(*args, **kwargs):
 
     monkeypatch.setattr(PATCH_DCNM_SEND, mock_dcnm_send)
 
-    match = r"FabricCreateBulk\._fixup_payloads_to_commit: "
+    match = r"FabricCreateBulk\._fixup_anycast_gw_mac: "
     match += "Error translating ANYCAST_GW_MAC for fabric f1, "
     match += "ANYCAST_GW_MAC: 00:12:34:56:78:9, "
     match += "Error detail: Invalid MAC address: 00123456789"

tests/unit/modules/dcnm/dcnm_fabric/test_fabric_update_bulk.py

@@ -413,7 +413,9 @@ def test_fabric_update_bulk_00031(monkeypatch, fabric_update_bulk) -> None:
         returns False.
     -   FabricUpdateCommon()._send_payloads() calls
         FabricUpdateCommon()._fixup_payloads_to_commit()
-    -   FabricUpdateCommon()._fixup_payloads_to_commit() updates ANYCAST_GW_MAC
+    -   FabricUpdateCommon()._fixup_payloads_to_commit() calls
+        FabricUpdateCommon()._fixup_anycast_gw_mac() which calls
+        Conversion().conversion.translate_mac_address() which updates ANYCAST_GW_MAC
         to conform with the controller's requirements.
     -   FabricUpdateCommon()._send_payloads() calls
         FabricUpdateCommon()._send_payload() for each fabric in
@@ -911,7 +913,9 @@ def test_fabric_update_bulk_00035(monkeypatch, fabric_update_bulk) -> None:
         returns True.
     -   FabricUpdateCommon()._send_payloads() calls
         FabricUpdateCommon()._fixup_payloads_to_commit()
-    -   FabricUpdateCommon()._fixup_payloads_to_commit() updates ANYCAST_GW_MAC
+    -   FabricUpdateCommon()._fixup_payloads_to_commit() calls
+        FabricUpdateCommon()._fixup_anycast_gw_mac() which calls
+        Conversion().conversion.translate_mac_address() which updates ANYCAST_GW_MAC
         to conform with the controller's requirements.
     -   FabricUpdateCommon()._send_payloads() calls
         FabricUpdateCommon()._send_payload() for each fabric in
@@ -1544,13 +1548,11 @@ def mock_send_payload(payload) -> None:
                 "BGP_AS": "65001",
                 "DEPLOY": "true",
                 "FABRIC_NAME": "f1",
-                "FABRIC_TYPE": "VXLAN_EVPN"
+                "FABRIC_TYPE": "VXLAN_EVPN",
             }
         ]
 
-    monkeypatch.setattr(
-        instance, "_send_payload", mock_send_payload
-    )
+    monkeypatch.setattr(instance, "_send_payload", mock_send_payload)
 
     match = r"raised FabricCommon\.self_payload exception\."
     with pytest.raises(ValueError, match=match):
@@ -1620,15 +1622,13 @@ def mock_dcnm_send(*args, **kwargs):
                 "BGP_AS": "65001",
                 "DEPLOY": "true",
                 "FABRIC_NAME": "f1",
-                "FABRIC_TYPE": "VXLAN_EVPN"
+                "FABRIC_TYPE": "VXLAN_EVPN",
             }
         ]
 
     monkeypatch.setattr(PATCH_DCNM_SEND, mock_dcnm_send)
 
-    monkeypatch.setattr(
-        instance, "_config_save", mock_config_save
-    )
+    monkeypatch.setattr(instance, "_config_save", mock_config_save)
 
     match = r"raised FabricCommon\.config_save exception\."
     with pytest.raises(ValueError, match=match):

tests/unit/modules/dcnm/dcnm_fabric/test_template_get.py

@@ -390,6 +390,7 @@ def test_template_get_00070(monkeypatch, template_get) -> None:
     -   Verify that TemplateGet()._set_template_endpoint() re-raises
         ``ValueError`` when ApiEndpoints() raises ``ValueError``.
     """
+
     class MockApiEndpoints:  # pylint: disable=too-few-public-methods
         """
         Mock the ApiEndpoints.template getter property to raise ``ValueError``.