From 031c388543f5159ffbd22bebb4b44a984cd47db0 Mon Sep 17 00:00:00 2001 From: Andrey Kostov Date: Mon, 29 Dec 2014 22:29:54 +0200 Subject: [PATCH 1/4] Add the secure flag for the s3 driver The secure flag will be true by default and will change the s3 endpoint of the region to http instead of https when selected as false. The main benefits of running with secure being false is that it apparently has a roughly 33% performance boost (even on pure data transfer, not only connection setup which is what I would have expected). --- storagedriver/s3/s3.go | 19 +++++++++++++++++-- storagedriver/s3/s3_test.go | 13 +++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/storagedriver/s3/s3.go b/storagedriver/s3/s3.go index ae036d47..e8bebd00 100644 --- a/storagedriver/s3/s3.go +++ b/storagedriver/s3/s3.go @@ -96,22 +96,37 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { } } + secureBool := false + secure, ok := parameters["secure"] + if !ok { + secureBool = true + } else { + secureBool, ok = secure.(bool) + if !ok { + return nil, fmt.Errorf("The secure parameter should be a boolean") + } + } + rootDirectory, ok := parameters["rootdirectory"] if !ok { rootDirectory = "" } - return New(fmt.Sprint(accessKey), fmt.Sprint(secretKey), fmt.Sprint(bucket), fmt.Sprint(rootDirectory), region, encryptBool) + return New(fmt.Sprint(accessKey), fmt.Sprint(secretKey), fmt.Sprint(bucket), fmt.Sprint(rootDirectory), region, encryptBool, secureBool) } // New constructs a new Driver with the given AWS credentials, region, encryption flag, and // bucketName -func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Region, encrypt bool) (*Driver, error) { +func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Region, encrypt, secure bool) (*Driver, error) { auth, err := aws.GetAuth(accessKey, secretKey, "", time.Time{}) if err != nil { return nil, err } + if !secure { + region.S3Endpoint = strings.Replace(region.S3Endpoint, "https", "http", 1) + } + s3obj := s3.New(auth, region) bucket := s3obj.Bucket(bucketName) diff --git a/storagedriver/s3/s3_test.go b/storagedriver/s3/s3_test.go index aaf7e5bd..1addf13b 100644 --- a/storagedriver/s3/s3_test.go +++ b/storagedriver/s3/s3_test.go @@ -21,6 +21,7 @@ func init() { secretKey := os.Getenv("AWS_SECRET_KEY") bucket := os.Getenv("S3_BUCKET") encrypt := os.Getenv("S3_ENCRYPT") + secure := os.Getenv("S3_SECURE") region := os.Getenv("AWS_REGION") root, err := ioutil.TempDir("", "driver-") if err != nil { @@ -28,11 +29,19 @@ func init() { } s3DriverConstructor := func(region aws.Region) (storagedriver.StorageDriver, error) { - shouldEncrypt, err := strconv.ParseBool(encrypt) + encryptBool, err := strconv.ParseBool(encrypt) if err != nil { return nil, err } - return New(accessKey, secretKey, bucket, root, region, shouldEncrypt) + + secureBool := true + if secure != "" { + secureBool, err = strconv.ParseBool(secure) + if err != nil { + return nil, err + } + } + return New(accessKey, secretKey, bucket, root, region, encryptBool, secureBool) } // Skip S3 storage driver tests if environment variable parameters are not provided From a0ef0d6aad7397ece93beed589bc83bccf5a8684 Mon Sep 17 00:00:00 2001 From: Andrey Kostov Date: Wed, 7 Jan 2015 11:45:31 +0200 Subject: [PATCH 2/4] Add the v4auth parameter v4auth will default to true and if the frankfurt (eu-central-1) region is selected with v4auth set to false explicitly, the driver will error out upon initialization. --- storagedriver/s3/README.md | 2 ++ storagedriver/s3/s3.go | 29 ++++++++++++++++++++++------- storagedriver/s3/s3_test.go | 20 ++++++++++++++++---- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/storagedriver/s3/README.md b/storagedriver/s3/README.md index 5a0cd03e..ac99d88d 100644 --- a/storagedriver/s3/README.md +++ b/storagedriver/s3/README.md @@ -19,4 +19,6 @@ An implementation of the `storagedriver.StorageDriver` interface which uses Amaz `secure`: (optional) Whether you would like to transfer data over ssl or not. Defaults to true (meaning transfering over ssl) if not specified. Note that while setting this to false will improve performance, it is not recommended due to security concerns. +`v4auth`: (optional) Whether you would like to use aws signature version 4 with your requests. This defaults to true if not specified (note that the eu-central-1 region does not work with version 2 signatures, so the driver will error out if initialized with this region and v4auth set to false) + `rootdirectory`: (optional) The root directory tree in which all registry files will be stored. Defaults to the empty string (bucket root). \ No newline at end of file diff --git a/storagedriver/s3/s3.go b/storagedriver/s3/s3.go index e8bebd00..a2a6758b 100644 --- a/storagedriver/s3/s3.go +++ b/storagedriver/s3/s3.go @@ -96,28 +96,35 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { } } - secureBool := false + secureBool := true secure, ok := parameters["secure"] - if !ok { - secureBool = true - } else { + if ok { secureBool, ok = secure.(bool) if !ok { return nil, fmt.Errorf("The secure parameter should be a boolean") } } + v4AuthBool := true + v4Auth, ok := parameters["v4auth"] + if ok { + v4AuthBool, ok = v4Auth.(bool) + if !ok { + return nil, fmt.Errorf("The v4auth parameter should be a boolean") + } + } + rootDirectory, ok := parameters["rootdirectory"] if !ok { rootDirectory = "" } - return New(fmt.Sprint(accessKey), fmt.Sprint(secretKey), fmt.Sprint(bucket), fmt.Sprint(rootDirectory), region, encryptBool, secureBool) + return New(fmt.Sprint(accessKey), fmt.Sprint(secretKey), fmt.Sprint(bucket), fmt.Sprint(rootDirectory), region, encryptBool, secureBool, v4AuthBool) } // New constructs a new Driver with the given AWS credentials, region, encryption flag, and // bucketName -func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Region, encrypt, secure bool) (*Driver, error) { +func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Region, encrypt, secure, v4auth bool) (*Driver, error) { auth, err := aws.GetAuth(accessKey, secretKey, "", time.Time{}) if err != nil { return nil, err @@ -130,6 +137,14 @@ func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Regi s3obj := s3.New(auth, region) bucket := s3obj.Bucket(bucketName) + if v4auth { + s3obj.Signature = aws.V4Signature + } else { + if region.Name == "eu-central-1" { + return nil, fmt.Errorf("The eu-central-1 region only works with v4 authentication") + } + } + if _, err := bucket.List("", "", "", 1); err != nil { return nil, err } @@ -428,7 +443,7 @@ func (d *Driver) WriteStream(path string, offset int64, reader io.Reader) (total } else { // offset > currentLength >= chunkSize _, part, err = multi.PutPartCopy(partNumber, - s3.CopyOptions{CopySourceOptions: "bytes=0-" + strconv.FormatInt(currentLength-1, 10)}, + s3.CopyOptions{}, d.Bucket.Name+"/"+d.s3Path(path)) if err != nil { return 0, err diff --git a/storagedriver/s3/s3_test.go b/storagedriver/s3/s3_test.go index 1addf13b..e3c3e535 100644 --- a/storagedriver/s3/s3_test.go +++ b/storagedriver/s3/s3_test.go @@ -22,6 +22,7 @@ func init() { bucket := os.Getenv("S3_BUCKET") encrypt := os.Getenv("S3_ENCRYPT") secure := os.Getenv("S3_SECURE") + v4auth := os.Getenv("S3_USE_V4_AUTH") region := os.Getenv("AWS_REGION") root, err := ioutil.TempDir("", "driver-") if err != nil { @@ -29,9 +30,12 @@ func init() { } s3DriverConstructor := func(region aws.Region) (storagedriver.StorageDriver, error) { - encryptBool, err := strconv.ParseBool(encrypt) - if err != nil { - return nil, err + encryptBool := true + if encrypt != "" { + encryptBool, err = strconv.ParseBool(encrypt) + if err != nil { + return nil, err + } } secureBool := true @@ -41,7 +45,15 @@ func init() { return nil, err } } - return New(accessKey, secretKey, bucket, root, region, encryptBool, secureBool) + + v4AuthBool := true + if v4auth != "" { + v4AuthBool, err = strconv.ParseBool(v4auth) + if err != nil { + return nil, err + } + } + return New(accessKey, secretKey, bucket, root, region, encryptBool, secureBool, v4AuthBool) } // Skip S3 storage driver tests if environment variable parameters are not provided From 7c9112fc3cdb237e45f17a6ca09ba2f9245a6806 Mon Sep 17 00:00:00 2001 From: Andrey Kostov Date: Wed, 7 Jan 2015 11:51:29 +0200 Subject: [PATCH 3/4] Make the encrpyt parameter default to false --- storagedriver/s3/README.md | 2 +- storagedriver/s3/s3.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/storagedriver/s3/README.md b/storagedriver/s3/README.md index ac99d88d..58f01da2 100644 --- a/storagedriver/s3/README.md +++ b/storagedriver/s3/README.md @@ -15,7 +15,7 @@ An implementation of the `storagedriver.StorageDriver` interface which uses Amaz `bucket`: The name of your s3 bucket where you wish to store objects (needs to already be created prior to driver initialization). -`encrypt`: (optional) Whether you would like your data encrypted on the server side (defaults to true if not specified). +`encrypt`: (optional) Whether you would like your data encrypted on the server side (defaults to false if not specified). `secure`: (optional) Whether you would like to transfer data over ssl or not. Defaults to true (meaning transfering over ssl) if not specified. Note that while setting this to false will improve performance, it is not recommended due to security concerns. diff --git a/storagedriver/s3/s3.go b/storagedriver/s3/s3.go index a2a6758b..3ef55ce1 100644 --- a/storagedriver/s3/s3.go +++ b/storagedriver/s3/s3.go @@ -87,7 +87,7 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { return nil, fmt.Errorf("No bucket parameter provided") } - encryptBool := true + encryptBool := false encrypt, ok := parameters["encrypt"] if ok { encryptBool, ok = encrypt.(bool) From bc9509d85f5c542932d5312c3838a37573f7c48c Mon Sep 17 00:00:00 2001 From: Andrey Kostov Date: Wed, 7 Jan 2015 12:18:42 +0200 Subject: [PATCH 4/4] Consolidate the s3 driver parameters with a struct --- storagedriver/s3/s3.go | 43 ++++++++++++++++++++++++++++--------- storagedriver/s3/s3_test.go | 17 +++++++++++++-- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/storagedriver/s3/s3.go b/storagedriver/s3/s3.go index 3ef55ce1..e7c6cb09 100644 --- a/storagedriver/s3/s3.go +++ b/storagedriver/s3/s3.go @@ -39,6 +39,18 @@ const chunkSize = 5 * 1024 * 1024 // listMax is the largest amount of objects you can request from S3 in a list call const listMax = 1000 +//DriverParameters A struct that encapsulates all of the driver parameters after all values have been set +type DriverParameters struct { + AccessKey string + SecretKey string + Bucket string + Region aws.Region + Encrypt bool + Secure bool + V4Auth bool + RootDirectory string +} + func init() { factory.Register(driverName, &s3DriverFactory{}) } @@ -119,28 +131,39 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { rootDirectory = "" } - return New(fmt.Sprint(accessKey), fmt.Sprint(secretKey), fmt.Sprint(bucket), fmt.Sprint(rootDirectory), region, encryptBool, secureBool, v4AuthBool) + params := DriverParameters{ + fmt.Sprint(accessKey), + fmt.Sprint(secretKey), + fmt.Sprint(bucket), + region, + encryptBool, + secureBool, + v4AuthBool, + fmt.Sprint(rootDirectory), + } + + return New(params) } // New constructs a new Driver with the given AWS credentials, region, encryption flag, and // bucketName -func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Region, encrypt, secure, v4auth bool) (*Driver, error) { - auth, err := aws.GetAuth(accessKey, secretKey, "", time.Time{}) +func New(params DriverParameters) (*Driver, error) { + auth, err := aws.GetAuth(params.AccessKey, params.SecretKey, "", time.Time{}) if err != nil { return nil, err } - if !secure { - region.S3Endpoint = strings.Replace(region.S3Endpoint, "https", "http", 1) + if !params.Secure { + params.Region.S3Endpoint = strings.Replace(params.Region.S3Endpoint, "https", "http", 1) } - s3obj := s3.New(auth, region) - bucket := s3obj.Bucket(bucketName) + s3obj := s3.New(auth, params.Region) + bucket := s3obj.Bucket(params.Bucket) - if v4auth { + if params.V4Auth { s3obj.Signature = aws.V4Signature } else { - if region.Name == "eu-central-1" { + if params.Region.Name == "eu-central-1" { return nil, fmt.Errorf("The eu-central-1 region only works with v4 authentication") } } @@ -164,7 +187,7 @@ func New(accessKey, secretKey, bucketName, rootDirectory string, region aws.Regi // } // } - return &Driver{s3obj, bucket, encrypt, rootDirectory}, nil + return &Driver{s3obj, bucket, params.Encrypt, params.RootDirectory}, nil } // Implement the storagedriver.StorageDriver interface diff --git a/storagedriver/s3/s3_test.go b/storagedriver/s3/s3_test.go index e3c3e535..f70080f2 100644 --- a/storagedriver/s3/s3_test.go +++ b/storagedriver/s3/s3_test.go @@ -28,9 +28,10 @@ func init() { if err != nil { panic(err) } + defer os.Remove(root) s3DriverConstructor := func(region aws.Region) (storagedriver.StorageDriver, error) { - encryptBool := true + encryptBool := false if encrypt != "" { encryptBool, err = strconv.ParseBool(encrypt) if err != nil { @@ -53,7 +54,19 @@ func init() { return nil, err } } - return New(accessKey, secretKey, bucket, root, region, encryptBool, secureBool, v4AuthBool) + + parameters := DriverParameters{ + accessKey, + secretKey, + bucket, + region, + encryptBool, + secureBool, + v4AuthBool, + root, + } + + return New(parameters) } // Skip S3 storage driver tests if environment variable parameters are not provided