diff --git a/garden-service/src/config/base.ts b/garden-service/src/config/base.ts index e210786cd6..3fbd480bc4 100644 --- a/garden-service/src/config/base.ts +++ b/garden-service/src/config/base.ts @@ -6,13 +6,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { join, relative, basename, sep, resolve } from "path" +import { join, basename, sep, resolve } from "path" import { findByName, getNames, } from "../util/util" import { baseModuleSpecSchema, ModuleConfig } from "./module" -import { validate } from "./common" +import { validateWithPath } from "./common" import { ConfigurationError } from "../exceptions" import * as Joi from "joi" import * as yaml from "js-yaml" @@ -54,7 +54,7 @@ export async function loadConfig(projectRoot: string, path: string): Promisevalidate(spec, configSchema, { context: relative(projectRoot, absPath) }) + const parsed = validateWithPath({ + config: spec, + schema: configSchema, + configType: "config", + path: absPath, + projectRoot, + }) const dirname = basename(path) const project = parsed.project diff --git a/garden-service/src/config/common.ts b/garden-service/src/config/common.ts index 281b7a5e6b..722efa984d 100644 --- a/garden-service/src/config/common.ts +++ b/garden-service/src/config/common.ts @@ -11,6 +11,7 @@ import * as Joi from "joi" import * as uuid from "uuid" import { ConfigurationError, LocalConfigError } from "../exceptions" import chalk from "chalk" +import { relative } from "path" export type Primitive = string | number | boolean @@ -113,10 +114,41 @@ const joiOptions = { } export interface ValidateOptions { - context?: string + context?: string // Descriptive text to include in validation error messages, e.g. "module at some/local/path" ErrorClass?: typeof ConfigurationError | typeof LocalConfigError } +export interface ValidateWithPathParams { + config: T, + schema: Joi.Schema, + path: string, // Absolute path to the config file, including filename + projectRoot: string, + name?: string, // Name of the top-level entity that the config belongs to, e.g. "some-module" or "some-project" + configType?: string // The type of top-level entity that the config belongs to, e.g. "module" or "project" + ErrorClass?: typeof ConfigurationError | typeof LocalConfigError +} + +/** + * Should be used whenever a path to the corresponding config file is available while validating config + * files. + * + * This is to ensure consistent error messages that include the relative path to the failing file. + */ +export function validateWithPath( + { config, schema, path, projectRoot, name, configType = "module", ErrorClass }: ValidateWithPathParams, +) { + + const validateOpts = { + context: `${configType} ${name ? name + " " : ""}in ${relative(projectRoot, path)}`, + } + + if (ErrorClass) { + validateOpts["ErrorClass"] = ErrorClass + } + + return validate(config, schema, validateOpts) +} + export function validate( value: T, schema: Joi.Schema, diff --git a/garden-service/src/plugins/container.ts b/garden-service/src/plugins/container.ts index 70c66ec77a..4b3ca9961d 100644 --- a/garden-service/src/plugins/container.ts +++ b/garden-service/src/plugins/container.ts @@ -17,9 +17,9 @@ import { joiEnvVars, joiUserIdentifier, joiArray, - validate, PrimitiveMap, joiPrimitive, + validateWithPath, } from "../config/common" import { pathExists } from "fs-extra" import { join } from "path" @@ -479,8 +479,15 @@ export const helpers = { } -export async function validateContainerModule({ moduleConfig }: ValidateModuleParams) { - moduleConfig.spec = validate(moduleConfig.spec, containerModuleSpecSchema, { context: `module ${moduleConfig.name}` }) +export async function validateContainerModule({ ctx, moduleConfig }: ValidateModuleParams) { + + moduleConfig.spec = validateWithPath({ + config: moduleConfig.spec, + schema: containerModuleSpecSchema, + name: moduleConfig.name, + path: moduleConfig.path, + projectRoot: ctx.projectRoot, + }) // validate services moduleConfig.serviceConfigs = moduleConfig.spec.services.map(spec => { diff --git a/garden-service/src/plugins/exec.ts b/garden-service/src/plugins/exec.ts index e456e49cb0..65c7226580 100644 --- a/garden-service/src/plugins/exec.ts +++ b/garden-service/src/plugins/exec.ts @@ -12,7 +12,7 @@ import { join } from "path" import { joiArray, joiEnvVars, - validate, + validateWithPath, } from "../config/common" import { GardenPlugin, @@ -84,9 +84,16 @@ export const execModuleSpecSchema = Joi.object() export interface ExecModule extends Module { } export async function parseExecModule( - { moduleConfig }: ValidateModuleParams, + { ctx, moduleConfig }: ValidateModuleParams, ): Promise { - moduleConfig.spec = validate(moduleConfig.spec, execModuleSpecSchema, { context: `module ${moduleConfig.name}` }) + + moduleConfig.spec = validateWithPath({ + config: moduleConfig.spec, + schema: execModuleSpecSchema, + name: moduleConfig.name, + path: moduleConfig.path, + projectRoot: ctx.projectRoot, + }) moduleConfig.taskConfigs = moduleConfig.spec.tasks.map(t => ({ name: t.name, diff --git a/garden-service/src/plugins/google/google-cloud-functions.ts b/garden-service/src/plugins/google/google-cloud-functions.ts index 5d5d7fa19c..34cfb42241 100644 --- a/garden-service/src/plugins/google/google-cloud-functions.ts +++ b/garden-service/src/plugins/google/google-cloud-functions.ts @@ -8,7 +8,7 @@ import { joiArray, - validate, + validateWithPath, } from "../../config/common" import { Module } from "../../types/module" import { ValidateModuleResult } from "../../types/plugin/outputs" @@ -76,12 +76,17 @@ const gcfModuleSpecSchema = Joi.object() export interface GcfModule extends Module { } export async function parseGcfModule( - { moduleConfig }: ValidateModuleParams, + { ctx, moduleConfig }: ValidateModuleParams, ): Promise> { + // TODO: check that each function exists at the specified path - moduleConfig.spec = validate( - moduleConfig.spec, gcfModuleSpecSchema, { context: `module ${moduleConfig.name}` }, - ) + moduleConfig.spec = validateWithPath({ + config: moduleConfig.spec, + schema: gcfModuleSpecSchema, + name: moduleConfig.name, + path: moduleConfig.path, + projectRoot: ctx.projectRoot, + }) moduleConfig.serviceConfigs = moduleConfig.spec.functions.map(f => ({ name: f.name, diff --git a/garden-service/src/plugins/kubernetes/helm.ts b/garden-service/src/plugins/kubernetes/helm.ts index 7b7f1428e3..7eb8333dad 100644 --- a/garden-service/src/plugins/kubernetes/helm.ts +++ b/garden-service/src/plugins/kubernetes/helm.ts @@ -19,7 +19,7 @@ import { joiIdentifier, joiPrimitive, Primitive, - validate, + validateWithPath, } from "../../config/common" import { Module } from "../../types/module" import { ModuleAndRuntimeActions } from "../../types/plugin/plugin" @@ -111,12 +111,15 @@ const helmStatusCodeMap: { [code: number]: ServiceState } = { } export const helmHandlers: Partial> = { - async validate({ moduleConfig }: ValidateModuleParams): Promise { - moduleConfig.spec = validate( - moduleConfig.spec, - helmModuleSpecSchema, - { context: `helm module ${moduleConfig.name}` }, - ) + async validate({ ctx, moduleConfig }: ValidateModuleParams): Promise { + + moduleConfig.spec = validateWithPath({ + config: moduleConfig.spec, + schema: helmModuleSpecSchema, + name: moduleConfig.name, + path: moduleConfig.path, + projectRoot: ctx.projectRoot, + }) const { chart, version, parameters, dependencies } = moduleConfig.spec diff --git a/garden-service/test/data/test-project-invalid-config/garden.yml b/garden-service/test/data/test-project-invalid-config/garden.yml new file mode 100644 index 0000000000..cb1b2ec4a2 --- /dev/null +++ b/garden-service/test/data/test-project-invalid-config/garden.yml @@ -0,0 +1,6 @@ +project: + name: test-project-invalid-config + environments: + - name: local + providers: + - name: test-plugin diff --git a/garden-service/test/data/test-project-invalid-config/invalid-config-module/garden.yml b/garden-service/test/data/test-project-invalid-config/invalid-config-module/garden.yml new file mode 100644 index 0000000000..e8d6e3b4d8 --- /dev/null +++ b/garden-service/test/data/test-project-invalid-config/invalid-config-module/garden.yml @@ -0,0 +1,2 @@ +module: + name: invalid-config \ No newline at end of file diff --git a/garden-service/test/data/test-project-invalid-config/invalid-syntax-module/garden.yml b/garden-service/test/data/test-project-invalid-config/invalid-syntax-module/garden.yml new file mode 100644 index 0000000000..dda47b85b1 --- /dev/null +++ b/garden-service/test/data/test-project-invalid-config/invalid-syntax-module/garden.yml @@ -0,0 +1,3 @@ +module: + name: invalid-syntax + foo \ No newline at end of file diff --git a/garden-service/test/src/config/base.ts b/garden-service/test/src/config/base.ts index 708903d51f..043533303c 100644 --- a/garden-service/test/src/config/base.ts +++ b/garden-service/test/src/config/base.ts @@ -1,14 +1,38 @@ import { expect } from "chai" import { loadConfig } from "../../../src/config/base" import { resolve } from "path" -import { dataDir } from "../../helpers" +import { dataDir, expectError } from "../../helpers" const projectPathA = resolve(dataDir, "test-project-a") const modulePathA = resolve(projectPathA, "module-a") -describe("loadConfig", async () => { +describe("loadConfig", () => { - // TODO: test more cases + error cases + it("should not throw an error if no file was found", async () => { + const parsed = await loadConfig(projectPathA, resolve(projectPathA, "non-existent-module")) + + expect(parsed).to.eql(undefined) + }) + + it("should throw a config error if the file couldn't be parsed°", async () => { + const projectPath = resolve(dataDir, "test-project-invalid-config") + await expectError( + async () => await loadConfig(projectPath, resolve(projectPath, "invalid-syntax-module")), + (err) => { + expect(err.message).to.match(/Could not parse/) + }) + }) + + it("should include the module's relative path in the error message for invalid config", async () => { + const projectPath = resolve(dataDir, "test-project-invalid-config") + await expectError( + async () => await loadConfig(projectPath, resolve(projectPath, "invalid-config-module")), + (err) => { + expect(err.message).to.match(/invalid-config-module\/garden.yml/) + }) + }) + + // TODO: test more cases it("should load and parse a project config", async () => { const parsed = await loadConfig(projectPathA, projectPathA)