aws-sdk-cpp icon indicating copy to clipboard operation
aws-sdk-cpp copied to clipboard

[aws-cpp-sdk-core]: allow disabling IMDS in ClientConfiguration default constructor

Open grrtrr opened this issue 2 years ago • 1 comments

Problem Description

The ClientConfiguration and its sub-classes allow disabling IMDS lookup in all but the default constructor:

        struct AWS_CORE_API ClientConfiguration
        {
            ClientConfiguration(); // Does IMDS look-up by default, also in sub-classes.
            ClientConfiguration(const char* profileName, bool shouldDisableIMDS = false);
            explicit ClientConfiguration(bool useSmartDefaults, const char* defaultMode = "legacy", bool shouldDisableIMDS = false);
        }

In the base class, profileName can be set to nullptr, but not in the sub-classes: e.g. in S3CrtClientConfiguration, the const char *inputProfileName gets converted to a std::string (which does not like nullptr).

Hence the following use-case will always cause IMDS look-up:

        Aws::S3Crt::ClientConfiguration config;   // Default constructor
        config.scheme = defaults.scheme;
        config.endpointOverride = defaults.endpointOverride;
        config.connectTimeoutMs = 30000;
        // ...

This is because the default constructor hard-codes IMDS lookup:

// src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp
ClientConfiguration::ClientConfiguration()
{
    this->disableIMDS = false;  // <=== HERE
    // ...

    if (!this->disableIMDS &&
        region.empty() &&
        Aws::Utils::StringUtils::ToLower(Aws::Environment::GetEnv("AWS_EC2_METADATA_DISABLED").c_str()) != "true")
    {
        auto client = Aws::Internal::GetEC2MetadataClient();
        if (client)
        {
            region = client->GetCurrentRegion();  // <== IMDS look-up
        }
    }
    // ...
}

For an earlier record of the problems (slowness) that unnecessary IMDS look-up causes, please see https://github.com/aws/aws-sdk-cpp/issues/1440.

What to do

For ClientConfiguration, GenericClientConfiguration, and Aws::S3Crt::ClientConfiguration, please provide a shouldDisableIMDS default argument like the 2 other constructors:

        struct AWS_CORE_API ClientConfiguration
        {
            explicit ClientConfiguration(bool shouldDisableIMDS = false) {
               this->disableIMDS = shouldDisableIMDS;
               setLegacyClientConfigurationParameters(*this);
               // ...
            }
        }

grrtrr avatar Aug 02 '23 14:08 grrtrr

Hello @grrtrr ,

Thank you very much for your submission. This is a reasonable request and would be a great addition to the SDK. I will mark this as p2 and post updates regarding implementation/release here.

Thank you very much for your time and feedback.

Sincerely,

Yasmine

yasminetalby avatar Aug 02 '23 15:08 yasminetalby

Fixed with this PR: https://github.com/aws/aws-sdk-cpp/pull/2618

jmklix avatar Jun 28 '24 19:06 jmklix

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.

github-actions[bot] avatar Jun 28 '24 19:06 github-actions[bot]