Skip to content

Commit

Permalink
feat(config): better error messages around schema validation (#4889)
Browse files Browse the repository at this point in the history
* feat(config): better error messages around schema validation

This attaches the YAML parse results to configs when loading, and leverages
that when rendering validation error messages.

This should be a useful step for an IDE highlighting integration as well.

* fix: loosen YAML validation to match prior compatibility (TBS)

* test: add test for error in doc in multi-doc YAML (TBS)

* chore: switch from peg.js to peggy

* chore: change resolveTemplateStrings function signature (TBS)

* improvement(config): add yaml source mapping to template string resolution

* chore: fix missing dev dependency

* chore: fix linting warnings

* chore: add try/catch around YAML context annotation

* chore: fix errors after merge (TBS)

* chore: update `package-lock.json`

---------

Co-authored-by: Vladimir Vagaytsev <vladimir.vagaitsev@gmail.com>
  • Loading branch information
edvald and vvagaytsev committed Sep 26, 2023
1 parent 76e186f commit a098a14
Show file tree
Hide file tree
Showing 50 changed files with 2,050 additions and 1,169 deletions.
25 changes: 0 additions & 25 deletions core/gulpfile.js

This file was deleted.

9 changes: 4 additions & 5 deletions core/package.json
Expand Up @@ -244,16 +244,15 @@
"eslint-plugin-no-null": "^1.0.2",
"eslint-plugin-react": "^7.32.2",
"finalhandler": "^1.2.0",
"gulp": "^4.0.2",
"gulp-pegjs": "^0.2.0",
"is-subset": "^0.1.1",
"md5": "^2.3.0",
"mocha": "^10.2.0",
"nock": "^12.0.3",
"nodemon": "^2.0.15",
"node-fetch": "^2.7.0",
"nyc": "^15.1.0",
"p-event": "^4.2.0",
"pegjs": "^0.10.0",
"peggy": "^3.0.2",
"prettier": "3.0.0",
"ps-tree": "^1.2.0",
"replace-in-file": "^6.3.5",
Expand All @@ -268,11 +267,11 @@
"utility-types": "^3.10.0"
},
"scripts": {
"build": "gulp pegjs",
"build": "mkdir -p build/src/template-string && peggy src/template-string/parser.pegjs --output build/src/template-string/parser.js",
"check-package-lock": "git diff-index --quiet HEAD -- package-lock.json || (echo 'package-lock.json is dirty!' && exit 1)",
"check-types": "tsc -p . --noEmit",
"clean": "shx rm -rf build",
"dev": "gulp pegjs-watch",
"dev": "nodemon --watch src/template-string/*.pegjs --exec yarn build",
"fix-format": "prettier --write \"{src,test}/**/*.ts\"",
"lint": "eslint -c ../.eslintrc --ignore-pattern 'src/lib/**' --ext .ts src/ test/",
"migration:generate": "typeorm migration:generate --config ormconfig.js -n",
Expand Down
3 changes: 2 additions & 1 deletion core/src/actions/types.ts
Expand Up @@ -20,6 +20,7 @@ import type { BaseAction } from "./base"
import type { ValidResultType } from "../tasks/base"
import type { BaseGardenResource, GardenResourceInternalFields } from "../config/base"
import type { LinkedSource } from "../config-store/local"
import { GardenApiVersion } from "../constants"

// TODO: split this file

Expand Down Expand Up @@ -48,7 +49,7 @@ export interface BaseActionConfig<K extends ActionKind = ActionKind, T = string,
extends BaseGardenResource {
// Basics
// -> No templating is allowed on these.
apiVersion?: string
apiVersion?: GardenApiVersion
kind: K
type: T
name: string
Expand Down
6 changes: 3 additions & 3 deletions core/src/cli/cli.ts
Expand Up @@ -32,7 +32,7 @@ import {
cliStyles,
} from "./helpers"
import { ParameterObject, globalOptions, OUTPUT_RENDERERS, GlobalOptions, ParameterValues } from "./params"
import { ProjectResource } from "../config/project"
import { ProjectConfig } from "../config/project"
import { ERROR_LOG_FILENAME, DEFAULT_GARDEN_DIR_NAME, LOGS_DIR_NAME, gardenEnv } from "../constants"
import { generateBasicDebugInfoReport } from "../commands/get/get-debug-info"
import { AnalyticsHandler } from "../analytics/analytics"
Expand Down Expand Up @@ -428,7 +428,7 @@ ${renderCommands(commands)}
return done(1, chalk.red(`Could not find specified root path (${argv.root})`))
}

let projectConfig: ProjectResource | undefined
let projectConfig: ProjectConfig | undefined

// First look for native Garden commands
let { command, rest, matchedPath } = pickCommand(Object.values(this.commands), argv._)
Expand Down Expand Up @@ -595,7 +595,7 @@ ${renderCommands(commands)}
}

@pMemoizeDecorator()
async getProjectConfig(log: Log, workingDir: string): Promise<ProjectResource | undefined> {
async getProjectConfig(log: Log, workingDir: string): Promise<ProjectConfig | undefined> {
return findProjectConfig({ log, path: workingDir })
}

Expand Down
23 changes: 20 additions & 3 deletions core/src/commands/custom.ts
Expand Up @@ -121,9 +121,15 @@ export class CustomCommandWrapper extends Command {
// Strip the command name and any specified arguments off the $rest variable
const rest = removeSlice(parsed._unknown, this.getPath()).slice(Object.keys(this.arguments || {}).length)

const yamlDoc = this.spec.internal.yamlDoc

// Render the command variables
const variablesContext = new CustomCommandContext({ ...garden, args, opts, rest })
const commandVariables = resolveTemplateStrings(this.spec.variables, variablesContext)
const commandVariables = resolveTemplateStrings({
value: this.spec.variables,
context: variablesContext,
source: { yamlDoc, basePath: ["variables"] },
})
const variables: any = jsonMerge(cloneDeep(garden.variables), commandVariables)

// Make a new template context with the resolved variables
Expand All @@ -137,11 +143,16 @@ export class CustomCommandWrapper extends Command {
const startedAt = new Date()

const exec = validateWithPath({
config: resolveTemplateStrings(this.spec.exec, commandContext),
config: resolveTemplateStrings({
value: this.spec.exec,
context: commandContext,
source: { yamlDoc, basePath: ["exec"] },
}),
schema: customCommandExecSchema(),
path: this.spec.internal.basePath,
projectRoot: garden.projectRoot,
configType: `exec field in custom Command '${this.name}'`,
source: undefined,
})

const command = exec.command
Expand Down Expand Up @@ -186,11 +197,16 @@ export class CustomCommandWrapper extends Command {
const startedAt = new Date()

let gardenCommand = validateWithPath({
config: resolveTemplateStrings(this.spec.gardenCommand, commandContext),
config: resolveTemplateStrings({
value: this.spec.gardenCommand,
context: commandContext,
source: { yamlDoc, basePath: ["gardenCommand"] },
}),
schema: customCommandGardenCommandSchema(),
path: this.spec.internal.basePath,
projectRoot: garden.projectRoot,
configType: `gardenCommand field in custom Command '${this.name}'`,
source: undefined,
})

log.debug(`Running Garden command: ${gardenCommand.join(" ")}`)
Expand Down Expand Up @@ -287,6 +303,7 @@ export async function getCustomCommands(log: Log, projectRoot: string) {
path: (<CommandResource>config).internal.basePath,
projectRoot,
configType: `custom Command '${config.name}'`,
source: { yamlDoc: (<CommandResource>config).internal.yamlDoc },
})
)

Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/login.ts
Expand Up @@ -15,7 +15,7 @@ import { ConfigurationError, TimeoutError, InternalError, CloudApiError } from "
import { AuthRedirectServer } from "../cloud/auth"
import { EventBus } from "../events/events"
import { getCloudDistributionName } from "../util/util"
import { ProjectResource } from "../config/project"
import { ProjectConfig } from "../config/project"
import { findProjectConfig } from "../config/base"
import { BooleanParameter } from "../cli/params"
import { deline } from "../util/string"
Expand Down Expand Up @@ -56,7 +56,7 @@ export class LoginCommand extends Command<{}, Opts> {
// so we initialize it here. noProject also make sure that the project config is not
// initialized in the garden class, so we need to read it in here to get the cloud
// domain.
let projectConfig: ProjectResource | undefined = undefined
let projectConfig: ProjectConfig | undefined = undefined
const forceProjectCheck = !opts["disable-project-check"]

if (forceProjectCheck) {
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/logout.ts
Expand Up @@ -12,7 +12,7 @@ import { CloudApi, getGardenCloudDomain } from "../cloud/api"
import { dedent, deline } from "../util/string"
import { getCloudDistributionName } from "../util/util"
import { ConfigurationError } from "../exceptions"
import { ProjectResource } from "../config/project"
import { ProjectConfig } from "../config/project"
import { findProjectConfig } from "../config/base"
import { BooleanParameter } from "../cli/params"

Expand Down Expand Up @@ -43,7 +43,7 @@ export class LogOutCommand extends Command<{}, Opts> {
// The Cloud API is missing from the Garden class for commands with noProject
// so we initialize it with a cloud domain derived from `getGardenCloudDomain`.

let projectConfig: ProjectResource | undefined = undefined
let projectConfig: ProjectConfig | undefined = undefined
const forceProjectCheck = !opts["disable-project-check"]

if (forceProjectCheck) {
Expand Down
16 changes: 13 additions & 3 deletions core/src/commands/workflow.ts
Expand Up @@ -92,7 +92,12 @@ export class WorkflowCommand extends Command<Args, {}> {
await registerAndSetUid(garden, log, workflow)
garden.events.emit("workflowRunning", {})
const templateContext = new WorkflowConfigContext(garden, garden.variables)
const files = resolveTemplateStrings(workflow.files || [], templateContext)
const yamlDoc = workflow.internal.yamlDoc
const files = resolveTemplateStrings({
value: workflow.files || [],
context: templateContext,
source: { yamlDoc, basePath: ["files"] },
})

// Write all the configured files for the workflow
await Promise.all(files.map((file) => writeWorkflowFile(garden, file)))
Expand Down Expand Up @@ -162,10 +167,15 @@ export class WorkflowCommand extends Command<Args, {}> {
stepBodyLog.root.storeEntries = true
try {
if (step.command) {
step.command = resolveTemplateStrings(step.command, stepTemplateContext).filter((arg) => !!arg)
step.command = resolveTemplateStrings({
value: step.command,
context: stepTemplateContext,
source: { yamlDoc, basePath: ["steps", index, "command"] },
}).filter((arg) => !!arg)

stepResult = await runStepCommand(stepParams)
} else if (step.script) {
step.script = resolveTemplateString(step.script, stepTemplateContext)
step.script = resolveTemplateString({ string: step.script, context: stepTemplateContext })
stepResult = await runStepScript(stepParams)
} else {
garden.events.emit("workflowStepError", getStepEndEvent(index, stepStartedAt))
Expand Down

0 comments on commit a098a14

Please sign in to comment.