Guest24897
/
DockerCLI
mirror of https://github.com/docker/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4316 from AkihiroSuda/rro 

mount: add `bind-recursive=<bool|string>` and deprecate `bind-nonrecursive=<bool>`
This commit is contained in:
Sebastiaan van Stijn 2023-10-02 12:20:11 -07:00 committed by GitHub
parent 05bec8dd43 fc6976db45
commit dcc1610768
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 149 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cli/command/container/opts.go
View File

@ -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

6
cli/command/service/opts.go
View File

@ -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

16
cli/command/utils.go
View File

@ -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
}

44
opts/mount.go
View File

@ -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
}

83
opts/mount_test.go
View File

@ -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()))
})
}