Merge pull request #510 from harshavardhana/pr_out_now_client_requests_for_acl_changes_are_honored_through_putbucketacl_api

pkg/api/api_bucket_handlers.go

@@ -148,6 +148,11 @@ func (server *minioAPI) listBucketsHandler(w http.ResponseWriter, req *http.Requ
 // ----------
 // This implementation of the PUT operation creates a new bucket for authenticated request
 func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Request) {
+	if isRequestBucketACL(req.URL.Query()) {
+		server.putBucketACLHandler(w, req)
+		return
+	}
+
 	acceptsContentType := getContentType(req)
 	if acceptsContentType == unknownContentType {
 		writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path)
@@ -191,6 +196,49 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques
 	}
 }
 
+// PUT Bucket ACL
+// ----------
+// This implementation of the PUT operation modifies the bucketACL for authenticated request
+func (server *minioAPI) putBucketACLHandler(w http.ResponseWriter, req *http.Request) {
+	acceptsContentType := getContentType(req)
+	if acceptsContentType == unknownContentType {
+		writeErrorResponse(w, req, NotAcceptable, acceptsContentType, req.URL.Path)
+		return
+	}
+
+	// read from 'x-amz-acl'
+	aclType := getACLType(req)
+	if aclType == unsupportedACLType {
+		writeErrorResponse(w, req, NotImplemented, acceptsContentType, req.URL.Path)
+		return
+	}
+
+	vars := mux.Vars(req)
+	bucket := vars["bucket"]
+	err := server.driver.SetBucketMetadata(bucket, getACLTypeString(aclType))
+	switch iodine.ToError(err).(type) {
+	case nil:
+		{
+			w.Header().Set("Server", "Minio")
+			w.Header().Set("Connection", "close")
+			w.WriteHeader(http.StatusOK)
+		}
+	case drivers.BucketNameInvalid:
+		{
+			writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path)
+		}
+	case drivers.BucketNotFound:
+		{
+			writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path)
+		}
+	default:
+		{
+			log.Error.Println(iodine.New(err, nil))
+			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
+		}
+	}
+}
+
 // HEAD Bucket
 // ----------
 // This operation is useful to determine if a bucket exists.

pkg/api/api_definitions.go

@@ -76,7 +76,6 @@ type Owner struct {
 
 // List of not implemented bucket queries
 var unimplementedBucketResourceNames = map[string]bool{
-	"acl":            true,
 	"policy":         true,
 	"cors":           true,
 	"lifecycle":      true,
@@ -94,7 +93,6 @@ var unimplementedBucketResourceNames = map[string]bool{
 // List of not implemented object queries
 var unimplementedObjectResourceNames = map[string]bool{
 	"uploadId": true,
-	"acl":      true,
 	"torrent":  true,
 	"uploads":  true,
 }

pkg/api/api_test.go

@@ -447,7 +447,7 @@ func (s *MySuite) TestNotImplemented(c *C) {
 	testServer := httptest.NewServer(httpHandler)
 	defer testServer.Close()
 
-	request, err := http.NewRequest("GET", testServer.URL+"/bucket/object?acl", nil)
+	request, err := http.NewRequest("GET", testServer.URL+"/bucket/object?policy", nil)
 	c.Assert(err, IsNil)
 	setAuthHeader(request)
 

pkg/api/resources.go

@@ -39,3 +39,16 @@ func getBucketResources(values url.Values) (v drivers.BucketResourcesMetadata) {
 	}
 	return
 }
+
+// check if req query values have acl
+func isRequestBucketACL(values url.Values) bool {
+	for key := range values {
+		switch true {
+		case key == "acl":
+			return true
+		default:
+			return false
+		}
+	}
+	return false
+}

pkg/storage/donut/objectstorage.go

@@ -61,7 +61,10 @@ func (d donut) SetBucketMetadata(bucket string, bucketMetadata map[string]string
 	if err != nil {
 		return iodine.New(err, nil)
 	}
-	metadata[bucket] = bucketMetadata
+	oldBucketMetadata := metadata[bucket]
+	// TODO ignore rest of the keys for now, only mutable data is "acl"
+	oldBucketMetadata["acl"] = bucketMetadata["acl"]
+	metadata[bucket] = oldBucketMetadata
 	return d.setDonutBucketMetadata(metadata)
 }
 

pkg/storage/drivers/donut/donut.go

@@ -153,7 +153,14 @@ func (d donutDriver) CreateBucket(bucketName, acl string) error {
 		if strings.TrimSpace(acl) == "" {
 			acl = "private"
 		}
-		return d.donut.MakeBucket(bucketName, acl)
+		if err := d.donut.MakeBucket(bucketName, acl); err != nil {
+			err = iodine.ToError(err)
+			if err.Error() == "bucket exists" {
+				return iodine.New(drivers.BucketExists{Bucket: bucketName}, nil)
+			}
+			return err
+		}
+		return nil
 	}
 	return iodine.New(drivers.BucketNameInvalid{Bucket: bucketName}, nil)
 }
@@ -183,6 +190,23 @@ func (d donutDriver) GetBucketMetadata(bucketName string) (drivers.BucketMetadat
 	return bucketMetadata, nil
 }
 
+// SetBucketMetadata sets bucket's metadata
+func (d donutDriver) SetBucketMetadata(bucketName, acl string) error {
+	if !drivers.IsValidBucket(bucketName) || strings.Contains(bucketName, ".") {
+		return drivers.BucketNameInvalid{Bucket: bucketName}
+	}
+	if strings.TrimSpace(acl) == "" {
+		acl = "private"
+	}
+	bucketMetadata := make(map[string]string)
+	bucketMetadata["acl"] = acl
+	err := d.donut.SetBucketMetadata(bucketName, bucketMetadata)
+	if err != nil {
+		return iodine.New(drivers.BucketNotFound{Bucket: bucketName}, nil)
+	}
+	return nil
+}
+
 // GetObject retrieves an object and writes it to a writer
 func (d donutDriver) GetObject(target io.Writer, bucketName, objectName string) (int64, error) {
 	if !drivers.IsValidBucket(bucketName) || strings.Contains(bucketName, ".") {

pkg/storage/drivers/driver.go

@@ -30,6 +30,7 @@ type Driver interface {
 	ListBuckets() ([]BucketMetadata, error)
 	CreateBucket(bucket, acl string) error
 	GetBucketMetadata(bucket string) (BucketMetadata, error)
+	SetBucketMetadata(bucket, acl string) error
 
 	// Object Operations
 	GetObject(w io.Writer, bucket, object string) (int64, error)

pkg/storage/drivers/memory/memory.go

@@ -154,6 +154,29 @@ func (memory *memoryDriver) GetBucketMetadata(bucket string) (drivers.BucketMeta
 	return memory.bucketMetadata[bucket].metadata, nil
 }
 
+// SetBucketMetadata -
+func (memory *memoryDriver) SetBucketMetadata(bucket, acl string) error {
+	memory.lock.RLock()
+	if !drivers.IsValidBucket(bucket) {
+		memory.lock.RUnlock()
+		return iodine.New(drivers.BucketNameInvalid{Bucket: bucket}, nil)
+	}
+	if _, ok := memory.bucketMetadata[bucket]; ok == false {
+		memory.lock.RUnlock()
+		return iodine.New(drivers.BucketNotFound{Bucket: bucket}, nil)
+	}
+	if strings.TrimSpace(acl) == "" {
+		acl = "private"
+	}
+	memory.lock.RUnlock()
+	memory.lock.Lock()
+	defer memory.lock.Unlock()
+	storedBucket := memory.bucketMetadata[bucket]
+	storedBucket.metadata.ACL = drivers.BucketACL(acl)
+	memory.bucketMetadata[bucket] = storedBucket
+	return nil
+}
+
 // isMD5SumEqual - returns error if md5sum mismatches, success its `nil`
 func isMD5SumEqual(expectedMD5Sum, actualMD5Sum string) error {
 	if strings.TrimSpace(expectedMD5Sum) != "" && strings.TrimSpace(actualMD5Sum) != "" {

pkg/storage/drivers/mocks/Driver.go

@@ -44,6 +44,15 @@ func (m *Driver) GetBucketMetadata(bucket string) (drivers.BucketMetadata, error
 	return r0, r1
 }
 
+// SetBucketMetadata is a mock
+func (m *Driver) SetBucketMetadata(bucket, acl string) error {
+	ret := m.Called(bucket, acl)
+
+	r0 := ret.Error(0)
+
+	return r0
+}
+
 // SetGetObjectWriter is a mock
 func (m *Driver) SetGetObjectWriter(bucket, object string, data []byte) {
 	m.ObjectWriterData[bucket+":"+object] = data