feat!: Modularize event streams & event streams auth #1530

jbelkins · 2024-05-29T03:55:40Z

Issue #

#1384 (Modularize Event Streams)
#1385 (Modularize Event Streams Auth)

Description of changes

Event streams code has been modularized by either moving it to a new module in this SDK or moving it out to smithy-swift. Several other functions (HTTPAuth, checksums, etc.) have been partially modularized along with event streams & event streams auth.

AWSSDKEventStreamsAuth module created for signing event streams with SigV4.
A few other SDK modules (AWSSDKIdentity, AWSSDKHTTPAuth were created to support event streams, but are not complete.
Changes in the corresponding smithy-swift PR feat!: Modularize event streams & event streams auth smithy-lang/smithy-swift#741 have been adapted by this SDK.

New/existing dependencies impact assessment, if applicable

No new dependencies were added to this change.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jbelkins · 2024-05-29T04:26:48Z

AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Resources/Package.Base.swift

+    static var smithyEventStreamsAPI: Self { .product(name: "SmithyEventStreamsAPI", package: "smithy-swift") }
+    static var smithyEventStreamsAuthAPI: Self { .product(name: "SmithyEventStreamsAuthAPI", package: "smithy-swift") }
+    static var smithyEventStreams: Self { .product(name: "SmithyEventStreams", package: "smithy-swift") }
+    static var smithyChecksumsAPI: Self { .product(name: "SmithyChecksumsAPI", package: "smithy-swift") }


Created several new modules in smithy-swift; they are added here as dependencies

jbelkins · 2024-05-29T04:28:11Z

AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Resources/Package.Base.swift

+        .library(name: "AWSClientRuntime", targets: ["AWSClientRuntime"]),
+        .library(name: "AWSSDKEventStreamsAuth", targets: ["AWSSDKEventStreamsAuth"]),
+        .library(name: "AWSSDKIdentity", targets: ["AWSSDKIdentity"]),
+        .library(name: "AWSSDKHTTPAuth", targets: ["AWSSDKHTTPAuth"]),


Several new AWS modules are added.

AWSSDKIdentity and AWSSDKHTTPAuth are likely not complete; enough types were added to them to allow event stream modularization.

jbelkins · 2024-05-29T04:29:01Z

AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Resources/Package.Base.swift

@@ -105,14 +147,30 @@ func addDoccDependency() {

 // MARK: - Services

+let serviceTargetDependencies: [Target.Dependency] = [


This long list is broken into a variable so it can be shared between service client targets and protocol test targets.

jbelkins · 2024-05-29T04:29:45Z

Package.swift

@@ -21,6 +21,12 @@ extension Target.Dependency {
    static var clientRuntime: Self { .product(name: "ClientRuntime", package: "smithy-swift") }
    static var smithyRetriesAPI: Self { .product(name: "SmithyRetriesAPI", package: "smithy-swift") }


This file is generated. See comments in the Package.Base.swift above.

jbelkins · 2024-05-29T04:30:41Z

Sources/Core/AWSClientRuntime/AWSClientConfigDefaultsProvider.swift

@@ -5,9 +5,10 @@
 // SPDX-License-Identifier: Apache-2.0
 //


Many files, like this one, have import and type namespace adjustments. I won't comment on those every time they happen in the following files.

jbelkins · 2024-05-29T04:31:43Z

...SClientRuntime/Auth/AWSCredentialIdentityResolvers/CachedAWSCredentialIdentityResolver.swift

@@ -5,13 +5,14 @@
 // SPDX-License-Identifier: Apache-2.0
 //


AWSCredentialIdentity (above) was moved out to smithy-swift because SRA specifies that it should be there.

jbelkins · 2024-05-29T04:33:41Z

Sources/Core/AWSClientRuntime/Errors/UnknownAWSHTTPServiceError.swift

@@ -3,6 +3,7 @@
 * SPDX-License-Identifier: Apache-2.0.
 */

+import class SmithyHTTPAPI.HttpResponse
 import ClientRuntime


The following files (the event stream files in AWSClientRuntime) have been moved to smithy-swift since they are not actually AWS-specific.

jbelkins · 2024-05-29T04:35:18Z

Sources/Core/AWSClientRuntime/Middlewares/AWSS3ErrorWith200StatusXMLMiddleware.swift

@@ -18,8 +20,7 @@ public struct AWSS3ErrorWith200StatusXMLMiddleware<OperationStackInput, Operatio
                          next: H) async throws -> OperationOutput<OperationStackOutput>
    where H: Handler,
          Self.MInput == H.Input,
-          Self.MOutput == H.Output,
-          Self.Context == H.Context {
+          Self.MOutput == H.Output {


As you will see later, Context is removed as a generic type on both the Middleware and Handler protocols, because it does not need to be specialized.

jbelkins · 2024-05-29T04:36:32Z

Sources/Core/AWSClientRuntime/Middlewares/Sha256TreeHashMiddleware.swift

@@ -1,9 +1,12 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0.

+import class Smithy.Context


HttpContext was moved into the new Smithy module and renamed to Context since it is not specific to HTTP.

jbelkins · 2024-05-29T04:37:55Z

Sources/Core/AWSClientRuntime/PrimitiveExtensions/String+Extension.swift

@@ -22,22 +22,3 @@ extension String {
        String(map { tokenNoHashCharacterSet.contains($0) ? $0 : substituteCharacter })
    }
 }
-
-extension String {


This extension was moved since it is used with checksums.

jbelkins · 2024-05-29T04:39:56Z

Sources/Core/AWSSDKEventStreamsAuth/AWSMessageSigner.swift


-extension AWSEventStream {
    /// Signs a `Message` using the AWS SigV4 signing algorithm
    public class AWSMessageSigner: MessageSigner {


This is the SigV4 signer for event streams. It is moved from AWSClientRuntime to the new AWSSDKEventStreamsAuth module.

jbelkins · 2024-05-29T04:41:27Z

Sources/Core/AWSSDKEventStreamsAuth/AWSSigV4Signer+EventStreams.swift

+import struct Foundation.Data
+import AwsCommonRuntimeKit
+
+extension AWSSigV4Signer: MessageDataSigner {


This extension was previously defined right on the signer. Moved it out separate so it could be packaged with event streams auth.

jbelkins · 2024-05-29T04:42:37Z

Sources/Core/AWSSDKEventStreamsAuth/Context+AWSEventStreamsAuth.swift

+import SmithyEventStreams
+import SmithyEventStreamsAuthAPI
+
+extension Context {


Some special functions on the Context object to support event streams auth.

jbelkins · 2024-05-29T04:43:41Z

Sources/Core/AWSSDKHTTPAuth/AWSSigV4Signer.swift

 import AwsCommonRuntimeKit
 import ClientRuntime
 import Foundation

-public class AWSSigV4Signer: ClientRuntime.Signer {
+public class AWSSigV4Signer: SmithyHTTPAuthAPI.Signer {


This is the regular signer for SigV4. It is split from AWSClientRuntime to the new AWSSDKHTTPAuth module.

jbelkins · 2024-05-29T04:48:44Z

Sources/Core/AWSSDKHTTPAuth/AWSSigV4Signer.swift

@@ -127,18 +135,6 @@ public class AWSSigV4Signer: ClientRuntime.Signer {
        )
    }

-    public func signEvent(


The event signing methods on this signer were split off to an extension in the event streams auth module.

sichanyoo

1 naming & 1 access level comment

sichanyoo · 2024-05-30T17:48:39Z

AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Resources/Package.Base.swift

@@ -35,7 +41,10 @@ let package = Package(
        .watchOS(.v6)
    ],
    products: [
-        .library(name: "AWSClientRuntime", targets: ["AWSClientRuntime"])
+        .library(name: "AWSClientRuntime", targets: ["AWSClientRuntime"]),
+        .library(name: "AWSSDKEventStreamsAuth", targets: ["AWSSDKEventStreamsAuth"]),


naming: For the new modules, I think it would be better to name it AWSXYZ instead of AWSSDKXYZ given SDK is redundant & AWSSDK is harder to read with 2 S's back to back

Good with either AWS prefix or SDK prefix. My preference is actually SDK because I think it is more clear. I.e. this is our runtime / our code not something determined by AWS service teams

The runtime & our code are determined by AWS specifications, hence the AWS prefix suggestion.

Imo SDK by itself would be ambiguous. By itself, it doesn't carry the meaning that it's built for AWS spec.

Come to think of it though, I guess making the prefix SDK does allow users to easily distinguish SDK module imports vs Service module imports since all the service modules start with AWS.

I used AWSSDK as the prefix for SDK runtime modules because all our service client modules are prefixed with only AWS.

For example, I can easily see an AWS service being launched with the name Identity... you can see the potential for namespace collisions.

So using AWSSDK as the prefix for runtime names helps to prevent namespace collisions between SDK components and AWS service clients.

sichanyoo · 2024-05-30T17:51:35Z

...SClientRuntime/Auth/AWSCredentialIdentityResolvers/CachedAWSCredentialIdentityResolver.swift

 import Foundation

 ///  A credential identity resolver that caches the credentials sourced from the provided resolver.
 public struct CachedAWSCredentialIdentityResolver: AWSCredentialIdentityResolvedByCRT {
-    let crtAWSCredentialIdentityResolver: AwsCommonRuntimeKit.CredentialsProvider
+    public let crtAWSCredentialIdentityResolver: AwsCommonRuntimeKit.CredentialsProvider


Q: Why was this property made public in this and other AWS credential identity resolvers? Afaik I don't think we want to expose the underlying CRT stuff where possible

+1 on keeping this private, if code outside needs to interact with this let's add own public methods for accomplishing that rather than utilizing it directly

Honestly I did not look at this closely; I added public to get the code to compile.

This should be sorted out in #1382 along with the rest of the identity code.

…streams

sichanyoo

lgtm

jbelkins added 8 commits May 27, 2024 00:01

All Swift unit tests pass

0452efe

Unit tests pass

97f1348

Protocol tests run & pass

8ce0e9c

Fix lint

b0db1b1

SDK builds

8f9a7f2

Fix ktlint

d790c28

Fix codegen tests

3c4896d

Fix protocol tests, restore integration test scheme

97de38c

jbelkins commented May 29, 2024

View reviewed changes

jbelkins added 4 commits May 28, 2024 23:50

Remove AWSSigningAlgorithm

e3edcbb

Revert AWSSigningAlgorithm to SigningAlgorithm in smithy-swift

6f7903e

Fix integration tests

fadf1b8

Change CI scheme name

b53af67

jbelkins changed the title ~~Jbe/modularize event streams~~ feat!: Modularize event streams & event streams auth May 29, 2024

jbelkins marked this pull request as ready for review May 29, 2024 15:19

jbelkins requested a review from dayaffe May 29, 2024 15:19

jbelkins requested a review from sichanyoo May 29, 2024 15:19

sichanyoo reviewed May 30, 2024

View reviewed changes

jbelkins added 5 commits May 30, 2024 20:35

Merge remote-tracking branch 'origin/main' into jbe/modularize_event_…

6381b15

…streams

Adapt to new URI types

5e2d70e

Use URIQueryItem

4398291

Fix SPI imports

bfa9fee

Fix AWSHTTPAuthTests & ktlint

1640378

sichanyoo self-requested a review May 31, 2024 20:59

sichanyoo approved these changes May 31, 2024

View reviewed changes

jbelkins merged commit bf8a8b3 into main May 31, 2024
29 checks passed

jbelkins deleted the jbe/modularize_event_streams branch May 31, 2024 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Modularize event streams & event streams auth #1530

feat!: Modularize event streams & event streams auth #1530

jbelkins commented May 29, 2024 •

edited

Loading

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

jbelkins May 29, 2024

sichanyoo left a comment

sichanyoo May 30, 2024

dayaffe May 31, 2024 •

edited

Loading

sichanyoo May 31, 2024

jbelkins May 31, 2024

sichanyoo May 30, 2024

dayaffe May 31, 2024 •

edited

Loading

jbelkins May 31, 2024

sichanyoo May 31, 2024

sichanyoo left a comment •

edited

Loading

		@@ -105,14 +147,30 @@ func addDoccDependency() {

		// MARK: - Services

		let serviceTargetDependencies: [Target.Dependency] = [

		@@ -21,6 +21,12 @@ extension Target.Dependency {
		static var clientRuntime: Self { .product(name: "ClientRuntime", package: "smithy-swift") }
		static var smithyRetriesAPI: Self { .product(name: "SmithyRetriesAPI", package: "smithy-swift") }

feat!: Modularize event streams & event streams auth #1530

feat!: Modularize event streams & event streams auth #1530

Conversation

jbelkins commented May 29, 2024 • edited Loading

Issue #

Description of changes

New/existing dependencies impact assessment, if applicable

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sichanyoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dayaffe May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dayaffe May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sichanyoo left a comment • edited Loading

Choose a reason for hiding this comment

jbelkins commented May 29, 2024 •

edited

Loading

dayaffe May 31, 2024 •

edited

Loading

dayaffe May 31, 2024 •

edited

Loading

sichanyoo left a comment •

edited

Loading