refactor(cli/compose/loader): extract ParseVolume() to its own package

Moves ParseVolume() to a new internal package to remove the dependency
on cli/compose/loader in cli/command/container/opts.go

refactor to keep types isolated

- rename the package to "volumespec" to reuse the name of the package
  as part of the name (parsevolume.ParseVolume() -> volumespec.Parse())
- move the related compose types to the internal package as well,
  and rename them to be more generic (not associated with "compose");
  - ServiceVolumeConfig -> VolumeConfig
  - ServiceVolumeBind -> BindOpts
  - ServiceVolumeVolume -> VolumeOpts
  - ServiceVolumeImage -> ImageOpts
  - ServiceVolumeTmpfs -> TmpFsOpts
  - ServiceVolumeCluster -> ClusterOpts
- alias the internal types inside cli/compose/types to keep backward
  compatibility (for any external consumers); even though the implementation
  is internal, Go allows aliasing types to use them externally.

Signed-off-by: Michael Tews <michael@tews.dev>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Michael Tews
2025-07-23 08:31:48 +02:00
committed by Sebastiaan van Stijn
parent 3046019d3b
commit ef7fd8bb67
6 changed files with 100 additions and 72 deletions

View File

@ -12,8 +12,8 @@ import (
"strings"
"time"
"github.com/docker/cli/cli/compose/loader"
"github.com/docker/cli/internal/lazyregexp"
"github.com/docker/cli/internal/volumespec"
"github.com/docker/cli/opts"
"github.com/docker/go-connections/nat"
"github.com/moby/moby/api/types/container"
@ -364,7 +364,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
volumes := copts.volumes.GetMap()
// add any bind targets to the list of container volumes
for bind := range copts.volumes.GetMap() {
parsed, err := loader.ParseVolume(bind)
parsed, err := volumespec.Parse(bind)
if err != nil {
return nil, err
}

View File

@ -17,6 +17,7 @@ import (
"github.com/docker/cli/cli/compose/schema"
"github.com/docker/cli/cli/compose/template"
"github.com/docker/cli/cli/compose/types"
"github.com/docker/cli/internal/volumespec"
"github.com/docker/cli/opts"
"github.com/docker/cli/opts/swarmopts"
"github.com/docker/go-connections/nat"
@ -41,6 +42,13 @@ type Options struct {
discardEnvFiles bool
}
// ParseVolume parses a volume spec without any knowledge of the target platform.
//
// This function is unused, but kept for backward-compatibility for external users.
func ParseVolume(spec string) (types.ServiceVolumeConfig, error) {
return volumespec.Parse(spec)
}
// WithDiscardEnvFiles sets the Options to discard the `env_file` section after resolving to
// the `environment` section
func WithDiscardEnvFiles(options *Options) {
@ -756,7 +764,7 @@ var transformBuildConfig TransformerFunc = func(data any) (any, error) {
var transformServiceVolumeConfig TransformerFunc = func(data any) (any, error) {
switch value := data.(type) {
case string:
return ParseVolume(value)
return volumespec.Parse(value)
case map[string]any:
return data, nil
default:

View File

@ -1,125 +0,0 @@
package loader
import (
"strings"
"unicode"
"unicode/utf8"
"github.com/docker/cli/cli/compose/types"
"github.com/moby/moby/api/types/mount"
"github.com/pkg/errors"
)
const endOfSpec = rune(0)
// ParseVolume parses a volume spec without any knowledge of the target platform
func ParseVolume(spec string) (types.ServiceVolumeConfig, error) {
volume := types.ServiceVolumeConfig{}
switch len(spec) {
case 0:
return volume, errors.New("invalid empty volume spec")
case 1, 2:
volume.Target = spec
volume.Type = string(mount.TypeVolume)
return volume, nil
}
buffer := []rune{}
for _, char := range spec + string(endOfSpec) {
switch {
case isWindowsDrive(buffer, char):
buffer = append(buffer, char)
case char == ':' || char == endOfSpec:
if err := populateFieldFromBuffer(char, buffer, &volume); err != nil {
populateType(&volume)
return volume, errors.Wrapf(err, "invalid spec: %s", spec)
}
buffer = []rune{}
default:
buffer = append(buffer, char)
}
}
populateType(&volume)
return volume, nil
}
func isWindowsDrive(buffer []rune, char rune) bool {
return char == ':' && len(buffer) == 1 && unicode.IsLetter(buffer[0])
}
func populateFieldFromBuffer(char rune, buffer []rune, volume *types.ServiceVolumeConfig) error {
strBuffer := string(buffer)
switch {
case len(buffer) == 0:
return errors.New("empty section between colons")
// Anonymous volume
case volume.Source == "" && char == endOfSpec:
volume.Target = strBuffer
return nil
case volume.Source == "":
volume.Source = strBuffer
return nil
case volume.Target == "":
volume.Target = strBuffer
return nil
case char == ':':
return errors.New("too many colons")
}
for _, option := range strings.Split(strBuffer, ",") {
switch option {
case "ro":
volume.ReadOnly = true
case "rw":
volume.ReadOnly = false
case "nocopy":
volume.Volume = &types.ServiceVolumeVolume{NoCopy: true}
default:
if isBindOption(option) {
volume.Bind = &types.ServiceVolumeBind{Propagation: option}
}
// ignore unknown options
}
}
return nil
}
func isBindOption(option string) bool {
for _, propagation := range mount.Propagations {
if mount.Propagation(option) == propagation {
return true
}
}
return false
}
func populateType(volume *types.ServiceVolumeConfig) {
switch {
// Anonymous volume
case volume.Source == "":
volume.Type = string(mount.TypeVolume)
case isFilePath(volume.Source):
volume.Type = string(mount.TypeBind)
default:
volume.Type = string(mount.TypeVolume)
}
}
func isFilePath(source string) bool {
switch source[0] {
case '.', '/', '~':
return true
}
if len([]rune(source)) == 1 {
return false
}
// windows named pipes
if strings.HasPrefix(source, `\\`) {
return true
}
first, nextIndex := utf8.DecodeRuneInString(source)
return isWindowsDrive([]rune{first}, rune(source[nextIndex]))
}

View File

@ -1,230 +0,0 @@
package loader
import (
"fmt"
"testing"
"github.com/docker/cli/cli/compose/types"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestParseVolumeAnonymousVolume(t *testing.T) {
for _, path := range []string{"/path", "/path/foo"} {
volume, err := ParseVolume(path)
expected := types.ServiceVolumeConfig{Type: "volume", Target: path}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeAnonymousVolumeWindows(t *testing.T) {
for _, path := range []string{"C:\\path", "Z:\\path\\foo"} {
volume, err := ParseVolume(path)
expected := types.ServiceVolumeConfig{Type: "volume", Target: path}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeTooManyColons(t *testing.T) {
_, err := ParseVolume("/foo:/foo:ro:foo")
assert.Error(t, err, "invalid spec: /foo:/foo:ro:foo: too many colons")
}
func TestParseVolumeShortVolumes(t *testing.T) {
for _, path := range []string{".", "/a"} {
volume, err := ParseVolume(path)
expected := types.ServiceVolumeConfig{Type: "volume", Target: path}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeMissingSource(t *testing.T) {
for _, spec := range []string{":foo", "/foo::ro"} {
_, err := ParseVolume(spec)
assert.ErrorContains(t, err, "empty section between colons")
}
}
func TestParseVolumeBindMount(t *testing.T) {
for _, path := range []string{"./foo", "~/thing", "../other", "/foo", "/home/user"} {
volume, err := ParseVolume(path + ":/target")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: path,
Target: "/target",
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeRelativeBindMountWindows(t *testing.T) {
for _, path := range []string{
"./foo",
"~/thing",
"../other",
"D:\\path", "/home/user",
} {
volume, err := ParseVolume(path + ":d:\\target")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: path,
Target: "d:\\target",
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeWithBindOptions(t *testing.T) {
volume, err := ParseVolume("/source:/target:slave")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: "/source",
Target: "/target",
Bind: &types.ServiceVolumeBind{Propagation: "slave"},
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
func TestParseVolumeWithBindOptionsWindows(t *testing.T) {
volume, err := ParseVolume("C:\\source\\foo:D:\\target:ro,rprivate")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: "C:\\source\\foo",
Target: "D:\\target",
ReadOnly: true,
Bind: &types.ServiceVolumeBind{Propagation: "rprivate"},
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
func TestParseVolumeWithInvalidVolumeOptions(t *testing.T) {
_, err := ParseVolume("name:/target:bogus")
assert.NilError(t, err)
}
func TestParseVolumeWithVolumeOptions(t *testing.T) {
volume, err := ParseVolume("name:/target:nocopy")
expected := types.ServiceVolumeConfig{
Type: "volume",
Source: "name",
Target: "/target",
Volume: &types.ServiceVolumeVolume{NoCopy: true},
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
func TestParseVolumeWithReadOnly(t *testing.T) {
for _, path := range []string{"./foo", "/home/user"} {
volume, err := ParseVolume(path + ":/target:ro")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: path,
Target: "/target",
ReadOnly: true,
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeWithRW(t *testing.T) {
for _, path := range []string{"./foo", "/home/user"} {
volume, err := ParseVolume(path + ":/target:rw")
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: path,
Target: "/target",
ReadOnly: false,
}
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(expected, volume))
}
}
func TestParseVolumeWindowsNamedPipe(t *testing.T) {
volume, err := ParseVolume(`\\.\pipe\docker_engine:\\.\pipe\inside`)
assert.NilError(t, err)
expected := types.ServiceVolumeConfig{
Type: "bind",
Source: `\\.\pipe\docker_engine`,
Target: `\\.\pipe\inside`,
}
assert.Check(t, is.DeepEqual(expected, volume))
}
func TestIsFilePath(t *testing.T) {
assert.Check(t, !isFilePath("a界"))
assert.Check(t, !isFilePath("1"))
assert.Check(t, !isFilePath("c"))
}
// Preserve the test cases for VolumeSplitN
func TestParseVolumeSplitCases(t *testing.T) {
for casenumber, x := range []struct {
input string
n int
expected []string
}{
{`C:\foo:d:`, -1, []string{`C:\foo`, `d:`}},
{`:C:\foo:d:`, -1, nil},
{`/foo:/bar:ro`, 3, []string{`/foo`, `/bar`, `ro`}},
{`/foo:/bar:ro`, 2, []string{`/foo`, `/bar:ro`}},
{`C:\foo\:/foo`, -1, []string{`C:\foo\`, `/foo`}},
{`d:\`, -1, []string{`d:\`}},
{`d:`, -1, []string{`d:`}},
{`d:\path`, -1, []string{`d:\path`}},
{`d:\path with space`, -1, []string{`d:\path with space`}},
{`d:\pathandmode:rw`, -1, []string{`d:\pathandmode`, `rw`}},
{`c:\:d:\`, -1, []string{`c:\`, `d:\`}},
{`c:\windows\:d:`, -1, []string{`c:\windows\`, `d:`}},
{`c:\windows:d:\s p a c e`, -1, []string{`c:\windows`, `d:\s p a c e`}},
{`c:\windows:d:\s p a c e:RW`, -1, []string{`c:\windows`, `d:\s p a c e`, `RW`}},
{`c:\program files:d:\s p a c e i n h o s t d i r`, -1, []string{`c:\program files`, `d:\s p a c e i n h o s t d i r`}},
{`0123456789name:d:`, -1, []string{`0123456789name`, `d:`}},
{`MiXeDcAsEnAmE:d:`, -1, []string{`MiXeDcAsEnAmE`, `d:`}},
{`name:D:`, -1, []string{`name`, `D:`}},
{`name:D::rW`, -1, []string{`name`, `D:`, `rW`}},
{`name:D::RW`, -1, []string{`name`, `D:`, `RW`}},
{`c:/:d:/forward/slashes/are/good/too`, -1, []string{`c:/`, `d:/forward/slashes/are/good/too`}},
{`c:\Windows`, -1, []string{`c:\Windows`}},
{`c:\Program Files (x86)`, -1, []string{`c:\Program Files (x86)`}},
{``, -1, nil},
{`.`, -1, []string{`.`}},
{`..\`, -1, []string{`..\`}},
{`c:\:..\`, -1, []string{`c:\`, `..\`}},
{`c:\:d:\:xyzzy`, -1, []string{`c:\`, `d:\`, `xyzzy`}},
// Cover directories with one-character name
{`/tmp/x/y:/foo/x/y`, -1, []string{`/tmp/x/y`, `/foo/x/y`}},
} {
parsed, _ := ParseVolume(x.input)
expected := len(x.expected) > 1
msg := fmt.Sprintf("Case %d: %s", casenumber, x.input)
assert.Check(t, is.Equal(expected, parsed.Source != ""), msg)
}
}
func TestParseVolumeInvalidEmptySpec(t *testing.T) {
_, err := ParseVolume("")
assert.ErrorContains(t, err, "invalid empty volume spec")
}
func TestParseVolumeInvalidSections(t *testing.T) {
_, err := ParseVolume("/foo::rw")
assert.ErrorContains(t, err, "invalid spec")
}
func TestParseVolumeWithEmptySource(t *testing.T) {
_, err := ParseVolume(":/vol")
assert.ErrorContains(t, err, "empty section between colons")
}

View File

@ -8,6 +8,8 @@ import (
"fmt"
"strconv"
"time"
"github.com/docker/cli/internal/volumespec"
)
// UnsupportedProperties not yet supported by this implementation of the compose file
@ -390,43 +392,23 @@ type ServicePortConfig struct {
}
// ServiceVolumeConfig are references to a volume used by a service
type ServiceVolumeConfig struct {
Type string `yaml:",omitempty" json:"type,omitempty"`
Source string `yaml:",omitempty" json:"source,omitempty"`
Target string `yaml:",omitempty" json:"target,omitempty"`
ReadOnly bool `mapstructure:"read_only" yaml:"read_only,omitempty" json:"read_only,omitempty"`
Consistency string `yaml:",omitempty" json:"consistency,omitempty"`
Bind *ServiceVolumeBind `yaml:",omitempty" json:"bind,omitempty"`
Volume *ServiceVolumeVolume `yaml:",omitempty" json:"volume,omitempty"`
Image *ServiceVolumeImage `yaml:",omitempty" json:"image,omitempty"`
Tmpfs *ServiceVolumeTmpfs `yaml:",omitempty" json:"tmpfs,omitempty"`
Cluster *ServiceVolumeCluster `yaml:",omitempty" json:"cluster,omitempty"`
}
type ServiceVolumeConfig = volumespec.VolumeConfig
// ServiceVolumeBind are options for a service volume of type bind
type ServiceVolumeBind struct {
Propagation string `yaml:",omitempty" json:"propagation,omitempty"`
}
type ServiceVolumeBind = volumespec.BindOpts
// ServiceVolumeVolume are options for a service volume of type volume
type ServiceVolumeVolume struct {
NoCopy bool `mapstructure:"nocopy" yaml:"nocopy,omitempty" json:"nocopy,omitempty"`
Subpath string `mapstructure:"subpath" yaml:"subpath,omitempty" json:"subpath,omitempty"`
}
type ServiceVolumeVolume = volumespec.VolumeOpts
// ServiceVolumeImage are options for a service volume of type image
type ServiceVolumeImage struct {
Subpath string `mapstructure:"subpath" yaml:"subpath,omitempty" json:"subpath,omitempty"`
}
type ServiceVolumeImage = volumespec.ImageOpts
// ServiceVolumeTmpfs are options for a service volume of type tmpfs
type ServiceVolumeTmpfs struct {
Size int64 `yaml:",omitempty" json:"size,omitempty"`
}
type ServiceVolumeTmpfs = volumespec.TmpFsOpts
// ServiceVolumeCluster are options for a service volume of type cluster.
// Deliberately left blank for future options, but unused now.
type ServiceVolumeCluster struct{}
type ServiceVolumeCluster = volumespec.ClusterOpts
// FileReferenceConfig for a reference to a swarm file object
type FileReferenceConfig struct {