-
Notifications
You must be signed in to change notification settings - Fork 95
feat: Decompose CRT default chain into individual wrappers #1926
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -9,6 +9,8 @@ import class AwsCommonRuntimeKit.CredentialsProvider | |
| import ClientRuntime | ||
| import protocol SmithyIdentity.AWSCredentialIdentityResolvedByCRT | ||
| @_spi(FileBasedConfig) import AWSSDKCommon | ||
| import protocol SmithyIdentity.AWSCredentialIdentityResolver | ||
| import struct Smithy.Attributes | ||
|
|
||
| // swiftlint:disable type_name | ||
| // ^ Required to mute swiftlint warning about type name being too long. | ||
|
|
@@ -24,21 +26,33 @@ import protocol SmithyIdentity.AWSCredentialIdentityResolvedByCRT | |
| /// 5. EC2 Instance Metadata (IMDSv2) | ||
| /// | ||
| /// The credentials retrieved from the chain are cached for 15 minutes. | ||
| public struct DefaultAWSCredentialIdentityResolverChain: AWSCredentialIdentityResolvedByCRT { | ||
| public let crtAWSCredentialIdentityResolver: AwsCommonRuntimeKit.CredentialsProvider | ||
|
|
||
| public struct DefaultAWSCredentialIdentityResolverChain: AWSCredentialIdentityResolver { | ||
| /// Creates a credential identity resolver that uses the default AWS credential identity resolver chain used by most AWS SDKs. | ||
| public init() throws { | ||
| let fileBasedConfig = try CRTFileBasedConfiguration() | ||
| try self.init(fileBasedConfig: fileBasedConfig) | ||
| } | ||
| public init() {} | ||
|
|
||
| public func getIdentity(identityProperties: Attributes?) async throws -> AWSCredentialIdentity { | ||
| typealias ResolverFactory = () throws -> any AWSCredentialIdentityResolver | ||
|
|
||
| let resolverFactories: [ResolverFactory] = [ | ||
| { try EnvironmentAWSCredentialIdentityResolver() }, | ||
| { try ProfileAWSCredentialIdentityResolver() }, | ||
| { try STSWebIdentityAWSCredentialIdentityResolver() }, | ||
| { try ECSAWSCredentialIdentityResolver() }, | ||
| { try IMDSAWSCredentialIdentityResolver() } | ||
| ] | ||
|
|
||
| let lastIndex = resolverFactories.count - 1 | ||
| for index in 0..<lastIndex { | ||
| do { | ||
| let resolver = try resolverFactories[index]() | ||
| return try await resolver.getIdentity(identityProperties: identityProperties) | ||
| } catch { | ||
| // Continue to the next resolver factory. | ||
dayaffe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| @_spi(DefaultAWSCredentialIdentityResolverChain) | ||
| public init(fileBasedConfig: CRTFileBasedConfiguration) throws { | ||
| self.crtAWSCredentialIdentityResolver = try AwsCommonRuntimeKit.CredentialsProvider(source: .defaultChain( | ||
| bootstrap: SDKDefaultIO.shared.clientBootstrap, | ||
| fileBasedConfiguration: fileBasedConfig | ||
| )) | ||
| // The error thrown from the last resolver is not caught and instead gets thrown to caller. | ||
| return try await resolverFactories[lastIndex]().getIdentity(identityProperties: identityProperties) | ||
|
Comment on lines
+54
to
+55
Contributor
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. will this always throw the IMDS error if all resolver factories fail? If so this was a huge pain from the user perspective when we used CRT so we should take this opportunity to throw a more descriptive error
Contributor
Author
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. That's a great point, should we include that as part of the final breaking change PR tho? To maintain error behavior the same for now.
Contributor
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. @jbelkins what do you think? Change this now or change this later
Contributor
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. I tend to agree with changing this now. The error should give some info about the specific providers in the chain that failed. Returning an IMDS error just because that’s the last in the chain does not make it clear what has really happened
Contributor
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. After discussion, I recommend changing this error along with other breaking changes, i.e. adding to or reordering the chain. |
||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.