Add a function to check the configuration for errors. (#1919)

This can be used as a sanity check before a deployment:

  chanzuckerberg/single-cell#63

server/common/config/server_config.py

@@ -114,7 +114,7 @@ class ServerConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("app__port", (type(None), int))
         self.validate_correct_type_of_configuration_attribute("app__open_browser", bool)
         self.validate_correct_type_of_configuration_attribute("app__force_https", bool)
-        self.validate_correct_type_of_configuration_attribute("app__flask_secret_key", (type(None), str))
+        self.validate_correct_type_of_configuration_attribute("app__flask_secret_key", str)
         self.validate_correct_type_of_configuration_attribute("app__generate_cache_control_headers", bool)
         self.validate_correct_type_of_configuration_attribute("app__server_timing_headers", bool)
         self.validate_correct_type_of_configuration_attribute("app__csp_directives", (type(None), dict))

server/eb/check_config.py

@@ -0,0 +1,39 @@
+import sys
+import argparse
+import yaml
+
+from server.common.config.app_config import AppConfig
+
+
+def main():
+    parser = argparse.ArgumentParser("A script to check hosted configuration files")
+    parser.add_argument("config_file", help="the configuration file")
+    parser.add_argument(
+        "-s",
+        "--show",
+        default=False,
+        action="store_true",
+        help="print the configuration. NOTE: this may print secret values to stdout",
+    )
+
+    args = parser.parse_args()
+
+    app_config = AppConfig()
+    app_config.update_from_config_file(args.config_file)
+    try:
+        app_config.complete_config()
+    except Exception as e:
+        print(f"Error: {str(e)}")
+        print("FAIL:", args.config_file)
+        sys.exit(1)
+
+    if args.show:
+        yaml_config = app_config.config_to_dict()
+        yaml.dump(yaml_config, sys.stdout)
+
+    print("PASS:", args.config_file)
+    sys.exit(0)
+
+
+if __name__ == "__main__":
+    main()

server/test/__init__.py

@@ -34,7 +34,7 @@ def data_with_tmp_tiledb_annotations(ext: MatrixDataType):
     data_locator = DataLocator(fname)
     config = AppConfig()
     config.update_server_config(
-        multi_dataset__dataroot=data_locator.path, authentication__type="test",
+        app__flask_secret_key="secret", multi_dataset__dataroot=data_locator.path, authentication__type="test",
     )
     config.update_default_dataset_config(
         embeddings__names=["umap"],
@@ -64,7 +64,10 @@ def data_with_tmp_annotations(ext: MatrixDataType, annotations_fixture=False):
     data_locator = DataLocator(fname)
     config = AppConfig()
     config.update_server_config(
-        single_dataset__obs_names=None, single_dataset__var_names=None, single_dataset__datapath=data_locator.path
+        app__flask_secret_key="secret",
+        single_dataset__obs_names=None,
+        single_dataset__var_names=None,
+        single_dataset__datapath=data_locator.path,
     )
     config.update_default_dataset_config(
         embeddings__names=["umap"], presentation__max_categories=100, diffexp__lfc_cutoff=0.01,
@@ -97,6 +100,7 @@ def skip_if(condition, reason: str):
 def app_config(data_locator, backed=False, extra_server_config={}, extra_dataset_config={}):
     config = AppConfig()
     config.update_server_config(
+        app__flask_secret_key="secret",
         single_dataset__obs_names=None,
         single_dataset__var_names=None,
         adaptor__anndata_adaptor__backed=backed,

server/test/unit/auth/test_auth.py

@@ -11,13 +11,14 @@ class AuthTest(unittest.TestCase):
         self.dataset_dataroot = FIXTURES_ROOT
 
     def test_auth_none(self):
-        c = AppConfig()
+        app_config = AppConfig()
-        c.update_server_config(authentication__type=None, multi_dataset__dataroot=self.dataset_dataroot)
+        app_config.update_server_config(app__flask_secret_key="secret")
-        c.update_default_dataset_config(user_annotations__enable=False)
+        app_config.update_server_config(authentication__type=None, multi_dataset__dataroot=self.dataset_dataroot)
+        app_config.update_default_dataset_config(user_annotations__enable=False)
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
             userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
@@ -25,12 +26,13 @@ class AuthTest(unittest.TestCase):
             self.assertIsNone(userinfo)
 
     def test_auth_session(self):
-        c = AppConfig()
+        app_config = AppConfig()
-        c.update_server_config(authentication__type="session", multi_dataset__dataroot=self.dataset_dataroot)
+        app_config.update_server_config(app__flask_secret_key="secret")
-        c.update_default_dataset_config(user_annotations__enable=True)
+        app_config.update_server_config(authentication__type="session", multi_dataset__dataroot=self.dataset_dataroot)
-        c.complete_config()
+        app_config.update_default_dataset_config(user_annotations__enable=True)
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
             userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
@@ -40,9 +42,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(userinfo["userinfo"]["username"], "anonymous")
 
     def test_auth_test(self):
-        c = AppConfig()
+        app_config = AppConfig()
-        c.update_server_config(authentication__type="test")
+        app_config.update_server_config(app__flask_secret_key="secret")
-        c.update_server_config(
+        app_config.update_server_config(authentication__type="test")
+        app_config.update_server_config(
             multi_dataset__dataroot=dict(
                 a1=dict(dataroot=self.dataset_dataroot, base_url="auth"),
                 a2=dict(dataroot=self.dataset_dataroot, base_url="no-auth"),
@@ -50,12 +53,12 @@ class AuthTest(unittest.TestCase):
         )
 
         # specialize the configs
-        c.add_dataroot_config("a1", app__authentication_enable=True, user_annotations__enable=True)
+        app_config.add_dataroot_config("a1", app__authentication_enable=True, user_annotations__enable=True)
-        c.add_dataroot_config("a2", app__authentication_enable=False, user_annotations__enable=False)
+        app_config.add_dataroot_config("a2", app__authentication_enable=False, user_annotations__enable=False)
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
 
             # auth datasets
@@ -102,20 +105,21 @@ class AuthTest(unittest.TestCase):
             self.assertFalse(config["config"]["parameters"]["annotations"])
 
             # login with a picture
-            r = session.get(f"{server}/{login_uri}&picture=myimage.png")
+            session.get(f"{server}/{login_uri}&picture=myimage.png")
             userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
             self.assertTrue(userinfo["userinfo"]["is_authenticated"])
             self.assertEqual(userinfo["userinfo"]["picture"], "myimage.png")
 
     def test_auth_test_single(self):
-        c = AppConfig()
+        app_config = AppConfig()
-        c.update_server_config(
+        app_config.update_server_config(app__flask_secret_key="secret")
+        app_config.update_server_config(
             authentication__type="test", single_dataset__datapath=f"{self.dataset_dataroot}/pbmc3k.cxg"
         )
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
@@ -130,10 +134,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(login_uri, "/login")
             self.assertEqual(logout_uri, "/logout")
 
-            r = session.get(f"{server}/{login_uri}")
+            response = session.get(f"{server}/{login_uri}")
             # check that the login redirect worked
-            self.assertEqual(r.history[0].status_code, 302)
+            self.assertEqual(response.history[0].status_code, 302)
-            self.assertEqual(r.url, f"{server}/")
+            self.assertEqual(response.url, f"{server}/")
 
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
@@ -141,10 +145,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(userinfo["userinfo"]["username"], "test_account")
             self.assertTrue(config["config"]["parameters"]["annotations"])
 
-            r = session.get(f"{server}/{logout_uri}")
+            response = session.get(f"{server}/{logout_uri}")
             # check that the logout redirect worked
-            self.assertEqual(r.history[0].status_code, 302)
+            self.assertEqual(response.history[0].status_code, 302)
-            self.assertEqual(r.url, f"{server}/")
+            self.assertEqual(response.url, f"{server}/")
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
             self.assertFalse(userinfo["userinfo"]["is_authenticated"])

server/test/unit/auth/test_oauth.py

@@ -160,12 +160,14 @@ class AuthTest(unittest.TestCase):
     def test_auth_oauth_session(self):
         # test with session cookies
         app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
         app_config.update_server_config(authentication__params_oauth__session_cookie=True,)
         self.auth_flow(app_config)
 
     def test_auth_oauth_cookie(self):
         # test with specified cookie
         app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
         app_config.update_server_config(
             authentication__params_oauth__session_cookie=False,
             authentication__params_oauth__cookie=dict(key="test_cxguser", httponly=True, max_age=60),

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

@@ -31,7 +31,7 @@ class ConfigTests(unittest.TestCase):
         port="null",
         open_browser="false",
         force_https="false",
-        flask_secret_key="null",
+        flask_secret_key="secret",
         generate_cache_control_headers="false",
         server_timing_headers="false",
         csp_directives="null",
@@ -82,7 +82,7 @@ class ConfigTests(unittest.TestCase):
         port="null",
         open_browser="false",
         force_https="false",
-        flask_secret_key="null",
+        flask_secret_key="secret",
         generate_cache_control_headers="false",
         server_timing_headers="false",
         csp_directives="null",

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

@@ -15,6 +15,7 @@ class AppConfigTest(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -106,6 +107,8 @@ class AppConfigTest(ConfigTests):
             with open(configfile, "w") as fconfig:
                 config = """
                 server:
+                    app:
+                        flask_secret_key: secret
                     multi_dataset:
                         dataroot: test_dataroot
 
@@ -116,7 +119,10 @@ class AppConfigTest(ConfigTests):
             app_config.update_from_config_file(configfile)
             server_changes = app_config.server_config.changes_from_default()
             dataset_changes = app_config.default_dataset_config.changes_from_default()
-            self.assertEqual(server_changes, [("multi_dataset__dataroot", "test_dataroot", None)])
+            self.assertEqual(
+                server_changes,
+                [("app__flask_secret_key", "secret", None), ("multi_dataset__dataroot", "test_dataroot", None)],
+            )
             self.assertEqual(dataset_changes, [])
 
     def test_configfile_no_server_section(self):

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

@@ -10,6 +10,7 @@ class BaseConfigTest(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -47,6 +48,7 @@ class BaseConfigTest(ConfigTests):
             server_changes,
             [
                 ("app__verbose", True, False),
+                ("app__flask_secret_key", "secret", None),
                 ("multi_dataset__dataroot", FIXTURES_ROOT, None),
                 ("multi_dataset__matrix_cache__timelimit_s", 5, 30),
                 ("data_locator__s3__region_name", "us-east-1", True),

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

@@ -19,6 +19,7 @@ class TestDatasetConfig(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.dataset_config = self.config.default_dataset_config
         self.config.complete_config()
@@ -155,7 +156,8 @@ 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": f"{PROJECT_ROOT}/example-dataset"}}
+                app__flask_secret_key="secret",
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}},
             )
             with self.assertRaises(ConfigurationError):
                 config.complete_config()
@@ -163,17 +165,19 @@ 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": f"{PROJECT_ROOT}/example-dataset"}}
+                app__flask_secret_key="secret",
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}},
             )
             config.complete_config()
 
         # test that multi dataroots work end to end
         config.update_server_config(
+            app__flask_secret_key="secret",
             multi_dataset__dataroot=dict(
                 s1=dict(dataroot=f"{PROJECT_ROOT}/example-dataset", base_url="set1/1/2"),
                 s2=dict(dataroot=f"{FIXTURES_ROOT}", base_url="set2"),
                 s3=dict(dataroot=f"{FIXTURES_ROOT}", base_url="set3"),
-            )
+            ),
         )
 
         # Change this default to test if the dataroot overrides below work.

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

@@ -23,6 +23,7 @@ class TestServerConfig(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -103,6 +104,7 @@ class TestServerConfig(ConfigTests):
         # Note if the port is set in the config file it will NOT be overwritten by a different envvar
         os.environ["CXG_SERVER_PORT"] = "4008"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.server_config.handle_app(self.context)
         self.assertEqual(self.config.server_config.app__port, 4008)
         del os.environ["CXG_SERVER_PORT"]
@@ -152,6 +154,7 @@ class TestServerConfig(ConfigTests):
         config = AppConfig()
         backend_port = find_available_port("localhost", 10000)
         config.update_server_config(
+            app__flask_secret_key="secret",
             app__api_base_url=f"http://localhost:{backend_port}/additional/path",
             multi_dataset__dataroot=f"{PROJECT_ROOT}/example-dataset",
         )

server/test/unit/eb/test_eb.py

@@ -6,6 +6,7 @@ from server.test import PROJECT_ROOT, FIXTURES_ROOT
 from server.common.config.app_config import AppConfig
 from contextlib import contextmanager
 import time
+import os
 
 
 @contextmanager
@@ -34,12 +35,12 @@ class Elastic_Beanstalk_Test(unittest.TestCase):
         tempdir = tempfile.TemporaryDirectory(dir=f"{PROJECT_ROOT}/server")
         tempdirname = tempdir.name
 
-        c = AppConfig()
+        config = AppConfig()
         # test that eb works
-        c.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}", app__flask_secret_key="open sesame")
+        config.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}", app__flask_secret_key="open sesame")
 
-        c.complete_config()
+        config.complete_config()
-        c.write_config(f"{tempdirname}/config.yaml")
+        config.write_config(f"{tempdirname}/config.yaml")
 
         subprocess.check_call(f"git ls-files . | cpio -pdm {tempdirname}", cwd=f"{PROJECT_ROOT}/server/eb", shell=True)
         subprocess.check_call(["make", "build"], cwd=tempdirname)
@@ -50,3 +51,33 @@ class Elastic_Beanstalk_Test(unittest.TestCase):
             r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
             data_config = r.json()
             assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"
+
+    def test_config(self):
+        check_config_script = os.path.join(PROJECT_ROOT, "server", "eb", "check_config.py")
+        with tempfile.TemporaryDirectory() as tempdir:
+            configfile = os.path.join(tempdir, "config.yaml")
+            app_config = AppConfig()
+            app_config.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}")
+            app_config.write_config(configfile)
+
+            command = ["python", check_config_script, configfile]
+
+            # test failure mode (flask_secret_key not set)
+            env = os.environ.copy()
+            env.pop("CXG_SECRET_KEY", None)
+            with self.assertRaises(subprocess.CalledProcessError) as exception_context:
+                subprocess.check_output(command, env=env)
+            output = str(exception_context.exception.stdout, "utf-8")
+            self.assertTrue(
+                output.startswith(
+                    "Error: Invalid type for attribute: app__flask_secret_key, expected type str, got NoneType"
+                )
+            )
+            self.assertEqual(exception_context.exception.returncode, 1)
+
+            # test passing case
+            env = os.environ.copy()
+            env["CXG_SECRET_KEY"] = "secret"
+            output = subprocess.check_output(command, env=env)
+            output = str(output, "utf-8")
+            self.assertTrue(output.startswith("PASS"))