From 2ff4edfb3d3c6368ceb6314c8689faab5f653eb1 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Tue, 23 Jul 2019 19:43:00 +0200 Subject: [PATCH] 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 | 2 +- garden-service/src/config/base.ts | 36 ++--- garden-service/src/config/module.ts | 3 + garden-service/src/config/project.ts | 8 +- garden-service/src/garden.ts | 6 +- garden-service/src/tasks/build.ts | 2 +- garden-service/src/types/module.ts | 8 -- garden-service/src/vcs/vcs.ts | 27 ++-- garden-service/src/watch.ts | 17 ++- .../multiple-module-config}/garden.yml | 0 .../module-a/.garden-version | 0 .../module-a/garden.yml | 0 .../module-b/garden.yml | 0 .../module-c/garden.yml | 0 garden-service/test/unit/src/build-dir.ts | 1 - garden-service/test/unit/src/config/base.ts | 17 ++- garden-service/test/unit/src/garden.ts | 2 +- .../src/plugins/kubernetes/helm/config.ts | 1 + garden-service/test/unit/src/types/module.ts | 13 -- garden-service/test/unit/src/vcs/vcs.ts | 132 ++++++++++++------ 20 files changed, 168 insertions(+), 107 deletions(-) rename garden-service/test/unit/data/{test-project-multiple-module-config => test-projects/multiple-module-config}/garden.yml (100%) rename garden-service/test/unit/data/{test-project-multiple-module-config => test-projects/multiple-module-config}/module-a/.garden-version (100%) rename garden-service/test/unit/data/{test-project-multiple-module-config => test-projects/multiple-module-config}/module-a/garden.yml (100%) rename garden-service/test/unit/data/{test-project-multiple-module-config => test-projects/multiple-module-config}/module-b/garden.yml (100%) rename garden-service/test/unit/data/{test-project-multiple-module-config => test-projects/multiple-module-config}/module-c/garden.yml (100%) delete mode 100644 garden-service/test/unit/src/types/module.ts diff --git a/garden-service/src/bin/add-version-files.ts b/garden-service/src/bin/add-version-files.ts index 04cec4c383..006d6f9b39 100644 --- a/garden-service/src/bin/add-version-files.ts +++ b/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)}`) diff --git a/garden-service/src/config/base.ts b/garden-service/src/config/base.ts index 3562a21654..e22677ff9e 100644 --- a/garden-service/src/config/base.ts +++ b/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,17 +23,17 @@ 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 { // 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 [] } @@ -41,13 +41,13 @@ export async function loadConfig(projectRoot: string, path: string): Promise 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, diff --git a/garden-service/src/config/module.ts b/garden-service/src/config/module.ts index e2126289a7..8ed1f5584e 100644 --- a/garden-service/src/config/module.ts +++ b/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[] testConfigs: TestConfig[] @@ -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."), diff --git a/garden-service/src/config/project.ts b/garden-service/src/config/project.ts index b831d81cbf..fdfec13c29 100644 --- a/garden-service/src/config/project.ts +++ b/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("") diff --git a/garden-service/src/garden.ts b/garden-service/src/garden.ts index 1ce6d5ade3..67650475fd 100644 --- a/garden-service/src/garden.ts +++ b/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, diff --git a/garden-service/src/tasks/build.ts b/garden-service/src/tasks/build.ts index 76554358f0..ad0adacc42 100644 --- a/garden-service/src/tasks/build.ts +++ b/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) { diff --git a/garden-service/src/types/module.ts b/garden-service/src/types/module.ts index 73cf995973..7829c95c0c 100644 --- a/garden-service/src/types/module.ts +++ b/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 { export async function moduleFromConfig(garden: Garden, graph: ConfigGraph, config: ModuleConfig): Promise { 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), diff --git a/garden-service/src/vcs/vcs.ts b/garden-service/src/vcs/vcs.ts index f59ba2fedd..561ac227de 100644 --- a/garden-service/src/vcs/vcs.ts +++ b/garden-service/src/vcs/vcs.ts @@ -85,22 +85,27 @@ export abstract class VcsHandler { abstract async ensureRemoteSource(params: RemoteSourceParams): Promise abstract async updateRemoteSource(params: RemoteSourceParams): Promise - // Note: explicitly requiring the include variable or null, to make sure it's specified - async getTreeVersion(path: string, include: string[] | null): Promise { - 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 { + 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 { + async resolveTreeVersion(moduleConfig: ModuleConfig): Promise { // 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 { - 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) diff --git a/garden-service/src/watch.ts b/garden-service/src/watch.ts index 8be89debd9..e54d2e6c14 100644 --- a/garden-service/src/watch.ts +++ b/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 }) } } diff --git a/garden-service/test/unit/data/test-project-multiple-module-config/garden.yml b/garden-service/test/unit/data/test-projects/multiple-module-config/garden.yml similarity index 100% rename from garden-service/test/unit/data/test-project-multiple-module-config/garden.yml rename to garden-service/test/unit/data/test-projects/multiple-module-config/garden.yml diff --git a/garden-service/test/unit/data/test-project-multiple-module-config/module-a/.garden-version b/garden-service/test/unit/data/test-projects/multiple-module-config/module-a/.garden-version similarity index 100% rename from garden-service/test/unit/data/test-project-multiple-module-config/module-a/.garden-version rename to garden-service/test/unit/data/test-projects/multiple-module-config/module-a/.garden-version diff --git a/garden-service/test/unit/data/test-project-multiple-module-config/module-a/garden.yml b/garden-service/test/unit/data/test-projects/multiple-module-config/module-a/garden.yml similarity index 100% rename from garden-service/test/unit/data/test-project-multiple-module-config/module-a/garden.yml rename to garden-service/test/unit/data/test-projects/multiple-module-config/module-a/garden.yml diff --git a/garden-service/test/unit/data/test-project-multiple-module-config/module-b/garden.yml b/garden-service/test/unit/data/test-projects/multiple-module-config/module-b/garden.yml similarity index 100% rename from garden-service/test/unit/data/test-project-multiple-module-config/module-b/garden.yml rename to garden-service/test/unit/data/test-projects/multiple-module-config/module-b/garden.yml diff --git a/garden-service/test/unit/data/test-project-multiple-module-config/module-c/garden.yml b/garden-service/test/unit/data/test-projects/multiple-module-config/module-c/garden.yml similarity index 100% rename from garden-service/test/unit/data/test-project-multiple-module-config/module-c/garden.yml rename to garden-service/test/unit/data/test-projects/multiple-module-config/module-c/garden.yml diff --git a/garden-service/test/unit/src/build-dir.ts b/garden-service/test/unit/src/build-dir.ts index 7fbcfa5290..6a5e411f0a 100644 --- a/garden-service/test/unit/src/build-dir.ts +++ b/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"), ] diff --git a/garden-service/test/unit/src/config/base.ts b/garden-service/test/unit/src/config/base.ts index 6674ca8261..a5ae23a11a 100644 --- a/garden-service/test/unit/src/config/base.ts +++ b/garden-service/test/unit/src/config/base.ts @@ -7,7 +7,7 @@ import { DEFAULT_API_VERSION } from "../../../../src/constants" const projectPathA = resolve(dataDir, "test-project-a") const modulePathA = resolve(projectPathA, "module-a") -const projectPathMultipleModules = resolve(dataDir, "test-project-multiple-module-config") +const projectPathMultipleModules = resolve(dataDir, "test-projects", "multiple-module-config") const modulePathAMultiple = resolve(projectPathMultipleModules, "module-a") const projectPathDuplicateProjects = resolve(dataDir, "test-project-duplicate-project-config") @@ -34,12 +34,14 @@ describe("loadConfig", () => { // TODO: test more cases it("should load and parse a project config", async () => { const parsed = await loadConfig(projectPathA, projectPathA) + const configPath = resolve(projectPathA, "garden.yml") expect(parsed).to.eql([ { apiVersion: "garden.io/v0", kind: "Project", path: projectPathA, + configPath, name: "test-project-a", environmentDefaults: { variables: { some: "variable" }, @@ -62,6 +64,7 @@ describe("loadConfig", () => { it("should load and parse a module config", async () => { const parsed = await loadConfig(projectPathA, modulePathA) + const configPath = resolve(modulePathA, "garden.yml") expect(parsed).to.eql([ { @@ -69,6 +72,7 @@ describe("loadConfig", () => { kind: "Module", name: "module-a", type: "test", + configPath, description: undefined, include: undefined, repositoryUrl: undefined, @@ -106,11 +110,13 @@ describe("loadConfig", () => { it("should load and parse a config file defining a project and a module", async () => { const parsed = await loadConfig(projectPathMultipleModules, projectPathMultipleModules) + const configPath = resolve(projectPathMultipleModules, "garden.yml") expect(parsed).to.eql([ { apiVersion: "garden.io/v0", kind: "Project", + configPath, path: projectPathMultipleModules, environmentDefaults: { variables: { @@ -136,6 +142,7 @@ describe("loadConfig", () => { kind: "Module", name: "module-from-project-config", type: "test", + configPath, description: undefined, include: undefined, repositoryUrl: undefined, @@ -158,6 +165,7 @@ describe("loadConfig", () => { it("should load and parse a config file defining multiple modules", async () => { const parsed = await loadConfig(projectPathMultipleModules, modulePathAMultiple) + const configPath = resolve(modulePathAMultiple, "garden.yml") expect(parsed).to.eql([ { @@ -165,6 +173,7 @@ describe("loadConfig", () => { kind: "Module", name: "module-a1", type: "test", + configPath, allowPublish: undefined, description: undefined, include: undefined, @@ -196,6 +205,7 @@ describe("loadConfig", () => { kind: "Module", name: "module-a2", type: "test", + configPath, allowPublish: undefined, description: undefined, include: undefined, @@ -221,12 +231,14 @@ describe("loadConfig", () => { it("should parse a config file using the flat config style", async () => { const parsed = await loadConfig(projectPathFlat, projectPathFlat) + const configPath = resolve(projectPathFlat, "garden.yml") expect(parsed).to.eql([ { apiVersion: "garden.io/v0", kind: "Project", path: projectPathFlat, + configPath, environmentDefaults: { variables: { some: "variable" }, }, @@ -249,6 +261,7 @@ describe("loadConfig", () => { kind: "Module", name: "module-from-project-config", type: "test", + configPath, description: undefined, build: { dependencies: [], @@ -280,6 +293,7 @@ describe("loadConfig", () => { apiVersion: "garden.io/v0", kind: "Project", path: projectPath, + configPath: resolve(projectPath, "garden.yml"), name: "test-project-a", environmentDefaults: { variables: { some: "variable" }, @@ -326,6 +340,7 @@ describe("loadConfig", () => { kind: "Project", name: "foo", path, + configPath: resolve(path, "garden.yml"), }, ]) }) diff --git a/garden-service/test/unit/src/garden.ts b/garden-service/test/unit/src/garden.ts index 4e9fc3625f..ca3b597466 100644 --- a/garden-service/test/unit/src/garden.ts +++ b/garden-service/test/unit/src/garden.ts @@ -654,7 +654,7 @@ describe("Garden", () => { }) it("should scan and add modules for projects with configs defining multiple modules", async () => { - const garden = await makeTestGarden(resolve(dataDir, "test-project-multiple-module-config")) + const garden = await makeTestGarden(resolve(dataDir, "test-projects", "multiple-module-config")) await garden.scanModules() const modules = await garden.resolveModuleConfigs() diff --git a/garden-service/test/unit/src/plugins/kubernetes/helm/config.ts b/garden-service/test/unit/src/plugins/kubernetes/helm/config.ts index bd2140f78d..bb3e3c353e 100644 --- a/garden-service/test/unit/src/plugins/kubernetes/helm/config.ts +++ b/garden-service/test/unit/src/plugins/kubernetes/helm/config.ts @@ -46,6 +46,7 @@ describe("validateHelmModule", () => { build: { dependencies: [], }, + configPath: resolve(ctx.projectRoot, "api", "garden.yml"), description: "The API backend for the voting UI", include: undefined, name: "api", diff --git a/garden-service/test/unit/src/types/module.ts b/garden-service/test/unit/src/types/module.ts deleted file mode 100644 index b6766d5138..0000000000 --- a/garden-service/test/unit/src/types/module.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { expect } from "chai" -import { getDataDir, makeTestGarden } from "../../../helpers" - -describe("moduleFromConfig", () => { - it("should add module config file to version files if needed", async () => { - const projectRoot = await getDataDir("test-projects", "include-field") - const garden = await makeTestGarden(projectRoot) - const graph = await garden.getConfigGraph() - const module = await graph.getModule("module-a") - - expect(module.version.files).to.include(module.configPath) - }) -}) diff --git a/garden-service/test/unit/src/vcs/vcs.ts b/garden-service/test/unit/src/vcs/vcs.ts index 5cdf8577ba..78166ade8c 100644 --- a/garden-service/test/unit/src/vcs/vcs.ts +++ b/garden-service/test/unit/src/vcs/vcs.ts @@ -16,7 +16,7 @@ import { GitHandler } from "../../../../src/vcs/git" import { resolve, join } from "path" import * as td from "testdouble" import * as tmp from "tmp-promise" -import { realpath } from "fs-extra" +import { realpath, readFile, writeFile } from "fs-extra" import { GARDEN_VERSIONFILE_NAME } from "../../../../src/constants" class TestVcsHandler extends VcsHandler { @@ -27,8 +27,8 @@ class TestVcsHandler extends VcsHandler { return [] } - async getTreeVersion(path: string, include: string[] | null) { - return this.testVersions[path] || super.getTreeVersion(path, include) + async getTreeVersion(moduleConfig: ModuleConfig) { + return this.testVersions[moduleConfig.path] || super.getTreeVersion(moduleConfig) } setTestVersion(path: string, version: TreeVersion) { @@ -45,8 +45,8 @@ class TestVcsHandler extends VcsHandler { } describe("VcsHandler", () => { - let handler: TestVcsHandler - let garden: Garden + let handlerA: TestVcsHandler + let gardenA: Garden // note: module-a has a version file with this content const versionA = { @@ -55,67 +55,83 @@ describe("VcsHandler", () => { } beforeEach(async () => { - handler = new TestVcsHandler(projectRootA) - garden = await makeTestGardenA() + handlerA = new TestVcsHandler(projectRootA) + gardenA = await makeTestGardenA() }) describe("getTreeVersion", () => { it("should sort the list of files in the returned version", async () => { - const getFiles = td.replace(handler, "getFiles") - const path = "foo" - td.when(getFiles(path, undefined)).thenResolve([ + const getFiles = td.replace(handlerA, "getFiles") + const moduleConfig = await gardenA.resolveModuleConfig("module-a") + td.when(getFiles(moduleConfig.path, undefined)).thenResolve([ { path: "c", hash: "c" }, { path: "b", hash: "b" }, { path: "d", hash: "d" }, ]) - const version = await handler.getTreeVersion(path, null) + const version = await handlerA.getTreeVersion(moduleConfig) expect(version.files).to.eql(["b", "c", "d"]) }) + it("should not include the module config file in the file list", async () => { + const getFiles = td.replace(handlerA, "getFiles") + const moduleConfig = await gardenA.resolveModuleConfig("module-a") + td.when(getFiles(moduleConfig.path, undefined)).thenResolve([ + { path: moduleConfig.configPath, hash: "c" }, + { path: "b", hash: "b" }, + { path: "d", hash: "d" }, + ]) + const version = await handlerA.getTreeVersion(moduleConfig) + expect(version.files).to.eql(["b", "d"]) + }) + it("should respect the include field, if specified", async () => { const includeProjectRoot = getDataDir("test-projects", "include-field") const includeGarden = await makeTestGarden(includeProjectRoot) - const module = await includeGarden.resolveModuleConfig("module-a") + const moduleConfig = await includeGarden.resolveModuleConfig("module-a") const includeHandler = new GitHandler(includeGarden.gardenDirPath) - const withInclude = await includeHandler.getTreeVersion(module.path, module.include!) - const withoutInclude = await includeHandler.getTreeVersion(module.path, null) + const withInclude = await includeHandler.getTreeVersion(moduleConfig) + const configWithoutInclude = { ...moduleConfig, include: undefined } + const withoutInclude = await includeHandler.getTreeVersion(configWithoutInclude) expect(withInclude).to.eql({ contentHash: "6413e73ab3", files: [ - resolve(module.path, "yes.txt"), + resolve(moduleConfig.path, "yes.txt"), ], }) expect(withoutInclude).to.eql({ - contentHash: "80077a6c44", + contentHash: "6dfaa33e8f", files: [ - resolve(module.path, "garden.yml"), - resolve(module.path, "nope.txt"), - resolve(module.path, "yes.txt"), + resolve(moduleConfig.path, "nope.txt"), + resolve(moduleConfig.path, "yes.txt"), ], }) }) - it("should call getTreeVersion if there is no version file", async () => { - const module = await garden.resolveModuleConfig("module-b") - - const version = { - contentHash: "qwerty", - files: [], + it("should not be affected by changes to the module's garden.yml that don't affect the module config", async () => { + const projectRoot = getDataDir("test-projects", "multiple-module-config") + const garden = await makeTestGarden(projectRoot) + const moduleConfigA1 = await garden.resolveModuleConfig("module-a1") + const configPath = moduleConfigA1.configPath! + const orgConfig = await readFile(configPath) + + try { + const version1 = await garden.vcs.getTreeVersion(moduleConfigA1) + await writeFile(configPath, orgConfig + "\n---") + const version2 = await garden.vcs.getTreeVersion(moduleConfigA1) + expect(version1).to.eql(version2) + } finally { + await writeFile(configPath, orgConfig) } - handler.setTestVersion(module.path, version) - - const result = await handler.resolveTreeVersion(module.path, null) - expect(result).to.eql(version) }) }) describe("resolveTreeVersion", () => { it("should return the version from a version file if it exists", async () => { - const module = await garden.resolveModuleConfig("module-a") - const result = await handler.resolveTreeVersion(module.path, null) + const moduleConfig = await gardenA.resolveModuleConfig("module-a") + const result = await handlerA.resolveTreeVersion(moduleConfig) expect(result).to.eql({ contentHash: "1234567890", @@ -124,15 +140,15 @@ describe("VcsHandler", () => { }) it("should call getTreeVersion if there is no version file", async () => { - const module = await garden.resolveModuleConfig("module-b") + const moduleConfig = await gardenA.resolveModuleConfig("module-b") const version = { contentHash: "qwerty", files: [], } - handler.setTestVersion(module.path, version) + handlerA.setTestVersion(moduleConfig.path, version) - const result = await handler.resolveTreeVersion(module.path, null) + const result = await handlerA.resolveTreeVersion(moduleConfig) expect(result).to.eql(version) }) }) @@ -200,7 +216,7 @@ describe("VcsHandler", () => { describe("hashVersions", () => { it("is stable with respect to key order in moduleConfig", async () => { - const originalConfig = await garden.resolveModuleConfig("module-a") + const originalConfig = await gardenA.resolveModuleConfig("module-a") const stirredConfig = cloneDeep(originalConfig) delete stirredConfig.name stirredConfig.name = originalConfig.name @@ -210,7 +226,7 @@ describe("VcsHandler", () => { }) it("is stable with respect to named version order", async () => { - const config = await garden.resolveModuleConfig("module-a") + const config = await gardenA.resolveModuleConfig("module-a") expect(getVersionString(config, [namedVersionA, namedVersionB, namedVersionC])) .to.eql(getVersionString(config, [namedVersionB, namedVersionA, namedVersionC])) @@ -219,10 +235,9 @@ describe("VcsHandler", () => { }) describe("resolveVersion", () => { - it("should return module version if there are no dependencies", async () => { - const module = await garden.resolveModuleConfig("module-a") - const result = await handler.resolveVersion(module, []) + const module = await gardenA.resolveModuleConfig("module-a") + const result = await handlerA.resolveVersion(module, []) expect(result).to.eql({ versionString: getVersionString(module, [{ ...versionA, name: "module-a" }]), @@ -232,23 +247,23 @@ describe("VcsHandler", () => { }) it("should hash together the version of the module and all dependencies", async () => { - const [moduleA, moduleB, moduleC] = await garden.resolveModuleConfigs(["module-a", "module-b", "module-c"]) + const [moduleA, moduleB, moduleC] = await gardenA.resolveModuleConfigs(["module-a", "module-b", "module-c"]) const versionStringB = "qwerty" const versionB = { contentHash: versionStringB, files: [], } - handler.setTestVersion(moduleB.path, versionB) + handlerA.setTestVersion(moduleB.path, versionB) const versionStringC = "asdfgh" const versionC = { contentHash: versionStringC, files: [], } - handler.setTestVersion(moduleC.path, versionC) + handlerA.setTestVersion(moduleC.path, versionC) - expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ + expect(await handlerA.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ versionString: getVersionString(moduleC, [ { ...versionA, name: "module-a" }, { ...versionB, name: "module-b" }, @@ -261,6 +276,37 @@ describe("VcsHandler", () => { files: [], }) }) + + it("should not include module's garden.yml in version file list", async () => { + const moduleConfig = await gardenA.resolveModuleConfig("module-a") + const version = await handlerA.resolveVersion(moduleConfig, []) + expect(version.files).to.not.include(moduleConfig.configPath!) + }) + + it("should be affected by changes to the module's config", async () => { + const moduleConfig = await gardenA.resolveModuleConfig("module-a") + const version1 = await handlerA.resolveVersion(moduleConfig, []) + moduleConfig.name = "foo" + const version2 = await handlerA.resolveVersion(moduleConfig, []) + expect(version1).to.not.eql(version2) + }) + + it("should not be affected by changes to the module's garden.yml that don't affect the module config", async () => { + const projectRoot = getDataDir("test-projects", "multiple-module-config") + const garden = await makeTestGarden(projectRoot) + const moduleConfigA1 = await garden.resolveModuleConfig("module-a1") + const configPath = moduleConfigA1.configPath! + const orgConfig = await readFile(configPath) + + try { + const version1 = await garden.vcs.resolveVersion(moduleConfigA1, []) + await writeFile(configPath, orgConfig + "\n---") + const version2 = await garden.vcs.resolveVersion(moduleConfigA1, []) + expect(version1).to.eql(version2) + } finally { + await writeFile(configPath, orgConfig) + } + }) }) })