Prevent in-built docker VOLUME commands (#1378)

Makefile

@@ -25,7 +25,7 @@ checkfmt:
 .PHONY: clear-images
 clear-images:
 	-docker images -q -f dangling=true | xargs docker rmi -f
-	for i in fnproject/fn-test-utils fnproject/fn-status-checker fnproject/dind fnproject/fnserver ; do \
+	for i in fnproject/fn-test-utils fnproject/fn-status-checker fnproject/dind fnproject/fnserver fnproject/fn-test-volume ; do \
 	    docker images "$$i" --format '{{ .ID }}\t{{ .Repository }}\t{{ .Tag}}' | while read id repo tag; do \
 	        if [ "$$tag" = "<none>" ]; then docker rmi "$$id"; else docker rmi "$$repo:$$tag"; fi; done; done
 
@@ -49,6 +49,10 @@ fn-status-checker: checkfmt
 fn-test-utils: checkfmt
 	cd images/fn-test-utils && ./build.sh
 
+.PHONY: fn-test-volume
+fn-test-volume:
+	cd images/fn-test-volume && ./build.sh
+
 .PHONY: test-middleware
 test-middleware: test-basic
 	cd examples/middleware && go build
@@ -58,7 +62,7 @@ test-extensions: test-basic
 	cd examples/extensions && go build
 
 .PHONY: test-basic
-test-basic: checkfmt pull-images fn-test-utils fn-status-checker
+test-basic: checkfmt pull-images fn-test-utils fn-status-checker fn-test-volume
 	./test.sh
 
 .PHONY: test

api/agent/agent.go

@@ -230,6 +230,7 @@ func NewDockerDriver(cfg *Config) (drivers.Driver, error) {
 		ContainerLabelTag:    cfg.ContainerLabelTag,
 		ImageCleanMaxSize:    cfg.ImageCleanMaxSize,
 		ImageCleanExemptTags: cfg.ImageCleanExemptTags,
+		ImageEnableVolume:    cfg.ImageEnableVolume,
 	})
 }
 

api/agent/config.go

@@ -43,6 +43,7 @@ type Config struct {
 	MaxDockerRetries        uint64        `json:"max_docker_retries"`
 	ImageCleanMaxSize       uint64        `json:"image_clean_max_size"`
 	ImageCleanExemptTags    string        `json:"image_clean_exempt_tags"`
+	ImageEnableVolume       bool          `json:"image_enable_volume"`
 }
 
 const (
@@ -52,6 +53,8 @@ const (
 	EnvImageCleanMaxSize = "FN_IMAGE_CLEAN_MAX_SIZE"
 	// EnvImageCleanExemptTags list of image names separated by whitespace that are exempt from removal in image cleaner
 	EnvImageCleanExemptTags = "FN_IMAGE_CLEAN_EXEMPT_TAGS"
+	// EnvImageEnableVolume allows image to contain VOLUME definitions
+	EnvImageEnableVolume = "FN_IMAGE_ENABLE_VOLUME"
 	// EnvDockerNetworks is a comma separated list of networks to attach to each container started
 	EnvDockerNetworks = "FN_DOCKER_NETWORKS"
 	// EnvDockerLoadFile is a file location for a file that contains a tarball of a docker image to load on startup
@@ -170,6 +173,7 @@ func NewConfig() (*Config, error) {
 	err = setEnvBool(err, EnvDisableDebugUserLogs, &cfg.DisableDebugUserLogs)
 	err = setEnvUint(err, EnvImageCleanMaxSize, &cfg.ImageCleanMaxSize)
 	err = setEnvStr(err, EnvImageCleanExemptTags, &cfg.ImageCleanExemptTags)
+	err = setEnvBool(err, EnvImageEnableVolume, &cfg.ImageEnableVolume)
 	if err != nil {
 		return cfg, err
 	}

api/agent/drivers/docker/cookie.go

@@ -3,6 +3,7 @@ package docker
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"path"
@@ -17,6 +18,8 @@ import (
 	"go.opencensus.io/trace"
 )
 
+var ErrImageWithVolume = models.NewAPIError(http.StatusBadRequest, errors.New("image has Volume definition"))
+
 // A cookie identifies a unique request to run a task.
 type cookie struct {
 	// namespace id used from prefork pool if applicable
@@ -368,6 +371,11 @@ func (c *cookie) ValidateImage(ctx context.Context) (bool, error) {
 		return false, err
 	}
 
+	// check image doesn't have Volumes
+	if !c.drv.conf.ImageEnableVolume && img.Config != nil && len(img.Config.Volumes) > 0 {
+		err = ErrImageWithVolume
+	}
+
 	c.image = &CachedImage{
 		ID:       img.ID,
 		ParentID: img.Parent,
@@ -376,7 +384,11 @@ func (c *cookie) ValidateImage(ctx context.Context) (bool, error) {
 	}
 
 	if c.drv.imgCache != nil {
-		c.drv.imgCache.MarkBusy(c.image)
+		if err == ErrImageWithVolume {
+			c.drv.imgCache.Update(c.image)
+		} else {
+			c.drv.imgCache.MarkBusy(c.image)
+		}
 	}
 	return false, err
 }

api/agent/drivers/docker/docker_test.go

@@ -87,6 +87,48 @@ func commonCookieRun(ctx context.Context, cookie drivers.Cookie) (error, drivers
 	return nil, waiter.Wait(ctx)
 }
 
+type taskDockerVolumeTest struct {
+	taskDockerTest
+}
+
+func (f *taskDockerVolumeTest) Image() string { return "fnproject/fn-test-volume:latest" }
+
+func TestVolumeValidation(t *testing.T) {
+	dkr := NewDocker(drivers.Config{})
+	defer dkr.Close()
+
+	ctx := context.Background()
+	var output bytes.Buffer
+	var errors bytes.Buffer
+
+	task := &taskDockerVolumeTest{taskDockerTest{id: "test-docker"}}
+	task.output = &output
+	task.errors = &errors
+
+	cookie, err := dkr.CreateCookie(ctx, task)
+	if err != nil {
+		t.Fatal("Couldn't create task cookie")
+	}
+
+	defer cookie.Close(ctx)
+
+	err = cookie.AuthImage(ctx)
+	if err != nil {
+		t.Fatalf("error during image authentication: %s", err)
+	}
+
+	shouldPull, err := cookie.ValidateImage(ctx)
+
+	if shouldPull == true {
+		t.Fatalf("shouldPull must be false, because the image [%s] must be locally available", task.Image())
+	}
+
+	if err != ErrImageWithVolume {
+		t.Fatalf("ValidateImage must fail with error: %s", ErrImageWithVolume)
+	}
+
+}
+
 func TestRunnerDocker(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(30)*time.Second)
 	defer cancel()

api/agent/drivers/driver.go

@@ -263,6 +263,7 @@ type Config struct {
 	InstanceId           string `json:"instance_id"`
 	ImageCleanMaxSize    uint64 `json:"image_clean_max_size"`
 	ImageCleanExemptTags string `json:"image_clean_exempt_tags"`
+	ImageEnableVolume    bool   `json:"image_enable_volume"`
 }
 
 func average(samples []Stat) (Stat, bool) {

images/fn-test-volume/Dockerfile

@@ -0,0 +1,4 @@
+# This image is used by runner tests only
+FROM scratch
+
+VOLUME /data

images/fn-test-volume/build.sh

@@ -0,0 +1,2 @@
+set -e
+docker build --build-arg HTTPS_PROXY --build-arg HTTP_PROXY -t fnproject/fn-test-volume:latest .