More efficient s3 paging

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)
+}

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)
+	}
+}

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])
 		}
 	}
 

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

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]))
 		}
 	}
 

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]))
 		}
 	}
 

physical/s3.go

@@ -205,30 +205,35 @@ 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 {
 			if page != nil {
+				// Add truncated 'folder' paths
+				for _, commonPrefix := range page.CommonPrefixes {
+					// Avoid panic
+					if commonPrefix == nil {
+						continue
+					}
+
+					commonPrefix := strings.TrimPrefix(*commonPrefix.Prefix, prefix)
+					keys = append(keys, commonPrefix)
+				}
+				// Add objects only from the current 'folder'
 				for _, key := range page.Contents {
 					// Avoid panic
 					if key == nil {
 						continue
 					}
 
 					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
@@ -242,12 +247,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)
-}

physical/s3_test.go

@@ -7,23 +7,21 @@ 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) {
-	if os.Getenv("AWS_ACCESS_KEY_ID") == "" || os.Getenv("AWS_SECRET_ACCESS_KEY") == "" {
-		t.SkipNow()
-	}
+	credsConfig := &awsutil.CredentialsConfig{}
 
-	creds, err := credentials.NewEnvCredentials().Get()
+	credsChain, err := credsConfig.GenerateCredentialChain()
 	if err != nil {
-		t.Fatalf("err: %v", err)
+		t.SkipNow()
 	}
 
 	// If the variable is empty or doesn't exist, the default
@@ -36,7 +34,7 @@ func TestS3Backend(t *testing.T) {
 	}
 
 	s3conn := s3.New(session.New(&aws.Config{
-		Credentials: credentials.NewEnvCredentials(),
+		Credentials: credsChain,
 		Endpoint:    aws.String(endpoint),
 		Region:      aws.String(region),
 	}))
@@ -77,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":    creds.AccessKeyID,
-		"secret_key":    creds.SecretAccessKey,
-		"session_token": creds.SessionToken,
-		"bucket":        bucket,
+		"bucket": bucket,
 	})
 	if err != nil {
 		t.Fatalf("err: %s", err)

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])
 		}
 	}