Add basic validation of Containers

pkg/api/validation.go

@@ -57,6 +57,26 @@ func validateVolumes(volumes []Volume) (util.StringSet, error) {
 	return allNames, nil
 }
 
+func validateContainers(containers []Container, volumes util.StringSet) error {
+	allNames := util.StringSet{}
+	for i := range containers {
+		ctr := &containers[i] // so we can set default values
+		if len(ctr.Name) > 63 || !util.IsDNSLabel(ctr.Name) {
+			return errInvalid("Container.Name", ctr.Name)
+		}
+		if allNames.Has(ctr.Name) {
+			return errNotUnique("Container.Name", ctr.Name)
+		}
+		allNames.Insert(ctr.Name)
+		if len(ctr.Image) == 0 {
+			return errInvalid("Container.Image", ctr.Name)
+		}
+
+		// TODO(thockin): finish validation.
+	}
+	return nil
+}
+
 // ValidateManifest tests that the specified ContainerManifest has valid data.
 // This includes checking formatting and uniqueness.  It also canonicalizes the
 // structure by setting default values and implementing any backwards-compatibility
@@ -72,10 +92,12 @@ func ValidateManifest(manifest *ContainerManifest) error {
 	if len(manifest.ID) > 255 || !util.IsDNSSubdomain(manifest.ID) {
 		return errInvalid("ContainerManifest.ID", manifest.ID)
 	}
-	_, err := validateVolumes(manifest.Volumes)
+	allVolumes, err := validateVolumes(manifest.Volumes)
 	if err != nil {
 		return err
 	}
-	// TODO(thockin): finish validation.
+	if err := validateContainers(manifest.Containers, allVolumes); err != nil {
+		return err
+	}
 	return nil
 }

pkg/api/validation_test.go

@@ -19,6 +19,8 @@ package api
 import (
 	"strings"
 	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 func TestValidateVolumes(t *testing.T) {
@@ -49,6 +51,37 @@ func TestValidateVolumes(t *testing.T) {
 	}
 }
 
+func TestValidateContainers(t *testing.T) {
+	volumes := util.StringSet{}
+
+	successCase := []Container{
+		{Name: "abc", Image: "image"},
+		{Name: "123", Image: "image"},
+		{Name: "abc-123", Image: "image"},
+	}
+	err := validateContainers(successCase, volumes)
+	if err != nil {
+		t.Errorf("expected success: %v", err)
+	}
+
+	errorCases := map[string][]Container{
+		"zero-length name":     {{Name: "", Image: "image"}},
+		"name > 63 characters": {{Name: strings.Repeat("a", 64), Image: "image"}},
+		"name not a DNS label": {{Name: "a.b.c", Image: "image"}},
+		"name not unique": {
+			{Name: "abc", Image: "image"},
+			{Name: "abc", Image: "image"},
+		},
+		"zero-length image": {{Name: "abc", Image: ""}},
+	}
+	for k, v := range errorCases {
+		err := validateContainers(v, volumes)
+		if err == nil {
+			t.Errorf("expected failure for %s", k)
+		}
+	}
+}
+
 func TestValidateManifest(t *testing.T) {
 	successCases := []ContainerManifest{
 		{Version: "v1beta1", ID: "abc"},
@@ -58,6 +91,16 @@ func TestValidateManifest(t *testing.T) {
 			Version: "v1beta1",
 			ID:      "abc",
 			Volumes: []Volume{{Name: "vol1"}, {Name: "vol2"}},
+			Containers: []Container{
+				{
+					Name:       "abc",
+					Image:      "image",
+					Command:    []string{"foo", "bar"},
+					WorkingDir: "/tmp",
+					Memory:     1,
+					CPU:        1,
+				},
+			},
 		},
 	}
 	for _, manifest := range successCases {
@@ -78,6 +121,11 @@ func TestValidateManifest(t *testing.T) {
 			ID:      "abc",
 			Volumes: []Volume{{Name: "vol.1"}},
 		},
+		"invalid container name": {
+			Version:    "v1beta1",
+			ID:         "abc",
+			Containers: []Container{{Name: "ctr.1", Image: "image"}},
+		},
 	}
 	for k, v := range errorCases {
 		err := ValidateManifest(&v)