Skip to content
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.

Sign up for GitHub

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

fix: only enable aws-chunked content encoding for S3 #1224

Merged
merged 8 commits into from
Feb 19, 2024
Merged

fix: only enable aws-chunked content encoding for S3 #1224

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
73facdf
Add `EnableAwsChunked` signing attribute
lauzadis Feb 19, 2024
42813a5
Add `enableAwsChunked` client config
lauzadis Feb 19, 2024
7f7a7f9
Changelog
lauzadis Feb 19, 2024
02aee31
ktlint
lauzadis Feb 19, 2024
8e1803c
remove unnecessary trimIndent
lauzadis Feb 19, 2024
7630f53
add `enableAwsChunked` to S3 Control
lauzadis Feb 19, 2024
7ab1529
remove comment
lauzadis Feb 19, 2024
c992f0a
Update libs.versions.toml
lauzadis Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changes/d3a9b18c-7f36-4718-a855-91e3ac6f6531.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "d3a9b18c-7f36-4718-a855-91e3ac6f6531",
"type": "bugfix",
"description": "Only enable aws-chunked content encoding for S3",
"issues": [
"https://github.com/awslabs/aws-sdk-kotlin/issues/1217"
]
}
7 changes: 7 additions & 0 deletions ...odegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ class ClientConfigIntegration : KotlinIntegration {
Flag to disable [S3 multi-region access points](https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiRegionAccessPoints.html).
""".trimIndent()
}

val EnableAwsChunked: ConfigProperty = ConfigProperty {
name = "enableAwsChunked"
useSymbolWithNullableBuilder(KotlinTypes.Boolean, "true")
documentation = "Flag to enable [aws-chunked](https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html) content encoding."
}
}

override fun preprocessModel(model: Model, settings: KotlinSettings): Model {
Expand All @@ -86,6 +92,7 @@ class ClientConfigIntegration : KotlinIntegration {
ForcePathStyleProp,
UseArnRegionProp,
DisableMrapProp,
EnableAwsChunked,
)

override val sectionWriters: List<SectionWriterBinding>
Expand Down
3 changes: 2 additions & 1 deletion ...ws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/S3SigningConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import software.amazon.smithy.model.shapes.ServiceShape
* Set default signing attributes in the execution context (which ultimately becomes the signing context) for S3.
*/
class S3SigningConfig : KotlinIntegration {
// auth schemes are de-duped by taking the last one registered
// integrations are de-duped by taking the last one registered
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: is this comment now correct? I wasn't sure why auth schemes were relevant here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think this comment is correct for integrations. All of the registered integrations are "taken" and there is no possibility of duplication.

The comment was correct for auth schemes and I believe it was referring to the order value defined below, indicating that whatever auth scheme this integration registered should be the one that's taken because it'll be the last one (unless a user overrides it even later in client config). That said, I don't see this integration registering an auth scheme so I'm not sure how it's relevant.

override val order: Byte
get() = 127

Expand All @@ -40,5 +40,6 @@ class S3SigningConfig : KotlinIntegration {
writer.putIfAbsent(signingAttrs, "NormalizeUriPath", "false")
writer.putIfAbsent(signingAttrs, "UseDoubleUriEncode", "false")
writer.putIfAbsent(signingAttrs, "SignedBodyHeader", writer.format("#T.X_AMZ_CONTENT_SHA256", RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSignedBodyHeader))
writer.putIfAbsent(signingAttrs, "EnableAwsChunked", "config.enableAwsChunked")
}
}
8 changes: 8 additions & 0 deletions ...src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3control/ClientConfigIntegration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import aws.sdk.kotlin.codegen.customization.s3.isS3Control
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.core.CodegenContext
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
import software.amazon.smithy.kotlin.codegen.model.expectShape
import software.amazon.smithy.kotlin.codegen.rendering.util.ConfigProperty
import software.amazon.smithy.model.Model
Expand All @@ -32,6 +33,12 @@ class ClientConfigIntegration : KotlinIntegration {
[S3 access points](https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-points.html).
""".trimIndent(),
)

val EnableAwsChunked: ConfigProperty = ConfigProperty {
name = "enableAwsChunked"
useSymbolWithNullableBuilder(KotlinTypes.Boolean, "true")
documentation = "Flag to enable [aws-chunked](https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html) content encoding."
}
}

override fun preprocessModel(model: Model, settings: KotlinSettings): Model {
Expand All @@ -51,5 +58,6 @@ class ClientConfigIntegration : KotlinIntegration {
override fun additionalServiceConfigProps(ctx: CodegenContext): List<ConfigProperty> =
listOf(
UseArnRegionProp,
EnableAwsChunked,
)
}
Loading