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.

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

feat: enable auth resolution via endpoints #1109

Merged
merged 11 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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/38a790e8-0d9a-4dfb-9a8d-05fba0325f1e.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "38a790e8-0d9a-4dfb-9a8d-05fba0325f1e",
"type": "feature",
"description": "Enable auth scheme resolution via endpoints for S3 and EventBridge"
}
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ buildscript {
classpath(libs.kotlinx.atomicfu.plugin)
classpath("aws.sdk.kotlin:build-plugins") {
version {
require("0.2.8")
require("0.2.9")
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions codegen/sdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import aws.sdk.kotlin.gradle.codegen.dsl.projectionRootDir
import aws.sdk.kotlin.gradle.codegen.dsl.smithyKotlinPlugin
import software.amazon.smithy.gradle.tasks.SmithyBuild
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.shapes.ServiceShape
import java.nio.file.Paths
import java.util.*
Expand Down Expand Up @@ -114,6 +115,20 @@ fun awsServiceProjections(): Provider<List<SmithyProjection>> {
imports = importPaths
transforms = transformsForService(service) ?: emptyList()

val packageSettingsFile = file(service.packageSettings)
val packageSettings = if (packageSettingsFile.exists()) {
val node = Node.parse(packageSettingsFile.inputStream())
node.asObjectNode().get()
} else {
Node.objectNode()
}

if (!packageSettings.isEmpty) {
packageSettings.expectMember("sdkId", "${packageSettingsFile.absolutePath} does not contain member `sdkId`")
val packageSdkId = packageSettings.getStringMember("sdkId").get().value
check(service.sdkId == packageSdkId) { "${packageSettingsFile.absolutePath} `sdkId` ($packageSdkId) does not match expected `${service.sdkId}`" }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: I'm a little confused by this logic. It looks like if the file doesn't exist or contains only {} then we won't assert the existence of an sdkId...why is that? Shouldn't it be an error if the file exists and is malformed?

val packageSettingsFile = file(service.packageSettings)
if (packageSettingsFile.exists()) {
    val node = Node.parse(packageSettingsFile.inputStream()).asObjectNode().get()
    node.expectMember("sdkId", "${packageSettingsFile.absolutePath} does not contain member `sdkId`")
    val packageSdkId = node.getStringMember("sdkId").get().value
    check(service.sdkId == packageSdkId) { "${packageSettingsFile.absolutePath} `sdkId` ($packageSdkId) does not match expected `${service.sdkId}`" }
}


smithyKotlinPlugin {
serviceShapeId = service.serviceShapeId
packageName = service.packageName
Expand All @@ -124,6 +139,9 @@ fun awsServiceProjections(): Provider<List<SmithyProjection>> {
generateFullProject = false
generateDefaultBuildFiles = false
}
apiSettings {
enableEndpointAuthProvider = packageSettings.getBooleanMember("enableEndpointAuthProvider").orNull()?.value
}
}
}
}
Expand Down Expand Up @@ -286,6 +304,12 @@ val AwsService.modelExtrasDir: String
val AwsService.transformsDir: String
get() = rootProject.file("$destinationDir/transforms").absolutePath

/**
* Service specific package settings
*/
val AwsService.packageSettings: String
get() = rootProject.file("$destinationDir/package.json").absolutePath
Copy link
Member

Choose a reason for hiding this comment

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

question/nit: Is package.json already an established name for SDK client codegen configuration? I think something like config.json makes more sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No established name, I don't have a strong preference. I think I went with package.json because that is what was proposed in the publishing QA doc. @ianbotsf do you have any preference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a preference on the name—either package.json or config.json sounds fine to me.

Taking a closer look at this now, does Smithy not already have a way to handle this? If not, should we consider moving this kind of functionality up into the smithy-kotlin later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle what? This is custom logic to feed into smithy-build.json.


fun forwardProperty(name: String) {
getProperty(name)?.let {
System.setProperty(name, it)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package aws.sdk.kotlin.codegen

import aws.sdk.kotlin.codegen.protocols.endpoints.*
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes
import software.amazon.smithy.kotlin.codegen.core.useFileWriter
import software.amazon.smithy.kotlin.codegen.model.buildSymbol
import software.amazon.smithy.kotlin.codegen.rendering.endpoints.*
Expand Down Expand Up @@ -66,13 +65,7 @@ class AwsEndpointDelegator : EndpointDelegator {

override fun generateEndpointResolverAdapter(ctx: ProtocolGenerator.GenerationContext) {
ctx.delegator.useFileWriter(EndpointResolverAdapterGenerator.getSymbol(ctx.settings)) {
EndpointResolverAdapterGenerator(ctx, it) {
it.write(
"endpoint.#T?.#T(request.context)",
RuntimeTypes.SmithyClient.Endpoints.signingContext,
RuntimeTypes.Auth.Signing.AwsSigningCommon.mergeInto,
)
}.render()
EndpointResolverAdapterGenerator(ctx, it).render()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,19 @@

package aws.sdk.kotlin.codegen.customization.s3

import aws.sdk.kotlin.codegen.protocols.core.AwsHttpProtocolClientGenerator
import aws.sdk.kotlin.codegen.protocols.core.putIfAbsent
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes
import software.amazon.smithy.kotlin.codegen.core.withBlock
import software.amazon.smithy.kotlin.codegen.integration.AuthSchemeHandler
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
import software.amazon.smithy.kotlin.codegen.integration.SectionWriterBinding
import software.amazon.smithy.kotlin.codegen.model.expectShape
import software.amazon.smithy.kotlin.codegen.model.knowledge.AwsSignatureVersion4
import software.amazon.smithy.kotlin.codegen.rendering.auth.SigV4AuthSchemeHandler
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape

/**
* Overrides the SigV4 auth scheme registered by [software.amazon.smithy.kotlin.codegen.rendering.auth.Sigv4AuthSchemeIntegration] for S3.
* 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
Expand All @@ -29,22 +27,14 @@ class S3SigningConfig : KotlinIntegration {
override fun enabledForService(model: Model, settings: KotlinSettings) =
model.expectShape<ServiceShape>(settings.service).isS3

override fun authSchemes(ctx: ProtocolGenerator.GenerationContext): List<AuthSchemeHandler> =
listOf(S3AuthSchemeHandler())
}

private class S3AuthSchemeHandler : SigV4AuthSchemeHandler() {
override fun instantiateAuthSchemeExpr(ctx: ProtocolGenerator.GenerationContext, writer: KotlinWriter) {
val signingService = AwsSignatureVersion4.signingServiceName(ctx.service)
writer.withBlock("#T(", ")", RuntimeTypes.Auth.HttpAuthAws.SigV4AuthScheme) {
withBlock("#T.Config().apply {", "}", RuntimeTypes.Auth.HttpAuthAws.AwsHttpSigner) {
write("signer = #T", RuntimeTypes.Auth.Signing.AwsSigningStandard.DefaultAwsSigner)
write("service = #S", signingService)
write("signedBodyHeader = #T.X_AMZ_CONTENT_SHA256", RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSignedBodyHeader)
// https://github.com/awslabs/aws-sdk-kotlin/issues/200
writer.write("useDoubleUriEncode = false")
writer.write("normalizeUriPath = false")
}
}
override val sectionWriters: List<SectionWriterBinding> = listOf(
SectionWriterBinding(AwsHttpProtocolClientGenerator.MergeServiceDefaults, ::renderDefaultSigningContext),
)
private fun renderDefaultSigningContext(writer: KotlinWriter, previousValue: String?) {
val signingAttrs = RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSigningAttributes
// https://github.com/awslabs/aws-sdk-kotlin/issues/200
Copy link
Member

Choose a reason for hiding this comment

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

question: this issue is closed, is it still relevant?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is here to remind us why path normalization and single URI encoding are important for S3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I left it as a clue for posterity about why we care about this and where the requirements come from.

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))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.kotlin.codegen.core.*
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSigningAttributes
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.SmithyClient.SdkClientOption
import software.amazon.smithy.kotlin.codegen.integration.SectionId
import software.amazon.smithy.kotlin.codegen.integration.SectionKey
import software.amazon.smithy.kotlin.codegen.integration.SectionWriter
import software.amazon.smithy.kotlin.codegen.model.hasIdempotentTokenMember
import software.amazon.smithy.kotlin.codegen.model.knowledge.AwsSignatureVersion4
Expand All @@ -31,6 +33,9 @@ open class AwsHttpProtocolClientGenerator(
middlewares: List<ProtocolMiddleware>,
httpBindingResolver: HttpBindingResolver,
) : HttpProtocolClientGenerator(ctx, middlewares, httpBindingResolver) {
object MergeServiceDefaults : SectionId {
val GenerationContext: SectionKey<ProtocolGenerator.GenerationContext> = SectionKey("GenerationContext")
}

override fun render(writer: KotlinWriter) {
writer.write("\n\n")
Expand Down Expand Up @@ -109,11 +114,13 @@ open class AwsHttpProtocolClientGenerator(
if (ctx.service.hasIdempotentTokenMember(ctx.model)) {
putIfAbsent(SdkClientOption, "IdempotencyTokenProvider", nullable = true)
}

writer.declareSection(MergeServiceDefaults)
}
}
}

private fun KotlinWriter.putIfAbsent(
internal fun KotlinWriter.putIfAbsent(
attributesSymbol: Symbol,
name: String,
literalValue: String? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package aws.sdk.kotlin.codegen.protocols.endpoints

import aws.sdk.kotlin.codegen.AwsRuntimeTypes
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes
import software.amazon.smithy.kotlin.codegen.core.withBlock
Expand All @@ -24,10 +25,10 @@ val awsEndpointPropertyRenderers = mapOf(
"authSchemes" to ::renderAuthSchemes,
)

private fun String.toSigningContextClassName(): String? =
private fun String.toAuthOptionFactoryFn(): Symbol? =
when (this) {
"sigv4" -> "SigV4"
"sigv4a" -> "SigV4A"
"sigv4" -> RuntimeTypes.Auth.HttpAuthAws.sigV4
"sigv4a" -> RuntimeTypes.Auth.HttpAuthAws.sigV4A
else -> null
}

Expand All @@ -37,16 +38,16 @@ private fun renderAuthSchemes(writer: KotlinWriter, authSchemes: Expression, exp
authSchemes.toNode().expectArrayNode().forEach {
val scheme = it.expectObjectNode()
val schemeName = scheme.expectStringMember("name").value
val className = schemeName.toSigningContextClassName() ?: return@forEach
val authFactoryFn = schemeName.toAuthOptionFactoryFn() ?: return@forEach

withBlock("#T.#L(", "),", RuntimeTypes.SmithyClient.Endpoints.SigningContext, className) {
withBlock("#T(", "),", authFactoryFn) {
// we delegate back to the expression visitor for each of these fields because it's possible to
// encounter template strings throughout

writeInline("signingName = ")
writeInline("serviceName = ")
renderOrElse(expressionRenderer, scheme.getStringMember("signingName"), "null")

writeInline("disableDoubleEncoding = ")
writeInline("disableDoubleUriEncode = ")
renderOrElse(expressionRenderer, scheme.getBooleanMember("disableDoubleEncoding"), "false")

when (schemeName) {
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ coroutines-version = "1.7.3"
atomicfu-version = "0.22.0"

# smithy-kotlin codegen and runtime are versioned together
smithy-kotlin-version = "0.28.1"
smithy-kotlin-version = "0.28.2-SNAPSHOT"

# codegen
smithy-version = "1.39.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@ import java.util.*

class SmithyKotlinApiSettings : ToNode {
var visibility: String? = null
var nullabilityCheckMode: String? = null
var defaultValueSerializationMode: String? = null
var enableEndpointAuthProvider: Boolean? = null

override fun toNode(): Node {
val builder = ObjectNode.objectNodeBuilder()
builder.withNullableMember("visibility", visibility)
builder.withNullableMember("nullabilityCheckMode", nullabilityCheckMode)
builder.withNullableMember("defaultValueSerializationMode", defaultValueSerializationMode)
builder.withNullableMember("enableEndpointAuthProvider", enableEndpointAuthProvider)
return builder.build()
}
}
Expand Down
4 changes: 4 additions & 0 deletions services/eventbridge/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"sdkId": "EventBridge",
"enableEndpointAuthProvider": true
}
4 changes: 4 additions & 0 deletions services/s3/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"sdkId": "S3",
"enableEndpointAuthProvider": true
}
Loading