fix(container): fix hot reload target validation

Fixes the logic that detects overlapping hot reload sync targets.
garden-io · Aug 10, 2021 · 2d47136 · 2d47136
1 parent 708f4c3
commit 2d47136
Show file tree

Hide file tree

Showing 3 changed files with 137 additions and 93 deletions.
diff --git a/core/src/plugins/container/container.ts b/core/src/plugins/container/container.ts
@@ -23,6 +23,7 @@ import { listDirectory } from "../../util/fs"
 import { dedent } from "../../util/string"
 import { getModuleTypeUrl } from "../../docs/common"
 import { Provider, GenericProviderConfig, providerConfigBaseSchema } from "../../config/provider"
+import { isSubdir } from "../../util/util"
 
 export interface ContainerProviderConfig extends GenericProviderConfig {}
 export type ContainerProvider = Provider<ContainerProviderConfig>
@@ -77,7 +78,7 @@ export async function configureContainerModule({ log, moduleConfig }: ConfigureM
     // another target. Mounting directories into mounted directories will cause unexpected results
     for (const t of targets) {
       for (const t2 of targets) {
-        if (t2.startsWith(t) && t !== t2) {
+        if (isSubdir(t2, t) && t !== t2) {
           invalidPairDescriptions.push(`${t} is a subdirectory of ${t2}.`)
         }
       }

diff --git a/core/src/util/util.ts b/core/src/util/util.ts
@@ -26,6 +26,7 @@ import { Writable, Readable } from "stream"
 import { LogEntry } from "../logger/log-entry"
 import execa = require("execa")
 import { PrimitiveMap } from "../config/common"
+import { isAbsolute, relative } from "path"
 export { v4 as uuidv4 } from "uuid"
 
 export type HookCallback = (callback?: () => void) => void
@@ -617,6 +618,14 @@ export function isTruthy<T>(value: T | undefined | null | false | 0 | ""): value
   return !!value
 }
 
+/**
+ * Returns `true` if `path` is a subdirectory of `ofPath`. Returns `false` otherwise.
+ */
+export function isSubdir(path: string, ofPath: string): boolean {
+  const rel = relative(path, ofPath)
+  return !!(rel && !rel.startsWith("..") && !isAbsolute(rel))
+}
+
 // Used to make the platforms more consistent with other tools
 const platformMap = {
   win32: "windows",

diff --git a/core/test/unit/src/plugins/container/container.ts b/core/test/unit/src/plugins/container/container.ts
@@ -206,110 +206,110 @@ describe("plugins.container", () => {
   })
 
   describe("configureContainerModule", () => {
-    it("should validate and parse a container module", async () => {
-      const moduleConfig: ContainerModuleConfig = {
-        allowPublish: false,
+    const containerModuleConfig: ContainerModuleConfig = {
+      allowPublish: false,
+      build: {
+        dependencies: [],
+      },
+      disabled: false,
+      apiVersion: "garden.io/v0",
+      name: "module-a",
+      path: modulePath,
+      type: "container",
+
+      spec: {
         build: {
           dependencies: [],
+          timeout: DEFAULT_BUILD_TIMEOUT,
         },
-        disabled: false,
-        apiVersion: "garden.io/v0",
-        name: "module-a",
-        path: modulePath,
-        type: "container",
-
-        spec: {
-          build: {
+        buildArgs: {},
+        extraFlags: [],
+        services: [
+          {
+            name: "service-a",
+            annotations: {},
+            args: ["echo"],
             dependencies: [],
-            timeout: DEFAULT_BUILD_TIMEOUT,
-          },
-          buildArgs: {},
-          extraFlags: [],
-          services: [
-            {
-              name: "service-a",
-              annotations: {},
-              args: ["echo"],
-              dependencies: [],
-              daemon: false,
-              disabled: false,
-              ingresses: [
-                {
-                  annotations: {},
-                  path: "/",
-                  port: "http",
-                },
-              ],
-              env: {
-                SOME_ENV_VAR: "value",
-              },
-              healthCheck: {
-                httpGet: {
-                  path: "/health",
-                  port: "http",
-                },
-                livenessTimeoutSeconds: 10,
-                readinessTimeoutSeconds: 10,
-              },
-              limits: {
-                cpu: 123,
-                memory: 456,
+            daemon: false,
+            disabled: false,
+            ingresses: [
+              {
+                annotations: {},
+                path: "/",
+                port: "http",
               },
-              cpu: defaultCpu,
-              memory: defaultMemory,
-              ports: [
-                {
-                  name: "http",
-                  protocol: "TCP",
-                  containerPort: 8080,
-                  servicePort: 8080,
-                },
-              ],
-              replicas: 1,
-              volumes: [],
+            ],
+            env: {
+              SOME_ENV_VAR: "value",
             },
-          ],
-          tasks: [
-            {
-              name: "task-a",
-              args: ["echo", "OK"],
-              artifacts: [],
-              cacheResult: true,
-              dependencies: [],
-              disabled: false,
-              env: {
-                TASK_ENV_VAR: "value",
+            healthCheck: {
+              httpGet: {
+                path: "/health",
+                port: "http",
               },
-              cpu: defaultCpu,
-              memory: defaultMemory,
-              timeout: null,
-              volumes: [],
+              livenessTimeoutSeconds: 10,
+              readinessTimeoutSeconds: 10,
             },
-          ],
-          tests: [
-            {
-              name: "unit",
-              args: ["echo", "OK"],
-              artifacts: [],
-              dependencies: [],
-              disabled: false,
-              env: {
-                TEST_ENV_VAR: "value",
+            limits: {
+              cpu: 123,
+              memory: 456,
+            },
+            cpu: defaultCpu,
+            memory: defaultMemory,
+            ports: [
+              {
+                name: "http",
+                protocol: "TCP",
+                containerPort: 8080,
+                servicePort: 8080,
               },
-              cpu: defaultCpu,
-              memory: defaultMemory,
-              timeout: null,
-              volumes: [],
+            ],
+            replicas: 1,
+            volumes: [],
+          },
+        ],
+        tasks: [
+          {
+            name: "task-a",
+            args: ["echo", "OK"],
+            artifacts: [],
+            cacheResult: true,
+            dependencies: [],
+            disabled: false,
+            env: {
+              TASK_ENV_VAR: "value",
             },
-          ],
-        },
+            cpu: defaultCpu,
+            memory: defaultMemory,
+            timeout: null,
+            volumes: [],
+          },
+        ],
+        tests: [
+          {
+            name: "unit",
+            args: ["echo", "OK"],
+            artifacts: [],
+            dependencies: [],
+            disabled: false,
+            env: {
+              TEST_ENV_VAR: "value",
+            },
+            cpu: defaultCpu,
+            memory: defaultMemory,
+            timeout: null,
+            volumes: [],
+          },
+        ],
+      },
 
-        serviceConfigs: [],
-        taskConfigs: [],
-        testConfigs: [],
-      }
+      serviceConfigs: [],
+      taskConfigs: [],
+      testConfigs: [],
+    }
 
-      const result = await configure({ ctx, moduleConfig, log })
+    it("should validate and parse a container module", async () => {
+      const result = await configure({ ctx, moduleConfig: containerModuleConfig, log })
 
       expect(result).to.eql({
         moduleConfig: {
@@ -492,6 +492,40 @@ describe("plugins.container", () => {
       })
     })
 
+    context("hot reloading", () => {
+      it("should pass if no target is a subdirectory of another target", async () => {
+        const moduleConfig: ContainerModuleConfig = {
+          ...containerModuleConfig,
+          spec: {
+            ...containerModuleConfig.spec,
+            hotReload: {
+              sync: [
+                { source: "./foo_bar", target: "/home/somedir/foo_bar" },
+                { source: "./bar", target: "/home/somedir/bar" },
+              ],
+            },
+          },
+        }
+        await configure({ ctx, moduleConfig, log })
+      })
+
+      it("should throw if a target is a subdirectory of another target", async () => {
+        const moduleConfig: ContainerModuleConfig = {
+          ...containerModuleConfig,
+          spec: {
+            ...containerModuleConfig.spec,
+            hotReload: {
+              sync: [
+                { source: "foo", target: "/somedir/" },
+                { source: "bar", target: "/somedir/bar" },
+              ],
+            },
+          },
+        }
+        await expectError(() => configure({ ctx, moduleConfig, log }))
+      })
+    })
+
     it("should add service volume modules as build and runtime dependencies", async () => {
       const moduleConfig: ContainerModuleConfig = {
         allowPublish: false,