feat: enable auth resolution via endpoints #1109
Conversation
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
| ) | ||
| private fun renderDefaultSigningContext(writer: KotlinWriter, previousValue: String?) { | ||
| val signingAttrs = RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSigningAttributes | ||
| // https://github.com/awslabs/aws-sdk-kotlin/issues/200 |
There was a problem hiding this comment.
question: this issue is closed, is it still relevant?
There was a problem hiding this comment.
This is here to remind us why path normalization and single URI encoding are important for S3.
There was a problem hiding this comment.
Yeah I left it as a clue for posterity about why we care about this and where the requirements come from.
| * Service specific package settings | ||
| */ | ||
| val AwsService.packageSettings: String | ||
| get() = rootProject.file("$destinationDir/package.json").absolutePath |
There was a problem hiding this comment.
question/nit: Is package.json already an established name for SDK client codegen configuration? I think something like config.json makes more sense
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Handle what? This is custom logic to feed into smithy-build.json.
|
A new generated diff is ready to view. |
codegen/sdk/build.gradle.kts
Outdated
| 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}`" } | ||
| } |
There was a problem hiding this comment.
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}`" }
}| ) | ||
| private fun renderDefaultSigningContext(writer: KotlinWriter, previousValue: String?) { | ||
| val signingAttrs = RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSigningAttributes | ||
| // https://github.com/awslabs/aws-sdk-kotlin/issues/200 |
There was a problem hiding this comment.
This is here to remind us why path normalization and single URI encoding are important for S3.
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
A new generated diff is ready to view. |
Issue #
upstream: smithy-lang/smithy-kotlin#993
Description of changes
Enable using endpoint resolution as a way of resolving auth schemes for select services (S3 and EventBridge). This comes as a requirement from SRA for these services where their auth scheme rules are intimately tied to endpoints. This is an opt-in codegen feature since most services don't need this.
Also refactors the way S3 signing properties are set by defaulting them in the execution context rather than overriding the auth scheme instantiation expression. This ends up being a better customer experience if someone wants to override existing auth schemes (e.g. to replace the signer only) as they don't have to know about all the other S3 specific signer settings.
Testing
Tested against an MRAP bucket
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.