From dd6b209617aa6d005f1fecd72c029afc419fb2f6 Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Fri, 4 Jul 2014 19:46:56 -0700
Subject: [PATCH] Add validation of VolumeMounts

---
 pkg/api/types.go           |  3 +++
 pkg/api/validation.go      | 31 ++++++++++++++++++++++++++-
 pkg/api/validation_test.go | 43 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/pkg/api/types.go b/pkg/api/types.go
index 92b91adc488..4fe5963e217 100644
--- a/pkg/api/types.go
+++ b/pkg/api/types.go
@@ -84,7 +84,10 @@ type VolumeMount struct {
 	// Optional: Defaults to false (read-write).
 	ReadOnly bool `yaml:"readOnly,omitempty" json:"readOnly,omitempty"`
 	// Required.
+	// Exactly one of the following must be set.  If both are set, prefer MountPath.
+	// DEPRECATED: Path will be removed in a future version of the API.
 	MountPath string `yaml:"mountPath,omitempty" json:"mountPath,omitempty"`
+	Path string `yaml:"Path,omitempty" json:"Path,omitempty"`
 	// One of: "LOCAL" (local volume) or "HOST" (external mount from the host). Default: LOCAL.
 	MountType string `yaml:"mountType,omitempty" json:"mountType,omitempty"`
 }
diff --git a/pkg/api/validation.go b/pkg/api/validation.go
index ded4b96bbd6..cdff3ad0fe6 100644
--- a/pkg/api/validation.go
+++ b/pkg/api/validation.go
@@ -35,6 +35,7 @@ const (
 	ErrTypeInvalid      ValidationErrorEnum = "invalid value"
 	ErrTypeNotSupported ValidationErrorEnum = "unsupported value"
 	ErrTypeDuplicate    ValidationErrorEnum = "duplicate value"
+	ErrTypeNotFound     ValidationErrorEnum = "not found"
 )
 
 // ValidationError is an implementation of the 'error' interface, which represents an error of validation.
@@ -61,6 +62,10 @@ func makeDuplicateError(field string, value interface{}) ValidationError {
 	return ValidationError{ErrTypeDuplicate, field, value}
 }
 
+func makeNotFoundError(field string, value interface{}) ValidationError {
+	return ValidationError{ErrTypeNotFound, field, value}
+}
+
 func validateVolumes(volumes []Volume) (util.StringSet, error) {
 	allNames := util.StringSet{}
 	for i := range volumes {
@@ -95,6 +100,28 @@ func validateEnv(vars []EnvVar) error {
 	return nil
 }
 
+func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) error {
+	for i := range mounts {
+		mnt := &mounts[i] // so we can set default values
+		if len(mnt.Name) == 0 {
+			return makeInvalidError("VolumeMount.Name", mnt.Name)
+		}
+		if !volumes.Has(mnt.Name) {
+			return makeNotFoundError("VolumeMount.Name", mnt.Name)
+		}
+		if len(mnt.MountPath) == 0 {
+			// Backwards compat.
+			if len(mnt.Path) == 0 {
+				return makeInvalidError("VolumeMount.MountPath", mnt.MountPath)
+			}
+			glog.Warning("DEPRECATED: VolumeMount.Path has been replaced by VolumeMount.MountPath")
+			mnt.MountPath = mnt.Path
+			mnt.Path = ""
+		}
+	}
+	return nil
+}
+
 func validateContainers(containers []Container, volumes util.StringSet) error {
 	allNames := util.StringSet{}
 	for i := range containers {
@@ -112,7 +139,9 @@ func validateContainers(containers []Container, volumes util.StringSet) error {
 		if err := validateEnv(ctr.Env); err != nil {
 			return err
 		}
-
+		if err := validateVolumeMounts(ctr.VolumeMounts, volumes); err != nil {
+			return err
+		}
 		// TODO(thockin): finish validation.
 	}
 	return nil
diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go
index 52b2c24c6a2..c55424c4598 100644
--- a/pkg/api/validation_test.go
+++ b/pkg/api/validation_test.go
@@ -82,6 +82,42 @@ func TestValidateEnv(t *testing.T) {
 	}
 }
 
+func TestValidateVolumeMounts(t *testing.T) {
+	volumes := util.NewStringSet("abc", "123", "abc-123")
+
+	successCase := []VolumeMount{
+		{Name: "abc", MountPath: "/foo"},
+		{Name: "123", MountPath: "/foo"},
+		{Name: "abc-123", MountPath: "/bar"},
+	}
+	if err := validateVolumeMounts(successCase, volumes); err != nil {
+		t.Errorf("expected success: %v", err)
+	}
+
+	nonCanonicalCase := []VolumeMount{
+		{Name: "abc", Path: "/foo"},
+	}
+	err := validateVolumeMounts(nonCanonicalCase, volumes)
+	if err != nil {
+		t.Errorf("expected success: %v", err)
+	}
+	if nonCanonicalCase[0].MountPath != "/foo" {
+		t.Errorf("expected canonicalized values: %+v", nonCanonicalCase[0])
+	}
+
+	errorCases := map[string][]VolumeMount{
+		"empty name":      {{Name: "", MountPath: "/foo"}},
+		"name not found":  {{Name: "", MountPath: "/foo"}},
+		"empty mountpath": {{Name: "abc", MountPath: ""}},
+	}
+	for k, v := range errorCases {
+		err := validateVolumeMounts(v, volumes)
+		if err == nil {
+			t.Errorf("expected failure for %s", k)
+		}
+	}
+}
+
 func TestValidateContainers(t *testing.T) {
 	volumes := util.StringSet{}
 
@@ -106,6 +142,9 @@ func TestValidateContainers(t *testing.T) {
 		"invalid env var name": {
 			{Name: "abc", Image: "image", Env: []EnvVar{{Name: "ev.1"}}},
 		},
+		"unknown volume name": {
+			{Name: "abc", Image: "image", VolumeMounts: []VolumeMount{{Name: "anything", MountPath: "/foo"}}},
+		},
 	}
 	for k, v := range errorCases {
 		if err := validateContainers(v, volumes); err == nil {
@@ -136,6 +175,10 @@ func TestValidateManifest(t *testing.T) {
 						{Name: "ev2", Value: "val2"},
 						{Key: "EV3", Value: "val3"},
 					},
+					VolumeMounts: []VolumeMount{
+						{Name: "vol1", MountPath: "/foo"},
+						{Name: "vol1", Path: "/bar"},
+					},
 				},
 			},
 		},