Browse Source

Re-validate Mounts on container start

Validation of Mounts was only performed on container _creation_, not on
container _start_. As a result, if the host-path no longer existed
when the container was started, a directory was created in the given
location.

This is the wrong behavior, because when using the `Mounts` API, host paths
should never be created, and an error should be produced instead.

This patch adds a validation step on container start, and produces an
error if the host path is not found.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 7 years ago
parent
commit
7cb96ba308
6 changed files with 17 additions and 13 deletions
  1. 9 0
      daemon/container.go
  2. 1 1
      volume/lcow_parser.go
  3. 1 1
      volume/linux_parser.go
  4. 1 2
      volume/parser.go
  5. 4 8
      volume/validate_test.go
  6. 1 1
      volume/windows_parser.go

+ 9 - 0
daemon/container.go

@@ -19,6 +19,7 @@ import (
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/truncindex"
 	"github.com/docker/docker/pkg/truncindex"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/runconfig"
+	"github.com/docker/docker/volume"
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-connections/nat"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
@@ -293,6 +294,14 @@ func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *conta
 		return nil, errors.Errorf("can't create 'AutoRemove' container with restart policy")
 		return nil, errors.Errorf("can't create 'AutoRemove' container with restart policy")
 	}
 	}
 
 
+	// Validate mounts; check if host directories still exist
+	parser := volume.NewParser(platform)
+	for _, cfg := range hostConfig.Mounts {
+		if err := parser.ValidateMountConfig(&cfg); err != nil {
+			return nil, err
+		}
+	}
+
 	for _, extraHost := range hostConfig.ExtraHosts {
 	for _, extraHost := range hostConfig.ExtraHosts {
 		if _, err := opts.ValidateExtraHost(extraHost); err != nil {
 		if _, err := opts.ValidateExtraHost(extraHost); err != nil {
 			return nil, err
 			return nil, err

+ 1 - 1
volume/lcow_parser.go

@@ -22,7 +22,7 @@ type lcowParser struct {
 	windowsParser
 	windowsParser
 }
 }
 
 
-func (p *lcowParser) validateMountConfig(mnt *mount.Mount) error {
+func (p *lcowParser) ValidateMountConfig(mnt *mount.Mount) error {
 	return p.validateMountConfigReg(mnt, rxLCOWDestination, lcowSpecificValidators)
 	return p.validateMountConfigReg(mnt, rxLCOWDestination, lcowSpecificValidators)
 }
 }
 
 

+ 1 - 1
volume/linux_parser.go

@@ -40,7 +40,7 @@ func linuxValidateAbsolute(p string) error {
 	}
 	}
 	return fmt.Errorf("invalid mount path: '%s' mount path must be absolute", p)
 	return fmt.Errorf("invalid mount path: '%s' mount path must be absolute", p)
 }
 }
-func (p *linuxParser) validateMountConfig(mnt *mount.Mount) error {
+func (p *linuxParser) ValidateMountConfig(mnt *mount.Mount) error {
 	// there was something looking like a bug in existing codebase:
 	// there was something looking like a bug in existing codebase:
 	// - validateMountConfig on linux was called with options skipping bind source existence when calling ParseMountRaw
 	// - validateMountConfig on linux was called with options skipping bind source existence when calling ParseMountRaw
 	// - but not when calling ParseMountSpec directly... nor when the unit test called it directly
 	// - but not when calling ParseMountSpec directly... nor when the unit test called it directly

+ 1 - 2
volume/parser.go

@@ -26,8 +26,7 @@ type Parser interface {
 	IsBackwardCompatible(m *MountPoint) bool
 	IsBackwardCompatible(m *MountPoint) bool
 	HasResource(m *MountPoint, absPath string) bool
 	HasResource(m *MountPoint, absPath string) bool
 	ValidateTmpfsMountDestination(dest string) error
 	ValidateTmpfsMountDestination(dest string) error
-
-	validateMountConfig(mt *mount.Mount) error
+	ValidateMountConfig(mt *mount.Mount) error
 }
 }
 
 
 // NewParser creates a parser for a given container OS, depending on the current host OS (linux on a windows host will resolve to an lcowParser)
 // NewParser creates a parser for a given container OS, depending on the current host OS (linux on a windows host will resolve to an lcowParser)

+ 4 - 8
volume/validate_test.go

@@ -31,13 +31,9 @@ func TestValidateMount(t *testing.T) {
 
 
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, nil},
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, nil},
 		{mount.Mount{Type: "invalid", Target: testDestinationPath}, errors.New("mount type unknown")},
 		{mount.Mount{Type: "invalid", Target: testDestinationPath}, errors.New("mount type unknown")},
+		{mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, errBindNotExist},
 	}
 	}
-	if runtime.GOOS == "windows" {
-		cases = append(cases, struct {
-			input    mount.Mount
-			expected error
-		}{mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, errBindNotExist}) // bind source existance is not checked on linux
-	}
+
 	lcowCases := []struct {
 	lcowCases := []struct {
 		input    mount.Mount
 		input    mount.Mount
 		expected error
 		expected error
@@ -54,7 +50,7 @@ func TestValidateMount(t *testing.T) {
 	}
 	}
 	parser := NewParser(runtime.GOOS)
 	parser := NewParser(runtime.GOOS)
 	for i, x := range cases {
 	for i, x := range cases {
-		err := parser.validateMountConfig(&x.input)
+		err := parser.ValidateMountConfig(&x.input)
 		if err == nil && x.expected == nil {
 		if err == nil && x.expected == nil {
 			continue
 			continue
 		}
 		}
@@ -65,7 +61,7 @@ func TestValidateMount(t *testing.T) {
 	if runtime.GOOS == "windows" {
 	if runtime.GOOS == "windows" {
 		parser = &lcowParser{}
 		parser = &lcowParser{}
 		for i, x := range lcowCases {
 		for i, x := range lcowCases {
-			err := parser.validateMountConfig(&x.input)
+			err := parser.ValidateMountConfig(&x.input)
 			if err == nil && x.expected == nil {
 			if err == nil && x.expected == nil {
 				continue
 				continue
 			}
 			}

+ 1 - 1
volume/windows_parser.go

@@ -189,7 +189,7 @@ func (p *windowsParser) ValidateVolumeName(name string) error {
 	}
 	}
 	return nil
 	return nil
 }
 }
-func (p *windowsParser) validateMountConfig(mnt *mount.Mount) error {
+func (p *windowsParser) ValidateMountConfig(mnt *mount.Mount) error {
 	return p.validateMountConfigReg(mnt, rxDestination, windowsSpecificValidators)
 	return p.validateMountConfigReg(mnt, rxDestination, windowsSpecificValidators)
 }
 }