From 137054cfcb91375a6e0142697d1e5fa9b128ece7 Mon Sep 17 00:00:00 2001 From: Matt Herrick <142412263+duplocloud-matt@users.noreply.github.com> Date: Mon, 7 Oct 2024 09:53:11 -0500 Subject: [PATCH 1/5] detecting merge on empty env and erroring --- src/duplo_resource/service.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/duplo_resource/service.py b/src/duplo_resource/service.py index a99b649..5074c47 100644 --- a/src/duplo_resource/service.py +++ b/src/duplo_resource/service.py @@ -237,6 +237,11 @@ def update_env(self, currentDockerconfig = loads(service["Template"]["OtherDockerConfig"]) currentEnv = currentDockerconfig.get("Env", []) + if currentEnv is None and strategy == "merge": + raise DuploError( + f"Cannot merge {name} service environment because it is empty, " + 'please use "--strategy replace" to set new environment variables' + ) newEnv = [] if setvar is not None: newEnv = [{"Name": i[0], "Value": i[1]} for i in setvar] From 14a25e06c63e1d2aeab7f9f0181fde5b43056097 Mon Sep 17 00:00:00 2001 From: Matt Herrick <142412263+duplocloud-matt@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:04:53 -0500 Subject: [PATCH 2/5] redoing logic for case handling in env vars, handling null envs, fixing arg syntax --- src/duplo_resource/service.py | 51 +++++++++++++++++++++++++++++------ src/duplocloud/args.py | 2 +- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/duplo_resource/service.py b/src/duplo_resource/service.py index 5074c47..67c5a7b 100644 --- a/src/duplo_resource/service.py +++ b/src/duplo_resource/service.py @@ -229,27 +229,62 @@ def update_env(self, ``` Args: name (str): The name of the service to update. + setvar/-V (list): A list of key value pairs to set as environment variables. + strategy/strat (str): The merge strategy to use for env vars. Valid options are "merge" or "replace". Default is merge. deletevar/-D (list): A list of keys to delete from the environment variables. """ - service = self.find(name) + # Returns an array of key and value mappings with provided keys + def new_env_vars(setvar, key_name="Name", value_name="Value"): + return [{key_name: i[0], value_name: i[1]} for i in setvar] + + def detect_case_format(env_list): + # Check if the environment variables are using upper, lower, or mixed case for Name and Value + if all('Name' in env and 'Value' in env for env in env_list): + return "title" + elif all('name' in env and 'value' in env for env in env_list): + return "lower" + # No consistent standard + return "mixed" + service = self.find(name) currentDockerconfig = loads(service["Template"]["OtherDockerConfig"]) currentEnv = currentDockerconfig.get("Env", []) + # Check if user is attempting to merge against a null Env. If so, set currentEnv to empty. if currentEnv is None and strategy == "merge": - raise DuploError( - f"Cannot merge {name} service environment because it is empty, " - 'please use "--strategy replace" to set new environment variables' - ) + self.duplo.logger.warn("Specified \"merge\" strategy on a service with" + " no environment variables defined, should use " + " \"replace\". Proceeding anyway") + currentEnv = [] newEnv = [] - if setvar is not None: - newEnv = [{"Name": i[0], "Value": i[1]} for i in setvar] + case_format = detect_case_format(currentEnv) if strategy == 'merge': - d = {d['Name']: d for d in currentEnv + newEnv} + # Attempt to merge with capital N Name key + try: + if case_format == "title": + if setvar is not None: + newEnv = new_env_vars(setvar) + d = {env['Name']: env for env in currentEnv + newEnv} + elif case_format == "lower": + if setvar is not None: + newEnv = new_env_vars(setvar, key_name="name", value_name="value") + d = {env['name']: env for env in currentEnv + newEnv} + else: + self.duplo.logger.warn("Possible attempt to merge env vars with" + " inconsistent Name/Value key case." + " Normalzing to capitalized" + " \"Name\" and \"Value\"") + norm_currentEnv = [{k.capitalize(): v for k, v in env.items()} for env in currentEnv] + if setvar is not None: + newEnv = new_env_vars(setvar) + d = {env['Name']: env for env in norm_currentEnv + newEnv} + except KeyError: + raise DuploError("Could not merge new and existing environment variables") mergedvars = list(d.values()) currentDockerconfig['Env'] = mergedvars else: + newEnv = new_env_vars(setvar) currentDockerconfig['Env'] = newEnv if deletevar is not None: for key in deletevar: diff --git a/src/duplocloud/args.py b/src/duplocloud/args.py index 63e2836..46175ba 100644 --- a/src/duplocloud/args.py +++ b/src/duplocloud/args.py @@ -173,7 +173,7 @@ nargs=2, metavar=('key', 'value')) -STRATEGY = Arg("-strategy", "-strat", +STRATEGY = Arg("strategy", "-strat", help='The merge strategy to use for env vars. Valid options are \"merge\" or \"replace\". Default is merge.', choices=['merge', 'replace'], default = 'merge') From 9a29d697a9634ee617be275828331ac55b7b2201 Mon Sep 17 00:00:00 2001 From: Matt Herrick <142412263+duplocloud-matt@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:21:35 -0500 Subject: [PATCH 3/5] cleaning up logic, fixing delete --- src/duplo_resource/service.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/duplo_resource/service.py b/src/duplo_resource/service.py index 67c5a7b..8947e6f 100644 --- a/src/duplo_resource/service.py +++ b/src/duplo_resource/service.py @@ -233,6 +233,7 @@ def update_env(self, setvar/-V (list): A list of key value pairs to set as environment variables. strategy/strat (str): The merge strategy to use for env vars. Valid options are "merge" or "replace". Default is merge. + deletevar/-D (list): A list of keys to delete from the environment variables. """ # Returns an array of key and value mappings with provided keys @@ -257,18 +258,14 @@ def detect_case_format(env_list): " no environment variables defined, should use " " \"replace\". Proceeding anyway") currentEnv = [] - newEnv = [] case_format = detect_case_format(currentEnv) if strategy == 'merge': - # Attempt to merge with capital N Name key try: if case_format == "title": - if setvar is not None: - newEnv = new_env_vars(setvar) + newEnv = new_env_vars(setvar) if setvar is not None else [] d = {env['Name']: env for env in currentEnv + newEnv} elif case_format == "lower": - if setvar is not None: - newEnv = new_env_vars(setvar, key_name="name", value_name="value") + newEnv = new_env_vars(setvar, key_name="name", value_name="value") if setvar is not None else [] d = {env['name']: env for env in currentEnv + newEnv} else: self.duplo.logger.warn("Possible attempt to merge env vars with" @@ -276,19 +273,21 @@ def detect_case_format(env_list): " Normalzing to capitalized" " \"Name\" and \"Value\"") norm_currentEnv = [{k.capitalize(): v for k, v in env.items()} for env in currentEnv] - if setvar is not None: - newEnv = new_env_vars(setvar) + newEnv = new_env_vars(setvar,) if setvar is not None else [] d = {env['Name']: env for env in norm_currentEnv + newEnv} except KeyError: raise DuploError("Could not merge new and existing environment variables") mergedvars = list(d.values()) currentDockerconfig['Env'] = mergedvars else: - newEnv = new_env_vars(setvar) + newEnv = new_env_vars(setvar,) if setvar is not None else [] currentDockerconfig['Env'] = newEnv if deletevar is not None: for key in deletevar: - currentDockerconfig['Env'] = [d for d in currentDockerconfig['Env'] if d['Name'] != key] + try: + currentDockerconfig['Env'] = [d for d in currentDockerconfig['Env'] if d['Name'] != key] + except KeyError: + currentDockerconfig['Env'] = [d for d in currentDockerconfig['Env'] if d['name'] != key] payload = { "Name": name, "OtherDockerConfig": dumps(currentDockerconfig), From c477e6809fe64f7a6fe585ab5dda5770f35b529e Mon Sep 17 00:00:00 2001 From: Matt Herrick <142412263+duplocloud-matt@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:26:55 -0500 Subject: [PATCH 4/5] updating changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c42ff5..6dd477d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] + - Fixed handling of case in name/value keys in environment variables as backend permits both. + - Fixes issue in service update argument where strategy required three dashes. + - Gracefully handles situations where user attempts to merge with a service that has no existing env vars. + ## [0.2.36] - 2024-09-25 ### Fixed From fd5a1bd9561d19df0aa8b94d75c925f029f5cef9 Mon Sep 17 00:00:00 2001 From: Matt Herrick <142412263+duplocloud-matt@users.noreply.github.com> Date: Tue, 8 Oct 2024 09:40:46 -0500 Subject: [PATCH 5/5] removing extra comma --- src/duplo_resource/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/duplo_resource/service.py b/src/duplo_resource/service.py index 8947e6f..72da5d1 100644 --- a/src/duplo_resource/service.py +++ b/src/duplo_resource/service.py @@ -280,7 +280,7 @@ def detect_case_format(env_list): mergedvars = list(d.values()) currentDockerconfig['Env'] = mergedvars else: - newEnv = new_env_vars(setvar,) if setvar is not None else [] + newEnv = new_env_vars(setvar) if setvar is not None else [] currentDockerconfig['Env'] = newEnv if deletevar is not None: for key in deletevar: