Enhance the AppConfig with external config sources. (#1904)

* Enhance the AppConfig with external config sources.

The external config sources are currently environment variables
and AWS secrets manager.

The config file can be augmented with a section describing how
environmen variables and secrets can update config parameters.

benefits:
 - it will enable the config to draw from more than one secret.  This is useful
   for shared secrets between cellxgene and data portal, as well as auth0 secrets.
 - it will make it very straightforward to check the config before a deployment.

 Part of #1859

server/common/aws_secret_utils.py

@@ -18,6 +18,6 @@ def get_secret_key(region_name, secret_name):
             return secret
     except Exception as e:
         logging.critical(f"Caught exception during get_secret_key, {e}", exc_info=True)
-        raise SecretKeyRetrievalError
+        raise SecretKeyRetrievalError(str(e))
 
     return None

server/common/config/app_config.py

@@ -4,6 +4,7 @@ from flatten_dict import unflatten
 from server.default_config import get_default_config
 from server.common.config.dataset_config import DatasetConfig
 from server.common.config.server_config import ServerConfig
+from server.common.config.external_config import ExternalConfig
 from server.common.errors import ConfigurationError
 
 
@@ -44,6 +45,9 @@ class AppConfig(object):
         #   dataroot config
         self.dataroot_config = {}
 
+        #  external config
+        self.external_config = ExternalConfig(self, self.default_config["external"])
+
         # Set to true when config_completed is called
         self.is_completed = False
 
@@ -61,6 +65,7 @@ class AppConfig(object):
         self.default_dataset_config.check_config()
         for dataset_config in self.dataroot_config.values():
             dataset_config.check_config()
+        self.external_config.check_config()
 
     def update_server_config(self, **kw):
         self.server_config.update(**kw)
@@ -73,6 +78,51 @@ class AppConfig(object):
             value.update(**kw)
         self.is_complete = False
 
+    def update_single_config_from_path_and_value(self, path, value):
+        """Update a single config parameter with the value.
+        Path is a list of string, that gives a path to the config parameter to be updated.
+        For example, path may be ["server","app","port"].
+        """
+        self.is_complete = False
+        if not isinstance(path, list):
+            raise ConfigurationError(f"path must be a list of strings, got '{str(path)}'")
+        for part in path:
+            if not isinstance(part, str):
+                raise ConfigurationError(f"path must be a list of strings, got '{str(path)}'")
+
+        if len(path) < 1 or path[0] not in ("server", "dataset", "per_dataset_config"):
+            raise ConfigurationError("path must start with 'server', 'dataset', or 'per_dataset_config'")
+
+        if path[0] == "server":
+            attr = "__".join(path[1:])
+            try:
+                self.update_server_config(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+        elif path[0] == "dataset":
+            attr = "__".join(path[1:])
+            try:
+                self.update_default_dataset_config(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+
+        elif path[0] == "per_dataset_config":
+            if len(path) < 2:
+                raise ConfigurationError(f"missing dataroot when using per_dataset_config: got '{path}'")
+            dataroot = path[1]
+            if dataroot not in self.dataroot_config:
+                dataroots = str(list(self.dataroot_config.keys()))
+                raise ConfigurationError(
+                    f"unknown dataroot when using per_dataset_config: got '{path}',"
+                    f" dataroots specified in config are {dataroots}"
+                )
+
+            attr = "__".join(path[2:])
+            try:
+                self.dataroot_config[dataroot].update(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+
     def update_from_config_file(self, config_file):
         try:
             with open(config_file) as yml_file:
@@ -94,12 +144,16 @@ class AppConfig(object):
             # then apply the per dataset configuration
             self.dataroot_config[key].update_from_config(dataroot_config, f"per_dataset_config__{key}")
 
+        if config.get("external"):
+            self.external_config.update_from_config(config["external"], "external")
+
         self.is_complete = False
 
-    def write_config(self, config_file):
-        """output the config to a yaml file"""
+    def config_to_dict(self):
+        """return the configuration as an unflattened dict"""
         server = self.server_config.create_mapping(self.server_config.default_config)
         dataset = self.default_dataset_config.create_mapping(self.default_dataset_config.default_config)
+        external = self.external_config.create_mapping(self.external_config.default_config)
         config = dict(server={}, dataset={})
         for attrname in server.keys():
             config["server__" + attrname] = getattr(self.server_config, attrname)
@@ -111,15 +165,23 @@ class AppConfig(object):
             dataset = dataroot_config.create_mapping(dataroot_config.default_config)
             for attrname in dataset.keys():
                 config[f"per_dataset_config__{dataroot_tag}__" + attrname] = getattr(dataroot_config, attrname)
+        for attrname in external.keys():
+            config["external__" + attrname] = getattr(self.external_config, attrname)
 
         config = unflatten(config, splitter=lambda key: key.split("__"))
+        return config
+
+    def write_config(self, config_file):
+        """output the config to a yaml file"""
+        config = self.config_to_dict()
         yaml.dump(config, open(config_file, "w"))
 
     def changes_from_default(self):
         """Return all the attribute that are different from the default"""
         diff_server = self.server_config.changes_from_default()
         diff_dataset = self.default_dataset_config.changes_from_default()
-        diff = dict(server=diff_server, dataset=diff_dataset)
+        diff_external = self.external.changes_from_default()
+        diff = dict(server=diff_server, dataset=diff_dataset, external=diff_external)
         return diff
 
     def add_dataroot_config(self, dataroot_tag, **kw):
@@ -154,6 +216,8 @@ class AppConfig(object):
         # messages we can give correct context for attributes with bad value.
         context = dict(messagefn=messagefn)
 
+        # complete config for external_config first, since this may update values in the other sections
+        self.external_config.complete_config(context)
         self.server_config.complete_config(context)
         self.default_dataset_config.complete_config(context)
         for dataroot_config in self.dataroot_config.values():

server/common/config/base_config.py

@@ -80,8 +80,27 @@ class BaseConfig(object):
                 raise ConfigurationError(f"The attr '{key}' has not been checked")
 
     def update(self, **kw):
+        """Update the attributes defined in kw with their new values."""
         for key, value in kw.items():
             if not hasattr(self, key):
+
+                # check if the key is setting into a dictval entry.
+                found_dictval = False
+                for dictval in self.dictval_cases:
+                    dictvalname = "__".join(dictval)
+                    if dictvalname + "__" in key:
+                        dictkey = key[len(dictvalname) + 2 :]
+                        curdictval = getattr(self, dictvalname)
+                        if curdictval is None:
+                            setattr(self, dictvalname, dict(dictkey=value))
+                        else:
+                            curdictval[dictkey] = value
+
+                        found_dictval = True
+                        break
+
+                if found_dictval:
+                    continue
                 raise ConfigurationError(f"unknown config parameter {key}.")
             try:
                 if type(value) == tuple:

server/common/config/dataset_config.py

@@ -30,22 +30,18 @@ class DatasetConfig(BaseConfig):
             self.user_annotations__type = default_config["user_annotations"]["type"]
             self.user_annotations__local_file_csv__directory = default_config["user_annotations"]["local_file_csv"][
                 "directory"
-            ]  # noqa E501
+            ]
             self.user_annotations__local_file_csv__file = default_config["user_annotations"]["local_file_csv"]["file"]
             self.user_annotations__ontology__enable = default_config["user_annotations"]["ontology"]["enable"]
             self.user_annotations__ontology__obo_location = default_config["user_annotations"]["ontology"][
                 "obo_location"
-            ]  # noqa E501
+            ]
             self.user_annotations__hosted_tiledb_array__db_uri = default_config["user_annotations"][
                 "hosted_tiledb_array"
-            ][
-                "db_uri"
-            ]  # noqa E501
+            ]["db_uri"]
             self.user_annotations__hosted_tiledb_array__hosted_file_directory = default_config["user_annotations"][
                 "hosted_tiledb_array"
-            ][
-                "hosted_file_directory"
-            ]  # noqa E501
+            ]["hosted_file_directory"]
 
             self.embeddings__names = default_config["embeddings"]["names"]
             self.embeddings__enable_reembedding = default_config["embeddings"]["enable_reembedding"]
@@ -98,20 +94,20 @@ class DatasetConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("user_annotations__type", str)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__local_file_csv__directory", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__local_file_csv__file", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("user_annotations__ontology__enable", bool)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__ontology__obo_location", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__db_uri", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__hosted_file_directory", (type(None), str)
-        )  # noqa E501
+        )
         if self.user_annotations__enable:
             server_config = self.app_config.server_config
             if not self.app__authentication_enable:
@@ -166,7 +162,7 @@ class DatasetConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("user_annotations__hosted_tiledb_array__db_uri", str)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__hosted_file_directory", str
-        )  # noqa E501
+        )
         self.user_annotations = AnnotationsHostedTileDB(
             directory_path=self.user_annotations__hosted_tiledb_array__hosted_file_directory,
             db=DbUtils(self.user_annotations__hosted_tiledb_array__db_uri),

server/common/config/external_config.py

@@ -0,0 +1,96 @@
+import os
+
+from server.common.config.base_config import BaseConfig
+from server.common.errors import ConfigurationError
+from server.common.config import get_secret_key
+from server.common.errors import SecretKeyRetrievalError
+from server.common.utils.type_conversion_utils import convert_string_to_value
+
+
+class ExternalConfig(BaseConfig):
+    """Manages the config attribute associated with external configuration sources, such as
+    environment variables or the AWS Secrets Manager."""
+
+    def __init__(self, app_config, default_config):
+        super().__init__(app_config, default_config)
+        try:
+            self.environment = default_config["environment"]
+            self.aws_secrets_manager__region = default_config["aws_secrets_manager"]["region"]
+            self.aws_secrets_manager__secrets = default_config["aws_secrets_manager"]["secrets"]
+
+        except KeyError as e:
+            raise ConfigurationError(f"Unexpected config: {str(e)}")
+
+    def complete_config(self, context):
+        self.handle_environment(context)
+        self.handle_aws_secrets_manager(context)
+
+    def handle_environment(self, context):
+        """For each environment variable defined, get the value (if it is set),
+        and set the specified config parameter"""
+        self.validate_correct_type_of_configuration_attribute("environment", list)
+        for envdict in self.environment:
+            name = envdict.get("name")
+            if name is None:
+                raise ConfigurationError("environment: 'name' is missing")
+            required = envdict.get("required", False)
+            if type(required) != bool:
+                raise ConfigurationError("environment: 'required' must be a bool")
+            path = envdict.get("path")
+            if path is None:
+                raise ConfigurationError("environment: 'path' is missing")
+
+            value = os.environ.get(name)
+            if value is None:
+                if required:
+                    raise ConfigurationError(f"required environment variable '{name}' not set")
+            else:
+                value = convert_string_to_value(value)
+                self.app_config.update_single_config_from_path_and_value(path, value)
+
+    def handle_aws_secrets_manager(self, context):
+        """For each aws secret defined, get the key/values, and set the specified config parameter"""
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__region", (type(None), str))
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__secrets", list)
+
+        if not self.aws_secrets_manager__secrets:
+            return
+
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__region", str)
+
+        for secret in self.aws_secrets_manager__secrets:
+            secret_name = secret.get("name")
+            if secret_name is None:
+                raise ConfigurationError("aws_secrets_manager: 'name' is missing")
+            if not isinstance(secret_name, str):
+                raise ConfigurationError("aws_secrets_manager: 'name' must be a string")
+
+            try:
+                secret_dict = get_secret_key(self.aws_secrets_manager__region, secret_name)
+            except SecretKeyRetrievalError as e:
+                raise ConfigurationError(f"Unable to retrieve secret {secret_name}: {str(e)}")
+
+            values = secret.get("values")
+            if values is None:
+                raise ConfigurationError("aws_secrets_manager: 'values' is missing")
+            if not isinstance(values, list):
+                raise ConfigurationError("aws_secrets_manager: 'values' must be a list")
+
+            for value in values:
+                key = value.get("key")
+                if key is None:
+                    raise ConfigurationError(f"missing 'key' in secret values: {secret_name}")
+                path = value.get("path")
+                if path is None:
+                    raise ConfigurationError(f"missing 'path' in secret values: {secret_name}")
+                required = value.get("required", False)
+                if type(required) != bool:
+                    raise ConfigurationError(f"wrong type for 'required' in secret values: {secret_name}")
+
+                secret_value = secret_dict.get(key)
+                if secret_value is None:
+                    if required:
+                        raise ConfigurationError(f"required secret '{secret_name}:{key}' not set")
+                else:
+                    secret_value = convert_string_to_value(secret_value)
+                    self.app_config.update_single_config_from_path_and_value(path, secret_value)

server/common/config/server_config.py

@@ -44,17 +44,17 @@ class ServerConfig(BaseConfig):
             self.authentication__type = default_config["authentication"]["type"]
             self.authentication__params_oauth__oauth_api_base_url = default_config["authentication"]["params_oauth"][
                 "oauth_api_base_url"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__client_id = default_config["authentication"]["params_oauth"]["client_id"]
             self.authentication__params_oauth__client_secret = default_config["authentication"]["params_oauth"][
                 "client_secret"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__jwt_decode_options = default_config["authentication"]["params_oauth"][
                 "jwt_decode_options"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__session_cookie = default_config["authentication"]["params_oauth"][
                 "session_cookie"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__cookie = default_config["authentication"]["params_oauth"]["cookie"]
 
             self.multi_dataset__dataroot = default_config["multi_dataset"]["dataroot"]
@@ -62,10 +62,10 @@ class ServerConfig(BaseConfig):
             self.multi_dataset__allowed_matrix_types = default_config["multi_dataset"]["allowed_matrix_types"]
             self.multi_dataset__matrix_cache__max_datasets = default_config["multi_dataset"]["matrix_cache"][
                 "max_datasets"
-            ]  # noqa E501
+            ]
             self.multi_dataset__matrix_cache__timelimit_s = default_config["multi_dataset"]["matrix_cache"][
                 "timelimit_s"
-            ]  # noqa E501
+            ]
 
             self.single_dataset__datapath = default_config["single_dataset"]["datapath"]
             self.single_dataset__obs_names = default_config["single_dataset"]["obs_names"]
@@ -151,11 +151,6 @@ class ServerConfig(BaseConfig):
         if not self.app__verbose:
             sys.tracebacklimit = 0
 
-        # secret key:
-        #   first, from CXG_SECRET_KEY environment variable
-        #   second, from config file
-        self.app__flask_secret_key = os.environ.get("CXG_SECRET_KEY", self.app__flask_secret_key)
-
         # CSP Directives are a dict of string: list(string) or string: string
         if self.app__csp_directives is not None:
             for k, v in self.app__csp_directives.items():
@@ -178,25 +173,20 @@ class ServerConfig(BaseConfig):
         ptypes = str if self.authentication__type == "oauth" else (type(None), str)
         self.validate_correct_type_of_configuration_attribute(
             "authentication__params_oauth__oauth_api_base_url", ptypes
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__client_id", ptypes)
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__client_secret", ptypes)
         self.validate_correct_type_of_configuration_attribute(
             "authentication__params_oauth__jwt_decode_options", (type(None), dict)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__session_cookie", bool)
 
         if self.authentication__params_oauth__session_cookie:
             self.validate_correct_type_of_configuration_attribute(
                 "authentication__params_oauth__cookie", (type(None), dict)
-            )  # noqa E501
+            )
         else:
             self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__cookie", dict)
-        #   secret key: first, from CXG_OAUTH_CLIENT_SECRET environment variable
-        #   second, from config file
-        self.authentication__params_oauth__client_secret = os.environ.get(
-            "CXG_OAUTH_CLIENT_SECRET", self.authentication__params_oauth__client_secret
-        )
 
         self.auth = AuthTypeFactory.create(self.authentication__type, self)
         if self.auth is None:
@@ -286,7 +276,7 @@ class ServerConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("multi_dataset__matrix_cache__max_datasets", int)
         self.validate_correct_type_of_configuration_attribute(
             "multi_dataset__matrix_cache__timelimit_s", (type(None), int, float)
-        )  # noqa E501
+        )
 
         if self.multi_dataset__dataroot is None:
             return

server/common/utils/type_conversion_utils.py

@@ -151,3 +151,17 @@ def convert_pandas_series_to_numpy(series_to_convert: pd.Series, dtype):
         logging.error("Cannot convert a pandas Series object to an integer dtype if it contains NaNs.")
 
     return series_to_convert.to_numpy(dtype)
+
+
+def convert_string_to_value(value: str):
+    """convert a string to value with the most appropriate type"""
+    if value.lower() == "true":
+        return True
+    if value.lower() == "false":
+        return False
+    if value == "null":
+        return None
+    try:
+        return eval(value)
+    except:  # noqa E722
+        return value

server/default_config.py

@@ -204,6 +204,61 @@ dataset:
     enable: true
     lfc_cutoff: 0.01
     top_n: 10
+
+external:
+  # You can retrieve configuration parameters from this config file, the environment,
+  # the AWS secrets manager, or from the "cellxgene launch" command line arguments.
+  # They are applied in that order, meaning that if a parameter is defined in more
+  # than one location, the last one applied takes effect.
+
+  # environment variables:
+  # This section describes how to map environment variables to configuration parameters.
+  # The format is a list defining an environment variable.
+  # Each entry in the list is a dictionary with three entries:
+  # name:  the name of the environment variable
+  # path: the path within the cellxgene configuration to update.
+  # required: (default=False) a boolean.  If true, then it is an error if the environment variable is not set.
+
+  environment:
+     - name: CXG_SECRET_KEY
+       path: [server, app, flask_secret_key]
+       required: false
+     - name: CXG_OAUTH_CLIENT_SECRET
+       path: [server, authentication, params_oauth, client_secret]
+       required: false
+
+  # AWS Secrets Manager
+  # This section describes how to map aws secrets to configuration parameters.
+  # The format is the region for the secrets manager, then a list of secrets.
+  # each secret has a name, and a list of values.
+  # Each entry in the list of values is a dictionary with three entries:
+  # key:  the key of the aws secret.
+  # path: the path within the cellxgene configuration to update.
+  # required: (default=False) a boolean.  If true, then it is an error if the key does not exist in the secret.
+  #
+  # example:
+  # aws_secrets_manager:
+  #   region: us-west-2
+  #    - name: my_first_secret
+  #      values:
+  #      - key: flask_secret_key
+  #        path: [server, app, flask_secret_key]
+  #        required: true
+  #      - key: db_uri
+  #        path: [dataset, user_annotations, hosted_tiledb_array, db_uri]
+  #        required: true
+  #    - name:  my_auth_secret
+  #      values:
+  #      - key: client_secret
+  #        path: [server, authentication, params_oauth, client_secret]
+  #        required: true
+  #      - key: client_id
+  #        path: [server, authentication, params_oauth, client_id]
+  #        required: true
+
+  aws_secrets_manager:
+    region: null
+    secrets: []
 """
 
 

server/test/__init__.py

@@ -117,7 +117,7 @@ def random_string(n):
     return "".join(random.choice(string.ascii_letters) for _ in range(n))
 
 
-def start_test_server(command_line_args=[], app_config=None):
+def start_test_server(command_line_args=[], app_config=None, env=None):
     """
     Command line arguments can be passed in, as well as an app_config.
     This function is meant to be used like this, for example:
@@ -155,7 +155,7 @@ def start_test_server(command_line_args=[], app_config=None):
         command.extend(["-c", config_file])
 
     server = f"http://localhost:{port}"
-    ps = Popen(command)
+    ps = Popen(command, env=env)
 
     for _ in range(10):
         try:
@@ -178,10 +178,10 @@ def stop_test_server(ps):
 
 
 @contextmanager
-def test_server(command_line_args=[], app_config=None):
+def test_server(command_line_args=[], app_config=None, env=None):
     """A context to run the cellxgene server."""
 
-    ps, server = start_test_server(command_line_args, app_config)
+    ps, server = start_test_server(command_line_args, app_config, env)
     try:
         yield server
     finally:

server/test/unit/common/config/__init__.py

@@ -3,6 +3,7 @@ import shutil
 import unittest
 import random
 from unittest import mock
+import yaml
 
 from server.test import FIXTURES_ROOT
 
@@ -133,6 +134,9 @@ class ConfigTests(unittest.TestCase):
         enable_difexp="true",
         lfc_cutoff=0.01,
         top_n=10,
+        environment=None,
+        aws_secrets_manager_region=None,
+        aws_secrets_manager_secrets=[],
         config_file_name="app_config.yml",
     ):
         random_num = random.randrange(999999)
@@ -201,13 +205,17 @@ class ConfigTests(unittest.TestCase):
             top_n=top_n,
             config_file_name=f"temp_dataset_config_{random_num}.yml",
         )
-        with open(server_config) as server_config:
-            with open(dataset_config) as dataset_config:
-                with open(configfile, "w") as app_config_file:
-                    for line in server_config:
-                        app_config_file.write(line)
-                    for line in dataset_config:
-                        app_config_file.write(line)
+        external_config = self.custom_external_config(
+            environment=environment,
+            aws_secrets_manager_region=aws_secrets_manager_region,
+            aws_secrets_manager_secrets=aws_secrets_manager_secrets,
+            config_file_name=f"temp_external_config_{random_num}.yml",
+        )
+
+        with open(configfile, "w") as app_config_file:
+            app_config_file.write(open(server_config).read())
+            app_config_file.write(open(dataset_config).read())
+            app_config_file.write(open(external_config).read())
 
         return configfile
 
@@ -244,3 +252,32 @@ class ConfigTests(unittest.TestCase):
             dataset_config_file.write(dataset_config)
 
         return configfile
+
+    def custom_external_config(
+        self,
+        environment=None,
+        aws_secrets_manager_region=None,
+        aws_secrets_manager_secrets=[],
+        config_file_name="external_config.yaml",
+    ):
+        # set to the default if environment is None
+        if environment is None:
+            environment = [
+                dict(name="CXG_SECRET_KEY", path=["server", "app", "flask_secret_key"], required=False),
+                dict(
+                    name="CXG_OAUTH_CLIENT_SECRET",
+                    path=["server", "authentication", "params_oauth", "client_secret"],
+                    required=False,
+                ),
+            ]
+        external_config = {
+            "external": {
+                "environment": environment,
+                "aws_secrets_manager": {"region": aws_secrets_manager_region, "secrets": aws_secrets_manager_secrets},
+            }
+        }
+
+        configfile = os.path.join(self.tmp_fixtures_directory, config_file_name)
+        with open(configfile, "w") as external_config_file:
+            yaml.dump(external_config, external_config_file)
+        return configfile

server/test/unit/common/config/test_app_config.py

@@ -138,3 +138,77 @@ class AppConfigTest(ConfigTests):
             dataset_changes = app_config.default_dataset_config.changes_from_default()
             self.assertEqual(server_changes, [])
             self.assertEqual(dataset_changes, [("user_annotations__enable", False, True)])
+
+    def test_simple_update_single_config_from_path_and_value(self):
+        """Update a simple config parameter"""
+
+        config = AppConfig()
+        config.server_config.multi_dataset__dataroot = dict(
+            s1=dict(dataroot="my_dataroot_s1", base_url="my_baseurl_s1"),
+            s2=dict(dataroot="my_dataroot_s2", base_url="my_baseurl_s2"),
+        )
+        config.add_dataroot_config("s1")
+        config.add_dataroot_config("s2")
+
+        # test simple value in server
+        config.update_single_config_from_path_and_value(["server", "app", "flask_secret_key"], "mysecret")
+        self.assertEqual(config.server_config.app__flask_secret_key, "mysecret")
+
+        # test simple value in default dataset
+        config.update_single_config_from_path_and_value(
+            ["dataset", "user_annotations", "hosted_tiledb_array", "db_uri"], "mydburi",
+        )
+        self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s1"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s2"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+
+        # test simple value in specific dataset
+        config.update_single_config_from_path_and_value(
+            ["per_dataset_config", "s1", "user_annotations", "hosted_tiledb_array", "db_uri"], "s1dburi"
+        )
+        self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s1"].user_annotations__hosted_tiledb_array__db_uri, "s1dburi")
+        self.assertEqual(config.dataroot_config["s2"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+
+        # error checking
+        bad_paths = [
+            (
+                ["dataset", "does", "not", "exist"],
+                "unknown config parameter at path: '['dataset', 'does', 'not', 'exist']'",
+            ),
+            (["does", "not", "exist"], "path must start with 'server', 'dataset', or 'per_dataset_config'"),
+            ([], "path must start with 'server', 'dataset', or 'per_dataset_config'"),
+            (["per_dataset_config"], "missing dataroot when using per_dataset_config: got '['per_dataset_config']'"),
+            (
+                ["per_dataset_config", "unknown"],
+                "unknown dataroot when using per_dataset_config: got '['per_dataset_config', 'unknown']',"
+                " dataroots specified in config are ['s1', 's2']",
+            ),
+            ([1, 2, 3], "path must be a list of strings, got '[1, 2, 3]'"),
+            ("string", "path must be a list of strings, got 'string'"),
+        ]
+        for bad_path, error_message in bad_paths:
+            with self.assertRaises(ConfigurationError) as config_error:
+                config.update_single_config_from_path_and_value(bad_path, "value")
+
+            self.assertEqual(config_error.exception.message, error_message)
+
+    def test_dict_update_single_config_from_path_and_value(self):
+        """Update a config parameter that has a value of dict"""
+
+        # the path leads to a dict config param, set the config parameter to the new value
+        config = AppConfig()
+        config.update_single_config_from_path_and_value(
+            ["server", "authentication", "params_oauth", "cookie"], dict(key="mykey1", max_age=100)
+        )
+        self.assertEqual(config.server_config.authentication__params_oauth__cookie, dict(key="mykey1", max_age=100))
+
+        # the path leads to an entry within a dict config param, the value is simple
+        config = AppConfig()
+        config.server_config.authentication__params_oauth__cookie = dict(key="mykey1", max_age=100)
+        config.update_single_config_from_path_and_value(
+            ["server", "authentication", "params_oauth", "cookie", "httponly"], True,
+        )
+        self.assertEqual(
+            config.server_config.authentication__params_oauth__cookie, dict(key="mykey1", max_age=100, httponly=True)
+        )

server/test/unit/common/config/test_dataset_config.py

@@ -155,7 +155,7 @@ class TestDatasetConfig(ConfigTests):
         # test for illegal url_dataroots
         for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             with self.assertRaises(ConfigurationError):
                 config.complete_config()
@@ -163,7 +163,7 @@ class TestDatasetConfig(ConfigTests):
         # test for legal url_dataroots
         for legal in ("d", "this.is-okay_", "a/b"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             config.complete_config()
 

server/test/unit/common/config/test_external_config.py

@@ -0,0 +1,231 @@
+import os
+from unittest.mock import patch
+
+import requests
+
+from server.common.errors import ConfigurationError
+from server.common.config.app_config import AppConfig
+from server.test import test_server, FIXTURES_ROOT
+from server.common.utils.type_conversion_utils import convert_string_to_value
+from server.test.unit.common.config import ConfigTests
+
+
+class TestExternalConfig(ConfigTests):
+    def test_type_convert(self):
+        # The values from environment variables and aws secrets are returned as strings.
+        # These values need to be converted to the proper types.
+
+        self.assertEqual(convert_string_to_value("1"), int(1))
+        self.assertEqual(convert_string_to_value("1.1"), float(1.1))
+        self.assertEqual(convert_string_to_value("string"), "string")
+        self.assertEqual(convert_string_to_value("true"), True)
+        self.assertEqual(convert_string_to_value("True"), True)
+        self.assertEqual(convert_string_to_value("false"), False)
+        self.assertEqual(convert_string_to_value("False"), False)
+        self.assertEqual(convert_string_to_value("null"), None)
+        self.assertEqual(convert_string_to_value("None"), None)
+        self.assertEqual(convert_string_to_value("{'a':10, 'b':'string'}"), dict(a=int(10), b="string"))
+
+    def test_environment_variable(self):
+        configfile = self.custom_external_config(
+            environment=[
+                dict(name="DATAPATH", path=["server", "single_dataset", "datapath"], required=True),
+                dict(name="DIFFEXP", path=["dataset", "diffexp", "enable"], required=True),
+            ],
+            config_file_name="environment_external_config.yaml",
+        )
+
+        env = os.environ
+        env["DATAPATH"] = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+        env["DIFFEXP"] = "False"
+        with test_server(command_line_args=["-c", configfile], env=env) as server:
+            session = requests.Session()
+            response = session.get(f"{server}/api/v0.2/config")
+            data_config = response.json()
+            self.assertEqual(data_config["config"]["displayNames"]["dataset"], "pbmc3k")
+            self.assertTrue(data_config["config"]["parameters"]["disable-diffexp"])
+
+        env["DATAPATH"] = f"{FIXTURES_ROOT}/a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad"
+        env["DIFFEXP"] = "True"
+        with test_server(command_line_args=["-c", configfile], env=env) as server:
+            session = requests.Session()
+            response = session.get(f"{server}/api/v0.2/config")
+            data_config = response.json()
+            self.assertEqual(data_config["config"]["displayNames"]["dataset"], "a95c59b4-7f5d-4b80-ad53-a694834ca18b")
+            self.assertFalse(data_config["config"]["parameters"]["disable-diffexp"])
+
+    def test_environment_variable_errors(self):
+
+        # no name
+        app_config = AppConfig()
+        app_config.external_config.environment = [dict(required=True, path=["this", "is", "a", "path"])]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'name' is missing")
+
+        # required has wrong type
+        app_config = AppConfig()
+        app_config.external_config.environment = [
+            dict(name="myenvar", required="optional", path=["this", "is", "a", "path"])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'required' must be a bool")
+
+        # no path
+        app_config = AppConfig()
+        app_config.external_config.environment = [dict(name="myenvar", required=True)]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'path' is missing")
+
+        # required environment variable is not set
+        app_config = AppConfig()
+        app_config.external_config.environment = [
+            dict(name="THIS_ENV_IS_NOT_SET", required=True, path=["this", "is", "a", "path"])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "required environment variable 'THIS_ENV_IS_NOT_SET' not set")
+
+    @patch("server.common.config.external_config.get_secret_key")
+    def test_aws_secrets_manager(self, mock_get_secret_key):
+        mock_get_secret_key.return_value = {
+            "oauth_client_secret": "mock_oauth_secret",
+            "db_uri": "mock_db_uri",
+        }
+        configfile = self.custom_external_config(
+            aws_secrets_manager_region="us-west-2",
+            aws_secrets_manager_secrets=[
+                dict(
+                    name="my_secret",
+                    values=[
+                        dict(key="flask_secret_key", path=["server", "app", "flask_secret_key"], required=False),
+                        dict(
+                            key="db_uri",
+                            path=["dataset", "user_annotations", "hosted_tiledb_array", "db_uri"],
+                            required=True,
+                        ),
+                        dict(
+                            key="oauth_client_secret",
+                            path=["server", "authentication", "params_oauth", "client_secret"],
+                            required=True,
+                        ),
+                    ],
+                )
+            ],
+            config_file_name="secret_external_config.yaml",
+        )
+
+        app_config = AppConfig()
+        app_config.update_from_config_file(configfile)
+        app_config.server_config.single_dataset__datapath = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+        app_config.server_config.app__flask_secret_key = "original"
+        app_config.server_config.single_dataset__datapath = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+
+        app_config.complete_config()
+
+        self.assertEqual(app_config.server_config.app__flask_secret_key, "original")
+        self.assertEqual(app_config.server_config.authentication__params_oauth__client_secret, "mock_oauth_secret")
+        self.assertEqual(app_config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mock_db_uri")
+
+    @patch("server.common.config.external_config.get_secret_key")
+    def test_aws_secrets_manager_error(self, mock_get_secret_key):
+        mock_get_secret_key.return_value = {
+            "oauth_client_secret": "mock_oauth_secret",
+            "db_uri": "mock_db_uri",
+        }
+
+        # no region
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = None
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="secret1", values=[dict(key="key1", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(
+            config_error.exception.message,
+            "Invalid type for attribute: aws_secrets_manager__region, expected type str, got NoneType",
+        )
+
+        # missing secret name
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(values=[dict(key="db_uri", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'name' is missing")
+
+        # secret name wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name=1, values=[dict(key="db_uri", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'name' must be a string")
+
+        # missing values name
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [dict(name="mysecret")]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'values' is missing")
+
+        # values wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=dict(key="db_uri", required=True, path=["this", "is", "my", "path"]))
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'values' must be a list")
+
+        # entry missing key
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "missing 'key' in secret values: mysecret")
+
+        # entry required is wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(key="db_uri", required="optional", path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "wrong type for 'required' in secret values: mysecret")
+
+        # entry missing path
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(key="db_uri", required=True)])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "missing 'path' in secret values: mysecret")
+
+        # secret missing required key
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(
+                name="mysecret",
+                values=[dict(key="KEY_DOES_NOT_EXIST", required=True, path=["this", "is", "a", "path"])],
+            )
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "required secret 'mysecret:KEY_DOES_NOT_EXIST' not set")

server/test/unit/common/config/test_server_config.py

@@ -105,13 +105,14 @@ class TestServerConfig(ConfigTests):
         self.config = AppConfig()
         self.config.server_config.handle_app(self.context)
         self.assertEqual(self.config.server_config.app__port, 4008)
+        del os.environ["CXG_SERVER_PORT"]
 
     def test_handle_app__can_get_secret_key_from_envvar_or_config_file_with_envvar_given_preference(self):
         config = self.get_config(flask_secret_key="KEY_FROM_FILE")
         self.assertEqual(config.server_config.app__flask_secret_key, "KEY_FROM_FILE")
 
         os.environ["CXG_SECRET_KEY"] = "KEY_FROM_ENV"
-        config.server_config.handle_app(self.context)
+        config.external_config.handle_environment(self.context)
         self.assertEqual(config.server_config.app__flask_secret_key, "KEY_FROM_ENV")
 
     def test_handle_app__sets_web_base_url(self):
@@ -124,7 +125,7 @@ class TestServerConfig(ConfigTests):
         self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "KEY_FROM_FILE")
 
         os.environ["CXG_OAUTH_CLIENT_SECRET"] = "KEY_FROM_ENV"
-        config.server_config.handle_authentication()
+        config.external_config.handle_environment(self.context)
 
         self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "KEY_FROM_ENV")
 
@@ -215,7 +216,7 @@ class TestServerConfig(ConfigTests):
         # test for illegal url_dataroots
         for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
             self.config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             with self.assertRaises(ConfigurationError):
                 self.config.complete_config()
@@ -224,7 +225,7 @@ class TestServerConfig(ConfigTests):
         # test for legal url_dataroots
         for legal in ("d", "this.is-okay_", "a/b"):
             self.config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             self.config.complete_config()