Skip to content

Commit

Permalink
Add the ability to disable interceptors via the config bag (#2757)
Browse files Browse the repository at this point in the history
This replaces the existing custom interceptor disable logic with shared
logic to allow generally disabling any public interceptors from Runtime
plugins. The previous behavior was very brittle because it relied
heavily on runtime plugin execution order.

## Motivation and Context
- simplify presigning behavior
- generalize logic to disable interceptors

## Description
Create `disable_interceptor` struct, which, when inserted into the
configuration bag can disable an interceptor via the `Interceptors`
execution interface.

## Testing
- ` (cd aws/sdk/build/aws-sdk/sdk/s3 && cargo test --test presigning)`

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] 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._
  • Loading branch information
rcoh authored Jun 8, 2023
1 parent ccbc9a1 commit dabbfaa
Show file tree
Hide file tree
Showing 9 changed files with 209 additions and 151 deletions.
17 changes: 9 additions & 8 deletions aws/rust-runtime/aws-inlineable/src/presigning_interceptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
use crate::presigning::PresigningConfig;
use crate::serialization_settings::HeaderSerializationSettings;
use aws_runtime::auth::sigv4::{HttpSignatureType, SigV4OperationSigningConfig};
use aws_runtime::invocation_id::DisableInvocationIdInterceptor;
use aws_runtime::request_info::DisableRequestInfoInterceptor;
use aws_runtime::user_agent::DisableUserAgentInterceptor;
use aws_runtime::invocation_id::InvocationIdInterceptor;
use aws_runtime::request_info::RequestInfoInterceptor;
use aws_runtime::user_agent::UserAgentInterceptor;
use aws_smithy_async::time::{SharedTimeSource, StaticTimeSource};
use aws_smithy_runtime_api::client::interceptors::{
BeforeSerializationInterceptorContextMut, BeforeTransmitInterceptorContextMut, BoxError,
Interceptor, InterceptorRegistrar, SharedInterceptor,
disable_interceptor, BeforeSerializationInterceptorContextMut,
BeforeTransmitInterceptorContextMut, BoxError, Interceptor, InterceptorRegistrar,
SharedInterceptor,
};
use aws_smithy_runtime_api::client::orchestrator::ConfigBagAccessors;
use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin;
Expand Down Expand Up @@ -92,9 +93,9 @@ impl RuntimePlugin for SigV4PresigningRuntimePlugin {
interceptors: &mut InterceptorRegistrar,
) -> Result<(), BoxError> {
// Disable some SDK interceptors that shouldn't run for presigning
cfg.put(DisableInvocationIdInterceptor::new("presigning"));
cfg.put(DisableRequestInfoInterceptor::new("presigning"));
cfg.put(DisableUserAgentInterceptor::new("presigning"));
cfg.put(disable_interceptor::<InvocationIdInterceptor>("presigning"));
cfg.put(disable_interceptor::<RequestInfoInterceptor>("presigning"));
cfg.put(disable_interceptor::<UserAgentInterceptor>("presigning"));

// Register the presigning interceptor
interceptors.register(self.interceptor.clone());
Expand Down
17 changes: 0 additions & 17 deletions aws/rust-runtime/aws-runtime/src/invocation_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,6 @@ pub use test_util::{NoInvocationIdGenerator, PredefinedInvocationIdGenerator};
#[allow(clippy::declare_interior_mutable_const)] // we will never mutate this
const AMZ_SDK_INVOCATION_ID: HeaderName = HeaderName::from_static("amz-sdk-invocation-id");

/// Config marker that disables the invocation ID interceptor.
#[doc(hidden)]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct DisableInvocationIdInterceptor {
why: &'static str,
}

impl DisableInvocationIdInterceptor {
/// Creates a new `DisableInvocationIdInterceptor`.
///
/// Takes a human readable string for the `Debug` impl to state why it is being disabled.
/// This is to assist with debugging issues with requests.
pub fn new(why: &'static str) -> Self {
Self { why }
}
}

/// A generator for returning new invocation IDs on demand.
pub trait InvocationIdGenerator: Debug + Send + Sync {
/// Call this function to receive a new [`InvocationId`] or an error explaining why one couldn't
Expand Down
17 changes: 0 additions & 17 deletions aws/rust-runtime/aws-runtime/src/request_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,6 @@ use std::time::{Duration, SystemTime};
#[allow(clippy::declare_interior_mutable_const)] // we will never mutate this
const AMZ_SDK_REQUEST: HeaderName = HeaderName::from_static("amz-sdk-request");

/// Config marker that disables the invocation ID interceptor.
#[doc(hidden)]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct DisableRequestInfoInterceptor {
why: &'static str,
}

impl DisableRequestInfoInterceptor {
/// Creates a new `DisableRequestInfoInterceptor`.
///
/// Takes a human readable string for the `Debug` impl to state why it is being disabled.
/// This is to assist with debugging issues with requests.
pub fn new(why: &'static str) -> Self {
Self { why }
}
}

/// Generates and attaches a request header that communicates request-related metadata.
/// Examples include:
///
Expand Down
17 changes: 0 additions & 17 deletions aws/rust-runtime/aws-runtime/src/user_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,6 @@ impl From<InvalidHeaderValue> for UserAgentInterceptorError {
}
}

/// Config marker that disables the user agent interceptor.
#[doc(hidden)]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct DisableUserAgentInterceptor {
why: &'static str,
}

impl DisableUserAgentInterceptor {
/// Creates a new `DisableUserAgentInterceptor`.
///
/// Takes a human readable string for the `Debug` impl to state why it is being disabled.
/// This is to assist with debugging issues with requests.
pub fn new(why: &'static str) -> Self {
Self { why }
}
}

/// Generates and attaches the AWS SDK's user agent to a HTTP request
#[non_exhaustive]
#[derive(Debug, Default)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegen
import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization
import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.util.letIf
Expand Down Expand Up @@ -38,14 +37,8 @@ private class InvocationIdRuntimePluginCustomization(

override fun section(section: ServiceRuntimePluginSection): Writable = writable {
if (section is ServiceRuntimePluginSection.AdditionalConfig) {
val invocationId = AwsRuntimeType.awsRuntime(codegenContext.runtimeConfig).resolve("invocation_id")
rustBlockTemplate(
"if cfg.get::<#{DisableInvocationIdInterceptor}>().is_none()",
"DisableInvocationIdInterceptor" to invocationId.resolve("DisableInvocationIdInterceptor"),
) {
section.registerInterceptor(codegenContext.runtimeConfig, this) {
rustTemplate("#{InvocationIdInterceptor}::new()", *codegenScope)
}
section.registerInterceptor(codegenContext.runtimeConfig, this) {
rustTemplate("#{InvocationIdInterceptor}::new()", *codegenScope)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRunti
import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.util.letIf
Expand All @@ -37,22 +36,17 @@ private class AddRetryInformationHeaderInterceptors(codegenContext: ClientCodege

override fun section(section: ServiceRuntimePluginSection): Writable = writable {
if (section is ServiceRuntimePluginSection.AdditionalConfig) {
rustBlockTemplate(
"if cfg.get::<#{DisableRequestInfoInterceptor}>().is_none()",
"DisableRequestInfoInterceptor" to awsRuntime.resolve("request_info::DisableRequestInfoInterceptor"),
) {
// Track the latency between client and server.
section.registerInterceptor(runtimeConfig, this) {
rust(
"#T::new()",
smithyRuntime.resolve("client::orchestrator::interceptors::ServiceClockSkewInterceptor"),
)
}
// Track the latency between client and server.
section.registerInterceptor(runtimeConfig, this) {
rust(
"#T::new()",
smithyRuntime.resolve("client::orchestrator::interceptors::ServiceClockSkewInterceptor"),
)
}

// Add request metadata to outgoing requests. Sets a header.
section.registerInterceptor(runtimeConfig, this) {
rust("#T::new()", awsRuntime.resolve("request_info::RequestInfoInterceptor"))
}
// Add request metadata to outgoing requests. Sets a header.
section.registerInterceptor(runtimeConfig, this) {
rust("#T::new()", awsRuntime.resolve("request_info::RequestInfoInterceptor"))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.config.Confi
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
Expand Down Expand Up @@ -105,16 +104,11 @@ class UserAgentDecorator : ClientCodegenDecorator {

override fun section(section: ServiceRuntimePluginSection): Writable = writable {
if (section is ServiceRuntimePluginSection.AdditionalConfig) {
rustBlockTemplate(
"if cfg.get::<#{DisableUserAgentInterceptor}>().is_none()",
"DisableUserAgentInterceptor" to awsRuntime.resolve("user_agent::DisableUserAgentInterceptor"),
) {
section.putConfigValue(this) {
rust("#T.clone()", ClientRustModule.Meta.toType().resolve("API_METADATA"))
}
section.registerInterceptor(runtimeConfig, this) {
rust("#T::new()", awsRuntime.resolve("user_agent::UserAgentInterceptor"))
}
section.putConfigValue(this) {
rust("#T.clone()", ClientRustModule.Meta.toType().resolve("API_METADATA"))
}
section.registerInterceptor(runtimeConfig, this) {
rust("#T::new()", awsRuntime.resolve("user_agent::UserAgentInterceptor"))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion design/src/client/orchestrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ The orchestrator's work is divided into four phases:
*NOTE: If an interceptor fails, then the other interceptors for that lifecycle event are still run. All resulting errors are collected and emitted together.*

0. **Building the `ConfigBag` and mounting interceptors**.
- *This phase is infallible.*
- *This phase is fallible.*
- An interceptor context is created. This will hold request and response objects, making them available to interceptors.
- All runtime plugins set at the client-level are run. These plugins can set config and mount interceptors. Any _"read before execution"_ interceptors that have been set get run.
- All runtime plugins set at the operation-level are run. These plugins can also set config and mount interceptors. Any new _"read before execution"_ interceptors that have been set get run.
Expand Down
Loading

0 comments on commit dabbfaa

Please sign in to comment.