Add --local-config-method, merge local app components with remote one…

…s by default (#301)

README.md

@@ -219,11 +219,49 @@ By default, if app components use `ClowdApp` resources in their templates, then
 
 `bonfire` ships with a [default config](bonfire/resources/default_config.yaml) that should be enough to get started for most internal Red Hat employees.
 
-By default, the configuration file will be stored in `~/.config/bonfire/config.yaml`.
+By default, the configuration file will be stored in `~/.config/bonfire/config.yaml`. 
 
-If you wish to override any app configurations, you can edit your local configuration file by typing `bonfire config edit`. If you define an app under the `apps` key of the config, it will take precedence over that app's configuration that was fetched from app-interface. Note that defining a local app configuration here will replace the configuration defined in app-interface for ALL components within that app.
+If you wish to override any app configurations, you can edit your local configuration file by typing `bonfire config edit`. You can then define an app under the `apps` key of the config. You can reset the config to default at any time using `bonfire config write-default`.
 
-You can reset the config to default at any time using `bonfire config write-default`.
+As of `bonfire` v5, there are two options for how the local configuration is loaded (controlled by the `--local-config-method` CLI option). Let's say we have an app configured in app-interface like this:
+
+```
+resourceTemplates:
+- name: mycomponent1
+  path: /deployment.yaml
+  url: https://github.com/myorg/myrepo1
+  targets:
+  - namespace:
+      $ref: /services/insights/ephemeral/namespaces/ephemeral-base.yml
+    parameters:
+      MIN_REPLICAS: 1
+- name: mycomponent2
+  path: /deployment.yaml
+  url: https://github.com/myorg/myrepo2
+  targets:
+  - namespace:
+      $ref: /services/insights/ephemeral/namespaces/ephemeral-base.yml
+    parameters:
+      MIN_REPLICAS: 2
+      SOME_OTHER_PARAM: some_other_value
+```
+
+and the local config file has this entry:
+
+```
+apps:
+- name: myapp
+  components:
+  - name: mycomponent2
+    parameters:
+      MIN_REPLICAS: 10
+```
+
+`bonfire` can override the remote configuration using one of two methods:
+
+1. `--local-config-method merge` (default). In this mode, the apps config in your local config is merged with the configuration that bonfire fetched remotely. With the above config, only the 'MIN_REPLICAS' parameter of 'mycomponent2' within app 'myapp' will be overridden. The 'SOME_OTHER_PARAM' parameter will still be present, and 'mycomponent1' would be unchanged.
+
+2. `--local-config-method override`. In this mode, the local app configuration will take precedence over the app's configuration that bonfire fetched remotely. In other words, defining a local app configuration will replace the configuration defined in app-interface for ALL components within that app. So 'mycomponent1' would be completely removed, and 'mycomponent2' would only have the parameters you defined in the local config.
 
 ## Local config examples
 
@@ -239,13 +277,15 @@ apps:
       host: local
       repo: ~/dev/projects/my-app
       path: /clowdapp.yaml
+      parameters:
+        SOME_PARAMETER: some_value
 ```
 
 - Where **host** set `local` indicates to look for the repo in a local directory
 - Where **repo** indicates the path to your git repo folder
 - Where **path** specifies the relative path to your ClowdApp template file in your git repo
 
-By default, `bonfire` will run `git rev-parse` to determine the current working commit hash in your repo folder, and this determines what IMAGE_TAG to set when the template is deployed. This means you would need to have a valid container image pushed for this commit hash. However, you can use `--set-image-tag` or `--set-parameter` to override the image you wish to use during the deployment.
+By default, `bonfire` will run `git rev-parse` to determine the current working commit hash in your repo folder, and this determines what IMAGE_TAG to set when the template is deployed. This means you would need to have a valid container image pushed for this commit hash. However, you can use `--set-image-tag`/`--set-parameter` or define the `IMAGE_TAG` parameter in your config in order to override the image you wish to use during the deployment.
 
 ### Deploying changes changes in a remote git branch
 

bonfire/bonfire.py

@@ -12,7 +12,8 @@
 from wait_for import TimedOutError
 
 import bonfire.config as conf
-from bonfire.local import get_local_apps
+from bonfire.local import get_local_apps, get_appsfile_apps
+from bonfire.utils import RepoFile, SYNTAX_ERR
 from bonfire.namespaces import (
     Namespace,
     extend_namespace,
@@ -47,12 +48,13 @@
     get_version,
     split_equals,
     validate_time_string,
+    merge_app_configs,
 )
 
 log = logging.getLogger(__name__)
 
 APP_SRE_SRC = "appsre"
-LOCAL_SRC = "local"
+FILE_SRC = "file"
 NO_RESERVATION_SYS = "this cluster does not use a namespace reservation system"
 
 _local_option = click.option(
@@ -370,7 +372,7 @@ def _validate_opposing_opts(ctx, param, value):
         "--source",
         "-s",
         help=f"Configuration source to use when fetching app templates (default: {APP_SRE_SRC})",
-        type=click.Choice([APP_SRE_SRC, LOCAL_SRC], case_sensitive=False),
+        type=click.Choice([APP_SRE_SRC, FILE_SRC], case_sensitive=False),
         default=APP_SRE_SRC,
     ),
     click.option(
@@ -379,25 +381,40 @@ def _validate_opposing_opts(ctx, param, value):
         help="File to use for local config (default: $XDG_CONFIG_HOME/bonfire/config.yaml)",
         default=None,
     ),
+    click.option(
+        "--local-config-method",
+        help="Selects method used when combining apps in local config with remote app config",
+        type=click.Choice(["merge", "override"], case_sensitive=False),
+        show_default=True,
+        default="merge",
+    ),
     click.option(
         "--target-env",
         help=(
-            f"When using source={APP_SRE_SRC}, name of environment to fetch templates for"
-            f" (default: {conf.EPHEMERAL_ENV_NAME})"
+            f"Target environment name when using source={APP_SRE_SRC}. "
+            f"Use to select which template parameters are fetched."
         ),
         type=str,
         default=conf.EPHEMERAL_ENV_NAME,
+        show_default=True,
     ),
 ]
 
-_process_options = [
+_process_options = _app_source_options + [
     click.argument(
         "app_names",
         required=True,
         nargs=-1,
     ),
-    _app_source_options[0],
-    _app_source_options[1],
+    click.option(
+        "--ref-env",
+        help=(
+            f"Reference environment name in {APP_SRE_SRC}. "
+            "Use to set default 'ref'/'IMAGE_TAG' for apps."
+        ),
+        type=str,
+        default=None,
+    ),
     click.option(
         "--set-image-tag",
         "-i",
@@ -430,13 +447,6 @@ def _validate_opposing_opts(ctx, param, value):
         type=str,
         default=None,
     ),
-    _app_source_options[2],
-    click.option(
-        "--ref-env",
-        help=f"Query {APP_SRE_SRC} for apps in this environment and substitute 'ref'/'IMAGE_TAG'",
-        type=str,
-        default=None,
-    ),
     click.option(
         "--get-dependencies/--no-get-dependencies",
         help="Recursively fetch dependencies listed in ClowdApps (default: true)",
@@ -805,7 +815,7 @@ def _describe_namespace(namespace):
     click.echo(describe_namespace(namespace))
 
 
-def _get_apps_config(source, target_env, ref_env, local_config_path):
+def _get_apps_config(source, target_env, ref_env, local_config_path, local_config_method):
     config = conf.load_config(local_config_path)
 
     if source == APP_SRE_SRC:
@@ -820,12 +830,25 @@ def _get_apps_config(source, target_env, ref_env, local_config_path):
                 for component in app_cfg.get("components", []):
                     component["ref"] = "master"
 
-        # override any apps that were defined in 'apps' setion of local config file
-        apps_config.update(get_local_apps(config, fetch_remote=False))
-
-    elif source == LOCAL_SRC:
+    elif source == FILE_SRC:
         log.info("fetching apps config using source: %s", source)
-        apps_config = get_local_apps(config, fetch_remote=True)
+        apps_config = get_appsfile_apps(config)
+
+    # merge remote apps config with local app config
+    local_apps = get_local_apps(config)
+    apps_config = merge_app_configs(apps_config, local_apps, local_config_method)
+
+    # validate the components look ok after merging
+    for app_name, app_config in apps_config.items():
+        for component in app_config["components"]:
+            # validate the config for a component
+            if not component.get("name"):
+                raise FatalError(f"{SYNTAX_ERR}, component is missing 'name'")
+            try:
+                RepoFile.from_config(component)
+            except FatalError as err:
+                # re-raise with a bit more context
+                raise FatalError(f"{str(err)}, hit on app {app_name}")
 
     if ref_env:
         log.info("subbing app template refs/image tags using environment: %s", ref_env)
@@ -850,6 +873,7 @@ def _process(
     source,
     get_dependencies,
     optional_deps_method,
+    local_config_method,
     set_image_tag,
     ref_env,
     target_env,
@@ -866,7 +890,9 @@ def _process(
     local,
     frontends,
 ):
-    apps_config = _get_apps_config(source, target_env, ref_env, local_config_path)
+    apps_config = _get_apps_config(
+        source, target_env, ref_env, local_config_path, local_config_method
+    )
 
     processor = TemplateProcessor(
         apps_config,
@@ -908,6 +934,7 @@ def _cmd_process(
     source,
     get_dependencies,
     optional_deps_method,
+    local_config_method,
     set_image_tag,
     ref_env,
     target_env,
@@ -933,6 +960,7 @@ def _cmd_process(
         source,
         get_dependencies,
         optional_deps_method,
+        local_config_method,
         set_image_tag,
         ref_env,
         target_env,
@@ -1090,6 +1118,7 @@ def _cmd_config_deploy(
     source,
     get_dependencies,
     optional_deps_method,
+    local_config_method,
     set_image_tag,
     ref_env,
     target_env,
@@ -1175,6 +1204,7 @@ def _err_handler(err):
             source,
             get_dependencies,
             optional_deps_method,
+            local_config_method,
             set_image_tag,
             ref_env,
             target_env,
@@ -1448,11 +1478,12 @@ def _cmd_edit_default_config(path):
 def _cmd_apps_list(
     source,
     local_config_path,
+    local_config_method,
     target_env,
     list_components,
 ):
     """List names of all apps that are marked for deployment in given 'target_env'"""
-    apps = _get_apps_config(source, target_env, None, local_config_path)
+    apps = _get_apps_config(source, target_env, None, local_config_path, local_config_method)
 
     print("")
     sorted_keys = sorted(apps.keys())
@@ -1474,11 +1505,12 @@ def _cmd_apps_list(
 def _cmd_apps_what_depends_on(
     source,
     local_config_path,
+    local_config_method,
     target_env,
     component,
 ):
     """Show any apps that depend on COMPONENT for deployments in given 'target_env'"""
-    apps = _get_apps_config(source, target_env, None, local_config_path)
+    apps = _get_apps_config(source, target_env, None, local_config_path, local_config_method)
     found = find_what_depends_on(apps, component)
     print("\n".join(found) or f"no apps depending on {component} found")
 

bonfire/local.py

@@ -2,7 +2,7 @@
 
 import yaml
 
-from bonfire.utils import FatalError, RepoFile, get_dupes
+from bonfire.utils import FatalError, RepoFile, get_dupes, SYNTAX_ERR
 
 log = logging.getLogger(__name__)
 
@@ -21,42 +21,60 @@ def _fetch_apps_file(config):
     fetched_apps = yaml.safe_load(content)
 
     if "apps" not in fetched_apps:
-        raise FatalError("fetched apps file has no 'apps' key")
+        raise FatalError(f"{SYNTAX_ERR}, fetched apps file has no 'apps' key")
 
     app_names = [a["name"] for a in fetched_apps["apps"]]
     dupes = get_dupes(app_names)
     if dupes:
-        raise FatalError("duplicate app names found in fetched apps file: {dupes}")
+        raise FatalError(f"{SYNTAX_ERR}, duplicate app names found in fetched apps file: {dupes}")
 
     return {a["name"]: a for a in fetched_apps["apps"]}
 
 
 def _parse_apps_in_cfg(config):
-    app_names = [a["name"] for a in config["apps"]]
+    app_names = []
+
+    for app in config["apps"]:
+        if not isinstance(app, dict):
+            raise FatalError(f"{SYNTAX_ERR} app type should be a dict type")
+        if "components" not in app:
+            raise FatalError(f"{SYNTAX_ERR} 'components' missing from an app")
+        for component in app["components"]:
+            if not isinstance(component, dict):
+                raise FatalError(f"{SYNTAX_ERR} component should be a dict type")
+        app_names.append(app["name"])
+
     dupes = get_dupes(app_names)
     if dupes:
-        raise FatalError("duplicate app names found in config: {dupes}")
+        raise FatalError(f"{SYNTAX_ERR} duplicate app names found in config: {dupes}")
+
     return {a["name"]: a for a in config["apps"]}
 
 
-def get_local_apps(config, fetch_remote=True):
-    # get any apps set directly in config
+def get_local_apps(config):
+    """
+    Get apps defined locally under 'apps' section of config
+    """
     config_apps = {}
+    if not isinstance(config, dict):
+        raise FatalError(f"{SYNTAX_ERR}, expected local config to be a dictionary")
     if "apps" in config:
         config_apps = _parse_apps_in_cfg(config)
-        log.info("local app configuration overrides found for: %s", list(config_apps.keys()))
-
-    if not fetch_remote:
-        final_apps = config_apps
-    else:
-        # fetch apps from repo if appsFile is provided in config
-        fetched_apps = {}
-        if "appsFile" in config:
-            log.info("local config has a remote 'appsFile' defined, fetching it...")
-            fetched_apps = _fetch_apps_file(config)
-
-        # override fetched apps with local apps if any were defined
-        fetched_apps.update(config_apps)
-        final_apps = fetched_apps
-
-    return final_apps
+        log.info("local configuration found for apps: %s", list(config_apps.keys()))
+
+    return config_apps
+
+
+def get_appsfile_apps(config):
+    """
+    Fetch apps from repo based on appsFile provided in config
+    """
+    if not isinstance(config, dict):
+        raise FatalError(f"{SYNTAX_ERR}, expected local config to be a dictionary")
+
+    if "appsFile" not in config:
+        raise FatalError(f"{SYNTAX_ERR}, config has no 'appsFile' defined")
+
+    log.info("local config has a remote 'appsFile' defined, fetching it...")
+    fetched_apps = _fetch_apps_file(config)
+    return fetched_apps