Minimal Support for Shared Config Loading #675
Conversation
This commit adds a new AWS config module. It only currently supports region resolution. A follow up commit will remove `ProvideRegion` from AWS types.
This commit introduces `aws-config`, the configuration loader entry point for the SDK. Currently, this adds very little new code, instead, it migrates code from `aws_types` into the shared-config package.
| /// will prevent the standard resolution chain from occuring (eg. overriding a region will prevent an HTTP request | ||
| /// to IMDS). | ||
| #[derive(Default, Debug)] | ||
| pub struct EnvLoader { |
There was a problem hiding this comment.
It kind of seems like this should get its own module, or some config loader module, as it is separate from the providers.
| /// This builder supports overriding individual components of the generated config. Overriding a component | ||
| /// will prevent the standard resolution chain from occuring (eg. overriding a region will prevent an HTTP request | ||
| /// to IMDS). |
There was a problem hiding this comment.
This is slightly confusing. I read it to mean credentials won't be retrieved from IMDS if a region is provided.
|
|
||
| impl ProvideRegion for RegionProviderChain { | ||
| fn region(&self) -> future::ProvideRegion { | ||
| future::ProvideRegion::new(self.region()) |
There was a problem hiding this comment.
This looks like it could be an infinite loop. Could we make the call to region more explicit?
| providers: Vec<Box<dyn ProvideRegion>>, | ||
| } | ||
|
|
||
| impl RegionProviderChain { |
There was a problem hiding this comment.
Any reason not to combine this impl block with the one below?
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0. | ||
| */ | ||
|
|
There was a problem hiding this comment.
Doesn't look like this module is referenced anywhere?
There was a problem hiding this comment.
good catch, result of a bad git add
| @@ -50,8 +50,10 @@ fn default_connector() -> Option<DynConnector> { | |||
| /// This provider chain will use defaults for all settings. The region will be resolved with the default | |||
| /// provider chain. To construct a custom provider, use [`default_provider_chain::Builder`](default_provider_chain::Builder). | |||
| pub async fn default_provider() -> impl AsyncProvideCredentials { | |||
| use aws_types::region::ProvideRegion; | |||
| let resolved_region = aws_types::region::default_provider().region().await; | |||
| use aws_config::meta::region::ProvideRegion; | |||
There was a problem hiding this comment.
Would it be worth it to duplicate the region method directly on the chain provider so that this import isn't necessary?
There was a problem hiding this comment.
for backwards compatibility reasons this doesn't return chain_provider it returns impl ProvideRegion
CHANGELOG.md
Outdated
| @@ -1,9 +1,21 @@ | |||
| vNext (Month Day, Year) | |||
| ----------------------- | |||
| **Breaking Changes** | |||
| - `<sevicename>::from_env()` has been removed (#675). A drop-in replacement is available: | |||
| 1. Add a dependency on `aws-config` | |||
There was a problem hiding this comment.
Give an example of adding this dependency
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Motivation and Context
#655
This diff adds support for region loading from the shared config. There were a few bits that needed to be moved around to make that happen. The next major piece is moving credential providers into aws-config.
Description
aws-configpackageProvideRegiontoaws-configaws-configEnvLoaderinaws-configTesting
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.