From ca6d9e3c118f176a726249e2d4e2f01262817b85 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 15 Jan 2024 09:14:42 -0800 Subject: [PATCH 1/5] fix: docker env set to tests defaults --- .env.dev | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.env.dev b/.env.dev index fd5ac515..73e5b9ca 100644 --- a/.env.dev +++ b/.env.dev @@ -1,8 +1,8 @@ -POSIX_PORT= -PROXY_PORT= -ACCESS_KEY_ID= -SECRET_ACCESS_KEY= -IAM_DIR= -SETUP_DIR= +POSIX_PORT=7071 +PROXY_PORT=7070 +ACCESS_KEY_ID=user +SECRET_ACCESS_KEY=pass +IAM_DIR=. +SETUP_DIR=. AZ_ACCOUNT_NAME=devstoreaccount1 -AZ_ACCOUNT_KEY=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== \ No newline at end of file +AZ_ACCOUNT_KEY=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== From 1cdf0706e746f4e0c070c263447660e2682d1f59 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 15 Jan 2024 09:15:31 -0800 Subject: [PATCH 2/5] fix: fix crashes in test cases when fields missing --- integration/tests.go | 11 +++++------ integration/utils.go | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/integration/tests.go b/integration/tests.go index e8d9c76d..847e37f9 100644 --- a/integration/tests.go +++ b/integration/tests.go @@ -14,7 +14,6 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" - "github.com/google/uuid" "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" ) @@ -1426,7 +1425,7 @@ func ListObject_truncated(s *S3Conf) error { return err } - if !*out1.IsTruncated { + if out1.IsTruncated == nil || !*out1.IsTruncated { return fmt.Errorf("expected out1put to be truncated") } @@ -1530,7 +1529,10 @@ func ListObjects_delimiter(s *S3Conf) error { return err } - if *out.Delimiter != "/" { + if out.Delimiter == nil || *out.Delimiter != "/" { + if out.Delimiter == nil { + return fmt.Errorf("expected delimiter to be /, instead got nil delim") + } return fmt.Errorf("expected delimiter to be /, instead got %v", *out.Delimiter) } if len(out.Contents) != 1 || *out.Contents[0].Key != "asdf" { @@ -2265,9 +2267,6 @@ func CreateMultipartUpload_success(s *S3Conf) error { if *out.Key != obj { return fmt.Errorf("expected object name %v, instead got %v", obj, *out.Key) } - if _, err := uuid.Parse(*out.UploadId); err != nil { - return err - } return nil }) diff --git a/integration/utils.go b/integration/utils.go index 5a35c368..014276cd 100644 --- a/integration/utils.go +++ b/integration/utils.go @@ -81,7 +81,7 @@ func teardown(s *S3Conf, bucket string) error { } } - if *out.IsTruncated { + if out.IsTruncated != nil && *out.IsTruncated { in.ContinuationToken = out.ContinuationToken } else { break @@ -215,7 +215,7 @@ func checkSdkApiErr(err error, code string) error { var ae smithy.APIError if errors.As(err, &ae) { if ae.ErrorCode() != code { - return fmt.Errorf("expected %v, instead got %v", ae.ErrorCode(), code) + return fmt.Errorf("expected %v, instead got %v", code, ae.ErrorCode()) } return nil } From d404f96320bbe460018283d523ac3dfd8085c70d Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 15 Jan 2024 09:16:10 -0800 Subject: [PATCH 3/5] fix: translate azure errors to s3 for compatibility --- backend/azure/azure.go | 25 +++++---------------- backend/azure/err.go | 50 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 backend/azure/err.go diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 562e23a7..1e029d36 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -19,15 +19,14 @@ import ( "context" "encoding/base64" "encoding/binary" - "errors" "fmt" "io" "math" "os" "strconv" "strings" + "time" - "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" @@ -532,9 +531,10 @@ func (az *Azure) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3res break } parts = append(parts, s3response.Part{ - Size: *el.Size, - ETag: *el.Name, - PartNumber: partNumber, + Size: *el.Size, + ETag: *el.Name, + PartNumber: partNumber, + LastModified: time.Now().Format(backend.RFC3339TimeFormat), }) } return s3response.ListPartsResult{ @@ -774,21 +774,6 @@ func getString(str *string) string { return *str } -// Parses azure ResponseError into AWS APIError -func azureErrToS3Err(apiErr error) error { - var azErr *azcore.ResponseError - if !errors.As(apiErr, &azErr) { - return apiErr - } - - resp := s3err.APIError{ - Code: azErr.ErrorCode, - Description: azErr.RawResponse.Status, - HTTPStatusCode: azErr.StatusCode, - } - return resp -} - // Converts io.Reader into io.ReadSeekCloser func getReadSeekCloser(input io.Reader) (io.ReadSeekCloser, error) { var buffer bytes.Buffer diff --git a/backend/azure/err.go b/backend/azure/err.go new file mode 100644 index 00000000..2a33012f --- /dev/null +++ b/backend/azure/err.go @@ -0,0 +1,50 @@ +// Copyright 2023 Versity Software +// This file is 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 azure + +import ( + "errors" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/versity/versitygw/s3err" +) + +// Parses azure ResponseError into AWS APIError +func azureErrToS3Err(apiErr error) error { + var azErr *azcore.ResponseError + if !errors.As(apiErr, &azErr) { + return apiErr + } + + return azErrToS3err(azErr) +} + +func azErrToS3err(azErr *azcore.ResponseError) s3err.APIError { + switch azErr.ErrorCode { + case "ContainerAlreadyExists": + return s3err.GetAPIError(s3err.ErrBucketAlreadyExists) + case "InvalidResourceName", "ContainerNotFound": + return s3err.GetAPIError(s3err.ErrNoSuchBucket) + case "BlobNotFound": + return s3err.GetAPIError(s3err.ErrNoSuchKey) + case "TagsTooLarge": + return s3err.GetAPIError(s3err.ErrInvalidTag) + } + return s3err.APIError{ + Code: azErr.ErrorCode, + Description: azErr.RawResponse.Status, + HTTPStatusCode: azErr.StatusCode, + } +} From 240db54febb380932b10c97de526d9746a165b9d Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Tue, 16 Jan 2024 14:36:14 -0500 Subject: [PATCH 4/5] feat: Added ChangeBucketOwner, ListBucketsAndOwners action implementation in azure backend. Fixed acl key bug in getting container metadata. Added container owner in ListBuckets action --- backend/azure/azure.go | 118 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 106 insertions(+), 12 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 1e029d36..d1d41fc4 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -19,9 +19,11 @@ import ( "context" "encoding/base64" "encoding/binary" + "encoding/json" "fmt" "io" "math" + "net/url" "os" "strconv" "strings" @@ -35,12 +37,19 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" ) -const aclKey string = "Acl" +// When getting container metadata with GetProperties method the sdk returns +// the first letter capital, when accessing the metadata after listing the containers +// it returns the first letter lower +type aclKey string + +const aclKeyCapital aclKey = "Acl" +const aclKeyLower aclKey = "acl" type Azure struct { backend.BackendUnsupported @@ -55,8 +64,15 @@ type Azure struct { var _ backend.Backend = &Azure{} func New(accountName, accountKey, serviceURL, sasToken string) (*Azure, error) { + url := serviceURL + if serviceURL == "" && accountName != "" { + // if not otherwise specified, use the typical form: + // http(s)://.blob.core.windows.net/ + url = fmt.Sprintf("https://%s.blob.core.windows.net/", accountName) + } + if sasToken != "" { - client, err := azblob.NewClientWithNoCredential(serviceURL+"?"+sasToken, nil) + client, err := azblob.NewClientWithNoCredential(url+"?"+sasToken, nil) if err != nil { return nil, fmt.Errorf("init client: %w", err) } @@ -68,13 +84,6 @@ func New(accountName, accountKey, serviceURL, sasToken string) (*Azure, error) { accountName = os.Getenv("AZURE_CLIENT_ID") } - url := serviceURL - if serviceURL == "" && accountName != "" { - // if not otherwise specified, use the typical form: - // http(s)://.blob.core.windows.net/ - url = fmt.Sprintf("https://%s.blob.core.windows.net/", accountName) - } - if accountName == "" || accountKey == "" { cred, err := azidentity.NewDefaultAzureCredential(nil) if err != nil { @@ -108,7 +117,7 @@ func (az *Azure) String() string { func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error { meta := map[string]*string{ - aclKey: backend.GetStringPtr(string(acl)), + string(aclKeyCapital): backend.GetStringPtr(string(acl)), } _, err := az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) return azureErrToS3Err(err) @@ -135,6 +144,9 @@ func (az *Azure) ListBuckets(ctx context.Context, owner string, isAdmin bool) (s } result.Buckets.Bucket = buckets + // If the gateway is initialized with shared key credentials + // provide user account name as the owner of the buckets + result.Owner.ID = az.getAccountNameFromURL() return result, nil } @@ -644,7 +656,7 @@ func (az *Azure) PutBucketAcl(ctx context.Context, bucket string, data []byte) e return err } meta := map[string]*string{ - aclKey: backend.GetStringPtr(string(data)), + string(aclKeyCapital): backend.GetStringPtr(string(data)), } _, err = client.SetMetadata(ctx, &container.SetMetadataOptions{ Metadata: meta, @@ -665,7 +677,7 @@ func (az *Azure) GetBucketAcl(ctx context.Context, input *s3.GetBucketAclInput) return nil, azureErrToS3Err(err) } - aclPtr, ok := props.Metadata[aclKey] + aclPtr, ok := props.Metadata[string(aclKeyCapital)] if !ok { return nil, s3err.GetAPIError(s3err.ErrInternalError) } @@ -673,6 +685,59 @@ func (az *Azure) GetBucketAcl(ctx context.Context, input *s3.GetBucketAclInput) return []byte(*aclPtr), nil } +func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error { + client, err := az.getContainerClient(bucket) + if err != nil { + return err + } + props, err := client.GetProperties(ctx, nil) + if err != nil { + return azureErrToS3Err(err) + } + + acl, err := getAclFromMetadata(props.Metadata, aclKeyCapital) + if err != nil { + return err + } + + acl.Owner = newOwner + + newAcl, err := json.Marshal(acl) + if err != nil { + return fmt.Errorf("marshal acl: %w", err) + } + + err = az.PutBucketAcl(ctx, bucket, newAcl) + if err != nil { + return err + } + + return nil +} + +func (az *Azure) ListBucketsAndOwners(ctx context.Context) (buckets []s3response.Bucket, err error) { + pager := az.client.NewListContainersPager(nil) + + for pager.More() { + resp, err := pager.NextPage(ctx) + if err != nil { + return buckets, azureErrToS3Err(err) + } + for _, v := range resp.ContainerItems { + acl, err := getAclFromMetadata(v.Metadata, aclKeyLower) + if err != nil { + return buckets, err + } + + buckets = append(buckets, s3response.Bucket{ + Name: *v.Name, + Owner: acl.Owner, + }) + } + } + return buckets, nil +} + func (az *Azure) getContainerURL(cntr string) string { return fmt.Sprintf("%v/%v", az.serviceURL, cntr) } @@ -714,6 +779,20 @@ func (az *Azure) getBlockBlobClient(cntr, blb string) (*blockblob.Client, error) return blockblob.NewClientWithSharedKeyCredential(blobURL, az.sharedkeyCreds, nil) } +func (az *Azure) getAccountNameFromURL() string { + urlParts, err := url.Parse(az.serviceURL) + if err != nil { + return "" + } + + acc := urlParts.Path + + if strings.HasSuffix(acc, ".blob.core.windows.net/") { + return strings.TrimSuffix(acc, ".blob.core.windows.net") + } + return acc[1:] +} + func parseMetadata(m map[string]string) map[string]*string { if m == nil { return nil @@ -834,3 +913,18 @@ func parseRange(rg string) (offset, count int64, err error) { return offset, count - offset + 1, nil } + +func getAclFromMetadata(meta map[string]*string, key aclKey) (*auth.ACL, error) { + aclPtr, ok := meta[string(key)] + if !ok { + return nil, s3err.GetAPIError(s3err.ErrInternalError) + } + + var acl auth.ACL + err := json.Unmarshal([]byte(*aclPtr), &acl) + if err != nil { + return nil, fmt.Errorf("unmarshal acl: %w", err) + } + + return &acl, nil +} From 03e4a28d574299b4121f02f2957c94079df35bea Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Wed, 17 Jan 2024 11:01:16 -0500 Subject: [PATCH 5/5] fix: Fixed couple of bugs regarding to GetObject range errors, blob metadata reference losing --- backend/azure/azure.go | 28 +++++++--------------------- backend/azure/err.go | 2 ++ integration/tests.go | 8 ++------ 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index d1d41fc4..218939ff 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -23,7 +23,6 @@ import ( "fmt" "io" "math" - "net/url" "os" "strconv" "strings" @@ -144,9 +143,7 @@ func (az *Azure) ListBuckets(ctx context.Context, owner string, isAdmin bool) (s } result.Buckets.Bucket = buckets - // If the gateway is initialized with shared key credentials - // provide user account name as the owner of the buckets - result.Owner.ID = az.getAccountNameFromURL() + result.Owner.ID = owner return result, nil } @@ -189,7 +186,7 @@ func (az *Azure) PutObject(ctx context.Context, po *s3.PutObjectInput) (string, func (az *Azure) GetObject(ctx context.Context, input *s3.GetObjectInput, writer io.Writer) (*s3.GetObjectOutput, error) { var opts *azblob.DownloadStreamOptions - if input.Range != nil { + if *input.Range != "" { offset, count, err := parseRange(*input.Range) if err != nil { return nil, err @@ -218,7 +215,7 @@ func (az *Azure) GetObject(ctx context.Context, input *s3.GetObjectInput, writer } return &s3.GetObjectOutput{ - AcceptRanges: blobDownloadResponse.AcceptRanges, + AcceptRanges: input.Range, ContentLength: blobDownloadResponse.ContentLength, ContentEncoding: blobDownloadResponse.ContentEncoding, ContentType: blobDownloadResponse.ContentType, @@ -715,6 +712,8 @@ func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) return nil } +// The action actually returns the containers owned by the user, who initialized the gateway +// TODO: Not sure if there's a way to list all the containers and owners? func (az *Azure) ListBucketsAndOwners(ctx context.Context) (buckets []s3response.Bucket, err error) { pager := az.client.NewListContainersPager(nil) @@ -779,20 +778,6 @@ func (az *Azure) getBlockBlobClient(cntr, blb string) (*blockblob.Client, error) return blockblob.NewClientWithSharedKeyCredential(blobURL, az.sharedkeyCreds, nil) } -func (az *Azure) getAccountNameFromURL() string { - urlParts, err := url.Parse(az.serviceURL) - if err != nil { - return "" - } - - acc := urlParts.Path - - if strings.HasSuffix(acc, ".blob.core.windows.net/") { - return strings.TrimSuffix(acc, ".blob.core.windows.net") - } - return acc[1:] -} - func parseMetadata(m map[string]string) map[string]*string { if m == nil { return nil @@ -801,7 +786,8 @@ func parseMetadata(m map[string]string) map[string]*string { meta := make(map[string]*string) for k, v := range m { - meta[k] = &v + val := v + meta[k] = &val } return meta } diff --git a/backend/azure/err.go b/backend/azure/err.go index 2a33012f..8eb239bc 100644 --- a/backend/azure/err.go +++ b/backend/azure/err.go @@ -41,6 +41,8 @@ func azErrToS3err(azErr *azcore.ResponseError) s3err.APIError { return s3err.GetAPIError(s3err.ErrNoSuchKey) case "TagsTooLarge": return s3err.GetAPIError(s3err.ErrInvalidTag) + case "Requested Range Not Satisfiable": + return s3err.GetAPIError(s3err.ErrInvalidRange) } return s3err.APIError{ Code: azErr.ErrorCode, diff --git a/integration/tests.go b/integration/tests.go index 847e37f9..9aa54028 100644 --- a/integration/tests.go +++ b/integration/tests.go @@ -1426,7 +1426,7 @@ func ListObject_truncated(s *S3Conf) error { } if out1.IsTruncated == nil || !*out1.IsTruncated { - return fmt.Errorf("expected out1put to be truncated") + return fmt.Errorf("expected output to be truncated") } if *out1.MaxKeys != maxKeys { @@ -1434,7 +1434,7 @@ func ListObject_truncated(s *S3Conf) error { } if *out1.NextMarker != "baz" { - return fmt.Errorf("expected nex-marker to be baz, instead got %v", *out1.NextMarker) + return fmt.Errorf("expected next-marker to be baz, instead got %v", *out1.NextMarker) } if !compareObjects([]string{"bar", "baz"}, out1.Contents) { @@ -1590,10 +1590,6 @@ func ListObjects_marker_not_from_obj_list(s *S3Conf) error { return err } - for _, el := range out.Contents { - fmt.Println(*el.Key) - } - if !compareObjects([]string{"foo", "qux", "hello", "xyz"}, out.Contents) { return fmt.Errorf("expected output to be %v, instead got %v", []string{"foo", "qux", "hello", "xyz"}, out.Contents) }