fix(k8s): containerModule refs in helm modules not creating build deps

This resolves an internal issue where module configure handlers were not able to reliably add a build dependency. This caused problems with another PR that I have in progress, but could also cause issues when using helm and kubernetes modules referencing container modules. The solution was to no longer use the TaskGraph for module resolution, and instead use a more bespoke flow that could handle dynamic addition of dependencies during resolution. Existing tests should mostly handle this. I just added one test to validate the specific case of adding a build dependency in a configure handler.
garden-io · Nov 24, 2020 · 3fdd0e5 · 3fdd0e5
1 parent 2709fab
commit 3fdd0e5
Show file tree

Hide file tree

Showing 8 changed files with 496 additions and 518 deletions.
diff --git a/core/src/garden.ts b/core/src/garden.ts
@@ -45,8 +45,8 @@ import { BaseTask } from "./tasks/base"
 import { LocalConfigStore, ConfigStore, GlobalConfigStore, LinkedSource } from "./config-store"
 import { getLinkedSources, ExternalSourceType } from "./util/ext-source-util"
 import { BuildDependencyConfig, ModuleConfig } from "./config/module"
-import { resolveModuleConfig } from "./resolve-module"
-import { ModuleConfigContext, OutputConfigContext, DefaultEnvironmentContext } from "./config/config-context"
+import { ModuleResolver, moduleResolutionConcurrencyLimit } from "./resolve-module"
+import { OutputConfigContext, DefaultEnvironmentContext } from "./config/config-context"
 import { createPluginContext, CommandInfo } from "./plugin-context"
 import { ModuleAndRuntimeActionHandlers, RegisterPluginParam } from "./types/plugin/plugin"
 import { SUPPORTED_PLATFORMS, SupportedPlatform, DEFAULT_GARDEN_DIR_NAME } from "./constants"
@@ -74,9 +74,8 @@ import { RuntimeContext } from "./runtime-context"
 import { loadPlugins, getDependencyOrder, getModuleTypes } from "./plugins"
 import { deline, naturalList } from "./util/string"
 import { ensureConnected } from "./db/connection"
-import { cyclesToString, DependencyValidationGraph } from "./util/validate-dependencies"
+import { DependencyValidationGraph } from "./util/validate-dependencies"
 import { Profile } from "./util/profiling"
-import { ResolveModuleTask, getResolvedModules, moduleResolutionConcurrencyLimit } from "./tasks/resolve-module"
 import username from "username"
 import { throwOnMissingSecretKeys, resolveTemplateString } from "./template-string"
 import { WorkflowConfig, WorkflowConfigMap, resolveWorkflowConfig } from "./config/workflow"
@@ -752,48 +751,21 @@ export class Garden {
    * For long-running processes, you need to call this again when any module or configuration has been updated.
    */
   async getConfigGraph(log: LogEntry, runtimeContext?: RuntimeContext) {
-    const providers = await this.resolveProviders(log)
-    const configs = await this.getRawModuleConfigs()
-    log.silly(`Resolving module configs`)
-    // Resolve the project module configs
-    const tasks = configs.map(
-      (moduleConfig) =>
-        new ResolveModuleTask({ garden: this, log, moduleConfig, resolvedProviders: providers, runtimeContext })
-    )
-
-    let results: GraphResults
-
-    try {
-      results = await this.processTasks(tasks)
-    } catch (err) {
-      // Wrap the circular dependency error to print a more specific message
-      if (err.type === "circular-dependencies") {
-        // Get the module names from the cycle keys (anything else is internal detail as far as users are concerned)
-        const cycleDescription = cyclesToString([err.detail.cycle.map((key: string) => key.split(".")[1])])
-        throw new ConfigurationError(
-          dedent`
-            Detected circular dependencies between module configurations:
-
-            ${cycleDescription}
-          `,
-          { cycle: err.detail.cycle }
-        )
-      } else {
-        throw err
-      }
-    }
+    const resolvedProviders = await this.resolveProviders(log)
+    const rawConfigs = await this.getRawModuleConfigs()
 
-    const failed = Object.values(results).filter((r) => r?.error)
+    log.silly(`Resolving module configs`)
 
-    if (failed.length > 0) {
-      const errors = failed.map((r) => `${chalk.white.bold(r!.name)}: ${r?.error?.message}`)
+    // Resolve the project module configs
+    const resolver = new ModuleResolver({
+      garden: this,
+      log,
+      rawConfigs,
+      resolvedProviders,
+      runtimeContext,
+    })
 
-      throw new ConfigurationError(chalk.red(`Failed resolving one or more modules:\n\n${errors.join("\n")}`), {
-        results,
-        errors,
-      })
-    }
-    const resolvedModules = getResolvedModules(results)
+    const resolvedModules = await resolver.resolveAll()
 
     const actions = await this.getActionRouter()
     const moduleTypes = await this.getModuleTypes()
@@ -808,7 +780,7 @@ export class Garden {
     }
 
     // Walk through all plugins in dependency order, and allow them to augment the graph
-    const providerConfigs = Object.values(providers).map((p) => p.config)
+    const providerConfigs = Object.values(resolvedProviders).map((p) => p.config)
 
     for (const provider of getDependencyOrder(providerConfigs, this.registeredPlugins)) {
       // Skip the routine if the provider doesn't have the handler
@@ -831,21 +803,10 @@ export class Garden {
       const { addBuildDependencies, addRuntimeDependencies, addModules } = await actions.augmentGraph({
         pluginName: provider.name,
         log,
-        providers,
+        providers: resolvedProviders,
         modules: resolvedModules,
       })
 
-      const configContext = new ModuleConfigContext({
-        garden: this,
-        resolvedProviders: keyBy(providers, "name"),
-        dependencies: resolvedModules,
-        runtimeContext,
-        parentName: undefined,
-        templateName: undefined,
-        inputs: {},
-        partialRuntimeResolution: true,
-      })
-
       // Resolve modules from specs and add to the list
       await Bluebird.map(addModules || [], async (spec) => {
         const path = spec.path || this.projectRoot
@@ -854,7 +815,7 @@ export class Garden {
         // There is no actual config file for plugin modules (which the prepare function assumes)
         delete moduleConfig.configPath
 
-        const resolvedConfig = await resolveModuleConfig(this, moduleConfig, { configContext })
+        const resolvedConfig = await resolver.resolveModuleConfig(moduleConfig, resolvedModules)
         resolvedModules.push(await moduleFromConfig(this, log, resolvedConfig, resolvedModules))
         graph = undefined
       })
@@ -1176,7 +1137,7 @@ export class Garden {
   }
 
   public makeOverlapError(moduleOverlaps: ModuleOverlap[]) {
-    const overlapList = moduleOverlaps
+    const overlapList = sortBy(moduleOverlaps, (o) => o.module.name)
       .map(({ module, overlaps }) => {
         const formatted = overlaps.map((o) => {
           const detail = o.path === module.path ? "same path" : "nested"