core: GetBucketPolicy API should return all errors. (#666)

GetBucketPolicy should return exact error received
from the server and let the caller manage it.

Fixes #664

api-get-policy.go

@@ -36,6 +36,10 @@ func (c Client) GetBucketPolicy(bucketName, objectPrefix string) (bucketPolicy p
 	}
 	policyInfo, err := c.getBucketPolicy(bucketName)
 	if err != nil {
+		errResponse := ToErrorResponse(err)
+		if errResponse.Code == "NoSuchBucketPolicy" {
+			return policy.BucketPolicyNone, nil
+		}
 		return policy.BucketPolicyNone, err
 	}
 	return policy.GetPolicy(policyInfo.Statements, bucketName, objectPrefix), nil
@@ -52,11 +56,20 @@ func (c Client) ListBucketPolicies(bucketName, objectPrefix string) (bucketPolic
 	}
 	policyInfo, err := c.getBucketPolicy(bucketName)
 	if err != nil {
+		errResponse := ToErrorResponse(err)
+		if errResponse.Code == "NoSuchBucketPolicy" {
+			return map[string]policy.BucketPolicy{}, nil
+		}
 		return map[string]policy.BucketPolicy{}, err
 	}
 	return policy.GetPolicies(policyInfo.Statements, bucketName), nil
 }
 
+// Default empty bucket access policy.
+var emptyBucketAccessPolicy = policy.BucketAccessPolicy{
+	Version: "2012-10-17",
+}
+
 // Request server for current bucket policy.
 func (c Client) getBucketPolicy(bucketName string) (policy.BucketAccessPolicy, error) {
 	// Get resources properly escaped and lined up before
@@ -72,21 +85,18 @@ func (c Client) getBucketPolicy(bucketName string) (policy.BucketAccessPolicy, e
 
 	defer closeResponse(resp)
 	if err != nil {
-		return policy.BucketAccessPolicy{}, err
+		return emptyBucketAccessPolicy, err
 	}
 
 	if resp != nil {
 		if resp.StatusCode != http.StatusOK {
-			errResponse := httpRespToErrorResponse(resp, bucketName, "")
-			if ToErrorResponse(errResponse).Code == "NoSuchBucketPolicy" {
-				return policy.BucketAccessPolicy{Version: "2012-10-17"}, nil
-			}
-			return policy.BucketAccessPolicy{}, errResponse
+			return emptyBucketAccessPolicy, httpRespToErrorResponse(resp, bucketName, "")
 		}
 	}
+
 	bucketPolicyBuf, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return policy.BucketAccessPolicy{}, err
+		return emptyBucketAccessPolicy, err
 	}
 
 	policy := policy.BucketAccessPolicy{}

api-put-bucket.go

@@ -163,7 +163,8 @@ func (c Client) SetBucketPolicy(bucketName string, objectPrefix string, bucketPo
 	}
 
 	policyInfo, err := c.getBucketPolicy(bucketName)
-	if err != nil {
+	errResponse := ToErrorResponse(err)
+	if err != nil && errResponse.Code != "NoSuchBucketPolicy" {
 		return err
 	}
 

core_test.go

@@ -0,0 +1,88 @@
+/*
+ * Minio Go Library for Amazon S3 Compatible Cloud Storage (C) 2017 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package minio
+
+import (
+	"math/rand"
+	"os"
+	"reflect"
+	"testing"
+	"time"
+)
+
+// Tests get bucket policy core API.
+func TestGetBucketPolicy(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping functional tests for short runs")
+	}
+
+	// Seed random based on current time.
+	rand.Seed(time.Now().Unix())
+
+	// Instantiate new minio client object.
+	c, err := NewCore(
+		os.Getenv("S3_ADDRESS"),
+		os.Getenv("ACCESS_KEY"),
+		os.Getenv("SECRET_KEY"),
+		mustParseBool(os.Getenv("S3_SECURE")),
+	)
+	if err != nil {
+		t.Fatal("Error:", err)
+	}
+
+	// Enable to debug
+	// c.TraceOn(os.Stderr)
+
+	// Set user agent.
+	c.SetAppInfo("Minio-go-FunctionalTest", "0.1.0")
+
+	// Generate a new random bucket name.
+	bucketName := randString(60, rand.NewSource(time.Now().UnixNano()), "minio-go-test")
+
+	// Make a new bucket.
+	err = c.MakeBucket(bucketName, "us-east-1")
+	if err != nil {
+		t.Fatal("Error:", err, bucketName)
+	}
+
+	// Verify if bucket exits and you have access.
+	var exists bool
+	exists, err = c.BucketExists(bucketName)
+	if err != nil {
+		t.Fatal("Error:", err, bucketName)
+	}
+	if !exists {
+		t.Fatal("Error: could not find ", bucketName)
+	}
+
+	// Asserting the default bucket policy.
+	bucketPolicy, err := c.GetBucketPolicy(bucketName)
+	if err != nil {
+		errResp := ToErrorResponse(err)
+		if errResp.Code != "NoSuchBucketPolicy" {
+			t.Error("Error:", err, bucketName)
+		}
+	}
+	if !reflect.DeepEqual(bucketPolicy, emptyBucketAccessPolicy) {
+		t.Errorf("Bucket policy expected %#v, got %#v", emptyBucketAccessPolicy, bucketPolicy)
+	}
+
+	err = c.RemoveBucket(bucketName)
+	if err != nil {
+		t.Fatal("Error:", err)
+	}
+}