diff --git a/src/python/Utils/Utilities.py b/src/python/Utils/Utilities.py index 78f8be9b99..b8f257e020 100644 --- a/src/python/Utils/Utilities.py +++ b/src/python/Utils/Utilities.py @@ -33,8 +33,6 @@ def makeList(stringList): throws a ValueError if the input is not well formed. If the stringList is already of type list, then return it untouched. """ - # NOTE: In case of a change to this function beware of side effects which - # might affect the behavior of WMCore.WMSpec.WMWorkloadTools.validateSiteLists if isinstance(stringList, list): return stringList if isinstance(stringList, str): diff --git a/src/python/WMCore/WMSpec/WMWorkload.py b/src/python/WMCore/WMSpec/WMWorkload.py index 6b67dc0a34..0f7e514b61 100644 --- a/src/python/WMCore/WMSpec/WMWorkload.py +++ b/src/python/WMCore/WMSpec/WMWorkload.py @@ -13,14 +13,16 @@ import inspect -from Utils.Utilities import strToBool +from Utils.Utilities import strToBool, makeList from WMCore.Configuration import ConfigSection from WMCore.Lexicon import sanitizeURL from WMCore.WMException import WMException +from WMCore.WMSpec.WMSpecErrors import WMSpecFactoryException from WMCore.WMSpec.ConfigSectionTree import findTop from WMCore.WMSpec.Persistency import PersistencyHelper from WMCore.WMSpec.WMTask import WMTask, WMTaskHelper -from WMCore.WMSpec.WMWorkloadTools import (validateArgumentsUpdate, loadSpecClassByType, +from WMCore.WMSpec.WMWorkloadTools import (validateArgumentsUpdate, validateUnknownArgs, validateSiteLists, + _validateArgumentOptions, loadSpecClassByType, setAssignArgumentsWithDefault) parseTaskPath = lambda p: [x for x in p.split('/') if x.strip() != ''] @@ -2053,14 +2055,43 @@ def validateArgumentForAssignment(self, schema): argumentDefinition = specClass.getWorkloadAssignArgs() validateArgumentsUpdate(schema, argumentDefinition) return + def validateSiteListsUpdate(self, arguments): + """ + Validate a dictionary of workflow arguments for possible Site lists update + It must catch any eventual conflict between the currently existing site lists + and the ones provided by the user, presuming that the change would happen by + fully substituting an already existing site list at the workflow if provided + with the arguments here. + """ + # NOTE: We have 3 different use cases to validate for siteLists conflicts: + # * A change to both SiteWhitelist and SiteBlacklist + # * A change only to SiteWhitelist + # * A change only to SiteBlacklist + if "SiteWhitelist" in arguments and "SiteBlacklist" in arguments: + validateSiteLists(arguments) + return + fullSiteWhitelist = set() + fullSiteBlacklist = set() + if "SiteWhitelist" in arguments and "SiteBlacklist" not in arguments: + fullSiteWhitelist = set(makeList(arguments["SiteWhitelist"])) + fullSiteBlacklist = set(self.getSiteBlacklist()) + if "SiteBlacklist" in arguments and "SiteWhitelist" not in arguments: + fullSiteWhitelist = set(self.getSiteWhitelist()) + fullSiteBlacklist = set(makeList(arguments["SiteBlacklist"])) + siteConflicts = fullSiteWhitelist & fullSiteBlacklist + if siteConflicts: + msg = "Validation of Site Lists for update failed due to conflicts with existing Site Lists." + msg += f"Site can only be black listed or whitelisted. Conflicting sites: {siteConflicts}" + raise WMSpecFactoryException(msg) + return - def validateArgumentsPartialUpdate(self, schema): + def validateArgumentsPartialUpdate(self, arguments): """ Validates the provided parameters schema for workflow arguments update, using the arguments definitions for assignment as provided at StdBase.getWorkloadAssignArgs - :param schema: Workflow arguments schema to be validated - Must be a properly - defined dictionary of {arg: value} pairs. - :return: Nothing. Raises proper exceptions when argument validation fails + :param arguments: Workflow arguments schema to be validated - Must be a properly + defined dictionary of {arg: value} pairs. + :return: Nothing. Raises proper exceptions when argument validation fails NOTE: In order to avoid full schema validation and enforcing mandatory arguments, we set the optionKey argument for this call to NONE. This way it is ignored @@ -2071,7 +2102,9 @@ def validateArgumentsPartialUpdate(self, schema): """ specClass = loadSpecClassByType(self.getRequestType()) argumentDefinition = specClass.getWorkloadAssignArgs() - validateArgumentsUpdate(schema, argumentDefinition, optionKey=None) + validateUnknownArgs(arguments, argumentDefinition) + _validateArgumentOptions(arguments, argumentDefinition, optionKey=None) + self.validateSiteListsUpdate(arguments) return def updateArguments(self, kwargs): diff --git a/src/python/WMCore/WMSpec/WMWorkloadTools.py b/src/python/WMCore/WMSpec/WMWorkloadTools.py index 12edb3ccba..d018779d81 100644 --- a/src/python/WMCore/WMSpec/WMWorkloadTools.py +++ b/src/python/WMCore/WMSpec/WMWorkloadTools.py @@ -254,10 +254,8 @@ def validateSiteLists(arguments): msg = "Validation failed: The same site cannot be white and blacklisted: %s" % list(res) raise WMSpecFactoryException(msg) # store the properly formatted values (list instead of string) - if whiteList: - arguments["SiteWhitelist"] = whiteList - if blackList: - arguments["SiteBlacklist"] = blackList + arguments["SiteWhitelist"] = whiteList + arguments["SiteBlacklist"] = blackList return