-
Notifications
You must be signed in to change notification settings - Fork 196
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
Add operation customization for disabling payload signing #3915
Changes from all commits
1542758
41ace66
b2a0ee7
7300526
55822e1
02f1da6
ea72d21
ab4987f
1f8fe73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
--- | ||
applies_to: ["client"] | ||
authors: ["Velfi"] | ||
references: ["smithy-rs#3583"] | ||
breaking: false | ||
new_feature: true | ||
bug_fix: false | ||
--- | ||
|
||
It is now possible to disable payload signing through an operation customization. | ||
|
||
```rust | ||
async fn put_example_object(client: &aws_sdk_s3::Client) { | ||
let res = client | ||
.put_object() | ||
.bucket("test-bucket") | ||
.key("test-key") | ||
.body(ByteStream::from_static(b"Hello, world!")) | ||
.customize() | ||
// Setting this will disable payload signing. | ||
.disable_payload_signing() | ||
.send() | ||
.await; | ||
} | ||
``` | ||
|
||
Disabling payload signing will result in a small speedup at the cost of removing a data integrity check. | ||
However, this is an advanced feature and **may not be supported by all services/operations**. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,9 +7,9 @@ | |
|
||
//! Interceptor for handling Smithy `@httpChecksum` request checksumming with AWS SigV4 | ||
|
||
use aws_runtime::auth::PayloadSigningOverride; | ||
use aws_runtime::content_encoding::header_value::AWS_CHUNKED; | ||
use aws_runtime::content_encoding::{AwsChunkedBody, AwsChunkedBodyOptions}; | ||
use aws_runtime::{auth::SigV4OperationSigningConfig, content_encoding::header_value::AWS_CHUNKED}; | ||
use aws_sigv4::http_request::SignableBody; | ||
use aws_smithy_checksums::ChecksumAlgorithm; | ||
use aws_smithy_checksums::{body::calculate, http::HttpChecksum}; | ||
use aws_smithy_runtime_api::box_error::BoxError; | ||
|
@@ -190,11 +190,8 @@ fn add_checksum_for_request_body( | |
// Body is streaming: wrap the body so it will emit a checksum as a trailer. | ||
None => { | ||
tracing::debug!("applying {checksum_algorithm:?} of the request body as a trailer"); | ||
if let Some(mut signing_config) = cfg.load::<SigV4OperationSigningConfig>().cloned() { | ||
signing_config.signing_options.payload_override = | ||
Some(SignableBody::StreamingUnsignedPayloadTrailer); | ||
cfg.interceptor_state().store_put(signing_config); | ||
} | ||
cfg.interceptor_state() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this not need gated anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously this info was stored within something in the config bag. Now I don't need to load anything so I therefore don't need an |
||
.store_put(PayloadSigningOverride::StreamingUnsignedPayloadTrailer); | ||
wrap_streaming_request_body_in_checksum_calculating_body(request, checksum_algorithm)?; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
[package] | ||
name = "aws-runtime" | ||
version = "1.4.4" | ||
version = "1.5.0" | ||
authors = ["AWS Rust SDK Team <[email protected]>"] | ||
description = "Runtime support code for the AWS SDK. This crate isn't intended to be used directly." | ||
edition = "2021" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,7 @@ val DECORATORS: List<ClientCodegenDecorator> = | |
TokenProvidersDecorator(), | ||
ServiceEnvConfigDecorator(), | ||
HttpRequestCompressionDecorator(), | ||
DisablePayloadSigningDecorator(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. correctness: Should we only be applying this to S3 (and possibly only specific S3 operations)? I'm guessing most operations are going to require signing and won't work if the payload is unsigned but you'd have to test it. I also can't imagine it matters much for most operations to need unsigned so I'd rather not support this for more than we need to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm going to store the allowlist in the plugin itself. Currently, it only targets two S3 operations. |
||
// TODO(https://github.com/smithy-lang/smithy-rs/issues/3863): Comment in once the issue has been resolved | ||
// SmokeTestsDecorator(), | ||
), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rustsdk | ||
|
||
import software.amazon.smithy.model.shapes.OperationShape | ||
import software.amazon.smithy.model.shapes.ShapeId | ||
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext | ||
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator | ||
import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCustomization | ||
import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationSection | ||
import software.amazon.smithy.rust.codegen.core.rustlang.Writable | ||
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.RuntimeType.Companion.preludeScope | ||
|
||
internal val DISABLE_PAYLOAD_SIGNING_OPERATIONS by lazy { | ||
listOf( | ||
// S3 | ||
ShapeId.from("com.amazonaws.s3#PutObject"), | ||
ShapeId.from("com.amazonaws.s3#UploadPart"), | ||
) | ||
} | ||
|
||
class DisablePayloadSigningDecorator : ClientCodegenDecorator { | ||
override val name: String = "DisablePayloadSigning" | ||
override val order: Byte = 0 | ||
|
||
override fun operationCustomizations( | ||
codegenContext: ClientCodegenContext, | ||
operation: OperationShape, | ||
baseCustomizations: List<OperationCustomization>, | ||
): List<OperationCustomization> { | ||
return baseCustomizations + | ||
object : OperationCustomization() { | ||
private val runtimeConfig = codegenContext.runtimeConfig | ||
|
||
override fun section(section: OperationSection): Writable { | ||
return writable { | ||
when (section) { | ||
is OperationSection.CustomizableOperationImpl -> { | ||
if (DISABLE_PAYLOAD_SIGNING_OPERATIONS.contains(operation.id)) { | ||
rustTemplate( | ||
""" | ||
/// Disable payload signing for this request. | ||
/// | ||
/// **WARNING:** This is an advanced feature that removes | ||
/// the cost of signing a request payload by removing a data | ||
/// integrity check. Not all services/operations support | ||
/// this feature. | ||
pub fn disable_payload_signing(self) -> Self { | ||
self.runtime_plugin(#{PayloadSigningOverrideRuntimePlugin}::unsigned()) | ||
} | ||
""", | ||
*preludeScope, | ||
"PayloadSigningOverrideRuntimePlugin" to | ||
AwsRuntimeType.awsRuntime(runtimeConfig) | ||
.resolve("auth::PayloadSigningOverrideRuntimePlugin"), | ||
) | ||
} | ||
} | ||
|
||
else -> {} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: given code changes are mostly under
aws
, the release note should probably show up in aaws-sdk-rust
release. This should be either["client", "aws-sdk-rust"]
or just["aws-sdk-rust"]