Merge pull request #549 from sparkprime/json_schema

JSONschema support

cmd/expandybird/expander/expander.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"github.com/ghodss/yaml"
 	"log"
-	"os"
 	"os/exec"
 
 	"github.com/kubernetes/helm/pkg/expansion"
@@ -50,7 +49,11 @@ type expandyBirdOutput struct {
 // expanded configuration as a string on success.
 func (e *expander) ExpandChart(request *expansion.ServiceRequest) (*expansion.ServiceResponse, error) {
 
-	err := expansion.ValidateRequest(request)
+	if err := expansion.ValidateRequest(request); err != nil {
+		return nil, err
+	}
+
+	request, err := expansion.ValidateProperties(request)
 	if err != nil {
 		return nil, err
 	}
@@ -65,23 +68,15 @@ func (e *expander) ExpandChart(request *expansion.ServiceRequest) (*expansion.Se
 	}
 
 	entrypointIndex := -1
-	schemaIndex := -1
 	for i, f := range chartMembers {
 		if f.Path == chartFile.Expander.Entrypoint {
 			entrypointIndex = i
 		}
-		if f.Path == chartFile.Schema {
-			schemaIndex = i
-		}
 	}
 	if entrypointIndex == -1 {
 		message := fmt.Sprintf("The entrypoint in the chart.yaml cannot be found: %s", chartFile.Expander.Entrypoint)
 		return nil, fmt.Errorf("%s: %s", chartInv.Name, message)
 	}
-	if chartFile.Schema != "" && schemaIndex == -1 {
-		message := fmt.Sprintf("The schema in the chart.yaml cannot be found: %s", chartFile.Schema)
-		return nil, fmt.Errorf("%s: %s", chartInv.Name, message)
-	}
 
 	// Those are automatically increasing buffers, so writing arbitrary large
 	// data here won't block the child process.
@@ -104,20 +99,12 @@ func (e *expander) ExpandChart(request *expansion.ServiceRequest) (*expansion.Se
 		Stderr: &stderr,
 	}
 
-	if chartFile.Schema != "" {
-		// appending to exsiting Env is required
-		cmd.Env = append(os.Environ(), "VALIDATE_SCHEMA=1")
-	}
-
 	for i, f := range chartMembers {
 		name := f.Path
 		path := f.Path
 		if i == entrypointIndex {
 			// This is how expandyBird identifies the entrypoint.
 			name = chartInv.Type
-		} else if i == schemaIndex {
-			// Doesn't matter what it was originally called, expandyBird expects to find it here.
-			name = chartInv.Type + ".schema"
 		}
 		cmd.Args = append(cmd.Args, name, path, string(f.Content))
 	}

cmd/expandybird/expander/expander_test.go

@@ -478,7 +478,7 @@ func TestSchemaFail(t *testing.T) {
 			},
 		},
 		nil, // Response.
-		"Invalid properties for",
+		`"prop2" property is missing and required`,
 	)
 }
 

cmd/goexpander/expander/expander.go

@@ -90,7 +90,10 @@ func (e *expander) ExpandChart(request *expansion.ServiceRequest) (*expansion.Se
 		return nil, err
 	}
 
-	// TODO(dcunnin): Validate via JSONschema.
+	request, err = expansion.ValidateProperties(request)
+	if err != nil {
+		return nil, err
+	}
 
 	chartInv := request.ChartInvocation
 	chartMembers := request.Chart.Members

glide.lock

-hash: a72f13699934fc94c8df5b56c76fc66d2279b2a4ff1396c2f1bff944456a8adf
-updated: 2016-03-30T23:15:39.980396345-04:00
+hash: f9b47a1852e40963671d923d7170aa4d730f7d74de94a3c66420a3c1635bc5c5
+updated: 2016-04-01T21:35:30.235550603-04:00
 imports:
 - name: github.com/aokoli/goutils
   version: 45307ec16e3cd47cd841506c081f7afd8237d210
 - name: github.com/cloudfoundry-incubator/candiedyaml
   version: 479485e9bfc69ee37d074b36ce36da5e4fba7941
 - name: github.com/codegangsta/cli
-  version: a2943485b110df8842045ae0600047f88a3a56a1
+  version: bc465becccd1d527002fda095fc3c19d9c115029
 - name: github.com/emicklei/go-restful
-  version: b86acf97a74ed7603ac78d012f5535b4d587b156
+  version: 402f11d42bfe18198ffd5c68258c631c8fbf2c3c
   subpackages:
   - log
 - name: github.com/ghodss/yaml
-  version: 73d445a93680fa1a78ae23a5839bad48f32ba1ee
+  version: 1a6f069841556a7bcaff4a397ca6e8328d266c2f
 - name: github.com/google/go-github
-  version: b8b4ac742977310ff6e75140a403a38dab109977
+  version: 4403af9a2a0f2c2577be18a928d98f77d5748168
   subpackages:
   - github
 - name: github.com/gorilla/context
-  version: 1c83b3eabd45b6d76072b66b746c20815fb2872d
+  version: 1ea25387ff6f684839d82767c1733ff4d4d15d0a
 - name: github.com/gorilla/handlers
   version: ee54c7b44cab12289237fb8631314790076e728b
 - name: github.com/gorilla/mux
-  version: 26a6070f849969ba72b72256e9f14cf519751690
+  version: 0eeaf8392f5b04950925b8a69fe70f110fa7cbfc
+- name: github.com/juju/gojsonpointer
+  version: afe8b77aa08f272b49e01b82de78510c11f61500
+- name: github.com/juju/gojsonreference
+  version: f0d24ac5ee330baa21721cdff56d45e4ee42628e
+- name: github.com/juju/gojsonschema
+  version: e1ad140384f254c82f89450d9a7c8dd38a632838
 - name: github.com/Masterminds/httputil
   version: e9b977e9cf16f9d339573e18f0f1f7ce5d3f419a
 - name: github.com/Masterminds/semver
@@ -30,39 +36,35 @@ imports:
 - name: github.com/Masterminds/sprig
   version: 679bb747f11c6ffc3373965988fea8877c40b47b
 - name: golang.org/x/net
-  version: 04b9de9b512f58addf28c9853d50ebef61c3953e
+  version: 3e8a7b0329d536af18e227bb21b6da4d1dbbe180
   subpackages:
   - context
   - context/ctxhttp
 - name: golang.org/x/oauth2
-  version: 8a57ed94ffd43444c0879fe75701732a38afc985
+  version: 33fa30fe45020622640e947917fd1fc4c81e3dce
   subpackages:
   - google
   - internal
   - jws
   - jwt
 - name: google.golang.org/api
-  version: 0caa37974a5f5ae67172acf68b4970f7864f994c
+  version: 43c645d4bcf9251ced36c823a93b6d198764aae4
   subpackages:
   - storage/v1
   - gensupport
   - googleapi
   - googleapi/internal/uritemplates
-- name: google.golang.org/appengine
-  version: a503df954af258b9a70918df2a524d6a85ecefdb
-  subpackages:
-  - urlfetch
 - name: google.golang.org/cloud
-  version: fb10e8da373d97f6ba5e648299a10b3b91f14cd5
+  version: 8a7fce32d2cdf2d4e19068ecc53164b973b3e958
   subpackages:
   - compute/metadata
   - internal
 - name: gopkg.in/mgo.v2
-  version: d90005c5262a3463800497ea5a89aed5fe22c886
+  version: b6e2fa371e64216a45e61072a96d4e3859f169da
   subpackages:
   - bson
   - internal/sasl
   - internal/scram
 - name: gopkg.in/yaml.v2
-  version: f7716cbe52baa25d2e9b0d0da546fcf909fc16b4
+  version: a83829b6f1293c91addabc89d0571c246397bbf4
 devImports: []

glide.yaml

@@ -21,3 +21,4 @@ import:
 - package: github.com/Masterminds/sprig
   version: ^2.1.0
 - package: github.com/Masterminds/httputil
+- package: github.com/juju/gojsonschema

pkg/expansion/validate.go

@@ -17,9 +17,12 @@ limitations under the License.
 package expansion
 
 import (
-	"github.com/kubernetes/helm/pkg/chart"
-
+	"bytes"
 	"fmt"
+
+	"github.com/ghodss/yaml"
+	"github.com/juju/gojsonschema"
+	"github.com/kubernetes/helm/pkg/chart"
 )
 
 // ValidateRequest does basic sanity checks on the request.
@@ -50,3 +53,74 @@ func ValidateRequest(request *ServiceRequest) error {
 
 	return nil
 }
+
+// ValidateProperties validates the properties in the chart invocation against the schema file in
+// the chart itself, which is assumed to be JSONschema.  It also modifies a copy of the request to
+// add defaults values if properties are not provided (according to the default field in
+// JSONschema), and returns this copy.
+func ValidateProperties(request *ServiceRequest) (*ServiceRequest, error) {
+
+	schemaFilename := request.Chart.Chartfile.Schema
+
+	if schemaFilename == "" {
+		// No schema, so perform no validation.
+		return request, nil
+	}
+
+	chartInv := request.ChartInvocation
+
+	var schemaBytes *[]byte
+	for _, f := range request.Chart.Members {
+		if f.Path == schemaFilename {
+			schemaBytes = &f.Content
+		}
+	}
+	if schemaBytes == nil {
+		return nil, fmt.Errorf("%s: The schema referenced from the Chart.yaml cannot be found: %s",
+			chartInv.Name, schemaFilename)
+	}
+	var schemaDoc interface{}
+
+	if err := yaml.Unmarshal(*schemaBytes, &schemaDoc); err != nil {
+		return nil, fmt.Errorf("%s: %s was not valid YAML: %v",
+			chartInv.Name, schemaFilename, err)
+	}
+
+	// Build a schema object
+	schema, err := gojsonschema.NewSchema(gojsonschema.NewGoLoader(schemaDoc))
+	if err != nil {
+		return nil, err
+	}
+
+	// Do validation
+	result, err := schema.Validate(gojsonschema.NewGoLoader(request.ChartInvocation.Properties))
+	if err != nil {
+		return nil, err
+	}
+
+	// Need to concat errors here
+	if !result.Valid() {
+		var message bytes.Buffer
+		message.WriteString("Properties failed validation:\n")
+		for _, err := range result.Errors() {
+			message.WriteString(fmt.Sprintf("- %s", err))
+		}
+		return nil, fmt.Errorf("%s: %s", chartInv.Name, message.String())
+	}
+
+	// Fill in defaults (after validation).
+	modifiedProperties, err := schema.InsertDefaults(request.ChartInvocation.Properties)
+	if err != nil {
+		return nil, err
+	}
+
+	modifiedResource := *request.ChartInvocation
+	modifiedResource.Properties = modifiedProperties
+
+	modifiedRequest := &ServiceRequest{
+		ChartInvocation: &modifiedResource,
+		Chart:           request.Chart,
+	}
+
+	return modifiedRequest, nil
+}

pkg/expansion/validate_test.go

+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package expansion
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/kubernetes/helm/pkg/chart"
+	"github.com/kubernetes/helm/pkg/common"
+)
+
+func testPropertiesValidation(t *testing.T, req *ServiceRequest, expRequest *ServiceRequest, expError string) {
+	modifiedRequest, err := ValidateProperties(req)
+	if err != nil {
+		message := err.Error()
+		if expRequest != nil || !strings.Contains(message, expError) {
+			t.Fatalf("unexpected error: %v\n", err)
+		}
+	} else {
+		if expRequest == nil {
+			t.Fatalf("expected error did not occur: %s\n", expError)
+		}
+		if !reflect.DeepEqual(modifiedRequest, expRequest) {
+			message := fmt.Sprintf("want:\n%s\nhave:\n%s\n", expRequest, modifiedRequest)
+			t.Fatalf("output mismatch:\n%s\n", message)
+		}
+	}
+}
+
+func TestNoSchema(t *testing.T) {
+	req := &ServiceRequest{
+		ChartInvocation: &common.Resource{
+			Properties: map[string]interface{}{
+				"prop1": 3.0,
+				"prop2": "foo",
+			},
+		},
+		Chart: &chart.Content{
+			Chartfile: &chart.Chartfile{},
+			Members:   []*chart.Member{},
+		},
+	}
+	testPropertiesValidation(t, req, req, "") // Returns it unchanged.
+}
+
+func TestSchemaNotFound(t *testing.T) {
+	testPropertiesValidation(
+		t,
+		&ServiceRequest{
+			ChartInvocation: &common.Resource{
+				Properties: map[string]interface{}{
+					"prop1": 3.0,
+					"prop2": "foo",
+				},
+			},
+			Chart: &chart.Content{
+				Chartfile: &chart.Chartfile{
+					Schema: "Schema.yaml",
+				},
+			},
+		},
+		nil, // No response to check.
+		"The schema referenced from the Chart.yaml cannot be found: Schema.yaml",
+	)
+}
+
+var schemaContent = []byte(`
+  required: ["prop2"]
+  additionalProperties: false
+  properties:
+    prop1:
+      description: Nice description.
+      type: integer
+      default: 42
+    prop2:
+      description: Nice description.
+      type: string
+`)
+
+func TestSchema(t *testing.T) {
+	req := &ServiceRequest{
+		ChartInvocation: &common.Resource{
+			Properties: map[string]interface{}{
+				"prop1": 3.0,
+				"prop2": "foo",
+			},
+		},
+		Chart: &chart.Content{
+			Chartfile: &chart.Chartfile{
+				Schema: "Schema.yaml",
+			},
+			Members: []*chart.Member{
+				{
+					Path:    "Schema.yaml",
+					Content: schemaContent,
+				},
+			},
+		},
+	}
+	// No defaults, returns it unchanged:
+	testPropertiesValidation(t, req, req, "")
+}
+
+func TestBadProperties(t *testing.T) {
+	testPropertiesValidation(
+		t,
+		&ServiceRequest{
+			ChartInvocation: &common.Resource{
+				Properties: map[string]interface{}{
+					"prop1": 3.0,
+					"prop3": map[string]interface{}{},
+				},
+			},
+			Chart: &chart.Content{
+				Chartfile: &chart.Chartfile{
+					Schema: "Schema.yaml",
+				},
+				Members: []*chart.Member{
+					{
+						Path:    "Schema.yaml",
+						Content: schemaContent,
+					},
+				},
+			},
+		},
+		nil,
+		"Properties failed validation:",
+	)
+}
+
+func TestDefault(t *testing.T) {
+	testPropertiesValidation(
+		t,
+		&ServiceRequest{
+			ChartInvocation: &common.Resource{
+				Name: "TestName",
+				Type: "TestType",
+				Properties: map[string]interface{}{
+					"prop2": "ok",
+				},
+			},
+			Chart: &chart.Content{
+				Chartfile: &chart.Chartfile{
+					Schema: "Schema.yaml",
+				},
+				Members: []*chart.Member{
+					{
+						Path:    "Schema.yaml",
+						Content: schemaContent,
+					},
+				},
+			},
+		},
+		&ServiceRequest{
+			ChartInvocation: &common.Resource{
+				Name: "TestName",
+				Type: "TestType",
+				Properties: map[string]interface{}{
+					"prop1": 42.0,
+					"prop2": "ok",
+				},
+			},
+			Chart: &chart.Content{
+				Chartfile: &chart.Chartfile{
+					Schema: "Schema.yaml",
+				},
+				Members: []*chart.Member{
+					{
+						Path:    "Schema.yaml",
+						Content: schemaContent,
+					},
+				},
+			},
+		},
+		"", // Error
+	)
+}