fix(config): throw error on base module schema validation errors

We previously ignored configs that did not have `kind` set, and threw
some confusing errors when e.g. `type` or `name` were missing.

Fixes #1280

garden-service/src/config/base.ts

@@ -6,15 +6,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { sep, resolve, relative, basename } from "path"
+import { sep, resolve, relative, basename, dirname } from "path"
 import yaml from "js-yaml"
 import { readFile } from "fs-extra"
 import { omit, flatten, isPlainObject, find } from "lodash"
-import { ModuleResource, moduleConfigSchema } from "./module"
+import { ModuleResource, moduleConfigSchema, coreModuleSpecSchema } from "./module"
 import { ConfigurationError } from "../exceptions"
 import { DEFAULT_API_VERSION } from "../constants"
 import { ProjectResource } from "../config/project"
 import { getConfigFilePath } from "../util/fs"
+import { validateWithPath } from "./common"
 
 export interface GardenResource {
   apiVersion: string
@@ -82,10 +83,11 @@ function prepareResources(spec: any, path: string, configPath: string, projectRo
     })
   }
 
-  if (spec.kind) {
-    return [prepareFlatConfigDoc(spec, path, configPath, projectRoot)]
+  // TODO: remove support for the older scoped config style in 0.11
+  if (spec.project || spec.module) {
+    return prepareScopedConfigDoc(spec, path, configPath, projectRoot)
   } else {
-    return prepareScopedConfigDoc(spec, path, configPath)
+    return [prepareFlatConfigDoc(spec, path, configPath, projectRoot)]
   }
 }
 
@@ -96,13 +98,18 @@ function prepareResources(spec: any, path: string, configPath: string, projectRo
  */
 function prepareFlatConfigDoc(spec: any, path: string, configPath: string, projectRoot: string): GardenResource {
   const kind = spec.kind
+  const relPath = `${relative(projectRoot, path)}/garden.yml`
 
   if (kind === "Project") {
     return prepareProjectConfig(spec, path, configPath)
   } else if (kind === "Module") {
-    return prepareModuleResource(spec, path, configPath)
+    return prepareModuleResource(spec, path, configPath, projectRoot)
+  } else if (!kind) {
+    throw new ConfigurationError(`Missing \`kind\` field in config at ${relPath}`, {
+      kind,
+      path: relPath,
+    })
   } else {
-    const relPath = `${relative(projectRoot, path)}/garden.yml`
     throw new ConfigurationError(`Unknown config kind ${kind} in ${relPath}`, {
       kind,
       path: relPath,
@@ -116,15 +123,15 @@ function prepareFlatConfigDoc(spec: any, path: string, configPath: string, proje
  * The spec defines a project and/or a module, with the config for each nested under the "project" / "module" field,
  * respectively.
  */
-function prepareScopedConfigDoc(spec: any, path: string, configPath: string): GardenResource[] {
+function prepareScopedConfigDoc(spec: any, path: string, configPath: string, projectRoot: string): GardenResource[] {
   const resources: GardenResource[] = []
 
   if (spec.project) {
     resources.push(prepareProjectConfig(spec.project, path, configPath))
   }
 
   if (spec.module) {
-    resources.push(prepareModuleResource(spec.module, path, configPath))
+    resources.push(prepareModuleResource(spec.module, path, configPath, projectRoot))
   }
 
   return resources
@@ -142,7 +149,7 @@ function prepareProjectConfig(spec: any, path: string, configPath: string): Proj
   return spec
 }
 
-function prepareModuleResource(spec: any, path: string, configPath: string): ModuleResource {
+function prepareModuleResource(spec: any, path: string, configPath: string, projectRoot: string): ModuleResource {
   /**
    * We allow specifying modules by name only as a shorthand:
    *   dependencies:
@@ -155,7 +162,7 @@ function prepareModuleResource(spec: any, path: string, configPath: string): Mod
       : []
 
   // Built-in keys are validated here and the rest are put into the `spec` field
-  return {
+  const config: ModuleResource = {
     apiVersion: spec.apiVersion || DEFAULT_API_VERSION,
     kind: "Module",
     allowPublish: spec.allowPublish,
@@ -179,6 +186,17 @@ function prepareModuleResource(spec: any, path: string, configPath: string): Mod
     type: spec.type,
     taskConfigs: [],
   }
+
+  validateWithPath({
+    config,
+    schema: coreModuleSpecSchema,
+    path: dirname(configPath),
+    projectRoot,
+    configType: "module",
+    ErrorClass: ConfigurationError,
+  })
+
+  return config
 }
 
 export async function findProjectConfig(path: string, allowInvalid = false): Promise<ProjectResource | undefined> {

garden-service/src/config/module.ts

@@ -97,7 +97,8 @@ export const baseBuildSpecSchema = joi
   .default(() => ({ dependencies: [] }), "{}")
   .description("Specify how to build the module. Note that plugins may define additional keys on this object.")
 
-export const baseModuleSpecSchema = joi
+// These fields are validated immediately when loading the config file
+export const coreModuleSpecSchema = joi
   .object()
   .keys({
     apiVersion: joi
@@ -117,12 +118,20 @@ export const baseModuleSpecSchema = joi
       .required()
       .description("The name of this module.")
       .example("my-sweet-module"),
-    description: joi.string(),
-    include: joi
-      .array()
-      .items(joi.string().posixPath({ allowGlobs: true, subPathOnly: true }))
-      .description(
-        dedent`Specify a list of POSIX-style paths or globs that should be regarded as the source files for this
+  })
+  .required()
+  .unknown(true)
+  .description("Configure a module whose sources are located in this directory.")
+  .meta({ extendable: true })
+
+// These fields may be resolved later in the process, and allow for usage of template strings
+export const baseModuleSpecSchema = coreModuleSpecSchema.keys({
+  description: joi.string(),
+  include: joi
+    .array()
+    .items(joi.string().posixPath({ allowGlobs: true, subPathOnly: true }))
+    .description(
+      dedent`Specify a list of POSIX-style paths or globs that should be regarded as the source files for this
         module. Files that do *not* match these paths or globs are excluded when computing the version of the module,
         when responding to filesystem watch events, and when staging builds.
 
@@ -131,13 +140,13 @@ export const baseModuleSpecSchema = joi
         [Configuration Files guide](${includeGuideLink}) for details.
 
         Also note that specifying an empty list here means _no sources_ should be included.`
-      )
-      .example([["Dockerfile", "my-app.js"], {}]),
-    exclude: joi
-      .array()
-      .items(joi.string().posixPath({ allowGlobs: true, subPathOnly: true }))
-      .description(
-        dedent`Specify a list of POSIX-style paths or glob patterns that should be excluded from the module. Files that
+    )
+    .example([["Dockerfile", "my-app.js"], {}]),
+  exclude: joi
+    .array()
+    .items(joi.string().posixPath({ allowGlobs: true, subPathOnly: true }))
+    .description(
+      dedent`Specify a list of POSIX-style paths or glob patterns that should be excluded from the module. Files that
         match these paths or globs are excluded when computing the version of the module, when responding to filesystem
         watch events, and when staging builds.
 
@@ -149,24 +158,20 @@ export const baseModuleSpecSchema = joi
         and directories are watched for changes. Use the project \`modules.exclude\` field to affect those, if you have
         large directories that should not be watched for changes.
         `
-      )
-      .example([["tmp/**/*", "*.log"], {}]),
-    repositoryUrl: joiRepositoryUrl().description(
-      dedent`${joiRepositoryUrl().describe().description}
+    )
+    .example([["tmp/**/*", "*.log"], {}]),
+  repositoryUrl: joiRepositoryUrl().description(
+    dedent`${joiRepositoryUrl().describe().description}
 
         Garden will import the repository source code into this module, but read the module's
         config from the local garden.yml file.`
-    ),
-    allowPublish: joi
-      .boolean()
-      .default(true)
-      .description("When false, disables pushing this module to remote registries."),
-    build: baseBuildSpecSchema.unknown(true),
-  })
-  .required()
-  .unknown(true)
-  .description("Configure a module whose sources are located in this directory.")
-  .meta({ extendable: true })
+  ),
+  allowPublish: joi
+    .boolean()
+    .default(true)
+    .description("When false, disables pushing this module to remote registries."),
+  build: baseBuildSpecSchema.unknown(true),
+})
 
 export interface ModuleConfig<
   M extends ModuleSpec = any,

garden-service/test/data/test-project-invalid-config/missing-kind/garden.yml

@@ -0,0 +1,2 @@
+type: foo
+name: bla

garden-service/test/data/test-project-invalid-config/missing-name/garden.yml

@@ -0,0 +1,2 @@
+kind: Module
+type: foo

garden-service/test/data/test-project-invalid-config/missing-type/garden.yml

@@ -0,0 +1,2 @@
+kind: Module
+name: foo

garden-service/test/unit/src/config/base.ts

@@ -3,6 +3,7 @@ import { loadConfig, findProjectConfig } from "../../../../src/config/base"
 import { resolve } from "path"
 import { dataDir, expectError, getDataDir } from "../../../helpers"
 import { DEFAULT_API_VERSION } from "../../../../src/constants"
+import stripAnsi = require("strip-ansi")
 
 const projectPathA = resolve(dataDir, "test-project-a")
 const modulePathA = resolve(projectPathA, "module-a")
@@ -32,6 +33,40 @@ describe("loadConfig", () => {
     )
   })
 
+  it("should throw if a config doesn't specify a kind", async () => {
+    const projectPath = resolve(dataDir, "test-project-invalid-config")
+    await expectError(
+      async () => await loadConfig(projectPath, resolve(projectPath, "missing-kind")),
+      (err) => {
+        expect(err.message).to.equal("Missing `kind` field in config at missing-kind/garden.yml")
+      }
+    )
+  })
+
+  it("should throw if a module config doesn't specify a type", async () => {
+    const projectPath = resolve(dataDir, "test-project-invalid-config")
+    await expectError(
+      async () => await loadConfig(projectPath, resolve(projectPath, "missing-type")),
+      (err) => {
+        expect(stripAnsi(err.message)).to.equal(
+          "Error validating module (missing-type/garden.yml): key .type is required"
+        )
+      }
+    )
+  })
+
+  it("should throw if a module config doesn't specify a name", async () => {
+    const projectPath = resolve(dataDir, "test-project-invalid-config")
+    await expectError(
+      async () => await loadConfig(projectPath, resolve(projectPath, "missing-name")),
+      (err) => {
+        expect(stripAnsi(err.message)).to.equal(
+          "Error validating module (missing-name/garden.yml): key .name is required"
+        )
+      }
+    )
+  })
+
   // TODO: test more cases
   it("should load and parse a project config", async () => {
     const parsed = await loadConfig(projectPathA, projectPathA)