Use aws sdk v2 #85

okkez · 2015-07-10T02:50:51Z

I'm trying to use aws-sdk v2.

TODOs

Fix unit test
Test in real world

See also #69 .

repeatedly · 2015-07-26T22:44:27Z

test/test_out_s3.rb


    config = CONFIG_TIME_SLICE + 'auto_create_bucket true'
    d = create_time_sliced_driver(config)
    assert_nothing_raised { d.run }
  end
-
-  def test_aws_credential_provider_default


Removing these tests are intented?

Yes.

See okkez@05bf971

I see. Thanks!

See https://github.com/fluent/fluent-plugin-s3/pull/85/files#r37147150

repeatedly · 2015-07-27T00:09:39Z

@ijin Could you also check this PR?

repeatedly · 2015-08-11T08:38:02Z

Sorry, could you rebase the commits?
After that, I will announce this PR for finding testers.

AWS -> Aws

Because aws-adk does not have such option.

This is for backward compatibility. If users use fluent-plugin-s3 0.59 or older with default configuration, aws-sdk v1 set "us-east-1" to S3 region automatically. But aws-sdk v2 does not set default value to S3 region, and raise error. See also: https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region

Because `Aws::Plugins::S3Md5s::Handler#md5` reads `Tempfile` object.

Because aws-sdk-core calculate md5 digest from tempfile. This change suppresses following warning: [warn]: fluent/output.rb:354:rescue in try_flush: temporarily failed to flush the buffer. next_retry=2015-07-23 11:25:46 +0900 error_class="Aws::S3::Errors::BadDigest" error="The Content-MD5 you specified did not match what we received." plugin_id="object:1e1e370"

This change follows c6e5665.

The following locations will be searched in order for credentials: * :access_key_id, :secret_access_key, and :session_token options * ENV['AWS_ACCESS_KEY_ID'], ENV['AWS_SECRET_ACCESS_KEY'] * HOME/.aws/credentials shared credentials file * EC2 instance profile credentials See Plugins::RequestSigner for more details. See also: http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Client.html#initialize-instance_method

aws-sdk v2 does not have API to set credential providers. It is calculated by aws-sdk automatically.

Because flexmock have not been maintained for several years.

okkez · 2015-08-11T14:18:38Z

Rebased!

sonots · 2015-08-12T00:34:55Z

lib/fluent/plugin/out_s3.rb

    config_param :use_server_side_encryption, :string, :default => nil
    config_param :aws_key_id, :string, :default => nil, :secret => true
    config_param :aws_sec_key, :string, :default => nil, :secret => true
    config_param :aws_iam_retries, :integer, :default => 5
    config_param :s3_bucket, :string
-    config_param :s3_region, :string, :default => nil
+    config_param :s3_region, :string, :default => "us-east-1"


Should keep with nil. Otherwise, ENV['AWS_REGION'] won't be effective.

When keep nil, it breaks backward compatibility.
See 4db98de.

Which is preferred?

Okay, then, how about :default => ENV['AWS_REGION'] || 'us-east-1'

Implemented e59099f.

ijin · 2015-08-16T16:03:56Z

This might break backwards compatibility for some users. A few are attributable to v2 simply not supporting them, but some are from code changes.

I do like the idea of replacing flex-mock with rr, though.

proxy_uri -> http_proxy see http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Client.html#initialize-instance_method

See http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Client.html#initialize-instance_method

* Aws::AssumeRoleCredentials * Aws::InstanceProfileCrecentials * Aws::SharedCredentials I cannot set some options via `config_param`, so comment out them, for now.

repeatedly · 2015-09-01T03:43:40Z

lib/fluent/plugin/out_s3.rb

+        credentials_options[:duration_seconds] = c.duration_seconds if c.duration_seconds
+        credentials_options[:external_id] = c.external_id if c.external_id
+        options[:credentials] = Aws::AssumeRoleCredentials.new(credentials_options)
+      when @instance_profile_credentials


How about using same credentials if user specifies "aws_iam_retries" for backward compatibility?

when @instance_profile_credentials #... when @aws_iam_retries $log.warn "'aws_iam_retries' parameter is deprecated. Use 'instance_profile_credentials' instead" credentials_options[:retries] = @aws_iam_retries options[:credentials] = Aws::InstanceProfileCredentials.new(credentials_options)

Good idea!
I've changed.

This reverts commit 7f783ac. For backward compatibility.

But `aws_iam_retries` is deprecated configuration. This change is only for backward compatibility.

repeatedly · 2015-09-01T05:46:01Z

Thanks for applying review!

@ijin How about recent changes? I want to merge this PR and release v0.6.0-pre1 for testing.

Use aws sdk v2

ijin · 2015-09-11T05:36:35Z

LGTM.

@okkez Does the required minimum IAM policy change at all?

okkez · 2015-09-11T08:56:45Z

@ijin I'm not sure about IAM policy.
I have not considered required minimum IAM policy.

okkez changed the title ~~WIP: Use aws sdk v2~~ Use aws sdk v2 Jul 23, 2015

repeatedly reviewed Jul 26, 2015
View reviewed changes

repeatedly mentioned this pull request Aug 7, 2015

aws-sdk version 2 support? #93

Closed

okkez and others added 15 commits August 11, 2015 21:57

Use aws-sdk v2

3f09b2d

Use new namespace

7c4bb12

AWS -> Aws

Remove :use_ssl option

3a382d2

Because aws-adk does not have such option.

Use aws-sdk v2 API

164472d

Do not close Tempfile object in Compressor#compress

1c33123

Because `Aws::Plugins::S3Md5s::Handler#md5` reads `Tempfile` object.

Remove needless assertion

884c8e6

This change follows c6e5665.

Remove tests related to credential providers

9198296

aws-sdk v2 does not have API to set credential providers. It is calculated by aws-sdk automatically.

Use rr instead of flexmock

0a3396b

Because flexmock have not been maintained for several years.

Re-implement setup_mocks using rr

cb0e90b

Use new setup_mocks

c7f3269

Rewrite test_write_with_custom_s3_object_key_format using rr

e9f94e8

Use rr instead of flexmock

b951d6e

okkez force-pushed the use-aws-sdk-v2 branch from 6c0ff66 to b951d6e Compare August 11, 2015 14:15

sonots reviewed Aug 12, 2015
View reviewed changes

okkez and others added 3 commits August 20, 2015 16:31

Change option name

7f4652d

proxy_uri -> http_proxy see http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Client.html#initialize-instance_method

Remove s3_ prefix from option name

100b16d

See http://docs.aws.amazon.com/sdkforruby/api/Aws/S3/Client.html#initialize-instance_method

Add credentials support

f692ff7

* Aws::AssumeRoleCredentials * Aws::InstanceProfileCrecentials * Aws::SharedCredentials I cannot set some options via `config_param`, so comment out them, for now.

okkez and others added 4 commits August 21, 2015 10:34

Remove unused option :aws_iam_retries

7f783ac

Add test for credentials

6f6f930

Use environment variable AWS_REGION as default value for s3_region

e59099f

Try to read object to check permissions

2f1102f

repeatedly reviewed Sep 1, 2015
View reviewed changes

okkez added 2 commits September 1, 2015 13:27

Revert "Remove unused option :aws_iam_retries"

40d3b5b

This reverts commit 7f783ac. For backward compatibility.

Use Aws::InstanceProfileCredentials when specify aws_iam_retries

bd74e3b

But `aws_iam_retries` is deprecated configuration. This change is only for backward compatibility.

repeatedly added a commit that referenced this pull request Sep 10, 2015

Merge pull request #85 from okkez/use-aws-sdk-v2

cc3a175

Use aws sdk v2

repeatedly merged commit cc3a175 into fluent:master Sep 10, 2015

okkez mentioned this pull request Oct 1, 2015

Added STS AssumeRole support #81

Closed

okkez deleted the use-aws-sdk-v2 branch January 25, 2016 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use aws sdk v2 #85

Use aws sdk v2 #85

okkez commented Jul 10, 2015

repeatedly Jul 26, 2015

okkez Jul 27, 2015

repeatedly Jul 27, 2015

ijin Aug 16, 2015

repeatedly commented Jul 27, 2015

repeatedly commented Aug 11, 2015

okkez commented Aug 11, 2015

sonots Aug 12, 2015

okkez Aug 20, 2015

sonots Aug 21, 2015

okkez Aug 25, 2015

ijin commented Aug 16, 2015

repeatedly Sep 1, 2015

okkez Sep 1, 2015

repeatedly commented Sep 1, 2015

ijin commented Sep 11, 2015

okkez commented Sep 11, 2015

Use aws sdk v2 #85

Use aws sdk v2 #85

Conversation

okkez commented Jul 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

repeatedly commented Jul 27, 2015

repeatedly commented Aug 11, 2015

okkez commented Aug 11, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijin commented Aug 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

repeatedly commented Sep 1, 2015

ijin commented Sep 11, 2015

okkez commented Sep 11, 2015