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

Require all credentials options to be explicit #15

Open
pnadolny13 opened this issue Apr 17, 2023 · 3 comments
Open

Require all credentials options to be explicit #15

pnadolny13 opened this issue Apr 17, 2023 · 3 comments

Comments

@pnadolny13
Copy link
Contributor

After this is resolved we'll have a code path where implicit credentials are used. This could be install config/credentials files on the machine or instance roles.

The idea would be to require the tap user to always explicitly define where the credentials are coming from so theres no chance of accidentally using the wrong ones.

My thoughts from slack https://meltano.slack.com/archives/C04TSH483DF/p1681752224881609?thread_ts=1681741163.334529&cid=C04TSH483DF

This was a temporary solution to an opinion that I had around requiring the tap user to explicitly configure how they want to authenticate. I described a bit in #3 (comment). I've had weird behavior in taps that pull credentials from my environment or aws config files on my machine so I was hoping to require explicit auth for every method the tap supports. For your use case it might make sense to have a instance_auth=True tap setting that allows a session to be created without parameters or maybe a generic installed_auth=True that means it could allows aws/config.json or aws/credentials files to be used as well. I would just want the user to tell the tap explicitly "use pre-installed configurations from my machine" vs inferring that.

@danilofuchs
Copy link

Implicit credentials is useful when running Meltano inside an ECS Task, as AWS provides credentials for Boto3 automatically

@pnadolny13
Copy link
Contributor Author

  • The order is:
    • access key/secret/region
    • access key/secret/region + session token
    • profile name
    • raise error for now but would like to accept a flag to allow use_implicit_credentials where we dont verify anything and just let boto do its thing
  • Like previously mentioned if use_aws_env_vars is set then credentials are pulled from the env and not from the config keys. My opinion is that we should force one or the other and not a mix.

@danilofuchs check out my write up in #3 (comment) that describes my thoughts. I dont have an issue using implicit credentials but I'm suggesting that the user should be required to tell the tap where the credentials are coming from, in this case use_implicit_credentials would need to be provided. The source of truth for what credentials are to be used should be defined in the tap config.

This is a bit of a crazy scenario 😄 but imagine the case where youre running meltano inside an ECS task that has proper IAM roles to access the data but theres also credentials set as environment variables for something else the ECS task does and also the meltano config has credentials set. Each has different levels of access. Its hard for me to understand which one will be used by the tap.

@danilofuchs
Copy link

  • The order is:
    • access key/secret/region
    • access key/secret/region + session token
    • profile name
    • raise error for now but would like to accept a flag to allow use_implicit_credentials where we dont verify anything and just let boto do its thing
  • Like previously mentioned if use_aws_env_vars is set then credentials are pulled from the env and not from the config keys. My opinion is that we should force one or the other and not a mix.

@danilofuchs check out my write up in #3 (comment) that describes my thoughts. I dont have an issue using implicit credentials but I'm suggesting that the user should be required to tell the tap where the credentials are coming from, in this case use_implicit_credentials would need to be provided. The source of truth for what credentials are to be used should be defined in the tap config.

This is a bit of a crazy scenario 😄 but imagine the case where youre running meltano inside an ECS task that has proper IAM roles to access the data but theres also credentials set as environment variables for something else the ECS task does and also the meltano config has credentials set. Each has different levels of access. Its hard for me to understand which one will be used by the tap.

Agreed! Makes sense to have this option explicit

When implementing the tap, I was confused if it was required to set env vars/config values.

So an explicit config for the credential source would make it better documented as well!

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