Skip to content

C++ support AWS_METADATA_SERVICE_TIMEOUT #3493

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@

namespace Aws
{
namespace Client
{
struct ClientConfiguration;
}
namespace Auth
{
constexpr int REFRESH_THRESHOLD = 1000 * 60 * 5;
Expand Down Expand Up @@ -212,6 +216,11 @@ namespace Aws
*/
InstanceProfileCredentialsProvider(const std::shared_ptr<Aws::Config::EC2InstanceProfileConfigLoader>&, long refreshRateMs = REFRESH_THRESHOLD);

/**
* Initializes the provider using ClientConfiguration for IMDS settings.
*/
InstanceProfileCredentialsProvider(const Aws::Client::ClientConfiguration::CredentialProviderConfiguration& credentialProviderConfig, long refreshRateMs = REFRESH_THRESHOLD);

/**
* Retrieves the credentials if found, otherwise returns empty credential set.
*/
Expand Down
15 changes: 15 additions & 0 deletions src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,21 @@ namespace Aws
* AWS profile name to use for credentials.
*/
Aws::String profile;

/**
* IMDS configuration settings
*/
struct {
/**
* Number of total attempts to make when retrieving data from IMDS. Default 1.
*/
long metadataServiceNumAttempts = 1;

/**
* Timeout in seconds when retrieving data from IMDS. Default 1.
*/
long metadataServiceTimeout = 1;
} imdsConfig;
}credentialProviderConfig;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#pragma once

#include <aws/core/config/AWSProfileConfigLoaderBase.h>
#include <aws/core/client/ClientConfiguration.h>

#include <aws/core/utils/memory/stl/AWSString.h>
#include <aws/core/utils/memory/stl/AWSMap.h>
Expand All @@ -19,6 +20,8 @@ namespace Aws
class EC2MetadataClient;
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace change without content changed


namespace Config
{
static const char* const INSTANCE_PROFILE_KEY = "InstanceProfile";
Expand All @@ -33,6 +36,16 @@ namespace Aws
* If client is nullptr, the default EC2MetadataClient will be created.
*/
EC2InstanceProfileConfigLoader(const std::shared_ptr<Aws::Internal::EC2MetadataClient>& = nullptr);

/**
* Creates EC2MetadataClient using the provided ClientConfiguration.
*/
EC2InstanceProfileConfigLoader(const Aws::Client::ClientConfiguration& clientConfig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we adding a additional for ClientConfiguration shouldn't we only be adding one for CredentialProviderConfiguration ?


/**
* Creates EC2MetadataClient using the provided CredentialProviderConfiguration.
*/
EC2InstanceProfileConfigLoader(const Aws::Client::ClientConfiguration::CredentialProviderConfiguration& credentialConfig);

virtual ~EC2InstanceProfileConfigLoader() = default;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ namespace Aws
*/
EC2MetadataClient(const char* endpoint = "http://169.254.169.254");
EC2MetadataClient(const Client::ClientConfiguration& clientConfiguration, const char* endpoint = "http://169.254.169.254");
EC2MetadataClient(const Client::ClientConfiguration::CredentialProviderConfiguration& credentialConfig, const char* endpoint = "http://169.254.169.254");

EC2MetadataClient& operator =(const EC2MetadataClient& rhs) = delete;
EC2MetadataClient(const EC2MetadataClient& rhs) = delete;
Expand Down
7 changes: 7 additions & 0 deletions src/aws-cpp-sdk-core/source/auth/AWSCredentialsProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <aws/core/auth/AWSCredentialsProvider.h>

#include <aws/core/config/AWSProfileConfigLoader.h>
#include <aws/core/client/ClientConfiguration.h>
#include <aws/core/platform/Environment.h>
#include <aws/core/platform/FileSystem.h>
#include <aws/core/platform/OSVersionInfo.h>
Expand Down Expand Up @@ -242,6 +243,12 @@ InstanceProfileCredentialsProvider::InstanceProfileCredentialsProvider(const std
AWS_LOGSTREAM_INFO(INSTANCE_LOG_TAG, "Creating Instance with injected EC2MetadataClient and refresh rate " << refreshRateMs);
}

InstanceProfileCredentialsProvider::InstanceProfileCredentialsProvider(const Aws::Client::ClientConfiguration::CredentialProviderConfiguration& credentialConfig, long refreshRateMs) :
m_ec2MetadataConfigLoader(Aws::MakeShared<Aws::Config::EC2InstanceProfileConfigLoader>(INSTANCE_LOG_TAG, credentialConfig)),
m_loadFrequencyMs(refreshRateMs)
{
AWS_LOGSTREAM_INFO(INSTANCE_LOG_TAG, "Creating Instance with IMDS timeout: " << credentialConfig.imdsConfig.metadataServiceTimeout << "s, attempts: " << credentialConfig.imdsConfig.metadataServiceNumAttempts);
}

AWSCredentials InstanceProfileCredentialsProvider::GetAWSCredentials()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <aws/core/auth/AWSCredentialsProviderChain.h>
#include <aws/core/auth/STSCredentialsProvider.h>
#include <aws/core/auth/SSOCredentialsProvider.h>
#include <aws/core/client/ClientConfiguration.h>
#include <aws/core/platform/Environment.h>
#include <aws/core/utils/memory/AWSMemory.h>
#include <aws/core/utils/StringUtils.h>
Expand Down Expand Up @@ -125,7 +126,7 @@ DefaultAWSCredentialsProviderChain::DefaultAWSCredentialsProviderChain(const Aws
}
else if (Aws::Utils::StringUtils::ToLower(ec2MetadataDisabled.c_str()) != "true")
{
AddProvider(Aws::MakeShared<InstanceProfileCredentialsProvider>(DefaultCredentialsProviderChainTag));
AddProvider(Aws::MakeShared<InstanceProfileCredentialsProvider>(DefaultCredentialsProviderChainTag, config));
AWS_LOGSTREAM_INFO(DefaultCredentialsProviderChainTag, "Added EC2 metadata service credentials provider to the provider chain.");
}
}
Expand Down
29 changes: 29 additions & 0 deletions src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ static const char* DISABLE_IMDSV1_CONFIG_VAR = "AWS_EC2_METADATA_V1_DISABLED";
static const char* DISABLE_IMDSV1_ENV_VAR = "ec2_metadata_v1_disabled";
static const char* AWS_ACCOUNT_ID_ENDPOINT_MODE_ENVIRONMENT_VARIABLE = "AWS_ACCOUNT_ID_ENDPOINT_MODE";
static const char* AWS_ACCOUNT_ID_ENDPOINT_MODE_CONFIG_FILE_OPTION = "account_id_endpoint_mode";
static const char* AWS_METADATA_SERVICE_TIMEOUT_ENV_VAR = "AWS_METADATA_SERVICE_TIMEOUT";
static const char* AWS_METADATA_SERVICE_TIMEOUT_CONFIG_VAR = "metadata_service_timeout";
static const char* AWS_METADATA_SERVICE_NUM_ATTEMPTS_ENV_VAR = "AWS_METADATA_SERVICE_NUM_ATTEMPTS";
static const char* AWS_METADATA_SERVICE_NUM_ATTEMPTS_CONFIG_VAR = "metadata_service_num_attempts";

using RequestChecksumConfigurationEnumMapping = std::pair<const char*, RequestChecksumCalculation>;
static const std::array<RequestChecksumConfigurationEnumMapping, 2> REQUEST_CHECKSUM_CONFIG_MAPPING = {{
Expand Down Expand Up @@ -288,6 +292,31 @@ void setConfigFromEnvOrProfile(ClientConfiguration &config)
AWS_ACCOUNT_ID_ENDPOINT_MODE_CONFIG_FILE_OPTION,
{"required", "disabled", "preferred"}, /* allowed values */
"preferred" /* default value */);

// Load IMDS configuration from environment variables and config file
Aws::String timeoutStr = ClientConfiguration::LoadConfigFromEnvOrProfile(AWS_METADATA_SERVICE_TIMEOUT_ENV_VAR,
config.profileName,
AWS_METADATA_SERVICE_TIMEOUT_CONFIG_VAR,
{}, /* allowed values */
"1" /* default value */);

// Load IMDS configuration from environment variables and config file
Aws::String numAttemptsStr = ClientConfiguration::LoadConfigFromEnvOrProfile(AWS_METADATA_SERVICE_NUM_ATTEMPTS_ENV_VAR,
config.profileName,
AWS_METADATA_SERVICE_NUM_ATTEMPTS_CONFIG_VAR,
{}, /* allowed values */
"1" /* default value */);

// Parse and set IMDS num attempts
long attempts = std::stol(numAttemptsStr.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont use std::stol use our StringUtils class , matching function.

if (attempts >= 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont think we need to bounds check this, should be able to just assign

config.credentialProviderConfig.imdsConfig.metadataServiceNumAttempts = attempts;
}
// Parse and set IMDS timeout
long timeout = std::stol(timeoutStr.c_str());
if (timeout >= 1) {
config.credentialProviderConfig.imdsConfig.metadataServiceTimeout = timeout;
}
}

ClientConfiguration::ClientConfiguration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <aws/core/config/AWSProfileConfigLoader.h>
#include <aws/core/internal/AWSHttpResourceClient.h>
#include <aws/core/auth/AWSCredentialsProvider.h>
#include <aws/core/client/ClientConfiguration.h>
#include <aws/core/utils/memory/stl/AWSList.h>
#include <aws/core/utils/logging/LogMacros.h>
#include <aws/core/utils/json/JsonSerializer.h>
Expand Down Expand Up @@ -38,6 +39,14 @@ namespace Aws
}
}

EC2InstanceProfileConfigLoader::EC2InstanceProfileConfigLoader(const Aws::Client::ClientConfiguration& clientConfig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont this we want to add a constructor for Aws::Client::ClientConfiguration we only want to add one for CredentialProviderConfiguration

: m_ec2metadataClient(Aws::MakeShared<Aws::Internal::EC2MetadataClient>(EC2_INSTANCE_PROFILE_LOG_TAG, clientConfig))
{}

EC2InstanceProfileConfigLoader::EC2InstanceProfileConfigLoader(const Aws::Client::ClientConfiguration::CredentialProviderConfiguration& credentialConfig)
: m_ec2metadataClient(Aws::MakeShared<Aws::Internal::EC2MetadataClient>(EC2_INSTANCE_PROFILE_LOG_TAG, credentialConfig))
{}

bool EC2InstanceProfileConfigLoader::LoadInternal()
{
// re-use old credentials until we need to call IMDS again.
Expand Down
24 changes: 24 additions & 0 deletions src/aws-cpp-sdk-core/source/internal/AWSHttpResourceClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,30 @@ namespace Aws
AWS_LOGSTREAM_TRACE(m_logtag.c_str(), "IMDSv1 had been disabled at the SDK build time");
#endif
}

EC2MetadataClient::EC2MetadataClient(const Aws::Client::ClientConfiguration::CredentialProviderConfiguration& credentialConfig,
const char *endpoint) :
AWSHttpResourceClient([&credentialConfig]() {
Aws::Client::ClientConfiguration clientConfig;
clientConfig.credentialProviderConfig = credentialConfig;
clientConfig.connectTimeoutMs = credentialConfig.imdsConfig.metadataServiceTimeout * 1000;
clientConfig.requestTimeoutMs = credentialConfig.imdsConfig.metadataServiceTimeout * 1000;
clientConfig.retryStrategy = Aws::MakeShared<DefaultRetryStrategy>(RESOURCE_CLIENT_CONFIGURATION_ALLOCATION_TAG, credentialConfig.imdsConfig.metadataServiceNumAttempts - 1, 1000);
clientConfig.maxConnections = 2;
clientConfig.scheme = Scheme::HTTP;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why HTTP, thats not secure why are we setting this at all?

return clientConfig;
}(), EC2_METADATA_CLIENT_LOG_TAG),
m_endpoint(endpoint),
m_disableIMDS(false),
m_tokenRequired(true),
m_disableIMDSV1(false)
{
#if defined(DISABLE_IMDSV1)
AWS_UNREFERENCED_PARAM(m_disableIMDSV1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have a AWS_UNREFERENCED_PARAM on a member variable that is referenced in the constructor above?

m_disableIMDSV1 = true;
AWS_LOGSTREAM_TRACE(m_logtag.c_str(), "IMDSv1 had been disabled at the SDK build time");
#endif
}

EC2MetadataClient::~EC2MetadataClient()
{
Expand Down
Loading