Skip to content

Commit dcc1610

Browse files
thaJeztahthaJeztah
authored
Merge pull request #4316 from AkihiroSuda/rro
mount: add `bind-recursive=<bool|string>` and deprecate `bind-nonrecursive=<bool>`
2 parents 05bec8d + fc6976d commit dcc1610

5 files changed

Lines changed: 149 additions & 6 deletions

File tree

‎cli/command/container/opts.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import (
1414
"time"
1515

1616
cdi "github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
17+
"github.com/docker/cli/cli/command"
1718
"github.com/docker/cli/cli/compose/loader"
1819
"github.com/docker/cli/opts"
1920
"github.com/docker/docker/api/types/container"
2021
mounttypes "github.com/docker/docker/api/types/mount"
2122
networktypes "github.com/docker/docker/api/types/network"
2223
"github.com/docker/docker/api/types/strslice"
23-
"github.com/docker/docker/api/types/versions"
2424
"github.com/docker/docker/errdefs"
2525
"github.com/docker/go-connections/nat"
2626
"github.com/pkg/errors"
@@ -1119,8 +1119,8 @@ func validateAttach(val string) (string, error) {
11191119

11201120
func validateAPIVersion(c *containerConfig, serverAPIVersion string) error {
11211121
for _, m := range c.HostConfig.Mounts {
1122-
if m.BindOptions != nil && m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") {
1123-
return errors.Errorf("bind-nonrecursive requires API v1.40 or later")
1122+
if err := command.ValidateMountWithAPIVersion(m, serverAPIVersion); err != nil {
1123+
return err
11241124
}
11251125
}
11261126
return nil

‎cli/command/service/opts.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import (
88
"strings"
99
"time"
1010

11+
"github.com/docker/cli/cli/command"
1112
"github.com/docker/cli/opts"
1213
"github.com/docker/docker/api/types"
1314
"github.com/docker/docker/api/types/container"
1415
"github.com/docker/docker/api/types/swarm"
15-
"github.com/docker/docker/api/types/versions"
1616
"github.com/docker/docker/client"
1717
gogotypes "github.com/gogo/protobuf/types"
1818
"github.com/google/shlex"
@@ -1043,8 +1043,8 @@ const (
10431043

10441044
func validateAPIVersion(c swarm.ServiceSpec, serverAPIVersion string) error {
10451045
for _, m := range c.TaskTemplate.ContainerSpec.Mounts {
1046-
if m.BindOptions != nil && m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") {
1047-
return errors.Errorf("bind-nonrecursive requires API v1.40 or later")
1046+
if err := command.ValidateMountWithAPIVersion(m, serverAPIVersion); err != nil {
1047+
return err
10481048
}
10491049
}
10501050
return nil

‎cli/command/utils.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111

1212
"github.com/docker/cli/cli/streams"
1313
"github.com/docker/docker/api/types/filters"
14+
mounttypes "github.com/docker/docker/api/types/mount"
15+
"github.com/docker/docker/api/types/versions"
1416
"github.com/moby/sys/sequential"
1517
"github.com/pkg/errors"
1618
"github.com/spf13/pflag"
@@ -195,3 +197,17 @@ func StringSliceReplaceAt(s, old, new []string, requireIndex int) ([]string, boo
195197
out = append(out, s[idx+len(old):]...)
196198
return out, true
197199
}
200+
201+
// ValidateMountWithAPIVersion validates a mount with the server API version.
202+
func ValidateMountWithAPIVersion(m mounttypes.Mount, serverAPIVersion string) error {
203+
if m.BindOptions != nil {
204+
if m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") {
205+
return errors.Errorf("bind-recursive=disabled requires API v1.40 or later")
206+
}
207+
// ReadOnlyNonRecursive can be safely ignored when API < 1.44
208+
if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(serverAPIVersion, "1.44") {
209+
return errors.Errorf("bind-recursive=readonly requires API v1.44 or later")
210+
}
211+
}
212+
return nil
213+
}

‎opts/mount.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package opts
22

33
import (
44
"encoding/csv"
5+
"errors"
56
"fmt"
67
"os"
78
"path/filepath"
@@ -10,6 +11,7 @@ import (
1011

1112
mounttypes "github.com/docker/docker/api/types/mount"
1213
"github.com/docker/go-units"
14+
"github.com/sirupsen/logrus"
1315
)
1416

1517
// MountOpt is a Value type for parsing mounts
@@ -112,6 +114,32 @@ func (m *MountOpt) Set(value string) error {
112114
if err != nil {
113115
return fmt.Errorf("invalid value for %s: %s", key, val)
114116
}
117+
logrus.Warn("bind-nonrecursive is deprecated, use bind-recursive=disabled instead")
118+
case "bind-recursive":
119+
valS := val
120+
// Allow boolean as an alias to "enabled" or "disabled"
121+
if b, err := strconv.ParseBool(valS); err == nil {
122+
if b {
123+
valS = "enabled"
124+
} else {
125+
valS = "disabled"
126+
}
127+
}
128+
switch valS {
129+
case "enabled": // read-only mounts are recursively read-only if Engine >= v25 && kernel >= v5.12, otherwise writable
130+
// NOP
131+
case "disabled": // alias of bind-nonrecursive=true
132+
bindOptions().NonRecursive = true
133+
case "writable": // conforms to the default read-only bind-mount of Docker v24; read-only mounts are recursively mounted but not recursively read-only
134+
bindOptions().ReadOnlyNonRecursive = true
135+
case "readonly": // force recursively read-only, or raise an error
136+
bindOptions().ReadOnlyForceRecursive = true
137+
// TODO: implicitly set propagation and error if the user specifies a propagation in a future refactor/UX polish pass
138+
// https://github.com/docker/cli/pull/4316#discussion_r1341974730
139+
default:
140+
return fmt.Errorf("invalid value for %s: %s (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")",
141+
key, val)
142+
}
115143
case "volume-nocopy":
116144
volumeOptions().NoCopy, err = strconv.ParseBool(val)
117145
if err != nil {
@@ -161,6 +189,22 @@ func (m *MountOpt) Set(value string) error {
161189
return fmt.Errorf("cannot mix 'tmpfs-*' options with mount type '%s'", mount.Type)
162190
}
163191

192+
if mount.BindOptions != nil {
193+
if mount.BindOptions.ReadOnlyNonRecursive {
194+
if !mount.ReadOnly {
195+
return errors.New("option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction")
196+
}
197+
}
198+
if mount.BindOptions.ReadOnlyForceRecursive {
199+
if !mount.ReadOnly {
200+
return errors.New("option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction")
201+
}
202+
if mount.BindOptions.Propagation != mounttypes.PropagationRPrivate {
203+
return errors.New("option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction")
204+
}
205+
}
206+
}
207+
164208
m.values = append(m.values, mount)
165209
return nil
166210
}

‎opts/mount_test.go‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,86 @@ func TestMountOptSetTmpfsError(t *testing.T) {
217217
assert.ErrorContains(t, m.Set("type=tmpfs,target=/foo,tmpfs-mode=foo"), "invalid value for tmpfs-mode")
218218
assert.ErrorContains(t, m.Set("type=tmpfs"), "target is required")
219219
}
220+
221+
func TestMountOptSetBindNonRecursive(t *testing.T) {
222+
var mount MountOpt
223+
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-nonrecursive"))
224+
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
225+
{
226+
Type: mounttypes.TypeBind,
227+
Source: "/foo",
228+
Target: "/bar",
229+
BindOptions: &mounttypes.BindOptions{
230+
NonRecursive: true,
231+
},
232+
},
233+
}, mount.Value()))
234+
}
235+
236+
func TestMountOptSetBindRecursive(t *testing.T) {
237+
t.Run("enabled", func(t *testing.T) {
238+
var mount MountOpt
239+
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=enabled"))
240+
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
241+
{
242+
Type: mounttypes.TypeBind,
243+
Source: "/foo",
244+
Target: "/bar",
245+
},
246+
}, mount.Value()))
247+
})
248+
249+
t.Run("disabled", func(t *testing.T) {
250+
var mount MountOpt
251+
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=disabled"))
252+
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
253+
{
254+
Type: mounttypes.TypeBind,
255+
Source: "/foo",
256+
Target: "/bar",
257+
BindOptions: &mounttypes.BindOptions{
258+
NonRecursive: true,
259+
},
260+
},
261+
}, mount.Value()))
262+
})
263+
264+
t.Run("writable", func(t *testing.T) {
265+
var mount MountOpt
266+
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable"),
267+
"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction")
268+
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable,readonly"))
269+
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
270+
{
271+
Type: mounttypes.TypeBind,
272+
Source: "/foo",
273+
Target: "/bar",
274+
ReadOnly: true,
275+
BindOptions: &mounttypes.BindOptions{
276+
ReadOnlyNonRecursive: true,
277+
},
278+
},
279+
}, mount.Value()))
280+
})
281+
282+
t.Run("readonly", func(t *testing.T) {
283+
var mount MountOpt
284+
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly"),
285+
"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction")
286+
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly"),
287+
"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction")
288+
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly,bind-propagation=rprivate"))
289+
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
290+
{
291+
Type: mounttypes.TypeBind,
292+
Source: "/foo",
293+
Target: "/bar",
294+
ReadOnly: true,
295+
BindOptions: &mounttypes.BindOptions{
296+
ReadOnlyForceRecursive: true,
297+
Propagation: mounttypes.PropagationRPrivate,
298+
},
299+
},
300+
}, mount.Value()))
301+
})
302+
}

0 commit comments

Comments
 (0)