From dc0a15ede235fbe21eddb312a9b7f12d74a404fb Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 20 Nov 2025 09:38:56 +0100 Subject: [PATCH 1/9] Implement missing storage-cli operations for AliCloud Missing Operations: copy, delete-recursive, list, properties, ensure-bucket-exists. --- alioss/client/storage_client.go | 234 ++++++++++++++++++++++++- alioss/integration/general_ali_test.go | 163 +++++++++++++++++ 2 files changed, 395 insertions(+), 2 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 2813512..8e447c4 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -1,7 +1,13 @@ package client import ( + "encoding/json" + "errors" + "fmt" "log" + "strconv" + "strings" + "time" "github.com/aliyun/aliyun-oss-go-sdk/oss" "github.com/cloudfoundry/storage-cli/alioss/config" @@ -20,10 +26,19 @@ type StorageClient interface { destinationFilePath string, ) error + Copy( + srcBlob string, + destBlob string, + ) error + Delete( object string, ) error + DeleteRecursive( + dest string, + ) error + Exists( object string, ) (bool, error) @@ -37,14 +52,49 @@ type StorageClient interface { object string, expiredInSec int64, ) (string, error) -} + List( + prefix string, + ) ([]string, error) + + Properties( + dest string, + ) error + + EnsureBucketExists() error +} type DefaultStorageClient struct { storageConfig config.AliStorageConfig + client *oss.Client + bucket *oss.Bucket + bucketURL string } func NewStorageClient(storageConfig config.AliStorageConfig) (StorageClient, error) { - return DefaultStorageClient{storageConfig: storageConfig}, nil + client, err := oss.New( + storageConfig.Endpoint, + storageConfig.AccessKeyID, + storageConfig.AccessKeySecret, + ) + if err != nil { + return nil, err + } + + bucket, err := client.Bucket(storageConfig.BucketName) + if err != nil { + return nil, err + } + + endpoint := strings.TrimPrefix(storageConfig.Endpoint, "https://") + endpoint = strings.TrimPrefix(endpoint, "http://") + bucketURL := fmt.Sprintf("https://%s.%s", storageConfig.BucketName, endpoint) + + return DefaultStorageClient{ + storageConfig: storageConfig, + client: client, + bucket: bucket, + bucketURL: bucketURL, + }, nil } func (dsc DefaultStorageClient) Upload( @@ -86,6 +136,19 @@ func (dsc DefaultStorageClient) Download( return bucket.GetObjectToFile(sourceObject, destinationFilePath) } +func (dsc DefaultStorageClient) Copy( + srcObject string, + destObject string, +) error { + log.Printf("Copying object from %s to %s", srcObject, destObject) + + if _, err := dsc.bucket.CopyObject(srcObject, destObject); err != nil { + return fmt.Errorf("failed to copy object from %s to %s: %w", srcObject, destObject, err) + } + + return nil +} + func (dsc DefaultStorageClient) Delete( object string, ) error { @@ -104,6 +167,49 @@ func (dsc DefaultStorageClient) Delete( return bucket.DeleteObject(object) } +func (dsc DefaultStorageClient) DeleteRecursive( + prefix string, +) error { + if prefix != "" { + log.Printf("Deleting all objects in bucket %s with prefix '%s'\n", + dsc.storageConfig.BucketName, prefix) + } else { + log.Printf("Deleting all objects in bucket %s\n", + dsc.storageConfig.BucketName) + } + + marker := "" + + for { + var listOptions []oss.Option + if prefix != "" { + listOptions = append(listOptions, oss.Prefix(prefix)) + } + if marker != "" { + listOptions = append(listOptions, oss.Marker(marker)) + } + + resp, err := dsc.bucket.ListObjects(listOptions...) + if err != nil { + return fmt.Errorf("error listing objects: %w", err) + } + + for _, object := range resp.Objects { + if err := dsc.bucket.DeleteObject(object.Key); err != nil { + log.Printf("Failed to delete object %s: %v\n", object.Key, err) + } + } + + if !resp.IsTruncated { + break + } + + marker = resp.NextMarker + } + + return nil +} + func (dsc DefaultStorageClient) Exists(object string) (bool, error) { log.Printf("Checking if blob: %s/%s\n", dsc.storageConfig.BucketName, object) @@ -170,3 +276,127 @@ func (dsc DefaultStorageClient) SignedUrlGet( return bucket.SignURL(object, oss.HTTPGet, expiredInSec) } + +func (dsc DefaultStorageClient) List( + prefix string, +) ([]string, error) { + if prefix != "" { + log.Printf("Listing objects in bucket %s with prefix '%s'\n", + dsc.storageConfig.BucketName, prefix) + } else { + log.Printf("Listing objects in bucket %s\n", dsc.storageConfig.BucketName) + } + + var ( + objects []string + marker string + ) + + for { + var opts []oss.Option + if prefix != "" { + opts = append(opts, oss.Prefix(prefix)) + } + if marker != "" { + opts = append(opts, oss.Marker(marker)) + } + + resp, err := dsc.bucket.ListObjects(opts...) + if err != nil { + return nil, fmt.Errorf("error retrieving page of objects: %w", err) + } + + for _, obj := range resp.Objects { + objects = append(objects, obj.Key) + } + + if !resp.IsTruncated { + break + } + marker = resp.NextMarker + } + + return objects, nil +} + +type BlobProperties struct { + ETag string `json:"etag,omitempty"` + LastModified time.Time `json:"last_modified,omitempty"` + ContentLength int64 `json:"content_length,omitempty"` +} + +func (dsc DefaultStorageClient) Properties( + dest string, +) error { + log.Printf("Getting properties for object %s/%s\n", + dsc.storageConfig.BucketName, dest) + + meta, err := dsc.bucket.GetObjectDetailedMeta(dest) + if err != nil { + var ossErr oss.ServiceError + if errors.As(err, &ossErr) && ossErr.StatusCode == 404 { + fmt.Println(`{}`) + return nil + } + + return fmt.Errorf("failed to get properties for object %s: %w", dest, err) + } + + eTag := meta.Get("ETag") + lastModifiedStr := meta.Get("Last-Modified") + contentLengthStr := meta.Get("Content-Length") + + var ( + lastModified time.Time + contentLength int64 + ) + + if lastModifiedStr != "" { + t, parseErr := time.Parse(time.RFC1123, lastModifiedStr) + if parseErr == nil { + lastModified = t + } + } + + if contentLengthStr != "" { + cl, convErr := strconv.ParseInt(contentLengthStr, 10, 64) + if convErr == nil { + contentLength = cl + } + } + + props := BlobProperties{ + ETag: strings.Trim(eTag, `"`), + LastModified: lastModified, + ContentLength: contentLength, + } + + output, err := json.MarshalIndent(props, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal object properties: %w", err) + } + + fmt.Println(string(output)) + return nil +} + +func (dsc DefaultStorageClient) EnsureBucketExists() error { + log.Printf("Ensuring bucket '%s' exists\n", dsc.storageConfig.BucketName) + + exists, err := dsc.client.IsBucketExist(dsc.storageConfig.BucketName) + if err != nil { + return fmt.Errorf("failed to check if bucket exists: %w", err) + } + + if exists { + log.Printf("Bucket '%s' already exists\n", dsc.storageConfig.BucketName) + return nil + } + + if err := dsc.client.CreateBucket(dsc.storageConfig.BucketName); err != nil { + return fmt.Errorf("failed to create bucket '%s': %w", dsc.storageConfig.BucketName, err) + } + + log.Printf("Bucket '%s' created successfully\n", dsc.storageConfig.BucketName) + return nil +} diff --git a/alioss/integration/general_ali_test.go b/alioss/integration/general_ali_test.go index cb64833..baea23d 100644 --- a/alioss/integration/general_ali_test.go +++ b/alioss/integration/general_ali_test.go @@ -151,6 +151,105 @@ var _ = Describe("General testing for all Ali regions", func() { }) }) + Describe("Invoking `delete-recursive`", func() { + It("deletes all objects with a given prefix", func() { + prefix := integration.GenerateRandomString() + blob1 := prefix + "/a" + blob2 := prefix + "/b" + otherBlob := integration.GenerateRandomString() + + // Create a temp file for uploads + contentFile1 := integration.MakeContentFile("content-1") + contentFile2 := integration.MakeContentFile("content-2") + contentFileOther := integration.MakeContentFile("other-content") + defer func() { + _ = os.Remove(contentFile1) //nolint:errcheck + _ = os.Remove(contentFile2) //nolint:errcheck + _ = os.Remove(contentFileOther) //nolint:errcheck + // make sure all are gone + for _, b := range []string{blob1, blob2, otherBlob} { + cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + if err == nil && (cliSession.ExitCode() == 0 || cliSession.ExitCode() == 3) { + continue + } + } + }() + + // Put three blobs + cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFile2, blob2) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFileOther, otherBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + // Call delete-recursive + cliSession, err = integration.RunCli(cliPath, configPath, "delete-recursive", prefix) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + // Objects with prefix should be gone (exit code 3) + cliSession, err = integration.RunCli(cliPath, configPath, "exists", blob1) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(Equal(3)) + + cliSession, err = integration.RunCli(cliPath, configPath, "exists", blob2) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(Equal(3)) + + // Other blob should still exist (exit code 0) + cliSession, err = integration.RunCli(cliPath, configPath, "exists", otherBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(Equal(0)) + }) + }) + + Describe("Invoking `copy`", func() { + It("copies the contents from one object to another", func() { + srcBlob := blobName + "-src" + destBlob := blobName + "-dest" + + // Clean up both at the end + defer func() { + cliSession, err := integration.RunCli(cliPath, configPath, "delete", srcBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + cliSession, err = integration.RunCli(cliPath, configPath, "delete", destBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + }() + + // Upload source + contentFile = integration.MakeContentFile("copied content") + cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile, srcBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + // Invoke copy + cliSession, err = integration.RunCli(cliPath, configPath, "copy", srcBlob, destBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + // Download destination and verify content + tmpLocalFile, _ := os.CreateTemp("", "ali-storage-cli-copy") //nolint:errcheck + tmpLocalFile.Close() //nolint:errcheck + defer func() { _ = os.Remove(tmpLocalFile.Name()) }() //nolint:errcheck + + cliSession, err = integration.RunCli(cliPath, configPath, "get", destBlob, tmpLocalFile.Name()) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + gottenBytes, _ := os.ReadFile(tmpLocalFile.Name()) //nolint:errcheck + Expect(string(gottenBytes)).To(Equal("copied content")) + }) + }) + Describe("Invoking `exists`", func() { It("returns 0 for an existing blob", func() { defer func() { @@ -211,4 +310,68 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(consoleOutput).To(ContainSubstring("version")) }) }) + + Describe("Invoking `list`", func() { + It("lists all blobs with a given prefix", func() { + prefix := integration.GenerateRandomString() + blob1 := prefix + "/a" + blob2 := prefix + "/b" + otherBlob := integration.GenerateRandomString() + + defer func() { + for _, b := range []string{blob1, blob2, otherBlob} { + _, err := integration.RunCli(cliPath, configPath, "delete", b) + Expect(err).ToNot(HaveOccurred()) + } + }() + + contentFile1 := integration.MakeContentFile("list-1") + contentFile2 := integration.MakeContentFile("list-2") + contentFileOther := integration.MakeContentFile("list-other") + defer func() { + _ = os.Remove(contentFile1) //nolint:errcheck + _ = os.Remove(contentFile2) //nolint:errcheck + _ = os.Remove(contentFileOther) //nolint:errcheck + }() + + // Put blobs + cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFile2, blob2) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFileOther, otherBlob) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + // List with prefix + cliSession, err = integration.RunCli(cliPath, configPath, "list", prefix) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + output := bytes.NewBuffer(cliSession.Out.Contents()).String() + + Expect(output).To(ContainSubstring(blob1)) + Expect(output).To(ContainSubstring(blob2)) + Expect(output).NotTo(ContainSubstring(otherBlob)) + }) + }) + + Describe("Invoking `ensure-bucket-exists`", func() { + It("is idempotent", func() { + // first run + s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + Expect(err).ToNot(HaveOccurred()) + Expect(s1.ExitCode()).To(BeZero()) + + // second run should also succeed (bucket already exists) + s2, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + Expect(err).ToNot(HaveOccurred()) + Expect(s2.ExitCode()).To(BeZero()) + }) + }) + }) From 65eda966d805b65c1e0d0a926f7223db78355a53 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 20 Nov 2025 10:25:05 +0100 Subject: [PATCH 2/9] add fake storage client --- .../client/clientfakes/fake_storage_client.go | 370 +++++++++++++++++- 1 file changed, 358 insertions(+), 12 deletions(-) diff --git a/alioss/client/clientfakes/fake_storage_client.go b/alioss/client/clientfakes/fake_storage_client.go index b963907..2630565 100644 --- a/alioss/client/clientfakes/fake_storage_client.go +++ b/alioss/client/clientfakes/fake_storage_client.go @@ -8,6 +8,18 @@ import ( ) type FakeStorageClient struct { + CopyStub func(string, string) error + copyMutex sync.RWMutex + copyArgsForCall []struct { + arg1 string + arg2 string + } + copyReturns struct { + result1 error + } + copyReturnsOnCall map[int]struct { + result1 error + } DeleteStub func(string) error deleteMutex sync.RWMutex deleteArgsForCall []struct { @@ -19,6 +31,17 @@ type FakeStorageClient struct { deleteReturnsOnCall map[int]struct { result1 error } + DeleteRecursiveStub func(string) error + deleteRecursiveMutex sync.RWMutex + deleteRecursiveArgsForCall []struct { + arg1 string + } + deleteRecursiveReturns struct { + result1 error + } + deleteRecursiveReturnsOnCall map[int]struct { + result1 error + } DownloadStub func(string, string) error downloadMutex sync.RWMutex downloadArgsForCall []struct { @@ -31,6 +54,16 @@ type FakeStorageClient struct { downloadReturnsOnCall map[int]struct { result1 error } + EnsureBucketExistsStub func() error + ensureBucketExistsMutex sync.RWMutex + ensureBucketExistsArgsForCall []struct { + } + ensureBucketExistsReturns struct { + result1 error + } + ensureBucketExistsReturnsOnCall map[int]struct { + result1 error + } ExistsStub func(string) (bool, error) existsMutex sync.RWMutex existsArgsForCall []struct { @@ -44,6 +77,30 @@ type FakeStorageClient struct { result1 bool result2 error } + ListStub func(string) ([]string, error) + listMutex sync.RWMutex + listArgsForCall []struct { + arg1 string + } + listReturns struct { + result1 []string + result2 error + } + listReturnsOnCall map[int]struct { + result1 []string + result2 error + } + PropertiesStub func(string) error + propertiesMutex sync.RWMutex + propertiesArgsForCall []struct { + arg1 string + } + propertiesReturns struct { + result1 error + } + propertiesReturnsOnCall map[int]struct { + result1 error + } SignedUrlGetStub func(string, int64) (string, error) signedUrlGetMutex sync.RWMutex signedUrlGetArgsForCall []struct { @@ -89,6 +146,68 @@ type FakeStorageClient struct { invocationsMutex sync.RWMutex } +func (fake *FakeStorageClient) Copy(arg1 string, arg2 string) error { + fake.copyMutex.Lock() + ret, specificReturn := fake.copyReturnsOnCall[len(fake.copyArgsForCall)] + fake.copyArgsForCall = append(fake.copyArgsForCall, struct { + arg1 string + arg2 string + }{arg1, arg2}) + stub := fake.CopyStub + fakeReturns := fake.copyReturns + fake.recordInvocation("Copy", []interface{}{arg1, arg2}) + fake.copyMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeStorageClient) CopyCallCount() int { + fake.copyMutex.RLock() + defer fake.copyMutex.RUnlock() + return len(fake.copyArgsForCall) +} + +func (fake *FakeStorageClient) CopyCalls(stub func(string, string) error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = stub +} + +func (fake *FakeStorageClient) CopyArgsForCall(i int) (string, string) { + fake.copyMutex.RLock() + defer fake.copyMutex.RUnlock() + argsForCall := fake.copyArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeStorageClient) CopyReturns(result1 error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = nil + fake.copyReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStorageClient) CopyReturnsOnCall(i int, result1 error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = nil + if fake.copyReturnsOnCall == nil { + fake.copyReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.copyReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStorageClient) Delete(arg1 string) error { fake.deleteMutex.Lock() ret, specificReturn := fake.deleteReturnsOnCall[len(fake.deleteArgsForCall)] @@ -150,6 +269,67 @@ func (fake *FakeStorageClient) DeleteReturnsOnCall(i int, result1 error) { }{result1} } +func (fake *FakeStorageClient) DeleteRecursive(arg1 string) error { + fake.deleteRecursiveMutex.Lock() + ret, specificReturn := fake.deleteRecursiveReturnsOnCall[len(fake.deleteRecursiveArgsForCall)] + fake.deleteRecursiveArgsForCall = append(fake.deleteRecursiveArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.DeleteRecursiveStub + fakeReturns := fake.deleteRecursiveReturns + fake.recordInvocation("DeleteRecursive", []interface{}{arg1}) + fake.deleteRecursiveMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeStorageClient) DeleteRecursiveCallCount() int { + fake.deleteRecursiveMutex.RLock() + defer fake.deleteRecursiveMutex.RUnlock() + return len(fake.deleteRecursiveArgsForCall) +} + +func (fake *FakeStorageClient) DeleteRecursiveCalls(stub func(string) error) { + fake.deleteRecursiveMutex.Lock() + defer fake.deleteRecursiveMutex.Unlock() + fake.DeleteRecursiveStub = stub +} + +func (fake *FakeStorageClient) DeleteRecursiveArgsForCall(i int) string { + fake.deleteRecursiveMutex.RLock() + defer fake.deleteRecursiveMutex.RUnlock() + argsForCall := fake.deleteRecursiveArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStorageClient) DeleteRecursiveReturns(result1 error) { + fake.deleteRecursiveMutex.Lock() + defer fake.deleteRecursiveMutex.Unlock() + fake.DeleteRecursiveStub = nil + fake.deleteRecursiveReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStorageClient) DeleteRecursiveReturnsOnCall(i int, result1 error) { + fake.deleteRecursiveMutex.Lock() + defer fake.deleteRecursiveMutex.Unlock() + fake.DeleteRecursiveStub = nil + if fake.deleteRecursiveReturnsOnCall == nil { + fake.deleteRecursiveReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.deleteRecursiveReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStorageClient) Download(arg1 string, arg2 string) error { fake.downloadMutex.Lock() ret, specificReturn := fake.downloadReturnsOnCall[len(fake.downloadArgsForCall)] @@ -212,6 +392,59 @@ func (fake *FakeStorageClient) DownloadReturnsOnCall(i int, result1 error) { }{result1} } +func (fake *FakeStorageClient) EnsureBucketExists() error { + fake.ensureBucketExistsMutex.Lock() + ret, specificReturn := fake.ensureBucketExistsReturnsOnCall[len(fake.ensureBucketExistsArgsForCall)] + fake.ensureBucketExistsArgsForCall = append(fake.ensureBucketExistsArgsForCall, struct { + }{}) + stub := fake.EnsureBucketExistsStub + fakeReturns := fake.ensureBucketExistsReturns + fake.recordInvocation("EnsureBucketExists", []interface{}{}) + fake.ensureBucketExistsMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeStorageClient) EnsureBucketExistsCallCount() int { + fake.ensureBucketExistsMutex.RLock() + defer fake.ensureBucketExistsMutex.RUnlock() + return len(fake.ensureBucketExistsArgsForCall) +} + +func (fake *FakeStorageClient) EnsureBucketExistsCalls(stub func() error) { + fake.ensureBucketExistsMutex.Lock() + defer fake.ensureBucketExistsMutex.Unlock() + fake.EnsureBucketExistsStub = stub +} + +func (fake *FakeStorageClient) EnsureBucketExistsReturns(result1 error) { + fake.ensureBucketExistsMutex.Lock() + defer fake.ensureBucketExistsMutex.Unlock() + fake.EnsureBucketExistsStub = nil + fake.ensureBucketExistsReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStorageClient) EnsureBucketExistsReturnsOnCall(i int, result1 error) { + fake.ensureBucketExistsMutex.Lock() + defer fake.ensureBucketExistsMutex.Unlock() + fake.EnsureBucketExistsStub = nil + if fake.ensureBucketExistsReturnsOnCall == nil { + fake.ensureBucketExistsReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.ensureBucketExistsReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStorageClient) Exists(arg1 string) (bool, error) { fake.existsMutex.Lock() ret, specificReturn := fake.existsReturnsOnCall[len(fake.existsArgsForCall)] @@ -276,6 +509,131 @@ func (fake *FakeStorageClient) ExistsReturnsOnCall(i int, result1 bool, result2 }{result1, result2} } +func (fake *FakeStorageClient) List(arg1 string) ([]string, error) { + fake.listMutex.Lock() + ret, specificReturn := fake.listReturnsOnCall[len(fake.listArgsForCall)] + fake.listArgsForCall = append(fake.listArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ListStub + fakeReturns := fake.listReturns + fake.recordInvocation("List", []interface{}{arg1}) + fake.listMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStorageClient) ListCallCount() int { + fake.listMutex.RLock() + defer fake.listMutex.RUnlock() + return len(fake.listArgsForCall) +} + +func (fake *FakeStorageClient) ListCalls(stub func(string) ([]string, error)) { + fake.listMutex.Lock() + defer fake.listMutex.Unlock() + fake.ListStub = stub +} + +func (fake *FakeStorageClient) ListArgsForCall(i int) string { + fake.listMutex.RLock() + defer fake.listMutex.RUnlock() + argsForCall := fake.listArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStorageClient) ListReturns(result1 []string, result2 error) { + fake.listMutex.Lock() + defer fake.listMutex.Unlock() + fake.ListStub = nil + fake.listReturns = struct { + result1 []string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) ListReturnsOnCall(i int, result1 []string, result2 error) { + fake.listMutex.Lock() + defer fake.listMutex.Unlock() + fake.ListStub = nil + if fake.listReturnsOnCall == nil { + fake.listReturnsOnCall = make(map[int]struct { + result1 []string + result2 error + }) + } + fake.listReturnsOnCall[i] = struct { + result1 []string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) Properties(arg1 string) error { + fake.propertiesMutex.Lock() + ret, specificReturn := fake.propertiesReturnsOnCall[len(fake.propertiesArgsForCall)] + fake.propertiesArgsForCall = append(fake.propertiesArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.PropertiesStub + fakeReturns := fake.propertiesReturns + fake.recordInvocation("Properties", []interface{}{arg1}) + fake.propertiesMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeStorageClient) PropertiesCallCount() int { + fake.propertiesMutex.RLock() + defer fake.propertiesMutex.RUnlock() + return len(fake.propertiesArgsForCall) +} + +func (fake *FakeStorageClient) PropertiesCalls(stub func(string) error) { + fake.propertiesMutex.Lock() + defer fake.propertiesMutex.Unlock() + fake.PropertiesStub = stub +} + +func (fake *FakeStorageClient) PropertiesArgsForCall(i int) string { + fake.propertiesMutex.RLock() + defer fake.propertiesMutex.RUnlock() + argsForCall := fake.propertiesArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStorageClient) PropertiesReturns(result1 error) { + fake.propertiesMutex.Lock() + defer fake.propertiesMutex.Unlock() + fake.PropertiesStub = nil + fake.propertiesReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStorageClient) PropertiesReturnsOnCall(i int, result1 error) { + fake.propertiesMutex.Lock() + defer fake.propertiesMutex.Unlock() + fake.PropertiesStub = nil + if fake.propertiesReturnsOnCall == nil { + fake.propertiesReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.propertiesReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStorageClient) SignedUrlGet(arg1 string, arg2 int64) (string, error) { fake.signedUrlGetMutex.Lock() ret, specificReturn := fake.signedUrlGetReturnsOnCall[len(fake.signedUrlGetArgsForCall)] @@ -472,18 +830,6 @@ func (fake *FakeStorageClient) UploadReturnsOnCall(i int, result1 error) { func (fake *FakeStorageClient) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.deleteMutex.RLock() - defer fake.deleteMutex.RUnlock() - fake.downloadMutex.RLock() - defer fake.downloadMutex.RUnlock() - fake.existsMutex.RLock() - defer fake.existsMutex.RUnlock() - fake.signedUrlGetMutex.RLock() - defer fake.signedUrlGetMutex.RUnlock() - fake.signedUrlPutMutex.RLock() - defer fake.signedUrlPutMutex.RUnlock() - fake.uploadMutex.RLock() - defer fake.uploadMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value From 6ee510a647ea9ec6ee06bbded35587231d561025 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 25 Nov 2025 18:12:13 +0100 Subject: [PATCH 3/9] clean up comments --- alioss/integration/general_ali_test.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/alioss/integration/general_ali_test.go b/alioss/integration/general_ali_test.go index baea23d..e300cd9 100644 --- a/alioss/integration/general_ali_test.go +++ b/alioss/integration/general_ali_test.go @@ -158,7 +158,6 @@ var _ = Describe("General testing for all Ali regions", func() { blob2 := prefix + "/b" otherBlob := integration.GenerateRandomString() - // Create a temp file for uploads contentFile1 := integration.MakeContentFile("content-1") contentFile2 := integration.MakeContentFile("content-2") contentFileOther := integration.MakeContentFile("other-content") @@ -166,7 +165,7 @@ var _ = Describe("General testing for all Ali regions", func() { _ = os.Remove(contentFile1) //nolint:errcheck _ = os.Remove(contentFile2) //nolint:errcheck _ = os.Remove(contentFileOther) //nolint:errcheck - // make sure all are gone + for _, b := range []string{blob1, blob2, otherBlob} { cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) if err == nil && (cliSession.ExitCode() == 0 || cliSession.ExitCode() == 3) { @@ -175,7 +174,6 @@ var _ = Describe("General testing for all Ali regions", func() { } }() - // Put three blobs cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -188,12 +186,10 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - // Call delete-recursive cliSession, err = integration.RunCli(cliPath, configPath, "delete-recursive", prefix) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - // Objects with prefix should be gone (exit code 3) cliSession, err = integration.RunCli(cliPath, configPath, "exists", blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(3)) @@ -202,7 +198,6 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(3)) - // Other blob should still exist (exit code 0) cliSession, err = integration.RunCli(cliPath, configPath, "exists", otherBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(0)) @@ -214,7 +209,6 @@ var _ = Describe("General testing for all Ali regions", func() { srcBlob := blobName + "-src" destBlob := blobName + "-dest" - // Clean up both at the end defer func() { cliSession, err := integration.RunCli(cliPath, configPath, "delete", srcBlob) Expect(err).ToNot(HaveOccurred()) @@ -225,18 +219,15 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(cliSession.ExitCode()).To(BeZero()) }() - // Upload source contentFile = integration.MakeContentFile("copied content") cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile, srcBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - // Invoke copy cliSession, err = integration.RunCli(cliPath, configPath, "copy", srcBlob, destBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - // Download destination and verify content tmpLocalFile, _ := os.CreateTemp("", "ali-storage-cli-copy") //nolint:errcheck tmpLocalFile.Close() //nolint:errcheck defer func() { _ = os.Remove(tmpLocalFile.Name()) }() //nolint:errcheck @@ -334,7 +325,6 @@ var _ = Describe("General testing for all Ali regions", func() { _ = os.Remove(contentFileOther) //nolint:errcheck }() - // Put blobs cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -347,7 +337,6 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - // List with prefix cliSession, err = integration.RunCli(cliPath, configPath, "list", prefix) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -362,12 +351,10 @@ var _ = Describe("General testing for all Ali regions", func() { Describe("Invoking `ensure-bucket-exists`", func() { It("is idempotent", func() { - // first run s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") Expect(err).ToNot(HaveOccurred()) Expect(s1.ExitCode()).To(BeZero()) - // second run should also succeed (bucket already exists) s2, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") Expect(err).ToNot(HaveOccurred()) Expect(s2.ExitCode()).To(BeZero()) From 329ca9b7bfc0a18684e33eb64090bffebd6d3774 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 4 Dec 2025 15:51:05 +0100 Subject: [PATCH 4/9] enhancement as proposed from review --- alioss/client/storage_client.go | 34 +++++----- alioss/integration/general_ali_test.go | 91 ++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 24 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 8e447c4..4442a85 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -36,7 +36,7 @@ type StorageClient interface { ) error DeleteRecursive( - dest string, + objects string, ) error Exists( @@ -54,11 +54,11 @@ type StorageClient interface { ) (string, error) List( - prefix string, + bucketPrefix string, ) ([]string, error) Properties( - dest string, + object string, ) error EnsureBucketExists() error @@ -137,13 +137,13 @@ func (dsc DefaultStorageClient) Download( } func (dsc DefaultStorageClient) Copy( - srcObject string, - destObject string, + sourceObject string, + destinationObject string, ) error { - log.Printf("Copying object from %s to %s", srcObject, destObject) + log.Printf("Copying object from %s to %s", sourceObject, destinationObject) - if _, err := dsc.bucket.CopyObject(srcObject, destObject); err != nil { - return fmt.Errorf("failed to copy object from %s to %s: %w", srcObject, destObject, err) + if _, err := dsc.bucket.CopyObject(sourceObject, destinationObject); err != nil { + return fmt.Errorf("failed to copy object from %s to %s: %w", sourceObject, destinationObject, err) } return nil @@ -168,11 +168,11 @@ func (dsc DefaultStorageClient) Delete( } func (dsc DefaultStorageClient) DeleteRecursive( - prefix string, + objectPrefix string, ) error { - if prefix != "" { + if objectPrefix != "" { log.Printf("Deleting all objects in bucket %s with prefix '%s'\n", - dsc.storageConfig.BucketName, prefix) + dsc.storageConfig.BucketName, objectPrefix) } else { log.Printf("Deleting all objects in bucket %s\n", dsc.storageConfig.BucketName) @@ -182,8 +182,8 @@ func (dsc DefaultStorageClient) DeleteRecursive( for { var listOptions []oss.Option - if prefix != "" { - listOptions = append(listOptions, oss.Prefix(prefix)) + if objectPrefix != "" { + listOptions = append(listOptions, oss.Prefix(objectPrefix)) } if marker != "" { listOptions = append(listOptions, oss.Marker(marker)) @@ -326,12 +326,12 @@ type BlobProperties struct { } func (dsc DefaultStorageClient) Properties( - dest string, + bucketObject string, ) error { log.Printf("Getting properties for object %s/%s\n", - dsc.storageConfig.BucketName, dest) + dsc.storageConfig.BucketName, bucketObject) - meta, err := dsc.bucket.GetObjectDetailedMeta(dest) + meta, err := dsc.bucket.GetObjectDetailedMeta(bucketObject) if err != nil { var ossErr oss.ServiceError if errors.As(err, &ossErr) && ossErr.StatusCode == 404 { @@ -339,7 +339,7 @@ func (dsc DefaultStorageClient) Properties( return nil } - return fmt.Errorf("failed to get properties for object %s: %w", dest, err) + return fmt.Errorf("failed to get properties for object %s: %w", bucketObject, err) } eTag := meta.Get("ETag") diff --git a/alioss/integration/general_ali_test.go b/alioss/integration/general_ali_test.go index e300cd9..a028612 100644 --- a/alioss/integration/general_ali_test.go +++ b/alioss/integration/general_ali_test.go @@ -2,9 +2,11 @@ package integration_test import ( "bytes" + "fmt" "os" + "github.com/aliyun/aliyun-oss-go-sdk/oss" "github.com/cloudfoundry/storage-cli/alioss/config" "github.com/cloudfoundry/storage-cli/alioss/integration" @@ -210,13 +212,16 @@ var _ = Describe("General testing for all Ali regions", func() { destBlob := blobName + "-dest" defer func() { - cliSession, err := integration.RunCli(cliPath, configPath, "delete", srcBlob) - Expect(err).ToNot(HaveOccurred()) - Expect(cliSession.ExitCode()).To(BeZero()) - - cliSession, err = integration.RunCli(cliPath, configPath, "delete", destBlob) - Expect(err).ToNot(HaveOccurred()) - Expect(cliSession.ExitCode()).To(BeZero()) + for _, b := range []string{srcBlob, destBlob} { + cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + if err != nil { + GinkgoWriter.Printf("cleanup: error deleting %s: %v\n", b, err) + continue + } + if cliSession.ExitCode() != 0 && cliSession.ExitCode() != 3 { + GinkgoWriter.Printf("cleanup: delete %s exited with code %d\n", b, cliSession.ExitCode()) + } + } }() contentFile = integration.MakeContentFile("copied content") @@ -347,9 +352,81 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(output).To(ContainSubstring(blob2)) Expect(output).NotTo(ContainSubstring(otherBlob)) }) + + It("lists all blobs across multiple pages", func() { + prefix := integration.GenerateRandomString() + const totalObjects = 120 + + var blobNames []string + var contentFiles []string + + for i := 0; i < totalObjects; i++ { + blobName := fmt.Sprintf("%s/%03d", prefix, i) + blobNames = append(blobNames, blobName) + + contentFile := integration.MakeContentFile(fmt.Sprintf("content-%d", i)) + contentFiles = append(contentFiles, contentFile) + + cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile, blobName) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + } + + defer func() { + for _, f := range contentFiles { + _ = os.Remove(f) //nolint:errcheck + } + + for _, b := range blobNames { + cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + if err == nil && (cliSession.ExitCode() == 0 || cliSession.ExitCode() == 3) { + continue + } + } + }() + + cliSession, err := integration.RunCli(cliPath, configPath, "list", prefix) + Expect(err).ToNot(HaveOccurred()) + Expect(cliSession.ExitCode()).To(BeZero()) + + output := bytes.NewBuffer(cliSession.Out.Contents()).String() + + for _, b := range blobNames { + Expect(output).To(ContainSubstring(b)) + } + }) }) Describe("Invoking `ensure-bucket-exists`", func() { + It("creates a bucket that can be observed via the OSS API", func() { + newBucketName := bucketName + "-bommel" + + cfg := defaultConfig + cfg.BucketName = newBucketName + + configPath = integration.MakeConfigFile(&cfg) + defer func() { _ = os.Remove(configPath) }() //nolint:errcheck + + ossClient, err := oss.New(cfg.Endpoint, cfg.AccessKeyID, cfg.AccessKeySecret) + Expect(err).ToNot(HaveOccurred()) + + defer func() { + if err := ossClient.DeleteBucket(newBucketName); err != nil { + if _, ferr := fmt.Fprintf(GinkgoWriter, "cleanup: failed to delete bucket %s: %v\n", newBucketName, err); ferr != nil { + GinkgoWriter.Printf("cleanup: failed to write cleanup message: %v\n", ferr) + } + } + }() + + s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + Expect(err).ToNot(HaveOccurred()) + Expect(s1.ExitCode()).To(BeZero()) + + exists, err := ossClient.IsBucketExist(newBucketName) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + It("is idempotent", func() { s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") Expect(err).ToNot(HaveOccurred()) From 4df1287415eb0a583738bcce082ddae6f473e540 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 8 Dec 2025 12:34:29 +0100 Subject: [PATCH 5/9] create a new bucket instance in methods for consistency with existing code --- alioss/client/storage_client.go | 78 ++++++++++++++++++++------ alioss/integration/general_ali_test.go | 43 +++++++++++--- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 4442a85..14302d7 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -141,9 +141,21 @@ func (dsc DefaultStorageClient) Copy( destinationObject string, ) error { log.Printf("Copying object from %s to %s", sourceObject, destinationObject) + srcURL := fmt.Sprintf("%s/%s", dsc.bucketURL, sourceObject) + destURL := fmt.Sprintf("%s/%s", dsc.bucketURL, destinationObject) - if _, err := dsc.bucket.CopyObject(sourceObject, destinationObject); err != nil { - return fmt.Errorf("failed to copy object from %s to %s: %w", sourceObject, destinationObject, err) + client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + if err != nil { + return err + } + + bucket, err := client.Bucket(dsc.storageConfig.BucketName) + if err != nil { + return err + } + + if _, err := bucket.CopyObject(sourceObject, destinationObject); err != nil { + return fmt.Errorf("failed to copy object from %s to %s: %w", srcURL, destURL, err) } return nil @@ -168,11 +180,11 @@ func (dsc DefaultStorageClient) Delete( } func (dsc DefaultStorageClient) DeleteRecursive( - objectPrefix string, + prefix string, ) error { - if objectPrefix != "" { + if prefix != "" { log.Printf("Deleting all objects in bucket %s with prefix '%s'\n", - dsc.storageConfig.BucketName, objectPrefix) + dsc.storageConfig.BucketName, prefix) } else { log.Printf("Deleting all objects in bucket %s\n", dsc.storageConfig.BucketName) @@ -182,20 +194,29 @@ func (dsc DefaultStorageClient) DeleteRecursive( for { var listOptions []oss.Option - if objectPrefix != "" { - listOptions = append(listOptions, oss.Prefix(objectPrefix)) + if prefix != "" { + listOptions = append(listOptions, oss.Prefix(prefix)) } if marker != "" { listOptions = append(listOptions, oss.Marker(marker)) } + client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + if err != nil { + return err + } - resp, err := dsc.bucket.ListObjects(listOptions...) + bucket, err := client.Bucket(dsc.storageConfig.BucketName) + if err != nil { + return err + } + + resp, err := bucket.ListObjects(listOptions...) if err != nil { return fmt.Errorf("error listing objects: %w", err) } for _, object := range resp.Objects { - if err := dsc.bucket.DeleteObject(object.Key); err != nil { + if err := bucket.DeleteObject(object.Key); err != nil { log.Printf("Failed to delete object %s: %v\n", object.Key, err) } } @@ -301,7 +322,17 @@ func (dsc DefaultStorageClient) List( opts = append(opts, oss.Marker(marker)) } - resp, err := dsc.bucket.ListObjects(opts...) + client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + if err != nil { + return nil, err + } + + bucket, err := client.Bucket(dsc.storageConfig.BucketName) + if err != nil { + return nil, err + } + + resp, err := bucket.ListObjects(opts...) if err != nil { return nil, fmt.Errorf("error retrieving page of objects: %w", err) } @@ -326,12 +357,22 @@ type BlobProperties struct { } func (dsc DefaultStorageClient) Properties( - bucketObject string, + object string, ) error { log.Printf("Getting properties for object %s/%s\n", - dsc.storageConfig.BucketName, bucketObject) + dsc.storageConfig.BucketName, object) + + client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + if err != nil { + return err + } - meta, err := dsc.bucket.GetObjectDetailedMeta(bucketObject) + bucket, err := client.Bucket(dsc.storageConfig.BucketName) + if err != nil { + return err + } + + meta, err := bucket.GetObjectDetailedMeta(object) if err != nil { var ossErr oss.ServiceError if errors.As(err, &ossErr) && ossErr.StatusCode == 404 { @@ -339,7 +380,7 @@ func (dsc DefaultStorageClient) Properties( return nil } - return fmt.Errorf("failed to get properties for object %s: %w", bucketObject, err) + return fmt.Errorf("failed to get properties for object %s: %w", object, err) } eTag := meta.Get("ETag") @@ -383,7 +424,12 @@ func (dsc DefaultStorageClient) Properties( func (dsc DefaultStorageClient) EnsureBucketExists() error { log.Printf("Ensuring bucket '%s' exists\n", dsc.storageConfig.BucketName) - exists, err := dsc.client.IsBucketExist(dsc.storageConfig.BucketName) + client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + if err != nil { + return err + } + + exists, err := client.IsBucketExist(dsc.storageConfig.BucketName) if err != nil { return fmt.Errorf("failed to check if bucket exists: %w", err) } @@ -393,7 +439,7 @@ func (dsc DefaultStorageClient) EnsureBucketExists() error { return nil } - if err := dsc.client.CreateBucket(dsc.storageConfig.BucketName); err != nil { + if err := client.CreateBucket(dsc.storageConfig.BucketName); err != nil { return fmt.Errorf("failed to create bucket '%s': %w", dsc.storageConfig.BucketName, err) } diff --git a/alioss/integration/general_ali_test.go b/alioss/integration/general_ali_test.go index a028612..e12a933 100644 --- a/alioss/integration/general_ali_test.go +++ b/alioss/integration/general_ali_test.go @@ -3,6 +3,9 @@ package integration_test import ( "bytes" "fmt" + "regexp" + "strings" + "time" "os" @@ -397,9 +400,33 @@ var _ = Describe("General testing for all Ali regions", func() { }) }) + const maxBucketLen = 63 + const suffixLen = 8 + Describe("Invoking `ensure-bucket-exists`", func() { It("creates a bucket that can be observed via the OSS API", func() { - newBucketName := bucketName + "-bommel" + + base := bucketName + + maxBaseLen := maxBucketLen - 1 - suffixLen + if maxBaseLen < 1 { + maxBaseLen = maxBucketLen - 1 + } + if len(base) > maxBaseLen { + base = base[:maxBaseLen] + } + + rawSuffix := integration.GenerateRandomString() + safe := strings.ToLower(rawSuffix) + + re := regexp.MustCompile(`[^a-z0-9]`) + safe = re.ReplaceAllString(safe, "") + if len(safe) < suffixLen { + safe = safe + "0123456789abcdef" + } + safe = safe[:suffixLen] + + newBucketName := fmt.Sprintf("%s-%s", base, safe) cfg := defaultConfig cfg.BucketName = newBucketName @@ -412,9 +439,10 @@ var _ = Describe("General testing for all Ali regions", func() { defer func() { if err := ossClient.DeleteBucket(newBucketName); err != nil { - if _, ferr := fmt.Fprintf(GinkgoWriter, "cleanup: failed to delete bucket %s: %v\n", newBucketName, err); ferr != nil { - GinkgoWriter.Printf("cleanup: failed to write cleanup message: %v\n", ferr) + if svcErr, ok := err.(oss.ServiceError); ok && svcErr.StatusCode == 404 { + return } + fmt.Fprintf(GinkgoWriter, "cleanup: failed to delete bucket %s: %v\n", newBucketName, err) //nolint:errcheck } }() @@ -422,9 +450,11 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(err).ToNot(HaveOccurred()) Expect(s1.ExitCode()).To(BeZero()) - exists, err := ossClient.IsBucketExist(newBucketName) - Expect(err).ToNot(HaveOccurred()) - Expect(exists).To(BeTrue()) + Eventually(func(g Gomega) bool { + exists, existsErr := ossClient.IsBucketExist(newBucketName) + g.Expect(existsErr).ToNot(HaveOccurred()) + return exists + }, 30*time.Second, 1*time.Second).Should(BeTrue()) }) It("is idempotent", func() { @@ -437,5 +467,4 @@ var _ = Describe("General testing for all Ali regions", func() { Expect(s2.ExitCode()).To(BeZero()) }) }) - }) From ee97fc4b13d181929103eb7550bc246543173c17 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 11 Dec 2025 13:16:07 +0100 Subject: [PATCH 6/9] adapt tests and client to new one main --- alioss/client/client.go | 11 +++--- alioss/client/storage_client.go | 2 +- alioss/integration/general_ali_test.go | 46 +++++++++++++------------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/alioss/client/client.go b/alioss/client/client.go index 712a42a..644db96 100644 --- a/alioss/client/client.go +++ b/alioss/client/client.go @@ -3,7 +3,6 @@ package client import ( "crypto/md5" "encoding/base64" - "errors" "fmt" "io" "log" @@ -80,21 +79,21 @@ func (client *AliBlobstore) getMD5(filePath string) (string, error) { } func (client *AliBlobstore) List(prefix string) ([]string, error) { - return nil, errors.New("not implemented") + return client.storageClient.List(prefix) } func (client *AliBlobstore) Copy(srcBlob string, dstBlob string) error { - return errors.New("not implemented") + return client.storageClient.Copy(srcBlob, dstBlob) } func (client *AliBlobstore) Properties(dest string) error { - return errors.New("not implemented") + return client.storageClient.Properties(dest) } func (client *AliBlobstore) EnsureStorageExists() error { - return errors.New("not implemented") + return client.storageClient.EnsureBucketExists() } func (client *AliBlobstore) DeleteRecursive(prefix string) error { - return errors.New("not implemented") + return client.storageClient.DeleteRecursive(prefix) } diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 14302d7..05bb0d3 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -54,7 +54,7 @@ type StorageClient interface { ) (string, error) List( - bucketPrefix string, + prefix string, ) ([]string, error) Properties( diff --git a/alioss/integration/general_ali_test.go b/alioss/integration/general_ali_test.go index e12a933..8a15d9f 100644 --- a/alioss/integration/general_ali_test.go +++ b/alioss/integration/general_ali_test.go @@ -172,38 +172,38 @@ var _ = Describe("General testing for all Ali regions", func() { _ = os.Remove(contentFileOther) //nolint:errcheck for _, b := range []string{blob1, blob2, otherBlob} { - cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "delete", b) if err == nil && (cliSession.ExitCode() == 0 || cliSession.ExitCode() == 3) { continue } } }() - cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "put", contentFile1, blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFile2, blob2) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "put", contentFile2, blob2) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFileOther, otherBlob) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "put", contentFileOther, otherBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "delete-recursive", prefix) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "delete-recursive", prefix) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "exists", blob1) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "exists", blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(3)) - cliSession, err = integration.RunCli(cliPath, configPath, "exists", blob2) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "exists", blob2) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(3)) - cliSession, err = integration.RunCli(cliPath, configPath, "exists", otherBlob) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "exists", otherBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(Equal(0)) }) @@ -216,7 +216,7 @@ var _ = Describe("General testing for all Ali regions", func() { defer func() { for _, b := range []string{srcBlob, destBlob} { - cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "delete", b) if err != nil { GinkgoWriter.Printf("cleanup: error deleting %s: %v\n", b, err) continue @@ -228,11 +228,11 @@ var _ = Describe("General testing for all Ali regions", func() { }() contentFile = integration.MakeContentFile("copied content") - cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile, srcBlob) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "put", contentFile, srcBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "copy", srcBlob, destBlob) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "copy", srcBlob, destBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -240,7 +240,7 @@ var _ = Describe("General testing for all Ali regions", func() { tmpLocalFile.Close() //nolint:errcheck defer func() { _ = os.Remove(tmpLocalFile.Name()) }() //nolint:errcheck - cliSession, err = integration.RunCli(cliPath, configPath, "get", destBlob, tmpLocalFile.Name()) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "get", destBlob, tmpLocalFile.Name()) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -319,7 +319,7 @@ var _ = Describe("General testing for all Ali regions", func() { defer func() { for _, b := range []string{blob1, blob2, otherBlob} { - _, err := integration.RunCli(cliPath, configPath, "delete", b) + _, err := integration.RunCli(cliPath, configPath, storageType, "delete", b) Expect(err).ToNot(HaveOccurred()) } }() @@ -333,19 +333,19 @@ var _ = Describe("General testing for all Ali regions", func() { _ = os.Remove(contentFileOther) //nolint:errcheck }() - cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile1, blob1) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "put", contentFile1, blob1) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFile2, blob2) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "put", contentFile2, blob2) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "put", contentFileOther, otherBlob) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "put", contentFileOther, otherBlob) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) - cliSession, err = integration.RunCli(cliPath, configPath, "list", prefix) + cliSession, err = integration.RunCli(cliPath, configPath, storageType, "list", prefix) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -370,7 +370,7 @@ var _ = Describe("General testing for all Ali regions", func() { contentFile := integration.MakeContentFile(fmt.Sprintf("content-%d", i)) contentFiles = append(contentFiles, contentFile) - cliSession, err := integration.RunCli(cliPath, configPath, "put", contentFile, blobName) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "put", contentFile, blobName) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) } @@ -381,14 +381,14 @@ var _ = Describe("General testing for all Ali regions", func() { } for _, b := range blobNames { - cliSession, err := integration.RunCli(cliPath, configPath, "delete", b) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "delete", b) if err == nil && (cliSession.ExitCode() == 0 || cliSession.ExitCode() == 3) { continue } } }() - cliSession, err := integration.RunCli(cliPath, configPath, "list", prefix) + cliSession, err := integration.RunCli(cliPath, configPath, storageType, "list", prefix) Expect(err).ToNot(HaveOccurred()) Expect(cliSession.ExitCode()).To(BeZero()) @@ -446,7 +446,7 @@ var _ = Describe("General testing for all Ali regions", func() { } }() - s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + s1, err := integration.RunCli(cliPath, configPath, storageType, "ensure-storage-exists") Expect(err).ToNot(HaveOccurred()) Expect(s1.ExitCode()).To(BeZero()) @@ -458,11 +458,11 @@ var _ = Describe("General testing for all Ali regions", func() { }) It("is idempotent", func() { - s1, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + s1, err := integration.RunCli(cliPath, configPath, storageType, "ensure-storage-exists") Expect(err).ToNot(HaveOccurred()) Expect(s1.ExitCode()).To(BeZero()) - s2, err := integration.RunCli(cliPath, configPath, "ensure-bucket-exists") + s2, err := integration.RunCli(cliPath, configPath, storageType, "ensure-storage-exists") Expect(err).ToNot(HaveOccurred()) Expect(s2.ExitCode()).To(BeZero()) }) From 340f5845c6f4c78c5da28916858c20a42e399c10 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 16 Dec 2025 10:53:12 +0100 Subject: [PATCH 7/9] remove obsolete parameters --- alioss/client/storage_client.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 05bb0d3..c43b07f 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -65,25 +65,10 @@ type StorageClient interface { } type DefaultStorageClient struct { storageConfig config.AliStorageConfig - client *oss.Client - bucket *oss.Bucket bucketURL string } func NewStorageClient(storageConfig config.AliStorageConfig) (StorageClient, error) { - client, err := oss.New( - storageConfig.Endpoint, - storageConfig.AccessKeyID, - storageConfig.AccessKeySecret, - ) - if err != nil { - return nil, err - } - - bucket, err := client.Bucket(storageConfig.BucketName) - if err != nil { - return nil, err - } endpoint := strings.TrimPrefix(storageConfig.Endpoint, "https://") endpoint = strings.TrimPrefix(endpoint, "http://") @@ -91,8 +76,6 @@ func NewStorageClient(storageConfig config.AliStorageConfig) (StorageClient, err return DefaultStorageClient{ storageConfig: storageConfig, - client: client, - bucket: bucket, bucketURL: bucketURL, }, nil } From 10a1a3165c95dff5a71b092a7ce8af9dfe5c2d17 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 16 Dec 2025 11:16:16 +0100 Subject: [PATCH 8/9] use DeleteObjects instead of DeleteObject --- alioss/client/storage_client.go | 52 +++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index c43b07f..0ca2b91 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -169,45 +169,61 @@ func (dsc DefaultStorageClient) DeleteRecursive( log.Printf("Deleting all objects in bucket %s with prefix '%s'\n", dsc.storageConfig.BucketName, prefix) } else { - log.Printf("Deleting all objects in bucket %s\n", - dsc.storageConfig.BucketName) + log.Printf("Deleting all objects in bucket %s\n", dsc.storageConfig.BucketName) } - marker := "" + client, err := oss.New( + dsc.storageConfig.Endpoint, + dsc.storageConfig.AccessKeyID, + dsc.storageConfig.AccessKeySecret, + ) + if err != nil { + return err + } + + bucket, err := client.Bucket(dsc.storageConfig.BucketName) + if err != nil { + return err + } + + var marker string for { - var listOptions []oss.Option + opts := []oss.Option{ + oss.MaxKeys(1000), + } if prefix != "" { - listOptions = append(listOptions, oss.Prefix(prefix)) + opts = append(opts, oss.Prefix(prefix)) } if marker != "" { - listOptions = append(listOptions, oss.Marker(marker)) + opts = append(opts, oss.Marker(marker)) } - client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) + + resp, err := bucket.ListObjects(opts...) if err != nil { - return err + return fmt.Errorf("error listing objects for delete: %w", err) } - bucket, err := client.Bucket(dsc.storageConfig.BucketName) - if err != nil { - return err + if len(resp.Objects) == 0 && !resp.IsTruncated { + return nil } - resp, err := bucket.ListObjects(listOptions...) - if err != nil { - return fmt.Errorf("error listing objects: %w", err) + keys := make([]string, 0, len(resp.Objects)) + for _, obj := range resp.Objects { + keys = append(keys, obj.Key) } - for _, object := range resp.Objects { - if err := bucket.DeleteObject(object.Key); err != nil { - log.Printf("Failed to delete object %s: %v\n", object.Key, err) + if len(keys) > 0 { + quiet := true + _, err := bucket.DeleteObjects(keys, oss.DeleteObjectsQuiet(quiet)) + if err != nil { + return fmt.Errorf("failed to batch delete %d objects (prefix=%q): %w", len(keys), prefix, err) } } if !resp.IsTruncated { break } - marker = resp.NextMarker } From 8d041981a1bbe7c327e86b52a9328ac4fd950bd0 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:49:47 +0100 Subject: [PATCH 9/9] remove obsolete bucketURL --- alioss/client/storage_client.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/alioss/client/storage_client.go b/alioss/client/storage_client.go index 0ca2b91..0ada623 100644 --- a/alioss/client/storage_client.go +++ b/alioss/client/storage_client.go @@ -65,18 +65,12 @@ type StorageClient interface { } type DefaultStorageClient struct { storageConfig config.AliStorageConfig - bucketURL string } func NewStorageClient(storageConfig config.AliStorageConfig) (StorageClient, error) { - endpoint := strings.TrimPrefix(storageConfig.Endpoint, "https://") - endpoint = strings.TrimPrefix(endpoint, "http://") - bucketURL := fmt.Sprintf("https://%s.%s", storageConfig.BucketName, endpoint) - return DefaultStorageClient{ storageConfig: storageConfig, - bucketURL: bucketURL, }, nil } @@ -124,8 +118,8 @@ func (dsc DefaultStorageClient) Copy( destinationObject string, ) error { log.Printf("Copying object from %s to %s", sourceObject, destinationObject) - srcURL := fmt.Sprintf("%s/%s", dsc.bucketURL, sourceObject) - destURL := fmt.Sprintf("%s/%s", dsc.bucketURL, destinationObject) + srcOut := fmt.Sprintf("%s/%s", dsc.storageConfig.BucketName, sourceObject) + destOut := fmt.Sprintf("%s/%s", dsc.storageConfig.BucketName, destinationObject) client, err := oss.New(dsc.storageConfig.Endpoint, dsc.storageConfig.AccessKeyID, dsc.storageConfig.AccessKeySecret) if err != nil { @@ -138,7 +132,7 @@ func (dsc DefaultStorageClient) Copy( } if _, err := bucket.CopyObject(sourceObject, destinationObject); err != nil { - return fmt.Errorf("failed to copy object from %s to %s: %w", srcURL, destURL, err) + return fmt.Errorf("failed to copy object from %s to %s: %w", srcOut, destOut, err) } return nil