Skip to content

Commit

Permalink
fix: improved error messages when deps are missing (#484)
Browse files Browse the repository at this point in the history
Output a more informative error message when invalid/unknown
dependencies are present in a module's configuration.
  • Loading branch information
thsig authored and edvald committed Jan 29, 2019
1 parent 7d5d74c commit c5e6dce
Show file tree
Hide file tree
Showing 19 changed files with 341 additions and 105 deletions.
2 changes: 1 addition & 1 deletion examples/hello-world/services/hello-container/garden.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ module:
env:
FUNCTION_ENDPOINT: ${services.hello-function.outputs.endpoint}
dependencies:
- hello-function
- hello-function
52 changes: 14 additions & 38 deletions garden-service/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions garden-service/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ import {
} from "./config/base"
import { BaseTask } from "./tasks/base"
import { LocalConfigStore } from "./config-store"
import { detectCircularDependencies } from "./util/detectCycles"
import { validateDependencies } from "./util/validate-dependencies"
import {
getLinkedSources,
ExternalSourceType,
Expand Down Expand Up @@ -837,14 +837,17 @@ export class Garden {
const moduleConfigContext = new ModuleConfigContext(
this, this.log, this.environment, Object.values(this.moduleConfigs),
)
this.moduleConfigs = await resolveTemplateStrings(this.moduleConfigs, moduleConfigContext)

await this.detectCircularDependencies()
this.moduleConfigs = await resolveTemplateStrings(this.moduleConfigs, moduleConfigContext)
this.validateDependencies()
})
}

private async detectCircularDependencies() {
return detectCircularDependencies(Object.values(this.moduleConfigs))
private validateDependencies() {
validateDependencies(
Object.values(this.moduleConfigs),
Object.keys(this.serviceNameIndex),
Object.keys(this.taskNameIndex))
}

/*
Expand Down Expand Up @@ -937,7 +940,7 @@ export class Garden {

if (this.modulesScanned) {
// need to re-run this if adding modules after initial scan
await this.detectCircularDependencies()
await this.validateDependencies()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,115 @@
*/

import dedent = require("dedent")
import { merge } from "lodash"
import * as indentString from "indent-string"
import { get, isEqual, join, set, uniqWith } from "lodash"
import { getModuleKey } from "../types/module"
import { ConfigurationError } from "../exceptions"
import { ServiceConfig } from "../config/service"
import { TaskConfig } from "../config/task"
import { ModuleConfig } from "../config/module"
import { deline } from "./string"

export type Cycle = string[]
export function validateDependencies(
moduleConfigs: ModuleConfig[], serviceNames: string[], taskNames: string[],
): void {

/*
Implements a variation on the Floyd-Warshall algorithm to compute minimal cycles.
const missingDepsError = detectMissingDependencies(moduleConfigs, serviceNames, taskNames)
const circularDepsError = detectCircularDependencies(moduleConfigs)

let errMsg = ""
let detail = {}

if (missingDepsError) {
errMsg += missingDepsError.message
detail = merge(detail, missingDepsError.detail)
}

if (circularDepsError) {
errMsg += "\n" + circularDepsError.message
detail = merge(detail, circularDepsError.detail)
}

if (missingDepsError || circularDepsError) {
throw new ConfigurationError(errMsg, detail)
}

This is approximately O(m^3) + O(s^3), where m is the number of modules and s is the number of services.
}

/**
* Looks for dependencies on non-existent modules, services or tasks, and returns an error
* if any were found.
*/
export function detectMissingDependencies(
moduleConfigs: ModuleConfig[], serviceNames: string[], taskNames: string[],
): ConfigurationError | null {

const moduleNames: Set<string> = new Set(moduleConfigs.map(m => m.name))
const runtimeNames: Set<string> = new Set([...serviceNames, ...taskNames])
const missingDepDescriptions: string[] = []

const runtimeDepTypes = [
["serviceConfigs", "Service"],
["taskConfigs", "Task"],
["testConfigs", "Test"],
]

for (const m of moduleConfigs) {

const buildDepKeys = m.build.dependencies.map(d => getModuleKey(d.name, d.plugin))

for (const missingModule of buildDepKeys.filter(k => !moduleNames.has(k))) {
missingDepDescriptions.push(
`Module '${m.name}': Unknown module '${missingModule}' referenced in build dependencies.`,
)
}

Throws an error if cycles were found.
*/
export async function detectCircularDependencies(moduleConfigs: ModuleConfig[]) {
for (const [configKey, entityName] of runtimeDepTypes) {
for (const config of m[configKey]) {
for (const missingRuntimeDep of config.dependencies.filter(d => !runtimeNames.has(d))) {
missingDepDescriptions.push(deline`
${entityName} '${config.name}' (in module '${m.name}'): Unknown service or task '${missingRuntimeDep}'
referenced in dependencies.`,
)
}
}
}

}

if (missingDepDescriptions.length > 0) {
const errMsg = "Unknown dependencies detected.\n\n" +
indentString(missingDepDescriptions.join("\n\n"), 2) + "\n"

return new ConfigurationError(errMsg, { "unknown-dependencies": missingDepDescriptions })
} else {
return null
}

}

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.
*
* Returns an error if cycles were found.
*/
export function detectCircularDependencies(moduleConfigs: ModuleConfig[]): ConfigurationError | null {
// Sparse matrices
const buildGraph = {}
const runtimeGraph = {}
const services: ServiceConfig[] = []
const tasks: TaskConfig[] = []

/*
There's no need to account for test dependencies here, since any circularities there
are accounted for via service dependencies.
*/
/**
* Since dependencies listed in test configs cannot introduce circularities (because
* builds/deployments/tasks/tests cannot currently depend on tests), we don't need to
* account for test dependencies here.
*/
for (const module of moduleConfigs) {
// Build dependencies
for (const buildDep of module.build.dependencies) {
Expand Down Expand Up @@ -83,8 +165,10 @@ export async function detectCircularDependencies(moduleConfigs: ModuleConfig[])
detail["circular-service-or-task-dependencies"] = runtimeCyclesDescription
}

throw new ConfigurationError(errMsg, detail)
return new ConfigurationError(errMsg, detail)
}

return null
}

export function detectCycles(graph, vertices: string[]): Cycle[] {
Expand Down
Loading

0 comments on commit c5e6dce

Please sign in to comment.