From 31049d83683fd746f34e9cdbd000e01a39b61f2f Mon Sep 17 00:00:00 2001 From: Mike Hendricks Date: Mon, 14 Nov 2022 15:15:53 -0800 Subject: [PATCH] Standardize site configuration resolution of multiple files - Dumping site platform_path_maps now sorts the keys alphabetically - Correct issues with left wins setup for using multiple site json files and make the tests for this easy to understand by using left/middle/right terminology. --- README.md | 56 ++++++++++-- hab/site.py | 2 +- hab/utils.py | 2 +- tests/site/site_left.json | 25 ++++++ tests/site/site_middle.json | 25 ++++++ tests/site/site_right.json | 25 ++++++ tests/test_parsing.py | 10 +-- tests/test_site.py | 169 +++++++++++++++++++++++++++++++++++- 8 files changed, 298 insertions(+), 16 deletions(-) create mode 100644 tests/site/site_left.json create mode 100644 tests/site/site_middle.json create mode 100644 tests/site/site_right.json diff --git a/README.md b/README.md index e56a0d8..c1ed083 100644 --- a/README.md +++ b/README.md @@ -122,16 +122,62 @@ configuration files. If the `--site` option is passed to the cli, it is used ins the environment variable. Each of the file paths specified are read and merged into a single site configuration -dictionary hab uses. The values in each file are merged so the value defined in the -right most path any given configuration option being used. See -[Defining Environments](#defining-environments) for how to structure the json to -prepend, append, set, unset values. +dictionary hab uses. When using multiple site json files here are some general +rules to keep in mind. + +1. The left most site configuration takes precedence for a given item. +2. For prepend/append operations on lists, the left site file's paths will placed +on the outside of the the right site file's paths. +3. For `platform_path_maps`, only the first key is kept and any duplicates +are discarded. + +See [Defining Environments](#defining-environments) for how to structure the json +to prepend, append, set, unset values. Developers can use this to load local site configurations loading their wip code -instead of the official releases. See the [TestResolvePaths::test_paths](tests/test_site.py) +instead of the official releases. See [TestResolvePaths::test_paths](tests/test_site.py) to see an example of [overriding](tests/site_override.json) the [main](tests/site_main.json) site settings. +You can inspect the site settings by using the `hab dump -t s` or +`hab dump --type site` cli command. See +[TestMultipleSites::test_left_right and TestMultipleSites::test_right_left](tests/test_site.py) +for an example of how these rules are applied. Here is a dump of the final result +of using all [3 site json files](tests/site). +```bash +$ cd tests/site +$ hab --site site_left.json --site site_middle.json --site site_right.json dump --type site -v +Dump of Site +------------------------------------------------------------------- +HAB_PATHS: C:\blur\dev\hab_\tests\site\site_left.json + C:\blur\dev\hab_\tests\site\site_middle.json + C:\blur\dev\hab_\tests\site\site_right.json +config_paths: +distro_paths: +ignored_distros: release, pre +platforms: windows, mac, linux +set_value: left +test_paths: left_prepend + middle_prepend + right_prepend + right_append + middle_append + left_append +platform_path_maps: host: linux: host-linux_left + windows: host-windows_left + mid: linux: mid-linux_middle + windows: mid-windows_middle + net: linux: net-linux_right + windows: net-windows_right + shared: linux: shared-linux_left + windows: shared-windows_left +------------------------------------------------------------------- +``` +Note the order of left/middle/right in the test_paths variable. Also, for +`platform_path_maps`, `host` is defined in all 3 site files, but only the first +site file with it defined is used. The other path maps are picked up from the +site file they are defined in. + ### Python version Hab uses shell script files instead of an entry_point executable. This allows it diff --git a/hab/site.py b/hab/site.py index 14b9344..e1a2e51 100644 --- a/hab/site.py +++ b/hab/site.py @@ -87,7 +87,7 @@ def load(self): """Iterates over each file in self.path. Replacing the value of each key. The last file in the list will have its settings applied even if other files define them.""" - for path in self.paths: + for path in reversed(self.paths): self.load_file(path) # Ensure any platform_path_maps are converted to pathlib objects. diff --git a/hab/utils.py b/hab/utils.py index ccd9047..b52a6a7 100644 --- a/hab/utils.py +++ b/hab/utils.py @@ -122,7 +122,7 @@ def dump_object(obj, label="", width=80, flat_list=False, color=False): elif isinstance(obj, (dict, UserDict)): rows = [] lbl = label - for k, v in obj.items(): + for k, v in sorted(obj.items()): rows.append( dump_object( v, diff --git a/tests/site/site_left.json b/tests/site/site_left.json new file mode 100644 index 0000000..bd572a3 --- /dev/null +++ b/tests/site/site_left.json @@ -0,0 +1,25 @@ +{ + "prepend": { + "test_paths": [ + "left_prepend" + ] + }, + "append": { + "test_paths": [ + "left_append" + ], + "platform_path_maps": { + "host": { + "linux": "host-linux_left", + "windows": "host-windows_left" + }, + "shared": { + "linux": "shared-linux_left", + "windows": "shared-windows_left" + } + } + }, + "set": { + "set_value": "left" + } +} diff --git a/tests/site/site_middle.json b/tests/site/site_middle.json new file mode 100644 index 0000000..d91a8d8 --- /dev/null +++ b/tests/site/site_middle.json @@ -0,0 +1,25 @@ +{ + "prepend": { + "test_paths": [ + "middle_prepend" + ] + }, + "append": { + "test_paths": [ + "middle_append" + ], + "platform_path_maps": { + "host": { + "linux": "host-linux_middle", + "windows": "host-windows_middle" + }, + "mid": { + "linux": "mid-linux_middle", + "windows": "mid-windows_middle" + } + } + }, + "set": { + "set_value": "middle" + } +} diff --git a/tests/site/site_right.json b/tests/site/site_right.json new file mode 100644 index 0000000..89f212d --- /dev/null +++ b/tests/site/site_right.json @@ -0,0 +1,25 @@ +{ + "prepend": { + "test_paths": [ + "right_prepend" + ] + }, + "append": { + "test_paths": [ + "right_append" + ], + "platform_path_maps": { + "host": { + "linux": "host-linux_right", + "windows": "host-windows_right" + }, + "net": { + "linux": "net-linux_right", + "windows": "net-windows_right" + } + } + }, + "set": { + "set_value": "right" + } +} diff --git a/tests/test_parsing.py b/tests/test_parsing.py index d90d0a4..5c3e5ff 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -218,15 +218,15 @@ def standardize(txt): pre = ["name: child", "uri: not_set/child"] post = ["inherits: True"] env = [ - "environment: UNSET_VARIABLE: None", + "environment: FMT_FOR_OS: a{;}b;c:{PATH!e}{;}d", " TEST: case", - " FMT_FOR_OS: a{;}b;c:{PATH!e}{;}d", + " UNSET_VARIABLE: None", ] env_config = [ - "environment_config: unset: UNSET_VARIABLE", - " set: TEST: case", - " FMT_FOR_OS: a{;}b;c:{PATH!e}{;}d", + "environment_config: set: FMT_FOR_OS: a{;}b;c:{PATH!e}{;}d", + " TEST: case", + " unset: UNSET_VARIABLE", ] cfg = resolver.closest_config("not_set/child") header = f"Dump of {type(cfg).__name__}('{cfg.fullpath}')" diff --git a/tests/test_site.py b/tests/test_site.py index 81bfa84..151982b 100644 --- a/tests/test_site.py +++ b/tests/test_site.py @@ -19,6 +19,167 @@ def test_environment_variables(config_root, monkeypatch): assert site.paths == paths +class TestMultipleSites: + """Check that various combinations of site json files results in the correct + merged site. The rules are: + + 1. The left most site configuration takes precedence for a given item. + 2. For prepend/append operations on lists, the left site file's paths will + placed on the outside of the the right site file's paths. + 3. For `platform_path_maps`, only the first key is kept and any duplicates + are discarded. + """ + + def host_left(self): + return { + 'linux': PurePosixPath('host-linux_left'), + 'windows': PureWindowsPath('host-windows_left'), + } + + def host_middle(self): + return { + 'linux': PurePosixPath('host-linux_middle'), + 'windows': PureWindowsPath('host-windows_middle'), + } + + def host_right(self): + return { + 'linux': PurePosixPath('host-linux_right'), + 'windows': PureWindowsPath('host-windows_right'), + } + + def mid(self): + return { + 'linux': PurePosixPath('mid-linux_middle'), + 'windows': PureWindowsPath('mid-windows_middle'), + } + + def net(self): + return { + 'linux': PurePosixPath('net-linux_right'), + 'windows': PureWindowsPath('net-windows_right'), + } + + def shared_left(self): + return { + 'linux': PurePosixPath('shared-linux_left'), + 'windows': PureWindowsPath('shared-windows_left'), + } + + def test_left(self, config_root): + """Check that site_left.json returns the correct results.""" + paths = [config_root / "site" / "site_left.json"] + site = Site(paths) + + assert site.get("set_value") == ["left"] + assert site.get("test_paths") == ["left_prepend", "left_append"] + + check = {'host': self.host_left(), 'shared': self.shared_left()} + + assert site.get("platform_path_maps") == check + + def test_middle(self, config_root): + """Check that site_middle.json returns the correct results.""" + paths = [config_root / "site" / "site_middle.json"] + site = Site(paths) + + assert site.get("set_value") == ["middle"] + assert site.get("test_paths") == ["middle_prepend", "middle_append"] + + check = {'host': self.host_middle(), 'mid': self.mid()} + + assert site.get("platform_path_maps") == check + + def test_right(self, config_root): + """Check that site_right.json returns the correct results.""" + paths = [config_root / "site" / "site_right.json"] + site = Site(paths) + + assert site.get("set_value") == ["right"] + assert site.get("test_paths") == ["right_prepend", "right_append"] + + check = {'host': self.host_right(), 'net': self.net()} + + assert site.get("platform_path_maps") == check + + def test_left_right(self, config_root): + """Check that site_left.json and site_right.json are merged correctly.""" + paths = [ + config_root / "site" / "site_left.json", + config_root / "site" / "site_right.json", + ] + site = Site(paths) + + assert site.get("set_value") == ["left"] + assert site.get("test_paths") == [ + "left_prepend", + "right_prepend", + "right_append", + "left_append", + ] + + check = { + 'host': self.host_left(), + 'net': self.net(), + 'shared': self.shared_left(), + } + + assert site.get("platform_path_maps") == check + + def test_right_left(self, config_root): + """Check that reversing site_left.json and site_right.json get merged + correctly.""" + paths = [ + config_root / "site" / "site_right.json", + config_root / "site" / "site_left.json", + ] + site = Site(paths) + + assert site.get("set_value") == ["right"] + assert site.get("test_paths") == [ + "right_prepend", + "left_prepend", + "left_append", + "right_append", + ] + + check = { + 'host': self.host_right(), + 'net': self.net(), + 'shared': self.shared_left(), + } + + assert site.get("platform_path_maps") == check + + def test_left_middle_right(self, config_root): + """Check that more than 2 site json files are merged correctly.""" + paths = [ + config_root / "site" / "site_left.json", + config_root / "site" / "site_middle.json", + config_root / "site" / "site_right.json", + ] + site = Site(paths) + + assert site.get("set_value") == ["left"] + assert site.get("test_paths") == [ + "left_prepend", + "middle_prepend", + "right_prepend", + "right_append", + "middle_append", + "left_append", + ] + + check = { + 'host': self.host_left(), + 'mid': self.mid(), + 'net': self.net(), + 'shared': self.shared_left(), + } + + assert site.get("platform_path_maps") == check + + class TestResolvePaths: def test_path(self, config_root): # Check that the correct values are resolved when processing a single result @@ -30,7 +191,7 @@ def test_path(self, config_root): def test_paths(self, config_root, helpers): # Check that values specified by additional files overwrite the previous values - paths = [config_root / "site_main.json", config_root / "site_override.json"] + paths = [config_root / "site_override.json", config_root / "site_main.json"] site = Site(paths) assert site.get("generic_value") is True assert site.get("override") == ["site_override.json"] @@ -53,7 +214,7 @@ def test_paths(self, config_root, helpers): def test_paths_reversed(self, config_root, helpers): # Check that values specified by additional files overwrite the previous values - paths = [config_root / "site_override.json", config_root / "site_main.json"] + paths = [config_root / "site_main.json", config_root / "site_override.json"] site = Site(paths) assert site.get("generic_value") is False assert site.get("override") == ["site_override.json"] @@ -276,7 +437,7 @@ def test_merged(self, config_root): ) # The right-most site file's settings are loaded when both have the same keys - self.assert_override(site) + self.assert_main(site) def test_reversed(self, config_root): """Test that the correct platform_path_map settings are resolved when using @@ -296,4 +457,4 @@ def test_reversed(self, config_root): ) # The right-most site file's settings are loaded when both have the same keys - self.assert_main(site) + self.assert_override(site)