Merge pull request #545 from garden-io/add-prefix-to-version-string

fix(templates): add prefix to versionString
garden-io · Feb 19, 2019 · 0b859ce · 0b859ce
2 parents da42b15 + fe9cd49
commit 0b859ce
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 14 deletions.
diff --git 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
@@ -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<any> {
@@ -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
@@ -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
@@ -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
@@ -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,