diff --git a/pkg/api/types.go b/pkg/api/types.go index 9e11e52714a..27b21c5276a 100644 --- a/pkg/api/types.go +++ b/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"` } diff --git a/pkg/api/validation.go b/pkg/api/validation.go index cf65ed382c4..19cf33662d0 100644 --- a/pkg/api/validation.go +++ b/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. } diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go index 13f3ca279df..52b2c24c6a2 100644 --- a/pkg/api/validation_test.go +++ b/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) } }