aws / aws-sdk-php

Official repository of the AWS SDK for PHP (@awsforphp)
http://aws.amazon.com/sdkforphp
Apache License 2.0
6.02k stars 1.22k forks source link

Class EcsCredentialProvider Doesn't Support Customized Guzzle Handler #2162

Closed deminy closed 11 months ago

deminy commented 3 years ago

Describe the bug

Class \Aws\Credentials\EcsCredentialProvider doesn't support customized Guzzle handler.

Version of AWS SDK for PHP?

v3.166.2

Version of PHP (php -v)?

PHP 7.4.12

To Reproduce (observed behavior)

We used to call some AWS APIs with credentials hardcoded. e.g.,

use Aws\Ses\SesClient;
use GuzzleHttp\HandlerStack;

new SesClient(
    [
        'region'       => 'us-east-1',
        'version'      => 'latest',
        'credentials'  => ['key' => '', 'secret' => ''],
        'http_handler' => HandlerStack::create(new CustomizedGuzzleHandler()),
    ]
);

Please note that here we use a customized Guzzle handler.

Recently we switched to use IAM Roles for Tasks, and started calling the AWS APIs like following:

 use Aws\Ses\SesClient;
 use GuzzleHttp\HandlerStack;

 new SesClient(
     [
         'region'       => 'us-east-1',
         'version'      => 'latest',
         'http_handler' => HandlerStack::create(new CustomizedGuzzleHandler()),
     ]
 );

The problem is that, the API call starts using the default handler, but not the customized one we specified.

Expected behavior

Here is the constructor in class \Aws\Credentials\EcsCredentialProvider:

class EcsCredentialProvider {
    public function __construct(array $config = []) {
        // ...
        $this->client = isset($config['client'])
            ? $config['client']
            : \Aws\default_http_handler();
    }
}

I patched it like following to make it work for us:

diff --git a/vendor/aws/aws-sdk-php/src/Credentials/EcsCredentialProvider.php b/vendor/aws/aws-sdk-php/src/Credentials/EcsCredentialProvider.php
index 915673512..7d116068f 100644
--- a/vendor/aws/aws-sdk-php/src/Credentials/EcsCredentialProvider.php
+++ b/vendor/aws/aws-sdk-php/src/Credentials/EcsCredentialProvider.php
@@ -32,9 +32,15 @@ class EcsCredentialProvider
     public function __construct(array $config = [])
     {
         $this->timeout = (float) getenv(self::ENV_TIMEOUT) ?: (isset($config['timeout']) ? $config['timeout'] : 1.0);
-        $this->client = isset($config['client'])
-            ? $config['client']
-            : \Aws\default_http_handler();
+        if (isset($config['client'])) {
+            $this->client = $config['client'];
+        } elseif (isset($config['http_handler'])) {
+            $this->client = $config['http_handler'];
+        } elseif (isset($config['handler'])) {
+            $this->client = $config['handler'];
+        } else {
+            $this->client = \Aws\default_http_handler();
+        }
     }

     /**

I'm not sure if the patch is the proper way to address the issue, and I don't have unit tests added, thus I'm submitting a bug report instead of a PR.

Screenshots

N/A

Additional context

Same issue could also happen on class \Aws\Credentials\InstanceProfileProvider.

ajredniwja commented 3 years ago

Hey @dminy, thanks for opening this issue, trying to understand the issue a bit more here so you are trying to make sure whether the credential provider uses your guzzle handler or not?

Looking at the code it seems your custom handler must be used elsewhere.

deminy commented 3 years ago

Looking at the code it seems your custom handler must be used elsewhere.

Right. I didn't include the code to make the actual API call. The bug happens when calling AWS APIs using ECS credentials (without credentials hardcoded in the PHP code).

Ideally, the customized handler should be used by package aws/aws-sdk-php when using ECS credentials to call AWS APIs, just like how class \Aws\AwsClient does.

The bug had been verified in our backend microservices, and we had made a patch to make it work for us. I'm reporting a bug, and hoping that you guys could make a proper fix for it (although the patch I included did work in our case). Thanks

pflueg commented 2 years ago

Hi everyone,

i was about to report the exact same issue, but in my case it affects the class InstanceProfileProvider. That one and the mentioned EcsCredentialProvider above are not using any custom handler given by the option http_handler. Instead they fallback to the GuzzleHttp\HandlerStack due to the \Aws\default_http_handler(); call.

$this->client = isset($config['client'])
    ? $config['client']
    : \Aws\default_http_handler();

As a workaround it is possible to set the http_handler option and the undocumented client option with the same custom handler, but beware of possible sideeffects. I couldnt see any until now though.

yenfryherrerafeliz commented 11 months ago

Hi @deminy, @pflueg, this behavior is actually expected since http_handler is a client parameter, which means that it will just apply to the client. If you want to pass a custom handler to the ECS or InstanceProfile provider, you can either or create an instance of the provider separated or pass a parameter called "client" in the arguments you provide when instancing the client. Please see my examples below:

Creating a separated instance of the provider: ```php new CustomizedGuzlleHandler() ]); $memoizedProvider = CredentialProvider::memoize($provider); $client = new S3Client([ 'region' => getenv('TEST_REGION'), 'credentials' => $memoizedProvider ]); $client->listBuckets(); ```
Providing the custom handler in the parameter called "client": ```php getenv('TEST_REGION'), 'client' => new CustomizedGuzzleHandler() ]); $client->listBuckets(); ```

Please let me know if that helps!

Thanks!

github-actions[bot] commented 11 months ago

This issue has not recieved a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

github-actions[bot] commented 11 months ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.