remove secrets from config yaml and terraform configuration files

.github/workflows/test.yaml

@@ -76,13 +76,13 @@ jobs:
         if: ${{ matrix.provider != 'local' }}
         uses: hashicorp/terraform-github-actions@master
         with:
-          tf_actions_version: 0.14.9
+          tf_actions_version: 1.0.0
           tf_actions_subcommand: 'fmt'
           tf_actions_working_dir: 'qhub-${{ matrix.provider }}-deployment/terraform-state'
       - name: 'Terraform Format'
         uses: hashicorp/terraform-github-actions@master
         with:
-          tf_actions_version: 0.14.9
+          tf_actions_version: 1.0.0
           tf_actions_subcommand: 'fmt'
           tf_actions_working_dir: 'qhub-${{ matrix.provider }}-deployment/infrastructure'
       - name: QHub Render Artifact

docs/source/02_get_started/04_configuration.md

@@ -144,6 +144,25 @@ security:
       gid: 102
 ```
 
+### Omitting sensitive values
+
+If you wish to avoid storing secrets etc. directly in the config yaml file you
+can instead set the values in environment variables. This substitution is
+triggered by setting config values to "QHUB_SECRET_" followed by the
+environment variable name. For example, you could set the environment variables
+"github_client_id" and "github_client_key" and write the following in your config
+file:
+
+```yaml
+security:
+  authentication:
+    type: GitHub
+    config:
+      client_id: QHUB_SECRET_github_client_id
+      client_secret: QHUB_SECRET_github_client_key
+      oauth_callback_url: https://do.qhub.dev/hub/oauth_callback
+```
+
 ### Authentication
 
 `security.authentication` is for configuring the OAuth and GitHub

qhub/deploy.py

@@ -1,4 +1,5 @@
 import logging
+import os
 import re
 from subprocess import CalledProcessError
 
@@ -42,6 +43,9 @@ def guided_install(
 ):
     # 01 Check Environment Variables
     check_cloud_credentials(config)
+    # Check that secrets required for terraform
+    # variables are set as required
+    check_secrets(config)
 
     # 02 Create terraform backend remote state bucket
     # backwards compatible with `qhub-config.yaml` which
@@ -127,3 +131,29 @@ def add_clearml_dns(zone_name, record_name, record_type, ip_or_hostname):
 
     for dns_record in dns_records:
         update_record(zone_name, dns_record, record_type, ip_or_hostname)
+
+
+def check_secrets(config):
+    """
+    Checks that the appropriate variables are set based on the current config.
+    These variables are prefixed with TF_VAR_ and are used to populate the
+    corresponding variables in the terraform deployment. e.g.
+    TF_VAR_prefect_token sets the prefect_token variable in Terraform. These
+    values are set in the terraform state but are not leaked when the
+    terraform render occurs.
+    """
+
+    missing_env_vars = []
+
+    # Check prefect integration set up.
+    if "prefect" in config and config["prefect"]["enabled"]:
+        var = "TF_VAR_prefect_token"
+        if var not in os.environ:
+            missing_env_vars.append(var)
+
+    if missing_env_vars:
+        raise EnvironmentError(
+            "Some environment variables used to propagate secrets to the "
+            "terraform deployment were not set. Please set these before "
+            f"continuing: {', '.join(missing_env_vars)}"
+        )

qhub/render/__init__.py

@@ -1,5 +1,6 @@
 import pathlib
 import collections
+import functools
 import json
 import os
 from shutil import copyfile
@@ -116,6 +117,12 @@ def render_template(output_directory, config_filename, force=False):
 
     with filename.open() as f:
         config = yaml.safe_load(f)
+
+        # For any config values that start with
+        # QHUB_SECRET_, set the values using the
+        # corresponding env var.
+        set_env_vars_in_config(config)
+
         config["repo_directory"] = repo_directory
         patch_dask_gateway_extra_config(config)
 
@@ -191,3 +198,68 @@ def qhubignore_matches(_):
                     os.rmdir(root_path / dir)
                 except OSError:
                     pass  # Silently fail if 'saved' files are present so dir not empty
+
+
+def set_env_vars_in_config(config):
+    """
+
+    For values in the config starting with 'QHUB_SECRET_XXX' the environment
+    variables are searched for the pattern XXX and the config value is
+    modified. This enables setting secret values that should not be directly
+    stored in the config file.
+
+    NOTE: variables are most likely written to a file somewhere upon render. In
+    order to further reduce risk of exposure of any of these variables you might
+    consider preventing storage of the terraform render output.
+    """
+    private_entries = get_secret_config_entries(config)
+    for idx in private_entries:
+        set_qhub_secret(config, idx)
+
+
+def get_secret_config_entries(config, config_idx=None, private_entries=None):
+    output = private_entries or []
+    if config_idx is None:
+        sub_dict = config
+        config_idx = []
+    else:
+        sub_dict = get_sub_config(config, config_idx)
+
+    for key, value in sub_dict.items():
+        if type(value) is dict:
+            sub_dict_outputs = get_secret_config_entries(
+                config, [*config_idx, key], private_entries
+            )
+            output = [*output, *sub_dict_outputs]
+        else:
+            if "QHUB_SECRET_" in str(value):
+                output = [*output, [*config_idx, key]]
+    return output
+
+
+def get_sub_config(conf, conf_idx):
+    sub_config = functools.reduce(dict.__getitem__, conf_idx, conf)
+    return sub_config
+
+
+def set_sub_config(conf, conf_idx, value):
+
+    get_sub_config(conf, conf_idx[:-1])[conf_idx[-1]] = value
+
+
+def set_qhub_secret(config, idx):
+    placeholder = get_sub_config(config, idx)
+    secret_var = get_qhub_secret(placeholder)
+    set_sub_config(config, idx, secret_var)
+
+
+def get_qhub_secret(secret_var):
+    env_var = secret_var.lstrip("QHUB_SECRET_")
+    val = os.environ.get(env_var)
+    if not val:
+        raise EnvironmentError(
+            f"Since '{secret_var}' was found in the"
+            " QHub config, the environment variable"
+            f" '{env_var}' must be set."
+        )
+    return val

qhub/template/{{ cookiecutter.repo_directory }}/infrastructure/modules/kubernetes/services/prefect/variables.tf

@@ -9,8 +9,7 @@ variable "namespace" {
 }
 
 variable "prefect_token" {
-  type    = string
-  default = ""
+  type = string
 }
 
 variable "image" {

tests/test_deploy.py

@@ -0,0 +1,18 @@
+import pytest
+from qhub.deploy import check_secrets
+
+
+def test_check_secrets(monkeypatch):
+
+    # Should do nothing without prefect key or not enabled
+    check_secrets({"key": "value"})
+    check_secrets({"prefect": {"enabled": False}})
+
+    # Should do nothing if appropriate var is set
+    monkeypatch.setenv("TF_VAR_prefect_token", "secret_token")
+    check_secrets({"prefect": {"enabled": True}})
+
+    # Should raise error if var is not set
+    monkeypatch.delenv("TF_VAR_prefect_token")
+    with pytest.raises(EnvironmentError):
+        check_secrets({"prefect": {"enabled": True}})

tests/test_render.py

@@ -2,7 +2,7 @@
 
 from ruamel import yaml
 
-from qhub.render import render_template
+from qhub.render import render_template, set_env_vars_in_config
 from qhub.initialize import render_config
 
 
@@ -15,7 +15,9 @@
         ("azure-pytest", "dev", "azure.qhub.dev", "azure", "github-actions", "github"),
     ],
 )
-def test_render(project, namespace, domain, cloud_provider, ci_provider, auth_provider, tmp_path):
+def test_render(
+    project, namespace, domain, cloud_provider, ci_provider, auth_provider, tmp_path
+):
     config = render_config(
         project_name=project,
         namespace=namespace,
@@ -37,3 +39,35 @@ def test_render(project, namespace, domain, cloud_provider, ci_provider, auth_pr
 
     output_directory = tmp_path / "test"
     render_template(str(output_directory), config_filename, force=True)
+
+
+def test_get_secret_config_entries(monkeypatch):
+    sec1 = "secret1"
+    sec2 = "nestedsecret1"
+    config_orig = {
+        "key1": "value1",
+        "key2": "QHUB_SECRET_secret_val",
+        "key3": {
+            "nested_key1": "nested_value1",
+            "nested_key2": "QHUB_SECRET_nested_secret_val",
+        },
+    }
+    expected = {
+        "key1": "value1",
+        "key2": sec1,
+        "key3": {
+            "nested_key1": "nested_value1",
+            "nested_key2": sec2,
+        },
+    }
+
+    # should raise error if implied env var is not set
+    with pytest.raises(EnvironmentError):
+        config = config_orig.copy()
+        set_env_vars_in_config(config)
+
+    monkeypatch.setenv("secret_val", sec1, prepend=False)
+    monkeypatch.setenv("nested_secret_val", sec2, prepend=False)
+    config = config_orig.copy()
+    set_env_vars_in_config(config)
+    assert config == expected