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.

Sign up for GitHub

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

Jump to bottom

Assume role from profile via instance metadata #383

Merged
merged 18 commits into from
Jul 16, 2021
Merged

Assume role from profile via instance metadata #383

merged 18 commits into from
Jul 16, 2021

Conversation

christopher-dG
Copy link
Member

Closes #287

[default]
region = us-east-2

[profile role-to-assume]
region = us-east-2
role_arn = arn:aws:iam::account:role/role-to-assume
credential_source = Ec2InstanceMetadata
julia> using AWS

julia> global_aws_config(; profile="default")
AWSConfig(arn:aws:iam::account:instance-profile/role-from-instance-profile (ASIASWQI5NDNRIZLJ3WJ, pAk..., IQo..., 2021-06-18T21:11:05), "us-east-2", "json")

julia> global_aws_config(; profile="role-to-assume")
AWSConfig(arn:aws:sts::account:assumed-role/role-to-assume/AWS.jl-role-role-to-assume-20210618T152950Z (ASIASWQI5NDNRFEK2HJS, AYR..., IQo..., 2021-06-18T15:44:53), "us-east-2", "json")

Needs some tests of course.

omus
omus reviewed Jun 18, 2021
View reviewed changes
src/AWSCredentials.jl Outdated Show resolved Hide resolved
@omus
Copy link
Member

omus commented Jun 18, 2021

bors try

bors bot added a commit that referenced this pull request Jun 18, 2021
@bors
Try #383:
7b8aa30
@omus
Copy link
Member

omus commented Jun 18, 2021

I'll start the CI before the tests are added just to verify the changes doesn't break anything

@bors
Copy link
Contributor

bors bot commented Jun 18, 2021

try

Build failed:

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 18, 2021
@bors
Try #383:
8d4df33
@bors
Copy link
Contributor

bors bot commented Jun 18, 2021

try

Build succeeded:

ericphanson
ericphanson reviewed Jun 18, 2021
View reviewed changes
src/AWSCredentials.jl Outdated Show resolved Hide resolved
mattBrzezinski
mattBrzezinski approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 18, 2021

Canceled.

@mattBrzezinski
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jun 18, 2021
@bors @christopher-dG @mattBrzezinski
Merge #383
f33bb32 
383: Assume role from profile via instance metadata r=mattBrzezinski a=christopher-dG

Closes #287 

```ini
[default]
region = us-east-2

[profile role-to-assume]
region = us-east-2
role_arn = arn:aws:iam::account:role/role-to-assume
credential_source = Ec2InstanceMetadata
```

```julia
julia> using AWS

julia> global_aws_config(; profile="default")
AWSConfig(arn:aws:iam::account:instance-profile/role-from-instance-profile (ASIASWQI5NDNRIZLJ3WJ, pAk..., IQo..., 2021-06-18T21:11:05), "us-east-2", "json")

julia> global_aws_config(; profile="role-to-assume")
AWSConfig(arn:aws:sts::account:assumed-role/role-to-assume/AWS.jl-role-role-to-assume-20210618T152950Z (ASIASWQI5NDNRFEK2HJS, AYR..., IQo..., 2021-06-18T15:44:53), "us-east-2", "json")
```

Needs some tests of course.

Co-authored-by: Chris de Graaf <me@cdg.dev>
Co-authored-by: mattBrzezinski <matt.brzezinski@invenia.ca>
@bors
Copy link
Contributor

bors bot commented Jun 18, 2021

Build failed:

@christopher-dG
Copy link
Member Author

bors try

(I dunno if I'm allowed to do this, if not can someone run tests?)

@bors
Copy link
Contributor

bors bot commented Jun 21, 2021

🔒 Permission denied

Existing reviewers: click here to make christopher-dG a reviewer

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 21, 2021
@bors
Try #383:
ef9caa2
@bors
Copy link
Contributor

bors bot commented Jun 21, 2021

try

Build failed:

@christopher-dG
Copy link
Member Author

One more? I think I caught everything now.

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 21, 2021
@bors
Try #383:
27b3399
@bors
Copy link
Contributor

bors bot commented Jun 21, 2021

try

Build failed:

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 21, 2021
@bors
Try #383:
8372c06
@bors
Copy link
Contributor

bors bot commented Jun 21, 2021

try

Build failed:

@christopher-dG
Copy link
Member Author

Now I'm a bit confused, the error seems to indicate that the _http_request_patch is not active. Does a nested apply cancel out any outer applys?

@mattBrzezinski
Copy link
Member

Now I'm a bit confused, the error seems to indicate that the _http_request_patch is not active. Does a nested apply cancel out any outer applys?

I'm not too familiar with Mocking.jl can we not just do:

apply([patch_1, patch_2]) do

@mattBrzezinski
Copy link
Member

bors try

@christopher-dG
Copy link
Member Author

There's also a line 507 somewhere in the output which is the call to ec2_instance_credentials

@christopher-dG
Copy link
Member Author

Here's a full output (it's 100K lines long): out.txt

@christopher-dG
Copy link
Member Author

Ok so I walked through this with a bunch of prints, and the problem is the STS.assume_role which eventually calls check_credentials from sign_aws4!. Since the expiry is mocked to always return a moment in the past, every call to check_credentials is going to return expired credentials.

The immediate fix would be to mock out the STS.assume_role but I'm curious as to why the AssumeRoleWithWebIdentity tests can get away with it but this can't.

@christopher-dG
Copy link
Member Author

@mattBrzezinski could I get a test run?

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jul 16, 2021
@bors
Try #383:
10c8339
@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

try

Build failed:

@christopher-dG
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 16, 2021
@bors
Try #383:
5e6def5
@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

try

Build succeeded:

@christopher-dG
Copy link
Member Author

success finally! 😄

@christopher-dG
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 16, 2021
@bors
Try #383:
f438b47
@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

try

Build succeeded:

@christopher-dG
Copy link
Member Author

@mattBrzezinski one last review? :)

mattBrzezinski
mattBrzezinski approved these changes Jul 16, 2021
View reviewed changes
Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

LGTM! :)

@mattBrzezinski
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 16, 2021
@bors @christopher-dG @mattBrzezinski
Merge #383
93ee9db 
383: Assume role from profile via instance metadata r=mattBrzezinski a=christopher-dG

Closes #287 

```ini
[default]
region = us-east-2

[profile role-to-assume]
region = us-east-2
role_arn = arn:aws:iam::account:role/role-to-assume
credential_source = Ec2InstanceMetadata
```

```julia
julia> using AWS

julia> global_aws_config(; profile="default")
AWSConfig(arn:aws:iam::account:instance-profile/role-from-instance-profile (ASIASWQI5NDNRIZLJ3WJ, pAk..., IQo..., 2021-06-18T21:11:05), "us-east-2", "json")

julia> global_aws_config(; profile="role-to-assume")
AWSConfig(arn:aws:sts::account:assumed-role/role-to-assume/AWS.jl-role-role-to-assume-20210618T152950Z (ASIASWQI5NDNRFEK2HJS, AYR..., IQo..., 2021-06-18T15:44:53), "us-east-2", "json")
```

Needs some tests of course.

Co-authored-by: Chris de Graaf <me@cdg.dev>
Co-authored-by: mattBrzezinski <matt.brzezinski@invenia.ca>
@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

Build failed:

@christopher-dG
Copy link
Member Author

error doesn't look like it's anything I had to do with

@mattBrzezinski
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

Build succeeded:

@bors bors bot merged commit d2a2359 into JuliaCloud:master Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericphanson ericphanson ericphanson left review comments

@omus omus omus left review comments

@mattBrzezinski mattBrzezinski mattBrzezinski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FR: support credential_source = Ec2InstanceMetadata in ~/.aws/config
4 participants
@christopher-dG @omus @mattBrzezinski @ericphanson