From 8396bccd5279deeb8663dea1c7370c882292baad Mon Sep 17 00:00:00 2001
From: Fabian Sommerauer <sommerauer@google.com>
Date: Fri, 6 Nov 2026 15:56:05 +0000
Subject: [PATCH] cmd: accept multi-value volume options and labels

`volume create -o` and `-l` were string slice flags, which split every
value on commas, and each item was then split on "=" with parts[1] read
unchecked. `-o options=rbind,rslave,ro` was split into "options=rbind",
"rslave" and "ro", and the CLI panicked with an index out of range on
"rslave". Mount options such as uid=1000 were turned into separate keys,
and an entry without "=" (e.g. `-l foo`) also panicked.

- Use string array flags: each -o/-l is one key=value entry, repeat the
  flag for multiple entries (same as `docker volume create -o`).
- Split each entry at the first "=" only, so values may contain "," and
  "=".
- Return an error for entries without "=" or with an empty key instead
  of panicking.

Behaviour change: several entries in one flag (`-o k1=v1,k2=v2`, the
form the old help text documented) are no longer split; repeat the flag
instead (`-o k1=v1 -o k2=v2`), which already worked before.
---
 cmd/volume_create.go      | 33 +++++++++++-----
 cmd/volume_create_test.go | 80 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 10 deletions(-)
 create mode 100644 cmd/volume_create_test.go

diff --git a/cmd/volume_create.go b/cmd/volume_create.go
index d24ff08..1201e13 100644
--- a/cmd/volume_create.go
+++ b/cmd/volume_create.go
@@ -32,16 +32,14 @@ var volCreateCmd = &cobra.Command{
 	Use:   "create",
 	Short: "Create volumes",
 	RunE: func(command *cobra.Command, args []string) error {
-		opts := map[string]string{}
-		for _, o := range options {
-			parts := strings.SplitN(o, "=", 2)
-			opts[parts[0]] = parts[1]
+		opts, err := parseKeyValues("option", options)
+		if err != nil {
+			return err
 		}
 
-		lbls := map[string]string{}
-		for _, l := range labels {
-			parts := strings.SplitN(l, "=", 2)
-			lbls[parts[0]] = parts[1]
+		lbls, err := parseKeyValues("label", labels)
+		if err != nil {
+			return err
 		}
 
 		resp, err := containerzClient.CreateVolume(command.Context(), name, driver, lbls, opts)
@@ -54,11 +52,26 @@ var volCreateCmd = &cobra.Command{
 	},
 }
 
+// parseKeyValues parses a list of key=value entries into a map. Only the first
+// "=" separates the key from the value, so values may contain "=" and ",".
+// kind names the entry type in error messages.
+func parseKeyValues(kind string, entries []string) (map[string]string, error) {
+	kvs := map[string]string{}
+	for _, e := range entries {
+		key, value, ok := strings.Cut(e, "=")
+		if !ok || key == "" {
+			return nil, fmt.Errorf("invalid %s %q: expected key=value", kind, e)
+		}
+		kvs[key] = value
+	}
+	return kvs, nil
+}
+
 func init() {
 	volumesCmd.AddCommand(volCreateCmd)
 
 	volCreateCmd.PersistentFlags().StringVar(&name, "name", "", "Name of the volume to create.")
 	volCreateCmd.PersistentFlags().StringVar(&driver, "driver", "", "Type of driver to use to create the volume.")
-	volCreateCmd.PersistentFlags().StringSliceVarP(&options, "options", "o", []string{}, "Options to pass to the driver in the form k1=v1,k2=v2,...")
-	volCreateCmd.PersistentFlags().StringSliceVarP(&labels, "labels", "l", []string{}, "Labels to tag. the volume with, in the form k1=v1")
+	volCreateCmd.PersistentFlags().StringArrayVarP(&options, "options", "o", []string{}, "Option to pass to the driver in the form key=value. Repeat for multiple options, e.g. -o options=rbind,ro -o mountpoint=/data")
+	volCreateCmd.PersistentFlags().StringArrayVarP(&labels, "labels", "l", []string{}, "Label to tag the volume with in the form key=value. Repeat for multiple labels.")
 }
diff --git a/cmd/volume_create_test.go b/cmd/volume_create_test.go
new file mode 100644
index 0000000..ba2fdb0
--- /dev/null
+++ b/cmd/volume_create_test.go
@@ -0,0 +1,80 @@
+// Copyright 2026 Google LLC
+//
+// 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 cmd
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestParseKeyValues(t *testing.T) {
+	tests := []struct {
+		name    string
+		in      []string
+		want    map[string]string
+		wantErr bool
+	}{
+		{
+			name: "empty",
+			want: map[string]string{},
+		},
+		{
+			name: "multiple-entries",
+			in:   []string{"options=bind", "mountpoint=/data"},
+			want: map[string]string{"options": "bind", "mountpoint": "/data"},
+		},
+		{
+			name: "value-with-commas",
+			in:   []string{"options=rbind,rslave,ro"},
+			want: map[string]string{"options": "rbind,rslave,ro"},
+		},
+		{
+			name: "value-with-equals",
+			in:   []string{"options=rbind,uid=1000"},
+			want: map[string]string{"options": "rbind,uid=1000"},
+		},
+		{
+			name: "empty-value",
+			in:   []string{"type="},
+			want: map[string]string{"type": ""},
+		},
+		{
+			name:    "missing-equals",
+			in:      []string{"foo"},
+			wantErr: true,
+		},
+		{
+			name:    "empty-key",
+			in:      []string{"=bar"},
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := parseKeyValues("option", tc.in)
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("parseKeyValues(%q) returned error %v, wantErr %t", tc.in, err, tc.wantErr)
+			}
+			if tc.wantErr {
+				return
+			}
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("parseKeyValues(%q) returned diff (-want, +got):\n%s", tc.in, diff)
+			}
+		})
+	}
+}