From b207520d98689616fd8eb07f61dfc74a49fce4a3 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Thu, 26 Mar 2020 05:06:03 +0100 Subject: [PATCH] Fix lifecycle GET: AWS SDK complaints on empty config (#9201) --- cmd/bucket-lifecycle-handlers_test.go | 303 ++++++++++++++++++++++++++ cmd/test-utils_test.go | 13 ++ pkg/bucket/lifecycle/filter.go | 42 +++- 3 files changed, 350 insertions(+), 8 deletions(-) create mode 100644 cmd/bucket-lifecycle-handlers_test.go diff --git a/cmd/bucket-lifecycle-handlers_test.go b/cmd/bucket-lifecycle-handlers_test.go new file mode 100644 index 000000000..b8d5772a2 --- /dev/null +++ b/cmd/bucket-lifecycle-handlers_test.go @@ -0,0 +1,303 @@ +/* + * MinIO Cloud Storage, (C) 2020 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 cmd + +import ( + "bytes" + "encoding/xml" + "net/http" + "net/http/httptest" + "testing" + + "github.com/minio/minio/pkg/auth" +) + +// Test S3 Bucket lifecycle APIs with wrong credentials +func TestBucketLifecycleWrongCredentials(t *testing.T) { + ExecObjectLayerAPITest(t, testBucketLifecycleHandlersWrongCredentials, []string{"GetBucketLifecycle", "PutBucketLifecycle", "DeleteBucketLifecycle"}) +} + +// Test for authentication +func testBucketLifecycleHandlersWrongCredentials(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials auth.Credentials, t *testing.T) { + // test cases with sample input and expected output. + testCases := []struct { + method string + bucketName string + accessKey string + secretKey string + // Sent body + body []byte + // Expected response + expectedRespStatus int + lifecycleResponse []byte + errorResponse APIErrorResponse + shouldPass bool + }{ + // GET empty credentials + { + method: "GET", bucketName: bucketName, + accessKey: "", + secretKey: "", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "AccessDenied", + Message: "Access Denied.", + }, + shouldPass: false, + }, + // GET wrong credentials + { + method: "GET", bucketName: bucketName, + accessKey: "abcd", + secretKey: "abcd", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "InvalidAccessKeyId", + Message: "The access key ID you provided does not exist in our records.", + }, + shouldPass: false, + }, + // PUT empty credentials + { + method: "PUT", + bucketName: bucketName, + accessKey: "", + secretKey: "", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "AccessDenied", + Message: "Access Denied.", + }, + shouldPass: false, + }, + // PUT wrong credentials + { + method: "PUT", + bucketName: bucketName, + accessKey: "abcd", + secretKey: "abcd", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "InvalidAccessKeyId", + Message: "The access key ID you provided does not exist in our records.", + }, + shouldPass: false, + }, + // DELETE empty credentials + { + method: "DELETE", + bucketName: bucketName, + accessKey: "", + secretKey: "", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "AccessDenied", + Message: "Access Denied.", + }, + shouldPass: false, + }, + // DELETE wrong credentials + { + method: "DELETE", + bucketName: bucketName, + accessKey: "abcd", + secretKey: "abcd", + expectedRespStatus: http.StatusForbidden, + lifecycleResponse: []byte(""), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "InvalidAccessKeyId", + Message: "The access key ID you provided does not exist in our records.", + }, + shouldPass: false, + }, + } + + testBucketLifecycle(obj, instanceType, bucketName, apiRouter, t, testCases) +} + +// Test S3 Bucket lifecycle APIs +func TestBucketLifecycle(t *testing.T) { + ExecObjectLayerAPITest(t, testBucketLifecycleHandlers, []string{"GetBucketLifecycle", "PutBucketLifecycle", "DeleteBucketLifecycle"}) +} + +// Simple tests of bucket lifecycle: PUT, GET, DELETE. +// Tests are related and the order is important. +func testBucketLifecycleHandlers(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + creds auth.Credentials, t *testing.T) { + + // test cases with sample input and expected output. + testCases := []struct { + method string + bucketName string + accessKey string + secretKey string + // Sent body + body []byte + // Expected response + expectedRespStatus int + lifecycleResponse []byte + errorResponse APIErrorResponse + shouldPass bool + }{ + // Test case - 1. + // Filter contains more than (Prefix,Tag,And) rule + { + method: "PUT", + bucketName: bucketName, + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + body: []byte(`idlogs/Key1Value1Enabled365`), + expectedRespStatus: http.StatusBadRequest, + lifecycleResponse: []byte(``), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "InvalidRequest", + Message: "Filter must have exactly one of Prefix, Tag, or And specified", + }, + + shouldPass: false, + }, + // Date contains wrong format + { + method: "PUT", + bucketName: bucketName, + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + body: []byte(`idlogs/Key1Value1Enabled365`), + expectedRespStatus: http.StatusBadRequest, + lifecycleResponse: []byte(``), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "InvalidRequest", + Message: "Date must be provided in ISO 8601 format", + }, + + shouldPass: false, + }, + { + method: "PUT", + bucketName: bucketName, + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + body: []byte(`idlogs/Enabled365`), + expectedRespStatus: http.StatusOK, + lifecycleResponse: []byte(``), + errorResponse: APIErrorResponse{}, + shouldPass: true, + }, + { + method: "GET", + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + bucketName: bucketName, + body: []byte(``), + expectedRespStatus: http.StatusOK, + lifecycleResponse: []byte(`idEnabledlogs/365`), + errorResponse: APIErrorResponse{}, + shouldPass: true, + }, + { + method: "DELETE", + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + bucketName: bucketName, + body: []byte(``), + expectedRespStatus: http.StatusNoContent, + lifecycleResponse: []byte(``), + errorResponse: APIErrorResponse{}, + shouldPass: true, + }, + { + method: "GET", + accessKey: creds.AccessKey, + secretKey: creds.SecretKey, + bucketName: bucketName, + body: []byte(``), + expectedRespStatus: http.StatusNotFound, + lifecycleResponse: []byte(``), + errorResponse: APIErrorResponse{ + Resource: SlashSeparator + bucketName + SlashSeparator, + Code: "NoSuchLifecycleConfiguration", + Message: "The lifecycle configuration does not exist", + }, + shouldPass: false, + }, + } + + testBucketLifecycle(obj, instanceType, bucketName, apiRouter, t, testCases) +} + +// testBucketLifecycle is a generic testing of lifecycle requests +func testBucketLifecycle(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + t *testing.T, testCases []struct { + method string + bucketName string + accessKey string + secretKey string + body []byte + expectedRespStatus int + lifecycleResponse []byte + errorResponse APIErrorResponse + shouldPass bool + }) { + + for i, testCase := range testCases { + // initialize httptest Recorder, this records any mutations to response writer inside the handler. + rec := httptest.NewRecorder() + // construct HTTP request + req, err := newTestSignedRequestV4(testCase.method, getBucketLifecycleURL("", testCase.bucketName), + int64(len(testCase.body)), bytes.NewReader(testCase.body), testCase.accessKey, testCase.secretKey, nil) + if err != nil { + t.Fatalf("Test %d: %s: Failed to create HTTP request for GetBucketLocationHandler: %v", i+1, instanceType, err) + } + // Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic of the handler. + // Call the ServeHTTP to execute the handler. + apiRouter.ServeHTTP(rec, req) + if rec.Code != testCase.expectedRespStatus { + t.Errorf("Test %d: %s: Expected the response status to be `%d`, but instead found `%d`", i+1, instanceType, testCase.expectedRespStatus, rec.Code) + } + if testCase.shouldPass && !bytes.Equal(testCase.lifecycleResponse, rec.Body.Bytes()) { + t.Errorf("Test %d: %s: Expected the response to be `%s`, but instead found `%s`", i+1, instanceType, string(testCase.lifecycleResponse), rec.Body.String()) + } + errorResponse := APIErrorResponse{} + err = xml.Unmarshal(rec.Body.Bytes(), &errorResponse) + if err != nil && !testCase.shouldPass { + t.Fatalf("Test %d: %s: Unable to marshal response body %s", i+1, instanceType, rec.Body.String()) + } + if errorResponse.Resource != testCase.errorResponse.Resource { + t.Errorf("Test %d: %s: Expected the error resource to be `%s`, but instead found `%s`", i+1, instanceType, testCase.errorResponse.Resource, errorResponse.Resource) + } + if errorResponse.Message != testCase.errorResponse.Message { + t.Errorf("Test %d: %s: Expected the error message to be `%s`, but instead found `%s`", i+1, instanceType, testCase.errorResponse.Message, errorResponse.Message) + } + if errorResponse.Code != testCase.errorResponse.Code { + t.Errorf("Test %d: %s: Expected the error code to be `%s`, but instead found `%s`", i+1, instanceType, testCase.errorResponse.Code, errorResponse.Code) + } + } +} diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 174308920..995d6e5e1 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1463,6 +1463,13 @@ func getBucketLocationURL(endPoint, bucketName string) string { return makeTestTargetURL(endPoint, bucketName, "", queryValue) } +// return URL For set/get lifecycle of the bucket. +func getBucketLifecycleURL(endPoint, bucketName string) (ret string) { + queryValue := url.Values{} + queryValue.Set("lifecycle", "") + return makeTestTargetURL(endPoint, bucketName, "", queryValue) +} + // return URL for listing objects in the bucket with V1 legacy API. func getListObjectsV1URL(endPoint, bucketName, prefix, maxKeys, encodingType string) string { queryValue := url.Values{} @@ -2070,6 +2077,12 @@ func registerBucketLevelFunc(bucket *mux.Router, api objectAPIHandlers, apiFunct case "GetBucketPolicy": // Register Get Bucket policy HTTP Handler. bucket.Methods("GET").HandlerFunc(api.GetBucketPolicyHandler).Queries("policy", "") + case "GetBucketLifecycle": + bucket.Methods("GET").HandlerFunc(api.GetBucketLifecycleHandler).Queries("lifecycle", "") + case "PutBucketLifecycle": + bucket.Methods("PUT").HandlerFunc(api.PutBucketLifecycleHandler).Queries("lifecycle", "") + case "DeleteBucketLifecycle": + bucket.Methods("DELETE").HandlerFunc(api.DeleteBucketLifecycleHandler).Queries("lifecycle", "") case "GetBucketLocation": // Register GetBucketLocation handler. bucket.Methods("GET").HandlerFunc(api.GetBucketLocationHandler).Queries("location", "") diff --git a/pkg/bucket/lifecycle/filter.go b/pkg/bucket/lifecycle/filter.go index 3817aae42..7692e9d36 100644 --- a/pkg/bucket/lifecycle/filter.go +++ b/pkg/bucket/lifecycle/filter.go @@ -22,18 +22,44 @@ import ( "github.com/minio/minio/pkg/bucket/object/tagging" ) -// Filter - a filter for a lifecycle configuration Rule. -type Filter struct { - XMLName xml.Name `xml:"Filter"` - Prefix string `xml:"Prefix,omitempty"` - And And `xml:"And,omitempty"` - Tag tagging.Tag `xml:"Tag,omitempty"` -} - var ( errInvalidFilter = Errorf("Filter must have exactly one of Prefix, Tag, or And specified") ) +// Filter - a filter for a lifecycle configuration Rule. +type Filter struct { + XMLName xml.Name `xml:"Filter"` + Prefix string + And And + Tag tagging.Tag +} + +// MarshalXML - produces the xml representation of the Filter struct +// only one of Prefix, And and Tag should be present in the output. +func (f Filter) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + if err := e.EncodeToken(start); err != nil { + return err + } + + switch { + case !f.And.isEmpty(): + if err := e.EncodeElement(f.And, xml.StartElement{Name: xml.Name{Local: "And"}}); err != nil { + return err + } + case !f.Tag.IsEmpty(): + if err := e.EncodeElement(f.Tag, xml.StartElement{Name: xml.Name{Local: "Tag"}}); err != nil { + return err + } + default: + // Always print Prefix field when both And & Tag are empty + if err := e.EncodeElement(f.Prefix, xml.StartElement{Name: xml.Name{Local: "Prefix"}}); err != nil { + return err + } + } + + return e.EncodeToken(xml.EndElement{Name: start.Name}) +} + // Validate - validates the filter element func (f Filter) Validate() error { // A Filter must have exactly one of Prefix, Tag, or And specified.