Add validation for Container.Env

This includes backwards compat with the older "key" field.

pkg/api/types.go

@@ -92,7 +92,10 @@ type VolumeMount struct {
 // EnvVar represents an environment variable present in a Container
 type EnvVar struct {
 	// Required: This must be a C_IDENTIFIER.
+	// Exactly one of the following must be set.  If both are set, prefer Name.
+	// DEPRECATED: EnvVar.Key will be removed in a future version of the API.
 	Name string `yaml:"name" json:"name"`
+	Key  string `yaml:"key,omitempty" json:"name,omitempty"`
 	// Optional: defaults to "".
 	Value string `yaml:"value,omitempty" json:"value,omitempty"`
 }

pkg/api/validation.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+	"github.com/golang/glog"
 )
 
 func isSupportedManifestVersion(value string) bool {
@@ -57,6 +58,25 @@ func validateVolumes(volumes []Volume) (util.StringSet, error) {
 	return allNames, nil
 }
 
+func validateEnv(vars []EnvVar) error {
+	for i := range vars {
+		ev := &vars[i] // so we can set default values
+		if len(ev.Name) == 0 {
+			// Backwards compat.
+			if len(ev.Key) == 0 {
+				return errInvalid("EnvVar.Name", ev.Name)
+			}
+			glog.Warning("DEPRECATED: EnvVar.Key has been replaced by EnvVar.Name")
+			ev.Name = ev.Key
+			ev.Key = ""
+		}
+		if !util.IsCIdentifier(ev.Name) {
+			return errInvalid("EnvVar.Name", ev.Name)
+		}
+	}
+	return nil
+}
+
 func validateContainers(containers []Container, volumes util.StringSet) error {
 	allNames := util.StringSet{}
 	for i := range containers {
@@ -71,6 +91,9 @@ func validateContainers(containers []Container, volumes util.StringSet) error {
 		if len(ctr.Image) == 0 {
 			return errInvalid("Container.Image", ctr.Name)
 		}
+		if err := validateEnv(ctr.Env); err != nil {
+			return err
+		}
 
 		// TODO(thockin): finish validation.
 	}

pkg/api/validation_test.go

@@ -44,8 +44,39 @@ func TestValidateVolumes(t *testing.T) {
 		"name not unique":      {{Name: "abc"}, {Name: "abc"}},
 	}
 	for k, v := range errorCases {
-		_, err := validateVolumes(v)
-		if err == nil {
+		if _, err := validateVolumes(v); err == nil {
+			t.Errorf("expected failure for %s", k)
+		}
+	}
+}
+
+func TestValidateEnv(t *testing.T) {
+	successCase := []EnvVar{
+		{Name: "abc", Value: "value"},
+		{Name: "ABC", Value: "value"},
+		{Name: "AbC_123", Value: "value"},
+		{Name: "abc", Value: ""},
+	}
+	if err := validateEnv(successCase); err != nil {
+		t.Errorf("expected success: %v", err)
+	}
+
+	nonCanonicalCase := []EnvVar{
+		{Key: "EV"},
+	}
+	if err := validateEnv(nonCanonicalCase); err != nil {
+		t.Errorf("expected success: %v", err)
+	}
+	if nonCanonicalCase[0].Name != "EV" || nonCanonicalCase[0].Value != "" {
+		t.Errorf("expected default values: %+v", nonCanonicalCase[0])
+	}
+
+	errorCases := map[string][]EnvVar{
+		"zero-length name":        {{Name: ""}},
+		"name not a C identifier": {{Name: "a.b.c"}},
+	}
+	for k, v := range errorCases {
+		if err := validateEnv(v); err == nil {
 			t.Errorf("expected failure for %s", k)
 		}
 	}
@@ -59,8 +90,7 @@ func TestValidateContainers(t *testing.T) {
 		{Name: "123", Image: "image"},
 		{Name: "abc-123", Image: "image"},
 	}
-	err := validateContainers(successCase, volumes)
-	if err != nil {
+	if err := validateContainers(successCase, volumes); err != nil {
 		t.Errorf("expected success: %v", err)
 	}
 
@@ -73,10 +103,12 @@ func TestValidateContainers(t *testing.T) {
 			{Name: "abc", Image: "image"},
 		},
 		"zero-length image": {{Name: "abc", Image: ""}},
+		"invalid env var name": {
+			{Name: "abc", Image: "image", Env: []EnvVar{{Name: "ev.1"}}},
+		},
 	}
 	for k, v := range errorCases {
-		err := validateContainers(v, volumes)
-		if err == nil {
+		if err := validateContainers(v, volumes); err == nil {
 			t.Errorf("expected failure for %s", k)
 		}
 	}
@@ -99,13 +131,17 @@ func TestValidateManifest(t *testing.T) {
 					WorkingDir: "/tmp",
 					Memory:     1,
 					CPU:        1,
+					Env: []EnvVar{
+						{Name: "ev1", Value: "val1"},
+						{Name: "ev2", Value: "val2"},
+						{Key: "EV3", Value: "val3"},
+					},
 				},
 			},
 		},
 	}
 	for _, manifest := range successCases {
-		err := ValidateManifest(&manifest)
-		if err != nil {
+		if err := ValidateManifest(&manifest); err != nil {
 			t.Errorf("expected success: %v", err)
 		}
 	}
@@ -128,8 +164,7 @@ func TestValidateManifest(t *testing.T) {
 		},
 	}
 	for k, v := range errorCases {
-		err := ValidateManifest(&v)
-		if err == nil {
+		if err := ValidateManifest(&v); err == nil {
 			t.Errorf("expected failure for %s", k)
 		}
 	}