Skip to content

Commit 6c39bc1

Browse files
thaJeztahthaJeztah
committed
opts: use strings.Cut for handling key/value pairs
Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent a473c5b commit 6c39bc1

File tree

15 files changed

+175
-189
lines changed

15 files changed

+175
-189
lines changed

‎cli/command/container/opts_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,8 +649,8 @@ func TestRunFlagsParseShmSize(t *testing.T) {
649649

650650
func TestParseRestartPolicy(t *testing.T) {
651651
invalids := map[string]string{
652-
"always:2:3": "invalid restart policy format",
653-
"on-failure:invalid": "maximum retry count must be an integer",
652+
"always:2:3": "invalid restart policy format: maximum retry count must be an integer",
653+
"on-failure:invalid": "invalid restart policy format: maximum retry count must be an integer",
654654
}
655655
valids := map[string]container.RestartPolicy{
656656
"": {},

‎opts/config.go‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,23 @@ func (o *ConfigOpt) Set(value string) error {
4040
}
4141

4242
for _, field := range fields {
43-
parts := strings.SplitN(field, "=", 2)
44-
key := strings.ToLower(parts[0])
45-
46-
if len(parts) != 2 {
43+
key, val, ok := strings.Cut(field, "=")
44+
if !ok || key == "" {
4745
return fmt.Errorf("invalid field '%s' must be a key=value pair", field)
4846
}
4947

50-
value := parts[1]
51-
switch key {
48+
// TODO(thaJeztah): these options should not be case-insensitive.
49+
switch strings.ToLower(key) {
5250
case "source", "src":
53-
options.ConfigName = value
51+
options.ConfigName = val
5452
case "target":
55-
options.File.Name = value
53+
options.File.Name = val
5654
case "uid":
57-
options.File.UID = value
55+
options.File.UID = val
5856
case "gid":
59-
options.File.GID = value
57+
options.File.GID = val
6058
case "mode":
61-
m, err := strconv.ParseUint(value, 0, 32)
59+
m, err := strconv.ParseUint(val, 0, 32)
6260
if err != nil {
6361
return fmt.Errorf("invalid mode specified: %v", err)
6462
}

‎opts/env.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@ import (
1616
//
1717
// The only validation here is to check if name is empty, per #25099
1818
func ValidateEnv(val string) (string, error) {
19-
arr := strings.SplitN(val, "=", 2)
20-
if arr[0] == "" {
19+
k, _, hasValue := strings.Cut(val, "=")
20+
if k == "" {
2121
return "", errors.New("invalid environment variable: " + val)
2222
}
23-
if len(arr) > 1 {
23+
if hasValue {
24+
// val contains a "=" (but value may be an empty string)
2425
return val, nil
2526
}
26-
if envVal, ok := os.LookupEnv(arr[0]); ok {
27-
return arr[0] + "=" + envVal, nil
27+
if envVal, ok := os.LookupEnv(k); ok {
28+
return k + "=" + envVal, nil
2829
}
2930
return val, nil
3031
}

‎opts/file.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,29 +46,28 @@ func parseKeyValueFile(filename string, emptyFn func(string) (string, bool)) ([]
4646
currentLine++
4747
// line is not empty, and not starting with '#'
4848
if len(line) > 0 && !strings.HasPrefix(line, "#") {
49-
data := strings.SplitN(line, "=", 2)
49+
variable, value, hasValue := strings.Cut(line, "=")
5050

5151
// trim the front of a variable, but nothing else
52-
variable := strings.TrimLeft(data[0], whiteSpaces)
52+
variable = strings.TrimLeft(variable, whiteSpaces)
5353
if strings.ContainsAny(variable, whiteSpaces) {
5454
return []string{}, ErrBadKey{fmt.Sprintf("variable '%s' contains whitespaces", variable)}
5555
}
5656
if len(variable) == 0 {
5757
return []string{}, ErrBadKey{fmt.Sprintf("no variable name on line '%s'", line)}
5858
}
5959

60-
if len(data) > 1 {
60+
if hasValue {
6161
// pass the value through, no trimming
62-
lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1]))
62+
lines = append(lines, variable+"="+value)
6363
} else {
64-
var value string
6564
var present bool
6665
if emptyFn != nil {
6766
value, present = emptyFn(line)
6867
}
6968
if present {
7069
// if only a pass-through variable is given, clean it up.
71-
lines = append(lines, fmt.Sprintf("%s=%s", strings.TrimSpace(line), value))
70+
lines = append(lines, strings.TrimSpace(variable)+"="+value)
7271
}
7372
}
7473
}

‎opts/gpus.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,13 @@ func (o *GpuOpts) Set(value string) error {
3838
seen := map[string]struct{}{}
3939
// Set writable as the default
4040
for _, field := range fields {
41-
parts := strings.SplitN(field, "=", 2)
42-
key := parts[0]
41+
key, val, withValue := strings.Cut(field, "=")
4342
if _, ok := seen[key]; ok {
4443
return fmt.Errorf("gpu request key '%s' can be specified only once", key)
4544
}
4645
seen[key] = struct{}{}
4746

48-
if len(parts) == 1 {
47+
if !withValue {
4948
seen["count"] = struct{}{}
5049
req.Count, err = parseCount(key)
5150
if err != nil {
@@ -54,21 +53,20 @@ func (o *GpuOpts) Set(value string) error {
5453
continue
5554
}
5655

57-
value := parts[1]
5856
switch key {
5957
case "driver":
60-
req.Driver = value
58+
req.Driver = val
6159
case "count":
62-
req.Count, err = parseCount(value)
60+
req.Count, err = parseCount(val)
6361
if err != nil {
6462
return err
6563
}
6664
case "device":
67-
req.DeviceIDs = strings.Split(value, ",")
65+
req.DeviceIDs = strings.Split(val, ",")
6866
case "capabilities":
69-
req.Capabilities = [][]string{append(strings.Split(value, ","), "gpu")}
67+
req.Capabilities = [][]string{append(strings.Split(val, ","), "gpu")}
7068
case "options":
71-
r := csv.NewReader(strings.NewReader(value))
69+
r := csv.NewReader(strings.NewReader(val))
7270
optFields, err := r.Read()
7371
if err != nil {
7472
return errors.Wrap(err, "failed to read gpu options")

‎opts/hosts.go‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ const (
3333
)
3434

3535
// ValidateHost validates that the specified string is a valid host and returns it.
36+
//
37+
// TODO(thaJeztah): ValidateHost appears to be unused; deprecate it.
3638
func ValidateHost(val string) (string, error) {
3739
host := strings.TrimSpace(val)
3840
// The empty string means default and is not handled by parseDockerDaemonHost
@@ -69,18 +71,19 @@ func ParseHost(defaultToTLS bool, val string) (string, error) {
6971
// parseDockerDaemonHost parses the specified address and returns an address that will be used as the host.
7072
// Depending of the address specified, this may return one of the global Default* strings defined in hosts.go.
7173
func parseDockerDaemonHost(addr string) (string, error) {
72-
addrParts := strings.SplitN(addr, "://", 2)
73-
if len(addrParts) == 1 && addrParts[0] != "" {
74-
addrParts = []string{"tcp", addrParts[0]}
74+
proto, host, hasProto := strings.Cut(addr, "://")
75+
if !hasProto && proto != "" {
76+
host = proto
77+
proto = "tcp"
7578
}
7679

77-
switch addrParts[0] {
80+
switch proto {
7881
case "tcp":
79-
return ParseTCPAddr(addrParts[1], defaultTCPHost)
82+
return ParseTCPAddr(host, defaultTCPHost)
8083
case "unix":
81-
return parseSimpleProtoAddr("unix", addrParts[1], defaultUnixSocket)
84+
return parseSimpleProtoAddr(proto, host, defaultUnixSocket)
8285
case "npipe":
83-
return parseSimpleProtoAddr("npipe", addrParts[1], defaultNamedPipe)
86+
return parseSimpleProtoAddr(proto, host, defaultNamedPipe)
8487
case "fd":
8588
return addr, nil
8689
case "ssh":
@@ -160,16 +163,18 @@ func ParseTCPAddr(tryAddr string, defaultAddr string) (string, error) {
160163

161164
// ValidateExtraHost validates that the specified string is a valid extrahost and returns it.
162165
// ExtraHost is in the form of name:ip where the ip has to be a valid ip (IPv4 or IPv6).
166+
//
167+
// TODO(thaJeztah): remove client-side validation, and delegate to the API server.
163168
func ValidateExtraHost(val string) (string, error) {
164169
// allow for IPv6 addresses in extra hosts by only splitting on first ":"
165-
arr := strings.SplitN(val, ":", 2)
166-
if len(arr) != 2 || len(arr[0]) == 0 {
170+
k, v, ok := strings.Cut(val, ":")
171+
if !ok || k == "" {
167172
return "", fmt.Errorf("bad format for add-host: %q", val)
168173
}
169174
// Skip IPaddr validation for "host-gateway" string
170-
if arr[1] != hostGatewayName {
171-
if _, err := ValidateIPAddress(arr[1]); err != nil {
172-
return "", fmt.Errorf("invalid IP address in add-host: %q", arr[1])
175+
if v != hostGatewayName {
176+
if _, err := ValidateIPAddress(v); err != nil {
177+
return "", fmt.Errorf("invalid IP address in add-host: %q", v)
173178
}
174179
}
175180
return val, nil

‎opts/mount.go‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,21 @@ func (m *MountOpt) Set(value string) error {
5656
}
5757

5858
setValueOnMap := func(target map[string]string, value string) {
59-
parts := strings.SplitN(value, "=", 2)
60-
if len(parts) == 1 {
61-
target[value] = ""
62-
} else {
63-
target[parts[0]] = parts[1]
59+
k, v, _ := strings.Cut(value, "=")
60+
if k != "" {
61+
target[k] = v
6462
}
6563
}
6664

6765
mount.Type = mounttypes.TypeVolume // default to volume mounts
6866
// Set writable as the default
6967
for _, field := range fields {
70-
parts := strings.SplitN(field, "=", 2)
71-
key := strings.ToLower(parts[0])
68+
key, val, ok := strings.Cut(field, "=")
7269

73-
if len(parts) == 1 {
70+
// TODO(thaJeztah): these options should not be case-insensitive.
71+
key = strings.ToLower(key)
72+
73+
if !ok {
7474
switch key {
7575
case "readonly", "ro":
7676
mount.ReadOnly = true
@@ -81,64 +81,61 @@ func (m *MountOpt) Set(value string) error {
8181
case "bind-nonrecursive":
8282
bindOptions().NonRecursive = true
8383
continue
84+
default:
85+
return fmt.Errorf("invalid field '%s' must be a key=value pair", field)
8486
}
8587
}
8688

87-
if len(parts) != 2 {
88-
return fmt.Errorf("invalid field '%s' must be a key=value pair", field)
89-
}
90-
91-
value := parts[1]
9289
switch key {
9390
case "type":
94-
mount.Type = mounttypes.Type(strings.ToLower(value))
91+
mount.Type = mounttypes.Type(strings.ToLower(val))
9592
case "source", "src":
96-
mount.Source = value
97-
if strings.HasPrefix(value, "."+string(filepath.Separator)) || value == "." {
98-
if abs, err := filepath.Abs(value); err == nil {
93+
mount.Source = val
94+
if strings.HasPrefix(val, "."+string(filepath.Separator)) || val == "." {
95+
if abs, err := filepath.Abs(val); err == nil {
9996
mount.Source = abs
10097
}
10198
}
10299
case "target", "dst", "destination":
103-
mount.Target = value
100+
mount.Target = val
104101
case "readonly", "ro":
105-
mount.ReadOnly, err = strconv.ParseBool(value)
102+
mount.ReadOnly, err = strconv.ParseBool(val)
106103
if err != nil {
107-
return fmt.Errorf("invalid value for %s: %s", key, value)
104+
return fmt.Errorf("invalid value for %s: %s", key, val)
108105
}
109106
case "consistency":
110-
mount.Consistency = mounttypes.Consistency(strings.ToLower(value))
107+
mount.Consistency = mounttypes.Consistency(strings.ToLower(val))
111108
case "bind-propagation":
112-
bindOptions().Propagation = mounttypes.Propagation(strings.ToLower(value))
109+
bindOptions().Propagation = mounttypes.Propagation(strings.ToLower(val))
113110
case "bind-nonrecursive":
114-
bindOptions().NonRecursive, err = strconv.ParseBool(value)
111+
bindOptions().NonRecursive, err = strconv.ParseBool(val)
115112
if err != nil {
116-
return fmt.Errorf("invalid value for %s: %s", key, value)
113+
return fmt.Errorf("invalid value for %s: %s", key, val)
117114
}
118115
case "volume-nocopy":
119-
volumeOptions().NoCopy, err = strconv.ParseBool(value)
116+
volumeOptions().NoCopy, err = strconv.ParseBool(val)
120117
if err != nil {
121-
return fmt.Errorf("invalid value for volume-nocopy: %s", value)
118+
return fmt.Errorf("invalid value for volume-nocopy: %s", val)
122119
}
123120
case "volume-label":
124-
setValueOnMap(volumeOptions().Labels, value)
121+
setValueOnMap(volumeOptions().Labels, val)
125122
case "volume-driver":
126-
volumeOptions().DriverConfig.Name = value
123+
volumeOptions().DriverConfig.Name = val
127124
case "volume-opt":
128125
if volumeOptions().DriverConfig.Options == nil {
129126
volumeOptions().DriverConfig.Options = make(map[string]string)
130127
}
131-
setValueOnMap(volumeOptions().DriverConfig.Options, value)
128+
setValueOnMap(volumeOptions().DriverConfig.Options, val)
132129
case "tmpfs-size":
133-
sizeBytes, err := units.RAMInBytes(value)
130+
sizeBytes, err := units.RAMInBytes(val)
134131
if err != nil {
135-
return fmt.Errorf("invalid value for %s: %s", key, value)
132+
return fmt.Errorf("invalid value for %s: %s", key, val)
136133
}
137134
tmpfsOptions().SizeBytes = sizeBytes
138135
case "tmpfs-mode":
139-
ui64, err := strconv.ParseUint(value, 8, 32)
136+
ui64, err := strconv.ParseUint(val, 8, 32)
140137
if err != nil {
141-
return fmt.Errorf("invalid value for %s: %s", key, value)
138+
return fmt.Errorf("invalid value for %s: %s", key, val)
142139
}
143140
tmpfsOptions().Mode = os.FileMode(ui64)
144141
default:

‎opts/network.go‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -48,34 +48,33 @@ func (n *NetworkOpt) Set(value string) error {
4848

4949
netOpt.Aliases = []string{}
5050
for _, field := range fields {
51-
parts := strings.SplitN(field, "=", 2)
52-
53-
if len(parts) < 2 {
51+
// TODO(thaJeztah): these options should not be case-insensitive.
52+
key, val, ok := strings.Cut(strings.ToLower(field), "=")
53+
if !ok || key == "" {
5454
return fmt.Errorf("invalid field %s", field)
5555
}
5656

57-
key := strings.TrimSpace(strings.ToLower(parts[0]))
58-
value := strings.TrimSpace(strings.ToLower(parts[1]))
57+
key = strings.TrimSpace(key)
58+
val = strings.TrimSpace(val)
5959

6060
switch key {
6161
case networkOptName:
62-
netOpt.Target = value
62+
netOpt.Target = val
6363
case networkOptAlias:
64-
netOpt.Aliases = append(netOpt.Aliases, value)
64+
netOpt.Aliases = append(netOpt.Aliases, val)
6565
case networkOptIPv4Address:
66-
netOpt.IPv4Address = value
66+
netOpt.IPv4Address = val
6767
case networkOptIPv6Address:
68-
netOpt.IPv6Address = value
68+
netOpt.IPv6Address = val
6969
case driverOpt:
70-
key, value, err = parseDriverOpt(value)
71-
if err == nil {
72-
if netOpt.DriverOpts == nil {
73-
netOpt.DriverOpts = make(map[string]string)
74-
}
75-
netOpt.DriverOpts[key] = value
76-
} else {
70+
key, val, err = parseDriverOpt(val)
71+
if err != nil {
7772
return err
7873
}
74+
if netOpt.DriverOpts == nil {
75+
netOpt.DriverOpts = make(map[string]string)
76+
}
77+
netOpt.DriverOpts[key] = val
7978
default:
8079
return fmt.Errorf("invalid field key %s", key)
8180
}
@@ -116,11 +115,13 @@ func (n *NetworkOpt) NetworkMode() string {
116115
}
117116

118117
func parseDriverOpt(driverOpt string) (string, string, error) {
119-
parts := strings.SplitN(driverOpt, "=", 2)
120-
if len(parts) != 2 {
118+
// TODO(thaJeztah): these options should not be case-insensitive.
119+
// TODO(thaJeztah): should value be converted to lowercase as well, or only the key?
120+
key, value, ok := strings.Cut(strings.ToLower(driverOpt), "=")
121+
if !ok || key == "" {
121122
return "", "", fmt.Errorf("invalid key value pair format in driver options")
122123
}
123-
key := strings.TrimSpace(strings.ToLower(parts[0]))
124-
value := strings.TrimSpace(strings.ToLower(parts[1]))
124+
key = strings.TrimSpace(key)
125+
value = strings.TrimSpace(value)
125126
return key, value, nil
126127
}

0 commit comments

Comments
 (0)