From b121c8588fdd0aa6b13912a6c3a184311f041470 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 22 Apr 2015 14:44:59 -0700 Subject: [PATCH] Remove bucketpolicy handlers and all its references --- .travis.yml | 3 +- pkg/api/api_bucket_handlers.go | 21 --- pkg/api/api_definitions.go | 1 + pkg/api/api_object_handlers.go | 14 -- pkg/api/api_policy_handlers.go | 121 ------------ pkg/api/errors.go | 12 -- pkg/api/resources.go | 2 - pkg/storage/drivers/bucket_policy.go | 199 -------------------- pkg/storage/drivers/bucket_policy_compat.go | 52 ----- pkg/storage/drivers/donut/donut.go | 10 - pkg/storage/drivers/driver.go | 3 - pkg/storage/drivers/errors.go | 8 - pkg/storage/drivers/memory/memory.go | 13 +- pkg/storage/drivers/mocks/Driver.go | 19 -- 14 files changed, 4 insertions(+), 474 deletions(-) delete mode 100644 pkg/api/api_policy_handlers.go delete mode 100644 pkg/storage/drivers/bucket_policy.go delete mode 100644 pkg/storage/drivers/bucket_policy_compat.go diff --git a/.travis.yml b/.travis.yml index f0b147abd..4bd0691cb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,8 +10,7 @@ before_install: - cd .. sudo: false go: -- 1.4 -- release +- 1.4.1 notifications: slack: secure: jlDBuqna7waJXJrl/EOeTH1fXgqJu3WrTDl0Sv7oJBNVH1Af4cGmHaa1oVWrYUMB7lEPjpuF+xcBNA+N+mcR53JbpqueR3sIKlokqHL4TPZBg4XX+1yqtmYMkL6V2woWQ7Wmtis0kDstSoVZjEUVHgk3YF8hcLlK49oMhTeqY08= diff --git a/pkg/api/api_bucket_handlers.go b/pkg/api/api_bucket_handlers.go index 23958c945..26a9b29e5 100644 --- a/pkg/api/api_bucket_handlers.go +++ b/pkg/api/api_bucket_handlers.go @@ -36,23 +36,9 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ bucket := vars["bucket"] resources := getBucketResources(req.URL.Query()) - if resources.Policy == true { - // TODO - // ---- - // This is handled here instead of router, is only because semantically - // resource queries are not treated differently by Gorilla mux - // - // In-fact a request coming in as /bucket/?policy={} and /bucket/object are - // treated similarly. A proper fix would be to remove this comment and - // find a right regex pattern for individual requests - server.getBucketPolicyHandler(w, req) - return - } - if resources.Maxkeys == 0 { resources.Maxkeys = maxObjectList } - acceptsContentType := getContentType(req) objects, resources, err := server.driver.ListObjects(bucket, resources) switch err.(type) { @@ -121,12 +107,6 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques bucket := vars["bucket"] err := server.driver.CreateBucket(bucket) - resources := getBucketResources(req.URL.Query()) - if resources.Policy == true { - server.putBucketPolicyHandler(w, req) - return - } - acceptsContentType := getContentType(req) switch err.(type) { case nil: @@ -158,7 +138,6 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques // have permission to access it. Otherwise, the operation might // return responses such as 404 Not Found and 403 Forbidden. func (server *minioAPI) headBucketHandler(w http.ResponseWriter, req *http.Request) { - // TODO need to peek into bucketPolicy return appropriate checks here vars := mux.Vars(req) bucket := vars["bucket"] acceptsContentType := getContentType(req) diff --git a/pkg/api/api_definitions.go b/pkg/api/api_definitions.go index 5a5883cf3..839a01e0e 100644 --- a/pkg/api/api_definitions.go +++ b/pkg/api/api_definitions.go @@ -77,6 +77,7 @@ type Owner struct { // List of not implemented bucket queries var unimplementedBucketResourceNames = map[string]bool{ "acl": true, + "policy": true, "cors": true, "lifecycle": true, "location": true, diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index 9b5a48772..28a6b24b4 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -134,20 +134,6 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques bucket = vars["bucket"] object = vars["object"] - resources := getBucketResources(req.URL.Query()) - if resources.Policy == true && object == "" { - // TODO - // ---- - // This is handled here instead of router, is only because semantically - // resource queries are not treated differently by Gorilla mux - // - // In-fact a request coming in as /bucket/?policy={} and /bucket/object are - // treated similarly. A proper fix would be to remove this comment and - // find a right regex pattern for individual requests - server.putBucketPolicyHandler(w, req) - return - } - // get Content-MD5 sent by client md5 := req.Header.Get("Content-MD5") err := server.driver.CreateObject(bucket, object, "", md5, req.Body) diff --git a/pkg/api/api_policy_handlers.go b/pkg/api/api_policy_handlers.go deleted file mode 100644 index e4d41308c..000000000 --- a/pkg/api/api_policy_handlers.go +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Minimalist Object Storage, (C) 2015 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 api - -import ( - "encoding/json" - "net/http" - - "github.com/gorilla/mux" - "github.com/minio-io/minio/pkg/iodine" - "github.com/minio-io/minio/pkg/storage/drivers" - "github.com/minio-io/minio/pkg/utils/log" -) - -// PUT Bucket policy -// ----------------- -// This implementation of the PUT operation uses the policy subresource -// to add to or replace a policy on a bucket -func (server *minioAPI) putBucketPolicyHandler(w http.ResponseWriter, req *http.Request) { - vars := mux.Vars(req) - bucket := vars["bucket"] - acceptsContentType := getContentType(req) - - policy, ok := drivers.Parsepolicy(req.Body) - if ok == false { - writeErrorResponse(w, req, InvalidPolicyDocument, acceptsContentType, req.URL.Path) - return - } - - err := server.driver.CreateBucketPolicy(bucket, policy) - switch err := err.(type) { - case nil: - { - w.WriteHeader(http.StatusNoContent) - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.Header().Set("Connection", "keep-alive") - } - case drivers.BucketNameInvalid: - { - writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path) - } - case drivers.BucketNotFound: - { - writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path) - } - case drivers.BackendCorrupted: - { - log.Error.Println(err) - writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) - } - case drivers.ImplementationError: - { - log.Error.Println(err) - writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) - } - } -} - -// GET Bucket policy -// ----------------- -// This implementation of the GET operation uses the policy subresource -// to return the policy of a specified bucket. -func (server *minioAPI) getBucketPolicyHandler(w http.ResponseWriter, req *http.Request) { - vars := mux.Vars(req) - bucket := vars["bucket"] - acceptsContentType := getContentType(req) - - p, err := server.driver.GetBucketPolicy(bucket) - switch err := err.(type) { - case nil: - { - responsePolicy, err := json.Marshal(p) - if err != nil { - // log error - log.Error.Println(iodine.New(err, nil)) - writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) - return - } - setCommonHeaders(w, getContentTypeString(acceptsContentType)) - w.Header().Set("Connection", "keep-alive") - w.WriteHeader(http.StatusOK) - w.Write(responsePolicy) - } - case drivers.BucketNameInvalid: - { - writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path) - } - case drivers.BucketNotFound: - { - writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path) - } - case drivers.BucketPolicyNotFound: - { - writeErrorResponse(w, req, NoSuchBucketPolicy, acceptsContentType, req.URL.Path) - } - case drivers.BackendCorrupted: - { - log.Error.Println(err) - writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) - } - case drivers.ImplementationError: - { - log.Error.Println(err) - writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path) - } - } -} diff --git a/pkg/api/errors.go b/pkg/api/errors.go index d5cdea3db..dcd7e2b8f 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -61,8 +61,6 @@ const ( RequestTimeTooSkewed SignatureDoesNotMatch TooManyBuckets - InvalidPolicyDocument - NoSuchBucketPolicy ) // Error code to Error structure map @@ -172,16 +170,6 @@ var errorCodeResponse = map[int]Error{ Description: "You have attempted to create more buckets than allowed.", HTTPStatusCode: http.StatusBadRequest, }, - InvalidPolicyDocument: { - Code: "InvalidPolicyDocument", - Description: "The content of the form does not meet the conditions specified in the policy document.", - HTTPStatusCode: http.StatusBadRequest, - }, - NoSuchBucketPolicy: { - Code: "NoSuchBucketPolicy", - Description: "The specified bucket does not have a bucket policy.", - HTTPStatusCode: http.StatusNotFound, - }, } // errorCodeError provides errorCode to Error. It returns empty if the code provided is unknown diff --git a/pkg/api/resources.go b/pkg/api/resources.go index 1f9b6b288..33807035a 100644 --- a/pkg/api/resources.go +++ b/pkg/api/resources.go @@ -33,8 +33,6 @@ func getBucketResources(values url.Values) (v drivers.BucketResourcesMetadata) { v.Marker = value[0] case key == "max-keys": v.Maxkeys, _ = strconv.Atoi(value[0]) - case key == "policy": - v.Policy = true case key == "delimiter": v.Delimiter = value[0] } diff --git a/pkg/storage/drivers/bucket_policy.go b/pkg/storage/drivers/bucket_policy.go deleted file mode 100644 index d49826e85..000000000 --- a/pkg/storage/drivers/bucket_policy.go +++ /dev/null @@ -1,199 +0,0 @@ -/* - * Minimalist Object Storage, (C) 2015 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 drivers - -import ( - "encoding/json" - "io" - "strings" -) - -// User - canonical -type User struct { - AWS string -} - -// Statement - minio policy statement -type Statement struct { - Sid string - Effect string - Principal User - Action []string - Resource []string - // add Condition struct/var TODO - fix it in future if necessary -} - -// BucketPolicy - minio policy collection -type BucketPolicy struct { - Version string // date in 0000-00-00 format - Statement []Statement -} - -// Resource delimiter -const ( - MinioResource = "minio:::" -) - -// TODO support canonical user -// Principal delimiter -const ( - MinioPrincipal = "minio::" -) - -// Action map -var SupportedActionMap = map[string]bool{ - "*": true, - "minio:GetObject": true, - "minio:ListBucket": true, - "minio:PutObject": true, - "minio:CreateBucket": true, - "minio:GetBucketPolicy": true, - "minio:DeleteBucketPolicy": true, - "minio:ListAllMyBuckets": true, - "minio:PutBucketPolicy": true, -} - -// Effect map -var SupportedEffectMap = map[string]bool{ - "Allow": true, - "Deny": true, -} - -func isValidAction(action []string) bool { - for _, a := range action { - if !SupportedActionMap[a] { - return false - } - } - return true -} - -func isValidEffect(effect string) bool { - if SupportedEffectMap[effect] { - return true - } - return false -} - -func isValidResource(resources []string) bool { - var ok bool - for _, resource := range resources { - switch true { - case strings.HasPrefix(resource, AwsResource): - bucket := strings.SplitAfter(resource, AwsResource)[1] - ok = true - if len(bucket) == 0 { - ok = false - } - case strings.HasPrefix(resource, MinioResource): - bucket := strings.SplitAfter(resource, MinioResource)[1] - ok = true - if len(bucket) == 0 { - ok = false - } - default: - ok = false - } - } - return ok -} - -func isValidPrincipal(principal string) bool { - var ok bool - if principal == "*" { - return true - } - switch true { - case strings.HasPrefix(principal, AwsPrincipal): - username := strings.SplitAfter(principal, AwsPrincipal)[1] - ok = true - if len(username) == 0 { - ok = false - } - - case strings.HasPrefix(principal, MinioPrincipal): - username := strings.SplitAfter(principal, MinioPrincipal)[1] - ok = true - if len(username) == 0 { - ok = false - } - default: - ok = false - } - return ok -} - -func parseStatement(statement Statement) bool { - if len(statement.Sid) == 0 { - return false - } - if len(statement.Effect) == 0 { - return false - } - if !isValidEffect(statement.Effect) { - return false - } - if len(statement.Principal.AWS) == 0 { - return false - } - if !isValidPrincipal(statement.Principal.AWS) { - return false - } - if len(statement.Action) == 0 { - return false - } - if !isValidAction(statement.Action) && !isValidActionS3(statement.Action) { - return false - } - if len(statement.Resource) == 0 { - return false - } - if !isValidResource(statement.Resource) { - return false - } - return true -} - -// Parsepolicy - validate request body is proper JSON and in accordance with policy standards -func Parsepolicy(data io.Reader) (BucketPolicy, bool) { - var policy BucketPolicy - decoder := json.NewDecoder(data) - err := decoder.Decode(&policy) - if err != nil { - goto error - } - if len(policy.Version) == 0 { - goto error - } - _, err = parseDate(policy.Version) - if err != nil { - goto error - } - if len(policy.Statement) == 0 { - goto error - } - - for _, statement := range policy.Statement { - if !parseStatement(statement) { - goto error - } - } - return policy, true - -error: - return BucketPolicy{}, false -} diff --git a/pkg/storage/drivers/bucket_policy_compat.go b/pkg/storage/drivers/bucket_policy_compat.go deleted file mode 100644 index 5280df224..000000000 --- a/pkg/storage/drivers/bucket_policy_compat.go +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Minimalist Object Storage, (C) 2015 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 drivers - -// This file implements compatability layer for AWS clients - -// Resource delimiter -const ( - AwsResource = "arn:aws:s3:::" -) - -// TODO support canonical user -// Principal delimiter -const ( - AwsPrincipal = "arn:aws:iam::" -) - -// Action map -var SupportedActionMapCompat = map[string]bool{ - "*": true, - "s3:GetObject": true, - "s3:ListBucket": true, - "s3:PutObject": true, - "s3:CreateBucket": true, - "s3:GetBucketPolicy": true, - "s3:DeleteBucketPolicy": true, - "s3:ListAllMyBuckets": true, - "s3:PutBucketPolicy": true, -} - -func isValidActionS3(action []string) bool { - for _, a := range action { - if !SupportedActionMapCompat[a] { - return false - } - } - return true -} diff --git a/pkg/storage/drivers/donut/donut.go b/pkg/storage/drivers/donut/donut.go index a8886dd56..e2ce2f223 100644 --- a/pkg/storage/drivers/donut/donut.go +++ b/pkg/storage/drivers/donut/donut.go @@ -144,16 +144,6 @@ func (d donutDriver) GetBucketMetadata(bucketName string) (drivers.BucketMetadat return bucketMetadata, nil } -// CreateBucketPolicy sets a bucket's access policy -func (d donutDriver) CreateBucketPolicy(bucket string, p drivers.BucketPolicy) error { - return iodine.New(errors.New("Not Implemented"), nil) -} - -// GetBucketPolicy returns a bucket's access policy -func (d donutDriver) GetBucketPolicy(bucket string) (drivers.BucketPolicy, error) { - return drivers.BucketPolicy{}, iodine.New(errors.New("Not Implemented"), nil) -} - // GetObject retrieves an object and writes it to a writer func (d donutDriver) GetObject(target io.Writer, bucketName, objectName string) (int64, error) { errParams := map[string]string{ diff --git a/pkg/storage/drivers/driver.go b/pkg/storage/drivers/driver.go index baecf4707..e6eb3571e 100644 --- a/pkg/storage/drivers/driver.go +++ b/pkg/storage/drivers/driver.go @@ -29,8 +29,6 @@ type Driver interface { ListBuckets() ([]BucketMetadata, error) CreateBucket(bucket string) error GetBucketMetadata(bucket string) (BucketMetadata, error) - CreateBucketPolicy(bucket string, p BucketPolicy) error - GetBucketPolicy(bucket string) (BucketPolicy, error) // Object Operations GetObject(w io.Writer, bucket, object string) (int64, error) @@ -78,7 +76,6 @@ type BucketResourcesMetadata struct { CommonPrefixes []string Mode FilterMode - Policy bool // TODO Logging string Notification string diff --git a/pkg/storage/drivers/errors.go b/pkg/storage/drivers/errors.go index bdede15d9..fa0a51031 100644 --- a/pkg/storage/drivers/errors.go +++ b/pkg/storage/drivers/errors.go @@ -56,9 +56,6 @@ type DigestError struct { /// Bucket related errors -// BucketPolicyNotFound - missing bucket policy -type BucketPolicyNotFound GenericBucketError - // BucketNameInvalid - bucketname provided is invalid type BucketNameInvalid GenericBucketError @@ -107,11 +104,6 @@ func EmbedError(bucket, object string, err error) ImplementationError { } } -// Return string an error formatted as the given text -func (e BucketPolicyNotFound) Error() string { - return "Bucket policy not found for: " + e.Bucket -} - // Return string an error formatted as the given text func (e ObjectNotFound) Error() string { return "Object not Found: " + e.Bucket + "#" + e.Object diff --git a/pkg/storage/drivers/memory/memory.go b/pkg/storage/drivers/memory/memory.go index d48aed75c..a42471c82 100644 --- a/pkg/storage/drivers/memory/memory.go +++ b/pkg/storage/drivers/memory/memory.go @@ -30,8 +30,9 @@ import ( "crypto/md5" "encoding/hex" - "github.com/golang/groupcache/lru" "io/ioutil" + + "github.com/golang/groupcache/lru" ) // memoryDriver - local variables @@ -117,16 +118,6 @@ func (memory memoryDriver) GetBucketMetadata(bucket string) (drivers.BucketMetad return memory.bucketMetadata[bucket].metadata, nil } -// CreateBucketPolicy - Not implemented -func (memory memoryDriver) CreateBucketPolicy(bucket string, policy drivers.BucketPolicy) error { - return drivers.APINotImplemented{API: "PutBucketPolicy"} -} - -// GetBucketPolicy - Not implemented -func (memory memoryDriver) GetBucketPolicy(bucket string) (drivers.BucketPolicy, error) { - return drivers.BucketPolicy{}, drivers.APINotImplemented{API: "GetBucketPolicy"} -} - // CreateObject - PUT object to memory buffer func (memory memoryDriver) CreateObject(bucket, key, contentType, md5sum string, data io.Reader) error { memory.lock.RLock() diff --git a/pkg/storage/drivers/mocks/Driver.go b/pkg/storage/drivers/mocks/Driver.go index 28e081ee5..5995328fd 100644 --- a/pkg/storage/drivers/mocks/Driver.go +++ b/pkg/storage/drivers/mocks/Driver.go @@ -44,25 +44,6 @@ func (m *Driver) GetBucketMetadata(bucket string) (drivers.BucketMetadata, error return r0, r1 } -// CreateBucketPolicy is a mock -func (m *Driver) CreateBucketPolicy(bucket string, p drivers.BucketPolicy) error { - ret := m.Called(bucket, p) - - r0 := ret.Error(0) - - return r0 -} - -// GetBucketPolicy is a mock -func (m *Driver) GetBucketPolicy(bucket string) (drivers.BucketPolicy, error) { - ret := m.Called(bucket) - - r0 := ret.Get(0).(drivers.BucketPolicy) - r1 := ret.Error(1) - - return r0, r1 -} - // SetGetObjectWriter is a mock func (m *Driver) SetGetObjectWriter(bucket, object string, data []byte) { m.ObjectWriterData[bucket+":"+object] = data