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

Add support for assumed_role and STS #414

Conversation

ShimiTaNaka
Copy link

Add support to get assumed_role credentials and add option to set default STS TTL and max STS TTL

@ghost ghost added the size/XL label May 12, 2019
@ShimiTaNaka
Copy link
Author

I have ran tests and acceptance tests against Vault 1.1.0 without AWS keys, so some of the tests didn't pass.
Would be happy to know if there are scenarios in which the tests fail.

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Jun 3, 2019
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, @ShimiTaNaka , thanks for adding all this, and with such great test coverage.

It's conflicting with master now because of how I merged #407 which adds support for role_arns. Would you be willing to merge in master and resolve the conflicts? After that I'll be able to run the tests with an AWS access key and secret, and if they don't pass I'll post the output; but if they do then this should be good to go.

@ShimiTaNaka
Copy link
Author

@tyrannosaurus-becks Looking Again on my changes, since I took another PR that wasn't maintained, I didn't notice I'm also adding the role_arns
Since there are few relatively important differences, I suggest having another pair of eyes
(For example, now in master, policy is marked as conflicting with role_arns, however, I do not see any evidence for it in the documentation, contrary, it seems that you can add both and have the policy as extra filter in addition to the roles
This obviously also affects the rest of the code.
@oxlay Would love your input as well since the conflicting PR is yours if possible :)
Thanks

@ShimiTaNaka
Copy link
Author

@tyrannosaurus-becks any updates?

@tyrannosaurus-becks
Copy link
Contributor

Happy to take a look at this once master is merged in!

@tyrannosaurus-becks
Copy link
Contributor

Closing this for now due to inactivity. If you end up wanting to reopen this, feel free to do so when you're ready. Thank you for your work!

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

Successfully merging this pull request may close these issues.

None yet

2 participants