-
Notifications
You must be signed in to change notification settings - Fork 50
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: endpoints2: aws middleware and integration changes #751
Changes from 2 commits
c14fa15
e8243a9
4f34a1f
9ab7df8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,43 +4,48 @@ | |
*/ | ||
package aws.sdk.kotlin.codegen.protocols.endpoints | ||
|
||
import aws.sdk.kotlin.codegen.AwsServiceConfigIntegration | ||
import software.amazon.smithy.codegen.core.Symbol | ||
import software.amazon.smithy.kotlin.codegen.KotlinSettings | ||
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter | ||
import software.amazon.smithy.kotlin.codegen.core.withBlock | ||
import software.amazon.smithy.kotlin.codegen.model.buildSymbol | ||
import software.amazon.smithy.kotlin.codegen.model.defaultName | ||
import software.amazon.smithy.kotlin.codegen.rendering.endpoints.EndpointParametersGenerator | ||
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator | ||
import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameter | ||
import aws.sdk.kotlin.codegen.customization.s3.ClientConfigIntegration as S3ClientConfigIntegration | ||
import aws.sdk.kotlin.codegen.customization.s3control.ClientConfigIntegration as S3ControlClientConfigIntegration | ||
|
||
/** | ||
* Render binding of AWS SDK endpoint parameter builtins. In practice, all of these values are sourced from the client | ||
* config. | ||
*/ | ||
fun renderBindAwsBuiltins(ctx: ProtocolGenerator.GenerationContext, writer: KotlinWriter, builtinParams: List<Parameter>) { | ||
writer.withBlock( | ||
"public fun #T.Builder.bindAwsBuiltins(config: #T.Config) {", | ||
"internal fun #T.Builder.bindAwsBuiltins(config: #T.Config) {", | ||
"}", | ||
EndpointParametersGenerator.getSymbol(ctx.settings), | ||
ctx.symbolProvider.toSymbol(ctx.service), | ||
) { | ||
builtinParams.forEach { | ||
when (it.builtIn.get()) { | ||
"AWS::Region" -> renderBasicConfigBinding(writer, it, "region") | ||
"AWS::UseFIPS" -> renderBasicConfigBinding(writer, it, "useFips") | ||
"AWS::UseDualStack" -> renderBasicConfigBinding(writer, it, "useDualStack") | ||
"AWS::Region" -> renderBasicConfigBinding(writer, it, AwsServiceConfigIntegration.RegionProp.propertyName) | ||
"AWS::UseFIPS" -> renderBasicConfigBinding(writer, it, AwsServiceConfigIntegration.UseFipsProp.propertyName) | ||
"AWS::UseDualStack" -> renderBasicConfigBinding(writer, it, AwsServiceConfigIntegration.UseDualStackProp.propertyName) | ||
|
||
"AWS::S3::Accelerate" -> renderBasicConfigBinding(writer, it, "enableAccelerate") | ||
"AWS::S3::ForcePathStyle" -> renderBasicConfigBinding(writer, it, "forcePathStyle") | ||
"AWS::S3::DisableMultiRegionAccessPoints" -> renderBasicConfigBinding(writer, it, "disableMrap") | ||
"AWS::S3::UseArnRegion", "AWS::S3Control::UseArnRegion" -> renderBasicConfigBinding(writer, it, "useArnRegion") | ||
"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) | ||
|
||
"SDK::Endpoint" -> | ||
writer.write("#L = config.endpointUrl?.toString()", EndpointParametersGenerator.getParamKotlinName(it)) | ||
writer.write("#L = config.#L?.toString()", it.defaultName(), AwsServiceConfigIntegration.EndpointUrlProp.propertyName) | ||
|
||
// as a newer SDK we do NOT support these values, they are always false | ||
"AWS::S3::UseGlobalEndpoint", "AWS::STS::UseGlobalEndpoint" -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question/Correctness: Should we even render this for the Kotlin SDK? This seems like a legacy concern that we could filter or make internal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to force them false here if we definitely don't want to support this route in the SDK, since the ruleset could default them to true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I meant was should we even be surfacing this at the config level? I guess it's fine, it's just legacy cruft that doesn't really apply to us though. |
||
writer.write("#L = false", EndpointParametersGenerator.getParamKotlinName(it)) | ||
writer.write("#L = false", it.defaultName()) | ||
} | ||
} | ||
} | ||
|
@@ -53,5 +58,5 @@ fun bindAwsBuiltinsSymbol(settings: KotlinSettings): Symbol = | |
} | ||
|
||
private fun renderBasicConfigBinding(writer: KotlinWriter, param: Parameter, configMember: String) { | ||
writer.write("#L = config.#L", EndpointParametersGenerator.getParamKotlinName(param), configMember) | ||
writer.write("#L = config.#L", param.defaultName(), configMember) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: I suggest moving the
get() =
to the next line with the getter body (ref: Kotlin coding conventions).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprised that ktlintFormat didn't catch this one tbh