From a537b7d35cc0fc28e777f2001c34a7f29a636fd1 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Tue, 19 Apr 2022 15:29:41 +0900 Subject: [PATCH] Revert wrong fix in #388 `Aws::AssumeRoleCredentials.new` and `Aws::AssumeRoleWebIdentityCredentials.new` expect a hash argument, not kwargs. Tests for them were wrong, not the implementation. https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/AssumeRoleCredentials.html#initialize-instance_method https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/AssumeRoleWebIdentityCredentials.html#initialize-instance_metho Signed-off-by: Takuro Ashie --- lib/fluent/plugin/out_s3.rb | 4 +-- test/test_out_s3.rb | 68 ++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/lib/fluent/plugin/out_s3.rb b/lib/fluent/plugin/out_s3.rb index 25450de..3ca7851 100644 --- a/lib/fluent/plugin/out_s3.rb +++ b/lib/fluent/plugin/out_s3.rb @@ -516,7 +516,7 @@ def setup_credentials end end - options[:credentials] = Aws::AssumeRoleCredentials.new(**credentials_options) + options[:credentials] = Aws::AssumeRoleCredentials.new(credentials_options) when @aws_key_id && @aws_sec_key options[:access_key_id] = @aws_key_id options[:secret_access_key] = @aws_sec_key @@ -532,7 +532,7 @@ def setup_credentials elsif @s3_region credentials_options[:client] = Aws::STS::Client.new(:region => @s3_region) end - options[:credentials] = Aws::AssumeRoleWebIdentityCredentials.new(**credentials_options) + options[:credentials] = Aws::AssumeRoleWebIdentityCredentials.new(credentials_options) when @instance_profile_credentials c = @instance_profile_credentials credentials_options[:retries] = c.retries if c.retries diff --git a/test/test_out_s3.rb b/test/test_out_s3.rb index b00bd3c..cbf7860 100644 --- a/test/test_out_s3.rb +++ b/test/test_out_s3.rb @@ -562,9 +562,9 @@ def test_credentials def test_assume_role_credentials expected_credentials = Aws::Credentials.new("test_key", "test_secret") - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: anything){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: anything }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -585,9 +585,9 @@ def test_assume_role_credentials_with_region expected_credentials = Aws::Credentials.new("test_key", "test_secret") sts_client = Aws::STS::Client.new(region: 'ap-northeast-1') mock(Aws::STS::Client).new(region: 'ap-northeast-1'){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -610,9 +610,9 @@ def test_assume_role_with_iam_credentials sts_client = Aws::STS::Client.new(region: 'ap-northeast-1', credentials: expected_credentials) mock(Aws::Credentials).new("test_key_id", "test_sec_key") { expected_credentials } mock(Aws::STS::Client).new(region: 'ap-northeast-1', credentials: expected_credentials){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client } ){ expected_credentials } config = CONFIG_TIME_SLICE @@ -637,10 +637,10 @@ def test_assume_role_credentials_with_region_and_sts_http_proxy expected_sts_http_proxy = 'http://example.com' sts_client = Aws::STS::Client.new(region: expected_region, http_proxy: expected_sts_http_proxy) mock(Aws::STS::Client).new(region:expected_region, http_proxy: expected_sts_http_proxy){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client, - sts_http_proxy: expected_sts_http_proxy){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client, + sts_http_proxy: expected_sts_http_proxy }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -664,10 +664,10 @@ def test_assume_role_credentials_with_sts_http_proxy expected_sts_http_proxy = 'http://example.com' sts_client = Aws::STS::Client.new(region: "us-east-1", http_proxy: expected_sts_http_proxy) mock(Aws::STS::Client).new(region: "us-east-1", http_proxy: expected_sts_http_proxy){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client, - sts_http_proxy: expected_sts_http_proxy){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client, + sts_http_proxy: expected_sts_http_proxy }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -690,10 +690,10 @@ def test_assume_role_credentials_with_sts_endpoint_url expected_sts_endpoint_url = 'http://example.com' sts_client = Aws::STS::Client.new(region: "us-east-1", endpoint: expected_sts_endpoint_url) mock(Aws::STS::Client).new(region: "us-east-1", endpoint: expected_sts_endpoint_url){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client, - sts_endpoint_url: expected_sts_endpoint_url){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client, + sts_endpoint_url: expected_sts_endpoint_url }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -716,9 +716,9 @@ def test_assume_role_credentials_with_sts_region expected_sts_region = 'ap-south-1' sts_client = Aws::STS::Client.new(region: expected_sts_region) mock(Aws::STS::Client).new(region: expected_sts_region){ sts_client } - mock(Aws::AssumeRoleCredentials).new(role_arn: "test_arn", - role_session_name: "test_session", - client: sts_client){ + mock(Aws::AssumeRoleCredentials).new({ role_arn: "test_arn", + role_session_name: "test_session", + client: sts_client }){ expected_credentials } config = CONFIG_TIME_SLICE.split("\n").reject{|x| x =~ /.+aws_.+/}.join("\n") @@ -739,10 +739,12 @@ def test_assume_role_credentials_with_sts_region def test_web_identity_credentials expected_credentials = Aws::Credentials.new("test_key", "test_secret") mock(Aws::AssumeRoleWebIdentityCredentials).new( - role_arn: "test_arn", - role_session_name: "test_session", - web_identity_token_file: "test_file", - client: anything + { + role_arn: "test_arn", + role_session_name: "test_session", + web_identity_token_file: "test_file", + client: anything + } ){ expected_credentials } @@ -767,10 +769,12 @@ def test_web_identity_credentials_with_sts_region sts_client = Aws::STS::Client.new(region: 'us-east-1') mock(Aws::STS::Client).new(region: 'us-east-1'){ sts_client } mock(Aws::AssumeRoleWebIdentityCredentials).new( - role_arn: "test_arn", - role_session_name: "test_session", - web_identity_token_file: "test_file", - client: sts_client + { + role_arn: "test_arn", + role_session_name: "test_session", + web_identity_token_file: "test_file", + client: sts_client + } ){ expected_credentials }