fix: copilot review

Author: avirtopeanu-ionos

commands/object-storage/bucket/delete.go

@@ -73,13 +73,11 @@ func deleteAllBuckets(c *core.CommandConfig) error {
 
 	if viper.IsSet(constants.FlagLocation) {
 		filterRegion := viper.GetString(constants.FlagLocation)
-		buckets = functional.Filter(buckets, func(b objectstorage.Bucket) bool {
-			loc, _, locErr := s3.BucketsApi.GetBucketLocation(c.Context, b.GetName()).Execute()
-			if locErr != nil {
-				return false
-			}
-			return loc.GetLocationConstraint() == filterRegion
-		})
+		filtered, locErr := filterBucketsByLocation(c.Context, s3, buckets, filterRegion)
+		if locErr != nil {
+			return locErr
+		}
+		buckets = filtered
 	}
 
 	return functional.ApplyAndAggregateErrors(buckets, func(b objectstorage.Bucket) error {
@@ -98,3 +96,19 @@ func deleteAllBuckets(c *core.CommandConfig) error {
 		return nil
 	})
 }
+
+// filterBucketsByLocation filters buckets by region, returning an error if any
+// location lookup fails (to avoid silently skipping buckets from deletion).
+func filterBucketsByLocation(ctx context.Context, s3 *objectstorage.APIClient, buckets []objectstorage.Bucket, region string) ([]objectstorage.Bucket, error) {
+	var filtered []objectstorage.Bucket
+	for _, b := range buckets {
+		loc, _, locErr := s3.BucketsApi.GetBucketLocation(ctx, b.GetName()).Execute()
+		if locErr != nil {
+			return nil, fmt.Errorf("cannot filter by location: failed to get location for bucket %q: %w", b.GetName(), locErr)
+		}
+		if loc.GetLocationConstraint() == region {
+			filtered = append(filtered, b)
+		}
+	}
+	return filtered, nil
+}

commands/object-storage/bucket/get.go

@@ -42,8 +42,14 @@ func getBucketInfo(ctx context.Context, name string) (*bucketInfo, error) {
 		return nil, fmt.Errorf("bucket %q not found", name)
 	}
 
-	loc, _, err := client.MustObjectStorage().ObjectStorageClient.BucketsApi.GetBucketLocation(ctx, name).Execute()
-	if err == nil && loc != nil {
+	loc, apiResp, locErr := client.MustObjectStorage().ObjectStorageClient.BucketsApi.GetBucketLocation(ctx, name).Execute()
+	if locErr != nil {
+		if apiResp != nil {
+			found.Region = fmt.Sprintf("<%s>", apiResp.Status)
+		} else {
+			found.Region = "<unknown>"
+		}
+	} else {
 		found.Region = loc.GetLocationConstraint()
 	}
 

commands/object-storage/bucket/list.go

@@ -29,6 +29,9 @@ func ListBucketsCmd() *core.Command {
 				return err
 			}
 
+			filterByLocation := viper.IsSet(constants.FlagLocation)
+			filterRegion := viper.GetString(constants.FlagLocation)
+
 			var buckets []bucketInfo
 			for _, b := range result.GetBuckets() {
 
@@ -39,6 +42,13 @@ func ListBucketsCmd() *core.Command {
 
 				loc, apiResp, locErr := client.MustObjectStorage().ObjectStorageClient.BucketsApi.GetBucketLocation(c.Context, b.GetName()).Execute()
 				if locErr != nil {
+					// When filtering by location, a failed lookup would silently
+					// exclude the bucket. Fail loudly so the user knows results
+					// may be incomplete.
+					if filterByLocation {
+						return fmt.Errorf("cannot filter by location: failed to get location for bucket %q: %w", b.GetName(), locErr)
+					}
+
 					c.Verbose("failed to get location for bucket %q: %v", b.GetName(), locErr)
 					if apiResp != nil {
 						bi.Region = fmt.Sprintf("<%s>", apiResp.Status)
@@ -49,7 +59,7 @@ func ListBucketsCmd() *core.Command {
 					bi.Region = loc.GetLocationConstraint()
 				}
 
-				if viper.IsSet(constants.FlagLocation) && bi.Region != viper.GetString(constants.FlagLocation) {
+				if filterByLocation && bi.Region != filterRegion {
 					continue
 				}
 

commands/object-storage/bucket/policy/put.go

@@ -15,6 +15,9 @@ import (
 	"github.com/ionos-cloud/ionosctl/v6/internal/core"
 )
 
+// policyExample uses S3-compatible IAM policy syntax. The "s3:" action prefix
+// and "arn:aws:s3:::" resource format are required by the S3-compatible API,
+// not references to AWS services.
 const policyExample = `{
   "Version": "2012-10-17",
   "Statement": [

commands/object-storage/object/delete.go

@@ -3,6 +3,7 @@ package object
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/ionos-cloud/sdk-go-bundle/products/objectstorage/v2"
 	"github.com/spf13/viper"
@@ -200,9 +201,12 @@ func batchDelete(ctx context.Context, s3 *objectstorage.APIClient, bucket string
 	}
 
 	if result != nil && len(result.Errors) > 0 {
-		first := result.Errors[0]
-		return fmt.Errorf("failed to delete %d object(s): %s: %s (key: %s)",
-			len(result.Errors), first.GetCode(), first.GetMessage(), first.GetKey())
+		var details []string
+		for _, e := range result.Errors {
+			details = append(details, fmt.Sprintf("  %s: %s (key: %s)", e.GetCode(), e.GetMessage(), e.GetKey()))
+		}
+		return fmt.Errorf("failed to delete %d object(s):\n%s",
+			len(result.Errors), strings.Join(details, "\n"))
 	}
 
 	return nil

test/suites/setup.bats

@@ -193,38 +193,6 @@ refute_stderr() {
     fi
 }
 
-# assert_stderr: like assert_output but checks $stderr (captured via --separate-stderr).
-# Supports: assert_stderr -p "substring"  |  assert_stderr "exact match"
-assert_stderr() {
-    local mode="equal"
-    local expected
-
-    while [[ $# -gt 0 ]]; do
-        case "$1" in
-            -p|--partial) mode="partial"; shift ;;
-            *) expected="$1"; shift ;;
-        esac
-    done
-
-    if [[ "$mode" == "partial" ]]; then
-        if [[ "$stderr" != *"$expected"* ]]; then
-            echo "-- stderr does not contain substring --"
-            echo "substring : $expected"
-            echo "stderr    : $(echo "$stderr" | redact)"
-            echo "--"
-            return 1
-        fi
-    else
-        if [[ "$stderr" != "$expected" ]]; then
-            echo "-- stderr is not equal to expected --"
-            echo "expected : $expected"
-            echo "actual   : $(echo "$stderr" | redact)"
-            echo "--"
-            return 1
-        fi
-    fi
-}
-
 setup_file() {
     uuid_v4_regex='^[0-9a-f]{8}-[0-9a-f]{4}-[4][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$'
     ip_regex='^([0-9]{1,3}\.){3}[0-9]{1,3}(\/[0-9]{1,2})?$'