-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check-local-portal-creds #244
Open
netsettler
wants to merge
4
commits into
master
Choose a base branch
from
kmp_misc_20230214
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+132
−1
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
74deabd
Some small updates to Makefile, and a new check-local-portal-creds co…
netsettler de10c89
Fix problem cited in code review. Re-implement using more data-driven…
netsettler b1c291d
A bit more aesthetic refactoring.
netsettler 98bf458
Merge branch 'master' into kmp_misc_20230214
netsettler File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
[tool.poetry] | ||
name = "dcicsnovault" | ||
version = "11.3.0" | ||
version = "11.4.0" | ||
description = "Storage support for 4DN Data Portals." | ||
authors = ["4DN-DCIC Team <[email protected]>"] | ||
license = "MIT" | ||
|
@@ -129,6 +129,7 @@ PyYAML = "^6.0.1" | |
wheel = ">=0.40.0" | ||
|
||
[tool.poetry.scripts] | ||
check-local-portal-creds = "snovault.commands.check_local_portal_creds:main" | ||
dev-servers-snovault = "snovault.dev_servers:main" | ||
list-db-tables = "snovault.commands.list_db_tables:main" | ||
prepare-local-dev = "snovault.commands.prepare_template:prepare_local_dev_main" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
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 | ||
} | ||
Comment on lines
+17
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer a structure that infers this from the calling repository, maybe by reading a file? |
||
|
||
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, | ||
} | ||
|
||
UNWANTED_PORTAL_ENV_VARS = ['CHECK_RUNNER', 'ACCOUNT_NUMBER', 'ENV_NAME'] | ||
NEEDED_PORTAL_ENV_VARS = ['Auth0Client', 'Auth0Secret'] | ||
|
||
|
||
def check_local_creds(appname): | ||
|
||
if not appname: | ||
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(f"No --appname given and can't figure out" | ||
f" if repo {REPO_NAME} is {disjoined_list(APP_BUCKET_MAPPINGS)}.") | ||
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 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') | ||
|
||
def check_global_env_bucket(var, val): | ||
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}.") | ||
|
||
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: | ||
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 aws_var, expected in AWS_PORTAL_CREDENTIALS_NEEDED.items(): | ||
value = os.environ.get(aws_var) | ||
if expected and not value: | ||
needs_value(aws_var) | ||
elif not expected and value: | ||
has_unwanted_value(aws_var) | ||
|
||
for var in NEEDED_PORTAL_ENV_VARS: | ||
if not os.environ.get(var): | ||
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.") | ||
|
||
|
||
def main(): | ||
parser = argparse.ArgumentParser( | ||
description='Echos version information from ~/.cgap-keys.json or override file.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This description seems lifted from somewhere else |
||
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() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I think this makes sense in utils rather than snovault is because these details already sort of exist there vs. here they are out of place IMO as I don't like creating a dependency on environments within snovault, feels circular.