CredentialProviderChain refactor #62
Conversation
mullermp
changed the title
...y-aws-ruby-codegen/src/main/java/software/amazon/smithy/aws/ruby/codegen/AWSIntegration.java
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/lib/aws-sdk-core/credentials_provider_chain.rb
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/lib/aws-sdk-core/credentials_provider_chain.rb
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/lib/aws-sdk-core/credentials_provider_chain.rb
Outdated
Show resolved
Hide resolved
...y-aws-ruby-codegen/src/main/java/software/amazon/smithy/aws/ruby/codegen/AWSIntegration.java
Show resolved
Hide resolved
| @@ -49,12 +49,18 @@ public List<ClientConfig> getAdditionalClientConfig(GenerationContext context) { | |||
| @see AWS::SDK::Core::CREDENTIALS_PROVIDER_CHAIN | |||
| """; | |||
|
|
|||
| String identityProviderChain = "AWS::SDK::Core::HTTPBearerProviderChain"; | |||
| String defaultIdentity = "Hearth::Identities::HTTPBearer.new(token: 'token')"; | |||
| String defaultConfigValue = "cfg[:stub_responses] ? %s.new(proc { %s }) : nil" | |||
There was a problem hiding this comment.
Naming nit: calling it defaultConfig value is a little confusing here to me since this is only for stubbed responses.
There was a problem hiding this comment.
It is a default value though.. either nil or a stub response.
...y-aws-ruby-codegen/src/main/java/software/amazon/smithy/aws/ruby/codegen/HttpBearerAuth.java
Show resolved
Hide resolved
...smithy-aws-ruby-codegen/src/main/java/software/amazon/smithy/aws/ruby/codegen/AWSConfig.java
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/lib/aws-sdk-core/instance_credentials_provider.rb
Outdated
Show resolved
Hide resolved
| "sessionToken": "0" | ||
| } | ||
| }, | ||
| "expected": { |
There was a problem hiding this comment.
Maybe not really related - but should account_id resolution be included in this as well?
There was a problem hiding this comment.
Possibly but I think we should test cred providers resolution only.
gems/aws-sdk-core/spec/aws-sdk-core/credentials_provider_chain_spec.rb
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/spec/aws-sdk-core/credentials_provider_chain_spec.rb
Outdated
Show resolved
Hide resolved
gems/aws-sdk-core/spec/aws-sdk-core/credentials_provider_chain_spec.rb
Outdated
Show resolved
Hide resolved
| module CredentialsProviderChain | ||
| def self.call(config) | ||
| class CredentialsProviderChain < Hearth::IdentityProvider | ||
| def initialize(config) |
There was a problem hiding this comment.
The way we're initializing this from the _defaults with the cfg provided by the resolver... this won't actually be the config, it will be the Config Resolver right? Is there any risk or issues in keeping that around instead of using the resolved config?
There was a problem hiding this comment.
No, I think it should be fine. I do think after discussion that we want to cache providers based on config inputs, so that a new profile or some value that influences credential providers when given per operation, resolves a new provider in the chain.
gems/aws-sdk-core/lib/aws-sdk-core/credentials_provider_chain.rb
Outdated
Show resolved
Hide resolved
| @@ -9,16 +9,18 @@ def initialize(config) | |||
| end | |||
|
|
|||
| def identity(_properties = {}) | |||
| return @identity if @identity | |||
| # TODO: cache the provider based on config keys - so that operation | |||
There was a problem hiding this comment.
I'm still on the fence on this one and not entirely convinced that we should redo default resolution based on operation config overrides.
There was a problem hiding this comment.
I think it's useful and correct to respect operation overrides for things like profile, region, credentials, etc, and we should re-resolve chain in that case.
Prototype moving providers to service gems