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

Added support of assume roles from profiles and the load of parameters from parent profile #1092

Closed
wants to merge 1 commit into from

Conversation

ssuprun
Copy link

@ssuprun ssuprun commented Feb 11, 2016

List of changes:

  • Added a basic changes to merge all parameters from source profile to called one
  • Added a support of assume role credentials, loaded from credential file

We can create the aws cli syntax compatible tool by use these changes, for example:

require 'aws-sdk'

ARGV.slice_before(/^--/).each do |name, value|
  case name
  when '--profile'
    ENV['AWS_PROFILE'] = value
    ENV['AWS_ACCESS_KEY'] = nil
  end
end

ec2 = Aws::EC2::Client.new()
# print the ID of security group "default"
p ec2.describe_security_groups(group_names: ['default']).security_groups[0].group_id

UPD: This change the mostly the same to #998. But I found it a too late, when my changes were ready to commit ;)

@ssuprun
Copy link
Author

ssuprun commented Feb 11, 2016

About the failed test... I'm not sure how the following error is related to my changes...

Failures:
  1) Aws::S3::Object multipart_copy: true #copy_from aborts the upload on errors
     Failure/Error:
       expect {
         object.copy_from('source-bucket/source/key', multipart_copy: true)
       }.to raise_error(Aws::S3::Errors::NoSuchKey)
       expected Aws::S3::Errors::NoSuchKey, got Aws::S3::Errors::BadRequest with backtrace:
         # ./aws-sdk-core/lib/seahorse/client/plugins/raise_response_errors.rb:15:in `call'
         # ./aws-sdk-core/lib/aws-sdk-core/plugins/s3_sse_cpk.rb:18:in `call'
         # ./aws-sdk-core/lib/aws-sdk-core/plugins/param_converter.rb:20:in `call'
         # ./aws-sdk-core/lib/seahorse/client/plugins/response_target.rb:21:in `call'
         # ./aws-sdk-core/lib/seahorse/client/request.rb:70:in `send_request'
         # ./aws-sdk-core/lib/seahorse/client/base.rb:207:in `upload_part_copy'
         # ./aws-sdk-resources/lib/aws-sdk-resources/services/s3/object_multipart_copier.rb:74:in `copy_part'
         # ./aws-sdk-resources/lib/aws-sdk-resources/services/s3/object_multipart_copier.rb:63:in `copy_part_thread'
     # ./aws-sdk-resources/spec/services/s3/object/multipart_copy_spec.rb:219:in `S3'
Finished in 10.77 seconds (files took 4.57 seconds to load)
383 examples, 1 failure

@awood45
Copy link
Member

awood45 commented Feb 11, 2016

On the test failure, I agree that I don't think it's your change.

@awood45
Copy link
Member

awood45 commented Feb 11, 2016

So, I want to make a master issue for this soon, but we're currently figuring out the correct approach for supporting these features across SDKs in a consistent way. There's probably going to be a delay in pulling this in while we are figuring it out, but we're not ignoring this.

@ssuprun
Copy link
Author

ssuprun commented Feb 19, 2016

Guys, do you have any updates?

@roman-parkhunovskyi
Copy link

Any news? This feature is a must.

profile['aws_access_key_id'],
profile['aws_secret_access_key'],
profile['aws_session_token']
)
@credentials = if role_arn = profile['role_arn']
AssumeRoleCredentials.new(
role_session_name: [*('A'..'Z')].sample(16).join,
Copy link

Choose a reason for hiding this comment

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

random name seems a bit strange ... how about "assumed-#{profile_name}-#{ENV['USER']}"

Copy link
Author

Choose a reason for hiding this comment

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

Actually you propose the cosmetic change. :) If this one is only blocker I'll do it ;)

@grosser
Copy link

grosser commented Mar 31, 2016

@trevorrowe could you comment on mergability / what is missing ?

@awood45
Copy link
Member

awood45 commented Mar 31, 2016

We're closing in on how we want to approach this. One change will be that some of these changes may be gated by an environment variable - anything that would risk possible backwards incompatibility.

Will be keeping this open in the meantime, the intention is to merge this or to otherwise implement this.

@grosser
Copy link

grosser commented Mar 31, 2016

I'd suggest a credentials setting itself new_fancyness=true so everything
stays inside that file

On Thu, Mar 31, 2016 at 9:55 AM, Alex Wood [email protected] wrote:

We're closing in on how we want to approach this. One change will be that
some of these changes may be gated by an environment variable - anything
that would risk possible backwards incompatibility.

Will be keeping this open in the meantime, the intention is to merge this
or to otherwise implement this.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1092 (comment)

@awood45
Copy link
Member

awood45 commented Mar 31, 2016

That's actually an interesting approach I hadn't considered. I'll definitely add that to the discussion we're having about broadening this support - I agree that also solves the problem. Would likely be implemented as ENV['AWS_CLI_CONFIG'] || use_in_sdk or similar.

@awood45
Copy link
Member

awood45 commented Apr 8, 2016

Thank you for this. We're moving forward, but I'm going to use #1132 as the main PR for this since you're doing essentially the same thing, and that PR is slightly further along. Let's move the conversation there.

@awood45 awood45 closed this Apr 8, 2016
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.

4 participants