fix: add path to module validation error messages

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): Promise<Gar
   let fileData
   let spec: any
 
-  // loadConfig returns null if config file is not found in the given directory
+  // loadConfig returns undefined if config file is not found in the given directory
   try {
     fileData = await readFile(absPath)
   } catch (err) {
@@ -81,7 +81,13 @@ export async function loadConfig(projectRoot: string, path: string): Promise<Gar
     }
   }
 
-  const parsed = <GardenConfig>validate(spec, configSchema, { context: relative(projectRoot, absPath) })
+  const parsed = <GardenConfig>validateWithPath({
+    config: spec,
+    schema: configSchema,
+    configType: "config",
+    path: absPath,
+    projectRoot,
+  })
 
   const dirname = basename(path)
   const project = parsed.project

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<T> {
+  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<T>(
+  { config, schema, path, projectRoot, name, configType = "module", ErrorClass }: ValidateWithPathParams<T>,
+) {
+
+  const validateOpts = {
+    context: `${configType} ${name ? name + " " : ""}in ${relative(projectRoot, path)}`,
+  }
+
+  if (ErrorClass) {
+    validateOpts["ErrorClass"] = ErrorClass
+  }
+
+  return validate(config, schema, validateOpts)
+}
+
 export function validate<T>(
   value: T,
   schema: Joi.Schema,

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<ContainerModule>) {
-  moduleConfig.spec = validate(moduleConfig.spec, containerModuleSpecSchema, { context: `module ${moduleConfig.name}` })
+export async function validateContainerModule({ ctx, moduleConfig }: ValidateModuleParams<ContainerModule>) {
+
+  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 => {

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<ExecModuleSpec, BaseServiceSpec, ExecTestSpec> { }
 
 export async function parseExecModule(
-  { moduleConfig }: ValidateModuleParams<ExecModule>,
+  { ctx, moduleConfig }: ValidateModuleParams<ExecModule>,
 ): Promise<ValidateModuleResult> {
-  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,

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<GcfModuleSpec, GcfServiceSpec, ExecTestSpec> { }
 
 export async function parseGcfModule(
-  { moduleConfig }: ValidateModuleParams<GcfModule>,
+  { ctx, moduleConfig }: ValidateModuleParams<GcfModule>,
 ): Promise<ValidateModuleResult<GcfModule>> {
+
   // 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,

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<ModuleAndRuntimeActions<HelmModule>> = {
-  async validate({ moduleConfig }: ValidateModuleParams): Promise<ValidateModuleResult> {
-    moduleConfig.spec = validate(
-      moduleConfig.spec,
-      helmModuleSpecSchema,
-      { context: `helm module ${moduleConfig.name}` },
-    )
+  async validate({ ctx, moduleConfig }: ValidateModuleParams): Promise<ValidateModuleResult> {
+
+    moduleConfig.spec = validateWithPath({
+      config: moduleConfig.spec,
+      schema: helmModuleSpecSchema,
+      name: moduleConfig.name,
+      path: moduleConfig.path,
+      projectRoot: ctx.projectRoot,
+    })
 
     const { chart, version, parameters, dependencies } = moduleConfig.spec
 

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

garden-service/test/data/test-project-invalid-config/invalid-config-module/garden.yml

@@ -0,0 +1,2 @@
+module:
+  name: invalid-config

garden-service/test/data/test-project-invalid-config/invalid-syntax-module/garden.yml

@@ -0,0 +1,3 @@
+module:
+  name: invalid-syntax
+  foo

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)