-
Notifications
You must be signed in to change notification settings - Fork 50
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: overhaul endpoint discovery to favor endpointUrl when provided, …
…make endpoint discoverers interfaces so custom implementations can be provided, and to respect environmental config (#1506)
- Loading branch information
Showing
14 changed files
with
236 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "id": "49af01b8-6fed-4add-ace0-9f027e83425a", | ||
| "type": "feature", | ||
| "description": "⚠️ **IMPORTANT**: Refactor endpoint discoverer classes into interfaces so custom implementations may be provided", | ||
| "issues": [ | ||
| "awslabs/aws-sdk-kotlin#1413" | ||
| ], | ||
| "requiresMinorVersionBump": true | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "id": "929f0e2a-3af9-4f73-9f1b-b4e97f91f0db", | ||
| "type": "feature", | ||
| "description": "⚠️ **IMPORTANT**: Add support for enabling/disabling endpoint discovery via [standard cross-SDK config mechanisms](https://docs.aws.amazon.com/sdkref/latest/guide/feature-endpoint-discovery.html)", | ||
| "issues": [ | ||
| "awslabs/aws-sdk-kotlin#1413" | ||
| ], | ||
| "requiresMinorVersionBump": true | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "id": "e6515649-dab5-4be9-b4b4-b289369960d5", | ||
| "type": "bugfix", | ||
| "description": "Favor `endpointUrl` instead of endpoint discovery if both are provided", | ||
| "issues": [ | ||
| "awslabs/aws-sdk-kotlin#1413" | ||
| ] | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
...onfig/common/test/aws/sdk/kotlin/runtime/config/endpoints/ResolveEndpointDiscoveryTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package aws.sdk.kotlin.runtime.config.endpoints | ||
|
|
||
| import aws.sdk.kotlin.runtime.config.profile.* | ||
| import aws.sdk.kotlin.runtime.config.profile.FileType | ||
| import aws.sdk.kotlin.runtime.config.profile.parse | ||
| import aws.sdk.kotlin.runtime.config.profile.toSharedConfig | ||
| import aws.smithy.kotlin.runtime.telemetry.logging.Logger | ||
| 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 ResolveEndpointDiscoveryTest { | ||
| @Test | ||
| fun testPrecedenceSysProps() = assertEpDiscovery( | ||
| sysProps = mapOf("aws.endpointDiscoveryEnabled" to "true"), | ||
| env = mapOf("AWS_ENABLE_ENDPOINT_DISCOVERY" to "false"), | ||
| config = """ | ||
| [${Literals.DEFAULT_PROFILE}] | ||
| endpoint_discovery_enabled = false | ||
| """.trimIndent(), | ||
| serviceRequiresEpDiscovery = false, | ||
| expected = true, | ||
| ) | ||
|
|
||
| @Test | ||
| fun testPrecedenceEnvVars() = assertEpDiscovery( | ||
| env = mapOf("AWS_ENABLE_ENDPOINT_DISCOVERY" to "true"), | ||
| config = """ | ||
| [${Literals.DEFAULT_PROFILE}] | ||
| endpoint_discovery_enabled = false | ||
| """.trimIndent(), | ||
| serviceRequiresEpDiscovery = false, | ||
| expected = true, | ||
| ) | ||
|
|
||
| @Test | ||
| fun testPrecedenceConfig() = assertEpDiscovery( | ||
| config = """ | ||
| [${Literals.DEFAULT_PROFILE}] | ||
| endpoint_discovery_enabled = true | ||
| """.trimIndent(), | ||
| serviceRequiresEpDiscovery = false, | ||
| expected = true, | ||
| ) | ||
|
|
||
| @Test | ||
| fun testPrecedenceDefault() = assertEpDiscovery( | ||
| serviceRequiresEpDiscovery = true, | ||
| expected = true, | ||
| ) | ||
| } | ||
|
|
||
| fun assertEpDiscovery( | ||
| sysProps: Map<String, String> = mapOf(), | ||
| env: Map<String, String> = mapOf(), | ||
| config: String = "", | ||
| serviceRequiresEpDiscovery: Boolean, | ||
| expected: Boolean, | ||
| ) = runTest { | ||
| val provider = TestPlatformProvider(env, sysProps) | ||
| val source = AwsConfigurationSource(Literals.DEFAULT_PROFILE, "", "") | ||
|
|
||
| val profile = asyncLazy { | ||
| parse(Logger.None, FileType.CONFIGURATION, config) | ||
| .toSharedConfig(source) | ||
| .activeProfile | ||
| } | ||
|
|
||
| val actual = resolveEndpointDiscoveryEnabled(provider, profile, serviceRequiresEpDiscovery) | ||
| assertEquals(expected, actual) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
95 changes: 95 additions & 0 deletions
95
...degen/src/main/kotlin/aws/sdk/kotlin/codegen/endpoints/AwsEndpointDiscoveryIntegration.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package aws.sdk.kotlin.codegen.endpoints | ||
|
|
||
| import aws.sdk.kotlin.codegen.AwsRuntimeTypes | ||
| import aws.sdk.kotlin.codegen.ServiceClientCompanionObjectWriter | ||
| import software.amazon.smithy.kotlin.codegen.KotlinSettings | ||
| import software.amazon.smithy.kotlin.codegen.core.CodegenContext | ||
| import software.amazon.smithy.kotlin.codegen.core.KotlinDelegator | ||
| import software.amazon.smithy.kotlin.codegen.core.getContextValue | ||
| import software.amazon.smithy.kotlin.codegen.integration.AppendingSectionWriter | ||
| import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration | ||
| import software.amazon.smithy.kotlin.codegen.integration.SectionWriterBinding | ||
| import software.amazon.smithy.kotlin.codegen.model.asNullable | ||
| import software.amazon.smithy.kotlin.codegen.rendering.endpoints.discovery.DefaultEndpointDiscovererGenerator | ||
| import software.amazon.smithy.kotlin.codegen.rendering.endpoints.discovery.EndpointDiscovererInterfaceGenerator | ||
| import software.amazon.smithy.kotlin.codegen.rendering.endpoints.discovery.EndpointDiscoveryIntegration | ||
| import software.amazon.smithy.kotlin.codegen.rendering.util.ConfigProperty | ||
| import software.amazon.smithy.kotlin.codegen.rendering.util.ConfigPropertyType | ||
| import software.amazon.smithy.model.Model | ||
|
|
||
| class AwsEndpointDiscoveryIntegration : KotlinIntegration { | ||
| override val order: Byte = (EndpointDiscoveryIntegration.ORDER + 1).toByte() // after EndpointDiscoveryIntegration | ||
|
|
||
| override fun additionalServiceConfigProps(ctx: CodegenContext): List<ConfigProperty> { | ||
| val endpointDiscoveryOptional = EndpointDiscoveryIntegration.isOptionalFor(ctx) | ||
| val interfaceSymbol = EndpointDiscovererInterfaceGenerator.symbolFor(ctx.settings) | ||
| return listOf( | ||
| ConfigProperty { | ||
| name = EndpointDiscoveryIntegration.CLIENT_CONFIG_NAME | ||
| symbol = interfaceSymbol.asNullable() | ||
|
|
||
| if (endpointDiscoveryOptional) { | ||
| documentation = """ | ||
| The endpoint discoverer for this client, if applicable. By default, no endpoint discovery is | ||
| provided. To use endpoint discovery, set this to a valid [${interfaceSymbol.name}] instance. | ||
| """.trimIndent() | ||
| propertyType = ConfigPropertyType.SymbolDefault | ||
| } else { | ||
| val defaultImplSymbol = DefaultEndpointDiscovererGenerator.symbolFor(ctx.settings) | ||
|
|
||
| documentation = """ | ||
| The endpoint discoverer for this client, [${defaultImplSymbol.name}] by default. | ||
| """.trimIndent() | ||
| propertyType = ConfigPropertyType.Custom( | ||
| render = { prop, writer -> | ||
| writer.write( | ||
| "#1L val #2L: #3T = builder.#2L ?: #4T()", | ||
| ctx.settings.api.visibility, | ||
| prop.propertyName, | ||
| prop.symbol, | ||
| defaultImplSymbol, | ||
| ) | ||
| }, | ||
| ) | ||
| } | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| override fun enabledForService(model: Model, settings: KotlinSettings): Boolean = | ||
| EndpointDiscoveryIntegration.isEnabledFor(model, settings) | ||
|
|
||
| private val resolveEndpointDiscoverer = AppendingSectionWriter { writer -> | ||
| val ctx = writer.getContextValue(CodegenContext.Key) | ||
| val endpointDiscoveryOptional = EndpointDiscoveryIntegration.isOptionalFor(ctx) | ||
|
|
||
| writer.write( | ||
| "val epDiscoveryEnabled = #T(profile = activeProfile, serviceRequiresEpDiscovery = #L)", | ||
| AwsRuntimeTypes.Config.Endpoints.resolveEndpointDiscoveryEnabled, | ||
| !endpointDiscoveryOptional, | ||
| ) | ||
|
|
||
| writer.write( | ||
| "builder.config.#1L = builder.config.#1L ?: if (epDiscoveryEnabled) #2T() else null", | ||
| EndpointDiscoveryIntegration.CLIENT_CONFIG_NAME, | ||
| DefaultEndpointDiscovererGenerator.symbolFor(ctx.settings), | ||
| ) | ||
| } | ||
|
|
||
| override val sectionWriters = listOf( | ||
| SectionWriterBinding(ServiceClientCompanionObjectWriter.FinalizeEnvironmentalConfig, resolveEndpointDiscoverer), | ||
| ) | ||
|
|
||
| override fun writeAdditionalFiles(ctx: CodegenContext, delegator: KotlinDelegator) { | ||
| // EndpointDiscoveryIntegration already renders the default endpoint discoverer for services that _require_ EP | ||
| // discovery. So we only need to render it for services which _do not require_ EP discovery in order to support | ||
| // enabling discovery via environmental config. | ||
| if (EndpointDiscoveryIntegration.isOptionalFor(ctx)) { | ||
| DefaultEndpointDiscovererGenerator(ctx, delegator).render() | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters