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

Env vars and profile to configure IMDS retry and timeouts #1821

Closed
wants to merge 2 commits into from
Closed

Env vars and profile to configure IMDS retry and timeouts #1821

wants to merge 2 commits into from

Conversation

kevinpark1217
Copy link
Contributor

@kevinpark1217 kevinpark1217 commented Oct 6, 2022

Motivation and Context

This is a PR for the issue #625 in aws-sdk-rust repo.

Description

Adds the ability to configure retry logic using profile and environment variables.
https://docs.aws.amazon.com/sdkref/latest/guide/feature-ec2-instance-metadata.html

Testing

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kevinpark1217 kevinpark1217 requested a review from a team as a code owner October 6, 2022 21:55
@kevinpark1217
Copy link
Contributor Author

I can also update, so that EndpointSource struct also relies on the parsing logic of Environment. But I would like initial feedbacks first.

@kevinpark1217
Copy link
Contributor Author

@Velfi It's ready for review 😄

@Velfi
Copy link
Contributor

Velfi commented Oct 10, 2022

Hey @kevinpark1217, I'm out sick today but I wanted to share that in the next release of the SDK (in part thanks to that bug you fixed) you'll be able to set timeout and retry behavior for the IMDS client with code similar to this:

let credentials_provider = ImdsCredentialsProvider::builder()
    .timout_config(timeout_config)
    .retry_config(retry_config)
    .build();
let credentials_provider = LazyCachingCredentialsProvider::new()
    .load(credentials_provider)
    .build();
let sdk_config = aws_config::from_env().credentials_provider(credentials_provider).load().await;

I'll comment again when the release goes out and we can discuss whether or not it's still worthwhile to support these env vars. I spoke to the authors of that page you linked in the issue and they conceded that it's not a true standard and that those env vars are supported for the CLI because it wouldn't be easily configurable otherwise. The Python SDK IS the CLI and so it supports them as well. However, no other SDK does excepting the PHP SDK which predates most other SDKs.

Thanks for your patience on this. Our end goal is that we make it convenient to configure this client, and I'm hoping that the features in the next release will accomplish that to your (and other user's) satisfaction.

@kevinpark1217
Copy link
Contributor Author

Hey @Velfi, has there been any internal discussion on supporting these environment variables?

I still think it is valuable in order to configure applications that relies on AWS SDK under the hood. Customers can deploy third-party products into their own "on-prem" AWS account where they don't have direct access to the source code and limited configuration.

@kevinpark1217
Copy link
Contributor Author

Also the example code does not work because, the retry and timeout is set as while building the Client struct, which is a layer below the ImdsCredentialsProvider.

There is just so many layers of configuration just to reach IMDS Client configuration that it's cumbersome to configure IMDS retry and timeout through code.

@jdisanti
Copy link
Collaborator

The example should work after the next SDK release since the bug that was preventing the config from applying has been fixed.

@kevinpark1217
Copy link
Contributor Author

@jdisanti Sorry, that is not what I meant.

Other than the bug fix that prevented setting timeouts in code, I also wanted the ability to implement them using environment variables and profile because IMDS client is hidden away in layers of credential provider structs.

But, I think I may have found another way that works for me. Please check out #1867.

With #1867 , then I can live without the new environment variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants