Clean AlibabaCloud physical backend code (#8186)

physical/alicloudoss/alicloudoss.go

@@ -5,19 +5,29 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"sort"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/aliyun/aliyun-oss-go-sdk/oss"
-	metrics "github.com/armon/go-metrics"
+	"github.com/armon/go-metrics"
 	"github.com/hashicorp/errwrap"
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/sdk/physical"
 )
 
+const (
+	AlibabaMetricKey = "alibaba"
+
+	AlibabaCloudOSSEndpointEnv = "ALICLOUD_OSS_ENDPOINT"
+	AlibabaCloudOSSBucketEnv   = "ALICLOUD_OSS_BUCKET"
+	AlibabaCloudAccessKeyEnv   = "ALICLOUD_ACCESS_KEY"
+	AlibabaCloudSecretKeyEnv   = "ALICLOUD_SECRET_KEY"
+)
+
 // Verify AliCloudOSSBackend satisfies the correct interfaces
 var _ physical.Backend = (*AliCloudOSSBackend)(nil)
 
@@ -34,31 +44,31 @@ type AliCloudOSSBackend struct {
 // bucket. Credentials can be provided to the backend, sourced
 // from the environment.
 func NewAliCloudOSSBackend(conf map[string]string, logger log.Logger) (physical.Backend, error) {
-	endpoint := os.Getenv("ALICLOUD_OSS_ENDPOINT")
+	endpoint := os.Getenv(AlibabaCloudOSSEndpointEnv)
 	if endpoint == "" {
 		endpoint = conf["endpoint"]
 		if endpoint == "" {
 			return nil, fmt.Errorf("'endpoint' must be set")
 		}
 	}
 
-	bucket := os.Getenv("ALICLOUD_OSS_BUCKET")
+	bucket := os.Getenv(AlibabaCloudOSSBucketEnv)
 	if bucket == "" {
 		bucket = conf["bucket"]
 		if bucket == "" {
 			return nil, fmt.Errorf("'bucket' must be set")
 		}
 	}
 
-	accessKeyID := os.Getenv("ALICLOUD_ACCESS_KEY")
+	accessKeyID := os.Getenv(AlibabaCloudAccessKeyEnv)
 	if accessKeyID == "" {
 		accessKeyID = conf["access_key"]
 		if accessKeyID == "" {
 			return nil, fmt.Errorf("'access_key' must be set")
 		}
 	}
 
-	accessKeySecret := os.Getenv("ALICLOUD_SECRET_KEY")
+	accessKeySecret := os.Getenv(AlibabaCloudSecretKeyEnv)
 	if accessKeySecret == "" {
 		accessKeySecret = conf["secret_key"]
 		if accessKeySecret == "" {
@@ -108,7 +118,7 @@ func NewAliCloudOSSBackend(conf map[string]string, logger log.Logger) (physical.
 
 // Put is used to insert or update an entry
 func (a *AliCloudOSSBackend) Put(ctx context.Context, entry *physical.Entry) error {
-	defer metrics.MeasureSince([]string{"alibaba", "put"}, time.Now())
+	defer metrics.MeasureSince([]string{AlibabaMetricKey, "put"}, time.Now())
 
 	a.permitPool.Acquire()
 	defer a.permitPool.Release()
@@ -124,7 +134,7 @@ func (a *AliCloudOSSBackend) Put(ctx context.Context, entry *physical.Entry) err
 
 // Get is used to fetch an entry
 func (a *AliCloudOSSBackend) Get(ctx context.Context, key string) (*physical.Entry, error) {
-	defer metrics.MeasureSince([]string{"alibaba", "get"}, time.Now())
+	defer metrics.MeasureSince([]string{AlibabaMetricKey, "get"}, time.Now())
 
 	a.permitPool.Acquire()
 	defer a.permitPool.Release()
@@ -138,7 +148,7 @@ func (a *AliCloudOSSBackend) Get(ctx context.Context, key string) (*physical.Ent
 	if err != nil {
 		switch err := err.(type) {
 		case oss.ServiceError:
-			if err.StatusCode == 404 && err.Code == "NoSuchKey" {
+			if err.StatusCode == http.StatusNotFound && err.Code == "NoSuchKey" {
 				return nil, nil
 			}
 		}
@@ -161,7 +171,7 @@ func (a *AliCloudOSSBackend) Get(ctx context.Context, key string) (*physical.Ent
 
 // Delete is used to permanently delete an entry
 func (a *AliCloudOSSBackend) Delete(ctx context.Context, key string) error {
-	defer metrics.MeasureSince([]string{"alibaba", "delete"}, time.Now())
+	defer metrics.MeasureSince([]string{AlibabaMetricKey, "delete"}, time.Now())
 
 	a.permitPool.Acquire()
 	defer a.permitPool.Release()
@@ -177,7 +187,7 @@ func (a *AliCloudOSSBackend) Delete(ctx context.Context, key string) error {
 // List is used to list all the keys under a given
 // prefix, up to the next prefix.
 func (a *AliCloudOSSBackend) List(ctx context.Context, prefix string) ([]string, error) {
-	defer metrics.MeasureSince([]string{"alibaba", "list"}, time.Now())
+	defer metrics.MeasureSince([]string{AlibabaMetricKey, "list"}, time.Now())
 
 	a.permitPool.Acquire()
 	defer a.permitPool.Release()
@@ -198,7 +208,6 @@ func (a *AliCloudOSSBackend) List(ctx context.Context, prefix string) ([]string,
 		}
 
 		for _, commonPrefix := range result.CommonPrefixes {
-
 			commonPrefix := strings.TrimPrefix(commonPrefix, prefix)
 			keys = append(keys, commonPrefix)
 		}

physical/alicloudoss/alicloudoss_test.go

@@ -14,11 +14,10 @@ import (
 )
 
 func TestAliCloudOSSBackend(t *testing.T) {
-
 	// ex. http://oss-us-east-1.aliyuncs.com
-	endpoint := os.Getenv("ALICLOUD_OSS_ENDPOINT")
-	accessKeyID := os.Getenv("ALICLOUD_ACCESS_KEY")
-	accessKeySecret := os.Getenv("ALICLOUD_SECRET_KEY")
+	endpoint := os.Getenv(AlibabaCloudOSSEndpointEnv)
+	accessKeyID := os.Getenv(AlibabaCloudAccessKeyEnv)
+	accessKeySecret := os.Getenv(AlibabaCloudSecretKeyEnv)
 
 	if endpoint == "" || accessKeyID == "" || accessKeySecret == "" {
 		t.SkipNow()
@@ -41,14 +40,24 @@ func TestAliCloudOSSBackend(t *testing.T) {
 		// Gotta list all the objects and delete them
 		// before being able to delete the bucket
 		b, err := conn.Bucket(bucket)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
 		listResp, err := b.ListObjects()
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
 
 		objects := []string{}
 		for _, object := range listResp.Objects {
 			objects = append(objects, object.Key)
 		}
 
-		b.DeleteObjects(objects)
+		_, err = b.DeleteObjects(objects)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
 
 		err = conn.DeleteBucket(bucket)
 		if err != nil {
@@ -59,9 +68,10 @@ func TestAliCloudOSSBackend(t *testing.T) {
 	logger := logging.NewVaultLogger(log.Debug)
 
 	// This uses the same logic to find the Alibaba credentials as we did at the beginning of the test
-	b, err := NewAliCloudOSSBackend(map[string]string{
-		"bucket": bucket,
-	}, logger)
+	b, err := NewAliCloudOSSBackend(
+		map[string]string{"bucket": bucket},
+		logger,
+	)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}