fix: as suggested switched to using StringSlice and cache_shared.go

Author: wolfeidau

clicommand/cache_shared.go

@@ -0,0 +1,62 @@
+package clicommand
+
+import "github.com/urfave/cli"
+
+// CacheConfig includes cache-related shared options for easy inclusion across
+// cache command config structs (via embedding).
+type CacheConfig struct {
+	Ids             []string `cli:"ids"`
+	Registry        string   `cli:"registry"`
+	BucketURL       string   `cli:"bucket-url" validate:"required"`
+	Branch          string   `cli:"branch" validate:"required"`
+	Pipeline        string   `cli:"pipeline" validate:"required"`
+	Organization    string   `cli:"organization" validate:"required"`
+	CacheConfigFile string   `cli:"cache-config-file"`
+}
+
+func cacheFlags() []cli.Flag {
+	return []cli.Flag{
+		cli.StringSliceFlag{
+			Name:   "ids",
+			Value:  &cli.StringSlice{},
+			Usage:  "Comma-separated list of cache IDs (if empty, processes all caches)",
+			EnvVar: "BUILDKITE_CACHE_IDS",
+		},
+		cli.StringFlag{
+			Name:   "registry",
+			Value:  "~",
+			Usage:  "The slug of the cache registry to use, defaults to the default registry (~)",
+			EnvVar: "BUILDKITE_CACHE_REGISTRY",
+		},
+		cli.StringFlag{
+			Name:   "bucket-url",
+			Value:  "",
+			Usage:  "The URL of the bucket (e.g., s3://bucket-name)",
+			EnvVar: "BUILDKITE_CACHE_BUCKET_URL",
+		},
+		cli.StringFlag{
+			Name:   "branch",
+			Value:  "",
+			Usage:  "Which branch should the cache be associated with",
+			EnvVar: "BUILDKITE_BRANCH",
+		},
+		cli.StringFlag{
+			Name:   "pipeline",
+			Value:  "",
+			Usage:  "The pipeline slug for this cache",
+			EnvVar: "BUILDKITE_PIPELINE_SLUG",
+		},
+		cli.StringFlag{
+			Name:   "organization",
+			Value:  "",
+			Usage:  "The organization slug for this cache",
+			EnvVar: "BUILDKITE_ORGANIZATION_SLUG",
+		},
+		cli.StringFlag{
+			Name:   "cache-config-file",
+			Value:  ".buildkite/cache.yml",
+			Usage:  "Path to the cache configuration YAML file",
+			EnvVar: "BUILDKITE_CACHE_CONFIG_FILE",
+		},
+	}
+}

clicommand/global.go

@@ -182,18 +182,6 @@ type APIConfig struct {
 	NoHTTP2          bool   `cli:"no-http2"`
 }
 
-// CacheConfig includes cache-related shared options for easy inclusion across
-// cache command config structs (via embedding).
-type CacheConfig struct {
-	Ids             string `cli:"ids"`
-	Registry        string `cli:"registry"`
-	BucketURL       string `cli:"bucket-url" validate:"required"`
-	Branch          string `cli:"branch" validate:"required"`
-	Pipeline        string `cli:"pipeline" validate:"required"`
-	Organization    string `cli:"organization" validate:"required"`
-	CacheConfigFile string `cli:"cache-config-file"`
-}
-
 func globalFlags() []cli.Flag {
 	return []cli.Flag{
 		NoColorFlag,
@@ -214,53 +202,6 @@ func apiFlags() []cli.Flag {
 	}
 }
 
-func cacheFlags() []cli.Flag {
-	return []cli.Flag{
-		cli.StringFlag{
-			Name:   "ids",
-			Value:  "",
-			Usage:  "Comma-separated list of cache IDs (if empty, processes all caches)",
-			EnvVar: "BUILDKITE_CACHE_IDS",
-		},
-		cli.StringFlag{
-			Name:   "registry",
-			Value:  "~",
-			Usage:  "The slug of the cache registry to use, defaults to the default registry (~)",
-			EnvVar: "BUILDKITE_CACHE_REGISTRY",
-		},
-		cli.StringFlag{
-			Name:   "bucket-url",
-			Value:  "",
-			Usage:  "The URL of the bucket (e.g., s3://bucket-name)",
-			EnvVar: "BUILDKITE_CACHE_BUCKET_URL",
-		},
-		cli.StringFlag{
-			Name:   "branch",
-			Value:  "",
-			Usage:  "Which branch should the cache be associated with",
-			EnvVar: "BUILDKITE_BRANCH",
-		},
-		cli.StringFlag{
-			Name:   "pipeline",
-			Value:  "",
-			Usage:  "The pipeline slug for this cache",
-			EnvVar: "BUILDKITE_PIPELINE_SLUG",
-		},
-		cli.StringFlag{
-			Name:   "organization",
-			Value:  "",
-			Usage:  "The organization slug for this cache",
-			EnvVar: "BUILDKITE_ORGANIZATION_SLUG",
-		},
-		cli.StringFlag{
-			Name:   "cache-config-file",
-			Value:  ".buildkite/cache.yml",
-			Usage:  "Path to the cache configuration YAML file",
-			EnvVar: "BUILDKITE_CACHE_CONFIG_FILE",
-		},
-	}
-}
-
 func CreateLogger(cfg any) logger.Logger {
 	var l logger.Logger
 	logFormat := "text"

internal/cache/cache.go

@@ -27,8 +27,8 @@ type Config struct {
 	Organization string
 	// CacheConfigFile is the path to the cache configuration YAML file
 	CacheConfigFile string
-	// Ids is a comma-separated list of cache IDs (if empty, processes all caches)
-	Ids string
+	// Ids is a list of cache IDs (if empty, processes all caches)
+	Ids []string
 	// APIEndpoint is the Agent API endpoint
 	APIEndpoint string
 	// APIToken is the access token used to authenticate
@@ -128,18 +128,15 @@ func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash
 	}
 
 	// Determine which cache IDs to process
-	var cacheIDs []string
-	if cfg.Ids != "" {
-		cacheIDs = strings.Split(cfg.Ids, ",")
-
+	if len(cfg.Ids) > 0 {
 		// Validate that specified cache IDs exist
 		validIDs := make(map[string]bool)
 		for _, cache := range cacheClient.ListCaches() {
 			validIDs[cache.ID] = true
 		}
 
 		var invalidIDs []string
-		for _, id := range cacheIDs {
+		for _, id := range cfg.Ids {
 			if !validIDs[id] {
 				invalidIDs = append(invalidIDs, id)
 			}
@@ -148,14 +145,16 @@ func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash
 		if len(invalidIDs) > 0 {
 			return nil, nil, fmt.Errorf("cache IDs not found in configuration: %s", strings.Join(invalidIDs, ", "))
 		}
-	} else {
-		// Process all caches configured in the client
-		for _, cache := range cacheClient.ListCaches() {
-			cacheIDs = append(cacheIDs, cache.ID)
-		}
+
+		return cacheClient, cfg.Ids, nil
+	}
+
+	var cacheDs []string
+	for _, cache := range cacheClient.ListCaches() {
+		cacheDs = append(cacheDs, cache.ID)
 	}
 
-	return cacheClient, cacheIDs, nil
+	return cacheClient, cacheDs, nil
 }
 
 // restoreWithClient performs the restore operation for the given cache IDs using the provided client

internal/cache/cache_test.go

@@ -368,7 +368,7 @@ func TestSetupCacheClient_InvalidCacheIDs(t *testing.T) {
 
 	cfg := Config{
 		CacheConfigFile: configFile,
-		Ids:             "cache1,invalid1,cache2,invalid2",
+		Ids:             []string{"cache1", "invalid1", "cache2", "invalid2"},
 		BucketURL:       "s3://test-bucket",
 		Branch:          "main",
 		Pipeline:        "test-pipeline",
@@ -402,7 +402,7 @@ func TestSetupCacheClient_ValidCacheIDs(t *testing.T) {
 
 	cfg := Config{
 		CacheConfigFile: configFile,
-		Ids:             "cache1,cache2",
+		Ids:             []string{"cache1", "cache2"},
 		BucketURL:       "s3://test-bucket",
 		Branch:          "main",
 		Pipeline:        "test-pipeline",
@@ -435,7 +435,7 @@ func TestSetupCacheClient_AllCaches(t *testing.T) {
 
 	cfg := Config{
 		CacheConfigFile: configFile,
-		Ids:             "",
+		Ids:             []string{},
 		BucketURL:       "s3://test-bucket",
 		Branch:          "main",
 		Pipeline:        "test-pipeline",