From afabda96dd8b6022549555c124a523bb51d9cfc0 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Tue, 22 Aug 2023 13:27:33 -0700 Subject: [PATCH] Ensuring we throw an error on bad YAML files (#1358) The goal is to throw an error when there is a duplicate key in the YAML file. Though if the key is heavily nested, deep in the YAML file, sometimes our YAML libraries will miss it. --- armi/nucDirectory/nuclideBases.py | 1 + .../blueprints/tests/test_blueprints.py | 19 +++++++++++++++++++ armi/reactor/systemLayoutInput.py | 1 + armi/settings/caseSettings.py | 5 +++-- armi/settings/settingsIO.py | 1 + armi/settings/tests/test_settingsIO.py | 15 +++++++++++++++ 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/armi/nucDirectory/nuclideBases.py b/armi/nucDirectory/nuclideBases.py index 3cc9427a3..6a2143ba5 100644 --- a/armi/nucDirectory/nuclideBases.py +++ b/armi/nucDirectory/nuclideBases.py @@ -1082,6 +1082,7 @@ def imposeBurnChain(burnChainStream): return burnChainImposed = True yaml = YAML(typ="rt") + yaml.allow_duplicate_keys = False burnData = yaml.load(burnChainStream) for nucName, burnInfo in burnData.items(): diff --git a/armi/reactor/blueprints/tests/test_blueprints.py b/armi/reactor/blueprints/tests/test_blueprints.py index 9428e2c21..81a54dfac 100644 --- a/armi/reactor/blueprints/tests/test_blueprints.py +++ b/armi/reactor/blueprints/tests/test_blueprints.py @@ -199,6 +199,25 @@ class TestBlueprintsSchema(unittest.TestCase): 2 2 2 2 2 """ + def test_noDuplicateKeysInYamlBlueprints(self): + """ + Prove that if you duplicate a section of a YAML blueprint file, + a hard error will be thrown. + """ + # loop through a few different sections, to test blueprints broadly + sections = ["blocks:", "components:", "component groups:"] + for sectionName in sections: + # modify blueprint YAML to duplicate this section + yamlString = str(self._yamlString) + i = yamlString.find(sectionName) + lenSection = yamlString[i:].find("\n\n") + section = yamlString[i : i + lenSection] + yamlString = yamlString[:i] + section + yamlString[i : i + lenSection] + + # validate that this is now an invalid YAML blueprint + with self.assertRaises(Exception): + _design = blueprints.Blueprints.load(yamlString) + def test_assemblyParameters(self): cs = settings.Settings() design = blueprints.Blueprints.load(self._yamlString) diff --git a/armi/reactor/systemLayoutInput.py b/armi/reactor/systemLayoutInput.py index 73cfd890b..cac411c7f 100644 --- a/armi/reactor/systemLayoutInput.py +++ b/armi/reactor/systemLayoutInput.py @@ -275,6 +275,7 @@ def _readYaml(self, stream): consistent inputs. """ yaml = YAML() + yaml.allow_duplicate_keys = False tree = yaml.load(stream) tree = INPUT_SCHEMA(tree) self.assemTypeByIndices.clear() diff --git a/armi/settings/caseSettings.py b/armi/settings/caseSettings.py index d5ab87fcc..d94f634a1 100644 --- a/armi/settings/caseSettings.py +++ b/armi/settings/caseSettings.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -r""" +""" This defines a Settings object that acts mostly like a dictionary. It is meant to be treated mostly like a singleton, where each custom ARMI object has access to it. It contains global user settings like the core @@ -22,7 +22,7 @@ A settings object can be saved as or loaded from an YAML file. The ARMI GUI is designed to create this settings file, which is then loaded by an ARMI process on the cluster. -A primary case settings is created as ``masterCs`` +A primary case settings is created as ``masterCs``. """ import io import logging @@ -401,6 +401,7 @@ def getSettingsSetByUser(self, fPath): # from the settings file to know which settings are user-defined with open(fPath, "r") as stream: yaml = YAML() + yaml.allow_duplicate_keys = False tree = yaml.load(stream) userSettings = tree[settingsIO.Roots.CUSTOM] diff --git a/armi/settings/settingsIO.py b/armi/settings/settingsIO.py index 9fa701beb..475a61352 100644 --- a/armi/settings/settingsIO.py +++ b/armi/settings/settingsIO.py @@ -208,6 +208,7 @@ def _readYaml(self, stream): from armi.settings.fwSettings.globalSettings import CONF_VERSIONS yaml = YAML(typ="rt") + yaml.allow_duplicate_keys = False tree = yaml.load(stream) if "settings" not in tree: raise InvalidSettingsFileError( diff --git a/armi/settings/tests/test_settingsIO.py b/armi/settings/tests/test_settingsIO.py index dfdc3ec3e..d9054db31 100644 --- a/armi/settings/tests/test_settingsIO.py +++ b/armi/settings/tests/test_settingsIO.py @@ -22,6 +22,7 @@ from armi.cli import entryPoint from armi.settings import setting from armi.settings import settingsIO +from armi.tests import TEST_ROOT from armi.utils import directoryChangers from armi.utils.customExceptions import ( InvalidSettingsFileError, @@ -68,6 +69,20 @@ def test_basicSettingsReader(self): self.assertFalse(getattr(reader, "filelessBP")) self.assertEqual(getattr(reader, "path"), "") + def test_readFromFile(self): + with directoryChangers.TemporaryDirectoryChanger(): + inPath = os.path.join(TEST_ROOT, "armiRun.yaml") + outPath = "test_readFromFile.yaml" + + txt = open(inPath, "r").read() + verb = "branchVerbosity:" + txt0, txt1 = txt.split(verb) + newTxt = f"{txt0}{verb} fake\n {verb}{txt1}" + open(outPath, "w").write(newTxt) + + with self.assertRaises(InvalidSettingsFileError): + settings.caseSettings.Settings(outPath) + class SettingsRenameTests(unittest.TestCase): testSettings = [