From b0f4a37b267d54718271a03d6a922090ef47bef5 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Wed, 7 Jul 2021 23:16:16 -0700 Subject: [PATCH 1/3] Allow array items to be annotated nullable --- pkg/cmd/template/schema_test.go | 100 ++++++++++++++++++++++---------- pkg/schema/schema.go | 9 --- 2 files changed, 68 insertions(+), 41 deletions(-) diff --git a/pkg/cmd/template/schema_test.go b/pkg/cmd/template/schema_test.go index edbf01a1..116d1203 100644 --- a/pkg/cmd/template/schema_test.go +++ b/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 --- diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 362a0d4a..7ef20aca 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -167,15 +167,6 @@ func NewArrayItemType(item *yamlmeta.ArrayItem) (*ArrayItemType, error) { 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) if err != nil { From 2dfc0d56e00d98ea2999f5c968ea7e065dde8503 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Wed, 7 Jul 2021 23:32:26 -0700 Subject: [PATCH 2/3] Extract function to derive a type from a node --- pkg/schema/schema.go | 64 +++++++++++++++----------------------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 7ef20aca..0721e7ee 100644 --- a/pkg/schema/schema.go +++ b/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 := getTypeFromValueHoldingNode(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 := getTypeFromValueHoldingNode(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,26 +120,38 @@ func NewArrayType(a *yamlmeta.Array) (*ArrayType, error) { } func NewArrayItemType(item *yamlmeta.ArrayItem) (*ArrayItemType, error) { + typeOfValue, err := getTypeFromValueHoldingNode(item) + if err != nil { + return nil, err + } + + return &ArrayItemType{ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue(), Position: item.GetPosition()}, nil +} + +// getTypeFromValueHoldingNode derives the yamlmeta.Type from the given `node`. +// a "value-holding node" is a node that holds an actual value: Document, ArrayItem, or MapItem +func getTypeFromValueHoldingNode(node yamlmeta.Node) (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.GetValues()[0], node.GetPosition()) if err != nil { return nil, err } } - err = valueTypeAllowsItemValue(typeOfValue, item.Value, item.Position) + + err = valueTypeAllowsItemValue(typeOfValue, node.GetValues()[0], 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) { From 4dc320c55f50ded0d09540f3eef1ecdce76aaef4 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Thu, 8 Jul 2021 12:12:30 -0700 Subject: [PATCH 3/3] Introduce ValidHoldingNode ... for algorithms that are only applicable to Nodes that hold exactly one value: documents, array items, and map items. --- pkg/schema/schema.go | 14 ++++++-------- pkg/yamlmeta/ast.go | 9 +++++++-- pkg/yamlmeta/value_holding_node.go | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 pkg/yamlmeta/value_holding_node.go diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 0721e7ee..338dcdd6 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -70,7 +70,7 @@ func NewNullSchema() *DocumentSchema { } func NewDocumentType(doc *yamlmeta.Document) (*DocumentType, error) { - typeOfValue, err := getTypeFromValueHoldingNode(doc) + typeOfValue, err := getType(doc) if err != nil { return nil, err } @@ -93,7 +93,7 @@ func NewMapType(m *yamlmeta.Map) (*MapType, error) { } func NewMapItemType(item *yamlmeta.MapItem) (*MapItemType, error) { - typeOfValue, err := getTypeFromValueHoldingNode(item) + typeOfValue, err := getType(item) if err != nil { return nil, err } @@ -120,7 +120,7 @@ func NewArrayType(a *yamlmeta.Array) (*ArrayType, error) { } func NewArrayItemType(item *yamlmeta.ArrayItem) (*ArrayItemType, error) { - typeOfValue, err := getTypeFromValueHoldingNode(item) + typeOfValue, err := getType(item) if err != nil { return nil, err } @@ -128,9 +128,7 @@ func NewArrayItemType(item *yamlmeta.ArrayItem) (*ArrayItemType, error) { return &ArrayItemType{ValueType: typeOfValue, defaultValue: typeOfValue.GetDefaultValue(), Position: item.GetPosition()}, nil } -// getTypeFromValueHoldingNode derives the yamlmeta.Type from the given `node`. -// a "value-holding node" is a node that holds an actual value: Document, ArrayItem, or MapItem -func getTypeFromValueHoldingNode(node yamlmeta.Node) (yamlmeta.Type, error) { +func getType(node yamlmeta.ValueHoldingNode) (yamlmeta.Type, error) { var typeOfValue yamlmeta.Type anns, err := collectAnnotations(node) @@ -140,13 +138,13 @@ func getTypeFromValueHoldingNode(node yamlmeta.Node) (yamlmeta.Type, error) { typeOfValue = getTypeFromAnnotations(anns) if typeOfValue == nil { - typeOfValue, err = inferTypeFromValue(node.GetValues()[0], node.GetPosition()) + typeOfValue, err = inferTypeFromValue(node.Val(), node.GetPosition()) if err != nil { return nil, err } } - err = valueTypeAllowsItemValue(typeOfValue, node.GetValues()[0], node.GetPosition()) + err = valueTypeAllowsItemValue(typeOfValue, node.Val(), node.GetPosition()) if err != nil { return nil, err } diff --git a/pkg/yamlmeta/ast.go b/pkg/yamlmeta/ast.go index 60f464a4..a8371b8e 100644 --- a/pkg/yamlmeta/ast.go +++ b/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 diff --git a/pkg/yamlmeta/value_holding_node.go b/pkg/yamlmeta/value_holding_node.go new file mode 100644 index 00000000..760fb2c2 --- /dev/null +++ b/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 +}