From 7659ebad679cfea74f260f976c5f7dc2ba148e18 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 1 Nov 2021 23:24:36 +0800 Subject: [PATCH 1/2] chore: add test case for malformed .lagoon.yml This one has a cronjob where an environment is expected. --- internal/lagoonyml/lint_test.go | 4 ++++ .../lagoonyml/testdata/invalid.4.lagoon.yml | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 internal/lagoonyml/testdata/invalid.4.lagoon.yml diff --git a/internal/lagoonyml/lint_test.go b/internal/lagoonyml/lint_test.go index 2c79eb6..812d6c9 100644 --- a/internal/lagoonyml/lint_test.go +++ b/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) { diff --git a/internal/lagoonyml/testdata/invalid.4.lagoon.yml b/internal/lagoonyml/testdata/invalid.4.lagoon.yml new file mode 100644 index 0000000..8a21007 --- /dev/null +++ b/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; From 8006af8c804fb169c739e28cdbd93d3a92943673 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 1 Nov 2021 23:25:28 +0800 Subject: [PATCH 2/2] feat: improve error diagnosis of lint issues --- internal/lagoonyml/lint.go | 71 ++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 25 deletions(-) diff --git a/internal/lagoonyml/lint.go b/internal/lagoonyml/lint.go index fde2604..c29201b 100644 --- a/internal/lagoonyml/lint.go +++ b/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) +}