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

@@ -126,7 +126,8 @@ def dataroot_test_index():
     data += "<body><H1>Welcome to cellxgene</H1>"
 
     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)

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:"):
+            if path.startswith("s3://"):
-                bucket = urlparse(path).netloc
+                region_name = discover_s3_region_name(path)
-                client = boto3.client("s3")
+                if region_name is None:
-                try:
+                    raise ConfigurationError(f"Unable to discover s3 region name from {path}")
-                    res = client.head_bucket(Bucket=bucket)
+            else:
-                except botocore.exceptions.ClientError:
+                region_name = None
-                    raise ConfigurationError(f"Unable to determine region from {path}")
+            self.data_locator__s3__region_name = region_name
-
-                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}")
 
     def handle_presentation(self, context):
         self.__check_attr("presentation__max_categories", int)

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

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

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.")
 

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=<location to your S3 bucket>
     $ CXG_CONFIG_FILE=<location to your 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

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)