From 7e7ed74b925a8078d2de558997fac2bee6d3649e Mon Sep 17 00:00:00 2001 From: bmccandless Date: Wed, 15 Apr 2020 14:33:40 -0700 Subject: [PATCH] Provide a hook into the AWS Secret Manager for the flask secret key (#1398) Also, the secret manager required a region name, so there was some refactoring around how regions are handled. Fixes #1239 --- server/app/app.py | 3 +- server/common/app_config.py | 23 ++++++--------- server/common/data_locator.py | 31 +++++++++++++++++++-- server/common/health.py | 2 +- server/data_common/matrix_loader.py | 3 +- server/eb/README.md | 25 ++++++++++++++--- server/eb/app.py | 43 +++++++++++++++++++++++++++-- 7 files changed, 102 insertions(+), 28 deletions(-) diff --git a/server/app/app.py b/server/app/app.py index ede90596..018a1097 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -126,7 +126,8 @@ def dataroot_test_index(): data += "

Welcome to cellxgene

" config = current_app.app_config - locator = DataLocator(config.multi_dataset__dataroot, app_config=config) + locator = DataLocator(config.multi_dataset__dataroot, + region_name=config.data_locator__s3__region_name) datasets = [] for fname in locator.ls(): location = path_join(config.multi_dataset__dataroot, fname) diff --git a/server/common/app_config.py b/server/common/app_config.py index 2090b889..e71bd344 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -6,8 +6,6 @@ import sys from urllib.parse import urlparse import yaml import copy -import boto3 -import botocore from server.common.default_config import get_default_config from server.common.errors import ConfigurationError, DatasetAccessError, OntologyLoadFailure @@ -17,6 +15,7 @@ import warnings from server.common.annotations import AnnotationsLocalFile from server.common.utils import custom_format_warning import server.compute.diffexp_cxg as diffexp_tiledb +from server.common.data_locator import discover_s3_region_name DEFAULT_SERVER_PORT = int(os.environ.get("CXG_SERVER_PORT", "5005")) # anything bigger than this will generate a special message @@ -266,19 +265,13 @@ class AppConfig(object): self.__check_attr("data_locator__s3__region_name", (type(None), bool, str)) if self.data_locator__s3__region_name is True: path = self.single_dataset__datapath or self.multi_dataset__dataroot - if path and path.startswith("s3:"): - bucket = urlparse(path).netloc - client = boto3.client("s3") - try: - res = client.head_bucket(Bucket=bucket) - except botocore.exceptions.ClientError: - raise ConfigurationError(f"Unable to determine region from {path}") - - region = res.get("ResponseMetadata", {}).get("HTTPHeaders", {}).get("x-amz-bucket-region") - if region: - self.data_locator__s3__region_name = region - else: - raise ConfigurationError(f"Unable to determine region from {path}") + if path.startswith("s3://"): + region_name = discover_s3_region_name(path) + if region_name is None: + raise ConfigurationError(f"Unable to discover s3 region name from {path}") + else: + region_name = None + self.data_locator__s3__region_name = region_name def handle_presentation(self, context): self.__check_attr("presentation__max_categories", int) diff --git a/server/common/data_locator.py b/server/common/data_locator.py index 71325f66..5b561302 100644 --- a/server/common/data_locator.py +++ b/server/common/data_locator.py @@ -2,6 +2,9 @@ import os import tempfile import fsspec from datetime import datetime +import boto3 +import botocore +from urllib.parse import urlparse class DataLocator: @@ -25,7 +28,7 @@ class DataLocator: """ - def __init__(self, uri_or_path, app_config=None): + def __init__(self, uri_or_path, region_name=None): if isinstance(uri_or_path, DataLocator): locator = uri_or_path self.uri_or_path = locator.uri_or_path @@ -39,9 +42,9 @@ class DataLocator: self.cname = self.path if self.protocol == "file" else self.uri_or_path # fsspec.filesystem will throw RuntimeError if the protocol is unsupported - if self.protocol == "s3" and app_config and app_config.data_locator__s3__region_name: + if self.protocol == "s3" and region_name: self.fs = fsspec.filesystem( - self.protocol, config_kwargs={"region_name": app_config.data_locator__s3__region_name} + self.protocol, config_kwargs={"region_name": region_name} ) else: self.fs = fsspec.filesystem(self.protocol) @@ -125,3 +128,25 @@ class LocalFilePath: def __exit__(self, *args): if self.delete: os.unlink(self.tmp_path) + + +def discover_s3_region_name(uri): + """If this is an s3 protocol, discover and return the (aws) region name. + If a return name could not be discovered, or if the uri is not an s3 protocol, return None.""" + + protocol, _ = DataLocator._get_protocol_and_path(uri) + if protocol == "s3": + bucket = urlparse(uri).netloc + client = boto3.client("s3") + try: + res = client.head_bucket(Bucket=bucket) + except botocore.exceptions.ClientError: + return None + + region = res.get("ResponseMetadata", {}).get("HTTPHeaders", {}).get("x-amz-bucket-region") + if region: + return region + else: + return None + + return None diff --git a/server/common/health.py b/server/common/health.py index 4bad3141..0fee633d 100644 --- a/server/common/health.py +++ b/server/common/health.py @@ -10,7 +10,7 @@ def _is_accessible(path, config): return True try: - dl = DataLocator(path, config) + dl = DataLocator(path, region_name=config.data_locator__s3__region_name) return dl.exists() except RuntimeError: return False diff --git a/server/data_common/matrix_loader.py b/server/data_common/matrix_loader.py index 40f797bf..fd386b14 100644 --- a/server/data_common/matrix_loader.py +++ b/server/data_common/matrix_loader.py @@ -214,7 +214,8 @@ class MatrixDataType(Enum): class MatrixDataLoader(object): def __init__(self, location, matrix_data_type=None, app_config=None): """ location can be a string or DataLocator """ - self.location = DataLocator(location, app_config) + region_name = None if app_config is None else app_config.data_locator__s3__region_name + self.location = DataLocator(location, region_name=region_name) if not self.location.exists(): raise DatasetAccessError("Dataset does not exist.") diff --git a/server/eb/README.md b/server/eb/README.md index 0d529045..4d082d40 100644 --- a/server/eb/README.md +++ b/server/eb/README.md @@ -93,8 +93,23 @@ There are many more options to these commands that may be important or necessary ``` $ make build ``` + +6. Flask secret key + + The application requires as secret key to be provided to flask, the web framework used by cellxgene. + There are three ways to provide the secret key: -6. Create an environment + - In the configuration file: update the server/flask_secret_key attribute. + - An environment variable: CXG_SECRET_KEY + - Managed by the AWS Secret Manager + + If using the AWS Secret Manager, then the secret name is passed as an environment variable: CXG_AWS_SECRET_NAME. + The secret must contain a key with the name "flask_secret_key". + Likely you have located the AWS Secret Manager in the same AWS region as the dataroot. If that is not the case + then the AWS Secret Manager region name can be specified in an environment variable: CXG_AWS_SECRET_REGION_NAME. + + +7. Create an environment ``` # name of the environment @@ -106,23 +121,25 @@ There are many more options to these commands that may be important or necessary # One or both of the following environment variables needs to be set $ CXG_DATAROOT= $ CXG_CONFIG_FILE= + + # Potentially also set envvars for the sercret key. $ eb create $EB_ENV --instance-type $EB_INSTANCE \ --envvars CXG_DATAROOT=$CXG_DATAROOT,CXG_CONFIG_FILE=$CXG_CONFIG_FILE ``` -7. Give the elastic beanstalk environment access to the S3 bucket. +8. Give the elastic beanstalk environment access to the S3 bucket. This link may provide some useful information: https://aws.amazon.com/premiumsupport/knowledge-center/elastic-beanstalk-s3-bucket-instance/ -8. Deploy the application +9. Deploy the application ``` $ eb deploy $EB_ENV ``` -9. Open the application in a browser +10. Open the application in a browser ``` $ eb open $EB_ENV diff --git a/server/eb/app.py b/server/eb/app.py index 29807634..d698a267 100644 --- a/server/eb/app.py +++ b/server/eb/app.py @@ -4,6 +4,8 @@ import sys import os import logging from flask_talisman import Talisman +import boto3 +import json if os.path.isdir("/opt/python/log"): # This is the standard location where Amazon EC2 instances store the application logs. @@ -23,12 +25,34 @@ sys.path.append(SERVERDIR) try: from server.common.app_config import AppConfig from server.app.app import Server - from server.common.data_locator import DataLocator + from server.common.data_locator import DataLocator, discover_s3_region_name except Exception: logging.critical("Exception importing server modules", exc_info=True) sys.exit(1) +def get_flask_secret_key(region_name, secret_name): + session = boto3.session.Session() + client = session.client( + service_name='secretsmanager', + region_name=region_name + ) + + try: + get_secret_value_response = client.get_secret_value( + SecretId=secret_name + ) + if 'SecretString' in get_secret_value_response: + var = get_secret_value_response['SecretString'] + secret = json.loads(var) + return secret.get("flask_secret_key") + except Exception: + logging.critical("Caught exception during get_secret_key", exc_info=True) + sys.exit(1) + + return None + + class WSGIServer(Server): def __init__(self, app_config): super().__init__(app_config) @@ -44,8 +68,12 @@ try: dataroot = os.getenv("CXG_DATAROOT") config_file = os.getenv("CXG_CONFIG_FILE") + secret_name = os.getenv("CXG_AWS_SECRET_NAME") + secret_region_name = os.getenv("CXG_AWS_SECRET_REGION_NAME") + if config_file: - config_location = DataLocator(config_file) + region_name = discover_s3_region_name(config_file) + config_location = DataLocator(config_file, region_name) if config_location.exists(): with config_location.local_handle() as lh: logging.info(f"Configuration from {config_file}") @@ -66,6 +94,14 @@ try: logging.info(f"Configuration from CXG_DATAROOT") app_config.update(multi_dataset__dataroot=dataroot) + if secret_name: + if secret_region_name is None: + secret_region_name = discover_s3_region_name(app_config.multi_dataset__dataroot) + if not secret_region_name: + logging.error(f"Expected to discover the s3 region name from {app_config.multi_dataset__dataroot}") + flask_secret_key = get_flask_secret_key(secret_region_name, secret_name) + app_config.update(server__flask_secret_key=flask_secret_key) + # features are unsupported in the current hosted server app_config.update( user_annotations__enable=False, @@ -77,7 +113,8 @@ try: if not app_config.server__flask_secret_key: logging.critical( - f"flask_secret_key is not provided. Either set in config file, or in CXG_SECRET_KEY environment variable" + f"flask_secret_key is not provided. Either set in config file, CXG_SECRET_KEY environment variable, " + "or in AWS Secret Manager" ) sys.exit(1)