Skip to content
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

Handwritten prototype of presign url client #880

Closed
wants to merge 1 commit into from

Conversation

skotambkar
Copy link
Contributor

Modifies existing RDS autofill custiomization to show case how we intend to expose pre-sign URL API Clients in the V2 SDK.

Only one operation CopyDBClusterSnapshot was changed (since the change is identical for other operations), we verified that the existing customization's middleware_test for the modified operation works. :)

There are few TODO's listed, that I would be glad to discuss about. Let me know your thoughts.

@skotambkar skotambkar requested a review from jasdel November 5, 2020 17:23
Comment on lines +204 to +210
type PresignClient struct {
// no need to export these members
// client used by presigner
client *Client
// signer used for presigning
presigner *v4.Signer
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially should use a PresignOptions struct to collect options for configuring the presign client. These would include things like the *Client, and the Presigner types as parameters.

type PresignOptions struct {
     Options    func(*Options)
     Presigner HTTPPresigner
}

@@ -378,3 +334,28 @@ func newServiceMetadataMiddleware_opCopyDBClusterSnapshot(region string) awsmidd
OperationName: "CopyDBClusterSnapshot",
}
}

// PresignCopyDBClusterSnapshot returns a Presigned HTTP request structure containing Request URL, method and SignedHeaders along with an error
func (c *PresignClient) PresignCopyDBClusterSnapshot(ctx context.Context, input *CopyDBClusterSnapshotInput, optFns ...func(*Options)) (v4.PresignedHTTPRequest, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For S3 operations that can use the Expires custom header for presign URL expiry, X-Amz-Expires.

presigner.PresignPutObject(ctx, input, func(o *rds.PresignOptions) {
     o.Presigner = v4.NewPresigner()
     o.OptionFns = append(o.OptionFns, func(o*Options){ o.Region = "us-west-2" })

     o.Expires = 20 * time.Minute // Only S3
})

Options for calling presign for an option

PresignPutObject(context.Context, *PutObjectInput, ...func(*PresignOption))

Could have helpers for presign option with expires that uses type alias of the value set to not have a allocation from a function.

func  WithPresignClientClientOptions(optFns ...func(*Options)) func(*PresignOptions) {
     return withPresignClientClientOptions(optFns).options
}
types withPresignClientClientOptions []func(*Options)
func (w withPresignClientClientOptions) option(o *PresignOptions) {
      o.ClientOptions = append(o.ClientOptions, w...)
}


func WithPresignExpires(dur time.Duration) func(*PresignOptions) {
      return withPresignExpires(dur).option
}
types withPresignExpires time.Duration
func (w withPresignExpires) option(o *PresignOptions) {
      o.Expires = time.Duration(w)
}

Comment on lines +212 to +220
// NewPresignClient returns a pre-signed client
func NewPresignClient(options Options, optFns ...func(*Options)) *PresignClient {
client := New(options, optFns...)
presigner := v4.NewSigner()
return &PresignClient{
client: client,
presigner: presigner,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If using the PresignOptions type with the constructor this would be adjust to take that value instead of API client's Options.

Taking PresignOption with optional client parameter.
client := rds.New(options)

// e.g. PresignClient creating it own API client with the options provided
presigner := rds.NewPresignClient(options,  func(o *rds.PresignOptions) {
     o.Presigner = v4.NewPresigner(/*signer options*/)
})

// Create presign client from API client with client options for region and presigner options
presigner := rds.NewPresignClientWithClient(client, func(o *rds.PresignOptions) {
     o.Presigner = v4.NewPresigner()
     o.OptionFns = append(o.OptionFns, func(o*Options){ o.Region = "us-west-2" })
})

// To use the presign client from a client with options, need to merge presign options together, before creating 
// copy of the client.
func NewPresignClientWithClient(client *Client, optFns ...func(*PresignOptions)) *PresignClient {
       var presignOptions PresignOptions
       for _, fn := range optFns {
            fn(&presignOptions)
       }

       client = client.copyAPIClient(presignOptions.OptionsFns....)
}

// If user calls NewPresignClientWithClient and provides functional options need to create
// a copy of the API client merging in the new options.
func (c *Client) copyAPIClient(optFns ...func(*Option)) *Client {
      return New(c.getAPIClientOptions(), optFns...)
}

// Create presign client from SDK configuration
presigner := rds.NewPresignClientFromConfig(cfg, func(o *rds.PresignOptions){
     o.Presigner = v4.NewPresigner()
})

// e.g. PresignClient given options, and optional presigner,
presigner := rds.NewPresignClient(options, func(o *rds.PresignOptions){
     o.Presigner = v4.NewPresigner()
})

@skotambkar
Copy link
Contributor Author

Replaced by #888

@skotambkar skotambkar closed this Nov 11, 2020
@skotambkar skotambkar deleted the presigner-init branch January 15, 2021 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants