From 8e83f3b72bb51efefa16f00df324d5fb700abbf6 Mon Sep 17 00:00:00 2001 From: Brandon Squizzato <35474886+bsquizz@users.noreply.github.com> Date: Fri, 8 Oct 2021 13:26:14 -0400 Subject: [PATCH] Validate whether components provided in CLI options exist in config (#128) * Validate whether components provided in CLI options exist in config * Thought of a more efficient way to do this --- bonfire/processor.py | 125 ++++++++++++++++++++++++++++++++++--------- bonfire/utils.py | 8 +-- 2 files changed, 105 insertions(+), 28 deletions(-) diff --git a/bonfire/processor.py b/bonfire/processor.py index 619728ff..9db547ab 100644 --- a/bonfire/processor.py +++ b/bonfire/processor.py @@ -3,6 +3,7 @@ import json import yaml import re +import traceback import uuid from pathlib import Path @@ -182,21 +183,84 @@ def _find_dupe_components(components_for_app): f"component '{component}' is not unique, found in apps: {found_in}" ) - def _validate_app_config(self, apps_config): + @staticmethod + def _validate_component_dict(all_components, data, name): + # Translate deprecated "APP_NAME/COMPONENT_NAME" path syntax + updated_data = {} + + # Max number of items we expect separated by '/' in the CLI option value + max_len = 2 + if name == "--set-parameter": + max_len = 3 + + for path, value in data.items(): + split = path.split("/") + if len(split) == max_len: + # first item was an app name + new_path = split[1:] + elif len(split) == max_len - 1: + # first item was a component name + new_path = split[0:] + else: + raise FatalError(f"invalid format for {name}: {path}={value}") + + component_name = new_path[0] + + # Make sure component name actually exists in app config + if component_name not in all_components: + raise FatalError( + f"component given for {name} not found in app config: {component_name}" + ) + + key = "/".join(new_path) + updated_data[key] = value + + # Update the paths + data.clear() + for key, val in updated_data.items(): + data[key] = val + + @staticmethod + def _validate_component_list(all_components, data, name): + for component_name in data: + if component_name not in all_components: + raise FatalError( + f"component given for {name} not found in app config: {component_name}" + ) + + def _validate_component_options(self, all_components, data, name): + if isinstance(data, dict): + self._validate_component_dict(all_components, data, name) + else: + self._validate_component_list(all_components, data, name) + + def _validate(self): + """ + Validate app configurations and options passed to the TemplateProcessor + + 1. Check that each app has required keys + 2. Check that each app name is unique + 3. Check that each component in an app has required keys + 4. Check that each component is a unique name across the whole config + 5. Check that CLI params requiring a component use a valid component name + """ components_for_app = {} - for app_name, app_cfg in apps_config.items(): + for app_name, app_cfg in self.apps_config.items(): + # Check that each app has required keys required_keys = ["name", "components"] missing_keys = [k for k in required_keys if k not in app_cfg] if missing_keys: raise FatalError(f"app '{app_name}' is missing required keys: {missing_keys}") + # Check that each app name is unique app_name = app_cfg["name"] if app_name in components_for_app: raise FatalError(f"app with name '{app_name}' is not unique") components_for_app[app_name] = [] for component in app_cfg.get("components", []): + # Check that each component in an app has required keys required_keys = ["name", "host", "repo", "path"] missing_keys = [k for k in required_keys if k not in component] if missing_keys: @@ -206,8 +270,32 @@ def _validate_app_config(self, apps_config): comp_name = component["name"] components_for_app[app_name].append(comp_name) + # Check that each component name is unique across the whole config self._find_dupe_components(components_for_app) + # Check that CLI params requiring a component use a valid component name + all_components = [] + for _, app_components in components_for_app.items(): + all_components.extend(app_components) + + log.debug("components found: %s", all_components) + + self._validate_component_options( + all_components, self.template_ref_overrides, "--set-template-ref" + ) + self._validate_component_options(all_components, self.param_overrides, "--set-parameter") + + # 'all' is a valid component keyword for these options below + all_components.append("all") + + self._validate_component_options( + all_components, self.remove_resources, "--remove-resources" + ) + self._validate_component_options( + all_components, self.no_remove_resources, "--no-remove-resources" + ) + self._validate_component_options(all_components, self.component_filter, "--component") + def __init__( self, apps_config, @@ -222,8 +310,6 @@ def __init__( single_replicas, component_filter, ): - self._validate_app_config(apps_config) - self.apps_config = apps_config self.requested_app_names = self._parse_app_names(app_names) self.get_dependencies = get_dependencies @@ -236,6 +322,8 @@ def __init__( self.single_replicas = single_replicas self.component_filter = component_filter + self._validate() + self.k8s_list = { "kind": "List", "apiVersion": "v1", @@ -268,18 +356,7 @@ def _sub_image_tags(self, items): return json.loads(content) def _sub_ref(self, current_component_name, repo_file): - for app_component, value in self.template_ref_overrides.items(): - # TODO: remove split when app_name syntax is fully deprecated - split = app_component.split("/") - if len(split) == 2: - _, component_name = split - elif len(split) == 1: - component_name = split[0] - else: - raise FatalError( - f"invalid format for template ref override: {app_component}={value}" - ) - + for component_name, value in self.template_ref_overrides.items(): if current_component_name == component_name: log.info( "component: '%s' overriding template ref to '%s'", @@ -289,15 +366,12 @@ def _sub_ref(self, current_component_name, repo_file): repo_file.ref = value def _sub_params(self, current_component_name, params): - for param_path, value in self.param_overrides.items(): - # TODO: remove split when app_name syntax is fully deprecated - split = param_path.split("/") - if len(split) == 3: - _, component_name, param_name = split - elif len(split) == 2: + for path, value in self.param_overrides.items(): + split = path.split("/") + if len(split) == 2: component_name, param_name = split else: - raise FatalError(f"invalid format for parameter override: {param_path}={value}") + raise FatalError(f"invalid format for --set-parameter: {path}={value}") if current_component_name == component_name: log.info( @@ -315,9 +389,10 @@ def _get_component_items(self, component_name): # override template ref if requested self._sub_ref(component_name, rf) commit, template_content = rf.fetch() - except Exception: + except Exception as err: log.error("failed to fetch template file for %s", component_name) - raise + log.debug(traceback.format_exc()) + raise FatalError(err) template = yaml.safe_load(template_content) diff --git a/bonfire/utils.py b/bonfire/utils.py index da6ee453..6e3382c1 100644 --- a/bonfire/utils.py +++ b/bonfire/utils.py @@ -217,10 +217,12 @@ def _get_ref(self, get_ref_func): log.info("trying alternate: %s", refs_to_try[idx + 1]) continue else: - alts = ", ".join(self._alternate_refs[self.ref]) + alts_txt = "" + if self.ref in self._alternate_refs: + alts = ", ".join(self._alternate_refs[self.ref]) + alts_txt = f" and its alternates: {alts}" raise Exception( - f"failed to fetch git ref '{self.ref}' or any of its alternates: '{alts}'," - " check logs for more details" + f"git ref fetch failed for '{self.ref}'{alts_txt}, see logs for details" ) return response