fix(vcs): garden.yml changes now only affect relevant module version (#…

…1009)

* fix(vcs): garden.yml changes now only affect relevant module version

Previously we were explicitly including the module config file in the
module version file list. Now we ensure that it is _not_ included,
which is the intended behavior after we started hashing the actual
module config as part of the module version.

garden-service/src/bin/add-version-files.ts

@@ -33,7 +33,7 @@ async function addVersionFiles() {
     const versionFilePath = resolve(path, GARDEN_VERSIONFILE_NAME)
 
     const vcsHandler = new GitHandler(garden.gardenDirPath)
-    const treeVersion = await vcsHandler.getTreeVersion(path, config.include || null)
+    const treeVersion = await vcsHandler.getTreeVersion(config)
 
     console.log(`${config.name} -> ${relative(STATIC_DIR, versionFilePath)}`)
 

garden-service/src/config/base.ts

@@ -10,7 +10,7 @@ import { sep, resolve, relative, basename } from "path"
 import * as yaml from "js-yaml"
 import { readFile } from "fs-extra"
 import { omit, flatten, isPlainObject, find } from "lodash"
-import { baseModuleSpecSchema, ModuleResource } from "./module"
+import { ModuleResource, moduleConfigSchema } from "./module"
 import { ConfigurationError } from "../exceptions"
 import { DEFAULT_API_VERSION } from "../constants"
 import { ProjectResource } from "../config/project"
@@ -23,31 +23,31 @@ export interface GardenResource {
   path: string
 }
 
-const baseModuleSchemaKeys = Object.keys(baseModuleSpecSchema.describe().children).concat(["kind"])
+const baseModuleSchemaKeys = Object.keys(moduleConfigSchema.describe().children).concat(["kind"])
 
 export async function loadConfig(projectRoot: string, path: string): Promise<GardenResource[]> {
   // TODO: nicer error messages when load/validation fails
-  const absPath = await getConfigFilePath(path)
+  const configPath = await getConfigFilePath(path)
   let fileData: Buffer
   let rawSpecs: any[]
 
   // loadConfig returns undefined if config file is not found in the given directory
   try {
-    fileData = await readFile(absPath)
+    fileData = await readFile(configPath)
   } catch (err) {
     return []
   }
 
   try {
     rawSpecs = yaml.safeLoadAll(fileData.toString()) || []
   } catch (err) {
-    throw new ConfigurationError(`Could not parse ${basename(absPath)} in directory ${path} as valid YAML`, err)
+    throw new ConfigurationError(`Could not parse ${basename(configPath)} in directory ${path} as valid YAML`, err)
   }
 
   // Ignore empty resources
   rawSpecs = rawSpecs.filter(Boolean)
 
-  const resources: GardenResource[] = flatten(rawSpecs.map(s => prepareResources(s, path, projectRoot)))
+  const resources: GardenResource[] = flatten(rawSpecs.map(s => prepareResources(s, path, configPath, projectRoot)))
 
   const projectSpecs = resources.filter(s => s.kind === "Project")
 
@@ -72,15 +72,15 @@ export type ConfigKind = "Module" | "Project"
  * definitions). The kind key is removed before validation, so that specs following both styles can be validated
  * with the same schema.
  */
-function prepareResources(spec: any, path: string, projectRoot: string): GardenResource[] {
+function prepareResources(spec: any, path: string, configPath: string, projectRoot: string): GardenResource[] {
   if (!isPlainObject(spec)) {
     throw new ConfigurationError(`Invalid configuration found in ${path}`, { spec, path })
   }
 
   if (spec.kind) {
-    return [prepareFlatConfigDoc(spec, path, projectRoot)]
+    return [prepareFlatConfigDoc(spec, path, configPath, projectRoot)]
   } else {
-    return prepareScopedConfigDoc(spec, path)
+    return prepareScopedConfigDoc(spec, path, configPath)
   }
 }
 
@@ -89,13 +89,13 @@ function prepareResources(spec: any, path: string, projectRoot: string): GardenR
  *
  * The spec defines either a project or a module (determined by its "kind" field).
  */
-function prepareFlatConfigDoc(spec: any, path: string, projectRoot: string): GardenResource {
+function prepareFlatConfigDoc(spec: any, path: string, configPath: string, projectRoot: string): GardenResource {
   const kind = spec.kind
 
   if (kind === "Project") {
-    return prepareProjectConfig(spec, path)
+    return prepareProjectConfig(spec, path, configPath)
   } else if (kind === "Module") {
-    return prepareModuleResource(spec, path)
+    return prepareModuleResource(spec, path, configPath)
   } else {
     const relPath = `${relative(projectRoot, path)}/garden.yml`
     throw new ConfigurationError(`Unknown config kind ${kind} in ${relPath}`, { kind, path: relPath })
@@ -108,32 +108,33 @@ function prepareFlatConfigDoc(spec: any, path: string, projectRoot: string): Gar
  * 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): GardenResource[] {
+function prepareScopedConfigDoc(spec: any, path: string, configPath: string): GardenResource[] {
   const resources: GardenResource[] = []
 
   if (spec.project) {
-    resources.push(prepareProjectConfig(spec.project, path))
+    resources.push(prepareProjectConfig(spec.project, path, configPath))
   }
 
   if (spec.module) {
-    resources.push(prepareModuleResource(spec.module, path))
+    resources.push(prepareModuleResource(spec.module, path, configPath))
   }
 
   return resources
 }
 
-function prepareProjectConfig(spec: any, path: string): ProjectResource {
+function prepareProjectConfig(spec: any, path: string, configPath: string): ProjectResource {
   if (!spec.apiVersion) {
     spec.apiVersion = DEFAULT_API_VERSION
   }
 
   spec.kind = "Project"
   spec.path = path
+  spec.configPath = configPath
 
   return spec
 }
 
-function prepareModuleResource(spec: any, path: string): ModuleResource {
+function prepareModuleResource(spec: any, path: string, configPath: string): ModuleResource {
   /**
    * We allow specifying modules by name only as a shorthand:
    *   dependencies:
@@ -152,6 +153,7 @@ function prepareModuleResource(spec: any, path: string): ModuleResource {
     build: {
       dependencies,
     },
+    configPath,
     description: spec.description,
     include: spec.include,
     name: spec.name,

garden-service/src/config/module.ts

@@ -148,6 +148,7 @@ export interface ModuleConfig
 
   outputs: PrimitiveMap
   path: string
+  configPath?: string
   plugin?: string   // used to identify modules that are bundled as part of a plugin
   serviceConfigs: ServiceConfig<S>[]
   testConfigs: TestConfig<T>[]
@@ -163,6 +164,8 @@ export const moduleConfigSchema = baseModuleSpecSchema
       .description("The outputs defined by the module (referenceable in other module configs)."),
     path: joi.string()
       .description("The filesystem path of the module."),
+    configPath: joi.string()
+      .description("The filesystem path of the module config file."),
     plugin: joiIdentifier()
       .meta({ internal: true })
       .description("The name of a the parent plugin of the module, if applicable."),

garden-service/src/config/project.ts

@@ -96,6 +96,7 @@ export interface ProjectConfig {
   kind: "Project",
   name: string
   path: string
+  configPath?: string
   defaultEnvironment: string
   environmentDefaults?: CommonEnvironmentConfig
   environments: EnvironmentConfig[]
@@ -138,7 +139,12 @@ export const projectSchema = joi.object()
       .only(DEFAULT_API_VERSION)
       .description("The schema version of this project's config (currently not used)."),
     kind: joi.string().default("Project").only("Project"),
-    path: joi.string().meta({ internal: true }),
+    path: joi.string()
+      .meta({ internal: true })
+      .description("The path to the project root."),
+    configPath: joi.string()
+      .meta({ internal: true })
+      .description("The path to the project config file."),
     name: projectNameSchema,
     defaultEnvironment: joi.string()
       .allow("")

garden-service/src/garden.ts

@@ -31,7 +31,7 @@ import { loadConfig, findProjectConfig } from "./config/base"
 import { BaseTask } from "./tasks/base"
 import { LocalConfigStore, ConfigStore, GlobalConfigStore } from "./config-store"
 import { getLinkedSources, ExternalSourceType } from "./util/ext-source-util"
-import { BuildDependencyConfig, ModuleConfig, baseModuleSpecSchema, ModuleResource } from "./config/module"
+import { BuildDependencyConfig, ModuleConfig, ModuleResource, moduleConfigSchema } from "./config/module"
 import { ModuleConfigContext, ContextResolveOpts } from "./config/config-context"
 import { createPluginContext } from "./plugin-context"
 import { ModuleAndRuntimeActions, Plugins, RegisterPluginParam } from "./types/plugin/plugin"
@@ -495,6 +495,7 @@ export class Garden {
       config = await resolveTemplateStrings(cloneDeep(config), opts.configContext!, opts)
       const description = await actions.describeType(config.type)
 
+      // Validate the module-type specific spec
       config.spec = validateWithPath({
         config: config.spec,
         schema: description.schema,
@@ -515,9 +516,10 @@ export class Garden {
           .map(dep => typeof dep === "string" ? { name: dep, copy: [] } : dep)
       }
 
+      // Validate the base config schema
       config = validateWithPath({
         config,
-        schema: baseModuleSpecSchema,
+        schema: moduleConfigSchema,
         configType: "module",
         name: config.name,
         path: config.path,

garden-service/src/tasks/build.ts

@@ -80,7 +80,7 @@ export class BuildTask extends BaseTask {
     await this.garden.buildDir.syncDependencyProducts(this.module, log)
 
     if (!this.force) {
-      log.setState({ msg: `Getting build status...` })
+      log.setState({ msg: `Getting build status for ${module.version.versionString}...` })
       const status = await actions.getBuildStatus({ log: this.log, module })
 
       if (status.ready) {

garden-service/src/types/module.ts

@@ -19,7 +19,6 @@ import { joiArray, joiIdentifier, joiIdentifierMap, joi } from "../config/common
 import { ConfigGraph } from "../config-graph"
 import * as Bluebird from "bluebird"
 import { getConfigFilePath } from "../util/fs"
-import { relative } from "path"
 
 export interface FileCopySpec {
   source: string
@@ -93,15 +92,8 @@ export interface ModuleConfigMap<T extends ModuleConfig = ModuleConfig> {
 
 export async function moduleFromConfig(garden: Garden, graph: ConfigGraph, config: ModuleConfig): Promise<Module> {
   const configPath = await getConfigFilePath(config.path)
-  const relativeConfigPath = relative(config.path, configPath)
   const version = await garden.resolveVersion(config.name, config.build.dependencies)
 
-  // Always include configuration file
-  // TODO: move this logic to resolveVersion()
-  if (!version.files.includes(configPath) && !version.files.includes(relativeConfigPath)) {
-    version.files.push(configPath)
-  }
-
   const module: Module = {
     ...cloneDeep(config),
 

garden-service/src/vcs/vcs.ts

@@ -85,22 +85,27 @@ export abstract class VcsHandler {
   abstract async ensureRemoteSource(params: RemoteSourceParams): Promise<string>
   abstract async updateRemoteSource(params: RemoteSourceParams): Promise<void>
 
-  // Note: explicitly requiring the include variable or null, to make sure it's specified
-  async getTreeVersion(path: string, include: string[] | null): Promise<TreeVersion> {
-    const files = sortBy(await this.getFiles(path, include || undefined), "path")
-    const contentHash = files.length > 0 ? hashFileHashes(files.map(f => f.hash)) : NEW_MODULE_VERSION
+  async getTreeVersion(moduleConfig: ModuleConfig): Promise<TreeVersion> {
+    const configPath = moduleConfig.configPath
+
+    const files = sortBy(await this.getFiles(moduleConfig.path, moduleConfig.include), "path")
+      // Don't include the config file in the file list
+      .filter(f => !configPath || f.path !== configPath)
+
+    const contentHash = files.length > 0 ? hashStrings(files.map(f => f.hash)) : NEW_MODULE_VERSION
+
     return { contentHash, files: files.map(f => f.path) }
   }
 
-  async resolveTreeVersion(path: string, include: string[] | null): Promise<TreeVersion> {
+  async resolveTreeVersion(moduleConfig: ModuleConfig): Promise<TreeVersion> {
     // the version file is used internally to specify versions outside of source control
-    const versionFilePath = join(path, GARDEN_TREEVERSION_FILENAME)
+    const versionFilePath = join(moduleConfig.path, GARDEN_TREEVERSION_FILENAME)
     const fileVersion = await readTreeVersionFile(versionFilePath)
-    return fileVersion || this.getTreeVersion(path, include)
+    return fileVersion || this.getTreeVersion(moduleConfig)
   }
 
   async resolveVersion(moduleConfig: ModuleConfig, dependencies: ModuleConfig[]): Promise<ModuleVersion> {
-    const treeVersion = await this.resolveTreeVersion(moduleConfig.path, moduleConfig.include || null)
+    const treeVersion = await this.resolveTreeVersion(moduleConfig)
 
     validate(treeVersion, treeVersionSchema, {
       context: `${this.name} tree version for module at ${moduleConfig.path}`,
@@ -120,7 +125,7 @@ export abstract class VcsHandler {
 
     const namedDependencyVersions = await Bluebird.map(
       dependencies,
-      async (m: ModuleConfig) => ({ name: m.name, ...await this.resolveTreeVersion(m.path, m.include || null) }),
+      async (m: ModuleConfig) => ({ name: m.name, ...await this.resolveTreeVersion(m) }),
     )
     const dependencyVersions = mapValues(keyBy(namedDependencyVersions, "name"), v => omit(v, "name"))
 
@@ -221,10 +226,10 @@ export function hashVersions(moduleConfig: ModuleConfig, versions: NamedTreeVers
   const configString = serializeConfig(moduleConfig)
   const versionStrings = sortBy(versions, "name")
     .map(v => `${v.name}_${v.contentHash}`)
-  return hashFileHashes([configString, ...versionStrings])
+  return hashStrings([configString, ...versionStrings])
 }
 
-export function hashFileHashes(hashes: string[]) {
+export function hashStrings(hashes: string[]) {
   const versionHash = createHash("sha256")
   versionHash.update(hashes.join("."))
   return versionHash.digest("hex").slice(0, 10)

garden-service/src/watch.ts

@@ -106,23 +106,25 @@ export class Watcher {
     return (path: string) => {
       this.log.debug(`Watcher: File ${path} ${type}`)
 
-      const changedModules = modules.filter(m => m.version.files.includes(path))
+      const changedModules = modules
+        .filter(m => m.version.files.includes(path) || m.configPath === path)
 
       this.sourcesChanged(modules, changedModules, path, type)
     }
   }
 
   private sourcesChanged(modules: Module[], changedModules: Module[], path: string, type: string) {
-    const changedModuleNames = changedModules.map(m => m.name)
-
     const parsed = parse(path)
     const filename = parsed.base
 
     if (isConfigFilename(filename) || filename === ".gitignore" || filename === ".gardenignore") {
       this.invalidateCached(modules)
 
-      if (changedModuleNames.length > 0) {
-        this.garden.events.emit("moduleConfigChanged", { names: changedModuleNames, path })
+      const changedModuleConfigs = changedModules.filter(m => m.configPath === path)
+
+      if (changedModuleConfigs.length > 0) {
+        const names = changedModuleConfigs.map(m => m.name)
+        this.garden.events.emit("moduleConfigChanged", { names, path })
       } else if (isConfigFilename(filename)) {
         if (parsed.dir === this.garden.projectRoot) {
           this.garden.events.emit("projectConfigChanged", {})
@@ -138,9 +140,10 @@ export class Watcher {
       return
     }
 
-    if (changedModuleNames.length > 0) {
+    if (changedModules.length > 0) {
+      const names = changedModules.map(m => m.name)
       this.invalidateCached(changedModules)
-      this.garden.events.emit("moduleSourcesChanged", { names: changedModuleNames, pathChanged: path })
+      this.garden.events.emit("moduleSourcesChanged", { names, pathChanged: path })
     }
   }
 

garden-service/test/unit/src/build-dir.ts

@@ -54,7 +54,6 @@ describe("BuildDir", () => {
     const buildDirA = await garden.buildDir.buildPath(moduleA.name)
 
     const copiedPaths = [
-      await getConfigFilePath(buildDirA),
       join(buildDirA, "some-dir", "some-file"),
     ]