Skip to content

Commit

Permalink
Integrate TimeSource into the orchestrator and middleware (breaking c…
Browse files Browse the repository at this point in the history
…hange warning is spurious) (#2728)

## Motivation and Context
- #2262 
- #2087 
- #2707 

This adds `TimeSource` in SDK and service configs so we can effectively
control time when executing requests with the SDK at the client level.

_note:_ the breaking change is in a trait implementation of a struct
we're going to delete, not a real breaking change

## Description
- Add `SharedTimeSource` and use it in SdkConfig / `aws-config` /
<service>::Config
- Wire up the signer and other uses of `SystemTime::now()` that I could
find
- track down broken tests

## Testing
- [x] various unit tests that all still pass

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [x] 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._
  • Loading branch information
rcoh committed May 30, 2023
1 parent cf292d5 commit b30b063
Show file tree
Hide file tree
Showing 37 changed files with 325 additions and 141 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,27 @@ references = ["smithy-rs#2539"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server" }
author = "david-perez"

[[smithy-rs]]
message = "Time is now controlled by the `TimeSource` trait. This facilitates testing as well as use cases like WASM where `SystemTime::now()` is not supported."
references = ["smithy-rs#2728", "smithy-rs#2262", "aws-sdk-rust#2087"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
author = "rcoh"

[[smithy-rs]]
message = "The property bag type for Time is now `SharedTimeSource`, not `SystemTime`. If your code relies on setting request time, use `aws_smithy_async::time::SharedTimeSource`."
references = ["smithy-rs#2728", "smithy-rs#2262", "aws-sdk-rust#2087"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
author = "rcoh"

[[aws-sdk-rust]]
message = "Time is now controlled by the `TimeSource` trait. This facilitates testing as well as use cases like WASM where `SystemTime::now()` is not supported."
references = ["smithy-rs#2728", "smithy-rs#2262", "aws-sdk-rust#2087"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "rcoh"

[[smithy-rs]]
message = "Bump dependency on `lambda_http` by `aws-smithy-http-server` to 0.8.0. This version of `aws-smithy-http-server` is only guaranteed to be compatible with 0.8.0, or semver-compatible versions of 0.8.0 of the `lambda_http` crate. It will not work with versions prior to 0.8.0 _at runtime_, making requests to your smithy-rs service unroutable, so please make sure you're running your service in a compatible configuration"
author = "david-perez"
references = ["smithy-rs#2676", "smithy-rs#2685"]
meta = { "breaking" = true, "tada" = false, "bug" = false }

1 change: 1 addition & 0 deletions aws/rust-runtime/aws-config/external-types.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ allowed_external_types = [
"aws_credential_types::provider::SharedCredentialsProvider",
"aws_sdk_sts::types::_policy_descriptor_type::PolicyDescriptorType",
"aws_smithy_async::rt::sleep::AsyncSleep",
"aws_smithy_async::time::TimeSource",
"aws_smithy_client::bounds::SmithyConnector",
"aws_smithy_client::conns::default_connector::default_connector",
"aws_smithy_client::erase::DynConnector",
Expand Down
8 changes: 4 additions & 4 deletions aws/rust-runtime/aws-config/src/imds/client/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
use crate::imds::client::error::{ImdsError, TokenError, TokenErrorKind};
use crate::imds::client::ImdsResponseRetryClassifier;
use aws_credential_types::cache::ExpiringCache;
use aws_credential_types::time_source::TimeSource;
use aws_http::user_agent::UserAgentStage;
use aws_smithy_async::rt::sleep::AsyncSleep;
use aws_smithy_async::time::SharedTimeSource;
use aws_smithy_client::erase::DynConnector;
use aws_smithy_client::retry;
use aws_smithy_http::body::SdkBody;
Expand Down Expand Up @@ -65,7 +65,7 @@ pub(super) struct TokenMiddleware {
client: Arc<aws_smithy_client::Client<DynConnector, MapRequestLayer<UserAgentStage>>>,
token_parser: GetTokenResponseHandler,
token: ExpiringCache<Token, ImdsError>,
time_source: TimeSource,
time_source: SharedTimeSource,
endpoint: Uri,
token_ttl: Duration,
}
Expand All @@ -79,7 +79,7 @@ impl Debug for TokenMiddleware {
impl TokenMiddleware {
pub(super) fn new(
connector: DynConnector,
time_source: TimeSource,
time_source: SharedTimeSource,
endpoint: Uri,
token_ttl: Duration,
retry_config: retry::Config,
Expand Down Expand Up @@ -170,7 +170,7 @@ impl AsyncMapRequest for TokenMiddleware {

#[derive(Clone)]
struct GetTokenResponseHandler {
time: TimeSource,
time: SharedTimeSource,
}

impl ParseStrictResponse for GetTokenResponseHandler {
Expand Down
9 changes: 6 additions & 3 deletions aws/rust-runtime/aws-config/src/imds/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use crate::imds::client::LazyClient;
use crate::json_credentials::{parse_json_credentials, JsonCredentials, RefreshableCredentials};
use crate::provider_config::ProviderConfig;
use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials};
use aws_credential_types::time_source::TimeSource;
use aws_credential_types::Credentials;
use aws_smithy_async::time::SharedTimeSource;
use aws_types::os_shim_internal::Env;
use std::borrow::Cow;
use std::error::Error as StdError;
Expand Down Expand Up @@ -53,7 +53,7 @@ pub struct ImdsCredentialsProvider {
client: LazyClient,
env: Env,
profile: Option<String>,
time_source: TimeSource,
time_source: SharedTimeSource,
last_retrieved_credentials: Arc<RwLock<Option<Credentials>>>,
}

Expand Down Expand Up @@ -390,7 +390,10 @@ mod test {
.build();
let creds = provider.provide_credentials().await.expect("valid creds");
// The expiry should be equal to what is originally set (==2021-09-21T04:16:53Z).
assert!(creds.expiry() == UNIX_EPOCH.checked_add(Duration::from_secs(1632197813)));
assert_eq!(
creds.expiry(),
UNIX_EPOCH.checked_add(Duration::from_secs(1632197813))
);
connection.assert_requests_match(&[]);

// There should not be logs indicating credentials are extended for stability.
Expand Down
15 changes: 14 additions & 1 deletion aws/rust-runtime/aws-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ mod loader {
use aws_credential_types::cache::CredentialsCache;
use aws_credential_types::provider::{ProvideCredentials, SharedCredentialsProvider};
use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep};
use aws_smithy_async::time::{SharedTimeSource, TimeSource};
use aws_smithy_client::http_connector::HttpConnector;
use aws_smithy_types::retry::RetryConfig;
use aws_smithy_types::timeout::TimeoutConfig;
Expand Down Expand Up @@ -192,6 +193,7 @@ mod loader {
profile_files_override: Option<ProfileFiles>,
use_fips: Option<bool>,
use_dual_stack: Option<bool>,
time_source: Option<SharedTimeSource>,
}

impl ConfigLoader {
Expand Down Expand Up @@ -262,6 +264,12 @@ mod loader {
self
}

/// Set the time source used for tasks like signing requests
pub fn time_source(mut self, time_source: impl TimeSource + 'static) -> Self {
self.time_source = Some(SharedTimeSource::new(time_source));
self
}

/// Override the [`HttpConnector`] for this [`ConfigLoader`]. The connector will be used when
/// sending operations. This **does not set** the HTTP connector used by config providers.
/// To change that connector, use [ConfigLoader::configure].
Expand Down Expand Up @@ -563,7 +571,9 @@ mod loader {
.unwrap_or_else(|| HttpConnector::ConnectorFn(Arc::new(default_connector)));

let credentials_cache = self.credentials_cache.unwrap_or_else(|| {
let mut builder = CredentialsCache::lazy_builder().time_source(conf.time_source());
let mut builder = CredentialsCache::lazy_builder().time_source(
aws_credential_types::time_source::TimeSource::shared(conf.time_source()),
);
builder.set_sleep(conf.sleep());
builder.into_credentials_cache()
});
Expand All @@ -588,12 +598,15 @@ mod loader {
SharedCredentialsProvider::new(builder.build().await)
};

let ts = self.time_source.unwrap_or_default();

let mut builder = SdkConfig::builder()
.region(region)
.retry_config(retry_config)
.timeout_config(timeout_config)
.credentials_cache(credentials_cache)
.credentials_provider(credentials_provider)
.time_source(ts)
.http_connector(http_connector);

builder.set_app_name(app_name);
Expand Down
18 changes: 12 additions & 6 deletions aws/rust-runtime/aws-config/src/profile/credentials/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::web_identity_token::{StaticConfiguration, WebIdentityTokenCredentials
use aws_credential_types::provider::{self, error::CredentialsError, ProvideCredentials};
use aws_sdk_sts::config::{Builder as StsConfigBuilder, Credentials};
use aws_sdk_sts::Client as StsClient;
use aws_smithy_async::time::SharedTimeSource;
use std::fmt::Debug;
use std::sync::Arc;

Expand All @@ -22,6 +23,7 @@ pub(super) struct AssumeRoleProvider {
role_arn: String,
external_id: Option<String>,
session_name: Option<String>,
time_source: SharedTimeSource,
}

impl AssumeRoleProvider {
Expand All @@ -35,11 +37,9 @@ impl AssumeRoleProvider {
.credentials_provider(input_credentials)
.build();
let client = StsClient::from_conf(config);
let session_name = &self
.session_name
.as_ref()
.cloned()
.unwrap_or_else(|| sts::util::default_session_name("assume-role-from-profile"));
let session_name = &self.session_name.as_ref().cloned().unwrap_or_else(|| {
sts::util::default_session_name("assume-role-from-profile", self.time_source.now())
});
let assume_role_creds = client
.assume_role()
.role_arn(&self.role_arn)
Expand Down Expand Up @@ -97,7 +97,12 @@ impl ProviderChain {
web_identity_token_file: web_identity_token_file.into(),
role_arn: role_arn.to_string(),
session_name: session_name.map(|sess| sess.to_string()).unwrap_or_else(
|| sts::util::default_session_name("web-identity-token-profile"),
|| {
sts::util::default_session_name(
"web-identity-token-profile",
provider_config.time_source().now(),
)
},
),
})
.configure(provider_config)
Expand Down Expand Up @@ -140,6 +145,7 @@ impl ProviderChain {
role_arn: role_arn.role_arn.into(),
external_id: role_arn.external_id.map(|id| id.into()),
session_name: role_arn.session_name.map(|id| id.into()),
time_source: provider_config.time_source(),
}
})
.collect();
Expand Down
14 changes: 7 additions & 7 deletions aws/rust-runtime/aws-config/src/provider_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use aws_credential_types::time_source::TimeSource;
use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep};
use aws_smithy_async::time::SharedTimeSource;
use aws_smithy_client::erase::DynConnector;
use aws_smithy_types::error::display::DisplayErrorContext;
use aws_types::os_shim_internal::{Env, Fs};
Expand Down Expand Up @@ -38,7 +39,7 @@ use crate::profile::{ProfileFileLoadError, ProfileSet};
pub struct ProviderConfig {
env: Env,
fs: Fs,
time_source: TimeSource,
time_source: SharedTimeSource,
connector: HttpConnector,
sleep: Option<Arc<dyn AsyncSleep>>,
region: Option<Region>,
Expand Down Expand Up @@ -72,7 +73,7 @@ impl Default for ProviderConfig {
Self {
env: Env::default(),
fs: Fs::default(),
time_source: TimeSource::default(),
time_source: SharedTimeSource::default(),
connector,
sleep: default_async_sleep(),
region: None,
Expand All @@ -90,7 +91,6 @@ impl ProviderConfig {
/// Unlike [`ProviderConfig::empty`] where `env` and `fs` will use their non-mocked implementations,
/// this method will use an empty mock environment and an empty mock file system.
pub fn no_configuration() -> Self {
use aws_credential_types::time_source::TestingTimeSource;
use std::collections::HashMap;
use std::time::UNIX_EPOCH;
let fs = Fs::from_raw_map(HashMap::new());
Expand All @@ -100,7 +100,7 @@ impl ProviderConfig {
profile_files: ProfileFiles::default(),
env,
fs,
time_source: TimeSource::testing(&TestingTimeSource::new(UNIX_EPOCH)),
time_source: SharedTimeSource::new(UNIX_EPOCH),
connector: HttpConnector::Prebuilt(None),
sleep: None,
region: None,
Expand Down Expand Up @@ -140,7 +140,7 @@ impl ProviderConfig {
ProviderConfig {
env: Env::default(),
fs: Fs::default(),
time_source: TimeSource::default(),
time_source: SharedTimeSource::default(),
connector: HttpConnector::Prebuilt(None),
sleep: None,
region: None,
Expand Down Expand Up @@ -179,7 +179,7 @@ impl ProviderConfig {
}

#[allow(dead_code)]
pub(crate) fn time_source(&self) -> TimeSource {
pub(crate) fn time_source(&self) -> SharedTimeSource {
self.time_source.clone()
}

Expand Down Expand Up @@ -293,7 +293,7 @@ impl ProviderConfig {
#[doc(hidden)]
pub fn with_time_source(self, time_source: TimeSource) -> Self {
ProviderConfig {
time_source,
time_source: SharedTimeSource::new(time_source),
..self
}
}
Expand Down
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-config/src/sts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ impl crate::provider_config::ProviderConfig {
.http_connector(expect_connector(self.connector(&Default::default())))
.retry_config(RetryConfig::standard())
.region(self.region())
.time_source(self.time_source())
.credentials_cache(CredentialsCache::no_caching());
builder.set_sleep_impl(self.sleep());
builder
Expand Down
7 changes: 4 additions & 3 deletions aws/rust-runtime/aws-config/src/sts/assume_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,14 @@ impl AssumeRoleProviderBuilder {
let mut config = aws_sdk_sts::Config::builder()
.credentials_cache(credentials_cache)
.credentials_provider(provider)
.time_source(conf.time_source())
.region(self.region.clone())
.http_connector(expect_connector(conf.connector(&Default::default())));
config.set_sleep_impl(conf.sleep());

let session_name = self
.session_name
.unwrap_or_else(|| super::util::default_session_name("assume-role-provider"));
let session_name = self.session_name.unwrap_or_else(|| {
super::util::default_session_name("assume-role-provider", conf.time_source().now())
});

let sts_client = StsClient::from_conf(config.build());
let fluent_builder = sts_client
Expand Down
7 changes: 3 additions & 4 deletions aws/rust-runtime/aws-config/src/sts/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use aws_credential_types::provider::{self, error::CredentialsError};
use aws_credential_types::Credentials as AwsCredentials;
use aws_sdk_sts::types::Credentials as StsCredentials;

use aws_smithy_async::time::TimeSource;
use std::convert::TryFrom;
use std::time::{SystemTime, UNIX_EPOCH};

Expand Down Expand Up @@ -45,9 +46,7 @@ pub(crate) fn into_credentials(
/// STS Assume Role providers MUST assign a name to their generated session. When a user does not
/// provide a name for the session, the provider will choose a name composed of a base + a timestamp,
/// e.g. `profile-file-provider-123456789`
pub(crate) fn default_session_name(base: &str) -> String {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("post epoch");
pub(crate) fn default_session_name(base: &str, ts: SystemTime) -> String {
let now = ts.now().duration_since(UNIX_EPOCH).expect("post epoch");
format!("{}-{}", base, now.as_millis())
}
9 changes: 6 additions & 3 deletions aws/rust-runtime/aws-config/src/web_identity_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use crate::provider_config::ProviderConfig;
use crate::sts;
use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials};
use aws_sdk_sts::Client as StsClient;
use aws_smithy_async::time::SharedTimeSource;
use aws_smithy_types::error::display::DisplayErrorContext;
use aws_types::os_shim_internal::{Env, Fs};
use std::borrow::Cow;
Expand All @@ -80,6 +81,7 @@ const ENV_VAR_SESSION_NAME: &str = "AWS_ROLE_SESSION_NAME";
#[derive(Debug)]
pub struct WebIdentityTokenCredentialsProvider {
source: Source,
time_source: SharedTimeSource,
fs: Fs,
sts_client: StsClient,
}
Expand Down Expand Up @@ -131,9 +133,9 @@ impl WebIdentityTokenCredentialsProvider {
"AWS_ROLE_ARN environment variable must be set",
)
})?;
let session_name = env
.get(ENV_VAR_SESSION_NAME)
.unwrap_or_else(|_| sts::util::default_session_name("web-identity-token"));
let session_name = env.get(ENV_VAR_SESSION_NAME).unwrap_or_else(|_| {
sts::util::default_session_name("web-identity-token", self.time_source.now())
});
Ok(Cow::Owned(StaticConfiguration {
web_identity_token_file: token_file.into(),
role_arn,
Expand Down Expand Up @@ -203,6 +205,7 @@ impl Builder {
source,
fs: conf.fs(),
sts_client: StsClient::from_conf(conf.sts_client_config().build()),
time_source: conf.time_source(),
}
}
}
Expand Down
Loading

0 comments on commit b30b063

Please sign in to comment.