From 74deabdc491cf61206f0c8695992d5cd979dabc0 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 14 Feb 2023 18:05:01 -0500 Subject: [PATCH 1/3] Some small updates to Makefile, and a new check-local-portal-creds command. --- CHANGELOG.rst | 9 +++ Makefile | 13 +++- pyproject.toml | 3 +- snovault/commands/check_local_portal_creds.py | 78 +++++++++++++++++++ 4 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 snovault/commands/check_local_portal_creds.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 38e26e3e8..66a02c311 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,15 @@ Change Log ---------- +7.2.0 +===== + +* Add command ``check-local-portal-creds``. +* In ``Makefile``: + * Add ``test-full`` (like ``test``, but not instafail) + * Enable compatibility warnings about SQLAlchemy 2.0 transition. + + 7.1.3 ===== diff --git a/Makefile b/Makefile index 884c5e5dc..ab6988258 100644 --- a/Makefile +++ b/Makefile @@ -29,15 +29,22 @@ build: test: @git log -1 --decorate | head -1 @date - pytest -xvv --instafail --timeout=200 + SQLALCHEMY_WARN_20=1 pytest -xvv --instafail --timeout=200 + @git log -1 --decorate | head -1 + @date + +test-full: + @git log -1 --decorate | head -1 + @date + SQLALCHEMY_WARN_20=1 pytest -vv --timeout=200 @git log -1 --decorate | head -1 @date remote-test-npm: - poetry run pytest -xvvv --timeout=400 --durations=100 --aws-auth --es search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 -m "indexing" + SQLALCHEMY_WARN_20=1 poetry run pytest -xvvv --timeout=400 --durations=100 --aws-auth --es search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 -m "indexing" remote-test-unit: - poetry run pytest -xvvv --timeout=400 --durations=100 --aws-auth --es search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 -m "not indexing" + SQLALCHEMY_WARN_20=1 poetry run pytest -xvvv --timeout=400 --durations=100 --aws-auth --es search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 -m "not indexing" update: poetry update diff --git a/pyproject.toml b/pyproject.toml index b4ff7d295..00c8130ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "dcicsnovault" -version = "7.1.3" +version = "7.2.0" description = "Storage support for 4DN Data Portals." authors = ["4DN-DCIC Team "] license = "MIT" @@ -144,6 +144,7 @@ PyYAML = ">=5.1,<5.5" wheel = ">=0.29.0" [tool.poetry.scripts] +check-local-portal-creds = "snovault.commands.check_local_portal_creds:main" wipe-test-indices = "snovault.commands.wipe_test_indices:main" [build-system] diff --git a/snovault/commands/check_local_portal_creds.py b/snovault/commands/check_local_portal_creds.py new file mode 100644 index 000000000..b0ed2d3c8 --- /dev/null +++ b/snovault/commands/check_local_portal_creds.py @@ -0,0 +1,78 @@ +import os +import argparse + +from dcicutils.misc_utils import PRINT +from dcicutils.command_utils import script_catch_errors + + +REPO_NAME = os.path.basename(os.path.abspath(os.curdir)) + + +def check_local_creds(appname): + + if not appname: + if 'cgap' in REPO_NAME: + appname = 'cgap' + elif 'ff' in REPO_NAME or 'ffourfront' in REPO_NAME: + appname = 'fourfront' + else: + PRINT("Can't figure out if this is cgap or fourfront.") + exit(1) + + class WarningMaker: + + count = 0 + + @classmethod + def warn(cls, *args, **kwargs): + cls.count += 1 + return PRINT(*args, **kwargs) + + warn = WarningMaker.warn + + if appname not in ['fourfront', 'ff', 'cgap']: + raise RuntimeError(f"Unknown appname {appname!r}. Expected 'cgap' or 'fourfront' (or 'ff').") + + global_env_bucket = os.environ.get('GLOBAL_ENV_BUCKET') + cgap_env_bucket = 'cgap-devtest-main-foursight-envs' + fourfront_env_bucket = 'foursight-prod-envs' + + def check_global_env_bucket(var, val): + if appname == 'cgap' and val != cgap_env_bucket: + warn(f"{var} is {val!r}, but should be {cgap_env_bucket}.") + elif appname == 'fourfront' and val != fourfront_env_bucket: + warn(f"{var} is {val!r}, but should be {fourfront_env_bucket}.") + + check_global_env_bucket('GLOBAL_ENV_BUCKET', global_env_bucket) + global_bucket_env = os.environ.get('GLOBAL_BUCKET_ENV') + if global_bucket_env: + if global_bucket_env == global_env_bucket: + warn("GLOBAL_BUCKET_ENV is the same as GLOBAL_ENV_BUCKET," + " but you can just get rid of GLOBAL_BUCKET_ENV now.") + elif not global_env_bucket: + warn("You need to set GLOBAL_ENV_BUCKET, not GLOBAL_BUCKET_ENV.") + check_global_env_bucket('GLOBAL_BUCKET_ENV', global_bucket_env) + for var in ['CHECK_RUNNER', 'ACCOUNT_NUMBER', 'ENV_NAME']: + if os.environ.get(var): + warn(f"The variable {var} has a non-null value but should be unset.") + for var in ['Auth0Client', 'Auth0Secret']: + if not os.environ.get(var): + warn(f"The variable {var} has no value but should be set.") + if WarningMaker.count == 0: + PRINT("Things look good.") + + +def main(): + parser = argparse.ArgumentParser( + description='Echos version information from ~/.cgap-keys.json or override file.') + parser.add_argument('--appname', help='Name of app to check for (cgap or ff/fourfront)', type=str, default=None) + args = parser.parse_args() + + appname = args.appname + + with script_catch_errors(): + check_local_creds(appname=appname) + + +if __name__ == '__main__': + main() From de10c891068151c85d29a826b5a422bd785151d2 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Wed, 15 Feb 2023 01:20:45 -0500 Subject: [PATCH 2/3] Fix problem cited in code review. Re-implement using more data-driven style. --- snovault/commands/check_local_portal_creds.py | 60 ++++++++++++++----- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/snovault/commands/check_local_portal_creds.py b/snovault/commands/check_local_portal_creds.py index b0ed2d3c8..39792e6a3 100644 --- a/snovault/commands/check_local_portal_creds.py +++ b/snovault/commands/check_local_portal_creds.py @@ -1,22 +1,50 @@ import os import argparse +from dcicutils.lang_utils import disjoined_list from dcicutils.misc_utils import PRINT from dcicutils.command_utils import script_catch_errors +from dcicutils.common import APP_CGAP, APP_FOURFRONT REPO_NAME = os.path.basename(os.path.abspath(os.curdir)) +PSEUDO_APP_SNOVAULT = 'snovault' + +CGAP_ENV_BUCKET = 'cgap-devtest-main-foursight-envs', +FOURFRONT_ENV_BUCKET = 'foursight-prod-envs' + +APP_NAMES = [APP_CGAP, APP_FOURFRONT] + +APP_BUCKET_MAPPINGS = { + APP_CGAP: CGAP_ENV_BUCKET, + APP_FOURFRONT: FOURFRONT_ENV_BUCKET, + PSEUDO_APP_SNOVAULT: FOURFRONT_ENV_BUCKET, # doesn't have a bucket of its own +} + +AWS_PORTAL_CREDENTIALS_NEEDED = { + # These are Needed for IAM credentials + 'AWS_ACCESS_KEY_ID': True, + 'AWS_SECRET_ACCESS_KEY': True, # Needed for IAM credentials + # These are needed only for federated temporary credentials, which we disallow in local portal deployments + # bevcause there's too much risk they are holdovers of more powerful credentials than we want, even if temporary. + 'AWS_SESSION_TOKEN': False, +} + def check_local_creds(appname): if not appname: - if 'cgap' in REPO_NAME: - appname = 'cgap' - elif 'ff' in REPO_NAME or 'ffourfront' in REPO_NAME: - appname = 'fourfront' + for appname_key, env_bucket in APP_BUCKET_MAPPINGS.items(): + if appname_key in REPO_NAME: + appname = appname_key + qualifier = "App" if appname in APP_NAMES else "Pseudo-app" + PRINT(f"No --appname given. {qualifier} {appname}, with global env bucket {env_bucket}," + f" is being assumed.") + break else: - PRINT("Can't figure out if this is cgap or fourfront.") + PRINT(f"No --appname given and can't figure out" + f" if repo {REPO_NAME} is {disjoined_list(APP_BUCKET_MAPPINGS)}.") exit(1) class WarningMaker: @@ -30,18 +58,15 @@ def warn(cls, *args, **kwargs): warn = WarningMaker.warn - if appname not in ['fourfront', 'ff', 'cgap']: - raise RuntimeError(f"Unknown appname {appname!r}. Expected 'cgap' or 'fourfront' (or 'ff').") - + if appname not in APP_BUCKET_MAPPINGS: + raise RuntimeError(f"Unknown appname {appname!r}. Expected {disjoined_list(APP_BUCKET_MAPPINGS)}.") + global_env_bucket = os.environ.get('GLOBAL_ENV_BUCKET') - cgap_env_bucket = 'cgap-devtest-main-foursight-envs' - fourfront_env_bucket = 'foursight-prod-envs' def check_global_env_bucket(var, val): - if appname == 'cgap' and val != cgap_env_bucket: - warn(f"{var} is {val!r}, but should be {cgap_env_bucket}.") - elif appname == 'fourfront' and val != fourfront_env_bucket: - warn(f"{var} is {val!r}, but should be {fourfront_env_bucket}.") + expected_val = APP_BUCKET_MAPPINGS.get(appname) + if expected_val and val != expected_val: + warn(f"{var} is {val!r}, but should be {expected_val!r}.") check_global_env_bucket('GLOBAL_ENV_BUCKET', global_env_bucket) global_bucket_env = os.environ.get('GLOBAL_BUCKET_ENV') @@ -55,6 +80,13 @@ def check_global_env_bucket(var, val): for var in ['CHECK_RUNNER', 'ACCOUNT_NUMBER', 'ENV_NAME']: if os.environ.get(var): warn(f"The variable {var} has a non-null value but should be unset.") + for aws_var, expected in AWS_PORTAL_CREDENTIALS_NEEDED.items(): + value = os.environ.get(aws_var) + if expected and not value: + warn(f"The variable {aws_var} needs a value.") + elif not expected and value: + warn(f"The variable {aws_var} has a value, but it should not.") + for var in ['Auth0Client', 'Auth0Secret']: if not os.environ.get(var): warn(f"The variable {var} has no value but should be set.") From b1c291d77d29a7db15a89c46ab6ac825661b97f4 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Wed, 15 Feb 2023 01:36:54 -0500 Subject: [PATCH 3/3] A bit more aesthetic refactoring. --- snovault/commands/check_local_portal_creds.py | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/snovault/commands/check_local_portal_creds.py b/snovault/commands/check_local_portal_creds.py index 39792e6a3..607a7568e 100644 --- a/snovault/commands/check_local_portal_creds.py +++ b/snovault/commands/check_local_portal_creds.py @@ -31,6 +31,9 @@ 'AWS_SESSION_TOKEN': False, } +UNWANTED_PORTAL_ENV_VARS = ['CHECK_RUNNER', 'ACCOUNT_NUMBER', 'ENV_NAME'] +NEEDED_PORTAL_ENV_VARS = ['Auth0Client', 'Auth0Secret'] + def check_local_creds(appname): @@ -68,6 +71,12 @@ def check_global_env_bucket(var, val): if expected_val and val != expected_val: warn(f"{var} is {val!r}, but should be {expected_val!r}.") + def needs_value(var): + warn(f"The variable {var} has no value but should be set.") + + def has_unwanted_value(var): + warn(f"The variable {var} has a non-null value but should be unset.") + check_global_env_bucket('GLOBAL_ENV_BUCKET', global_env_bucket) global_bucket_env = os.environ.get('GLOBAL_BUCKET_ENV') if global_bucket_env: @@ -77,19 +86,21 @@ def check_global_env_bucket(var, val): elif not global_env_bucket: warn("You need to set GLOBAL_ENV_BUCKET, not GLOBAL_BUCKET_ENV.") check_global_env_bucket('GLOBAL_BUCKET_ENV', global_bucket_env) - for var in ['CHECK_RUNNER', 'ACCOUNT_NUMBER', 'ENV_NAME']: - if os.environ.get(var): - warn(f"The variable {var} has a non-null value but should be unset.") + for aws_var, expected in AWS_PORTAL_CREDENTIALS_NEEDED.items(): value = os.environ.get(aws_var) if expected and not value: - warn(f"The variable {aws_var} needs a value.") + needs_value(aws_var) elif not expected and value: - warn(f"The variable {aws_var} has a value, but it should not.") + has_unwanted_value(aws_var) - for var in ['Auth0Client', 'Auth0Secret']: + for var in NEEDED_PORTAL_ENV_VARS: if not os.environ.get(var): - warn(f"The variable {var} has no value but should be set.") + needs_value(var) + for var in UNWANTED_PORTAL_ENV_VARS: + if os.environ.get(var): + has_unwanted_value(var) + if WarningMaker.count == 0: PRINT("Things look good.")