Merge pull request #453 from vmware-tanzu/allow-null-array-items

Allow null array items

pkg/cmd/template/schema_test.go

@@ -1106,7 +1106,7 @@ rendered: #@ data.values
 
 		assertSucceeds(t, filesToProcess, expected, opts)
 	})
-	t.Run("when the value is a map", func(t *testing.T) {
+	t.Run("on a map", func(t *testing.T) {
 		schemaYAML := `#@data/values-schema
 ---
 defaults:
@@ -1148,7 +1148,38 @@ overriden:
 
 		assertSucceeds(t, filesToProcess, expected, opts)
 	})
-	t.Run("when the value is a array", func(t *testing.T) {
+	t.Run("on a map item", func(t *testing.T) {
+		schemaYAML := `#@data/values-schema
+---
+map:
+  #@schema/nullable
+  a: 1
+  #@schema/nullable
+  b: 2
+`
+		dataValuesYAML := `#@data/values
+---
+map:
+  a: 1
+`
+		templateYAML := `#@ load("@ytt:data", "data")
+---
+map: #@ data.values.map
+`
+		expected := `map:
+  a: 1
+  b: null
+`
+
+		filesToProcess := files.NewSortedFiles([]*files.File{
+			files.MustNewFileFromSource(files.NewBytesSource("schema.yml", []byte(schemaYAML))),
+			files.MustNewFileFromSource(files.NewBytesSource("dataValues.yml", []byte(dataValuesYAML))),
+			files.MustNewFileFromSource(files.NewBytesSource("template.yml", []byte(templateYAML))),
+		})
+
+		assertSucceeds(t, filesToProcess, expected, opts)
+	})
+	t.Run("on an array", func(t *testing.T) {
 		schemaYAML := `#@data/values-schema
 ---
 defaults:
@@ -1188,7 +1219,41 @@ overriden:
 
 		assertSucceeds(t, filesToProcess, expected, opts)
 	})
-	t.Run("when the value is a scalar", func(t *testing.T) {
+	t.Run("on an array item", func(t *testing.T) {
+		schemaYAML := `#@data/values-schema
+---
+array:
+#@schema/nullable
+- ""
+`
+		dataValuesYAML := `#@data/values
+---
+array:
+- one
+- null
+- two
+-
+`
+		templateYAML := `#@ load("@ytt:data", "data")
+---
+array: #@ data.values.array
+`
+		expected := `array:
+- one
+- null
+- two
+- null
+`
+
+		filesToProcess := files.NewSortedFiles([]*files.File{
+			files.MustNewFileFromSource(files.NewBytesSource("schema.yml", []byte(schemaYAML))),
+			files.MustNewFileFromSource(files.NewBytesSource("dataValues.yml", []byte(dataValuesYAML))),
+			files.MustNewFileFromSource(files.NewBytesSource("template.yml", []byte(templateYAML))),
+		})
+
+		assertSucceeds(t, filesToProcess, expected, opts)
+	})
+	t.Run("on a scalar", func(t *testing.T) {
 		schemaYAML := `#@data/values-schema
 ---
 defaults:
@@ -2246,35 +2311,6 @@ schema.yml:
 
 		assertFails(t, filesToProcess, expectedErr, opts)
 	})
-	t.Run("array with a nullable annotation", func(t *testing.T) {
-		schemaYAML := `#@data/values-schema
----
-vpc:
-  subnet_ids:
-  #@schema/nullable
-  - 0
-`
-
-		filesToProcess := files.NewSortedFiles([]*files.File{
-			files.MustNewFileFromSource(files.NewBytesSource("schema.yml", []byte(schemaYAML))),
-		})
-
-		expectedErr := `
-Invalid schema - @schema/nullable is not supported on array items
-=================================================================
-
-schema.yml:
-    |
-  6 |   - 0
-    |
-
-    = found: @schema/nullable
-    = expected: a valid annotation
-    = hint: Remove the @schema/nullable annotation from array item
-`
-
-		assertFails(t, filesToProcess, expectedErr, opts)
-	})
 	t.Run("array with a null value", func(t *testing.T) {
 		schemaYAML := `#@data/values-schema
 ---

pkg/schema/schema.go

@@ -70,29 +70,12 @@ func NewNullSchema() *DocumentSchema {
 }
 
 func NewDocumentType(doc *yamlmeta.Document) (*DocumentType, error) {
-	var typeOfValue yamlmeta.Type
-
-	anns, err := collectAnnotations(doc)
-	if err != nil {
-		return nil, NewSchemaError("Invalid schema", err)
-	}
-	typeOfValue = getTypeFromAnnotations(anns)
-
-	if typeOfValue == nil {
-		typeOfValue, err = inferTypeFromValue(doc.Value, doc.GetPosition())
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	err = valueTypeAllowsItemValue(typeOfValue, doc.Value, doc.Position)
+	typeOfValue, err := getType(doc)
 	if err != nil {
 		return nil, err
 	}
 
-	defaultValue := typeOfValue.GetDefaultValue()
-
-	return &DocumentType{Source: doc, Position: doc.Position, ValueType: typeOfValue, defaultValue: defaultValue}, nil
+	return &DocumentType{Source: doc, Position: doc.Position, ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue()}, nil
 }
 
 func NewMapType(m *yamlmeta.Map) (*MapType, error) {
@@ -110,29 +93,12 @@ func NewMapType(m *yamlmeta.Map) (*MapType, error) {
 }
 
 func NewMapItemType(item *yamlmeta.MapItem) (*MapItemType, error) {
-	var typeOfValue yamlmeta.Type
-
-	anns, err := collectAnnotations(item)
-	if err != nil {
-		return nil, NewSchemaError("Invalid schema", err)
-	}
-	typeOfValue = getTypeFromAnnotations(anns)
-
-	if typeOfValue == nil {
-		typeOfValue, err = inferTypeFromValue(item.Value, item.GetPosition())
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	err = valueTypeAllowsItemValue(typeOfValue, item.Value, item.Position)
+	typeOfValue, err := getType(item)
 	if err != nil {
 		return nil, err
 	}
 
-	defaultValue := typeOfValue.GetDefaultValue()
-
-	return &MapItemType{Key: item.Key, ValueType: typeOfValue, defaultValue: defaultValue, Position: item.Position}, nil
+	return &MapItemType{Key: item.Key, ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue(), Position: item.Position}, nil
 }
 
 func NewArrayType(a *yamlmeta.Array) (*ArrayType, error) {
@@ -154,35 +120,36 @@ func NewArrayType(a *yamlmeta.Array) (*ArrayType, error) {
 }
 
 func NewArrayItemType(item *yamlmeta.ArrayItem) (*ArrayItemType, error) {
+	typeOfValue, err := getType(item)
+	if err != nil {
+		return nil, err
+	}
+
+	return &ArrayItemType{ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue(), Position: item.GetPosition()}, nil
+}
+
+func getType(node yamlmeta.ValueHoldingNode) (yamlmeta.Type, error) {
 	var typeOfValue yamlmeta.Type
 
-	anns, err := collectAnnotations(item)
+	anns, err := collectAnnotations(node)
 	if err != nil {
-		return nil, err
+		return nil, NewSchemaError("Invalid schema", err)
 	}
 	typeOfValue = getTypeFromAnnotations(anns)
 
 	if typeOfValue == nil {
-		typeOfValue, err = inferTypeFromValue(item.Value, item.Position)
+		typeOfValue, err = inferTypeFromValue(node.Val(), node.GetPosition())
 		if err != nil {
 			return nil, err
 		}
-	} else {
-		if _, ok := typeOfValue.(*NullType); ok {
-			return nil, NewSchemaError("Invalid schema - @schema/nullable is not supported on array items", schemaAssertionError{
-				position: item.Position,
-				expected: "a valid annotation",
-				found:    fmt.Sprintf("@%v", AnnotationNullable),
-				hints:    []string{"Remove the @schema/nullable annotation from array item"},
-			})
-		}
 	}
-	err = valueTypeAllowsItemValue(typeOfValue, item.Value, item.Position)
+
+	err = valueTypeAllowsItemValue(typeOfValue, node.Val(), node.GetPosition())
 	if err != nil {
 		return nil, err
 	}
 
-	return &ArrayItemType{ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue(), Position: item.Position}, nil
+	return typeOfValue, nil
 }
 
 func inferTypeFromValue(value interface{}, position *filepos.Position) (yamlmeta.Type, error) {

pkg/yamlmeta/ast.go

@@ -30,8 +30,13 @@ type Node interface {
 	sealed() // limit the concrete types of Node to map directly only to types allowed in YAML spec.
 }
 
-// Ensure: all types are — in fact — assignable to Node
-var _ = []Node{&DocumentSet{}, &Document{}, &Map{}, &MapItem{}, &Array{}, &ArrayItem{}}
+type ValueHoldingNode interface {
+	Node
+	Val() interface{}
+}
+
+var _ = []Node{&DocumentSet{}, &Map{}, &Array{}}
+var _ = []ValueHoldingNode{&Document{}, &MapItem{}, &ArrayItem{}}
 
 type DocumentSet struct {
 	Metas    []*Meta

pkg/yamlmeta/value_holding_node.go

@@ -0,0 +1,14 @@
+// Copyright 2020 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package yamlmeta
+
+func (d *Document) Val() interface{} {
+	return d.Value
+}
+func (mi *MapItem) Val() interface{} {
+	return mi.Value
+}
+func (ai *ArrayItem) Val() interface{} {
+	return ai.Value
+}