From 0ff97cb5e1f9c0ac75975747a17344903d4b9ecd Mon Sep 17 00:00:00 2001 From: Christian Siegel Date: Wed, 13 May 2020 14:01:34 +0200 Subject: [PATCH 1/3] refactor: remove unused 'create_new' arg from 'update_yaml_file()' --- gitopscli/yaml/yaml_util.py | 8 +++---- tests/yaml/test_yaml_util.py | 42 ------------------------------------ 2 files changed, 3 insertions(+), 47 deletions(-) diff --git a/gitopscli/yaml/yaml_util.py b/gitopscli/yaml/yaml_util.py index 21d324cf..078bf751 100644 --- a/gitopscli/yaml/yaml_util.py +++ b/gitopscli/yaml/yaml_util.py @@ -20,7 +20,7 @@ def get_string(self): return stream.get_string() -def update_yaml_file(file_path, key, value, create_new=False): +def update_yaml_file(file_path, key, value): yaml = YAML() with open(file_path, "r") as stream: content = yaml.load(stream) @@ -28,13 +28,11 @@ def update_yaml_file(file_path, key, value, create_new=False): keys, obj = key.split("."), content for k in keys[:-1]: if k not in obj or not isinstance(obj[k], dict): - if not create_new: - raise KeyError(f"Key '{key}' not found in YAML!") - obj[k] = dict() + raise KeyError(f"Key '{key}' not found in YAML!") obj = obj[k] if keys[-1] in obj and obj[keys[-1]] == value: return False # nothing to update - if not create_new and keys[-1] not in obj: + if keys[-1] not in obj: raise KeyError(f"Key '{key}' not found in YAML!") obj[keys[-1]] = value diff --git a/tests/yaml/test_yaml_util.py b/tests/yaml/test_yaml_util.py index 11c02886..01e71c56 100644 --- a/tests/yaml/test_yaml_util.py +++ b/tests/yaml/test_yaml_util.py @@ -72,48 +72,6 @@ def test_update_yaml_file(self): with pytest.raises(KeyError): updated = update_yaml_file(test_file, "a.x", "foo") - updated = update_yaml_file(test_file, "a.x", "foo", create_new=True) - self.assertTrue(updated) - - expected = """\ -a: # comment -# comment - b: - d: 1 # comment - c: '2' # comment - x: foo -""" - actual = self._read_file(test_file) - self.assertEqual(expected, actual) - - with pytest.raises(KeyError): - updated = update_yaml_file(test_file, "a.x.z", "foo_z") - updated = update_yaml_file(test_file, "a.x.z", "foo_z", create_new=True) - self.assertTrue(updated) - - with pytest.raises(KeyError): - updated = update_yaml_file(test_file, "a.x.y", "foo_y") - updated = update_yaml_file(test_file, "a.x.y", "foo_y", create_new=True) - self.assertTrue(updated) - - with pytest.raises(KeyError): - updated = update_yaml_file(test_file, "a.x.y.z", "foo_y_z") - updated = update_yaml_file(test_file, "a.x.y.z", "foo_y_z", create_new=True) - self.assertTrue(updated) - - expected = """\ -a: # comment -# comment - b: - d: 1 # comment - c: '2' # comment - x: - z: foo_z - y: - z: foo_y_z -""" - actual = self._read_file(test_file) - self.assertEqual(expected, actual) def test_merge_yaml_element(self): test_file = self._create_file( From 9cb5bb2499958d573496d863d13e752e9fb80c2c Mon Sep 17 00:00:00 2001 From: Christian Siegel Date: Wed, 13 May 2020 14:10:32 +0200 Subject: [PATCH 2/3] feat(deploy): add support for updating values in YAML arrays Support YAML paths containing array indexes, e.g. `a.b.[1].c` for `{a: {b: [foo, {c: bar}]}}` --- docs/commands/deploy.md | 14 ++++++-- gitopscli/yaml/yaml_util.py | 34 ++++++++++++------ tests/yaml/test_yaml_util.py | 69 ++++++++++++++++++++++++++++-------- 3 files changed, 91 insertions(+), 26 deletions(-) diff --git a/docs/commands/deploy.md b/docs/commands/deploy.md index 79904aab..4a76cc3f 100644 --- a/docs/commands/deploy.md +++ b/docs/commands/deploy.md @@ -13,6 +13,9 @@ frontend: backend: repository: my-app/backend tag: 1.0.0 # <- and this one + env: + - name: TEST + value: foo # <- and even one in a list ``` With the following command GitOps CLI will update both values to `1.1.0` on the `master` branch. @@ -27,7 +30,7 @@ gitopscli deploy \ --organisation "deployment" \ --repository-name "myapp-non-prod" \ --file "example/values.yaml" \ - --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0}" + --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0, backend.env.[0].value: bar}" ``` ### Number Of Commits @@ -35,6 +38,12 @@ gitopscli deploy \ Note that by default GitOps CLI will create a separate commit for every value change: ``` +commit 0dcaa136b4c5249576bb1f40b942bff6ac718144 +Author: GitOpsCLI +Date: Thu Mar 12 15:30:32 2020 +0100 + + changed 'backend.env.[0].value' to 'bar' in example/values.yaml + commit d98913ad8fecf571d5f8c3635f8070b05c43a9ca Author: GitOpsCLI Date: Thu Mar 12 15:30:32 2020 +0100 @@ -59,6 +68,7 @@ Date: Thu Mar 12 15:30:00 2020 +0100 frontend.tag: '1.1.0' backend.tag: '1.1.0' + backend.env.[0].value: 'bar' ``` ### Create Pull Request @@ -75,7 +85,7 @@ gitopscli deploy \ --organisation "deployment" \ --repository-name "myapp-non-prod" \ --file "example/values.yaml" \ - --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0}" \ + --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0, backend.env.[0].value: bar}" \ --create-pr \ --auto-merge ``` diff --git a/gitopscli/yaml/yaml_util.py b/gitopscli/yaml/yaml_util.py index 078bf751..f50ad999 100644 --- a/gitopscli/yaml/yaml_util.py +++ b/gitopscli/yaml/yaml_util.py @@ -1,5 +1,8 @@ +import re from ruamel.yaml import YAML +INDEX_PATTERN = re.compile(r"\[(\d+)\]") + def yaml_load(doc): return YAML().load(doc) @@ -25,16 +28,27 @@ def update_yaml_file(file_path, key, value): with open(file_path, "r") as stream: content = yaml.load(stream) - keys, obj = key.split("."), content - for k in keys[:-1]: - if k not in obj or not isinstance(obj[k], dict): - raise KeyError(f"Key '{key}' not found in YAML!") - obj = obj[k] - if keys[-1] in obj and obj[keys[-1]] == value: - return False # nothing to update - if keys[-1] not in obj: - raise KeyError(f"Key '{key}' not found in YAML!") - obj[keys[-1]] = value + keys, item = key.split("."), content + leaf_idx = len(keys) - 1 + current_key_segments = [] + current_key = "" + for i, k in enumerate(keys): + current_key_segments.append(k) + current_key = ".".join(current_key_segments) + is_array = INDEX_PATTERN.match(k) + if is_array: + k = int(is_array.group(1)) + if not isinstance(item, list) or k >= len(item): + raise KeyError(f"Key '{current_key}' not found in YAML!") + else: + if not isinstance(item, dict) or k not in item: + raise KeyError(f"Key '{current_key}' not found in YAML!") + if i == leaf_idx: + if item[k] == value: + return False # nothing to update + item[k] = value + break + item = item[k] with open(file_path, "w+") as stream: yaml.dump(content, stream) diff --git a/tests/yaml/test_yaml_util.py b/tests/yaml/test_yaml_util.py index 01e71c56..22c456d4 100644 --- a/tests/yaml/test_yaml_util.py +++ b/tests/yaml/test_yaml_util.py @@ -47,31 +47,72 @@ def test_yaml_dump(self): def test_update_yaml_file(self): test_file = self._create_file( """\ -a: # comment -# comment +a: # comment 1 +# comment 2 b: - d: 1 # comment - c: 2 # comment""" + d: 1 # comment 3 + c: 2 # comment 4 + e: + - f: 3 # comment 5 + g: 4 # comment 6 + - [hello, world] # comment 7 + - foo: # comment 8 + bar # comment 9""" ) - updated = update_yaml_file(test_file, "a.b.c", "2") - self.assertTrue(updated) + self.assertTrue(update_yaml_file(test_file, "a.b.c", "2")) + self.assertFalse(update_yaml_file(test_file, "a.b.c", "2")) # already updated - updated = update_yaml_file(test_file, "a.b.c", "2") - self.assertFalse(updated) # already updated + self.assertTrue(update_yaml_file(test_file, "a.e.[0].g", 42)) + self.assertFalse(update_yaml_file(test_file, "a.e.[0].g", 42)) # already updated + + self.assertTrue(update_yaml_file(test_file, "a.e.[1].[1]", "tester")) + self.assertFalse(update_yaml_file(test_file, "a.e.[1].[1]", "tester")) # already updated + + self.assertTrue(update_yaml_file(test_file, "a.e.[2]", "replaced object")) + self.assertFalse(update_yaml_file(test_file, "a.e.[2]", "replaced object")) # already updated expected = """\ -a: # comment -# comment +a: # comment 1 +# comment 2 b: - d: 1 # comment - c: '2' # comment + d: 1 # comment 3 + c: '2' # comment 4 + e: + - f: 3 # comment 5 + g: 42 # comment 6 + - [hello, tester] # comment 7 + - replaced object """ actual = self._read_file(test_file) self.assertEqual(expected, actual) - with pytest.raises(KeyError): - updated = update_yaml_file(test_file, "a.x", "foo") + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "x.y", "foo") + self.assertEqual("\"Key 'x' not found in YAML!\"", str(ex.value)) + + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "[42].y", "foo") + self.assertEqual("\"Key '[42]' not found in YAML!\"", str(ex.value)) + + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "a.x", "foo") + self.assertEqual("\"Key 'a.x' not found in YAML!\"", str(ex.value)) + + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "a.[42]", "foo") + self.assertEqual("\"Key 'a.[42]' not found in YAML!\"", str(ex.value)) + + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "a.e.[3]", "foo") + self.assertEqual("\"Key 'a.e.[3]' not found in YAML!\"", str(ex.value)) + + with pytest.raises(KeyError) as ex: + update_yaml_file(test_file, "a.e.[2].[2]", "foo") + self.assertEqual("\"Key 'a.e.[2].[2]' not found in YAML!\"", str(ex.value)) + + actual = self._read_file(test_file) + self.assertEqual(expected, actual) def test_merge_yaml_element(self): test_file = self._create_file( From 9c0546d98bd47486683e99f79a6df5b66ffe933b Mon Sep 17 00:00:00 2001 From: Christian Siegel Date: Thu, 14 May 2020 08:25:23 +0200 Subject: [PATCH 3/3] refactor: improve variable names in 'update_yaml_file()' --- gitopscli/yaml/yaml_util.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/gitopscli/yaml/yaml_util.py b/gitopscli/yaml/yaml_util.py index f50ad999..abbef2a7 100644 --- a/gitopscli/yaml/yaml_util.py +++ b/gitopscli/yaml/yaml_util.py @@ -1,7 +1,7 @@ import re from ruamel.yaml import YAML -INDEX_PATTERN = re.compile(r"\[(\d+)\]") +ARRAY_KEY_SEGMENT_PATTERN = re.compile(r"\[(\d+)\]") def yaml_load(doc): @@ -28,31 +28,29 @@ def update_yaml_file(file_path, key, value): with open(file_path, "r") as stream: content = yaml.load(stream) - keys, item = key.split("."), content - leaf_idx = len(keys) - 1 + key_segments = key.split(".") current_key_segments = [] - current_key = "" - for i, k in enumerate(keys): - current_key_segments.append(k) + parent_item = content + for current_key_segment in key_segments: + current_key_segments.append(current_key_segment) current_key = ".".join(current_key_segments) - is_array = INDEX_PATTERN.match(k) + is_array = ARRAY_KEY_SEGMENT_PATTERN.match(current_key_segment) if is_array: - k = int(is_array.group(1)) - if not isinstance(item, list) or k >= len(item): + current_key_segment = int(is_array.group(1)) + if not isinstance(parent_item, list) or current_key_segment >= len(parent_item): raise KeyError(f"Key '{current_key}' not found in YAML!") else: - if not isinstance(item, dict) or k not in item: + if not isinstance(parent_item, dict) or current_key_segment not in parent_item: raise KeyError(f"Key '{current_key}' not found in YAML!") - if i == leaf_idx: - if item[k] == value: + if current_key == key: + if parent_item[current_key_segment] == value: return False # nothing to update - item[k] = value - break - item = item[k] - - with open(file_path, "w+") as stream: - yaml.dump(content, stream) - return True + parent_item[current_key_segment] = value + with open(file_path, "w+") as stream: + yaml.dump(content, stream) + return True + parent_item = parent_item[current_key_segment] + raise KeyError(f"Empty key!") def merge_yaml_element(file_path, element_path, desired_value, delete_missing_key=False):