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

Class EcsCredentialProvider Doesn't Support Customized Guzzle Handler #2162

Closed
3 tasks done
deminy opened this issue Dec 7, 2020 · 6 comments
Closed
3 tasks done
Assignees
Labels
closing-soon This issue will automatically close in 4 days unless further comments are made. feature-request A feature should be added or improved. p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@deminy
Copy link

deminy commented Dec 7, 2020

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.

@deminy deminy added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 7, 2020
@ajredniwja
Copy link
Contributor

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.

@ajredniwja ajredniwja added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 8, 2020
@deminy
Copy link
Author

deminy commented Dec 8, 2020

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

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 9, 2020
@SamRemis SamRemis added the feature-request A feature should be added or improved. label Jan 4, 2021
@pflueg
Copy link

pflueg commented Jan 16, 2022

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.

@ajredniwja ajredniwja removed the needs-triage This issue or PR still needs to be triaged. label Mar 16, 2022
@yenfryherrerafeliz yenfryherrerafeliz added the p2 This is a standard priority issue label Jan 2, 2023
@stobrien89 stobrien89 removed the bug This issue is a bug. label Mar 29, 2023
@yenfryherrerafeliz
Copy link
Contributor

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

require '../vendor/autoload.php';

use Aws\Credentials\CredentialProvider;
use Aws\S3\S3Client;

$provider = CredentialProvider::ecsCredentials([
    'client' => 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

require '../vendor/autoload.php';

use Aws\S3\S3Client;

$client = new S3Client([
    'region' => getenv('TEST_REGION'),
    'client' => new CustomizedGuzzleHandler()
]);
$client->listBuckets();

Please let me know if that helps!

Thanks!

@yenfryherrerafeliz yenfryherrerafeliz added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 27, 2023
Copy link

github-actions bot commented Dec 3, 2023

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 github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Dec 3, 2023
Copy link

github-actions bot commented Dec 5, 2023

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closing-soon This issue will automatically close in 4 days unless further comments are made. feature-request A feature should be added or improved. p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

6 participants