From 4b3c78a5eadc27206b25cb1c8b72e7c5e42190cb Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 1 Nov 2023 10:53:05 -0400 Subject: [PATCH 1/2] fix: add forcePathStyle and enableAccelerate config properties back to the S3 client --- .../256a9c00-f658-4ee8-9e02-2cf46b76a900.json | 8 ++ .../s3/ClientConfigIntegration.kt | 22 +++- .../endpoints/BindAwsEndpointBuiltins.kt | 2 + .../services/s3/internal/FinalizeS3Config.kt | 15 ++- .../kotlin/services/s3/internal/S3Setting.kt | 24 +++++ .../s3/internal/FinalizeConfigTest.kt | 100 ++++++++++++++++++ 6 files changed, 164 insertions(+), 7 deletions(-) create mode 100644 .changes/256a9c00-f658-4ee8-9e02-2cf46b76a900.json create mode 100644 services/s3/common/src/aws/sdk/kotlin/services/s3/internal/S3Setting.kt create mode 100644 services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt diff --git a/.changes/256a9c00-f658-4ee8-9e02-2cf46b76a900.json b/.changes/256a9c00-f658-4ee8-9e02-2cf46b76a900.json new file mode 100644 index 00000000000..ee7d1ce44e8 --- /dev/null +++ b/.changes/256a9c00-f658-4ee8-9e02-2cf46b76a900.json @@ -0,0 +1,8 @@ +{ + "id": "256a9c00-f658-4ee8-9e02-2cf46b76a900", + "type": "bugfix", + "description": "Add `forcePathStyle` and `enableAccelerate` config properties back to the S3 client.", + "issues": [ + "awslabs/aws-sdk-kotlin#1098" + ] +} diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt index b04fa56f497..cbdc2e73aa9 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt @@ -28,6 +28,23 @@ class ClientConfigIntegration : KotlinIntegration { model.expectShape(settings.service).isS3 companion object { + val EnableAccelerateProp: ConfigProperty = ConfigProperty { + name = "enableAccelerate" + useSymbolWithNullableBuilder(KotlinTypes.Boolean, "false") + documentation = """ + Flag to support [S3 transfer acceleration](https://docs.aws.amazon.com/AmazonS3/latest/userguide/transfer-acceleration.html) + with this client. + """.trimIndent() + } + + val ForcePathStyleProp: ConfigProperty = ConfigProperty { + name = "forcePathStyle" + useSymbolWithNullableBuilder(KotlinTypes.Boolean, "false") + documentation = """ + Flag to use legacy path-style addressing when making requests. + """.trimIndent() + } + val UseArnRegionProp: ConfigProperty = ConfigProperty { name = "useArnRegion" useSymbolWithNullableBuilder(KotlinTypes.Boolean, "false") @@ -37,10 +54,9 @@ class ClientConfigIntegration : KotlinIntegration { """.trimIndent() } - // FIXME: default signer doesn't yet implement sigv4a, default to mrap OFF until it does val DisableMrapProp: ConfigProperty = ConfigProperty { name = "disableMrap" - useSymbolWithNullableBuilder(KotlinTypes.Boolean, "true") + useSymbolWithNullableBuilder(KotlinTypes.Boolean, "false") documentation = """ Flag to disable [S3 multi-region access points](https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiRegionAccessPoints.html). """.trimIndent() @@ -66,6 +82,8 @@ class ClientConfigIntegration : KotlinIntegration { override fun additionalServiceConfigProps(ctx: CodegenContext): List = listOf( + EnableAccelerateProp, + ForcePathStyleProp, UseArnRegionProp, DisableMrapProp, ) diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/endpoints/BindAwsEndpointBuiltins.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/endpoints/BindAwsEndpointBuiltins.kt index 0096cd59131..4344fefb4bd 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/endpoints/BindAwsEndpointBuiltins.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/endpoints/BindAwsEndpointBuiltins.kt @@ -50,6 +50,8 @@ fun renderBindAwsBuiltins(ctx: ProtocolGenerator.GenerationContext, writer: Kotl "AWS::UseFIPS" -> renderBasicConfigBinding(writer, it, AwsServiceConfigIntegration.UseFipsProp.propertyName) "AWS::UseDualStack" -> renderBasicConfigBinding(writer, it, AwsServiceConfigIntegration.UseDualStackProp.propertyName) + "AWS::S3::Accelerate" -> renderBasicConfigBinding(writer, it, S3ClientConfigIntegration.EnableAccelerateProp.propertyName) + "AWS::S3::ForcePathStyle" -> renderBasicConfigBinding(writer, it, S3ClientConfigIntegration.ForcePathStyleProp.propertyName) "AWS::S3::DisableMultiRegionAccessPoints" -> renderBasicConfigBinding(writer, it, S3ClientConfigIntegration.DisableMrapProp.propertyName) "AWS::S3::UseArnRegion" -> renderBasicConfigBinding(writer, it, S3ClientConfigIntegration.DisableMrapProp.propertyName) "AWS::S3Control::UseArnRegion" -> renderBasicConfigBinding(writer, it, S3ControlClientConfigIntegration.UseArnRegionProp.propertyName) diff --git a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/FinalizeS3Config.kt b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/FinalizeS3Config.kt index 2806118eaca..cf75cc2bafd 100644 --- a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/FinalizeS3Config.kt +++ b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/FinalizeS3Config.kt @@ -8,13 +8,18 @@ import aws.sdk.kotlin.runtime.config.profile.AwsProfile import aws.sdk.kotlin.runtime.config.profile.AwsSharedConfig import aws.sdk.kotlin.runtime.config.profile.getBooleanOrNull import aws.sdk.kotlin.services.s3.S3Client +import aws.smithy.kotlin.runtime.config.resolve import aws.smithy.kotlin.runtime.util.LazyAsyncValue +import aws.smithy.kotlin.runtime.util.PlatformProvider -internal suspend fun finalizeS3Config(builder: S3Client.Builder, sharedConfig: LazyAsyncValue) { - sharedConfig.get().activeProfile.let { - builder.config.useArnRegion = builder.config.useArnRegion ?: it.useArnRegion - builder.config.disableMrap = builder.config.disableMrap ?: it.disableMrap - } +internal suspend fun finalizeS3Config( + builder: S3Client.Builder, + sharedConfig: LazyAsyncValue, + provider: PlatformProvider = PlatformProvider.System, +) { + val activeProfile = sharedConfig.get().activeProfile + builder.config.useArnRegion = builder.config.useArnRegion ?: S3Setting.UseArnRegion.resolve(provider) ?: activeProfile.useArnRegion + builder.config.disableMrap = builder.config.disableMrap ?: S3Setting.DisableMultiRegionAccessPoints.resolve(provider) ?: activeProfile.disableMrap } private val AwsProfile.useArnRegion: Boolean? diff --git a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/S3Setting.kt b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/S3Setting.kt new file mode 100644 index 00000000000..bd90cd01200 --- /dev/null +++ b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/S3Setting.kt @@ -0,0 +1,24 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package aws.sdk.kotlin.services.s3.internal + +import aws.smithy.kotlin.runtime.config.* + +/** + * S3 specific system settings + */ +internal object S3Setting { + /** + * Configure whether the S3 client should uses the access point ARN AWS region to construct the regional endpoint + * for the request. See [Amazon S3 access points](https://docs.aws.amazon.com/sdkref/latest/guide/feature-s3-access-point.html) + */ + public val UseArnRegion: EnvironmentSetting = boolEnvSetting("aws.s3UseArnRegion", "AWS_S3_USE_ARN_REGION") + + /** + * Configure whether the S3 client potentially attempts cross-Region requests. + * See [Amazon S3 Multi-Region Access Points](https://docs.aws.amazon.com/sdkref/latest/guide/feature-s3-mrap.html) + */ + public val DisableMultiRegionAccessPoints: EnvironmentSetting = boolEnvSetting("aws.s3DisableMultiRegionAccessPoints", "AWS_S3_DISABLE_MULTIREGION_ACCESS_POINTS") +} diff --git a/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt b/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt new file mode 100644 index 00000000000..819af0b2edf --- /dev/null +++ b/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt @@ -0,0 +1,100 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package aws.sdk.kotlin.services.s3.internal + +import aws.sdk.kotlin.runtime.config.profile.loadAwsSharedConfig +import aws.sdk.kotlin.services.s3.S3Client +import aws.smithy.kotlin.runtime.util.TestPlatformProvider +import aws.smithy.kotlin.runtime.util.asyncLazy +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals + +class FinalizeConfigTest { + @Test + fun testExplicitConfigTakesPrecedence() = runTest { + val builder = S3Client.builder() + builder.config.useArnRegion = false + builder.config.disableMrap = true + val platform = TestPlatformProvider( + fs = mapOf( + "/users/test/.aws/config" to "[default]\ns3_use_arn_region = true\ns3_disable_multiregion_access_points = false", + ), + ) + val sharedConfig = asyncLazy { loadAwsSharedConfig(platform) } + + finalizeS3Config(builder, sharedConfig, platform) + assertEquals(false, builder.config.useArnRegion) + assertEquals(true, builder.config.disableMrap) + } + + @Test + fun testSystemProperties() = runTest { + val builder = S3Client.builder() + val platform = TestPlatformProvider( + env = mapOf( + S3Setting.UseArnRegion.envVar to "false", + S3Setting.DisableMultiRegionAccessPoints.envVar to "true", + ), + props = mapOf( + S3Setting.UseArnRegion.sysProp to "true", + S3Setting.DisableMultiRegionAccessPoints.sysProp to "false", + ), + fs = mapOf( + "/users/test/.aws/config" to "[default]\ns3_use_arn_region = false\ns3_disable_multiregion_access_points = true", + ), + ) + val sharedConfig = asyncLazy { loadAwsSharedConfig(platform) } + + finalizeS3Config(builder, sharedConfig, platform) + assertEquals(true, builder.config.useArnRegion) + assertEquals(false, builder.config.disableMrap) + } + + @Test + fun testEnvironmentVariables() = runTest { + val builder = S3Client.builder() + val platform = TestPlatformProvider( + env = mapOf( + S3Setting.UseArnRegion.envVar to "false", + S3Setting.DisableMultiRegionAccessPoints.envVar to "true", + ), + fs = mapOf( + "/users/test/.aws/config" to "[default]\ns3_use_arn_region = true\ns3_disable_multiregion_access_points = false", + ), + ) + val sharedConfig = asyncLazy { loadAwsSharedConfig(platform) } + + finalizeS3Config(builder, sharedConfig, platform) + assertEquals(false, builder.config.useArnRegion) + assertEquals(true, builder.config.disableMrap) + } + + @Test + fun testProfile() = runTest { + val builder = S3Client.builder() + val platform = TestPlatformProvider( + fs = mapOf( + "/users/test/.aws/config" to "[default]\ns3_use_arn_region = true\ns3_disable_multiregion_access_points = false", + ), + ) + val sharedConfig = asyncLazy { loadAwsSharedConfig(platform) } + + finalizeS3Config(builder, sharedConfig, platform) + assertEquals(true, builder.config.useArnRegion) + assertEquals(false, builder.config.disableMrap) + } + + @Test + fun testConfigPropertiesPresent() { + val builder = S3Client.builder() + builder.config.disableMrap = true + builder.config.useArnRegion = true + builder.config.forcePathStyle = true + builder.config.enableAccelerate = true + builder.config.useDualStack = true + builder.config.useFips = true + } +} From 946092dded1c59937471aa4d7502443d3a6fa0fe Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 1 Nov 2023 12:32:16 -0400 Subject: [PATCH 2/2] feedback --- .../kotlin/codegen/customization/s3/ClientConfigIntegration.kt | 3 ++- .../aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt index cbdc2e73aa9..b81781e24de 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/s3/ClientConfigIntegration.kt @@ -54,9 +54,10 @@ class ClientConfigIntegration : KotlinIntegration { """.trimIndent() } + // FIXME: default signer doesn't yet implement sigv4a, default to mrap OFF until it does val DisableMrapProp: ConfigProperty = ConfigProperty { name = "disableMrap" - useSymbolWithNullableBuilder(KotlinTypes.Boolean, "false") + useSymbolWithNullableBuilder(KotlinTypes.Boolean, "true") documentation = """ Flag to disable [S3 multi-region access points](https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiRegionAccessPoints.html). """.trimIndent() diff --git a/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt b/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt index 819af0b2edf..3040acfe527 100644 --- a/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt +++ b/services/s3/common/test/aws/sdk/kotlin/services/s3/internal/FinalizeConfigTest.kt @@ -89,6 +89,8 @@ class FinalizeConfigTest { @Test fun testConfigPropertiesPresent() { + // regression test to verify that these config properties are generated and can be set + // see https://github.com/awslabs/aws-sdk-kotlin/issues/1098 val builder = S3Client.builder() builder.config.disableMrap = true builder.config.useArnRegion = true