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

Allow role assumption #153

Merged
merged 4 commits into from
Feb 1, 2022
Merged

Conversation

mdreem
Copy link
Contributor

@mdreem mdreem commented Dec 10, 2021

Enable AWS role assumption mechanism after logging in with the given credentials. See #151.

@mdreem
Copy link
Contributor Author

mdreem commented Dec 10, 2021

I needed to use the AWS role assumption mechanism after logging in with the given credentials so I have implemented it.
This has been attempted in #151 already, but apparently has been abandoned.

…en credentials

Signed-off-by: Marc Schiereck <mdreem@fastmail.fm>
Signed-off-by: Marc Schiereck <mdreem@fastmail.fm>
Signed-off-by: Marc Schiereck <mdreem@fastmail.fm>
Signed-off-by: Marc Schiereck <mdreem@fastmail.fm>
Copy link
Contributor

@clarafu clarafu left a comment

Choose a reason for hiding this comment

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

Sorry for the late review! It looks great, I was wondering if there would be a way to have it use the role ARN in a few specific integrations tests and without it in others? Just so that we can have the integration tests run tests on both using role ARN and without?

integration/integration_suite_test.go Show resolved Hide resolved
@mdreem
Copy link
Contributor Author

mdreem commented Jan 11, 2022

Sorry for the late review! It looks great, I was wondering if there would be a way to have it use the role ARN in a few specific integrations tests and without it in others? Just so that we can have the integration tests run tests on both using role ARN and without?

Thanks for checking it out!
When I tested it locally I actually created two buckets, in order to separate :
One I can only access via the role assumption after using credentials, which only allowed the role-assumption.
Another one which I could only access via credentials.

Then I was curious if it still works in all use-cases, so I just switched the bucket, when I ran the test-suite. But if you think there are some specific tests that are sufficient, I think it would also be possible. I'd just need to define an additional parameter for the bucket using the role-assumption, I guess.

Copy link
Contributor

@clarafu clarafu left a comment

Choose a reason for hiding this comment

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

I think everything here is great already, I'm just going to merge it because its been delayed for so long haha. I was just suggesting the test changes so that we can run the tests once to cover all the cases of using role ARN and without but it's honestly not a big deal! Thanks for the PR!!

@clarafu clarafu merged commit 80c54bd into concourse:master Feb 1, 2022
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

Successfully merging this pull request may close these issues.

2 participants