fix(container): use configured image ID locally

garden-io · Mar 8, 2019 · bf5d428 · bf5d428
1 parent 85c1026
commit bf5d428
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 12 deletions.
diff --git a/garden-service/src/plugins/container/container.ts b/garden-service/src/plugins/container/container.ts
@@ -126,18 +126,18 @@ export async function configureContainerModule({ ctx, moduleConfig }: ConfigureM
 
   const hasDockerfile = await pathExists(containerHelpers.getDockerfilePathFromConfig(moduleConfig))
 
+  // make sure we can build the thing
   if (moduleConfig.spec.dockerfile && !hasDockerfile) {
     throw new ConfigurationError(
       `Dockerfile not found at specififed path ${moduleConfig.spec.dockerfile} for module ${moduleConfig.name}`,
       {},
     )
   }
 
-  // make sure we can build the thing
   if (!moduleConfig.spec.image && !hasDockerfile) {
     throw new ConfigurationError(
       `Module ${moduleConfig.name} neither specifies image nor provides Dockerfile`,
-      {},
+      { moduleConfig },
     )
   }
 

diff --git a/garden-service/src/plugins/container/helpers.ts b/garden-service/src/plugins/container/helpers.ts
@@ -11,7 +11,7 @@ import { join } from "path"
 import { ConfigurationError } from "../../exceptions"
 import { splitFirst, spawn } from "../../util/util"
 import { ModuleConfig } from "../../config/module"
-import { ContainerModule, ContainerRegistryConfig, defaultTag, defaultNamespace } from "./config"
+import { ContainerModule, ContainerRegistryConfig, defaultTag } from "./config"
 
 interface ParsedImageId {
   host?: string
@@ -35,11 +35,22 @@ export const containerHelpers = {
    * (when we don't need to push to remote registries).
    */
   async getLocalImageId(module: ContainerModule): Promise<string> {
-    if (await containerHelpers.hasDockerfile(module)) {
+    const hasDockerfile = await containerHelpers.hasDockerfile(module)
+
+    if (module.spec.image && hasDockerfile) {
+      const { versionString } = module.version
+      const parsedImage = containerHelpers.parseImageId(module.spec.image || module.name)
+      return containerHelpers.unparseImageId({ ...parsedImage, tag: versionString })
+    } else if (!module.spec.image && hasDockerfile) {
       const { versionString } = module.version
-      return `${module.name}:${versionString}`
+      return containerHelpers.unparseImageId({ repository: module.name, tag: versionString })
+    } else if (module.spec.image && !hasDockerfile) {
+      return module.spec.image
     } else {
-      return module.spec.image!
+      throw new ConfigurationError(
+        `Module ${module.name} neither specifies image nor provides Dockerfile`,
+        { module },
+      )
     }
   },
 
@@ -98,21 +109,27 @@ export const containerHelpers = {
 
     if (parts.length === 1) {
       return {
-        namespace: defaultNamespace,
         repository,
         tag,
       }
     } else if (parts.length === 2) {
       return {
         namespace: parts[0],
-        repository,
+        repository: parts[1],
         tag,
       }
     } else if (parts.length === 3) {
       return {
         host: parts[0],
         namespace: parts[1],
-        repository,
+        repository: parts[2],
+        tag,
+      }
+    } else if (parts.length > 3) {
+      return {
+        host: parts[0],
+        namespace: parts.slice(1, parts.length - 1).join("/"),
+        repository: parts[parts.length - 1],
         tag,
       }
     } else {

diff --git a/garden-service/test/src/plugins/container.ts b/garden-service/test/src/plugins/container.ts
@@ -77,17 +77,17 @@ describe("plugins.container", () => {
   }
 
   describe("getLocalImageId", () => {
-    it("should create identifier with commit hash version if module has a Dockerfile", async () => {
+    it("should return configured image name with local version if module has a Dockerfile", async () => {
       const config = cloneDeep(baseConfig)
       config.spec.image = "some/image:1.1"
       const module = await getTestModule(config)
 
       td.replace(containerHelpers, "hasDockerfile", async () => true)
 
-      expect(await containerHelpers.getLocalImageId(module)).to.equal("test:1234")
+      expect(await containerHelpers.getLocalImageId(module)).to.equal("some/image:1234")
     })
 
-    it("should create identifier with image name if module has no Dockerfile", async () => {
+    it("should return configured image name and tag if module has no Dockerfile and name includes tag", async () => {
       const config = cloneDeep(baseConfig)
       config.spec.image = "some/image:1.1"
       const module = await getTestModule(config)
@@ -96,6 +96,34 @@ describe("plugins.container", () => {
 
       expect(await containerHelpers.getLocalImageId(module)).to.equal("some/image:1.1")
     })
+
+    it("should return configured image with local version if module has Dockerfile and name has no tag", async () => {
+      const config = cloneDeep(baseConfig)
+      config.spec.image = "some/image"
+      const module = await getTestModule(config)
+
+      td.replace(containerHelpers, "hasDockerfile", async () => true)
+
+      expect(await containerHelpers.getLocalImageId(module)).to.equal("some/image:1234")
+    })
+
+    it("should return module name with local version if there is no configured name", async () => {
+      const config = cloneDeep(baseConfig)
+      const module = await getTestModule(config)
+
+      td.replace(containerHelpers, "hasDockerfile", async () => true)
+
+      expect(await containerHelpers.getLocalImageId(module)).to.equal("test:1234")
+    })
+
+    it("should throw if there is no Dockerfile and no image specified", async () => {
+      const config = cloneDeep(baseConfig)
+      const module = await getTestModule(config)
+
+      td.replace(containerHelpers, "hasDockerfile", async () => false)
+
+      await expectError(() => containerHelpers.getLocalImageId(module), "configuration")
+    })
   })
 
   describe("getDockerfilePathFromModule", () => {