Add customization to ensure S3 `Expires` field is always a `DateTime` (#3730)

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
At some point in the near future (after this customization is applied to
all existing SDKs) the S3 model will change the type of `Expires`
members to `String` from the current `Timestamp`. This change would
break backwards compatibility for us.

## Description
<!--- Describe your changes in detail -->
Add customization to ensure S3 `Expires` field is always a `Timestamp`
and ass a new synthetic member `ExpiresString` that persists the
un-parsed data from the `Expires` header.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Added tests to ensure that the model is pre-processed correctly. Added
integration tests for S3. Considered making this more generic codegen
tests, but since this customization will almost certainly only ever
apply to S3 and I wanted to ensure that it was properly applied to the
generated S3 SDK I opted for this route.

## 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 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._
This commit is contained in:
Landon James 2024-07-01 11:48:05 -07:00 committed by GitHub
parent 5c0baa7907
commit 9af72f5f2a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 423 additions and 1 deletions

View File

@ -23,3 +23,9 @@ Content-Type header validation now ignores parameter portion of media types.
references = ["smithy-rs#3471","smithy-rs#3724"]
meta = { "breaking" = false, "tada" = false, "bug" = true, target = "server" }
authors = ["djedward"]
[[aws-sdk-rust]]
message = "Add customizations for S3 Expires fields."
references = ["smithy-rs#3730"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "landonxjames"

View File

@ -56,6 +56,9 @@ pub mod endpoint_discovery;
// the `presigning_interceptors` module can refer to it.
mod serialization_settings;
/// Parse the Expires and ExpiresString fields correctly
pub mod s3_expires_interceptor;
// just so docs work
#[allow(dead_code)]
/// allow docs to work

View File

@ -0,0 +1,56 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
use aws_smithy_runtime_api::box_error::BoxError;
use aws_smithy_runtime_api::client::interceptors::context::BeforeDeserializationInterceptorContextMut;
use aws_smithy_runtime_api::client::interceptors::Intercept;
use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents;
use aws_smithy_types::config_bag::ConfigBag;
use aws_smithy_types::date_time::{DateTime, Format};
/// An interceptor to implement custom parsing logic for S3's `Expires` header. This
/// intercaptor copies the value of the `Expires` header to a (synthetically added)
/// `ExpiresString` header. It also attempts to parse the header as an `HttpDate`, if
/// that parsing fails the header is removed so the `Expires` field in the final output
/// will be `None`.
#[derive(Debug)]
pub(crate) struct S3ExpiresInterceptor;
const EXPIRES: &str = "Expires";
const EXPIRES_STRING: &str = "ExpiresString";
impl Intercept for S3ExpiresInterceptor {
fn name(&self) -> &'static str {
"S3ExpiresInterceptor"
}
fn modify_before_deserialization(
&self,
context: &mut BeforeDeserializationInterceptorContextMut<'_>,
_: &RuntimeComponents,
_: &mut ConfigBag,
) -> Result<(), BoxError> {
let headers = context.response_mut().headers_mut();
if headers.contains_key(EXPIRES) {
let expires_header = headers.get(EXPIRES).unwrap().to_string();
// If the Expires header fails to parse to an HttpDate we remove the header so
// it is parsed to None. We use HttpDate since that is the SEP defined default
// if no other format is specified in the model.
if DateTime::from_str(&expires_header, Format::HttpDate).is_err() {
tracing::debug!(
"Failed to parse the header `{EXPIRES}` = \"{expires_header}\" as an HttpDate. The raw string value can found in `{EXPIRES_STRING}`."
);
headers.remove(EXPIRES);
}
// Regardless of parsing success we copy the value of the Expires header to the
// ExpiresString header.
headers.insert(EXPIRES_STRING, expires_header);
}
Ok(())
}
}

View File

@ -21,6 +21,7 @@ import software.amazon.smithy.rustsdk.customize.lambda.LambdaDecorator
import software.amazon.smithy.rustsdk.customize.onlyApplyTo
import software.amazon.smithy.rustsdk.customize.route53.Route53Decorator
import software.amazon.smithy.rustsdk.customize.s3.S3Decorator
import software.amazon.smithy.rustsdk.customize.s3.S3ExpiresDecorator
import software.amazon.smithy.rustsdk.customize.s3.S3ExpressDecorator
import software.amazon.smithy.rustsdk.customize.s3.S3ExtendedRequestIdDecorator
import software.amazon.smithy.rustsdk.customize.s3control.S3ControlDecorator
@ -79,6 +80,7 @@ val DECORATORS: List<ClientCodegenDecorator> =
S3ExpressDecorator(),
S3ExtendedRequestIdDecorator(),
IsTruncatedPaginatorDecorator(),
S3ExpiresDecorator(),
),
S3ControlDecorator().onlyApplyTo("com.amazonaws.s3control#AWSS3ControlServiceV20180820"),
STSDecorator().onlyApplyTo("com.amazonaws.sts#AWSSecurityTokenServiceV20110615"),

View File

@ -0,0 +1,149 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package software.amazon.smithy.rustsdk.customize.s3
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeType
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.traits.DeprecatedTrait
import software.amazon.smithy.model.traits.DocumentationTrait
import software.amazon.smithy.model.traits.HttpHeaderTrait
import software.amazon.smithy.model.traits.OutputTrait
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustSettings
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
import software.amazon.smithy.rust.codegen.core.util.getTrait
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.outputShape
import software.amazon.smithy.rustsdk.InlineAwsDependency
import kotlin.streams.asSequence
/**
* Enforces that Expires fields have the DateTime type (since in the future the model will change to model them as String),
* and add an ExpiresString field to maintain the raw string value sent.
*/
class S3ExpiresDecorator : ClientCodegenDecorator {
override val name: String = "S3ExpiresDecorator"
override val order: Byte = 0
private val expires = "Expires"
private val expiresString = "ExpiresString"
override fun transformModel(
service: ServiceShape,
model: Model,
settings: ClientRustSettings,
): Model {
val transformer = ModelTransformer.create()
// Ensure all `Expires` shapes are timestamps
val expiresShapeTimestampMap =
model.shapes()
.asSequence()
.mapNotNull { shape ->
shape.members()
.singleOrNull { member -> member.memberName.equals(expires, ignoreCase = true) }
?.target
}
.associateWith { ShapeType.TIMESTAMP }
var transformedModel = transformer.changeShapeType(model, expiresShapeTimestampMap)
// Add an `ExpiresString` string shape to the model
val expiresStringShape = StringShape.builder().id("aws.sdk.rust.s3.synthetic#$expiresString").build()
transformedModel = transformedModel.toBuilder().addShape(expiresStringShape).build()
// For output shapes only, deprecate `Expires` and add a synthetic member that targets `ExpiresString`
transformedModel =
transformer.mapShapes(transformedModel) { shape ->
if (shape.hasTrait<OutputTrait>() && shape.memberNames.any { it.equals(expires, ignoreCase = true) }) {
val builder = (shape as StructureShape).toBuilder()
// Deprecate `Expires`
val expiresMember = shape.members().single { it.memberName.equals(expires, ignoreCase = true) }
builder.removeMember(expiresMember.memberName)
val deprecatedTrait =
DeprecatedTrait.builder()
.message("Please use `expires_string` which contains the raw, unparsed value of this field.")
.build()
builder.addMember(
expiresMember.toBuilder()
.addTrait(deprecatedTrait)
.build(),
)
// Add a synthetic member targeting `ExpiresString`
val expiresStringMember = MemberShape.builder()
expiresStringMember.target(expiresStringShape.id)
expiresStringMember.id(expiresMember.id.toString() + "String") // i.e. com.amazonaws.s3.<MEMBER_NAME>$ExpiresString
expiresStringMember.addTrait(HttpHeaderTrait(expiresString)) // Add HttpHeaderTrait to ensure the field is deserialized
expiresMember.getTrait<DocumentationTrait>()?.let {
expiresStringMember.addTrait(it) // Copy documentation from `Expires`
}
builder.addMember(expiresStringMember.build())
builder.build()
} else {
shape
}
}
return transformedModel
}
override fun operationCustomizations(
codegenContext: ClientCodegenContext,
operation: OperationShape,
baseCustomizations: List<OperationCustomization>,
): List<OperationCustomization> {
val outputShape = operation.outputShape(codegenContext.model)
if (outputShape.memberNames.any { it.equals(expires, ignoreCase = true) }) {
return baseCustomizations +
ParseExpiresFieldsCustomization(
codegenContext,
)
} else {
return baseCustomizations
}
}
}
class ParseExpiresFieldsCustomization(
private val codegenContext: ClientCodegenContext,
) : OperationCustomization() {
override fun section(section: OperationSection): Writable =
writable {
when (section) {
is OperationSection.AdditionalInterceptors -> {
section.registerInterceptor(codegenContext.runtimeConfig, this) {
val interceptor =
RuntimeType.forInlineDependency(
InlineAwsDependency.forRustFile("s3_expires_interceptor"),
).resolve("S3ExpiresInterceptor")
rustTemplate(
"""
#{S3ExpiresInterceptor}
""",
"S3ExpiresInterceptor" to interceptor,
)
}
}
else -> {}
}
}
}

View File

@ -0,0 +1,102 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
package software.amazon.smithy.rustsdk.customize.s3
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.DeprecatedTrait
import software.amazon.smithy.rust.codegen.client.testutil.testClientRustSettings
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.targetOrSelf
import kotlin.jvm.optionals.getOrNull
internal class S3ExpiresDecoratorTest {
private val baseModel =
"""
namespace smithy.example
use aws.protocols#restXml
use aws.auth#sigv4
use aws.api#service
@restXml
@sigv4(name: "s3")
@service(
sdkId: "S3"
arnNamespace: "s3"
)
service S3 {
version: "1.0.0",
operations: [GetFoo, NewGetFoo]
}
operation GetFoo {
input: GetFooInput
output: GetFooOutput
}
operation NewGetFoo {
input: GetFooInput
output: NewGetFooOutput
}
structure GetFooInput {
payload: String
expires: String
}
@output
structure GetFooOutput {
expires: Timestamp
}
@output
structure NewGetFooOutput {
expires: String
}
""".asSmithyModel()
private val serviceShape = baseModel.expectShape(ShapeId.from("smithy.example#S3"), ServiceShape::class.java)
private val settings = testClientRustSettings()
private val model = S3ExpiresDecorator().transformModel(serviceShape, baseModel, settings)
@Test
fun `Model is pre-processed correctly`() {
val expiresShapes =
listOf(
model.expectShape(ShapeId.from("smithy.example#GetFooInput\$expires")),
model.expectShape(ShapeId.from("smithy.example#GetFooOutput\$expires")),
model.expectShape(ShapeId.from("smithy.example#NewGetFooOutput\$expires")),
)
// Expires should always be Timestamp, even if not modeled as such since its
// type will change in the future
assertTrue(expiresShapes.all { it.targetOrSelf(model).isTimestampShape })
// All Expires output members should be marked with the deprecated trait
assertTrue(
expiresShapes
.filter { it.id.toString().contains("Output") }
.all { it.hasTrait<DeprecatedTrait>() },
)
// No ExpiresString member should be added to the input shape
assertNull(model.getShape(ShapeId.from("smithy.example#GetFooInput\$expiresString")).getOrNull())
val expiresStringOutputFields =
listOf(
model.expectShape(ShapeId.from("smithy.example#GetFooOutput\$expiresString")),
model.expectShape(ShapeId.from("smithy.example#NewGetFooOutput\$expiresString")),
)
// There should be a synthetic ExpiresString string member added to output shapes
assertTrue(expiresStringOutputFields.all { it.targetOrSelf(model).isStringShape })
// The synthetic fields should not be deprecated
assertTrue(expiresStringOutputFields.none { it.hasTrait<DeprecatedTrait>() })
}
}

View File

@ -0,0 +1,90 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
#![cfg(feature = "test-util")]
use aws_sdk_s3::{config::Region, Client, Config};
use aws_smithy_runtime::client::http::test_util::{ReplayEvent, StaticReplayClient};
use aws_smithy_types::body::SdkBody;
use aws_smithy_types::date_time::{DateTime, Format};
fn make_client(expires_val: &str) -> Client {
let http_client = StaticReplayClient::new(vec![ReplayEvent::new(
http::Request::builder()
.uri(http::Uri::from_static(
"https://some-test-bucket.s3.us-east-1.amazonaws.com/test.txt?attributes",
))
.body(SdkBody::empty())
.unwrap(),
http::Response::builder()
.header("Expires", expires_val)
.status(200)
.body(SdkBody::empty())
.unwrap(),
)]);
let config = Config::builder()
.region(Region::new("us-east-1"))
.http_client(http_client)
.with_test_defaults()
.build();
Client::from_conf(config)
}
#[allow(deprecated)]
#[tokio::test]
async fn expires_customization_works_with_non_date_value() {
let client = make_client("foo");
let out = client
.get_object()
.bucket("some-test-bucket")
.key("test.txt")
.send()
.await
.unwrap();
assert_eq!(out.expires, None);
assert_eq!(out.expires_string.unwrap(), "foo".to_string())
}
#[allow(deprecated)]
#[tokio::test]
async fn expires_customization_works_with_valid_date_format() {
let date = "Tue, 29 Apr 2014 18:30:38 GMT";
let date_time = DateTime::from_str(date, Format::HttpDate).unwrap();
let client = make_client(date);
let out = client
.get_object()
.bucket("some-test-bucket")
.key("test.txt")
.send()
.await
.unwrap();
assert_eq!(out.expires.unwrap(), date_time);
assert_eq!(out.expires_string.unwrap(), date);
}
#[allow(deprecated)]
#[tokio::test]
async fn expires_customization_works_with_non_http_date_format() {
let date = "1985-04-12T23:20:50.52Z";
let client = make_client(date);
let out = client
.get_object()
.bucket("some-test-bucket")
.key("test.txt")
.send()
.await
.unwrap();
assert_eq!(out.expires, None);
assert_eq!(out.expires_string.unwrap(), date);
}

View File

@ -100,7 +100,11 @@ fun ServiceShape.hasEventStreamOperations(model: Model): Boolean =
fun Shape.shouldRedact(model: Model): Boolean =
when (this) {
is MemberShape -> model.expectShape(this.target).shouldRedact(model) || model.expectShape(this.container).shouldRedact(model)
is MemberShape ->
model.expectShape(target).shouldRedact(model) ||
model.expectShape(container)
.shouldRedact(model)
else -> this.hasTrait<SensitiveTrait>()
}
@ -133,6 +137,16 @@ inline fun <reified T : Trait> UnionShape.findMemberWithTrait(model: Model): Mem
return this.members().find { it.getMemberTrait(model, T::class.java).isPresent }
}
/**
* If is member shape returns target, otherwise returns self.
* @param model for loading the target shape
*/
fun Shape.targetOrSelf(model: Model): Shape =
when (this) {
is MemberShape -> model.expectShape(this.target)
else -> this
}
/** Kotlin sugar for hasTrait() check. e.g. shape.hasTrait<EnumTrait>() instead of shape.hasTrait(EnumTrait::class.java) */
inline fun <reified T : Trait> Shape.hasTrait(): Boolean = hasTrait(T::class.java)