-
Notifications
You must be signed in to change notification settings - Fork 2
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 export sts credentials providers #36
Conversation
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.
One possible suggestion/alternative but otherwise looks good.
val adaptedCreds = adapted.credentials.get() | ||
|
||
// https://discuss.kotlinlang.org/t/bytearray-comparison/1689/12 | ||
assertEquals(adaptedCreds.accessKeyId.toList(), ACCESS_KEY.toList()) |
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.
nit
assertContentEquals
does structural comparison of the underlying elements
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.
nice, thanks
} | ||
|
||
// Convert the SDK version of CredentialsProvider type to the CRT version | ||
internal fun adapt(credentialsProvider: CredentialsProvider?): CredentialsProviderJni? { |
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.
suggestion
I was going to ask about this. I'm assuming this is because the CredentialsProvider
used for sourcing credentials MUST be one of the aws-crt-java
credential providers to actually work (see #15). An arbitrary customer provided provider will fail.
Instead of using runBlocking
we could just do this at the time getCredentials
is called. So instead of inheriting from JniCredentialsProvider()
, just specialize this one since it has custom requirements.
e.g. something like
public actual class StsAssumeRoleCredentialsProvider
internal actual constructor(builder: Builder) : CredentialsProvider {
private val sourceCredentialsProvider = builder.credentialsProvider
private val jniBuilder = StsCredentialsProviderJni.builder()
.withClientBootstrap(...)
// ... everything except withCredsProvider()
// leave it as a builder() not fully constructed
override suspend fun getCredentials(): Credentials {
val sourceCreds = sourceCredentialsProvider.getCredentials()
val staticProvider = StaticCredentialsProviderJni(sourceCreds)
// complete building the JNI provider here, it will be a single use provider
val stsProvider = jniBuilder.withCredsProvider(staticProvider)
val jniCreds = stsProvider.use {
stsProvider.credentials.await()
}
return Credentials(jniCreds.accessKeyId, jniCreds.secretAccessKey, jniCreds.sessionToken)
}
// we keep no references to an actual provider past getCredentials() so close/waitForShutdown are no-ops
override fun close() {}
override suspend fun waitForShutdown() { }
}
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.
That's the constraint as I understand it yes. Thanks for taking the time to work through this alternative. I like it over what I've provided as it avoids the runBlocking. I've worked through getting your sample to compile however it fails to return credentials in my unit test. I spent some time trying to debug but no luck. I don't think it's a big task to get this working, it's just not clear to me what's wrong. Debugging is difficult due to the async. I'll continue on with what I have for now and double back when there is more time.
My version of your proposal:
public actual class StsAssumeRoleCredentialsProvider
internal actual constructor(builder: StsAssumeRoleCredentialsProviderBuilder) : CredentialsProvider {
public actual companion object {}
private val sourceCredentialsProvider = builder.credentialsProvider
private val jniBuilder = StsCredentialsProviderJni.builder()
.withClientBootstrap(builder.clientBootstrap?.jniBootstrap)
.withTlsContext(builder.tlsContext?.jniCtx)
.withDurationSeconds(builder.durationSeconds ?: DEFAULT_DURATION_SECONDS)
.withRoleArn(builder.roleArn)
.withSessionName(builder.sessionName)
override suspend fun getCredentials(): Credentials {
val sourceCreds = sourceCredentialsProvider!!.getCredentials()
val staticProvider = StaticCredentialsProviderJni
.StaticCredentialsProviderBuilder()
.withCredentials(toCrtCredentials(sourceCreds))
.build()
val stsProvider = jniBuilder.withCredsProvider(staticProvider).build()
val jniCreds = stsProvider.use {
stsProvider.credentials.await()
}
return Credentials(jniCreds.accessKeyId, jniCreds.secretAccessKey, jniCreds.sessionToken)
}
// we keep no references to an actual provider past getCredentials() so close/waitForShutdown are no-ops
override fun close() {}
override suspend fun waitForShutdown() { }
}
Failing unit test:
@Test
fun itPassesCredentials() = runSuspendTest {
val sdkCredentialsProvider = object : CredentialsProvider {
override suspend fun getCredentials(): Credentials =
Credentials(ACCESS_KEY, SECRET_ACCESS_KEY, SESSION_TOKEN)
override fun close() {}
override suspend fun waitForShutdown() {}
}
val builder = StsAssumeRoleCredentialsProviderBuilder()
builder.credentialsProvider = sdkCredentialsProvider
builder.tlsContext = TlsContext(TlsContextOptions.defaultClient())
val unit = StsAssumeRoleCredentialsProvider(builder)
val adaptedCreds = unit.getCredentials()
assertEquals(adaptedCreds.accessKeyId, ACCESS_KEY)
assertEquals(adaptedCreds.secretAccessKey, SECRET_ACCESS_KEY)
assertEquals(adaptedCreds.sessionToken, SESSION_TOKEN)
}
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.
Tracking issue: awslabs/aws-sdk-kotlin#357
private fun toCrtCredentialsProvider(sdkCredentialsProvider: CredentialsProvider): CredentialsProviderJni { | ||
return object : CredentialsProviderJni() { | ||
|
||
override fun getCredentials(): CompletableFuture<CredentialsJni> = runBlocking { |
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.
this will block until the deferred
is complete. In general all coroutine builders will block until their child jobs are complete.
I suggested a potential alternative above. I'm not overly concerned though since this runs in a CRT thread ultimately.
Issue #, if available:
awslabs/aws-sdk-kotlin#325
awslabs/aws-sdk-kotlin#19
Description of changes:
Expose the two STS based credentials provider from
crt-java
into Kotlin.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.