From c16e0241422361fc66dc769661eecb5090b2662f Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 30 May 2017 12:23:17 +1000 Subject: [PATCH 1/7] Move appendIfMissing function to common location. The appendIfMissing function is used by several physical backends so move the function to a common file. --- physical/common.go | 10 ++++++++++ physical/common_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ physical/s3.go | 9 --------- 3 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 physical/common.go create mode 100644 physical/common_test.go diff --git a/physical/common.go b/physical/common.go new file mode 100644 index 000000000000..7013f954b0e8 --- /dev/null +++ b/physical/common.go @@ -0,0 +1,10 @@ +package physical + +func appendIfMissing(slice []string, i string) []string { + for _, ele := range slice { + if ele == i { + return slice + } + } + return append(slice, i) +} diff --git a/physical/common_test.go b/physical/common_test.go new file mode 100644 index 000000000000..e3dfeb4ce044 --- /dev/null +++ b/physical/common_test.go @@ -0,0 +1,40 @@ +package physical + +import "testing" + +func TestAppendIfMissing(t *testing.T) { + keys := []string{} + + keys = appendIfMissing(keys, "foo") + + if len(keys) != 1 { + t.Fatalf("expected slice to be length of 1: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to contain key 'foo': %v", keys) + } + + keys = appendIfMissing(keys, "bar") + + if len(keys) != 2 { + t.Fatalf("expected slice to be length of 2: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to contain key 'foo': %v", keys) + } + if keys[1] != "bar" { + t.Fatalf("expected slice to contain key 'bar': %v", keys) + } + + keys = appendIfMissing(keys, "foo") + + if len(keys) != 2 { + t.Fatalf("expected slice to still be length of 2: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to still contain key 'foo': %v", keys) + } + if keys[1] != "bar" { + t.Fatalf("expected slice to still contain key 'bar': %v", keys) + } +} diff --git a/physical/s3.go b/physical/s3.go index 8271be76d521..07263c17ac52 100644 --- a/physical/s3.go +++ b/physical/s3.go @@ -235,12 +235,3 @@ func (s *S3Backend) List(prefix string) ([]string, error) { return keys, nil } - -func appendIfMissing(slice []string, i string) []string { - for _, ele := range slice { - if ele == i { - return slice - } - } - return append(slice, i) -} From a81736e4479b39b1ebe8ea122c5f29ccc3952a85 Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 30 May 2017 12:26:10 +1000 Subject: [PATCH 2/7] Have S3 group folders before sending response. The ListObjectsV2 API allows you to specify a folder delimiter when listing objects in a bucket. This makes S3 group the folders together and only list the objects in the immediate prefix. For prefixes with a large number of objects underneath, this can speed up querying of intermediate paths significantly. --- physical/s3.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/physical/s3.go b/physical/s3.go index 07263c17ac52..66d333149aa9 100644 --- a/physical/s3.go +++ b/physical/s3.go @@ -205,24 +205,24 @@ func (s *S3Backend) List(prefix string) ([]string, error) { defer s.permitPool.Release() params := &s3.ListObjectsV2Input{ - Bucket: aws.String(s.bucket), - Prefix: aws.String(prefix), + Bucket: aws.String(s.bucket), + Prefix: aws.String(prefix), + Delimiter: aws.String("/"), } keys := []string{} err := s.client.ListObjectsV2Pages(params, func(page *s3.ListObjectsV2Output, lastPage bool) bool { + // Add truncated 'folder' paths + for _, commonPrefix := range page.CommonPrefixes { + commonPrefix := strings.TrimPrefix(*commonPrefix.Prefix, prefix) + keys = append(keys, commonPrefix) + } + // Add objects only from the current 'folder' for _, key := range page.Contents { key := strings.TrimPrefix(*key.Key, prefix) - - if i := strings.Index(key, "/"); i == -1 { - // Add objects only from the current 'folder' - keys = append(keys, key) - } else if i != -1 { - // Add truncated 'folder' paths - keys = appendIfMissing(keys, key[:i+1]) - } + keys = append(keys, key) } return true }) From 8f58b3d12e16ca325fe523df775e2509a5a7360b Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 30 May 2017 12:28:40 +1000 Subject: [PATCH 3/7] Allow S3 integration tests to use shared creds. This allows you to run the S3 integration tests with the ~/.aws/credentials file and AWS_PROFILE environment variable. --- physical/s3_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/physical/s3_test.go b/physical/s3_test.go index 8fdb882fb5a2..d2ed21a9e011 100644 --- a/physical/s3_test.go +++ b/physical/s3_test.go @@ -17,13 +17,15 @@ import ( ) func TestS3Backend(t *testing.T) { - if os.Getenv("AWS_ACCESS_KEY_ID") == "" || os.Getenv("AWS_SECRET_ACCESS_KEY") == "" { - t.SkipNow() - } + creds := credentials.NewChainCredentials( + []credentials.Provider{ + &credentials.EnvProvider{}, + &credentials.SharedCredentialsProvider{Filename: "", Profile: ""}, + }) - creds, err := credentials.NewEnvCredentials().Get() + credValue, err := creds.Get() if err != nil { - t.Fatalf("err: %v", err) + t.SkipNow() } // If the variable is empty or doesn't exist, the default @@ -36,7 +38,7 @@ func TestS3Backend(t *testing.T) { } s3conn := s3.New(session.New(&aws.Config{ - Credentials: credentials.NewEnvCredentials(), + Credentials: creds, Endpoint: aws.String(endpoint), Region: aws.String(region), })) @@ -78,9 +80,9 @@ func TestS3Backend(t *testing.T) { logger := logformat.NewVaultLogger(log.LevelTrace) b, err := NewBackend("s3", logger, map[string]string{ - "access_key": creds.AccessKeyID, - "secret_key": creds.SecretAccessKey, - "session_token": creds.SessionToken, + "access_key": credValue.AccessKeyID, + "secret_key": credValue.SecretAccessKey, + "session_token": credValue.SessionToken, "bucket": bucket, }) if err != nil { From 9dde16048090048443db635d6ffe6e192aa6b9bf Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 6 Jun 2017 10:07:52 +1000 Subject: [PATCH 4/7] Use the awsutil helper to find test creds --- physical/s3_test.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/physical/s3_test.go b/physical/s3_test.go index d2ed21a9e011..741e74f2dcda 100644 --- a/physical/s3_test.go +++ b/physical/s3_test.go @@ -7,23 +7,19 @@ import ( "testing" "time" + "github.com/hashicorp/vault/helper/awsutil" "github.com/hashicorp/vault/helper/logformat" log "github.com/mgutz/logxi/v1" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" ) func TestS3Backend(t *testing.T) { - creds := credentials.NewChainCredentials( - []credentials.Provider{ - &credentials.EnvProvider{}, - &credentials.SharedCredentialsProvider{Filename: "", Profile: ""}, - }) + credsConfig := &awsutil.CredentialsConfig{} - credValue, err := creds.Get() + credsChain, err := credsConfig.GenerateCredentialChain() if err != nil { t.SkipNow() } @@ -38,7 +34,7 @@ func TestS3Backend(t *testing.T) { } s3conn := s3.New(session.New(&aws.Config{ - Credentials: creds, + Credentials: credsChain, Endpoint: aws.String(endpoint), Region: aws.String(region), })) @@ -79,11 +75,9 @@ func TestS3Backend(t *testing.T) { logger := logformat.NewVaultLogger(log.LevelTrace) + // This uses the same logic to find the AWS credentials as we did at the beginning of the test b, err := NewBackend("s3", logger, map[string]string{ - "access_key": credValue.AccessKeyID, - "secret_key": credValue.SecretAccessKey, - "session_token": credValue.SessionToken, - "bucket": bucket, + "bucket": bucket, }) if err != nil { t.Fatalf("err: %s", err) From a51865ce4db7c6c1258b843cb148a21f518b5f0e Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 6 Jun 2017 11:44:26 +1000 Subject: [PATCH 5/7] Move appendIfMissing to helper/strutil --- helper/strutil/strutil.go | 8 +++++++ helper/strutil/strutil_test.go | 37 +++++++++++++++++++++++++++++++ physical/azure.go | 3 ++- physical/common.go | 10 --------- physical/common_test.go | 40 ---------------------------------- physical/etcd3.go | 3 ++- physical/mssql.go | 3 ++- physical/mysql.go | 3 ++- physical/swift.go | 3 ++- 9 files changed, 55 insertions(+), 55 deletions(-) delete mode 100644 physical/common.go delete mode 100644 physical/common_test.go diff --git a/helper/strutil/strutil.go b/helper/strutil/strutil.go index eaab5007bb13..b5e69c4f25a3 100644 --- a/helper/strutil/strutil.go +++ b/helper/strutil/strutil.go @@ -303,3 +303,11 @@ func GlobbedStringsMatch(item, val string) bool { return val == item } + +// AppendIfMissing adds a string to a slice if the given string is not present +func AppendIfMissing(slice []string, i string) []string { + if StrListContains(slice, i) { + return slice + } + return append(slice, i) +} diff --git a/helper/strutil/strutil_test.go b/helper/strutil/strutil_test.go index 5a76cd2ddab5..ce02719d1f5c 100644 --- a/helper/strutil/strutil_test.go +++ b/helper/strutil/strutil_test.go @@ -330,3 +330,40 @@ func TestTrimStrings(t *testing.T) { t.Fatalf("Bad TrimStrings: expected:%#v, got:%#v", expected, actual) } } + +func TestStrutil_AppendIfMissing(t *testing.T) { + keys := []string{} + + keys = AppendIfMissing(keys, "foo") + + if len(keys) != 1 { + t.Fatalf("expected slice to be length of 1: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to contain key 'foo': %v", keys) + } + + keys = AppendIfMissing(keys, "bar") + + if len(keys) != 2 { + t.Fatalf("expected slice to be length of 2: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to contain key 'foo': %v", keys) + } + if keys[1] != "bar" { + t.Fatalf("expected slice to contain key 'bar': %v", keys) + } + + keys = AppendIfMissing(keys, "foo") + + if len(keys) != 2 { + t.Fatalf("expected slice to still be length of 2: %v", keys) + } + if keys[0] != "foo" { + t.Fatalf("expected slice to still contain key 'foo': %v", keys) + } + if keys[1] != "bar" { + t.Fatalf("expected slice to still contain key 'bar': %v", keys) + } +} diff --git a/physical/azure.go b/physical/azure.go index 4d5083e71411..94a0024bf1ab 100644 --- a/physical/azure.go +++ b/physical/azure.go @@ -15,6 +15,7 @@ import ( "github.com/Azure/azure-storage-go" "github.com/armon/go-metrics" "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault/helper/strutil" ) // MaxBlobSize at this time @@ -181,7 +182,7 @@ func (a *AzureBackend) List(prefix string) ([]string, error) { if i := strings.Index(key, "/"); i == -1 { keys = append(keys, key) } else { - keys = appendIfMissing(keys, key[:i+1]) + keys = strutil.AppendIfMissing(keys, key[:i+1]) } } diff --git a/physical/common.go b/physical/common.go deleted file mode 100644 index 7013f954b0e8..000000000000 --- a/physical/common.go +++ /dev/null @@ -1,10 +0,0 @@ -package physical - -func appendIfMissing(slice []string, i string) []string { - for _, ele := range slice { - if ele == i { - return slice - } - } - return append(slice, i) -} diff --git a/physical/common_test.go b/physical/common_test.go deleted file mode 100644 index e3dfeb4ce044..000000000000 --- a/physical/common_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package physical - -import "testing" - -func TestAppendIfMissing(t *testing.T) { - keys := []string{} - - keys = appendIfMissing(keys, "foo") - - if len(keys) != 1 { - t.Fatalf("expected slice to be length of 1: %v", keys) - } - if keys[0] != "foo" { - t.Fatalf("expected slice to contain key 'foo': %v", keys) - } - - keys = appendIfMissing(keys, "bar") - - if len(keys) != 2 { - t.Fatalf("expected slice to be length of 2: %v", keys) - } - if keys[0] != "foo" { - t.Fatalf("expected slice to contain key 'foo': %v", keys) - } - if keys[1] != "bar" { - t.Fatalf("expected slice to contain key 'bar': %v", keys) - } - - keys = appendIfMissing(keys, "foo") - - if len(keys) != 2 { - t.Fatalf("expected slice to still be length of 2: %v", keys) - } - if keys[0] != "foo" { - t.Fatalf("expected slice to still contain key 'foo': %v", keys) - } - if keys[1] != "bar" { - t.Fatalf("expected slice to still contain key 'bar': %v", keys) - } -} diff --git a/physical/etcd3.go b/physical/etcd3.go index 6fecc7372061..7f86e4eed17e 100644 --- a/physical/etcd3.go +++ b/physical/etcd3.go @@ -15,6 +15,7 @@ import ( "github.com/coreos/etcd/clientv3" "github.com/coreos/etcd/clientv3/concurrency" "github.com/coreos/etcd/pkg/transport" + "github.com/hashicorp/vault/helper/strutil" log "github.com/mgutz/logxi/v1" "golang.org/x/net/context" ) @@ -204,7 +205,7 @@ func (c *EtcdBackend) List(prefix string) ([]string, error) { if i := strings.Index(key, "/"); i == -1 { keys = append(keys, key) } else if i != -1 { - keys = appendIfMissing(keys, key[:i+1]) + keys = strutil.AppendIfMissing(keys, key[:i+1]) } } return keys, nil diff --git a/physical/mssql.go b/physical/mssql.go index 25709a22b159..9764574b0e21 100644 --- a/physical/mssql.go +++ b/physical/mssql.go @@ -9,6 +9,7 @@ import ( "github.com/armon/go-metrics" _ "github.com/denisenkom/go-mssqldb" + "github.com/hashicorp/vault/helper/strutil" log "github.com/mgutz/logxi/v1" ) @@ -206,7 +207,7 @@ func (m *MsSQLBackend) List(prefix string) ([]string, error) { if i := strings.Index(key, "/"); i == -1 { keys = append(keys, key) } else if i != -1 { - keys = appendIfMissing(keys, string(key[:i+1])) + keys = strutil.AppendIfMissing(keys, string(key[:i+1])) } } diff --git a/physical/mysql.go b/physical/mysql.go index d063df4223aa..73aed84e54fc 100644 --- a/physical/mysql.go +++ b/physical/mysql.go @@ -17,6 +17,7 @@ import ( "github.com/armon/go-metrics" mysql "github.com/go-sql-driver/mysql" "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault/helper/strutil" ) // Unreserved tls key @@ -222,7 +223,7 @@ func (m *MySQLBackend) List(prefix string) ([]string, error) { keys = append(keys, key) } else if i != -1 { // Add truncated 'folder' paths - keys = appendIfMissing(keys, string(key[:i+1])) + keys = strutil.AppendIfMissing(keys, string(key[:i+1])) } } diff --git a/physical/swift.go b/physical/swift.go index eab8dc98cbc8..cff664e6fd4a 100644 --- a/physical/swift.go +++ b/physical/swift.go @@ -13,6 +13,7 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/errwrap" "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/vault/helper/strutil" "github.com/ncw/swift" ) @@ -207,7 +208,7 @@ func (s *SwiftBackend) List(prefix string) ([]string, error) { keys = append(keys, key) } else if i != -1 { // Add truncated 'folder' paths - keys = appendIfMissing(keys, key[:i+1]) + keys = strutil.AppendIfMissing(keys, key[:i+1]) } } From 9b6b9c1d42a06005c2b5badf89af93bc051a4a0b Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 6 Jun 2017 13:48:24 +1000 Subject: [PATCH 6/7] Check we can get credentials before starting test. --- physical/s3_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/physical/s3_test.go b/physical/s3_test.go index 741e74f2dcda..aec859d11601 100644 --- a/physical/s3_test.go +++ b/physical/s3_test.go @@ -24,6 +24,11 @@ func TestS3Backend(t *testing.T) { t.SkipNow() } + credsValue, err := credsChain.Get() + if err != nil { + t.skipNow() + } + // If the variable is empty or doesn't exist, the default // AWS endpoints will be used endpoint := os.Getenv("AWS_S3_ENDPOINT") From 9c1935e5852f8ace7060f6064fe6148791e17603 Mon Sep 17 00:00:00 2001 From: Dan Everton Date: Tue, 6 Jun 2017 13:55:56 +1000 Subject: [PATCH 7/7] Typo fix. --- physical/s3_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/physical/s3_test.go b/physical/s3_test.go index aec859d11601..7191cfec44ba 100644 --- a/physical/s3_test.go +++ b/physical/s3_test.go @@ -24,9 +24,9 @@ func TestS3Backend(t *testing.T) { t.SkipNow() } - credsValue, err := credsChain.Get() + _, err = credsChain.Get() if err != nil { - t.skipNow() + t.SkipNow() } // If the variable is empty or doesn't exist, the default