From fe9cd49c35054294ab3d9040a97ace360d562114 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Mon, 18 Feb 2019 14:50:06 +0100 Subject: [PATCH] fix(templates): add prefix to versionString Added the "v-" prefix to the versionString-s generated by VcsHandler.resolveVersion. This should resolve an issue where module versions would be read as numbers when used as template variables when the first several characters of the commit hash were numbers. --- garden-service/src/config/config-context.ts | 2 +- garden-service/src/vcs/base.ts | 20 +++++++++++++++----- garden-service/test/helpers.ts | 2 +- garden-service/test/src/commands/deploy.ts | 2 +- garden-service/test/src/vcs/base.ts | 12 ++++++------ 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/garden-service/src/config/config-context.ts b/garden-service/src/config/config-context.ts index 8006138847..b1b597b079 100644 --- a/garden-service/src/config/config-context.ts +++ b/garden-service/src/config/config-context.ts @@ -188,7 +188,7 @@ class EnvironmentContext extends ConfigContext { } const exampleOutputs = { endpoint: "http://my-service/path/to/endpoint" } -const exampleVersion = "v17ad4cb3fd" +const exampleVersion = "v-v17ad4cb3fd" class ModuleContext extends ConfigContext { @schema( diff --git a/garden-service/src/vcs/base.ts b/garden-service/src/vcs/base.ts index 3478ff0dfc..840e8613c2 100644 --- a/garden-service/src/vcs/base.ts +++ b/garden-service/src/vcs/base.ts @@ -43,6 +43,7 @@ interface NamedTreeVersion extends TreeVersion { } const versionStringSchema = Joi.string() + .regex(/^v/) .required() .description("String representation of the module version.") @@ -136,7 +137,7 @@ export abstract class VcsHandler { if (latestDirty.length > 1) { // if the last modified timestamp is common across multiple modules, hash their versions - const versionString = `${hashVersions(latestDirty)}-${dirtyTimestamp}` + const versionString = addVersionPrefix(`${hashVersions(latestDirty)}-${dirtyTimestamp}`) return { versionString, @@ -153,7 +154,7 @@ export abstract class VcsHandler { } } else { // otherwise derive the version from all the modules - const versionString = hashVersions(allVersions) + const versionString = addVersionPrefix(hashVersions(allVersions)) return { versionString, @@ -175,8 +176,7 @@ export abstract class VcsHandler { function hashVersions(versions: NamedTreeVersion[]) { const versionHash = createHash("sha256") versionHash.update(versions.map(v => `${v.name}_${v.latestCommit}`).join(".")) - // this format is kinda arbitrary, but prefixing the "v" is useful to visually spot hashed versions - return "v" + versionHash.digest("hex").slice(0, 10) + return versionHash.digest("hex").slice(0, 10) } async function readVersionFile(path: string, schema): Promise { @@ -222,7 +222,17 @@ export async function writeModuleVersionFile(path: string, version: ModuleVersio } export function getVersionString(treeVersion: TreeVersion) { - return treeVersion.dirtyTimestamp + const rawVersion = treeVersion.dirtyTimestamp ? `${treeVersion.latestCommit}-${treeVersion.dirtyTimestamp}` : treeVersion.latestCommit + return addVersionPrefix(rawVersion) +} + +/** + * We prefix with "v-" to prevent this.version from being read as a number when only a prefix of the + * commit hash is used, and that prefix consists of only numbers. This can cause errors in certain contexts + * when the version string is used in template variables in configuration files. + */ +export function addVersionPrefix(versionString: string) { + return `v-${versionString}` } diff --git a/garden-service/test/helpers.ts b/garden-service/test/helpers.ts index 4d248a41b8..4b0db93d54 100644 --- a/garden-service/test/helpers.ts +++ b/garden-service/test/helpers.ts @@ -45,7 +45,7 @@ import timekeeper = require("timekeeper") export const dataDir = resolve(__dirname, "data") export const examplesDir = resolve(__dirname, "..", "..", "examples") export const testNow = new Date() -export const testModuleVersionString = "1234512345" +export const testModuleVersionString = "v-1234512345" export const testModuleVersion: ModuleVersion = { versionString: testModuleVersionString, dirtyTimestamp: null, diff --git a/garden-service/test/src/commands/deploy.ts b/garden-service/test/src/commands/deploy.ts index 504a99a3f7..eb0e1ceeae 100644 --- a/garden-service/test/src/commands/deploy.ts +++ b/garden-service/test/src/commands/deploy.ts @@ -20,7 +20,7 @@ const placeholderTaskResult = (moduleName, taskName, command) => ({ taskName, command, version: { - versionString: "1", + versionString: "v-1", dirtyTimestamp: null, dependencyVersions: {}, }, diff --git a/garden-service/test/src/vcs/base.ts b/garden-service/test/src/vcs/base.ts index b84917aad3..d226f2263c 100644 --- a/garden-service/test/src/vcs/base.ts +++ b/garden-service/test/src/vcs/base.ts @@ -74,7 +74,7 @@ describe("VcsHandler", () => { const result = await handler.resolveVersion(module, []) expect(result).to.eql({ - versionString: versionA.latestCommit, + versionString: `v-${versionA.latestCommit}`, dirtyTimestamp: null, dependencyVersions: {}, }) @@ -93,7 +93,7 @@ describe("VcsHandler", () => { const result = await handler.resolveVersion(module, []) expect(result).to.eql({ - versionString: "abcdef-1234", + versionString: "v-abcdef-1234", dirtyTimestamp: 1234, dependencyVersions: {}, }) @@ -116,7 +116,7 @@ describe("VcsHandler", () => { handler.setTestVersion(moduleC.path, versionC) expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ - versionString: "asdfgh-123", + versionString: "v-asdfgh-123", dirtyTimestamp: 123, dependencyVersions: { "module-a": versionA, @@ -142,7 +142,7 @@ describe("VcsHandler", () => { handler.setTestVersion(moduleC.path, versionC) expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ - versionString: "qwerty-456", + versionString: "v-qwerty-456", dirtyTimestamp: 456, dependencyVersions: { "module-a": versionA, @@ -169,7 +169,7 @@ describe("VcsHandler", () => { handler.setTestVersion(moduleC.path, versionC) expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ - versionString: "v5ff3a146d9", + versionString: "v-5ff3a146d9", dirtyTimestamp: null, dependencyVersions: { "module-a": versionA, @@ -199,7 +199,7 @@ describe("VcsHandler", () => { handler.setTestVersion(moduleC.path, versionC) expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({ - versionString: "vcfa6d28ec5-1234", + versionString: "v-cfa6d28ec5-1234", dirtyTimestamp: 1234, dependencyVersions: { "module-a": versionA,