Merge pull request #2621 from brendandburns/kubecfg

Add validation back in

pkg/api/validation/schema.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/emicklei/go-restful/swagger"
 	"github.com/golang/glog"
+	"gopkg.in/v2/yaml"
 )
 
 type InvalidTypeError struct {
@@ -40,12 +41,21 @@ func NewInvalidTypeError(expected reflect.Kind, observed reflect.Kind, fieldName
 	return &InvalidTypeError{expected, observed, fieldName}
 }
 
-type Schema struct {
+// Schema is an interface that knows how to validate an API object serialized to a byte array.
+type Schema interface {
+	ValidateBytes(data []byte) error
+}
+
+type NullSchema struct{}
+
+func (NullSchema) ValidateBytes(data []byte) error { return nil }
+
+type SwaggerSchema struct {
 	api swagger.ApiDeclaration
 }
 
-func NewSchemaFromBytes(data []byte) (*Schema, error) {
-	schema := &Schema{}
+func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) {
+	schema := &SwaggerSchema{}
 	err := json.Unmarshal(data, &schema.api)
 	if err != nil {
 		return nil, err
@@ -53,19 +63,19 @@ func NewSchemaFromBytes(data []byte) (*Schema, error) {
 	return schema, nil
 }
 
-func (s *Schema) ValidateBytes(data []byte) error {
+func (s *SwaggerSchema) ValidateBytes(data []byte) error {
 	var obj interface{}
-	err := json.Unmarshal(data, &obj)
+	err := yaml.Unmarshal(data, &obj)
 	if err != nil {
 		return err
 	}
-	fields := obj.(map[string]interface{})
+	fields := obj.(map[interface{}]interface{})
 	apiVersion := fields["apiVersion"].(string)
 	kind := fields["kind"].(string)
 	return s.ValidateObject(obj, apiVersion, "", apiVersion+"."+kind)
 }
 
-func (s *Schema) ValidateObject(obj interface{}, apiVersion, fieldName, typeName string) error {
+func (s *SwaggerSchema) ValidateObject(obj interface{}, apiVersion, fieldName, typeName string) error {
 	models := s.api.Models
 	// TODO: handle required fields here too.
 	model, ok := models[typeName]
@@ -74,12 +84,12 @@ func (s *Schema) ValidateObject(obj interface{}, apiVersion, fieldName, typeName
 		return nil
 	}
 	properties := model.Properties
-	fields := obj.(map[string]interface{})
+	fields := obj.(map[interface{}]interface{})
 	if len(fieldName) > 0 {
 		fieldName = fieldName + "."
 	}
 	for key, value := range fields {
-		details, ok := properties[key]
+		details, ok := properties[key.(string)]
 		if !ok {
 			glog.V(2).Infof("couldn't find properties for %s, skipping", key)
 			continue
@@ -89,7 +99,7 @@ func (s *Schema) ValidateObject(obj interface{}, apiVersion, fieldName, typeName
 			glog.V(2).Infof("Skipping nil field: %s", key)
 			continue
 		}
-		err := s.validateField(value, apiVersion, fieldName+key, fieldType, &details)
+		err := s.validateField(value, apiVersion, fieldName+key.(string), fieldType, &details)
 		if err != nil {
 			glog.Errorf("Validation failed for: %s, %v", key, value)
 			return err
@@ -98,7 +108,7 @@ func (s *Schema) ValidateObject(obj interface{}, apiVersion, fieldName, typeName
 	return nil
 }
 
-func (s *Schema) validateField(value interface{}, apiVersion, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) error {
+func (s *SwaggerSchema) validateField(value interface{}, apiVersion, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) error {
 	if strings.HasPrefix(fieldType, apiVersion) {
 		return s.ValidateObject(value, apiVersion, fieldName, fieldType)
 	}
@@ -107,7 +117,8 @@ func (s *Schema) validateField(value interface{}, apiVersion, fieldName, fieldTy
 		// Be loose about what we accept for 'string' since we use IntOrString in a couple of places
 		_, isString := value.(string)
 		_, isNumber := value.(float64)
-		if !isString && !isNumber {
+		_, isInteger := value.(int)
+		if !isString && !isNumber && !isInteger {
 			return NewInvalidTypeError(reflect.String, reflect.TypeOf(value).Kind(), fieldName)
 		}
 	case "array":
@@ -124,7 +135,9 @@ func (s *Schema) validateField(value interface{}, apiVersion, fieldName, fieldTy
 		}
 	case "uint64":
 	case "integer":
-		if _, ok := value.(float64); !ok {
+		_, isNumber := value.(float64)
+		_, isInteger := value.(int)
+		if !isNumber && !isInteger {
 			return NewInvalidTypeError(reflect.Int, reflect.TypeOf(value).Kind(), fieldName)
 		}
 	case "float64":

pkg/api/validation/schema_test.go

@@ -30,12 +30,12 @@ import (
 	fuzz "github.com/google/gofuzz"
 )
 
-func LoadSchemaForTest(file string) (*Schema, error) {
+func LoadSchemaForTest(file string) (Schema, error) {
 	data, err := ioutil.ReadFile(file)
 	if err != nil {
 		return nil, err
 	}
-	return NewSchemaFromBytes(data)
+	return NewSwaggerSchemaFromBytes(data)
 }
 
 // TODO: this is cloned from serialization_test.go, refactor to somewhere common like util
@@ -251,12 +251,63 @@ var invalidPod3 = `{
 }
 `
 
+var invalidYaml = `
+id: name
+kind: Pod
+apiVersion: v1beta1
+desiredState:
+  manifest:
+    version: v1beta1
+    id: redis-master
+    containers:
+      - name: "master"
+        image: "dockerfile/redis"
+        command: "this is a bad command"
+labels:
+  name: "redis-master"
+`
+
 func TestInvalid(t *testing.T) {
 	schema, err := LoadSchemaForTest("v1beta1-swagger.json")
 	if err != nil {
 		t.Errorf("Failed to load: %v", err)
 	}
-	tests := []string{invalidPod, invalidPod2}
+	tests := []string{invalidPod, invalidPod2, invalidPod3, invalidYaml}
+	for _, test := range tests {
+		err = schema.ValidateBytes([]byte(test))
+		if err == nil {
+			t.Errorf("unexpected non-error\n%s", test)
+		}
+	}
+}
+
+var validYaml = `
+id: name
+kind: Pod
+apiVersion: v1beta1
+desiredState:
+  manifest:
+    version: v1beta1
+    id: redis-master
+    containers:
+      - name: "master"
+        image: "dockerfile/redis"
+        command:
+        	- this
+        	- is
+        	- an
+        	- ok
+        	- command
+labels:
+  name: "redis-master"
+`
+
+func TestValid(t *testing.T) {
+	schema, err := LoadSchemaForTest("v1beta1-swagger.json")
+	if err != nil {
+		t.Errorf("Failed to load: %v", err)
+	}
+	tests := []string{validYaml}
 	for _, test := range tests {
 		err = schema.ValidateBytes([]byte(test))
 		if err == nil {