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: application of sigv4a #1186

Merged
merged 16 commits into from
Jan 24, 2024
Merged

fix: application of sigv4a #1186

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5464aa1
Fix sigv4a application
lauzadis Jan 22, 2024
c04e2d5
Simplify
lauzadis Jan 22, 2024
88a29b2
ktlint
lauzadis Jan 22, 2024
4b19660
changelog
lauzadis Jan 22, 2024
7acd860
Add a check for if SigV4ATrait is already applied
lauzadis Jan 22, 2024
fd3e47b
Merge branch 'main' of github.com:awslabs/aws-sdk-kotlin into fix-sigv4a
lauzadis Jan 23, 2024
17a5bbc
Add a special-case for CloudFront KVS
lauzadis Jan 23, 2024
4966aed
ktlint
lauzadis Jan 23, 2024
088f490
Merge branch 'main' of github.com:awslabs/aws-sdk-kotlin into fix-sigv4a
lauzadis Jan 23, 2024
10804da
Relocate file and preserve existing auth trait values
lauzadis Jan 23, 2024
9138074
Check first element
lauzadis Jan 23, 2024
dc4e09d
Add explanation of SigV4A being added to the end
lauzadis Jan 23, 2024
d60f45e
Remove "cloudfront keyvaluestore" from SigV4AsymmetricTraitCustomization
lauzadis Jan 23, 2024
5184124
Remove lonely comment
lauzadis Jan 23, 2024
17ab7d9
Add a check when applying the SigV4A trait
lauzadis Jan 23, 2024
7761766
ktlint
lauzadis Jan 23, 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
5 changes: 5 additions & 0 deletions .changes/517318e5-0ba6-472c-a51f-bb4e758215e2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "517318e5-0ba6-472c-a51f-bb4e758215e2",
"type": "bugfix",
"description": "Fix application of sigv4a authentication scheme for S3, Eventbridge, and CloudFront KeyValueStore"
}
34 changes: 20 additions & 14 deletions ...src/main/kotlin/aws/sdk/kotlin/codegen/customization/SigV4AsymmetricTraitCustomization.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import software.amazon.smithy.aws.traits.auth.SigV4ATrait
import software.amazon.smithy.aws.traits.auth.SigV4Trait
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.model.expectTrait
import software.amazon.smithy.kotlin.codegen.model.getTrait
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.traits.AuthTrait
Expand All @@ -33,19 +33,25 @@ class SigV4AsymmetricTraitCustomization : KotlinIntegration {
override fun preprocessModel(model: Model, settings: KotlinSettings): Model =
ModelTransformer.create().mapShapes(model) { shape ->
when (shape.isServiceShape) {
true ->
(shape as ServiceShape)
.toBuilder()
.addTraits(
mutableSetOf(
SigV4ATrait
.builder()
.name(shape.expectTrait<ServiceTrait>().arnNamespace)
.build(),
AuthTrait(mutableSetOf(SigV4ATrait.ID, SigV4Trait.ID)),
),
)
.build()
true -> {
val builder = (shape as ServiceShape).toBuilder()
builder.addTrait(
lauzadis marked this conversation as resolved.
Show resolved Hide resolved
SigV4ATrait.builder()
.name(shape.getTrait<SigV4Trait>()?.name ?: shape.getTrait<ServiceTrait>()?.arnNamespace)
.build(),
)

val authTrait = shape.getTrait<AuthTrait>()?.let {
if (it.valueSet.contains(SigV4ATrait.ID)) {
it
} else {
AuthTrait(it.valueSet + mutableSetOf(SigV4ATrait.ID))
}
} ?: AuthTrait(mutableSetOf(SigV4Trait.ID, SigV4ATrait.ID))
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: Probably want a remark in here about why this is ordered this way.

(It's because existing services (cloudfront KVS excluded) that use sigv4a do so through endpoint rules and it's not on the model. We don't know which operations it should apply for in those cases and so the safest thing to do is add it at the end and let endpoint rules change the priority as needed. Adding it this way registers support for the trait to let the rest of codegen Just Work (TM)).

builder.addTrait(authTrait)

builder.build()
}
false -> shape
}
}
Expand Down
7 changes: 4 additions & 3 deletions .../kotlin/aws/sdk/kotlin/codegen/customization/s3/UnsupportedSigningAlgorithmIntegration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import aws.sdk.kotlin.codegen.AwsRuntimeTypes
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.model.expectShape
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolMiddleware
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ServiceShape

// FIXME: Remove this once sigV4a is supported by default AWS signer
/**
Expand All @@ -22,7 +20,10 @@ import software.amazon.smithy.model.shapes.ServiceShape
*/
class UnsupportedSigningAlgorithmIntegration : KotlinIntegration {
override fun enabledForService(model: Model, settings: KotlinSettings): Boolean =
model.expectShape<ServiceShape>(settings.service).isS3
when (settings.sdkId.lowercase()) {
"s3", "eventbridge", "cloudfront keyvaluestore" -> true
else -> false
}

override fun customizeMiddleware(
ctx: ProtocolGenerator.GenerationContext,
Expand Down
Loading