diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index 5c8c15964a..d06285e5b0 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -14,13 +14,13 @@ import ( "time" cdi "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" + "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/compose/loader" "github.com/docker/cli/opts" "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/strslice" - "github.com/docker/docker/api/types/versions" "github.com/docker/docker/errdefs" "github.com/docker/go-connections/nat" "github.com/pkg/errors" @@ -1119,8 +1119,8 @@ func validateAttach(val string) (string, error) { func validateAPIVersion(c *containerConfig, serverAPIVersion string) error { for _, m := range c.HostConfig.Mounts { - if m.BindOptions != nil && m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") { - return errors.Errorf("bind-nonrecursive requires API v1.40 or later") + if err := command.ValidateMountWithAPIVersion(m, serverAPIVersion); err != nil { + return err } } return nil diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 9b449971ea..c47ca82055 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -8,11 +8,11 @@ import ( "strings" "time" + "github.com/docker/cli/cli/command" "github.com/docker/cli/opts" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/api/types/versions" "github.com/docker/docker/client" gogotypes "github.com/gogo/protobuf/types" "github.com/google/shlex" @@ -1043,8 +1043,8 @@ const ( func validateAPIVersion(c swarm.ServiceSpec, serverAPIVersion string) error { for _, m := range c.TaskTemplate.ContainerSpec.Mounts { - if m.BindOptions != nil && m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") { - return errors.Errorf("bind-nonrecursive requires API v1.40 or later") + if err := command.ValidateMountWithAPIVersion(m, serverAPIVersion); err != nil { + return err } } return nil diff --git a/cli/command/utils.go b/cli/command/utils.go index 753f428aa0..91b9fc1891 100644 --- a/cli/command/utils.go +++ b/cli/command/utils.go @@ -11,6 +11,8 @@ import ( "github.com/docker/cli/cli/streams" "github.com/docker/docker/api/types/filters" + mounttypes "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/versions" "github.com/moby/sys/sequential" "github.com/pkg/errors" "github.com/spf13/pflag" @@ -195,3 +197,17 @@ func StringSliceReplaceAt(s, old, new []string, requireIndex int) ([]string, boo out = append(out, s[idx+len(old):]...) return out, true } + +// ValidateMountWithAPIVersion validates a mount with the server API version. +func ValidateMountWithAPIVersion(m mounttypes.Mount, serverAPIVersion string) error { + if m.BindOptions != nil { + if m.BindOptions.NonRecursive && versions.LessThan(serverAPIVersion, "1.40") { + return errors.Errorf("bind-recursive=disabled requires API v1.40 or later") + } + // ReadOnlyNonRecursive can be safely ignored when API < 1.44 + if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(serverAPIVersion, "1.44") { + return errors.Errorf("bind-recursive=readonly requires API v1.44 or later") + } + } + return nil +} diff --git a/opts/mount.go b/opts/mount.go index 2b531127eb..e2c7e039f7 100644 --- a/opts/mount.go +++ b/opts/mount.go @@ -2,6 +2,7 @@ package opts import ( "encoding/csv" + "errors" "fmt" "os" "path/filepath" @@ -10,6 +11,7 @@ import ( mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/go-units" + "github.com/sirupsen/logrus" ) // MountOpt is a Value type for parsing mounts @@ -112,6 +114,32 @@ func (m *MountOpt) Set(value string) error { if err != nil { return fmt.Errorf("invalid value for %s: %s", key, val) } + logrus.Warn("bind-nonrecursive is deprecated, use bind-recursive=disabled instead") + case "bind-recursive": + valS := val + // Allow boolean as an alias to "enabled" or "disabled" + if b, err := strconv.ParseBool(valS); err == nil { + if b { + valS = "enabled" + } else { + valS = "disabled" + } + } + switch valS { + case "enabled": // read-only mounts are recursively read-only if Engine >= v25 && kernel >= v5.12, otherwise writable + // NOP + case "disabled": // alias of bind-nonrecursive=true + bindOptions().NonRecursive = true + case "writable": // conforms to the default read-only bind-mount of Docker v24; read-only mounts are recursively mounted but not recursively read-only + bindOptions().ReadOnlyNonRecursive = true + case "readonly": // force recursively read-only, or raise an error + bindOptions().ReadOnlyForceRecursive = true + // TODO: implicitly set propagation and error if the user specifies a propagation in a future refactor/UX polish pass + // https://github.com/docker/cli/pull/4316#discussion_r1341974730 + default: + return fmt.Errorf("invalid value for %s: %s (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", + key, val) + } case "volume-nocopy": volumeOptions().NoCopy, err = strconv.ParseBool(val) if err != nil { @@ -161,6 +189,22 @@ func (m *MountOpt) Set(value string) error { return fmt.Errorf("cannot mix 'tmpfs-*' options with mount type '%s'", mount.Type) } + if mount.BindOptions != nil { + if mount.BindOptions.ReadOnlyNonRecursive { + if !mount.ReadOnly { + return errors.New("option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction") + } + } + if mount.BindOptions.ReadOnlyForceRecursive { + if !mount.ReadOnly { + return errors.New("option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction") + } + if mount.BindOptions.Propagation != mounttypes.PropagationRPrivate { + return errors.New("option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction") + } + } + } + m.values = append(m.values, mount) return nil } diff --git a/opts/mount_test.go b/opts/mount_test.go index cdb04aa1d2..fb3cd0329a 100644 --- a/opts/mount_test.go +++ b/opts/mount_test.go @@ -217,3 +217,86 @@ func TestMountOptSetTmpfsError(t *testing.T) { assert.ErrorContains(t, m.Set("type=tmpfs,target=/foo,tmpfs-mode=foo"), "invalid value for tmpfs-mode") assert.ErrorContains(t, m.Set("type=tmpfs"), "target is required") } + +func TestMountOptSetBindNonRecursive(t *testing.T) { + var mount MountOpt + assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-nonrecursive")) + assert.Check(t, is.DeepEqual([]mounttypes.Mount{ + { + Type: mounttypes.TypeBind, + Source: "/foo", + Target: "/bar", + BindOptions: &mounttypes.BindOptions{ + NonRecursive: true, + }, + }, + }, mount.Value())) +} + +func TestMountOptSetBindRecursive(t *testing.T) { + t.Run("enabled", func(t *testing.T) { + var mount MountOpt + assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=enabled")) + assert.Check(t, is.DeepEqual([]mounttypes.Mount{ + { + Type: mounttypes.TypeBind, + Source: "/foo", + Target: "/bar", + }, + }, mount.Value())) + }) + + t.Run("disabled", func(t *testing.T) { + var mount MountOpt + assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=disabled")) + assert.Check(t, is.DeepEqual([]mounttypes.Mount{ + { + Type: mounttypes.TypeBind, + Source: "/foo", + Target: "/bar", + BindOptions: &mounttypes.BindOptions{ + NonRecursive: true, + }, + }, + }, mount.Value())) + }) + + t.Run("writable", func(t *testing.T) { + var mount MountOpt + assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable"), + "option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction") + assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable,readonly")) + assert.Check(t, is.DeepEqual([]mounttypes.Mount{ + { + Type: mounttypes.TypeBind, + Source: "/foo", + Target: "/bar", + ReadOnly: true, + BindOptions: &mounttypes.BindOptions{ + ReadOnlyNonRecursive: true, + }, + }, + }, mount.Value())) + }) + + t.Run("readonly", func(t *testing.T) { + var mount MountOpt + assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly"), + "option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction") + assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly"), + "option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction") + assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly,bind-propagation=rprivate")) + assert.Check(t, is.DeepEqual([]mounttypes.Mount{ + { + Type: mounttypes.TypeBind, + Source: "/foo", + Target: "/bar", + ReadOnly: true, + BindOptions: &mounttypes.BindOptions{ + ReadOnlyForceRecursive: true, + Propagation: mounttypes.PropagationRPrivate, + }, + }, + }, mount.Value())) + }) +}