-
Notifications
You must be signed in to change notification settings - Fork 360
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
Upgrade code to use aws sdk go v2 #6486
Changes from 1 commit
3196b0d
30d8dc0
060799c
d85509c
d93e992
a14993e
d3527db
00a9f0c
51e994b
4f26ee9
b721eb5
ce41258
1ee0ddf
b5fbc02
c0081dd
6c01a0b
677f97e
e31bb51
f21425c
3521484
ae4be31
e28b0be
436e3f1
709a8ef
8182940
1a8300d
5840bf8
2036f81
39d01de
fce82bb
05f3ecd
01ff3fb
46fd9d3
999e2ad
9132dda
2cc3704
35c1836
c7f5bf8
e13681c
858b73a
1ccc476
19b787e
dc5ee48
ce58347
b7c67d9
4d0c693
0577667
e081750
9a0e575
cd7a211
433e52c
4940464
872dc6f
dcae21e
d6f1e39
914a53c
e712b20
955ac3f
204c1f4
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 |
---|---|---|
|
@@ -349,14 +349,25 @@ func (a *Adapter) GetWalker(uri *url.URL) (block.Walker, error) { | |
return NewS3Walker(a.clients.GetDefault()), nil | ||
} | ||
|
||
type CaptureExpiresPresigner struct { | ||
Presigner s3.HTTPPresignerV4 | ||
CredentialsCanExpire bool | ||
CredentialsExpireAt time.Time | ||
} | ||
|
||
func (c *CaptureExpiresPresigner) PresignHTTP(ctx context.Context, credentials aws.Credentials, r *http.Request, payloadHash string, service string, region string, signingTime time.Time, optFns ...func(*v4.SignerOptions)) (url string, signedHeader http.Header, err error) { | ||
// capture credentials expiry | ||
c.CredentialsCanExpire = credentials.CanExpire | ||
c.CredentialsExpireAt = credentials.Expires | ||
return c.Presigner.PresignHTTP(ctx, credentials, r, payloadHash, service, region, signingTime, optFns...) | ||
} | ||
|
||
func (a *Adapter) GetPreSignedURL(ctx context.Context, obj block.ObjectPointer, mode block.PreSignMode) (string, time.Time, error) { | ||
if a.disablePreSigned { | ||
return "", time.Time{}, block.ErrOperationNotSupported | ||
} | ||
|
||
// TODO(barak): handle expiry window of the client credentials when pre-signed | ||
// support enabled | ||
expiry := time.Now().Add(a.preSignedExpiry) | ||
expiry := time.Now().UTC().Add(a.preSignedExpiry) | ||
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. UTC is a timezone. The best kind of timezone, but still a timezone. Unless Golang defaults yo something line SQL TIMESTAMP (which is without TIMEZONE), I don't think UTC is needed or required here. 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. true, I will remove it. |
||
|
||
log := a.log(ctx).WithFields(logging.Fields{ | ||
"operation": "GetPreSignedURL", | ||
|
@@ -371,28 +382,39 @@ func (a *Adapter) GetPreSignedURL(ctx context.Context, obj block.ObjectPointer, | |
} | ||
|
||
client := a.clients.Get(ctx, bucket) | ||
presigner := s3.NewPresignClient(client, func(options *s3.PresignOptions) { | ||
options.Expires = a.preSignedExpiry | ||
}) | ||
presigner := s3.NewPresignClient(client, | ||
func(options *s3.PresignOptions) { | ||
options.Expires = a.preSignedExpiry | ||
}) | ||
|
||
captureExpiresPresigner := &CaptureExpiresPresigner{} | ||
var req *v4.PresignedHTTPRequest | ||
if mode == block.PreSignModeWrite { | ||
putObjectInput := &s3.PutObjectInput{ | ||
Bucket: aws.String(bucket), | ||
Key: aws.String(key), | ||
} | ||
req, err = presigner.PresignPutObject(ctx, putObjectInput) | ||
req, err = presigner.PresignPutObject(ctx, putObjectInput, func(o *s3.PresignOptions) { | ||
captureExpiresPresigner.Presigner = o.Presigner | ||
o.Presigner = captureExpiresPresigner | ||
}) | ||
} else { | ||
getObjectInput := &s3.GetObjectInput{ | ||
Bucket: aws.String(bucket), | ||
Key: aws.String(key), | ||
} | ||
req, err = presigner.PresignGetObject(ctx, getObjectInput) | ||
req, err = presigner.PresignGetObject(ctx, getObjectInput, func(o *s3.PresignOptions) { | ||
captureExpiresPresigner.Presigner = o.Presigner | ||
o.Presigner = captureExpiresPresigner | ||
}) | ||
} | ||
if err != nil { | ||
log.WithError(err).Error("could not pre-sign request") | ||
return "", time.Time{}, err | ||
} | ||
if captureExpiresPresigner.CredentialsCanExpire && captureExpiresPresigner.CredentialsExpireAt.Before(expiry) { | ||
expiry = captureExpiresPresigner.CredentialsExpireAt | ||
} | ||
return req.URL, expiry, nil | ||
} | ||
|
||
|
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.
I think this could just capture credentials and make them available later on. No need to run the processing logic here. I prefer to keep the signing path as simple and unmodified as possible.
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.
I was worried about capturing something that holds a state that can change and instead capture the same information the signer accepted.
Let me know if you think. if the risk is low I'll update the code.