Unverified Commit 26827f1a authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Don't allow `test-util` feature in compile time dependencies (#2843)

During the orchestrator implementation, a bug was introduced that always
enabled the `test-util` feature on `aws-smithy-runtime` in the generated
crates. This led to several pieces of non-test code relying on test-only
code, specifically around `SystemTime` implementing `TimeSource`.

This PR fixes that issue, and also fixes another issue where the
`RuntimeComponentsBuilder` was being initialized without a name for the
service config.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 3d1c34d3
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -91,6 +91,7 @@ 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_smithy_async::time::StaticTimeSource;
        use std::collections::HashMap;
        use std::time::UNIX_EPOCH;
        let fs = Fs::from_raw_map(HashMap::new());
@@ -100,7 +101,7 @@ impl ProviderConfig {
            profile_files: ProfileFiles::default(),
            env,
            fs,
            time_source: SharedTimeSource::new(UNIX_EPOCH),
            time_source: SharedTimeSource::new(StaticTimeSource::new(UNIX_EPOCH)),
            connector: HttpConnector::Prebuilt(None),
            sleep: None,
            region: None,
+1 −2
Original line number Diff line number Diff line
@@ -7,7 +7,6 @@ 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};

@@ -47,6 +46,6 @@ pub(crate) fn into_credentials(
/// 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, ts: SystemTime) -> String {
    let now = ts.now().duration_since(UNIX_EPOCH).expect("post epoch");
    let now = ts.duration_since(UNIX_EPOCH).expect("post epoch");
    format!("{}-{}", base, now.as_millis())
}
+57 −12
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@

package software.amazon.smithy.rustsdk

import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule
import software.amazon.smithy.rust.codegen.client.smithy.generators.client.CustomizableOperationCustomization
import software.amazon.smithy.rust.codegen.client.smithy.generators.client.CustomizableOperationSection
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
@@ -18,28 +19,70 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) :
    CustomizableOperationCustomization() {
    private val codegenScope = arrayOf(
        *RuntimeType.preludeScope,
        "AwsUserAgent" to AwsRuntimeType.awsHttp(runtimeConfig)
            .resolve("user_agent::AwsUserAgent"),
        "AwsUserAgent" to AwsRuntimeType.awsHttp(runtimeConfig).resolve("user_agent::AwsUserAgent"),
        "BeforeTransmitInterceptorContextMut" to RuntimeType.beforeTransmitInterceptorContextMut(runtimeConfig),
        "ConfigBag" to RuntimeType.configBag(runtimeConfig),
        "ConfigBagAccessors" to RuntimeType.configBagAccessors(runtimeConfig),
        "http" to CargoDependency.Http.toType(),
        "InterceptorContext" to RuntimeType.interceptorContext(runtimeConfig),
        "StaticRuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig)
            .resolve("client::runtime_plugin::StaticRuntimePlugin"),
        "RuntimeComponentsBuilder" to RuntimeType.runtimeComponentsBuilder(runtimeConfig),
        "SharedTimeSource" to CargoDependency.smithyAsync(runtimeConfig).withFeature("test-util").toType()
            .resolve("time::SharedTimeSource"),
        "SharedInterceptor" to RuntimeType.smithyRuntimeApi(runtimeConfig)
            .resolve("client::interceptors::SharedInterceptor"),
        "TestParamsSetterInterceptor" to CargoDependency.smithyRuntime(runtimeConfig).withFeature("test-util")
            .toType().resolve("client::test_util::interceptors::TestParamsSetterInterceptor"),
        "SharedInterceptor" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::SharedInterceptor"),
        "SharedTimeSource" to CargoDependency.smithyAsync(runtimeConfig).toType().resolve("time::SharedTimeSource"),
        "StaticRuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::runtime_plugin::StaticRuntimePlugin"),
        "StaticTimeSource" to CargoDependency.smithyAsync(runtimeConfig).toType().resolve("time::StaticTimeSource"),
        "TestParamsSetterInterceptor" to testParamsSetterInterceptor(),
    )

    // TODO(enableNewSmithyRuntimeCleanup): Delete this once test helpers on `CustomizableOperation` have been removed
    private fun testParamsSetterInterceptor(): RuntimeType = RuntimeType.forInlineFun("TestParamsSetterInterceptor", ClientRustModule.Client.customize) {
        rustTemplate(
            """
            mod test_params_setter_interceptor {
                use aws_smithy_runtime_api::box_error::BoxError;
                use aws_smithy_runtime_api::client::interceptors::context::BeforeTransmitInterceptorContextMut;
                use aws_smithy_runtime_api::client::interceptors::Interceptor;
                use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents;
                use aws_smithy_types::config_bag::ConfigBag;
                use std::fmt;

                pub(super) struct TestParamsSetterInterceptor<F> { f: F }

                impl<F> fmt::Debug for TestParamsSetterInterceptor<F> {
                    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                        write!(f, "TestParamsSetterInterceptor")
                    }
                }

                impl<F> TestParamsSetterInterceptor<F> {
                    pub fn new(f: F) -> Self { Self { f } }
                }

                impl<F> Interceptor for TestParamsSetterInterceptor<F>
                where
                    F: Fn(&mut BeforeTransmitInterceptorContextMut<'_>, &mut ConfigBag) + Send + Sync + 'static,
                {
                    fn modify_before_signing(
                        &self,
                        context: &mut BeforeTransmitInterceptorContextMut<'_>,
                        _runtime_components: &RuntimeComponents,
                        cfg: &mut ConfigBag,
                    ) -> Result<(), BoxError> {
                        (self.f)(context, cfg);
                        Ok(())
                    }
                }
            }
            use test_params_setter_interceptor::TestParamsSetterInterceptor;
            """,
            *codegenScope,
        )
    }

    override fun section(section: CustomizableOperationSection): Writable =
        writable {
            if (section is CustomizableOperationSection.CustomizableOperationImpl) {
                if (section.isRuntimeModeOrchestrator) {
                    // TODO(enableNewSmithyRuntimeCleanup): Delete these utilities
                    rustTemplate(
                        """
                        ##[doc(hidden)]
@@ -49,7 +92,7 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) :
                                #{StaticRuntimePlugin}::new()
                                    .with_runtime_components(
                                        #{RuntimeComponentsBuilder}::new("request_time_for_tests")
                                            .with_time_source(Some(#{SharedTimeSource}::new(request_time)))
                                            .with_time_source(Some(#{SharedTimeSource}::new(#{StaticTimeSource}::new(request_time))))
                                    )
                            )
                        }
@@ -92,7 +135,9 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) :
                        ##[doc(hidden)]
                        // This is a temporary method for testing. NEVER use it in production
                        pub fn request_time_for_tests(mut self, request_time: ::std::time::SystemTime) -> Self {
                            self.operation.properties_mut().insert(#{SharedTimeSource}::new(request_time));
                            self.operation.properties_mut().insert(
                                #{SharedTimeSource}::new(#{StaticTimeSource}::new(request_time))
                            );
                            self
                        }

+3 −1
Original line number Diff line number Diff line
@@ -216,10 +216,12 @@ class AwsInputPresignedMethod(
                    """
                    // Change signature type to query params and wire up presigning config
                    let mut props = request.properties_mut();
                    props.insert(#{SharedTimeSource}::new(presigning_config.start_time()));
                    props.insert(#{SharedTimeSource}::new(#{StaticTimeSource}::new(presigning_config.start_time())));
                    """,
                    "SharedTimeSource" to RuntimeType.smithyAsync(runtimeConfig)
                        .resolve("time::SharedTimeSource"),
                    "StaticTimeSource" to RuntimeType.smithyAsync(runtimeConfig)
                        .resolve("time::StaticTimeSource"),
                )
                withBlock("props.insert(", ");") {
                    rustTemplate(
+3 −3
Original line number Diff line number Diff line
@@ -80,14 +80,14 @@ class IntegrationTestDependencies(
    override fun section(section: LibRsSection) = when (section) {
        is LibRsSection.Body -> testDependenciesOnly {
            if (hasTests) {
                val smithyClient = CargoDependency.smithyClient(codegenContext.runtimeConfig)
                    .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                val smithyAsync = CargoDependency.smithyAsync(codegenContext.runtimeConfig)
                    .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                val smithyClient = CargoDependency.smithyClient(codegenContext.runtimeConfig)
                    .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                val smithyTypes = CargoDependency.smithyTypes(codegenContext.runtimeConfig)
                    .copy(features = setOf("test-util"), scope = DependencyScope.Dev)
                addDependency(smithyClient)
                addDependency(smithyAsync)
                addDependency(smithyClient)
                addDependency(smithyTypes)
                addDependency(CargoDependency.smithyProtocolTestHelpers(codegenContext.runtimeConfig))
                addDependency(SerdeJson)
Loading