Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
0marperez committed Dec 23, 2024
1 parent 4b61347 commit 756f66d
Show file tree
Hide file tree
Showing 22 changed files with 388 additions and 1,039 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import aws.smithy.kotlin.runtime.auth.awscredentials.SigV4aClientConfig
import aws.smithy.kotlin.runtime.client.*
import aws.smithy.kotlin.runtime.client.config.ClientSettings
import aws.smithy.kotlin.runtime.client.config.CompressionClientConfig
import aws.smithy.kotlin.runtime.client.config.HttpChecksumClientConfig
import aws.smithy.kotlin.runtime.client.config.HttpChecksumConfig
import aws.smithy.kotlin.runtime.config.resolve
import aws.smithy.kotlin.runtime.telemetry.TelemetryConfig
import aws.smithy.kotlin.runtime.telemetry.TelemetryProvider
Expand Down Expand Up @@ -97,7 +97,7 @@ public abstract class AbstractAwsSdkClientFactory<
config.sigV4aSigningRegionSet ?: resolveSigV4aSigningRegionSet(platform, profile)
}

if (config is HttpChecksumClientConfig.Builder) {
if (config is HttpChecksumConfig.Builder) {
config.requestChecksumCalculation =
config.requestChecksumCalculation ?: resolveRequestChecksumCalculation(platform, profile)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import aws.sdk.kotlin.runtime.config.AwsSdkSetting
import aws.sdk.kotlin.runtime.config.profile.AwsProfile
import aws.sdk.kotlin.runtime.config.profile.requestChecksumCalculation
import aws.sdk.kotlin.runtime.config.profile.responseChecksumValidation
import aws.smithy.kotlin.runtime.client.config.HttpChecksumConfigOption
import aws.smithy.kotlin.runtime.client.config.RequestHttpChecksumConfig
import aws.smithy.kotlin.runtime.client.config.ResponseHttpChecksumConfig
import aws.smithy.kotlin.runtime.config.resolve
import aws.smithy.kotlin.runtime.util.LazyAsyncValue
import aws.smithy.kotlin.runtime.util.PlatformProvider
Expand All @@ -16,27 +17,43 @@ import aws.smithy.kotlin.runtime.util.PlatformProvider
* @return requestChecksumCalculation setting if found, the default value if not.
*/
@InternalSdkApi
public suspend fun resolveRequestChecksumCalculation(platform: PlatformProvider = PlatformProvider.System, profile: LazyAsyncValue<AwsProfile>): HttpChecksumConfigOption {
public suspend fun resolveRequestChecksumCalculation(
platform: PlatformProvider = PlatformProvider.System,
profile: LazyAsyncValue<AwsProfile>,
): RequestHttpChecksumConfig {
val unparsedString = AwsSdkSetting.AwsRequestChecksumCalculation.resolve(platform) ?: profile.get().requestChecksumCalculation
return parseHttpChecksumConfigOption(unparsedString, "requestChecksumCalculation")
return parseRequestHttpChecksumConfig(unparsedString)
}

private fun parseRequestHttpChecksumConfig(unparsedString: String?): RequestHttpChecksumConfig =
when (unparsedString?.uppercase()) {
null -> RequestHttpChecksumConfig.WHEN_SUPPORTED
"WHEN_SUPPORTED" -> RequestHttpChecksumConfig.WHEN_SUPPORTED
"WHEN_REQUIRED" -> RequestHttpChecksumConfig.WHEN_REQUIRED
else -> throw ConfigurationException(
"'$unparsedString' is not a valid value for 'requestChecksumCalculation'. Valid values are: ${RequestHttpChecksumConfig.entries}",
)
}

/**
* Attempts to resolve responseChecksumValidation from the specified sources.
* @return responseChecksumValidation setting if found, the default value if not.
*/
@InternalSdkApi
public suspend fun resolveResponseChecksumValidation(platform: PlatformProvider = PlatformProvider.System, profile: LazyAsyncValue<AwsProfile>): HttpChecksumConfigOption {
public suspend fun resolveResponseChecksumValidation(
platform: PlatformProvider = PlatformProvider.System,
profile: LazyAsyncValue<AwsProfile>,
): ResponseHttpChecksumConfig {
val unparsedString = AwsSdkSetting.AwsResponseChecksumValidation.resolve(platform) ?: profile.get().responseChecksumValidation
return parseHttpChecksumConfigOption(unparsedString, "responseChecksumValidation")
return parseResponseHttpChecksumConfig(unparsedString)
}

private fun parseHttpChecksumConfigOption(unparsedString: String?, configOption: String): HttpChecksumConfigOption =
private fun parseResponseHttpChecksumConfig(unparsedString: String?): ResponseHttpChecksumConfig =
when (unparsedString?.uppercase()) {
null -> HttpChecksumConfigOption.WHEN_SUPPORTED
"WHEN_SUPPORTED" -> HttpChecksumConfigOption.WHEN_SUPPORTED
"WHEN_REQUIRED" -> HttpChecksumConfigOption.WHEN_REQUIRED
null -> ResponseHttpChecksumConfig.WHEN_SUPPORTED
"WHEN_SUPPORTED" -> ResponseHttpChecksumConfig.WHEN_SUPPORTED
"WHEN_REQUIRED" -> ResponseHttpChecksumConfig.WHEN_REQUIRED
else -> throw ConfigurationException(
"'$unparsedString' is not a valid value for $configOption. Valid values are: ${HttpChecksumConfigOption.entries}",
"'$unparsedString' is not a valid value for 'responseChecksumValidation'. Valid values are: ${ResponseHttpChecksumConfig.entries}",
)
}
4 changes: 2 additions & 2 deletions aws-runtime/aws-http/api/aws-http.api
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ public final class aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInter
public fun readBeforeTransmit (Laws/smithy/kotlin/runtime/client/ProtocolRequestInterceptorContext;)V
}

public final class aws/sdk/kotlin/runtime/http/interceptors/S3FlexibleChecksumResponseInterceptor : aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptor {
public fun <init> (ZLaws/smithy/kotlin/runtime/client/config/HttpChecksumConfigOption;)V
public final class aws/sdk/kotlin/runtime/http/interceptors/IgnoreCompositeFlexibleChecksumResponseInterceptor : aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptor {
public fun <init> (ZLaws/smithy/kotlin/runtime/client/config/ResponseHttpChecksumConfig;)V
public fun ignoreChecksum (Ljava/lang/String;)Z
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package aws.sdk.kotlin.runtime.http.interceptors

import aws.smithy.kotlin.runtime.client.config.HttpChecksumConfigOption
import aws.smithy.kotlin.runtime.client.config.ResponseHttpChecksumConfig
import aws.smithy.kotlin.runtime.http.interceptors.FlexibleChecksumsResponseInterceptor

/**
* S3 variant of the flexible checksum interceptor where composite checksums are not validated
* Variant of the [FlexibleChecksumsResponseInterceptor] where composite checksums are not validated
*/
public class S3FlexibleChecksumResponseInterceptor(
public class IgnoreCompositeFlexibleChecksumResponseInterceptor(
responseValidationRequired: Boolean,
responseChecksumValidation: HttpChecksumConfigOption?,
responseChecksumValidation: ResponseHttpChecksumConfig?,
) : FlexibleChecksumsResponseInterceptor(
responseValidationRequired,
responseChecksumValidation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ data class CodegenTest(
val name: String,
val model: Model,
val serviceShapeId: String,
val protocolName: String? = null,
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ object AwsRuntimeTypes {
val UnsupportedSigningAlgorithmInterceptor = symbol("UnsupportedSigningAlgorithmInterceptor")
val BusinessMetricsInterceptor = symbol("BusinessMetricsInterceptor")
val AwsBusinessMetric = symbol("AwsBusinessMetric")
val S3FlexibleChecksumResponseInterceptor = symbol("S3FlexibleChecksumResponseInterceptor")
val IgnoreCompositeFlexibleChecksumResponseInterceptor = symbol("IgnoreCompositeFlexibleChecksumResponseInterceptor")
}

object Retries {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package aws.sdk.kotlin.codegen

import aws.sdk.kotlin.codegen.model.traits.Presignable
import aws.smithy.kotlin.runtime.hashing.algorithmsSupportedForFlexibleChecksums
import software.amazon.smithy.aws.traits.HttpChecksumTrait
import software.amazon.smithy.aws.traits.auth.SigV4Trait
import software.amazon.smithy.aws.traits.protocols.AwsQueryTrait
Expand All @@ -14,10 +13,10 @@ import software.amazon.smithy.kotlin.codegen.core.*
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Hashing.isSupportedForFlexibleChecksums
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Hashing.toHashFunctionOrThrow
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Text.Encoding.encodeBase64String
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Utils.runBlocking
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Http.HttpBody
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Http.readAll
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.KotlinCoroutines.coroutineContext
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.KotlinxCoroutines.runBlocking
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Observability.TelemetryApi.warn
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.integration.SectionId
Expand Down Expand Up @@ -304,8 +303,7 @@ class PresignerGenerator : KotlinIntegration {
"#T.#T<Presigner> { #S }",
coroutineContext,
warn,
"The requested checksum algorithm (\${checksumAlgorithmName}) is not supported for pre-signed URL checksums, sending request without checksum. " +
"Supported checksums for pre-signed URLs: $algorithmsSupportedForFlexibleChecksums",
"The requested checksum algorithm (\${checksumAlgorithmName}) is not supported for pre-signed URL checksums, sending request without checksum.",
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class FlexibleChecksumsRequest : KotlinIntegration {
// Allows flexible checksum request configuration
ConfigProperty {
name = "requestChecksumCalculation"
symbol = RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption
baseClass = RuntimeTypes.SmithyClient.Config.HttpChecksumClientConfig
symbol = RuntimeTypes.SmithyClient.Config.RequestHttpChecksumConfig
baseClass = RuntimeTypes.SmithyClient.Config.HttpChecksumConfig
useNestedBuilderBaseClass()
documentation = "Configures request checksum calculation"
propertyType = ConfigPropertyType.RequiredWithDefault("HttpChecksumConfigOption.WHEN_SUPPORTED")
propertyType = ConfigPropertyType.RequiredWithDefault("RequestHttpChecksumConfig.WHEN_SUPPORTED")
},
)

Expand All @@ -59,14 +59,14 @@ private val requestChecksumCalculationBusinessMetric = object : ProtocolMiddlewa
// Supported
writer.write(
"#T.WHEN_SUPPORTED -> op.context.#T(#T.FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED)",
RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption,
RuntimeTypes.SmithyClient.Config.RequestHttpChecksumConfig,
RuntimeTypes.Core.BusinessMetrics.emitBusinessMetric,
RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric,
)
// Required
writer.write(
"#T.WHEN_REQUIRED -> op.context.#T(#T.FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED)",
RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption,
RuntimeTypes.SmithyClient.Config.RequestHttpChecksumConfig,
RuntimeTypes.Core.BusinessMetrics.emitBusinessMetric,
RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric,
)
Expand All @@ -82,7 +82,7 @@ private val httpChecksumDefaultAlgorithmMiddleware = object : ProtocolMiddleware
override val order: Byte = -2 // Before S3 Express (possibly) changes the default (-1) and before calculating checksum (0)

override fun isEnabledFor(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Boolean =
requestChecksumsConfigured(ctx, op)
op.hasRequestAlgorithmMember(ctx)

override fun render(ctx: ProtocolGenerator.GenerationContext, op: OperationShape, writer: KotlinWriter) {
writer.write(
Expand All @@ -100,7 +100,7 @@ private val flexibleChecksumsRequestMiddleware = object : ProtocolMiddleware {
override val name: String = "flexibleChecksumsRequestMiddleware"

override fun isEnabledFor(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Boolean =
requestChecksumsConfigured(ctx, op)
op.hasRequestAlgorithmMember(ctx)

override fun render(ctx: ProtocolGenerator.GenerationContext, op: OperationShape, writer: KotlinWriter) {
val httpChecksumTrait = op.getTrait<HttpChecksumTrait>()!!
Expand All @@ -125,9 +125,9 @@ private val flexibleChecksumsRequestMiddleware = object : ProtocolMiddleware {
/**
* Determines if an operation is set up to send flexible request checksums
*/
private fun requestChecksumsConfigured(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Boolean {
val httpChecksumTrait = op.getTrait<HttpChecksumTrait>()
val inputShape = op.input.getOrNull()?.let { ctx.model.expectShape<StructureShape>(it) }
private fun OperationShape.hasRequestAlgorithmMember(ctx: ProtocolGenerator.GenerationContext): Boolean {
val httpChecksumTrait = this.getTrait<HttpChecksumTrait>()
val inputShape = this.input.getOrNull()?.let { ctx.model.expectShape<StructureShape>(it) }

return (
(httpChecksumTrait != null) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class FlexibleChecksumsResponse : KotlinIntegration {
// Allows flexible checksum response configuration
ConfigProperty {
name = "responseChecksumValidation"
symbol = RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption
baseClass = RuntimeTypes.SmithyClient.Config.HttpChecksumClientConfig
symbol = RuntimeTypes.SmithyClient.Config.ResponseHttpChecksumConfig
baseClass = RuntimeTypes.SmithyClient.Config.HttpChecksumConfig
useNestedBuilderBaseClass()
documentation = "Configures response checksum validation"
propertyType = ConfigPropertyType.RequiredWithDefault("HttpChecksumConfigOption.WHEN_SUPPORTED")
propertyType = ConfigPropertyType.RequiredWithDefault("ResponseHttpChecksumConfig.WHEN_SUPPORTED")
},
)

Expand All @@ -56,14 +56,14 @@ private val responseChecksumValidationBusinessMetric = object : ProtocolMiddlewa
// Supported
writer.write(
"#T.WHEN_SUPPORTED -> op.context.#T(#T.FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED)",
RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption,
RuntimeTypes.SmithyClient.Config.ResponseHttpChecksumConfig,
RuntimeTypes.Core.BusinessMetrics.emitBusinessMetric,
RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric,
)
// Required
writer.write(
"#T.WHEN_REQUIRED -> op.context.#T(#T.FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED)",
RuntimeTypes.SmithyClient.Config.HttpChecksumConfigOption,
RuntimeTypes.SmithyClient.Config.ResponseHttpChecksumConfig,
RuntimeTypes.Core.BusinessMetrics.emitBusinessMetric,
RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric,
)
Expand Down Expand Up @@ -94,7 +94,8 @@ private val flexibleChecksumsResponseMiddleware = object : ProtocolMiddleware {
val requestValidationModeMemberName = ctx.symbolProvider.toMemberName(requestValidationModeMember)

val interceptor = if (ctx.model.expectShape<ServiceShape>(ctx.settings.service).isS3) {
AwsRuntimeTypes.Http.Interceptors.S3FlexibleChecksumResponseInterceptor
// S3 needs a custom interceptor because it can send composite checksums, which should be ignored
AwsRuntimeTypes.Http.Interceptors.IgnoreCompositeFlexibleChecksumResponseInterceptor
} else {
RuntimeTypes.HttpClient.Interceptors.FlexibleChecksumsResponseInterceptor
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal class S3ExpressDefaultChecksumAlgorithm(
private val isS3UploadPart: Boolean,
) : HttpInterceptor {
override suspend fun modifyBeforeSigning(context: ProtocolRequestInterceptorContext<Any, HttpRequest>): HttpRequest {
if (usingS3Express(context.executionContext)) {
if (context.executionContext.usingS3Express()) {
if (isS3UploadPart) {
context.executionContext.remove(HttpOperationContext.DefaultChecksumAlgorithm)
} else {
Expand All @@ -33,5 +33,5 @@ internal class S3ExpressDefaultChecksumAlgorithm(
}
}

private fun usingS3Express(executionContext: ExecutionContext): Boolean =
executionContext.getOrNull(AttributeKey(S3_EXPRESS_ENDPOINT_PROPERTY_KEY)) != S3_EXPRESS_ENDPOINT_PROPERTY_VALUE
private fun ExecutionContext.usingS3Express(): Boolean =
this.getOrNull(AttributeKey(S3_EXPRESS_ENDPOINT_PROPERTY_KEY)) != S3_EXPRESS_ENDPOINT_PROPERTY_VALUE

This file was deleted.

1 change: 1 addition & 0 deletions tests/codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ subprojects {
implementation(libraries.smithy.kotlin.smithy.test)
implementation(libraries.smithy.kotlin.aws.signing.default)
implementation(libraries.smithy.kotlin.telemetry.api)
implementation(libraries.smithy.kotlin.http.test)
}
}
jvmTest {
Expand Down
Loading

0 comments on commit 756f66d

Please sign in to comment.