feat: Detect circular dependencies.

Added a check for circular build or service dependencies to the
scanModules method of the Garden class.

src/garden.ts

@@ -95,6 +95,7 @@ import { Task } from "./types/task"
 import {
   LocalConfigStore,
 } from "./config-store"
+import { detectCircularDependencies } from "./util/detectCycles"
 
 export interface ModuleMap<T extends Module> {
   [key: string]: T
@@ -571,7 +572,7 @@ export class Garden {
   }
 
   /*
-    Scans the project root for modules and adds them to the context
+    Scans the project root for modules and adds them to the context.
    */
   async scanModules() {
     const ignorer = getIgnorer(this.projectRoot)
@@ -603,6 +604,10 @@ export class Garden {
     }
 
     this.modulesScanned = true
+
+    await detectCircularDependencies(
+      await this.getModules(),
+      (await this.getServices()).map(s => s.name))
   }
 
   /*

src/plugins/kubernetes/kubectl.ts

@@ -47,7 +47,7 @@ export class Kubectl {
     args: string[],
     { data, ignoreError = false, silent = true, timeout = KUBECTL_DEFAULT_TIMEOUT }: KubectlParams = {},
   ): Promise<KubectlOutput> {
-    // TODO: use the spawn helper from util.ts
+    // TODO: use the spawn helper from index.ts
     const logger = getLogger()
     const out: KubectlOutput = {
       code: 0,

src/util/detectCycles.ts

@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2018 Garden Technologies, Inc. <[email protected]>
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import { forEach, get, isEqual, join, set, uniqWith } from "lodash"
+import { Module, ModuleConfig } from "../types/module"
+import { ConfigurationError } from "../exceptions"
+
+export type Cycle = string[]
+
+/*
+  Implements a variation on the Floyd-Warshall algorithm to compute minimal cycles.
+
+  This is approximately O(m^3) + O(s^3), where m is the number of modules and s is the number of services.
+
+  Throws an error if cycles were found.
+*/
+export async function detectCircularDependencies(modules: Module[], serviceNames: string[]) {
+
+  // Sparse matrices
+  const buildGraph = {}
+  const serviceGraph = {}
+
+  /*
+    There's no need to account for test dependencies here, since any circularities there
+    are accounted for via service dependencies.
+    */
+  for (const module of modules) {
+    const conf: ModuleConfig = await module.getConfig()
+
+    // Build dependencies
+    for (const buildDep of get(conf, ["build", "dependencies"], [])) {
+      const depName = buildDep.name
+      set(buildGraph, [module.name, depName], { distance: 1, next: depName })
+    }
+
+    // Service dependencies
+    forEach(get(conf, ["services"], {}), (serviceConfig, serviceName) => {
+      for (const depName of get(serviceConfig, ["dependencies"], [])) {
+        set(serviceGraph, [serviceName, depName], { distance: 1, next: depName })
+      }
+    })
+
+  }
+
+  const buildCycles = detectCycles(buildGraph, modules.map(m => m.name))
+  const serviceCycles = detectCycles(serviceGraph, serviceNames)
+
+  if (buildCycles.length > 0 || serviceCycles.length > 0) {
+    const detail = {}
+
+    if (buildCycles.length > 0) {
+      detail["circular-build-dependencies"] = cyclesToString(buildCycles)
+    }
+
+    if (serviceCycles.length > 0) {
+      detail["circular-service-dependencies"] = cyclesToString(serviceCycles)
+    }
+
+    throw new ConfigurationError("Circular dependencies detected", detail)
+  }
+}
+
+export function detectCycles(graph, vertices: string[]): Cycle[] {
+  // Compute shortest paths
+  for (const k of vertices) {
+    for (const i of vertices) {
+      for (const j of vertices) {
+        const distanceViaK: number = distance(graph, i, k) + distance(graph, k, j)
+        if (distanceViaK < distance(graph, i, j)) {
+          const nextViaK = next(graph, i, k)
+          set(graph, [i, j], { distance: distanceViaK, next: nextViaK })
+        }
+      }
+    }
+  }
+
+  // Reconstruct cycles, if any
+  const cycleVertices = vertices.filter(v => next(graph, v, v))
+  const cycles: Cycle[] = cycleVertices.map(v => {
+    const cycle = [v]
+    let nextInCycle = next(graph, v, v)
+    while (nextInCycle !== v) {
+      cycle.push(nextInCycle)
+      nextInCycle = next(graph, nextInCycle, v)
+    }
+    return cycle
+  })
+
+  return uniqWith(
+    cycles, // The concat calls below are to prevent in-place sorting.
+    (c1, c2) => isEqual(c1.concat().sort(), c2.concat().sort()))
+}
+
+function distance(graph, source, destination): number {
+  return get(graph, [source, destination, "distance"], Infinity)
+}
+
+function next(graph, source, destination): string {
+  return get(graph, [source, destination, "next"])
+}
+
+function cyclesToString(cycles: Cycle[]) {
+  const cycleDescriptions = cycles.map(c => join(c.concat([c[0]]), " <- "))
+  return cycleDescriptions.length === 1 ? cycleDescriptions[0] : cycleDescriptions
+}

src/util.ts → src/util/index.ts

@@ -19,16 +19,16 @@ import { spawn as _spawn } from "child_process"
 import { existsSync, readFileSync, writeFileSync } from "fs"
 import { join } from "path"
 import { find } from "lodash"
-import { getLogger, RootLogNode } from "./logger"
+import { getLogger, RootLogNode } from "../logger/index"
 import {
   TimeoutError,
   GardenError,
-} from "./exceptions"
+} from "../exceptions"
 import { PassThrough } from "stream"
 import { isArray, isPlainObject, extend, mapValues, pickBy } from "lodash"
 import highlight from "cli-highlight"
 import chalk from "chalk"
-import { FancyConsoleWriter } from "./logger/writers"
+import { FancyConsoleWriter } from "../logger/writers"
 import hasAnsi = require("has-ansi")
 
 // shim to allow async generator functions

test/data/test-project-circular-deps/garden.yml

@@ -0,0 +1,6 @@
+project:
+  name: test-project-b
+  environments:
+    - name: local
+      providers:
+        - name: test-plugin

test/data/test-project-circular-deps/module-a/garden.yml

@@ -0,0 +1,14 @@
+module:
+  name: module-a
+  type: generic
+  services:
+    - name: service-a
+      endpoints:
+        - paths: [/path-a]
+          containerPort: 8080
+      dependencies:
+        - service-c
+  build:
+    command: echo A
+    dependencies:
+      - module-c

test/data/test-project-circular-deps/module-b/garden.yml

@@ -0,0 +1,15 @@
+module:
+  name: module-b
+  type: generic
+  services:
+    - name: service-b:
+      endpoints:
+        - paths: [/path-b]
+          containerPort: 8080
+      dependencies:
+        - service-a
+        - service-c
+  build:
+    command: echo B
+    dependencies:
+      - module-a

test/data/test-project-circular-deps/module-c/garden.yml

@@ -0,0 +1,14 @@
+module:
+  name: module-c
+  type: generic
+  allowPush: false
+  services:
+    - name: service-c:
+      endpoints:
+        - paths: [/path-c]
+          containerPort: 8080
+      dependencies:
+        - service-b
+  build:
+    dependencies:
+      - module-b

test/src/garden.ts

@@ -1,6 +1,7 @@
+import { expect } from "chai"
 import { join } from "path"
 import { Garden } from "../../src/garden"
-import { expect } from "chai"
+import { detectCycles } from "../../src/util/detectCycles"
 import {
   dataDir,
   expectError,
@@ -226,9 +227,45 @@ describe("Garden", () => {
       const garden = await makeTestGardenA()
       await garden.scanModules()
 
-      const modules = await garden.getModules(undefined, true)
+    const modules = await garden.getModules(undefined, true)
       expect(getNames(modules)).to.eql(["module-a", "module-b", "module-c"])
     })
+
+    describe("detectCircularDependencies", () => {
+      it("should throw an exception when circular dependencies are present", async () => {
+        const circularProjectRoot = join(__dirname, "..", "data", "test-project-circular-deps")
+        const garden = await makeTestGarden(circularProjectRoot)
+        await expectError(
+          async () => await garden.scanModules(),
+          "configuration")
+      })
+
+      it("should not throw an exception when no circular dependencies are present", async () => {
+        const nonCircularProjectRoot = join(__dirname, "..", "data", "test-project-b")
+        const garden = await makeTestGarden(nonCircularProjectRoot)
+        expect(async () => { await garden.scanModules() }).to.not.throw()
+      })
+    })
+
+    describe("detectCycles", () => {
+      it("should detect self-to-self cycles", () => {
+        const cycles = detectCycles({
+          a: {a: {distance: 1, next: "a"}},
+        }, ["a"])
+
+        expect(cycles).to.deep.eq([["a"]])
+      })
+
+      it("should preserve dependency order when returning cycles", () => {
+        const cycles = detectCycles({
+          foo: {bar: {distance: 1, next: "bar"}},
+          bar: {baz: {distance: 1, next: "baz"}},
+          baz: {foo: {distance: 1, next: "foo"}},
+        }, ["foo", "bar", "baz"])
+
+        expect(cycles).to.deep.eq([["foo", "bar", "baz"]])
+      })
+    })
   })
 
   describe("addModule", () => {

test/src/util.ts

@@ -1,6 +1,6 @@
-import { scanDirectory } from "../../src/util"
-import { join } from "path"
 import { expect } from "chai"
+import { join } from "path"
+import { scanDirectory } from "../../src/util"
 
 describe("util", () => {
   describe("scanDirectory", () => {
@@ -33,4 +33,5 @@ describe("util", () => {
       expect(count).to.eq(3)
     })
   })
+
 })