-
Notifications
You must be signed in to change notification settings - Fork 35
feat: Automatic clock skew adjustment #968
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
d0dd976
049737e
0e113e8
cce2bb9
ac093b5
ca13a9f
e4beca8
8c9e5d2
44ac3c9
614285d
1576a53
ab142e1
9de28f0
d1bc67a
70926f1
d95d2d3
0931eda
c8a9789
e5b6694
b187d32
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 |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import struct Foundation.TimeInterval | ||
import protocol Smithy.RequestMessage | ||
import protocol Smithy.ResponseMessage | ||
|
||
/// A closure that is called to determine what, if any, correction should be made to the system's clock when signing requests. | ||
/// | ||
/// Returns: a `TimeInterval` that represents the correction ("clock skew") that should be applied to the system clock, | ||
/// or `nil` if no correction should be applied. | ||
/// - Parameters: | ||
/// - request: The request that was sent to the server. (Typically this is a `HTTPRequest`) | ||
/// - response: The response that was returned from the server. (Typically this is a `HTTPResponse`) | ||
/// - error: The error that was returned by the server; typically this is a `ServiceError` with an error code that | ||
/// indicates clock skew is or might be the cause of the failed request. | ||
/// - previous: The previously measured clock skew value, or `nil` if none was recorded. | ||
/// - Returns: The calculated clock skew `TimeInterval`, or `nil` if no clock skew adjustment is to be applied. | ||
public typealias ClockSkewProvider<Request: RequestMessage, Response: ResponseMessage> = | ||
@Sendable (_ request: Request, _ response: Response, _ error: Error, _ previous: TimeInterval?) -> TimeInterval? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import struct Foundation.TimeInterval | ||
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. This type is just a concurrency-safe store for clock skew values, keyed by the hostname of the server where that clock skew was measured. |
||
|
||
/// Serves as a concurrency-safe repository for recorded clock skew values, keyed by hostname. | ||
/// | ||
/// Storing clock skew values in a shared repository allows future operations to include the clock skew | ||
/// correction on their initial attempt. It also allows multiple clients to share clock skew values. | ||
actor ClockSkewStore { | ||
static let shared = ClockSkewStore() | ||
|
||
/// Stores clock skew values, keyed by hostname. | ||
private var clockSkewStorage = [String: TimeInterval]() | ||
|
||
// Disable creation of new instances of this type. | ||
private init() {} | ||
|
||
/// Retrieves the clock skew value for the passed host. | ||
/// - Parameter host: The host name for which to retrieve clock skew | ||
/// - Returns: The clock skew for the indicated host or `nil` if none is set. | ||
func clockSkew(host: String) async -> TimeInterval? { | ||
clockSkewStorage[host] | ||
} | ||
|
||
/// Calls the passed block to modify the clock skew value for the passed host. | ||
/// | ||
/// Returns a `Bool` indicating whether the clock skew value changed. | ||
/// - Parameters: | ||
/// - host: The host for which clock skew is to be updated. | ||
/// - block: A block that accepts the previous clock skew value, and returns the updated value. | ||
/// - Returns: `true` if the clock skew value was changed, `false` otherwise. | ||
func setClockSkew(host: String, block: @Sendable (TimeInterval?) -> TimeInterval?) async -> Bool { | ||
let previousValue = clockSkewStorage[host] | ||
let newValue = block(previousValue) | ||
clockSkewStorage[host] = newValue | ||
return newValue != previousValue | ||
} | ||
|
||
/// Clears all saved clock skew values. For use during testing. | ||
func clear() async { | ||
clockSkewStorage = [:] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import struct Foundation.Date | ||
import struct Foundation.TimeInterval | ||
import protocol Smithy.RequestMessage | ||
import protocol Smithy.ResponseMessage | ||
|
||
public enum DefaultClockSkewProvider { | ||
|
||
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. This is the "generic Smithy" clock skew provider. It does not do anything. aws-sdk-swift provides its own implementation of this which actually does measure clock skew. |
||
public static func provider<Request: RequestMessage, Response: ResponseMessage>( | ||
) -> ClockSkewProvider<Request, Response> { | ||
return clockSkew(request:response:error:previous:) | ||
} | ||
|
||
@Sendable | ||
private static func clockSkew<Request: RequestMessage, Response: ResponseMessage>( | ||
request: Request, | ||
response: Response, | ||
error: Error, | ||
previous: TimeInterval? | ||
) -> TimeInterval? { | ||
// The default clock skew provider does not determine clock skew. | ||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,8 +50,14 @@ extension SignerMiddleware: ApplySigner { | |
) | ||
} | ||
|
||
// Check if CRT should be provided a pre-computed Sha256 SignedBodyValue | ||
var updatedSigningProperties = signingProperties | ||
|
||
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. Below, the clock skew store is checked for a clock skew value. If one exists, it is entered into signing properties. Then it will be applied when calculating the signature. |
||
// Look up & apply any applicable clock skew for this request | ||
if let clockSkew = await ClockSkewStore.shared.clockSkew(host: request.host) { | ||
updatedSigningProperties.set(key: AttributeKey(name: "ClockSkew"), value: clockSkew) | ||
} | ||
|
||
// Check if CRT should be provided a pre-computed Sha256 SignedBodyValue | ||
let sha256: String? = attributes.get(key: AttributeKey(name: "X-Amz-Content-Sha256")) | ||
if let bodyValue = sha256 { | ||
updatedSigningProperties.set(key: AttributeKey(name: "SignedBodyValue"), value: bodyValue) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
// | ||
|
||
import struct Foundation.Date | ||
import struct Foundation.TimeInterval | ||
import class Smithy.Context | ||
import enum Smithy.ClientError | ||
import protocol Smithy.RequestMessage | ||
|
@@ -76,6 +77,7 @@ public struct Orchestrator< | |
private let deserialize: (ResponseType, Context) async throws -> OutputType | ||
private let retryStrategy: (any RetryStrategy)? | ||
private let retryErrorInfoProvider: (Error) -> RetryErrorInfo? | ||
private let clockSkewProvider: ClockSkewProvider<RequestType, ResponseType> | ||
private let telemetry: OrchestratorTelemetry | ||
private let selectAuthScheme: SelectAuthScheme | ||
private let applyEndpoint: any ApplyEndpoint<RequestType> | ||
|
@@ -96,6 +98,12 @@ public struct Orchestrator< | |
self.retryErrorInfoProvider = { _ in nil } | ||
} | ||
|
||
if let clockSkewProvider = builder.clockSkewProvider { | ||
self.clockSkewProvider = clockSkewProvider | ||
} else { | ||
self.clockSkewProvider = { (_, _, _, _) in nil } | ||
} | ||
|
||
if let selectAuthScheme = builder.selectAuthScheme { | ||
self.selectAuthScheme = selectAuthScheme | ||
} else { | ||
|
@@ -262,9 +270,29 @@ public struct Orchestrator< | |
do { | ||
_ = try context.getOutput() | ||
await strategy.recordSuccess(token: token) | ||
} catch let error { | ||
// If we can't get errorInfo, we definitely can't retry | ||
guard let errorInfo = retryErrorInfoProvider(error) else { return } | ||
} catch { | ||
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. Below, the clock skew provider is called to obtain a clock skew value. If one is provided, it is recorded in the clock skew store and a retry will be performed (subject to if there are any retries remaining.) |
||
let clockSkewStore = ClockSkewStore.shared | ||
var clockSkewErrorInfo: RetryErrorInfo? | ||
|
||
// Clock skew can't be calculated when there is no request/response, so safe-unwrap them | ||
if let request = context.getRequest(), let response = context.getResponse() { | ||
// Assign clock skew to local var to prevent capturing self in block below | ||
let clockSkewProvider = self.clockSkewProvider | ||
// Check for clock skew, and if found, store in the shared map of hosts to clock skews | ||
let clockSkewDidChange = await clockSkewStore.setClockSkew(host: request.host) { @Sendable previous in | ||
clockSkewProvider(request, response, error, previous) | ||
} | ||
// Retry only if the new clock skew is different than previous. | ||
// If clock skew was unchanged on this errored request, then clock skew is likely not the | ||
// cause of the error | ||
if clockSkewDidChange { | ||
clockSkewErrorInfo = .clockSkewErrorInfo | ||
} | ||
} | ||
|
||
// If clock skew was found or has substantially changed, then retry on that | ||
// Else get errorInfo on the error | ||
guard let errorInfo = clockSkewErrorInfo ?? retryErrorInfoProvider(error) else { return } | ||
|
||
// If the body is a nonseekable stream, we also can't retry | ||
do { | ||
|
@@ -459,3 +487,11 @@ public struct Orchestrator< | |
} | ||
} | ||
} | ||
|
||
private extension RetryErrorInfo { | ||
|
||
/// `RetryErrorInfo` value used to signal that a retry should be performed due to clock skew. | ||
static var clockSkewErrorInfo: RetryErrorInfo { | ||
RetryErrorInfo(errorType: .clientError, retryAfterHint: nil, isTimeout: false) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import struct Foundation.Date | ||
import struct Foundation.TimeInterval | ||
import class Smithy.Context | ||
import class Smithy.ContextBuilder | ||
import protocol Smithy.RequestMessage | ||
|
@@ -33,6 +35,7 @@ public class OrchestratorBuilder< | |
internal var deserialize: ((ResponseType, Context) async throws -> OutputType)? | ||
internal var retryStrategy: (any RetryStrategy)? | ||
internal var retryErrorInfoProvider: ((Error) -> RetryErrorInfo?)? | ||
internal var clockSkewProvider: (ClockSkewProvider<RequestType, ResponseType>)? | ||
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. The |
||
internal var telemetry: OrchestratorTelemetry? | ||
internal var selectAuthScheme: SelectAuthScheme? | ||
internal var applyEndpoint: (any ApplyEndpoint<RequestType>)? | ||
|
@@ -105,6 +108,14 @@ public class OrchestratorBuilder< | |
return self | ||
} | ||
|
||
/// - Parameter clockSkewProvider: Function that turns operation errors into a clock skew value | ||
/// - Returns: Builder | ||
@discardableResult | ||
public func clockSkewProvider(_ clockSkewProvider: @escaping ClockSkewProvider<RequestType, ResponseType>) -> Self { | ||
self.clockSkewProvider = clockSkewProvider | ||
return self | ||
} | ||
|
||
/// - Parameter telemetry: container for telemetry | ||
/// - Returns: Builder | ||
@discardableResult | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
// | ||
|
||
/// Message that is sent from client to service. | ||
public protocol RequestMessage { | ||
public protocol RequestMessage: Sendable { | ||
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.
( |
||
|
||
/// The type of the builder that can build this request message. | ||
associatedtype RequestBuilderType: RequestMessageBuilder<Self> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ import protocol Smithy.RequestMessage | |
import protocol Smithy.RequestMessageBuilder | ||
import enum Smithy.ByteStream | ||
import enum Smithy.ClientError | ||
import struct Foundation.Date | ||
import struct Foundation.CharacterSet | ||
import struct Foundation.URLQueryItem | ||
import struct Foundation.URLComponents | ||
|
@@ -35,24 +36,27 @@ public final class HTTPRequest: RequestMessage, @unchecked Sendable { | |
public var path: String { destination.path } | ||
public var queryItems: [URIQueryItem]? { destination.queryItems } | ||
public var trailingHeaders: Headers = Headers() | ||
public var endpoint: Endpoint { | ||
return Endpoint(uri: self.destination, headers: self.headers) | ||
} | ||
public var endpoint: Endpoint { .init(uri: destination, headers: headers) } | ||
public internal(set) var signedAt: Date? | ||
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.
|
||
|
||
public convenience init(method: HTTPMethodType, | ||
endpoint: Endpoint, | ||
body: ByteStream = ByteStream.noStream) { | ||
self.init(method: method, uri: endpoint.uri, headers: endpoint.headers, body: body) | ||
} | ||
|
||
public init(method: HTTPMethodType, | ||
uri: URI, | ||
headers: Headers, | ||
body: ByteStream = ByteStream.noStream) { | ||
public init( | ||
method: HTTPMethodType, | ||
uri: URI, | ||
headers: Headers, | ||
body: ByteStream = ByteStream.noStream, | ||
signedAt: Date? = nil | ||
) { | ||
self.method = method | ||
self.destination = uri | ||
self.headers = headers | ||
self.body = body | ||
self.signedAt = signedAt | ||
} | ||
|
||
public func toBuilder() -> HTTPRequestBuilder { | ||
|
@@ -66,6 +70,7 @@ public final class HTTPRequest: RequestMessage, @unchecked Sendable { | |
.withPort(self.destination.port) | ||
.withProtocol(self.destination.scheme) | ||
.withQueryItems(self.destination.queryItems) | ||
.withSignedAt(signedAt) | ||
return builder | ||
} | ||
|
||
|
@@ -156,6 +161,7 @@ public final class HTTPRequestBuilder: RequestMessageBuilder { | |
public private(set) var port: UInt16? | ||
public private(set) var protocolType: URIScheme = .https | ||
public private(set) var trailingHeaders: Headers = Headers() | ||
public private(set) var signedAt: Date? | ||
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. Here, |
||
|
||
public var currentQueryItems: [URIQueryItem]? { | ||
return queryItems | ||
|
@@ -254,6 +260,12 @@ public final class HTTPRequestBuilder: RequestMessageBuilder { | |
return self | ||
} | ||
|
||
@discardableResult | ||
public func withSignedAt(_ value: Date?) -> HTTPRequestBuilder { | ||
self.signedAt = value | ||
return self | ||
} | ||
|
||
public func build() -> HTTPRequest { | ||
let uri = URIBuilder() | ||
.withScheme(protocolType) | ||
|
@@ -262,7 +274,7 @@ public final class HTTPRequestBuilder: RequestMessageBuilder { | |
.withPort(port) | ||
.withQueryItems(queryItems) | ||
.build() | ||
return HTTPRequest(method: methodType, uri: uri, headers: headers, body: body) | ||
return HTTPRequest(method: methodType, uri: uri, headers: headers, body: body, signedAt: signedAt) | ||
} | ||
} | ||
|
||
|
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.
Removed the CRT dependency from SmithyRetries since the only CRT references in that target were unused and are now removed.