Skip to content

Commit 6d365fb

Browse files
MadrigalMadrigal
committed
address feedback from pr, mostly moving things around
1 parent 49e7140 commit 6d365fb

File tree

13 files changed

+407
-378
lines changed

13 files changed

+407
-378
lines changed

‎aws/middleware/middleware.go

-56
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package middleware
33
import (
44
"context"
55
"fmt"
6-
"sync/atomic"
76
"time"
87

98
"github.com/aws/aws-sdk-go-v2/internal/rand"
@@ -125,19 +124,6 @@ func setAttemptSkew(metadata *middleware.Metadata, v time.Duration) {
125124
metadata.Set(attemptSkewKey{}, v)
126125
}
127126

128-
type clockSkew struct{}
129-
130-
// SetAttemptSkewContext sets the clock skew value on the context
131-
func SetAttemptSkewContext(ctx context.Context, v time.Duration) context.Context {
132-
return middleware.WithStackValue(ctx, clockSkew{}, v)
133-
}
134-
135-
// GetAttemptSkewContext gets the clock skew value from the context
136-
func GetAttemptSkewContext(ctx context.Context) time.Duration {
137-
x, _ := middleware.GetStackValue(ctx, clockSkew{}).(time.Duration)
138-
return x
139-
}
140-
141127
// AddClientRequestIDMiddleware adds ClientRequestID to the middleware stack
142128
func AddClientRequestIDMiddleware(stack *middleware.Stack) error {
143129
return stack.Build.Add(&ClientRequestID{}, middleware.After)
@@ -180,45 +166,3 @@ func AddRawResponseToMetadata(stack *middleware.Stack) error {
180166
func GetRawResponse(metadata middleware.Metadata) interface{} {
181167
return metadata.Get(rawResponseKey{})
182168
}
183-
184-
// AddTimeOffsetBuildMiddleware sets a value representing clock skew on the request context.
185-
// This can be read by other operations (such as signing) to correct the date value they send
186-
// on the request
187-
type AddTimeOffsetBuildMiddleware struct {
188-
Offset *atomic.Int64
189-
}
190-
191-
// ID the identifier for AddTimeOffsetBuildMiddleware
192-
func (m *AddTimeOffsetBuildMiddleware) ID() string { return "AddTimeOffsetMiddleware" }
193-
194-
// HandleBuild sets a value for attemptSkew on the request context if one is set on the client.
195-
func (m AddTimeOffsetBuildMiddleware) HandleBuild(ctx context.Context, in middleware.BuildInput, next middleware.BuildHandler) (
196-
out middleware.BuildOutput, metadata middleware.Metadata, err error,
197-
) {
198-
if m.Offset != nil {
199-
offset := time.Duration(m.Offset.Load())
200-
ctx = SetAttemptSkewContext(ctx, offset)
201-
}
202-
return next.HandleBuild(ctx, in)
203-
}
204-
205-
// AddTimeOffsetDeserializeMiddleware sets the clock skew on the client if it's present on the context
206-
// at the end of the request
207-
type AddTimeOffsetDeserializeMiddleware struct {
208-
Offset *atomic.Int64
209-
}
210-
211-
// ID the identifier for AddTimeOffsetDeserializeMiddleware
212-
func (m *AddTimeOffsetDeserializeMiddleware) ID() string { return "AddTimeOffsetDeserializeMiddleware" }
213-
214-
// HandleDeserialize gets the clock skew context from the context, and if set, sets it on the pointer
215-
// held by AddTimeOffsetDeserializeMiddleware
216-
func (m *AddTimeOffsetDeserializeMiddleware) HandleDeserialize(ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler) (
217-
out middleware.DeserializeOutput, metadata middleware.Metadata, err error,
218-
) {
219-
v := GetAttemptSkewContext(ctx)
220-
if v != 0 {
221-
m.Offset.Store(v.Nanoseconds())
222-
}
223-
return next.HandleDeserialize(ctx, in)
224-
}

‎aws/middleware/middleware_test.go

-240
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,9 @@ package middleware_test
33
import (
44
"bytes"
55
"context"
6-
"fmt"
7-
"github.com/aws/aws-sdk-go-v2/aws"
8-
"github.com/aws/aws-sdk-go-v2/aws/retry"
96
"net/http"
107
"reflect"
118
"strings"
12-
"sync/atomic"
139
"testing"
1410
"time"
1511

@@ -191,239 +187,3 @@ func TestAttemptClockSkewHandler(t *testing.T) {
191187
})
192188
}
193189
}
194-
195-
type HTTPClient interface {
196-
Do(*http.Request) (*http.Response, error)
197-
}
198-
199-
type Options struct {
200-
HTTPClient HTTPClient
201-
RetryMode aws.RetryMode
202-
Retryer aws.Retryer
203-
Offset *atomic.Int64
204-
}
205-
206-
type MockClient struct {
207-
options Options
208-
}
209-
210-
func addRetry(stack *smithymiddleware.Stack, o Options) error {
211-
attempt := retry.NewAttemptMiddleware(o.Retryer, smithyhttp.RequestCloner, func(m *retry.Attempt) {
212-
m.LogAttempts = false
213-
})
214-
return stack.Finalize.Add(attempt, smithymiddleware.After)
215-
}
216-
217-
func addOffset(stack *smithymiddleware.Stack, o Options) error {
218-
buildOffset := middleware.AddTimeOffsetBuildMiddleware{Offset: o.Offset}
219-
deserializeOffset := middleware.AddTimeOffsetDeserializeMiddleware{Offset: o.Offset}
220-
err := stack.Build.Add(&buildOffset, smithymiddleware.After)
221-
if err != nil {
222-
return err
223-
}
224-
err = stack.Deserialize.Add(&deserializeOffset, smithymiddleware.Before)
225-
if err != nil {
226-
return err
227-
}
228-
return nil
229-
}
230-
231-
// Middleware to set a `Date` object that includes sdk time and offset
232-
type MockAddDateHeader struct {
233-
}
234-
235-
func (l *MockAddDateHeader) ID() string {
236-
return "MockAddDateHeader"
237-
}
238-
239-
func (l *MockAddDateHeader) HandleFinalize(
240-
ctx context.Context, in smithymiddleware.FinalizeInput, next smithymiddleware.FinalizeHandler,
241-
) (
242-
out smithymiddleware.FinalizeOutput, metadata smithymiddleware.Metadata, attemptError error,
243-
) {
244-
req := in.Request.(*smithyhttp.Request)
245-
date := sdk.NowTime()
246-
skew := middleware.GetAttemptSkewContext(ctx)
247-
date = date.Add(skew)
248-
req.Header.Set("Date", date.Format(time.RFC850))
249-
return next.HandleFinalize(ctx, in)
250-
}
251-
252-
// Middleware to deserialize the response which just says "OK" if the response is 200
253-
type DeserializeFailIfNotHTTP200 struct {
254-
}
255-
256-
func (*DeserializeFailIfNotHTTP200) ID() string {
257-
return "DeserializeFailIfNotHTTP200"
258-
}
259-
260-
func (m *DeserializeFailIfNotHTTP200) HandleDeserialize(ctx context.Context, in smithymiddleware.DeserializeInput, next smithymiddleware.DeserializeHandler) (
261-
out smithymiddleware.DeserializeOutput, metadata smithymiddleware.Metadata, err error,
262-
) {
263-
out, metadata, err = next.HandleDeserialize(ctx, in)
264-
if err != nil {
265-
return out, metadata, err
266-
}
267-
response, ok := out.RawResponse.(*smithyhttp.Response)
268-
if !ok {
269-
return out, metadata, fmt.Errorf("expected raw response to be set on testing")
270-
}
271-
if response.StatusCode != 200 {
272-
return out, metadata, mockRetryableError{true}
273-
}
274-
return out, metadata, err
275-
}
276-
277-
func (c *MockClient) setupMiddleware(stack *smithymiddleware.Stack) error {
278-
err := error(nil)
279-
if c.options.Retryer != nil {
280-
err = addRetry(stack, c.options)
281-
if err != nil {
282-
return err
283-
}
284-
}
285-
if c.options.Offset != nil {
286-
err = addOffset(stack, c.options)
287-
if err != nil {
288-
return err
289-
}
290-
}
291-
err = stack.Finalize.Add(&MockAddDateHeader{}, smithymiddleware.After)
292-
if err != nil {
293-
return err
294-
}
295-
err = middleware.AddRecordResponseTiming(stack)
296-
if err != nil {
297-
return err
298-
}
299-
err = stack.Deserialize.Add(&DeserializeFailIfNotHTTP200{}, smithymiddleware.After)
300-
if err != nil {
301-
return err
302-
}
303-
return nil
304-
}
305-
306-
func (c *MockClient) Do(ctx context.Context) (interface{}, error) {
307-
// setup middlewares
308-
ctx = smithymiddleware.ClearStackValues(ctx)
309-
stack := smithymiddleware.NewStack("stack", smithyhttp.NewStackRequest)
310-
err := c.setupMiddleware(stack)
311-
if err != nil {
312-
return nil, err
313-
}
314-
handler := smithymiddleware.DecorateHandler(smithyhttp.NewClientHandler(c.options.HTTPClient), stack)
315-
result, _, err := handler.Handle(ctx, 1)
316-
if err != nil {
317-
return nil, err
318-
}
319-
return result, err
320-
}
321-
322-
type mockRetryableError struct{ b bool }
323-
324-
func (m mockRetryableError) RetryableError() bool { return m.b }
325-
func (m mockRetryableError) Error() string {
326-
return fmt.Sprintf("mock retryable %t", m.b)
327-
}
328-
329-
func failRequestIfSkewed() smithyhttp.ClientDoFunc {
330-
return func(req *http.Request) (*http.Response, error) {
331-
dateHeader := req.Header.Get("Date")
332-
if dateHeader == "" {
333-
return nil, fmt.Errorf("expected `Date` header to be set")
334-
}
335-
reqDate, err := time.Parse(time.RFC850, dateHeader)
336-
if err != nil {
337-
return nil, err
338-
}
339-
parsedReqTime := time.Now().Sub(reqDate)
340-
parsedReqTime = time.Duration.Abs(parsedReqTime)
341-
thresholdForSkewError := 4 * time.Minute
342-
if thresholdForSkewError-parsedReqTime <= 0 {
343-
return &http.Response{
344-
StatusCode: 403,
345-
Header: http.Header{
346-
"Date": {time.Now().Format(time.RFC850)},
347-
},
348-
}, nil
349-
}
350-
// else, return OK
351-
return &http.Response{
352-
StatusCode: 200,
353-
Header: http.Header{},
354-
}, nil
355-
}
356-
}
357-
358-
func TestSdkOffsetIsSet(t *testing.T) {
359-
nowTime := sdk.NowTime
360-
defer func() {
361-
sdk.NowTime = nowTime
362-
}()
363-
fiveMinuteSkew := func() time.Time {
364-
return time.Now().Add(5 * time.Minute)
365-
}
366-
sdk.NowTime = fiveMinuteSkew
367-
c := MockClient{
368-
Options{
369-
HTTPClient: failRequestIfSkewed(),
370-
},
371-
}
372-
resp, err := c.Do(context.Background())
373-
if err == nil {
374-
t.Errorf("Expected first request to fail since clock skew logic has not run. Got %v and err %v", resp, err)
375-
}
376-
}
377-
378-
func TestRetrySetsSkewInContext(t *testing.T) {
379-
defer resetDefaults(sdk.TestingUseNopSleep())
380-
fiveMinuteSkew := func() time.Time {
381-
return time.Now().Add(5 * time.Minute)
382-
}
383-
sdk.NowTime = fiveMinuteSkew
384-
c := MockClient{
385-
Options{
386-
HTTPClient: failRequestIfSkewed(),
387-
Retryer: retry.NewStandard(func(s *retry.StandardOptions) {
388-
}),
389-
},
390-
}
391-
resp, err := c.Do(context.Background())
392-
if err != nil {
393-
t.Errorf("Expected request to succeed on retry. Got %v and err %v", resp, err)
394-
}
395-
}
396-
397-
func TestSkewIsSetOnTheWholeClient(t *testing.T) {
398-
defer resetDefaults(sdk.TestingUseNopSleep())
399-
fiveMinuteSkew := func() time.Time {
400-
return time.Now().Add(5 * time.Minute)
401-
}
402-
sdk.NowTime = fiveMinuteSkew
403-
var offset atomic.Int64
404-
offset.Store(0)
405-
c := MockClient{
406-
Options{
407-
HTTPClient: failRequestIfSkewed(),
408-
Retryer: retry.NewStandard(func(s *retry.StandardOptions) {
409-
}),
410-
Offset: &offset,
411-
},
412-
}
413-
resp, err := c.Do(context.Background())
414-
if err != nil {
415-
t.Errorf("Expected request to succeed on retry. Got %v and err %v", resp, err)
416-
}
417-
// Remove retryer so it has to succeed on first call
418-
c.options.Retryer = nil
419-
// same client, new request
420-
resp, err = c.Do(context.Background())
421-
if err != nil {
422-
t.Errorf("Expected second request to succeed since the skew should be set on the client. Got %v and err %v", resp, err)
423-
}
424-
}
425-
426-
func resetDefaults(restoreSleepFunc func()) {
427-
sdk.NowTime = time.Now
428-
restoreSleepFunc()
429-
}

0 commit comments

Comments
 (0)