create a new bucket instance in methods for consistency with existing code

Author: kathap

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,20 +357,30 @@ 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 {
 			fmt.Println(`{}`)
 			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)
 	}
 

alioss/integration/general_ali_test.go

@@ -3,6 +3,9 @@ package integration_test
 import (
 	"bytes"
 	"fmt"
+	"regexp"
+	"strings"
+	"time"
 
 	"os"
 
@@ -396,9 +399,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
@@ -411,19 +438,22 @@ 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
 				}
 			}()
 
 			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())
+			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() {
@@ -436,5 +466,4 @@ var _ = Describe("General testing for all Ali regions", func() {
 			Expect(s2.ExitCode()).To(BeZero())
 		})
 	})
-
 })