kn: signing #103
kn: signing #103
Conversation
…on the thread, not isolated to the test case
| val previousSignature = "106d0654706e3e8dde144d69ca9882ea38d4d72576056c724ba763f8ed3074f3".encodeToByteArray() | ||
|
|
||
| val signature = AwsSigner.signChunkTrailer(trailingHeaders, previousSignature, signingConfig).signature.decodeToString() | ||
| val expectedSignature = "8b578658fa1705d62bf26aa73e764ac4b705e6d9efd223a2d9e156580f085de4" // validated using DefaultAwsSigner |
There was a problem hiding this comment.
Note: the expected signature changed due to different credentials being used, but I did check again with DefaultAwsSigner to ensure correctness
| name: Lint | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - '**' | ||
| - '!main' | ||
| pull_request: | ||
| branches: [ main ] | ||
| workflow_dispatch: | ||
|
|
||
| env: | ||
| PACKAGE_NAME: aws-crt-kotlin | ||
|
|
||
| jobs: | ||
| ktlint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout sources | ||
| uses: actions/checkout@v2 | ||
| - name: Lint ${{ env.PACKAGE_NAME }} | ||
| run: | | ||
| ./gradlew ktlint No newline at end of file |
There was a problem hiding this comment.
Comment: Nice, thanks for adding this!
| public actual class StaticCredentialsProvider internal actual constructor(private val builder: StaticCredentialsProviderBuilder) : CredentialsProvider { | ||
| public actual companion object {} | ||
|
|
||
| override suspend fun getCredentials(): Credentials { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override fun close() { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override suspend fun waitForShutdown() { | ||
| TODO("Not yet implemented") | ||
| } | ||
| override suspend fun getCredentials(): Credentials = | ||
| Credentials(builder.accessKeyId!!, builder.secretAccessKey!!, builder.sessionToken) | ||
| override fun close() { } | ||
| override suspend fun waitForShutdown() { } | ||
| } |
There was a problem hiding this comment.
Correctness: Typically, retaining a reference to a mutable builder is a bad idea since it means the values used by your supposedly-static provider can be changed after the fact:
val builder = StaticCredentialsProviderBuilder().apply {
accessKeyId = "foo"
secretAccessKey = "bar"
}
val built = builder.build()
println(built.getCredentials()) // foo/bar
builder.secretAccessKey = "baz"
println(built.getCredentials()) // foo/bazWe should either create the credentials once at initialization time or copy the builder values to local private members.
| ) { "sign() aws_sign_request_aws" } | ||
|
|
||
| val callbackChannel = userDataStableRef.get().second | ||
| val signature = runBlocking { callbackChannel.receive() } // wait for async signing to complete.... |
There was a problem hiding this comment.
Question: Why is runBlocking necessary inside of a suspend function?
| // FIXME Can't convert Kotlin config's shouldSignHeader to a C function without capturing the config variable, and staticCFunction cannot capture variables. | ||
| // should_sign_header = this@toNativeSigningConfig.shouldSignHeader?.toNativeShouldSignHeaderFn() | ||
| // should_sign_header_ud = |
There was a problem hiding this comment.
Question: Is there no way to avoid capturing references here? The shouldSignHeader param is just a (String) -> Boolean so I'm surprised it's causing an issue.
There was a problem hiding this comment.
I tried a second attempt, but staticCFunction can't capture any state, not even the shouldSignHeader param. Out Kotlin implementation of the native aws_should_sign_header function can only use global-level declarations.
Do you have any ideas I might be missing here?
|
3 participants
Description of changes:
This PR implements native signing APIs via CRT, including SigV4, SigV4a, and chunked signing.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.