Skip to content

Commit

Permalink
fix(core): skip invalid yaml by default
Browse files Browse the repository at this point in the history
We'd sometimes get errors when trying to dump JSON that includes the
value `undefined` since that's not a part of the formal YAML spec. We
address this by setting `skipInvalid: true` by default when dumping
YAML. This means other invalid values such as functions will also be
excluded.
  • Loading branch information
eysi09 committed Apr 9, 2020
1 parent 6804733 commit 5a44da5
Show file tree
Hide file tree
Showing 17 changed files with 84 additions and 55 deletions.
5 changes: 2 additions & 3 deletions garden-service/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import sywac from "sywac"
import chalk from "chalk"
import { intersection, merge, sortBy } from "lodash"
import { resolve, join } from "path"
import { safeDump } from "js-yaml"
import { coreCommands } from "../commands/commands"
import { DeepPrimitiveMap } from "../config/common"
import { shutdown, sleep, getPackageVersion, uuidv4 } from "../util/util"
import { shutdown, sleep, getPackageVersion, uuidv4, safeDumpYaml } from "../util/util"
import { deline } from "../util/string"
import {
BooleanParameter,
Expand Down Expand Up @@ -62,7 +61,7 @@ const OUTPUT_RENDERERS = {
return stringify(data, null, 2)
},
yaml: (data: DeepPrimitiveMap) => {
return safeDump(data, { noRefs: true, skipInvalid: true })
return safeDumpYaml(data, { noRefs: true })
},
}

Expand Down
5 changes: 2 additions & 3 deletions garden-service/src/commands/get/get-debug-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import yaml from "js-yaml"
import { Command, CommandParams, ChoicesParameter, BooleanParameter } from "../base"
import { findProjectConfig } from "../../config/base"
import { ensureDir, copy, remove, pathExists, writeFile } from "fs-extra"
import { getPackageVersion, exec } from "../../util/util"
import { getPackageVersion, exec, safeDumpYaml } from "../../util/util"
import { platform, release } from "os"
import { join, relative, basename, dirname } from "path"
import { LogEntry } from "../../logger/log-entry"
Expand Down Expand Up @@ -225,7 +224,7 @@ function renderInfo(info: any, format: string) {
if (format === "json") {
return JSON.stringify(info, null, 4)
} else {
return yaml.safeDump(info, { noRefs: true, skipInvalid: true })
return safeDumpYaml(info, { noRefs: true })
}
}

Expand Down
5 changes: 2 additions & 3 deletions garden-service/src/commands/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import { Command, CommandParams, CommandResult, BooleanParameter, StringsParameter } from "./base"
import { safeDump } from "js-yaml"
import { dedent } from "../util/string"
import { readFile, writeFile } from "fs-extra"
import { cloneDeep, isEqual } from "lodash"
Expand All @@ -16,7 +15,7 @@ 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 { exec, safeDumpYaml } from "../util/util"
import { LoggerType } from "../logger/logger"
import Bluebird from "bluebird"
import { loadAndValidateYaml } from "../config/base"
Expand Down Expand Up @@ -167,7 +166,7 @@ export class MigrateCommand extends Command<Args, Opts> {
* Dump JSON specs to YAML. Join specs by `---`.
*/
export function dumpSpec(specs: any[]) {
return specs.map((spec) => safeDump(spec)).join("\n---\n\n")
return specs.map((spec) => safeDumpYaml(spec)).join("\n---\n\n")
}

/**
Expand Down
7 changes: 3 additions & 4 deletions garden-service/src/commands/run/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { safeDump } from "js-yaml"
import { RuntimeContext } from "../../runtime-context"
import { highlightYaml } from "../../util/util"
import { highlightYaml, safeDumpYaml } from "../../util/util"
import { Command } from "../base"
import { RunModuleCommand } from "./module"
import { RunServiceCommand } from "./service"
Expand All @@ -30,8 +29,8 @@ export class RunCommand extends Command {
export function printRuntimeContext(log: LogEntry, runtimeContext: RuntimeContext) {
log.verbose("-----------------------------------\n")
log.verbose("Environment variables:")
log.verbose(highlightYaml(safeDump(runtimeContext.envVars)))
log.verbose(highlightYaml(safeDumpYaml(runtimeContext.envVars)))
log.verbose("Dependencies:")
log.verbose(highlightYaml(safeDump(runtimeContext.dependencies)))
log.verbose(highlightYaml(safeDumpYaml(runtimeContext.dependencies)))
log.verbose("-----------------------------------\n")
}
6 changes: 2 additions & 4 deletions garden-service/src/commands/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { safeDump } from "js-yaml"
import { DeepPrimitiveMap } from "../config/common"
import { highlightYaml } from "../util/util"
import { highlightYaml, safeDumpYaml } from "../util/util"
import { Command, CommandParams, CommandResult } from "./base"
import { omit } from "lodash"

Expand All @@ -33,9 +32,8 @@ export class ScanCommand extends Command {

log.info(
highlightYaml(
safeDump(shortOutput, {
safeDumpYaml(shortOutput, {
noRefs: true,
skipInvalid: true,
sortKeys: true,
})
)
Expand Down
6 changes: 3 additions & 3 deletions garden-service/src/docs/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import Joi from "@hapi/joi"
import { readFileSync } from "fs"
import { safeDump } from "js-yaml"
import linewrap from "linewrap"
import { resolve } from "path"
import { projectDocsSchema } from "../config/project"
Expand All @@ -19,6 +18,7 @@ import { joi } from "../config/common"
import { STATIC_DIR } from "../constants"
import { indent, renderMarkdownTable, convertMarkdownLinks, NormalizedSchemaDescription } from "./common"
import { normalizeJoiSchemaDescription, JoiDescription } from "./joi-schema"
import { safeDumpYaml } from "../util/util"

export const TEMPLATES_DIR = resolve(STATIC_DIR, "docs", "templates")
const partialTemplatePath = resolve(TEMPLATES_DIR, "config-partial.hbs")
Expand Down Expand Up @@ -246,10 +246,10 @@ export function renderSchemaDescriptionYaml(
if (value === undefined) {
formattedValue = ""
} else if (isPrimitive || exceptionallyTreatAsPrimitive) {
formattedValue = safeDump(value)
formattedValue = safeDumpYaml(value)
} else {
formattedValue = indent(
safeDump(value)
safeDumpYaml(value)
.trim()
.split("\n"),
1
Expand Down
5 changes: 2 additions & 3 deletions garden-service/src/docs/joi-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
*/

import Joi from "@hapi/joi"
import { safeDump } from "js-yaml"
import { flatten, uniq, isFunction, extend } from "lodash"
import { NormalizedSchemaDescription, NormalizeOptions } from "./common"
import { findByName } from "../util/util"
import { findByName, safeDumpYaml } from "../util/util"
import { normalizeJsonSchema } from "./json-schema"

// Need this to fix the Joi typing
Expand Down Expand Up @@ -129,7 +128,7 @@ function normalizeJoiKeyDescription(schemaDescription: JoiDescription): Normaliz
if (schemaDescription.examples && schemaDescription.examples.length) {
const example = schemaDescription.examples[0]
if (schemaDescription.type === "object" || schemaDescription.type === "array") {
formattedExample = safeDump(example).trim()
formattedExample = safeDumpYaml(example).trim()
} else {
formattedExample = JSON.stringify(example)
}
Expand Down
4 changes: 2 additions & 2 deletions garden-service/src/docs/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { safeDump } from "js-yaml"
import { flatten, isArray } from "lodash"
import { NormalizedSchemaDescription, NormalizeOptions } from "./common"
import { ValidationError } from "../exceptions"
import { safeDumpYaml } from "../util/util"

/**
* Takes a JSON Schema and translates to a list of NormalizedKeyDescription objects.
Expand Down Expand Up @@ -85,7 +85,7 @@ function normalizeJsonKeyDescription(
if (schema.examples && schema.examples.length > 0) {
const example = schema.examples[0]
if (type === "object" || type === "array") {
formattedExample = safeDump(example).trim()
formattedExample = safeDumpYaml(example).trim()
} else {
formattedExample = JSON.stringify(example)
}
Expand Down
7 changes: 3 additions & 4 deletions garden-service/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import logSymbols from "log-symbols"
import yaml from "js-yaml"
import chalk from "chalk"
import stripAnsi from "strip-ansi"
import { isArray, isEmpty, repeat } from "lodash"
Expand All @@ -17,7 +16,7 @@ import hasAnsi = require("has-ansi")

import { LogEntry, MessageState } from "./log-entry"
import { JsonLogEntry } from "./writers/json-terminal-writer"
import { highlightYaml, deepFilter, PickFromUnion } from "../util/util"
import { highlightYaml, deepFilter, PickFromUnion, safeDumpYaml } from "../util/util"
import { isNumber } from "util"
import { printEmoji, sanitizeObject } from "./util"
import { LoggerType, Logger } from "./logger"
Expand Down Expand Up @@ -98,7 +97,7 @@ export function renderError(entry: LogEntry) {
if (!isEmpty(filteredDetail)) {
try {
const sanitized = sanitizeObject(filteredDetail)
const yamlDetail = yaml.safeDump(sanitized, { noRefs: true, skipInvalid: true })
const yamlDetail = safeDumpYaml(sanitized, { noRefs: true })
out += `\nError Details:\n${yamlDetail}`
} catch (err) {
out += `\nUnable to render error details:\n${err.message}`
Expand Down Expand Up @@ -157,7 +156,7 @@ export function renderData(entry: LogEntry): string {
return ""
}
if (!dataFormat || dataFormat === "yaml") {
const asYaml = yaml.safeDump(data, { noRefs: true, skipInvalid: true })
const asYaml = safeDumpYaml(data, { noRefs: true })
return highlightYaml(asYaml)
}
return JSON.stringify(data, null, 2)
Expand Down
6 changes: 3 additions & 3 deletions garden-service/src/plugins/kubernetes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import {
import AsyncLock = require("async-lock")
import request = require("request-promise")
import requestErrors = require("request-promise/errors")
import { safeLoad, safeDump } from "js-yaml"
import { safeLoad } from "js-yaml"
import { readFile } from "fs-extra"

import { Omit } from "../../util/util"
import { Omit, safeDumpYaml } from "../../util/util"
import { omitBy, isObject, isPlainObject, keyBy } from "lodash"
import { GardenBaseError, RuntimeError, ConfigurationError } from "../../exceptions"
import { KubernetesResource, KubernetesServerResource, KubernetesServerList } from "./types"
Expand Down Expand Up @@ -505,7 +505,7 @@ async function getContextConfig(log: LogEntry, provider: KubernetesProvider): Pr
const kc = new KubeConfig()

// There doesn't appear to be a method to just load the parsed config :/
kc.loadFromString(safeDump(rawConfig))
kc.loadFromString(safeDumpYaml(rawConfig))
kc.setCurrentContext(context)

cachedConfigs[cacheKey] = kc
Expand Down
6 changes: 3 additions & 3 deletions garden-service/src/plugins/kubernetes/helm/tiller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
import { LogEntry } from "../../../logger/log-entry"
import { KubernetesResource } from "../types"
import { helm, helmPlugin2to3 } from "./helm-cli"
import { safeLoadAll, safeDump } from "js-yaml"
import { safeLoadAll } from "js-yaml"
import { KubeApi, getKubeConfig } from "../api"
import { checkResourceStatuses } from "../status/status"
import { combineStates } from "../../../types/service"
import { KubernetesPluginContext } from "../config"
import { convertDeprecatedManifestVersion } from "../util"
import Bluebird from "bluebird"
import tmp from "tmp-promise"
import { findByName, getNames } from "../../../util/util"
import { findByName, getNames, safeDumpYaml } from "../../../util/util"
import { ConfigurationError } from "../../../exceptions"
import { writeFile } from "fs-extra"
import { resolve } from "path"
Expand Down Expand Up @@ -187,7 +187,7 @@ export async function migrateToHelm3({
}

const configPath = resolve(tmpDir.path, "kubeconfig.json")
await writeFile(configPath, safeDump(resolvedConfig))
await writeFile(configPath, safeDumpYaml(resolvedConfig))

log.debug(
// It's not possible to install/update/execute Helm plugins on Windows because of this:
Expand Down
5 changes: 2 additions & 3 deletions garden-service/src/task-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@

import Bluebird from "bluebird"
import chalk from "chalk"
import yaml from "js-yaml"
import { every, flatten, intersection, merge, union, uniqWith, without } from "lodash"
import { BaseTask, TaskDefinitionError, TaskType } from "./tasks/base"

import { LogEntry, LogEntryMetadata, TaskLogStatus } from "./logger/log-entry"
import { toGardenError, GardenBaseError } from "./exceptions"
import { Garden } from "./garden"
import { dedent } from "./util/string"
import { defer, relationshipClasses, uuidv4 } from "./util/util"
import { defer, relationshipClasses, uuidv4, safeDumpYaml } from "./util/util"
import { renderError } from "./logger/renderers"
import { cyclesToString } from "./util/validate-dependencies"
import { Profile } from "./util/profiling"
Expand Down Expand Up @@ -318,7 +317,7 @@ export class TaskGraph {
this.log.silly("")
this.log.silly("TaskGraph: this.index before processing")
this.log.silly("---------------------------------------")
this.log.silly(yaml.safeDump(this.index.inspect(), { noRefs: true, skipInvalid: true }))
this.log.silly(safeDumpYaml(this.index.inspect(), { noRefs: true }))

this.garden.events.emit("taskGraphProcessing", { startedAt: new Date() })
}
Expand Down
16 changes: 11 additions & 5 deletions garden-service/src/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import split2 = require("split2")
import Bluebird = require("bluebird")
import { ResolvableProps } from "bluebird"
import exitHook from "async-exit-hook"
import yaml from "js-yaml"
import Cryo from "cryo"
import _spawn from "cross-spawn"
import { readFile, writeFile } from "fs-extra"
Expand All @@ -20,7 +19,7 @@ import { TimeoutError, ParameterError, RuntimeError, GardenError } from "../exce
import { isArray, isPlainObject, extend, mapValues, pickBy, range, some } from "lodash"
import highlight from "cli-highlight"
import chalk from "chalk"
import { safeDump } from "js-yaml"
import { safeDump, safeLoad, DumpOptions } from "js-yaml"
import { createHash } from "crypto"
import { tailString, dedent } from "./string"
import { Writable } from "stream"
Expand Down Expand Up @@ -303,14 +302,21 @@ export function spawn(cmd: string, args: string[], opts: SpawnOpts = {}) {
}

export async function dumpYaml(yamlPath, data) {
return writeFile(yamlPath, yaml.safeDump(data, { noRefs: true }))
return writeFile(yamlPath, safeDumpYaml(data, { noRefs: true }))
}

/**
* Wraps safeDump and enforces that invalid values are skipped
*/
export function safeDumpYaml(data, opts: DumpOptions = {}) {
return safeDump(data, { ...opts, skipInvalid: true })
}

/**
* Encode multiple objects as one multi-doc YAML file
*/
export function encodeYamlMulti(objects: object[]) {
return objects.map((s) => safeDump(s, { noRefs: true }) + "---\n").join("")
return objects.map((s) => safeDumpYaml(s, { noRefs: true }) + "---\n").join("")
}

/**
Expand Down Expand Up @@ -457,7 +463,7 @@ export function highlightYaml(s: string) {

export async function loadYamlFile(path: string): Promise<any> {
const fileData = await readFile(path)
return yaml.safeLoad(fileData.toString())
return safeLoad(fileData.toString())
}

export interface ObjectWithName {
Expand Down
8 changes: 4 additions & 4 deletions garden-service/test/unit/src/commands/create/create-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { makeDummyGarden } from "../../../../../src/cli/cli"
import { Garden } from "../../../../../src/garden"
import { basename, join } from "path"
import { pathExists, readFile, writeFile, mkdirp } from "fs-extra"
import { safeLoadAll, safeDump } from "js-yaml"
import { exec } from "../../../../../src/util/util"
import { safeLoadAll } from "js-yaml"
import { exec, safeDumpYaml } from "../../../../../src/util/util"
import stripAnsi = require("strip-ansi")
import { getModuleTypes } from "../../../../../src/plugins"
import { supportedPlugins } from "../../../../../src/plugins/plugins"
Expand Down Expand Up @@ -92,7 +92,7 @@ describe("CreateModuleCommand", () => {
type: "foo",
name: "foo",
}
await writeFile(join(tmp.path, "garden.yml"), safeDump(existing))
await writeFile(join(tmp.path, "garden.yml"), safeDumpYaml(existing))

const { result } = await command.action({
garden,
Expand Down Expand Up @@ -122,7 +122,7 @@ describe("CreateModuleCommand", () => {
type: "exec",
}
const configPath = join(tmp.path, "garden.yml")
await writeFile(configPath, safeDump(existing))
await writeFile(configPath, safeDumpYaml(existing))

await expectError(
() =>
Expand Down
Loading

0 comments on commit 5a44da5

Please sign in to comment.