-
Notifications
You must be signed in to change notification settings - Fork 91
feat: Automatic clock skew adjustment #2023
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
Conversation
.SmithyIdentity, | ||
.SmithyRetriesAPI, | ||
.SmithyRetries, | ||
.SmithyTimestamps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new AWS-specific clock skew provider uses SmithyTimestamps
to read the date header in HTTP responses, hence the dependency is added.
runtimeSymbol("UnknownAWSHTTPServiceError", SwiftDeclaration.STRUCT, listOf("UnknownAWSHTTPServiceError")) | ||
val AWSServiceError = runtimeSymbol("AWSServiceError", SwiftDeclaration.PROTOCOL) | ||
val Sha256TreeHashMiddleware = runtimeSymbol("Sha256TreeHashMiddleware", SwiftDeclaration.STRUCT) | ||
val AWSClockSkewProvider = runtimeSymbol("AWSClockSkewProvider", SwiftDeclaration.ENUM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SwiftSymbol for the AWS-specific clock skew provider.
|
||
override val clockSkewProviderSymbol: Symbol | ||
get() = AWSClientRuntimeTypes.Core.AWSClockSkewProvider | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overrides the clock skew provider to use the AWS-specific implementation.
builder.interceptors.add(ClientRuntime.ContentLengthMiddleware<NoInputAndOutputInput, NoInputAndOutputOutput>()) | ||
builder.deserialize(ClientRuntime.DeserializeMiddleware<NoInputAndOutputOutput>(NoInputAndOutputOutput.httpOutput(from:), NoInputAndOutputOutputError.httpError(from:))) | ||
builder.interceptors.add(ClientRuntime.LoggerMiddleware<NoInputAndOutputInput, NoInputAndOutputOutput>(clientLogMode: config.clientLogMode)) | ||
builder.clockSkewProvider(AWSClientRuntime.AWSClockSkewProvider.clockSkew(request:response:error:now:)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here & below, expectations for several tests are changed to accommodate new codegen.
@_spi(SmithyTimestamps) import struct SmithyTimestamps.TimestampFormatter | ||
|
||
public enum AWSClockSkewProvider { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the AWS-specific clock skew provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No further explanation, see inline code comments.
} else { | ||
throw TestHTTPError(statusCode: response.statusCode) | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test file changed comparable to the retry integration tests in smithy-swift.
Sources/Core/AWSSDKHTTPAuth/Sources/AWSSDKHTTPAuth/AWSSigV4Signer.swift
Outdated
Show resolved
Hide resolved
import class Smithy.Context | ||
import class Smithy.ContextBuilder | ||
import class SmithyHTTPAPI.HTTPRequest | ||
import class SmithyHTTPAPI.HTTPResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file & the next few below demonstrate the changes to code-generated clients.
builder.interceptors.add(ClientRuntime.ContentLengthMiddleware<NoInputAndOutputInput, NoInputAndOutputOutput>()) | ||
builder.deserialize(ClientRuntime.DeserializeMiddleware<NoInputAndOutputOutput>(NoInputAndOutputOutput.httpOutput(from:), NoInputAndOutputOutputError.httpError(from:))) | ||
builder.interceptors.add(ClientRuntime.LoggerMiddleware<NoInputAndOutputInput, NoInputAndOutputOutput>(clientLogMode: config.clientLogMode)) | ||
builder.clockSkewProvider(AWSClientRuntime.AWSClockSkewProvider.provider()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here & below, test expectations are updated for clock skew codegen.
(Not sure why the change in escaping of $
but it works... I suspect it's a change in IntelliJ copy/paste of Kotlin strings.)
requestBuilder.withQueryItem(URIQueryItem(name: "DBUser", value: username)) | ||
|
||
let signedAt = Date() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AuthTokenGenerator
modified to provide signedAt
to signer.
import SmithyHTTPAPI | ||
@_spi(SmithyTimestamps) import SmithyTimestamps | ||
|
||
class AWSClockSkewProviderTests: XCTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests for the behavior of the clock skew provider above.
) | ||
} | ||
|
||
var signingConfig = try constructSigningConfig(identity: identity, signingProperties: signingProperties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWSSigV4Signer
changed comparably to the one in smithy-swift, to set signedAt
.
.SmithyIdentity, | ||
.SmithyRetriesAPI, | ||
.SmithyRetries, | ||
.SmithyTimestamps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Package.Base.swift
at top; this file's just generated from that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just needs Package.version.next unstaged, but otherwise everything lgtm
Description of changes
smithy-lang/smithy-swift#968 (the companion to this PR) implements clock skew interfaces and infrastructure.
This PR replaces some "generic Smithy" implementation of clock skew with behaviors specialized for AWS.
AWSClockSkewProvider
calculates clock skew based on characteristics of AWS-specific modeled errors. Codegen changes install this provider in the orchestrator for AWS operation invocations.AWSSigV4Signer
and various token providers are modified for changes to signing of HTTP requests made in the companion PR.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.