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

Use boto3 to manage credentials #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smartynoah
Copy link

The code previously used boto for delegated role credentials. This patch advances that development line to use boto3 for all credential managment.

An important effect of this change is that the plugin now works in ECS docker contexts.

The code previously used boto for delegated role credentials.  This patch advances that development line to use boto3 for all credential managment.

An important effect of this change is that the plugin now works in ECS docker contexts.
@andrewegel
Copy link

andrewegel commented Jun 23, 2021

I'll +1 what was mentioned here: #82 (comment)

This change is nice in that it simplifies the code, but my concerns are around pulling in a boto dependency - Limiting to distributions is one thing, but something else to bring up as a Linux Sys-Admin, is users often times pip install awscli, which may (or may not) "clobber" the system's boto, possibly changing up your boto "interface" you've coded against, thus rendering the system package manager in a slightly in-operable state.

If you compare that burden against removing ~69 lines of code thats a tough sell to me IMO.

Alternatively if this route is chosen by the maintainer or others in the community, then it would also be good to remove a bulk of the sigv2 and sigv4 methods, and rely on the underlying botocore library to do that work for you.

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

Successfully merging this pull request may close these issues.

3 participants