diff --git a/docs/cisco.dcnm.dcnm_fabric_module.rst b/docs/cisco.dcnm.dcnm_fabric_module.rst index 453e30966..59f4eb8a5 100644 --- a/docs/cisco.dcnm.dcnm_fabric_module.rst +++ b/docs/cisco.dcnm.dcnm_fabric_module.rst @@ -1940,6 +1940,7 @@ Parameters
list + / elements=string
@@ -1958,6 +1959,7 @@ Parameters
list + / elements=string
@@ -1976,6 +1978,7 @@ Parameters
list + / elements=string
@@ -1994,6 +1997,7 @@ Parameters
list + / elements=string
@@ -7325,6 +7329,25 @@ Parameters + + +
+ skip_validation + +
+ boolean +
+ + + + + +
Skip playbook parameter validation. Useful for debugging.
+ +
@@ -7407,6 +7430,27 @@ Examples - debug: var: result + # Setting skip_validation to True to bypass parameter validation in the module. + # Note, this does not bypass parameter validation in NDFC. skip_validation + # can be useful to verify that the dcnm_fabric module's parameter validation + # is disallowing parameter combinations that would also be disallowed by + # NDFC. + + - name: Update fabrics + cisco.dcnm.dcnm_fabric: + state: merged + skip_validation: True + config: + - FABRIC_NAME: VXLAN_Fabric + FABRIC_TYPE: VXLAN_EVPN + BGP_AS: 65000 + ANYCAST_GW_MAC: 0001.aabb.ccdd + UNDERLAY_IS_V6: false + EXTRA_CONF_LEAF: | + interface Ethernet1/1-16 + description managed by NDFC + DEPLOY: false + # Use replaced state to return the fabrics to their default configurations. - name: Return fabrics to default configuration. @@ -7453,6 +7497,28 @@ Examples - debug: var: result + # When skip_validation is False (the default), some error messages might be + # misleading. For example, with the playbook below, the error message + # that follows should be interpreted as "ENABLE_PVLAN is mutually-exclusive + # to ENABLE_SGT and should be removed from the playbook if ENABLE_SGT is set + # to True." In the NDFC GUI, if Security Groups is enabled, NDFC disables + # the ability to modify the PVLAN option. Hence, even a valid value for + # ENABLE_PVLAN in the playbook will generate an error. + + - name: merge fabric MyFabric + cisco.dcnm.dcnm_fabric: + state: merged + skip_validation: false + config: + - FABRIC_NAME: MyFabric + FABRIC_TYPE: VXLAN_EVPN + BGP_AS: 65001 + ENABLE_SGT: true + ENABLE_PVLAN: false + + # Resulting error message (edited for brevity) + # "The following parameter(value) combination(s) are invalid and need to be reviewed: Fabric: f3, ENABLE_PVLAN(False) requires ENABLE_SGT != True." + diff --git a/plugins/module_utils/fabric/verify_playbook_params.py b/plugins/module_utils/fabric/verify_playbook_params.py index 1116b074f..f4c79c61b 100644 --- a/plugins/module_utils/fabric/verify_playbook_params.py +++ b/plugins/module_utils/fabric/verify_playbook_params.py @@ -213,30 +213,59 @@ def eval_parameter_rule(self, rule) -> bool: """ method_name = inspect.stack()[0][3] + msg = f"{self.class_name}.{method_name}: " + msg += f"rule: {rule}" + self.log.debug(msg) + parameter = rule.get("parameter", None) if parameter is None: - msg = f"'parameter' not found in rule: {rule}" + msg = f"{self.class_name}.{method_name}: " + msg += f"'parameter' not found in rule: {rule}" raise KeyError(msg) user_value = rule.get("user_value", None) if user_value is None: - msg = f"'user_value' not found in parameter {parameter} rule: {rule}" + msg = f"{self.class_name}.{method_name}: " + msg += f"'user_value' not found in parameter {parameter} rule: {rule}" raise KeyError(msg) operator = rule.get("operator", None) if operator is None: - msg = f"'operator' not found in parameter {parameter} rule: {rule}" + msg = f"{self.class_name}.{method_name}: " + msg += f"'operator' not found in parameter {parameter} rule: {rule}" raise KeyError(msg) rule_value = rule.get("value", None) if rule_value is None: - msg = f"'value' not found in parameter {parameter} rule: {rule}" + msg = f"{self.class_name}.{method_name}: " + msg += f"'value' not found in parameter {parameter} rule: {rule}" raise KeyError(msg) + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"user_value: {user_value}, " + msg += f"operator: {operator}, " + msg += f"rule_value: {rule_value}" + self.log.debug(msg) + + if rule_value in [None, "", "null"]: + msg = f"{self.class_name}.{method_name}: " + msg += "rule_value is None. Returning True." + self.log.debug(msg) + return True + if user_value in [None, "", "null"]: + msg = f"{self.class_name}.{method_name}: " + msg += f"playbook value for parameter {parameter} cannot be null" + self.log.debug(msg) + raise ValueError(msg) + + eval_string = f"user_value {operator} rule_value" + msg = f"{self.class_name}.{method_name}: " + msg += f"eval_string {eval_string}" + self.log.debug(msg) # While eval() can be dangerous with unknown input, the input # we're feeding it is from a known source and has been pretty # heavily massaged before it gets here. - eval_string = f"user_value {operator} rule_value" result = eval(eval_string) # pylint: disable=eval-used msg = f"{self.class_name}.{method_name}: " @@ -251,9 +280,15 @@ def eval_parameter_rule(self, rule) -> bool: def controller_param_is_valid(self, item) -> bool: """ - - Return None if the controller fabric config does not contain - the dependent parameter. This removes the controller result - from consideration when determining parameter validity. + - Return None in the following cases + - The fabric does not exist on the controller. + - The controller's value for the dependent parameter is None + (more accurately, "") + - The controller fabric config does not contain the dependent + parameter (this is not likely) + + Returning One removes the controller result from consideration + when determining parameter validity. - Return the evaluated result (True or False) if the controller fabric config does contain the dependent parameter. The @@ -290,11 +325,24 @@ def controller_param_is_valid(self, item) -> bool: self.log.debug(msg) return None - user_value = self.conversion.make_boolean( - self.config_controller[rule_parameter] + controller_value = self.conversion.make_none( + self.conversion.make_boolean(self.config_controller[rule_parameter]) ) + + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter {rule_parameter}, " + msg += f"controller_value: type {type(controller_value)}, " + msg += f"value {controller_value}" + self.log.debug(msg) + + # If the controller value is None, remove it from consideration. + if controller_value is None: + msg = f"{self.class_name}.{method_name}: " + msg += f"Early return: {rule_parameter} is None. Returning None." + self.log.debug(msg) + return None # update item with user's parameter value - item["user_value"] = user_value + item["user_value"] = controller_value try: return self.eval_parameter_rule(item) except KeyError as error: @@ -309,7 +357,7 @@ def playbook_param_is_valid(self, item) -> bool: the dependent parameter. The evaluated result is calculated from: - eval(playbook_param rule_operator rule_value) + eval(playbook_param rule_operator rule_value) - raise KeyError if self.eval_parameter_rule() fails """ @@ -320,7 +368,7 @@ def playbook_param_is_valid(self, item) -> bool: rule_operator = item.get("operator", None) msg = f"{self.class_name}.{method_name}: " - msg = f"rule_parameter: {rule_parameter}, " + msg += f"rule_parameter: {rule_parameter}, " msg += f"rule_operator: {rule_operator}, " msg += f"rule_value: {rule_value}, " self.log.debug(msg) @@ -333,10 +381,18 @@ def playbook_param_is_valid(self, item) -> bool: self.log.debug(msg) return None - user_value = self.conversion.make_boolean(self.config_playbook[rule_parameter]) + playbook_value = self.conversion.make_none( + self.conversion.make_boolean(self.config_playbook[rule_parameter]) + ) - # update item with user's parameter value - item["user_value"] = user_value + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter {rule_parameter}, " + msg += f"controller_value: type {type(playbook_value)}, " + msg += f"value {playbook_value}" + self.log.debug(msg) + + # update item with playbook's parameter value + item["user_value"] = playbook_value try: return self.eval_parameter_rule(item) except KeyError as error: @@ -385,8 +441,8 @@ def default_param_is_valid(self, item) -> bool: default_value = self._param_info.parameter(rule_parameter).get("default", None) if default_value is None: - msg = f"Early return: parameter: {rule_parameter} has no default value. " - msg += "Returning None." + msg = f"Early return: parameter: {rule_parameter} " + msg += "has no default value. Returning None." self.log.debug(msg) return None @@ -400,6 +456,7 @@ def default_param_is_valid(self, item) -> bool: def update_decision_set(self, item) -> set: """ + ### Summary Update the decision set with the aggregate of results from the - controller fabric configuration - playbook configuration @@ -409,33 +466,103 @@ def update_decision_set(self, item) -> set: - Raise KeyError if controller_param_is_valid() fails - Raise KeyError if playbook_param_is_valid() fails - Raise KeyError if default_param_is_valid() fails + + ### item format + item is a dictionary with the following keys + - parameter: the parameter from the controller config, playbook, or template default + - operator: The rule operator e.g. "==", "!=" + - value: The parameter's value in the controller config, playbook, or template default + - Example + ```json + {'parameter': 'UNDERLAY_IS_V6', 'operator': '!=', 'value': True} + ``` + + ### Notes + 1. If all of the following return None, then we add True to the decision_set. + - controller_param_is_valid() + - playbook_param_is_valid() + - default_param_is_valid() """ + method_name = inspect.stack()[0][3] # pylint: disable=unused-variable decision_set = set() + + msg = f"{self.class_name}.{method_name}: " + msg += f"item {item}" + self.log.debug(msg) + + parameter = item.get("parameter") + try: controller_is_valid = self.controller_param_is_valid(item) except KeyError as error: raise KeyError(f"{error}") from error + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"controller_is_valid: {controller_is_valid}" + self.log.debug(msg) + try: playbook_is_valid = self.playbook_param_is_valid(item) except KeyError as error: raise KeyError(f"{error}") from error + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"playbook_is_valid: {playbook_is_valid}" + self.log.debug(msg) + try: default_is_valid = self.default_param_is_valid(item) except KeyError as error: raise KeyError(f"{error}") from error + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"default_is_valid: {default_is_valid}" + self.log.debug(msg) + if controller_is_valid is not None: + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, add to decision set: " + msg += f"controller_is_valid: {controller_is_valid}" + self.log.debug(msg) decision_set.add(controller_is_valid) if default_is_valid is not None: + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, add to decision set: " + msg += f"default_is_valid: {default_is_valid}" + self.log.debug(msg) decision_set.add(default_is_valid) if playbook_is_valid is not None: + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, add to decision set: " + msg += f"playbook_is_valid: {playbook_is_valid}" + self.log.debug(msg) decision_set.add(playbook_is_valid) + # If playbook config is not valid, ignore all other results - if not playbook_is_valid: + if playbook_is_valid is False: + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"playbook is invalid: {playbook_is_valid}. " + msg += "Setting decision_set to False." + self.log.debug(msg) decision_set = {False} + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += f"decision_set: ({decision_set})" + self.log.debug(msg) + + # If the decision_set is empty, add True. + if len(decision_set) == 0: + msg = f"{self.class_name}.{method_name}: " + msg += f"parameter: {parameter}, " + msg += "decision_set is empty. Setting to True." + self.log.debug(msg) + decision_set = {True} + msg = f"parameter {self.parameter}, " msg += f"item: {item}, " msg += f"controller_is_valid: {controller_is_valid}, " @@ -542,12 +669,17 @@ def update_decision_set_for_and_rules(self, param_rule) -> None: - Raise ``KeyError`` if an error is encountered while updating the decision set. """ + method_name = inspect.stack()[0][3] # pylint: disable=unused-variable for item in param_rule.get("terms", {}).get("and"): try: decision_set = self.update_decision_set(item) except KeyError as error: raise KeyError(f"{error}") from error + msg = f"{self.class_name}.{method_name}: " + msg += f"decision_set: ({decision_set})" + self.log.debug(msg) + # bad_params[fabric][param] = if True not in decision_set: self.params_are_valid.add(False) @@ -729,13 +861,25 @@ def verify_parameter(self): "terms" ) case_na_rule = "na" in param_rule.get("terms") + msg = f"{self.class_name}.{method_name}: " + msg += f"PRE_UPDATE: self.params_are_valid: {self.params_are_valid}" + self.log.debug(msg) try: if case_and_rule: self.update_decision_set_for_and_rules(param_rule) + msg = f"{self.class_name}.{method_name}: " + msg += f"UPDATE_FOR_AND_RULES: parameter: {self.parameter} self.params_are_valid: {self.params_are_valid}" + self.log.debug(msg) elif case_or_rule: self.update_decision_set_for_or_rules(param_rule) + msg = f"{self.class_name}.{method_name}: " + msg += f"UPDATE_FOR_OR_RULES: parameter: {self.parameter} self.params_are_valid: {self.params_are_valid}" + self.log.debug(msg) elif case_na_rule: self.update_decision_set_for_na_rules(param_rule) + msg = f"{self.class_name}.{method_name}: " + msg += f"UPDATE_FOR_NA_RULES: parameter: {self.parameter} self.params_are_valid: {self.params_are_valid}" + self.log.debug(msg) else: msg = f"{self.class_name}.{method_name}: " msg += "TODO: Unhandled parameter rule: " diff --git a/plugins/modules/dcnm_fabric.py b/plugins/modules/dcnm_fabric.py index 68eb28b76..f2cd63f5f 100644 --- a/plugins/modules/dcnm_fabric.py +++ b/plugins/modules/dcnm_fabric.py @@ -37,6 +37,11 @@ description: - The state of the feature or object after module completion type: str + skip_validation: + default: false + description: + - Skip playbook parameter validation. Useful for debugging. + type: bool config: description: - A list of fabric configuration dictionaries @@ -2569,6 +2574,27 @@ - debug: var: result +# Setting skip_validation to True to bypass parameter validation in the module. +# Note, this does not bypass parameter validation in NDFC. skip_validation +# can be useful to verify that the dcnm_fabric module's parameter validation +# is disallowing parameter combinations that would also be disallowed by +# NDFC. + +- name: Update fabrics + cisco.dcnm.dcnm_fabric: + state: merged + skip_validation: True + config: + - FABRIC_NAME: VXLAN_Fabric + FABRIC_TYPE: VXLAN_EVPN + BGP_AS: 65000 + ANYCAST_GW_MAC: 0001.aabb.ccdd + UNDERLAY_IS_V6: false + EXTRA_CONF_LEAF: | + interface Ethernet1/1-16 + description managed by NDFC + DEPLOY: false + # Use replaced state to return the fabrics to their default configurations. - name: Return fabrics to default configuration. @@ -2615,6 +2641,28 @@ - debug: var: result +# When skip_validation is False (the default), some error messages might be +# misleading. For example, with the playbook below, the error message +# that follows should be interpreted as "ENABLE_PVLAN is mutually-exclusive +# to ENABLE_SGT and should be removed from the playbook if ENABLE_SGT is set +# to True." In the NDFC GUI, if Security Groups is enabled, NDFC disables +# the ability to modify the PVLAN option. Hence, even a valid value for +# ENABLE_PVLAN in the playbook will generate an error. + +- name: merge fabric MyFabric + cisco.dcnm.dcnm_fabric: + state: merged + skip_validation: false + config: + - FABRIC_NAME: MyFabric + FABRIC_TYPE: VXLAN_EVPN + BGP_AS: 65001 + ENABLE_SGT: true + ENABLE_PVLAN: false + +# Resulting error message (edited for brevity) +# "The following parameter(value) combination(s) are invalid and need to be reviewed: Fabric: f3, ENABLE_PVLAN(False) requires ENABLE_SGT != True." + """ # pylint: disable=wrong-import-position import copy @@ -3054,10 +3102,17 @@ def get_need(self): except TypeError as error: raise ValueError(f"{error}") from error - try: - self._verify_playbook_params.commit() - except ValueError as error: - raise ValueError(f"{error}") from error + if self.params.get("skip_validation") is False: + try: + self._verify_playbook_params.commit() + except ValueError as error: + raise ValueError(f"{error}") from error + else: + msg = f"{self.class_name}.{method_name}: " + msg += "skip_validation: " + msg += f"{self.params.get('skip_validation')}, " + msg += "skipping parameter validation." + self.log.debug(msg) self.need_create.append(want) @@ -3068,10 +3123,17 @@ def get_need(self): self._verify_playbook_params.config_controller = nv_pairs except TypeError as error: raise ValueError(f"{error}") from error - try: - self._verify_playbook_params.commit() - except (ValueError, KeyError) as error: - raise ValueError(f"{error}") from error + if self.params.get("skip_validation") is False: + try: + self._verify_playbook_params.commit() + except (ValueError, KeyError) as error: + raise ValueError(f"{error}") from error + else: + msg = f"{self.class_name}.{method_name}: " + msg += "skip_validation: " + msg += f"{self.params.get('skip_validation')}, " + msg += "skipping parameter validation." + self.log.debug(msg) self.need_update.append(want) @@ -3424,6 +3486,11 @@ def main(): argument_spec = {} argument_spec["config"] = {"required": False, "type": "list", "elements": "dict"} + argument_spec["skip_validation"] = { + "required": False, + "type": "bool", + "default": False, + } argument_spec["state"] = { "default": "merged", "choices": ["deleted", "merged", "query", "replaced"], diff --git a/tests/unit/modules/dcnm/dcnm_fabric/fixtures/nv_pairs_VerifyPlaybookParams.json b/tests/unit/modules/dcnm/dcnm_fabric/fixtures/nv_pairs_VerifyPlaybookParams.json index 9a29a2c70..45b763e49 100644 --- a/tests/unit/modules/dcnm/dcnm_fabric/fixtures/nv_pairs_VerifyPlaybookParams.json +++ b/tests/unit/modules/dcnm/dcnm_fabric/fixtures/nv_pairs_VerifyPlaybookParams.json @@ -26,6 +26,6 @@ ], "BGP_AS": "65001", "FABRIC_NAME": "f1", - "REPLICATION_MODE": "Ingress" + "V6_SUBNET_RANGE": "fd00::a04:0/112" } } diff --git a/tests/unit/modules/dcnm/dcnm_fabric/fixtures/payloads_VerifyPlaybookParams.json b/tests/unit/modules/dcnm/dcnm_fabric/fixtures/payloads_VerifyPlaybookParams.json index ef25bdcf2..29a258fa7 100644 --- a/tests/unit/modules/dcnm/dcnm_fabric/fixtures/payloads_VerifyPlaybookParams.json +++ b/tests/unit/modules/dcnm/dcnm_fabric/fixtures/payloads_VerifyPlaybookParams.json @@ -44,23 +44,23 @@ }, "test_verify_playbook_params_00070a": { "TEST_NOTES": [ - "Dependent parameters for REPLICATION_MODE are not satisfied." + "Dependent parameters for V6_SUBNET_RANGE are not satisfied." ], "BGP_AS": 65001, "DEPLOY": true, "FABRIC_NAME": "f1", "FABRIC_TYPE": "VXLAN_EVPN", - "REPLICATION_MODE": "Ingress" + "V6_SUBNET_RANGE": "fd00::a05:0/112" }, "test_verify_playbook_params_00080a": { "TEST_NOTES": [ - "Dependent parameters for REPLICATION_MODE are not satisfied." + "Dependent parameters for V6_SUBNET_RANGE are not satisfied." ], "BGP_AS": 65001, "DEPLOY": true, "FABRIC_NAME": "f1", "FABRIC_TYPE": "VXLAN_EVPN", - "REPLICATION_MODE": "Ingress" + "V6_SUBNET_RANGE": "fd00::a05:0/112" }, "test_verify_playbook_params_00090a": { "TEST_NOTES": [ diff --git a/tests/unit/modules/dcnm/dcnm_fabric/test_verify_playbook_params.py b/tests/unit/modules/dcnm/dcnm_fabric/test_verify_playbook_params.py index 2868ab0ae..be8b853e2 100644 --- a/tests/unit/modules/dcnm/dcnm_fabric/test_verify_playbook_params.py +++ b/tests/unit/modules/dcnm/dcnm_fabric/test_verify_playbook_params.py @@ -357,7 +357,8 @@ def test_verify_playbook_params_00070() -> None: instance.config_playbook = payloads_verify_playbook_params(key) instance.config_controller = None match = r"The following parameter\(value\) combination\(s\) are invalid\s+" - match += r"and need to be reviewed: Fabric: f1,\s+REPLICATION_MODE" + match += r"and need to be reviewed: " + match += r"Fabric: f1,\s+V6_SUBNET_RANGE" with pytest.raises(ValueError, match=match): instance.commit() @@ -392,7 +393,8 @@ def test_verify_playbook_params_00080() -> None: instance.config_playbook = payloads_verify_playbook_params(key) instance.config_controller = nv_pairs_verify_playbook_params(key) match = r"The following parameter\(value\) combination\(s\) are invalid\s+" - match += r"and need to be reviewed: Fabric: f1,\s+REPLICATION_MODE" + match += r"and need to be reviewed: " + match += r"Fabric: f1,\s+V6_SUBNET_RANGE" with pytest.raises(ValueError, match=match): instance.commit()