From 33d01b1a7abbab6809af3aad2ced3cf2c6a36401 Mon Sep 17 00:00:00 2001
From: James Naftel <james_naftel@yahoo.com>
Date: Tue, 16 Jul 2019 18:23:04 -0400
Subject: [PATCH] Fixed Minio NewStorage to return the MakeBucket error instead
 of the (#1302)

Bucket exists error.  See #1295
---
 pkg/storage/minio/minio.go      |  5 +--
 pkg/storage/minio/minio_test.go | 60 +++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/pkg/storage/minio/minio.go b/pkg/storage/minio/minio.go
index d70ae237..dddb274a 100644
--- a/pkg/storage/minio/minio.go
+++ b/pkg/storage/minio/minio.go
@@ -39,8 +39,9 @@ func NewStorage(conf *config.MinioConfig, timeout time.Duration) (storage.Backen
 	err = minioClient.MakeBucket(bucketName, region)
 	if err != nil {
 		// Check to see if we already own this bucket
-		exists, err := minioClient.BucketExists(bucketName)
-		if err == nil && !exists {
+		exists, _ := minioClient.BucketExists(bucketName)
+		if !exists {
+			// MakeBucket Error takes priority
 			return nil, errors.E(op, err)
 		}
 	}
diff --git a/pkg/storage/minio/minio_test.go b/pkg/storage/minio/minio_test.go
index 3af29d47..85c87473 100644
--- a/pkg/storage/minio/minio_test.go
+++ b/pkg/storage/minio/minio_test.go
@@ -13,6 +13,66 @@ func TestBackend(t *testing.T) {
 	compliance.RunTests(t, backend, backend.clear)
 }
 
+// TestNewStorageExists tests the logic around MakeBucket and BucketExists
+func TestNewStorageExists(t *testing.T) {
+	url := os.Getenv("ATHENS_MINIO_ENDPOINT")
+	if url == "" {
+		t.SkipNow()
+	}
+
+	tests := []struct {
+		name         string
+		deleteBucket bool
+	}{
+		{"testbucket", false}, // test creation
+		{"testbucket", true},  // test exists
+	}
+
+	for _, test := range tests {
+		backend, err := NewStorage(&config.MinioConfig{
+			Endpoint: url,
+			Key:      "minio",
+			Secret:   "minio123",
+			Bucket:   test.name,
+		}, config.GetTimeoutDuration(300))
+		if err != nil {
+			t.Fatalf("TestNewStorageExists failed for bucketname:  %s, error: %v\n", test.name, err)
+		}
+
+		client, ok := backend.(*storageImpl)
+		if test.deleteBucket && ok {
+			client.minioClient.RemoveBucket(test.name)
+		}
+	}
+}
+
+// TestNewStorageError tests the logic around MakeBucket and BucketExists
+// MakeBucket uses a strict naming path in minio while BucketExists does not.
+// To ensure both paths are tested, there is a strict path error using the
+// "_" and a non strict error using less than 3 characters
+func TestNewStorageError(t *testing.T) {
+	url := os.Getenv("ATHENS_MINIO_ENDPOINT")
+	if url == "" {
+		t.SkipNow()
+	}
+
+	// "_" is not allowed in a bucket name
+	// bucket name must be bigger than 3
+	tests := []string{"test_bucket", "1"}
+
+	for _, bucketName := range tests {
+		_, err := NewStorage(&config.MinioConfig{
+			Endpoint: url,
+			Key:      "minio",
+			Secret:   "minio123",
+			Bucket:   bucketName,
+		}, config.GetTimeoutDuration(300))
+		if err == nil {
+			t.Fatalf("TestNewStorageError failed for bucketname:  %s\n", bucketName)
+		}
+	}
+}
+
 func BenchmarkBackend(b *testing.B) {
 	backend := getStorage(b)
 	compliance.RunBenchmarks(b, backend, backend.clear)