From 099b5293a9680cb0b8663356d7001715beb43cae Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 9 Oct 2017 16:41:35 -0700 Subject: [PATCH] Move gateway unsupported functions into a common struct. (#5009) This is done to avoid repeated declaration of not-implemented functions for each gateway. It also avoids a possible bug in go https://github.com/golang/go/issues/18468 which is triggered on our multiple PRs already. --- cmd/gateway-azure-anonymous.go | 7 - cmd/gateway-azure.go | 6 +- cmd/gateway-gcs-anonymous.go | 6 - cmd/gateway-gcs-unsupported.go | 42 ------ cmd/gateway-gcs.go | 1 + cmd/gateway-s3-unsupported.go | 42 ------ cmd/gateway-s3.go | 8 +- ...-unsupported.go => gateway-unsupported.go} | 26 +++- cmd/server_test.go | 11 ++ cmd/server_utils_test.go | 127 ------------------ cmd/test-utils_test.go | 110 +++++++++++++++ cmd/to_err_test.go | 31 ----- 12 files changed, 145 insertions(+), 272 deletions(-) delete mode 100644 cmd/gateway-gcs-unsupported.go delete mode 100644 cmd/gateway-s3-unsupported.go rename cmd/{gateway-azure-unsupported.go => gateway-unsupported.go} (50%) delete mode 100644 cmd/server_utils_test.go delete mode 100644 cmd/to_err_test.go diff --git a/cmd/gateway-azure-anonymous.go b/cmd/gateway-azure-anonymous.go index 2cf09d1e1..346ac8e83 100644 --- a/cmd/gateway-azure-anonymous.go +++ b/cmd/gateway-azure-anonymous.go @@ -142,13 +142,6 @@ func (a *azureObjects) AnonGetBucketInfo(bucket string) (bucketInfo BucketInfo, return bucketInfo, nil } -// AnonPutObject - SendPUT request without authentication. -// This is needed when clients send PUT requests on objects that can be uploaded without auth. -func (a *azureObjects) AnonPutObject(bucket, object string, size int64, data io.Reader, metadata map[string]string, sha256sum string) (objInfo ObjectInfo, err error) { - // azure doesn't support anonymous put - return ObjectInfo{}, traceError(NotImplemented{}) -} - // AnonGetObject - SendGET request without authentication. // This is needed when clients send GET requests on objects that can be downloaded without auth. func (a *azureObjects) AnonGetObject(bucket, object string, startOffset int64, length int64, writer io.Writer) (err error) { diff --git a/cmd/gateway-azure.go b/cmd/gateway-azure.go index 9d9b89f00..32cfa8512 100644 --- a/cmd/gateway-azure.go +++ b/cmd/gateway-azure.go @@ -158,6 +158,7 @@ func azureToS3ETag(etag string) string { // azureObjects - Implements Object layer for Azure blob storage. type azureObjects struct { + gatewayUnsupported client storage.BlobStorageClient // Azure sdk client } @@ -610,11 +611,6 @@ func (a *azureObjects) NewMultipartUpload(bucket, object string, metadata map[st return uploadID, nil } -// CopyObjectPart - Not implemented. -func (a *azureObjects) CopyObjectPart(srcBucket, srcObject, destBucket, destObject string, uploadID string, partID int, startOffset int64, length int64) (info PartInfo, err error) { - return info, traceError(NotImplemented{}) -} - // PutObjectPart - Use Azure equivalent PutBlockWithLength. func (a *azureObjects) PutObjectPart(bucket, object, uploadID string, partID int, data *HashReader) (info PartInfo, err error) { if err = a.checkUploadIDExists(bucket, object, uploadID); err != nil { diff --git a/cmd/gateway-gcs-anonymous.go b/cmd/gateway-gcs-anonymous.go index b111b1948..b165ec2cf 100644 --- a/cmd/gateway-gcs-anonymous.go +++ b/cmd/gateway-gcs-anonymous.go @@ -28,12 +28,6 @@ func toGCSPublicURL(bucket, object string) string { return fmt.Sprintf("https://storage.googleapis.com/%s/%s", bucket, object) } -// AnonPutObject creates a new object anonymously with the incoming data, -func (l *gcsGateway) AnonPutObject(bucket, object string, size int64, data io.Reader, metadata map[string]string, sha256sum string) (ObjectInfo, error) { - - return ObjectInfo{}, NotImplemented{} -} - // AnonGetObject - Get object anonymously func (l *gcsGateway) AnonGetObject(bucket string, object string, startOffset int64, length int64, writer io.Writer) error { req, err := http.NewRequest("GET", toGCSPublicURL(bucket, object), nil) diff --git a/cmd/gateway-gcs-unsupported.go b/cmd/gateway-gcs-unsupported.go deleted file mode 100644 index 5c1b6fda8..000000000 --- a/cmd/gateway-gcs-unsupported.go +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Minio 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 cmd - -// HealBucket - Not relevant. -func (l *gcsGateway) HealBucket(bucket string) error { - return traceError(NotImplemented{}) -} - -// ListBucketsHeal - Not relevant. -func (l *gcsGateway) ListBucketsHeal() (buckets []BucketInfo, err error) { - return []BucketInfo{}, traceError(NotImplemented{}) -} - -// HealObject - Not relevant. -func (l *gcsGateway) HealObject(bucket string, object string) (int, int, error) { - return 0, 0, traceError(NotImplemented{}) -} - -// ListObjectsHeal - Not relevant. -func (l *gcsGateway) ListObjectsHeal(bucket string, prefix string, marker string, delimiter string, maxKeys int) (ListObjectsInfo, error) { - return ListObjectsInfo{}, traceError(NotImplemented{}) -} - -// ListUploadsHeal - Not relevant. -func (l *gcsGateway) ListUploadsHeal(bucket string, prefix string, marker string, uploadIDMarker string, delimiter string, maxUploads int) (ListMultipartsInfo, error) { - return ListMultipartsInfo{}, traceError(NotImplemented{}) -} diff --git a/cmd/gateway-gcs.go b/cmd/gateway-gcs.go index 9849e4bc3..2259648d3 100644 --- a/cmd/gateway-gcs.go +++ b/cmd/gateway-gcs.go @@ -239,6 +239,7 @@ func checkGCSProjectID(ctx context.Context, projectID string) error { // gcsGateway - Implements gateway for Minio and GCS compatible object storage servers. type gcsGateway struct { + gatewayUnsupported client *storage.Client anonClient *minio.Core projectID string diff --git a/cmd/gateway-s3-unsupported.go b/cmd/gateway-s3-unsupported.go deleted file mode 100644 index 4b9526565..000000000 --- a/cmd/gateway-s3-unsupported.go +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Minio 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 cmd - -// HealBucket - Not relevant. -func (l *s3Objects) HealBucket(bucket string) error { - return traceError(NotImplemented{}) -} - -// ListBucketsHeal - Not relevant. -func (l *s3Objects) ListBucketsHeal() (buckets []BucketInfo, err error) { - return []BucketInfo{}, traceError(NotImplemented{}) -} - -// HealObject - Not relevant. -func (l *s3Objects) HealObject(bucket string, object string) (int, int, error) { - return 0, 0, traceError(NotImplemented{}) -} - -// ListObjectsHeal - Not relevant. -func (l *s3Objects) ListObjectsHeal(bucket string, prefix string, marker string, delimiter string, maxKeys int) (loi ListObjectsInfo, e error) { - return loi, traceError(NotImplemented{}) -} - -// ListUploadsHeal - Not relevant. -func (l *s3Objects) ListUploadsHeal(bucket string, prefix string, marker string, uploadIDMarker string, delimiter string, maxUploads int) (lmi ListMultipartsInfo, e error) { - return lmi, traceError(NotImplemented{}) -} diff --git a/cmd/gateway-s3.go b/cmd/gateway-s3.go index d36c9bcef..a3d370d54 100644 --- a/cmd/gateway-s3.go +++ b/cmd/gateway-s3.go @@ -97,13 +97,13 @@ func s3ToObjectError(err error, params ...string) error { // s3Objects implements gateway for Minio and S3 compatible object storage servers. type s3Objects struct { + gatewayUnsupported Client *minio.Core anonClient *minio.Core } // newS3Gateway returns s3 gatewaylayer func newS3Gateway(host string) (GatewayLayer, error) { - var err error var endpoint string var secure = true @@ -461,12 +461,6 @@ func (l *s3Objects) NewMultipartUpload(bucket string, object string, metadata ma return uploadID, nil } -// CopyObjectPart copy part of object to other bucket and object -func (l *s3Objects) CopyObjectPart(srcBucket string, srcObject string, destBucket string, destObject string, uploadID string, partID int, startOffset int64, length int64) (info PartInfo, err error) { - // FIXME: implement CopyObjectPart - return PartInfo{}, traceError(NotImplemented{}) -} - // fromMinioClientObjectPart converts minio ObjectPart to PartInfo func fromMinioClientObjectPart(op minio.ObjectPart) PartInfo { return PartInfo{ diff --git a/cmd/gateway-azure-unsupported.go b/cmd/gateway-unsupported.go similarity index 50% rename from cmd/gateway-azure-unsupported.go rename to cmd/gateway-unsupported.go index e2aeeecdd..c257005ff 100644 --- a/cmd/gateway-azure-unsupported.go +++ b/cmd/gateway-unsupported.go @@ -16,28 +16,44 @@ package cmd +import "io" + +type gatewayUnsupported struct{} + +// CopyObjectPart - Not implemented. +func (a gatewayUnsupported) CopyObjectPart(srcBucket, srcObject, destBucket, destObject string, uploadID string, + partID int, startOffset int64, length int64) (info PartInfo, err error) { + return info, traceError(NotImplemented{}) +} + +// AnonPutObject creates a new object anonymously with the incoming data, +func (a gatewayUnsupported) AnonPutObject(bucket, object string, size int64, data io.Reader, + metadata map[string]string, sha256sum string) (ObjectInfo, error) { + return ObjectInfo{}, traceError(NotImplemented{}) +} + // HealBucket - Not relevant. -func (a *azureObjects) HealBucket(bucket string) error { +func (a gatewayUnsupported) HealBucket(bucket string) error { return traceError(NotImplemented{}) } // ListBucketsHeal - Not relevant. -func (a *azureObjects) ListBucketsHeal() (buckets []BucketInfo, err error) { +func (a gatewayUnsupported) ListBucketsHeal() (buckets []BucketInfo, err error) { return nil, traceError(NotImplemented{}) } // HealObject - Not relevant. -func (a *azureObjects) HealObject(bucket, object string) (int, int, error) { +func (a gatewayUnsupported) HealObject(bucket, object string) (int, int, error) { return 0, 0, traceError(NotImplemented{}) } // ListObjectsHeal - Not relevant. -func (a *azureObjects) ListObjectsHeal(bucket, prefix, marker, delimiter string, maxKeys int) (loi ListObjectsInfo, e error) { +func (a gatewayUnsupported) ListObjectsHeal(bucket, prefix, marker, delimiter string, maxKeys int) (loi ListObjectsInfo, e error) { return loi, traceError(NotImplemented{}) } // ListUploadsHeal - Not relevant. -func (a *azureObjects) ListUploadsHeal(bucket, prefix, marker, uploadIDMarker, +func (a gatewayUnsupported) ListUploadsHeal(bucket, prefix, marker, uploadIDMarker, delimiter string, maxUploads int) (lmi ListMultipartsInfo, e error) { return lmi, traceError(NotImplemented{}) } diff --git a/cmd/server_test.go b/cmd/server_test.go index bfa557de9..8dd59b54c 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -60,6 +60,17 @@ var _ = Suite(&TestSuiteCommon{serverType: "FS", signer: signerV4, secure: true} // Init and run test on XL backend. var _ = Suite(&TestSuiteCommon{serverType: "XL", signer: signerV4}) +func verifyError(c *C, response *http.Response, code, description string, statusCode int) { + data, err := ioutil.ReadAll(response.Body) + c.Assert(err, IsNil) + errorResponse := APIErrorResponse{} + err = xml.Unmarshal(data, &errorResponse) + c.Assert(err, IsNil) + c.Assert(errorResponse.Code, Equals, code) + c.Assert(errorResponse.Message, Equals, description) + c.Assert(response.StatusCode, Equals, statusCode) +} + // Setting up the test suite. // Starting the Test server with temporary FS backend. func (s *TestSuiteCommon) SetUpSuite(c *C) { diff --git a/cmd/server_utils_test.go b/cmd/server_utils_test.go deleted file mode 100644 index a500ad772..000000000 --- a/cmd/server_utils_test.go +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Minio Cloud Storage, (C) 2016 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 ( - "encoding/xml" - "fmt" - "io/ioutil" - "net" - "net/http" - - . "gopkg.in/check.v1" -) - -// concurreny level for certain parallel tests. -const ( - testConcurrencyLevel = 10 -) - -/// -/// Excerpts from @lsegal - https://github.com/aws/aws-sdk-js/issues/659#issuecomment-120477258 -/// -/// User-Agent: -/// -/// This is ignored from signing because signing this causes problems with generating pre-signed URLs -/// (that are executed by other agents) or when customers pass requests through proxies, which may -/// modify the user-agent. -/// -/// Content-Length: -/// -/// This is ignored from signing because generating a pre-signed URL should not provide a content-length -/// constraint, specifically when vending a S3 pre-signed PUT URL. The corollary to this is that when -/// sending regular requests (non-pre-signed), the signature contains a checksum of the body, which -/// implicitly validates the payload length (since changing the number of bytes would change the checksum) -/// and therefore this header is not valuable in the signature. -/// -/// Content-Type: -/// -/// Signing this header causes quite a number of problems in browser environments, where browsers -/// like to modify and normalize the content-type header in different ways. There is more information -/// on this in https://github.com/aws/aws-sdk-js/issues/244. Avoiding this field simplifies logic -/// and reduces the possibility of future bugs -/// -/// Authorization: -/// -/// Is skipped for obvious reasons -/// -var ignoredHeaders = map[string]bool{ - "Authorization": true, - "Content-Type": true, - "Content-Length": true, - "User-Agent": true, -} - -// Headers to ignore in streaming v4 -var ignoredStreamingHeaders = map[string]bool{ - "Authorization": true, - "Content-Type": true, - "Content-Md5": true, - "User-Agent": true, -} - -// calculateSignedChunkLength - calculates the length of chunk metadata -func calculateSignedChunkLength(chunkDataSize int64) int64 { - return int64(len(fmt.Sprintf("%x", chunkDataSize))) + - 17 + // ";chunk-signature=" - 64 + // e.g. "f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2" - 2 + // CRLF - chunkDataSize + - 2 // CRLF -} - -// calculateSignedChunkLength - calculates the length of the overall stream (data + metadata) -func calculateStreamContentLength(dataLen, chunkSize int64) int64 { - if dataLen <= 0 { - return 0 - } - chunksCount := int64(dataLen / chunkSize) - remainingBytes := int64(dataLen % chunkSize) - var streamLen int64 - streamLen += chunksCount * calculateSignedChunkLength(chunkSize) - if remainingBytes > 0 { - streamLen += calculateSignedChunkLength(remainingBytes) - } - streamLen += calculateSignedChunkLength(0) - return streamLen -} - -// Ask the kernel for a free open port. -func getFreePort() string { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - panic(err) - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - panic(err) - } - defer l.Close() - return fmt.Sprintf("%d", l.Addr().(*net.TCPAddr).Port) -} - -func verifyError(c *C, response *http.Response, code, description string, statusCode int) { - data, err := ioutil.ReadAll(response.Body) - c.Assert(err, IsNil) - errorResponse := APIErrorResponse{} - err = xml.Unmarshal(data, &errorResponse) - c.Assert(err, IsNil) - c.Assert(errorResponse.Code, Equals, code) - c.Assert(errorResponse.Message, Equals, description) - c.Assert(response.StatusCode, Equals, statusCode) -} diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 5d0c1edb4..efaa68083 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -73,6 +73,95 @@ func init() { log.logger.Hooks = nil } +// concurreny level for certain parallel tests. +const ( + testConcurrencyLevel = 10 +) + +/// +/// Excerpts from @lsegal - https://github.com/aws/aws-sdk-js/issues/659#issuecomment-120477258 +/// +/// User-Agent: +/// +/// This is ignored from signing because signing this causes problems with generating pre-signed URLs +/// (that are executed by other agents) or when customers pass requests through proxies, which may +/// modify the user-agent. +/// +/// Content-Length: +/// +/// This is ignored from signing because generating a pre-signed URL should not provide a content-length +/// constraint, specifically when vending a S3 pre-signed PUT URL. The corollary to this is that when +/// sending regular requests (non-pre-signed), the signature contains a checksum of the body, which +/// implicitly validates the payload length (since changing the number of bytes would change the checksum) +/// and therefore this header is not valuable in the signature. +/// +/// Content-Type: +/// +/// Signing this header causes quite a number of problems in browser environments, where browsers +/// like to modify and normalize the content-type header in different ways. There is more information +/// on this in https://github.com/aws/aws-sdk-js/issues/244. Avoiding this field simplifies logic +/// and reduces the possibility of future bugs +/// +/// Authorization: +/// +/// Is skipped for obvious reasons +/// +var ignoredHeaders = map[string]bool{ + "Authorization": true, + "Content-Type": true, + "Content-Length": true, + "User-Agent": true, +} + +// Headers to ignore in streaming v4 +var ignoredStreamingHeaders = map[string]bool{ + "Authorization": true, + "Content-Type": true, + "Content-Md5": true, + "User-Agent": true, +} + +// calculateSignedChunkLength - calculates the length of chunk metadata +func calculateSignedChunkLength(chunkDataSize int64) int64 { + return int64(len(fmt.Sprintf("%x", chunkDataSize))) + + 17 + // ";chunk-signature=" + 64 + // e.g. "f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2" + 2 + // CRLF + chunkDataSize + + 2 // CRLF +} + +// calculateSignedChunkLength - calculates the length of the overall stream (data + metadata) +func calculateStreamContentLength(dataLen, chunkSize int64) int64 { + if dataLen <= 0 { + return 0 + } + chunksCount := int64(dataLen / chunkSize) + remainingBytes := int64(dataLen % chunkSize) + var streamLen int64 + streamLen += chunksCount * calculateSignedChunkLength(chunkSize) + if remainingBytes > 0 { + streamLen += calculateSignedChunkLength(remainingBytes) + } + streamLen += calculateSignedChunkLength(0) + return streamLen +} + +// Ask the kernel for a free open port. +func getFreePort() string { + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + panic(err) + } + + l, err := net.ListenTCP("tcp", addr) + if err != nil { + panic(err) + } + defer l.Close() + return fmt.Sprintf("%d", l.Addr().(*net.TCPAddr).Port) +} + func prepareFS() (ObjectLayer, string, error) { nDisks := 1 fsDirs, err := getRandomDisks(nDisks) @@ -2299,3 +2388,24 @@ func randomizeBytes(s []byte, seed int64) []byte { } return s } + +func TestToErrIsNil(t *testing.T) { + if toObjectErr(nil) != nil { + t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", toObjectErr(nil)) + } + if toStorageErr(nil) != nil { + t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", toStorageErr(nil)) + } + if toAPIErrorCode(nil) != ErrNone { + t.Errorf("Test expected error code to be ErrNone, failed instead provided %d", toAPIErrorCode(nil)) + } + if s3ToObjectError(nil) != nil { + t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", s3ToObjectError(nil)) + } + if azureToObjectError(nil) != nil { + t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", azureToObjectError(nil)) + } + if gcsToObjectError(nil) != nil { + t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", gcsToObjectError(nil)) + } +} diff --git a/cmd/to_err_test.go b/cmd/to_err_test.go deleted file mode 100644 index 2311b3619..000000000 --- a/cmd/to_err_test.go +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Minio 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 cmd - -import "testing" - -func TestToErrIsNil(t *testing.T) { - if toObjectErr(nil) != nil { - t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", toObjectErr(nil)) - } - if toStorageErr(nil) != nil { - t.Errorf("Test expected to return nil, failed instead got a non-nil value %s", toStorageErr(nil)) - } - if toAPIErrorCode(nil) != ErrNone { - t.Errorf("Test expected error code to be ErrNone, failed instead provided %d", toAPIErrorCode(nil)) - } -}