visitor: more strict object parsing (#4346)

internal/cli/visitor/decode.go

@@ -1,8 +1,9 @@
 package visitor
 
 import (
-	"bufio"
 	"bytes"
+	"encoding/json"
+	"fmt"
 	"io"
 
 	"github.com/pkg/errors"
@@ -40,32 +41,42 @@ func Decode(scheme *runtime.Scheme, v Interface) ([]runtime.Object, error) {
 	return result, nil
 }
 
+// Parses a stream of Tilt configuration objects.
+//
+// In kubectl, the CLI has to get the type information from the server in order
+// to perform validation. In Tilt (today), we don't have to worry about version skew,
+// so we can more aggressively validate up-front for misspelled fields
+// and malformed YAML. So this parser is a bit stricter than the normal kubectl code.
 func ParseStream(scheme *runtime.Scheme, r io.Reader) ([]runtime.Object, error) {
-	var current bytes.Buffer
-	reader := io.TeeReader(bufio.NewReader(r), &current)
-
-	objDecoder := yaml.NewYAMLOrJSONDecoder(&current, 4096)
-	typeDecoder := yaml.NewYAMLOrJSONDecoder(reader, 4096)
+	decoder := yaml.NewYAMLOrJSONDecoder(r, 4096)
 	result := []runtime.Object{}
 	for {
-		tm := metav1.TypeMeta{}
-		if err := typeDecoder.Decode(&tm); err != nil {
+		msg := json.RawMessage{} // First convert into json bytes
+		if err := decoder.Decode(&msg); err != nil {
 			if err == io.EOF {
 				break
 			}
 			return nil, err
 		}
 
+		// Then decode into the type.
+		tm := metav1.TypeMeta{}
+		err := json.Unmarshal([]byte(msg), &tm)
+		if err != nil {
+			return nil, err
+		}
+
+		// Turn the type name into a native go object.
 		obj, err := scheme.New(tm.GroupVersionKind())
 		if err != nil {
 			return nil, err
 		}
 
-		if err := objDecoder.Decode(obj); err != nil {
-			if err == io.EOF {
-				break
-			}
-			return nil, errors.Wrapf(err, "decoding %s", tm)
+		// Then decode the object into its native go object
+		objDecoder := json.NewDecoder(bytes.NewBuffer([]byte(msg)))
+		objDecoder.DisallowUnknownFields()
+		if err := objDecoder.Decode(&obj); err != nil {
+			return nil, fmt.Errorf("decoding %s: %v\nOriginal object:\n%s", tm, err, string([]byte(msg)))
 		}
 
 		result = append(result, obj)

internal/cli/visitor/decode_test.go

@@ -0,0 +1,56 @@
+package visitor_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/tilt-dev/tilt/internal/cli/visitor"
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
+)
+
+func TestDecode(t *testing.T) {
+	cmds, err := decode(t, `
+apiVersion: tilt.dev/v1alpha1
+kind: Cmd
+metadata:
+  name: hello-world
+spec:
+  args: ["echo", "hello world"]
+---
+apiVersion: tilt.dev/v1alpha1
+kind: Cmd
+metadata:
+  name: goodbye-world
+spec:
+  args: ["echo", "goodbye world"]
+`)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(cmds))
+	assert.Equal(t, "hello-world", cmds[0].(*v1alpha1.Cmd).Name)
+	assert.Equal(t, "goodbye-world", cmds[1].(*v1alpha1.Cmd).Name)
+}
+
+func TestDecodeMisspelledField(t *testing.T) {
+	_, err := decode(t, `
+apiVersion: tilt.dev/v1alpha1
+kind: Cmd
+metadata:
+  name: hello-world
+spec:
+  misspell: ["echo", "hello world"]
+`)
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), `unknown field "misspell"`)
+		assert.Contains(t, err.Error(), `hello world`)
+	}
+}
+
+func decode(t *testing.T, s string) ([]runtime.Object, error) {
+	visitors, err := visitor.FromStrings([]string{"-"}, strings.NewReader(s))
+	require.NoError(t, err)
+	return visitor.DecodeAll(v1alpha1.NewScheme(), visitors)
+}