From 6720b95fb89dc7ddc66790b4cd3d6d154b9401aa Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Tue, 7 Apr 2020 11:59:27 +0200 Subject: [PATCH] fix(core): better error messages for invalid YAML When YAML parsing fails, emit more useful error messages by passing the failed file contents through a YAML validator. This will e.g. identify duplicate keys and other syntax errors. --- garden-service/package-lock.json | 212 ++++++++++++++++++ garden-service/package.json | 3 +- garden-service/src/commands/migrate.ts | 17 +- garden-service/src/config/base.ts | 34 ++- .../invalid-syntax-module/garden.yml | 1 + garden-service/test/unit/src/config/base.ts | 1 + 6 files changed, 248 insertions(+), 20 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index a6ede24fe3..d66b20ab0b 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -7336,6 +7336,11 @@ } } }, + "invert-kv": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/invert-kv/-/invert-kv-1.0.0.tgz", + "integrity": "sha1-EEqOSqym09jNFXqO+L+rLXo//bY=" + }, "ip": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/ip/-/ip-1.1.5.tgz", @@ -8239,6 +8244,14 @@ "readable-stream": "^2.0.5" } }, + "lcid": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/lcid/-/lcid-1.0.0.tgz", + "integrity": "sha1-MIrMr6C8SDo4Z7S28rlQYlHRuDU=", + "requires": { + "invert-kv": "^1.0.0" + } + }, "lead": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/lead/-/lead-1.0.0.tgz", @@ -8248,6 +8261,72 @@ "flush-write-stream": "^1.0.2" } }, + "leprechaun": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/leprechaun/-/leprechaun-0.0.2.tgz", + "integrity": "sha1-i5ZRSp5jTFP75ZqAlPM3jI+yCE0=", + "requires": { + "log-symbols": "^1.0.2" + }, + "dependencies": { + "ansi-regex": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz", + "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=" + }, + "ansi-styles": { + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-2.2.1.tgz", + "integrity": "sha1-tDLdM1i2NM914eRmQ2gkBTPB3b4=" + }, + "chalk": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", + "requires": { + "ansi-styles": "^2.2.1", + "escape-string-regexp": "^1.0.2", + "has-ansi": "^2.0.0", + "strip-ansi": "^3.0.0", + "supports-color": "^2.0.0" + } + }, + "escape-string-regexp": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz", + "integrity": "sha1-G2HAViGQqN/2rjuyzwIAyhMLhtQ=" + }, + "has-ansi": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-2.0.0.tgz", + "integrity": "sha1-NPUEnOHs3ysGSa8+8k5F7TVBbZE=", + "requires": { + "ansi-regex": "^2.0.0" + } + }, + "log-symbols": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-1.0.2.tgz", + "integrity": "sha1-N2/3tY6jCGoPCfrMdGF+ylAeGhg=", + "requires": { + "chalk": "^1.0.0" + } + }, + "strip-ansi": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", + "requires": { + "ansi-regex": "^2.0.0" + } + }, + "supports-color": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", + "integrity": "sha1-U10EXOa2Nj+kARcIRimZXp3zJMc=" + } + } + }, "levn": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/levn/-/levn-0.3.0.tgz", @@ -8372,6 +8451,16 @@ "resolved": "https://registry.npmjs.org/lodash.isstring/-/lodash.isstring-4.0.1.tgz", "integrity": "sha1-1SfftUVuynzJu5XV2ur4i6VKVFE=" }, + "lodash.merge": { + "version": "4.6.2", + "resolved": "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.2.tgz", + "integrity": "sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==" + }, + "lodash.snakecase": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz", + "integrity": "sha1-OdcUo1NXFHg3rv1ktdy7Fr7Nj40=" + }, "lodash.template": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.template/-/lodash.template-4.5.0.tgz", @@ -9103,6 +9192,98 @@ "to-regex": "^3.0.1" } }, + "nconf": { + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/nconf/-/nconf-0.10.0.tgz", + "integrity": "sha512-fKiXMQrpP7CYWJQzKkPPx9hPgmq+YLDyxcG9N8RpiE9FoCkCbzD0NyW0YhE3xn3Aupe7nnDeIx4PFzYehpHT9Q==", + "requires": { + "async": "^1.4.0", + "ini": "^1.3.0", + "secure-keys": "^1.0.0", + "yargs": "^3.19.0" + }, + "dependencies": { + "ansi-regex": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz", + "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=" + }, + "async": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz", + "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=" + }, + "camelcase": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-2.1.1.tgz", + "integrity": "sha1-fB0W1nmhu+WcoCys7PsBHiAfWh8=" + }, + "cliui": { + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-3.2.0.tgz", + "integrity": "sha1-EgYBU3qRbSmUD5NNo7SNWFo5IT0=", + "requires": { + "string-width": "^1.0.1", + "strip-ansi": "^3.0.1", + "wrap-ansi": "^2.0.0" + } + }, + "is-fullwidth-code-point": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz", + "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", + "requires": { + "number-is-nan": "^1.0.0" + } + }, + "string-width": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", + "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", + "requires": { + "code-point-at": "^1.0.0", + "is-fullwidth-code-point": "^1.0.0", + "strip-ansi": "^3.0.0" + } + }, + "strip-ansi": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", + "requires": { + "ansi-regex": "^2.0.0" + } + }, + "wrap-ansi": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", + "requires": { + "string-width": "^1.0.1", + "strip-ansi": "^3.0.1" + } + }, + "y18n": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-3.2.1.tgz", + "integrity": "sha1-bRX7qITAhnnA136I53WegR4H+kE=" + }, + "yargs": { + "version": "3.32.0", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-3.32.0.tgz", + "integrity": "sha1-AwiOnr+edWtpdRYR0qXvWRSCyZU=", + "requires": { + "camelcase": "^2.0.1", + "cliui": "^3.0.3", + "decamelize": "^1.1.1", + "os-locale": "^1.4.0", + "string-width": "^1.0.1", + "window-size": "^0.1.4", + "y18n": "^3.2.0" + } + } + } + }, "needle": { "version": "2.4.0", "resolved": "https://registry.npmjs.org/needle/-/needle-2.4.0.tgz", @@ -9869,6 +10050,14 @@ "resolved": "https://registry.npmjs.org/os-homedir/-/os-homedir-1.0.2.tgz", "integrity": "sha1-/7xJiDNuDoM94MFox+8VISGqf7M=" }, + "os-locale": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-1.4.0.tgz", + "integrity": "sha1-IPnxeuKe00XoveWDsT0gCYA8FNk=", + "requires": { + "lcid": "^1.0.0" + } + }, "os-tmpdir": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/os-tmpdir/-/os-tmpdir-1.0.2.tgz", @@ -11242,6 +11431,11 @@ } } }, + "secure-keys": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/secure-keys/-/secure-keys-1.0.0.tgz", + "integrity": "sha1-8MgtmKOxOah3aogIBQuCRDEIf8o=" + }, "semver": { "version": "5.5.1", "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.1.tgz", @@ -13097,6 +13291,11 @@ } } }, + "window-size": { + "version": "0.1.4", + "resolved": "https://registry.npmjs.org/window-size/-/window-size-0.1.4.tgz", + "integrity": "sha1-+OGqHuWlPsW/FR/6CXQqatdpeHY=" + }, "winston": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/winston/-/winston-3.2.1.tgz", @@ -13239,6 +13438,19 @@ "resolved": "https://registry.npmjs.org/yallist/-/yallist-2.1.2.tgz", "integrity": "sha1-HBH5IY8HYImkfdUS+TxmmaaoHVI=" }, + "yaml-lint": { + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/yaml-lint/-/yaml-lint-1.2.4.tgz", + "integrity": "sha512-qpKE0szyKsE9TrlVPi+bxKxVAjl30QjNAOyOxy7noQdf/WCCYUlT4xiCRxMG48eyeBzMBtBN6PgGfaB0MJePNw==", + "requires": { + "glob": "^7.1.2", + "js-yaml": "^3.10.0", + "leprechaun": "0.0.2", + "lodash.merge": "^4.6.1", + "lodash.snakecase": "^4.1.1", + "nconf": "^0.10.0" + } + }, "yargonaut": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/yargonaut/-/yargonaut-1.1.4.tgz", diff --git a/garden-service/package.json b/garden-service/package.json index 93c61a4e34..2b5d9b7e21 100644 --- a/garden-service/package.json +++ b/garden-service/package.json @@ -131,7 +131,8 @@ "winston": "^3.2.1", "wrap-ansi": "^6.2.0", "ws": "^7.2.1", - "xml-js": "^1.6.11" + "xml-js": "^1.6.11", + "yaml-lint": "^1.2.4" }, "devDependencies": { "@commitlint/cli": "^8.3.5", diff --git a/garden-service/src/commands/migrate.ts b/garden-service/src/commands/migrate.ts index d6b3e48c21..da66cba794 100644 --- a/garden-service/src/commands/migrate.ts +++ b/garden-service/src/commands/migrate.ts @@ -7,18 +7,19 @@ */ import { Command, CommandParams, CommandResult, BooleanParameter, StringsParameter } from "./base" -import yaml, { safeDump } from "js-yaml" +import { safeDump } from "js-yaml" import { dedent } from "../util/string" import { readFile, writeFile } from "fs-extra" import { cloneDeep, isEqual } from "lodash" import { ConfigurationError, RuntimeError } from "../exceptions" -import { basename, resolve, parse } from "path" +import { resolve, parse } from "path" import { findConfigPathsInPath, getConfigFilePath } from "../util/fs" import { GitHandler } from "../vcs/git" import { DEFAULT_GARDEN_DIR_NAME } from "../constants" import { exec } from "../util/util" import { LoggerType } from "../logger/logger" import Bluebird from "bluebird" +import { loadAndValidateYaml } from "../config/base" const migrateOptions = { write: new BooleanParameter({ help: "Update the `garden.yml` in place." }), @@ -196,17 +197,9 @@ async function findRoot(path: string): Promise { * Read the contents of a YAML file and dump to JSON */ async function readYaml(path: string) { - let rawSpecs: any[] const fileData = await readFile(path) - - try { - rawSpecs = yaml.safeLoadAll(fileData.toString()) || [] - } catch (err) { - throw new ConfigurationError(`Could not parse ${basename(path)} in directory ${path} as valid YAML`, err) - } - - // Ignore empty resources - return rawSpecs.filter(Boolean) + const rawSpecs = await loadAndValidateYaml(fileData.toString(), path) + return rawSpecs.filter(Boolean) // Ignore empty resources } /** diff --git a/garden-service/src/config/base.ts b/garden-service/src/config/base.ts index 09a21499e3..32c08709c5 100644 --- a/garden-service/src/config/base.ts +++ b/garden-service/src/config/base.ts @@ -8,6 +8,7 @@ import { sep, resolve, relative, basename, dirname } from "path" import yaml from "js-yaml" +import yamlLint from "yaml-lint" import { readFile } from "fs-extra" import { omit, isPlainObject, find, isArray } from "lodash" import { ModuleResource, coreModuleSpecSchema, baseModuleSchemaKeys, BuildDependencyConfig } from "./module" @@ -24,11 +25,34 @@ export interface GardenResource { path: string } +/** + * Attempts to parse content as YAML, and applies a linter to produce more informative error messages when + * content is not valid YAML. + * + * @param content - The contents of the file as a string. + * @param path - The path to the file. + */ +export async function loadAndValidateYaml(content: string, path: string): Promise { + try { + return yaml.safeLoadAll(content) || [] + } catch (err) { + // We try to find the error using a YAML linter + try { + await yamlLint(content) + } catch (linterErr) { + throw new ConfigurationError( + `Could not parse ${basename(path)} in directory ${path} as valid YAML: ${err.message}`, + linterErr + ) + } + // ... but default to throwing a generic error, in case the error wasn't caught by yaml-lint. + throw new ConfigurationError(`Could not parse ${basename(path)} in directory ${path} as valid YAML.`, err) + } +} + export async function loadConfig(projectRoot: string, path: string): Promise { - // TODO: nicer error messages when load/validation fails const configPath = await getConfigFilePath(path) let fileData: Buffer - let rawSpecs: any[] // loadConfig returns undefined if config file is not found in the given directory try { @@ -37,11 +61,7 @@ export async function loadConfig(projectRoot: string, path: string): Promise { async () => await loadConfig(projectPath, resolve(projectPath, "invalid-syntax-module")), (err) => { expect(err.message).to.match(/Could not parse/) + expect(err.message).to.match(/duplicated mapping key/) // include syntax erorrs in the output } ) })