Remove eventLoop from CredentialProviderFactory.Context (#557)

soto-project · Jul 29, 2023 · c3efb09 · c3efb09
1 parent 63e5af8
commit c3efb09
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 60 deletions.
diff --git a/Sources/SotoCore/AWSClient.swift b/Sources/SotoCore/AWSClient.swift
@@ -90,7 +90,6 @@ public final class AWSClient: Sendable {
 
         self.credentialProvider = credentialProviderFactory.createProvider(context: .init(
             httpClient: httpClient,
-            eventLoop: httpClient.eventLoopGroup.next(),
             logger: clientLogger,
             options: options
         ))

diff --git a/Sources/SotoCore/Credential/ConfigFileLoader.swift b/Sources/SotoCore/Credential/ConfigFileLoader.swift
@@ -101,16 +101,16 @@ enum ConfigFileLoader {
         let fileIO = NonBlockingFileIO(threadPool: threadPool)
 
         // Load credentials file
-        return self.loadFile(path: credentialsFilePath, on: context.eventLoop, using: fileIO)
+        return self.loadFile(path: credentialsFilePath, on: context.httpClient.eventLoopGroup.any(), using: fileIO)
             .flatMap { credentialsByteBuffer in
                 // Load profile config file
-                return loadFile(path: configFilePath, on: context.eventLoop, using: fileIO)
+                return loadFile(path: configFilePath, on: context.httpClient.eventLoopGroup.any(), using: fileIO)
                     .map {
                         (credentialsByteBuffer, $0)
                     }
                     .flatMapError { _ in
                         // Recover from error if profile config file does not exist
-                        context.eventLoop.makeSucceededFuture((credentialsByteBuffer, nil))
+                        context.httpClient.eventLoopGroup.any().makeSucceededFuture((credentialsByteBuffer, nil))
                     }
             }
             .flatMapErrorThrowing { _ in

diff --git a/Sources/SotoCore/Credential/CredentialProvider.swift b/Sources/SotoCore/Credential/CredentialProvider.swift
@@ -22,12 +22,10 @@ import SotoSignerV4
 public protocol CredentialProvider: Sendable, CustomStringConvertible {
     /// Return credential
     /// - Parameters:
-    ///   - eventLoop: EventLoop to run on
     ///   - logger: Logger to use
     func getCredential(logger: Logger) async throws -> Credential
 
     /// Shutdown credential provider
-    /// - Parameter eventLoop: EventLoop to use when shutiting down
     func shutdown() async throws
 }
 
@@ -40,14 +38,12 @@ extension CredentialProvider {
 /// Provides factory functions for `CredentialProvider`s.
 ///
 /// The factory functions are only called once the `AWSClient` has been setup. This means we can supply
-/// things like a `Logger`, `EventLoop` and `HTTPClient` to the credential provider when we construct it.
+/// things like a `Logger` and `HTTPClient` to the credential provider when we construct it.
 public struct CredentialProviderFactory {
     /// The initialization context for a `ContextProvider`
     public struct Context {
         /// The `AWSClient`s internal `HTTPClient`
         public let httpClient: HTTPClient
-        /// The `EventLoop` that the `CredentialProvider` should use for credential refreshs
-        public let eventLoop: EventLoop
         /// The `Logger` attached to the AWSClient
         public let logger: Logger
         /// AWSClient options

diff --git a/Tests/SotoCoreTests/Credential/ConfigFileCredentialProviderTests.swift b/Tests/SotoCoreTests/Credential/ConfigFileCredentialProviderTests.swift
@@ -24,16 +24,14 @@ import XCTest
 class ConfigFileCredentialProviderTests: XCTestCase {
     // MARK: - Credential Provider
 
-    func makeContext() -> (CredentialProviderFactory.Context, MultiThreadedEventLoopGroup, HTTPClient) {
-        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        let eventLoop = eventLoopGroup.next()
-        let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoop))
-        return (.init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()), eventLoopGroup, httpClient)
+    func makeContext() -> (CredentialProviderFactory.Context, HTTPClient) {
+        let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
+        return (.init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()), httpClient)
     }
 
     func testCredentialProviderStatic() async throws {
         let credentials = ConfigFileLoader.SharedCredentials.staticCredential(credential: StaticCredential(accessKeyId: "foo", secretAccessKey: "bar"))
-        let (context, eventLoopGroup, httpClient) = self.makeContext()
+        let (context, httpClient) = self.makeContext()
 
         let provider = try ConfigFileCredentialProvider.credentialProvider(
             from: credentials,
@@ -45,7 +43,6 @@ class ConfigFileCredentialProviderTests: XCTestCase {
 
         try await provider.shutdown()
         try await httpClient.shutdown()
-        XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
     }
 
     func testCredentialProviderSTSAssumeRole() async throws {
@@ -55,7 +52,7 @@ class ConfigFileCredentialProviderTests: XCTestCase {
             region: nil,
             sourceCredentialProvider: .static(accessKeyId: "foo", secretAccessKey: "bar")
         )
-        let (context, eventLoopGroup, httpClient) = self.makeContext()
+        let (context, httpClient) = self.makeContext()
 
         let provider = try ConfigFileCredentialProvider.credentialProvider(
             from: credentials,
@@ -67,7 +64,6 @@ class ConfigFileCredentialProviderTests: XCTestCase {
 
         try await provider.shutdown()
         try await httpClient.shutdown()
-        XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
     }
 
     // MARK: - Config File Credentials Provider
@@ -90,7 +86,7 @@ class ConfigFileCredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
         let factory = CredentialProviderFactory.configFile(credentialsFilePath: filenameURL.path)
 
-        let provider = factory.createProvider(context: .init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()))
+        let provider = factory.createProvider(context: .init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()))
 
         let credential: Credential = try await provider.getCredential(logger: TestEnvironment.logger)
         XCTAssertEqual(credential.accessKeyId, "AWSACCESSKEYID")
@@ -118,7 +114,7 @@ class ConfigFileCredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
         let factory = CredentialProviderFactory.configFile(credentialsFilePath: filenameURL.path)
 
-        let provider = factory.createProvider(context: .init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()))
+        let provider = factory.createProvider(context: .init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()))
 
         let credential: Credential = try await provider.getCredential(logger: TestEnvironment.logger)
         XCTAssertEqual(credential.accessKeyId, "TESTPROFILE-AWSACCESSKEYID")
@@ -136,7 +132,7 @@ class ConfigFileCredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
         let factory = CredentialProviderFactory.configFile(credentialsFilePath: filenameURL.path)
 
-        let provider = factory.createProvider(context: .init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()))
+        let provider = factory.createProvider(context: .init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()))
 
         do {
             _ = try await provider.getCredential(logger: TestEnvironment.logger)
@@ -166,12 +162,11 @@ class ConfigFileCredentialProviderTests: XCTestCase {
         let filename = #function
         let filenameURL = URL(fileURLWithPath: filename)
         XCTAssertNoThrow(try Data(credentialsFile.utf8).write(to: filenameURL))
-        let (context, eventLoopGroup, httpClient) = makeContext()
+        let (context, httpClient) = makeContext()
 
         defer {
             XCTAssertNoThrow(try FileManager.default.removeItem(at: filenameURL))
             XCTAssertNoThrow(try httpClient.syncShutdown())
-            XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
         }
 
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(

diff --git a/Tests/SotoCoreTests/Credential/ConfigFileLoaderTests.swift b/Tests/SotoCoreTests/Credential/ConfigFileLoaderTests.swift
@@ -24,11 +24,9 @@ import XCTest
 class ConfigFileLoadersTests: XCTestCase {
     // MARK: - File Loading
 
-    func makeContext() throws -> (CredentialProviderFactory.Context, MultiThreadedEventLoopGroup, HTTPClient) {
-        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        let eventLoop = eventLoopGroup.next()
-        let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoop))
-        return (.init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()), eventLoopGroup, httpClient)
+    func makeContext() throws -> (CredentialProviderFactory.Context, HTTPClient) {
+        let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
+        return (.init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()), httpClient)
     }
 
     func save(content: String, prefix: String) throws -> String {
@@ -48,12 +46,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
@@ -95,13 +92,12 @@ class ConfigFileLoadersTests: XCTestCase {
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
         let configPath = try save(content: configFile, prefix: "config")
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? FileManager.default.removeItem(atPath: configPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
@@ -134,12 +130,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
@@ -170,12 +165,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         do {
@@ -201,12 +195,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         do {
@@ -237,12 +230,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         do {
@@ -273,12 +265,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         do {
@@ -304,12 +295,11 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, eventLoopGroup, httpClient) = try makeContext()
+        let (context, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
             try? httpClient.syncShutdown()
-            try? eventLoopGroup.syncShutdownGracefully()
         }
 
         do {

diff --git a/Tests/SotoCoreTests/Credential/CredentialProviderTests.swift b/Tests/SotoCoreTests/Credential/CredentialProviderTests.swift
@@ -47,8 +47,7 @@ final class CredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
         let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
-        let eventLoop = eventLoopGroup.next()
-        let context = CredentialProviderFactory.Context(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init())
+        let context = CredentialProviderFactory.Context(httpClient: httpClient, logger: TestEnvironment.logger, options: .init())
         let myCredentialProvider = MyCredentialProvider()
         let deferredProvider = DeferredCredentialProvider(context: context, provider: myCredentialProvider)
         _ = try await deferredProvider.getCredential(logger: TestEnvironment.logger)
@@ -70,8 +69,8 @@ final class CredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
         let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
-        let eventLoop = eventLoopGroup.next()
-        let context = CredentialProviderFactory.Context(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init())
+
+        let context = CredentialProviderFactory.Context(httpClient: httpClient, logger: TestEnvironment.logger, options: .init())
         let myCredentialProvider = MyCredentialProvider()
         let deferredProvider = DeferredCredentialProvider(context: context, provider: myCredentialProvider)
         try await deferredProvider.shutdown()
@@ -95,7 +94,7 @@ final class CredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
         let factory = CredentialProviderFactory.configFile(credentialsFilePath: filenameURL.path)
 
-        let provider = factory.createProvider(context: .init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()))
+        let provider = factory.createProvider(context: .init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()))
 
         let credential = try await provider.getCredential(logger: TestEnvironment.logger)
         XCTAssertEqual(credential.accessKeyId, "AWSACCESSKEYID")
@@ -113,7 +112,7 @@ final class CredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
         let factory = CredentialProviderFactory.configFile(credentialsFilePath: filenameURL.path)
 
-        let provider = factory.createProvider(context: .init(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init()))
+        let provider = factory.createProvider(context: .init(httpClient: httpClient, logger: TestEnvironment.logger, options: .init()))
 
         do {
             _ = try await provider.getCredential(logger: TestEnvironment.logger)
@@ -140,8 +139,7 @@ final class CredentialProviderTests: XCTestCase {
         defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
         let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
         defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }
-        let eventLoop = eventLoopGroup.next()
-        let context = CredentialProviderFactory.Context(httpClient: httpClient, eventLoop: eventLoop, logger: TestEnvironment.logger, options: .init())
+        let context = CredentialProviderFactory.Context(httpClient: httpClient, logger: TestEnvironment.logger, options: .init())
         let testCredentialProvider = TestCredentialProvider()
 
         let deferredProvider = DeferredCredentialProvider(context: context, provider: testCredentialProvider)