From b034055c35bdabb8f9e2477cf9cb410b741157c1 Mon Sep 17 00:00:00 2001 From: Madison Dunitz Date: Fri, 14 Aug 2020 18:17:21 -0500 Subject: [PATCH] update to get_secrets_key (#1755) * raise exception when get_secrets fails, get db_uri and set as a default_dataset_config var * log as info not an error --- server/common/aws_secret_utils.py | 29 +++++++++++++++++----- server/common/errors.py | 1 + server/eb/app.py | 7 +++++- server/test/unit/common/test_app_config.py | 20 +++++++++------ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/server/common/aws_secret_utils.py b/server/common/aws_secret_utils.py index a88d9e7d..47f7690b 100644 --- a/server/common/aws_secret_utils.py +++ b/server/common/aws_secret_utils.py @@ -6,6 +6,7 @@ import boto3 from flask import json from server.common.data_locator import discover_s3_region_name +from server.common.errors import SecretKeyRetrievalError def handle_config_from_secret(app_config): @@ -33,12 +34,16 @@ def handle_config_from_secret(app_config): if not secrets: return - keyattrs = ( + server_attrs = ( ("flask_secret_key", "app__flask_secret_key"), ("oauth_client_secret", "authentication__params_oauth__client_secret"), ) + default_dataset_attrs = ( + ("db_uri", "user_annotations__hosted_tiledb_array__db_uri"), + ) - for key, attr in keyattrs: + # update server configuration attributes + for key, attr in server_attrs: cur_val = getattr(app_config.server_config, attr) if cur_val: continue @@ -46,9 +51,21 @@ def handle_config_from_secret(app_config): # replace the attr with the secret if it is not set val = secrets.get(key) if val: - logging.error(f"set {attr} from secret") + logging.info(f"set {attr} from secret") app_config.update_server_config(**{attr : val}) + # update default dataset configuration attributes + for key, attr in default_dataset_attrs: + cur_val = getattr(app_config.default_dataset_config, attr) + if cur_val: + continue + + # replace the attr with the secret if it is not set + val = secrets.get(key) + if val: + logging.info(f"set {attr} from secret") + app_config.update_default_dataset_config(**{attr : val}) + def get_secret_key(region_name, secret_name): session = boto3.session.Session() @@ -60,8 +77,8 @@ def get_secret_key(region_name, secret_name): var = get_secret_value_response["SecretString"] secret = json.loads(var) return secret - except Exception: - logging.critical("Caught exception during get_secret_key", exc_info=True) - sys.exit(1) + except Exception as e: + logging.critical(f"Caught exception during get_secret_key, {e}", exc_info=True) + raise SecretKeyRetrievalError return None diff --git a/server/common/errors.py b/server/common/errors.py index effcaec2..5e8281e6 100644 --- a/server/common/errors.py +++ b/server/common/errors.py @@ -54,3 +54,4 @@ define_request_exception( define_exception("OntologyLoadFailure", "Raised when reading the ontology file fails") define_exception("ConfigurationError", "Raised when checking configuration errors") define_exception("PrepareError", "Raised when data is misprepared") +define_exception("SecretKeyRetrievalError", "Raised when get_secret_key from AWS fails") diff --git a/server/eb/app.py b/server/eb/app.py index 1e047010..9aa86263 100644 --- a/server/eb/app.py +++ b/server/eb/app.py @@ -9,6 +9,8 @@ import logging from flask_talisman import Talisman from server.common.aws_secret_utils import handle_config_from_secret +from server.common.errors import SecretKeyRetrievalError + if os.path.isdir("/opt/python/log"): # This is the standard location where Amazon EC2 instances store the application logs. @@ -149,7 +151,10 @@ try: app_config.update_server_config(multi_dataset__dataroot=dataroot) # update from secret manager - handle_config_from_secret(app_config) + try: + handle_config_from_secret(app_config) + except SecretKeyRetrievalError: + sys.exit(1) # features are unsupported in the current hosted server app_config.update_default_dataset_config( diff --git a/server/test/unit/common/test_app_config.py b/server/test/unit/common/test_app_config.py index ed21e3cc..e7fc7434 100644 --- a/server/test/unit/common/test_app_config.py +++ b/server/test/unit/common/test_app_config.py @@ -112,19 +112,25 @@ class AppConfigTest(unittest.TestCase): def test_get_config_vars_from_aws_secrets(self, mock_get_secret_key): mock_get_secret_key.return_value = { "flask_secret_key": "mock_flask_secret", - "oauth_client_secret": "mock_oauth_secret" + "oauth_client_secret": "mock_oauth_secret", + "db_uri": "mock_db_uri" } config = AppConfig() - with self.assertLogs(level="ERROR") as logger: + with self.assertLogs(level="INFO") as logger: from server.common.aws_secret_utils import handle_config_from_secret # should not throw error - # "AttributeError: 'ServerConfig' object has no attribute 'user_annotations__hosted_tiledb_array__db_uri'" + # "AttributeError: 'XConfig' object has no attribute 'x'" handle_config_from_secret(config) - # should throw 2 errors (one for each var set from a secret) - self.assertEqual(len(logger.output), 2) - self.assertIn('ERROR:root:set app__flask_secret_key from secret', logger.output[0]) - self.assertIn('ERROR:root:set authentication__params_oauth__client_secret from secret', logger.output[1]) + # should log 3 lines (one for each var set from a secret) + self.assertEqual(len(logger.output), 3) + self.assertIn('INFO:root:set app__flask_secret_key from secret', logger.output[0]) + self.assertIn('INFO:root:set authentication__params_oauth__client_secret from secret', logger.output[1]) + self.assertIn('INFO:root:set user_annotations__hosted_tiledb_array__db_uri from secret', logger.output[2]) + self.assertEqual(config.server_config.app__flask_secret_key, "mock_flask_secret") + self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "mock_oauth_secret") + self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mock_db_uri") +