Skip to content
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

Provider env var maps being hardcoded into core #10

Open
medikoo opened this issue Oct 28, 2019 · 2 comments
Open

Provider env var maps being hardcoded into core #10

medikoo opened this issue Oct 28, 2019 · 2 comments

Comments

@medikoo
Copy link
Contributor

medikoo commented Oct 28, 2019

Currently we hard-code handling of env var credential providers into context.setCredentials().
It won't scale, and in general it'll be better to encapsulate such information within components that are specific to given provider

cli/src/Context.js

Lines 107 to 129 in 9e48ce0

// AWS
providers.aws = {}
providers.aws.AWS_ACCESS_KEY_ID = 'accessKeyId'
providers.aws.AWS_SECRET_ACCESS_KEY = 'secretAccessKey'
providers.aws.AWS_REGION = 'region'
// Google
providers.google = {}
providers.google.GOOGLE_APPLICATION_CREDENTIALS = 'applicationCredentials'
providers.google.GOOGLE_PROJECT_ID = 'projectId'
providers.google.GOOGLE_CLIENT_EMAIL = 'clientEmail'
providers.google.GOOGLE_PRIVATE_KEY = 'privateKey'
// Tencent
providers.tencent = {}
providers.tencent.TENCENT_APP_ID = 'AppId'
providers.tencent.TENCENT_SECRET_ID = 'SecretId'
providers.tencent.TENCENT_SECRET_KEY = 'SecretKey'
// Docker
providers.docker = {}
providers.docker.DOCKER_USERNAME = 'username'
providers.docker.DOCKER_PASSWORD = 'password'

@eahefnawy
Copy link
Contributor

@medikoo do you mean that we'd an aws-credentials component that just handles the aws credentials? Seems like an overkill 🤔

Why do you think this won't scale? Adding new provider credentials in the CLI is pretty easy.

@medikoo
Copy link
Contributor Author

medikoo commented Oct 29, 2019

It doesn't scale in sense, that if there will be 1000+ providers, it'll require writing in, those 1000+ cases manually here. of course that's just theoretical for now.

Anyway any provider-specific logic in context of such generic module (which by definition is expected to be provider agnostic) just doesn't feel right. I think this design require some rethinking.

I haven't thought that much about specific alternative (I'm still learning about components), still my first thought is that those credentials should be naturally assumed from env vars in provider components (it's how other libraries do that I think).
And if resolution is not as straightforward, and doing that will require repeating same tedious steps in each provider component, then credentials resolution logic can be provided neatly via some external e.g. {provider}-credentials package, which should be promoted to be used in such components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants