feat: add system properties credentials provider & include it in default credentials provider chain #1037
Conversation
| { | ||
| "id": "ec4b3c6c-c63f-499d-a032-026139b4627e", | ||
| "type": "feature", | ||
| "description": "Add Jvm credentails provider and make it first in default chain credentials provider", |
There was a problem hiding this comment.
nit: "Jvm credentails" -> "JVM system property credentials"
| private val SECRET_ACCESS_KEY = AwsSdkSetting.AwsSecretAccessKey.sysProp | ||
| private val SESSION_TOKEN = AwsSdkSetting.AwsSessionToken.sysProp | ||
|
|
||
| public class JvmCredentialsProvider |
There was a problem hiding this comment.
nit: I'd call this the JvmSystemPropertyCredentialsProvider. There can be different types of credentials providers used on JVM, so JvmCredentialsProvider seems too vague to me.
Same comment applies everywhere just "JVM" is used.
There was a problem hiding this comment.
I agree JvmCredentialsProvider is too vague but I don't think the JVM part is important. Sure, it'd only work on JVM for now but there may be other future platforms which have some kind of system properties. I'd suggest SystemPropertyCredentialsProvider.
| public constructor() : this(PlatformProvider.System::getProperty) | ||
|
|
||
| private fun requireProperty(variable: String): String = | ||
| getProperty(variable) ?: throw ProviderConfigurationException("Missing value for JVM system properties `$variable`") |
There was a problem hiding this comment.
nit: "for JVM system property"
| public constructor(private val getProperty: (String) -> String?) : CredentialsProvider { | ||
| public constructor() : this(PlatformProvider.System::getProperty) |
There was a problem hiding this comment.
Instead of two constructors you can just have one which defaults to PlatformProvider.System::getProperty
public constructor(private val getProperty: (String) -> String? = PlatformProvider.System::getProperty)| @@ -47,6 +48,7 @@ class DefaultChainCredentialsProviderTest { | |||
|
|
|||
| class DefaultChainPlatformProvider( | |||
| private val env: Map<String, String>, | |||
| private val jvm: Map<String, String>, | |||
| private val SECRET_ACCESS_KEY = AwsSdkSetting.AwsSecretAccessKey.sysProp | ||
| private val SESSION_TOKEN = AwsSdkSetting.AwsSessionToken.sysProp | ||
|
|
||
| public class JvmCredentialsProvider |
There was a problem hiding this comment.
Nit: Needs KDocs, which should indicate exactly which system properties will be used.
| getProperty(variable) ?: throw ProviderConfigurationException("Missing value for JVM system properties `$variable`") | ||
|
|
||
| override suspend fun resolve(attributes: Attributes): Credentials { | ||
| coroutineContext.trace<EnvironmentCredentialsProvider> { |
0marperez
changed the title
… jvm-credentials-provider
|
Kudos, SonarCloud Quality Gate passed!
|
Issue #
closes #1033
Description of changes
-Created system properties credentials provider
-Added it to the default credentials provider chain as the first one
It's essentially a copy of the environment credentials provider
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.