Improve the error message for bad YAML

internal/lagoonyml/lint.go

@@ -1,6 +1,8 @@
 package lagoonyml
 
 import (
+	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 
@@ -10,19 +12,34 @@ import (
 // Linter validates the given Lagoon struct.
 type Linter func(*Lagoon) error
 
-// LintFile takes a file path, reads it, and applies `.lagoon.yml` lint policy to
-// it. Lint returns an error of type ErrLint if it finds problems with the
-// file, a regular error if something else went wrong, and nil if the
-// `.lagoon.yml` is valid.
-func LintFile(path string, linters ...Linter) error {
+func lint(rawYAML []byte, linters []Linter) error {
 	var l Lagoon
-	rawYAML, err := os.ReadFile(path)
-	if err != nil {
-		return fmt.Errorf("couldn't read %v: %v", path, err)
-	}
-	err = yaml.Unmarshal(rawYAML, &l)
-	if err != nil {
-		return fmt.Errorf("couldn't unmarshal %v: %v", path, err)
+	yamlErr := yaml.Unmarshal(rawYAML, &l)
+	if yamlErr != nil {
+		// try to extract more detail from the Unmarshal error
+		rawJSON, err := yaml.YAMLToJSON(rawYAML)
+		if err != nil {
+			// can't even convert this so return the original error
+			return yamlErr
+		}
+		// json.Unmarshal returns richer errors than yaml.Unmarshal, which helps to
+		// diagnose exactly what went wrong
+		err = json.Unmarshal(rawJSON, &l)
+		var jTypeErr *json.UnmarshalTypeError
+		if errors.As(err, &jTypeErr) {
+			// this rawJSON slice will be partial JSON, but JSONToYAML ignores junk
+			// at the end of the snippet.
+			badYAML, err := yaml.JSONToYAML(rawJSON[jTypeErr.Offset:])
+			if err != nil {
+				// can't convert this snippet, so return the original error
+				return yamlErr
+			}
+			return fmt.Errorf(
+				"couldn't unmarshal YAML: %v.\nThere appears to be invalid YAML in the `%s` field:\n\n%s",
+				yamlErr, jTypeErr.Field, badYAML)
+		}
+		// this isn't a json.UnmarshalTypeError, so just return the original error
+		return yamlErr
 	}
 	for _, linter := range linters {
 		if err := linter(&l); err != nil {
@@ -34,21 +51,25 @@ func LintFile(path string, linters ...Linter) error {
 	return nil
 }
 
-// LintYAML takes a byte slice containing raw YAML and applies `.lagoon.yml` lint policy to
-// it. Lint returns an error of type ErrLint if it finds problems with the
-// file, a regular error if something else went wrong, and nil if the
+// LintFile takes a file path, reads it, and applies `.lagoon.yml` lint policy
+// to it. LintFile returns an error of type ErrLint if it finds problems with
+// the file, a regular error if something else went wrong, and nil if the
 // `.lagoon.yml` is valid.
-func LintYAML(rawYAML []byte, linters ...Linter) error {
-	var l Lagoon
-	if err := yaml.Unmarshal(rawYAML, &l); err != nil {
-		return fmt.Errorf("couldn't unmarshal YAML: %v", err)
+func LintFile(path string, linters ...Linter) error {
+	rawYAML, err := os.ReadFile(path)
+	if err != nil {
+		return fmt.Errorf("couldn't read %v: %v", path, err)
 	}
-	for _, linter := range linters {
-		if err := linter(&l); err != nil {
-			return &ErrLint{
-				Detail: err.Error(),
-			}
-		}
+	if err = lint(rawYAML, linters); err != nil {
+		return fmt.Errorf("couldn't validate: %v: %v", path, err)
 	}
 	return nil
 }
+
+// LintYAML takes a byte slice containing raw YAML and applies `.lagoon.yml`
+// lint policy to it. LintYAML returns an error of type ErrLint if it finds
+// problems with the YAML, a regular error if something else went wrong, and
+// nil if the YAML is valid.
+func LintYAML(rawYAML []byte, linters ...Linter) error {
+	return lint(rawYAML, linters)
+}

internal/lagoonyml/lint_test.go

@@ -55,6 +55,10 @@ func TestLint(t *testing.T) {
 			input: "testdata/invalid.3.lagoon.yml",
 			valid: false,
 		},
+		"cronjob as environment": {
+			input: "testdata/invalid.4.lagoon.yml",
+			valid: false,
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(tt *testing.T) {

internal/lagoonyml/testdata/invalid.4.lagoon.yml

@@ -0,0 +1,23 @@
+environments:
+  cronjobs:
+  - name: a cronjob defined as environment
+    schedule: "* * * * *"
+    command: echo "broken definition"
+    service: cli
+  main:
+    routes:
+    - nginx:
+      - example.com
+      - "www.example.com":
+          tls-acme: 'true'
+          insecure: Redirect
+          hsts: max-age=31536000
+      - "example.com":
+          annotations:
+            nginx.ingress.kubernetes.io/server-snippet: |
+              set_real_ip_from 1.2.3.4/32;
+      - "dev.example.com":
+          annotations:
+            nginx.ingress.kubernetes.io/server-snippet: |
+              set_real_ip_from 1.2.3.4/32;
+              add_header Content-type text/plain;