Skip to content

Commit 7ed22ab

Browse files
thaJeztahthaJeztah
committed
opts: MountOpt: extract utility functions and don't set empty values
Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 2a3e024 commit 7ed22ab

File tree

3 files changed

+69
-74
lines changed

3 files changed

+69
-74
lines changed

‎opts/mount.go‎

Lines changed: 16 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -33,50 +33,10 @@ func (m *MountOpt) Set(value string) error {
3333
return err
3434
}
3535

36-
mount := mounttypes.Mount{}
37-
38-
volumeOptions := func() *mounttypes.VolumeOptions {
39-
if mount.VolumeOptions == nil {
40-
mount.VolumeOptions = &mounttypes.VolumeOptions{
41-
Labels: make(map[string]string),
42-
}
43-
}
44-
if mount.VolumeOptions.DriverConfig == nil {
45-
mount.VolumeOptions.DriverConfig = &mounttypes.Driver{}
46-
}
47-
return mount.VolumeOptions
48-
}
49-
50-
imageOptions := func() *mounttypes.ImageOptions {
51-
if mount.ImageOptions == nil {
52-
mount.ImageOptions = new(mounttypes.ImageOptions)
53-
}
54-
return mount.ImageOptions
55-
}
56-
57-
bindOptions := func() *mounttypes.BindOptions {
58-
if mount.BindOptions == nil {
59-
mount.BindOptions = new(mounttypes.BindOptions)
60-
}
61-
return mount.BindOptions
36+
mount := mounttypes.Mount{
37+
Type: mounttypes.TypeVolume, // default to volume mounts
6238
}
6339

64-
tmpfsOptions := func() *mounttypes.TmpfsOptions {
65-
if mount.TmpfsOptions == nil {
66-
mount.TmpfsOptions = new(mounttypes.TmpfsOptions)
67-
}
68-
return mount.TmpfsOptions
69-
}
70-
71-
setValueOnMap := func(target map[string]string, value string) {
72-
k, v, _ := strings.Cut(value, "=")
73-
if k != "" {
74-
target[k] = v
75-
}
76-
}
77-
78-
mount.Type = mounttypes.TypeVolume // default to volume mounts
79-
8040
for _, field := range fields {
8141
key, val, hasValue := strings.Cut(field, "=")
8242
if k := strings.TrimSpace(key); k != key {
@@ -124,54 +84,53 @@ func (m *MountOpt) Set(value string) error {
12484
case "consistency":
12585
mount.Consistency = mounttypes.Consistency(strings.ToLower(val))
12686
case "bind-propagation":
127-
bindOptions().Propagation = mounttypes.Propagation(strings.ToLower(val))
87+
ensureBindOptions(&mount).Propagation = mounttypes.Propagation(strings.ToLower(val))
12888
case "bind-nonrecursive":
12989
return errors.New("bind-nonrecursive is deprecated, use bind-recursive=disabled instead")
13090
case "bind-recursive":
13191
switch val {
13292
case "enabled": // read-only mounts are recursively read-only if Engine >= v25 && kernel >= v5.12, otherwise writable
13393
// NOP
13494
case "disabled": // previously "bind-nonrecursive=true"
135-
bindOptions().NonRecursive = true
95+
ensureBindOptions(&mount).NonRecursive = true
13696
case "writable": // conforms to the default read-only bind-mount of Docker v24; read-only mounts are recursively mounted but not recursively read-only
137-
bindOptions().ReadOnlyNonRecursive = true
97+
ensureBindOptions(&mount).ReadOnlyNonRecursive = true
13898
case "readonly": // force recursively read-only, or raise an error
139-
bindOptions().ReadOnlyForceRecursive = true
99+
ensureBindOptions(&mount).ReadOnlyForceRecursive = true
140100
// TODO: implicitly set propagation and error if the user specifies a propagation in a future refactor/UX polish pass
141101
// https://github.com/docker/cli/pull/4316#discussion_r1341974730
142102
default:
143103
return fmt.Errorf(`invalid value for %s: %s (must be "enabled", "disabled", "writable", or "readonly")`, key, val)
144104
}
145105
case "volume-subpath":
146-
volumeOptions().Subpath = val
106+
ensureVolumeOptions(&mount).Subpath = val
147107
case "volume-nocopy":
148-
volumeOptions().NoCopy, err = parseBoolValue(key, val, hasValue)
108+
ensureVolumeOptions(&mount).NoCopy, err = parseBoolValue(key, val, hasValue)
149109
if err != nil {
150110
return err
151111
}
152112
case "volume-label":
153-
setValueOnMap(volumeOptions().Labels, val)
113+
volumeOpts := ensureVolumeOptions(&mount)
114+
volumeOpts.Labels = setValueOnMap(volumeOpts.Labels, val)
154115
case "volume-driver":
155-
volumeOptions().DriverConfig.Name = val
116+
ensureVolumeDriver(&mount).Name = val
156117
case "volume-opt":
157-
if volumeOptions().DriverConfig.Options == nil {
158-
volumeOptions().DriverConfig.Options = make(map[string]string)
159-
}
160-
setValueOnMap(volumeOptions().DriverConfig.Options, val)
118+
volumeDriver := ensureVolumeDriver(&mount)
119+
volumeDriver.Options = setValueOnMap(volumeDriver.Options, val)
161120
case "image-subpath":
162-
imageOptions().Subpath = val
121+
ensureImageOptions(&mount).Subpath = val
163122
case "tmpfs-size":
164123
sizeBytes, err := units.RAMInBytes(val)
165124
if err != nil {
166125
return fmt.Errorf("invalid value for %s: %s", key, val)
167126
}
168-
tmpfsOptions().SizeBytes = sizeBytes
127+
ensureTmpfsOptions(&mount).SizeBytes = sizeBytes
169128
case "tmpfs-mode":
170129
ui64, err := strconv.ParseUint(val, 8, 32)
171130
if err != nil {
172131
return fmt.Errorf("invalid value for %s: %s", key, val)
173132
}
174-
tmpfsOptions().Mode = os.FileMode(ui64)
133+
ensureTmpfsOptions(&mount).Mode = os.FileMode(ui64)
175134
default:
176135
return fmt.Errorf("unknown option '%s' in '%s'", key, field)
177136
}

‎opts/mount_test.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
282282
Labels: map[string]string{
283283
"foo": "foo-value",
284284
},
285-
DriverConfig: &mount.Driver{},
286285
},
287286
},
288287
},
@@ -297,7 +296,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
297296
"foo": "foo-value",
298297
"bar": "bar-value",
299298
},
300-
DriverConfig: &mount.Driver{},
301299
},
302300
},
303301
},
@@ -312,7 +310,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
312310
"foo": "",
313311
"bar": "",
314312
},
315-
DriverConfig: &mount.Driver{},
316313
},
317314
},
318315
},
@@ -321,12 +318,9 @@ func TestMountOptVolumeOptions(t *testing.T) {
321318
doc: "volume-label empty key",
322319
value: `type=volume,target=/foo,volume-label==foo-value`,
323320
exp: mount.Mount{
324-
Type: mount.TypeVolume,
325-
Target: "/foo",
326-
VolumeOptions: &mount.VolumeOptions{
327-
Labels: map[string]string{},
328-
DriverConfig: &mount.Driver{},
329-
},
321+
Type: mount.TypeVolume,
322+
Target: "/foo",
323+
VolumeOptions: &mount.VolumeOptions{},
330324
},
331325
},
332326
{
@@ -336,7 +330,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
336330
Type: mount.TypeVolume,
337331
Target: "/foo",
338332
VolumeOptions: &mount.VolumeOptions{
339-
Labels: map[string]string{},
340333
DriverConfig: &mount.Driver{
341334
Name: "my-driver",
342335
},
@@ -350,7 +343,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
350343
Type: mount.TypeVolume,
351344
Target: "/foo",
352345
VolumeOptions: &mount.VolumeOptions{
353-
Labels: map[string]string{},
354346
DriverConfig: &mount.Driver{
355347
Options: map[string]string{
356348
"foo": "foo-value",
@@ -366,7 +358,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
366358
Type: mount.TypeVolume,
367359
Target: "/foo",
368360
VolumeOptions: &mount.VolumeOptions{
369-
Labels: map[string]string{},
370361
DriverConfig: &mount.Driver{
371362
Options: map[string]string{
372363
"foo": "foo-value",
@@ -383,7 +374,6 @@ func TestMountOptVolumeOptions(t *testing.T) {
383374
Type: mount.TypeVolume,
384375
Target: "/foo",
385376
VolumeOptions: &mount.VolumeOptions{
386-
Labels: map[string]string{},
387377
DriverConfig: &mount.Driver{
388378
Options: map[string]string{
389379
"foo": "",
@@ -401,10 +391,7 @@ func TestMountOptVolumeOptions(t *testing.T) {
401391
Type: mount.TypeVolume,
402392
Target: "/foo",
403393
VolumeOptions: &mount.VolumeOptions{
404-
Labels: map[string]string{},
405-
DriverConfig: &mount.Driver{
406-
Options: map[string]string{},
407-
},
394+
DriverConfig: &mount.Driver{},
408395
},
409396
},
410397
},

‎opts/mount_utils.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package opts
33
import (
44
"errors"
55
"fmt"
6+
"strings"
67

78
"github.com/moby/moby/api/types/mount"
89
)
@@ -84,3 +85,51 @@ func parseBoolValue(key string, val string, hasValue bool) (bool, error) {
8485
return false, fmt.Errorf(`invalid value for '%s': invalid boolean value (%q): must be one of "true", "1", "false", or "0" (default "true")`, key, val)
8586
}
8687
}
88+
89+
func ensureVolumeOptions(m *mount.Mount) *mount.VolumeOptions {
90+
if m.VolumeOptions == nil {
91+
m.VolumeOptions = &mount.VolumeOptions{}
92+
}
93+
return m.VolumeOptions
94+
}
95+
96+
func ensureVolumeDriver(m *mount.Mount) *mount.Driver {
97+
ensureVolumeOptions(m)
98+
if m.VolumeOptions.DriverConfig == nil {
99+
m.VolumeOptions.DriverConfig = &mount.Driver{}
100+
}
101+
return m.VolumeOptions.DriverConfig
102+
}
103+
104+
func ensureImageOptions(m *mount.Mount) *mount.ImageOptions {
105+
if m.ImageOptions == nil {
106+
m.ImageOptions = &mount.ImageOptions{}
107+
}
108+
return m.ImageOptions
109+
}
110+
111+
func ensureBindOptions(m *mount.Mount) *mount.BindOptions {
112+
if m.BindOptions == nil {
113+
m.BindOptions = &mount.BindOptions{}
114+
}
115+
return m.BindOptions
116+
}
117+
118+
func ensureTmpfsOptions(m *mount.Mount) *mount.TmpfsOptions {
119+
if m.TmpfsOptions == nil {
120+
m.TmpfsOptions = &mount.TmpfsOptions{}
121+
}
122+
return m.TmpfsOptions
123+
}
124+
125+
func setValueOnMap(target map[string]string, keyValue string) map[string]string {
126+
k, v, _ := strings.Cut(keyValue, "=")
127+
if k == "" {
128+
return target
129+
}
130+
if target == nil {
131+
target = map[string]string{}
132+
}
133+
target[k] = v
134+
return target
135+
}

0 commit comments

Comments
 (0)