Merge pull request #6202 from Mewsen/issue/6188

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

Author: thaJeztah

cli/command/container/opts.go

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

cli/compose/loader/loader.go

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

cli/compose/types/types.go

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

internal/volumespec/types.go

@@ -0,0 +1,40 @@
+package volumespec
+
+// VolumeConfig are references to a volume used by a service
+type VolumeConfig 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        *BindOpts    `yaml:",omitempty" json:"bind,omitempty"`
+	Volume      *VolumeOpts  `yaml:",omitempty" json:"volume,omitempty"`
+	Image       *ImageOpts   `yaml:",omitempty" json:"image,omitempty"`
+	Tmpfs       *TmpFsOpts   `yaml:",omitempty" json:"tmpfs,omitempty"`
+	Cluster     *ClusterOpts `yaml:",omitempty" json:"cluster,omitempty"`
+}
+
+// BindOpts are options for a service volume of type bind
+type BindOpts struct {
+	Propagation string `yaml:",omitempty" json:"propagation,omitempty"`
+}
+
+// VolumeOpts are options for a service volume of type volume
+type VolumeOpts struct {
+	NoCopy  bool   `mapstructure:"nocopy" yaml:"nocopy,omitempty" json:"nocopy,omitempty"`
+	Subpath string `mapstructure:"subpath" yaml:"subpath,omitempty" json:"subpath,omitempty"`
+}
+
+// ImageOpts are options for a service volume of type image
+type ImageOpts struct {
+	Subpath string `mapstructure:"subpath" yaml:"subpath,omitempty" json:"subpath,omitempty"`
+}
+
+// TmpFsOpts are options for a service volume of type tmpfs
+type TmpFsOpts struct {
+	Size int64 `yaml:",omitempty" json:"size,omitempty"`
+}
+
+// ClusterOpts are options for a service volume of type cluster.
+// Deliberately left blank for future options, but unused now.
+type ClusterOpts struct{}

cli/compose/loader/volume.go renamed to internal/volumespec/volumespec.go

@@ -1,20 +1,19 @@
-package loader
+package volumespec
 
 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{}
+// Parse parses a volume spec without any knowledge of the target platform
+func Parse(spec string) (VolumeConfig, error) {
+	volume := VolumeConfig{}
 
 	switch len(spec) {
 	case 0:
@@ -49,7 +48,7 @@ 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 {
+func populateFieldFromBuffer(char rune, buffer []rune, volume *VolumeConfig) error {
 	strBuffer := string(buffer)
 	switch {
 	case len(buffer) == 0:
@@ -74,10 +73,10 @@ func populateFieldFromBuffer(char rune, buffer []rune, volume *types.ServiceVolu
 		case "rw":
 			volume.ReadOnly = false
 		case "nocopy":
-			volume.Volume = &types.ServiceVolumeVolume{NoCopy: true}
+			volume.Volume = &VolumeOpts{NoCopy: true}
 		default:
 			if isBindOption(option) {
-				volume.Bind = &types.ServiceVolumeBind{Propagation: option}
+				volume.Bind = &BindOpts{Propagation: option}
 			}
 			// ignore unknown options
 		}
@@ -94,7 +93,7 @@ func isBindOption(option string) bool {
 	return false
 }
 
-func populateType(volume *types.ServiceVolumeConfig) {
+func populateType(volume *VolumeConfig) {
 	switch {
 	// Anonymous volume
 	case volume.Source == "":