From 79f3826863d46d820ef450d140acd8c8b7a7ba15 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Wed, 19 Aug 2020 17:48:37 +0200 Subject: [PATCH] feat(config): allow explicitly declaring provider dependencies This adds a `providers[].dependencies` field, which can be used to explicitly declare dependencies at config time. Previously you could only create implicit dependencies by referencing another provider's outputs in template strings. This should be handy when e.g. using the `exec` provider with an `initScript`, which should run before resolving other providers. --- core/src/config/provider.ts | 10 +- core/src/garden.ts | 4 +- core/src/plugins/container/container.ts | 3 +- core/src/tasks/resolve-provider.ts | 15 ++- core/test/unit/src/actions.ts | 2 +- core/test/unit/src/config/project.ts | 16 +++- core/test/unit/src/garden.ts | 96 ++++++++++++++----- docs/reference/config.md | 37 +++++++ .../reference/providers/conftest-container.md | 21 ++++ .../providers/conftest-kubernetes.md | 21 ++++ docs/reference/providers/conftest.md | 21 ++++ docs/reference/providers/exec.md | 21 ++++ docs/reference/providers/hadolint.md | 21 ++++ docs/reference/providers/kubernetes.md | 23 ++++- docs/reference/providers/local-kubernetes.md | 23 ++++- docs/reference/providers/maven-container.md | 21 ++++ docs/reference/providers/openfaas.md | 23 ++++- docs/reference/providers/terraform.md | 21 ++++ 18 files changed, 357 insertions(+), 42 deletions(-) diff --git a/core/src/config/provider.ts b/core/src/config/provider.ts index afdd730bc0..3200f25873 100644 --- a/core/src/config/provider.ts +++ b/core/src/config/provider.ts @@ -18,6 +18,7 @@ import { environmentStatusSchema } from "./status" export interface BaseProviderConfig { name: string + dependencies?: string[] environments?: string[] } @@ -31,6 +32,9 @@ const providerFixedFieldsSchema = () => .required() .description("The name of the provider plugin to use.") .example("local-kubernetes"), + dependencies: joiArray(joiIdentifier()) + .description("List other providers that should be resolved before this one.") + .example(["exec"]), environments: joi .array() .items(joiUserIdentifier()) @@ -104,7 +108,11 @@ export function providerFromConfig( * as well as implicit dependencies based on template strings. */ export async function getAllProviderDependencyNames(plugin: GardenPlugin, config: GenericProviderConfig) { - return uniq([...(plugin.dependencies || []), ...(await getProviderTemplateReferences(config))]).sort() + return uniq([ + ...(plugin.dependencies || []), + ...(config.dependencies || []), + ...(await getProviderTemplateReferences(config)), + ]).sort() } /** diff --git a/core/src/garden.ts b/core/src/garden.ts index d697118242..7d3ad5b70d 100644 --- a/core/src/garden.ts +++ b/core/src/garden.ts @@ -779,7 +779,9 @@ export class Garden { } // Walk through all plugins in dependency order, and allow them to augment the graph - for (const provider of getDependencyOrder(Object.values(providers), this.registeredPlugins)) { + const providerConfigs = Object.values(providers).map((p) => p.config) + + for (const provider of getDependencyOrder(providerConfigs, this.registeredPlugins)) { // Skip the routine if the provider doesn't have the handler const handler = await actions.getActionHandler({ actionType: "augmentGraph", diff --git a/core/src/plugins/container/container.ts b/core/src/plugins/container/container.ts index efb93ef49e..c0a17ace1a 100644 --- a/core/src/plugins/container/container.ts +++ b/core/src/plugins/container/container.ts @@ -23,7 +23,7 @@ import { SuggestModulesParams, SuggestModulesResult } from "../../types/plugin/m import { listDirectory } from "../../util/fs" import { dedent } from "../../util/string" import { getModuleTypeUrl } from "../../docs/common" -import { Provider, GenericProviderConfig } from "../../config/provider" +import { Provider, GenericProviderConfig, providerConfigBaseSchema } from "../../config/provider" export interface ContainerProviderConfig extends GenericProviderConfig {} export type ContainerProvider = Provider @@ -246,6 +246,7 @@ export const gardenPlugin = createGardenPlugin({ }, }, ], + configSchema: providerConfigBaseSchema(), tools: [ { name: "docker", diff --git a/core/src/tasks/resolve-provider.ts b/core/src/tasks/resolve-provider.ts index 5fd5ba761a..14db0f9107 100644 --- a/core/src/tasks/resolve-provider.ts +++ b/core/src/tasks/resolve-provider.ts @@ -17,7 +17,7 @@ import { } from "../config/provider" import { resolveTemplateStrings } from "../template-string" import { ConfigurationError, PluginError } from "../exceptions" -import { keyBy, omit, flatten } from "lodash" +import { keyBy, omit, flatten, uniq } from "lodash" import { GraphResults } from "../task-graph" import { ProviderConfigContext } from "../config/config-context" import { ModuleConfig } from "../config/module" @@ -85,16 +85,15 @@ export class ResolveProviderTask extends BaseTask { } async resolveDependencies() { - const explicitDeps = this.plugin.dependencies - const implicitDeps = (await getProviderTemplateReferences(this.config)).filter( - (depName) => !explicitDeps.includes(depName) - ) - const allDeps = [...explicitDeps, ...implicitDeps] + const pluginDeps = this.plugin.dependencies + const explicitDeps = this.config.dependencies || [] + const implicitDeps = await getProviderTemplateReferences(this.config) + const allDeps = uniq([...pluginDeps, ...explicitDeps, ...implicitDeps]) const rawProviderConfigs = this.garden.getRawProviderConfigs() const plugins = keyBy(await this.garden.getPlugins(), "name") - const matchDependencies = (depName) => { + const matchDependencies = (depName: string) => { // Match against a provider if its name matches directly, or it inherits from a base named `depName` return rawProviderConfigs.filter( (c) => c.name === depName || getPluginBaseNames(c.name, plugins).includes(depName) @@ -102,7 +101,7 @@ export class ResolveProviderTask extends BaseTask { } // Make sure explicit dependencies are configured - await Bluebird.map(explicitDeps, async (depName) => { + await Bluebird.map(pluginDeps, async (depName) => { const matched = matchDependencies(depName) if (matched.length === 0) { diff --git a/core/test/unit/src/actions.ts b/core/test/unit/src/actions.ts index 9bcadb255a..35c77b64b9 100644 --- a/core/test/unit/src/actions.ts +++ b/core/test/unit/src/actions.ts @@ -95,7 +95,7 @@ describe("ActionRouter", () => { describe("environment actions", () => { describe("configureProvider", () => { it("should configure the provider", async () => { - const config = { name: "test-plugin", foo: "bar" } + const config = { name: "test-plugin", foo: "bar", dependencies: [] } const result = await actions.configureProvider({ ctx: await garden.getPluginContext(providerFromConfig(config, {}, [], { ready: false, outputs: {} })), environmentName: "default", diff --git a/core/test/unit/src/config/project.ts b/core/test/unit/src/config/project.ts index c1ce2e4ad1..5efc6df775 100644 --- a/core/test/unit/src/config/project.ts +++ b/core/test/unit/src/config/project.ts @@ -39,7 +39,7 @@ describe("resolveProjectConfig", () => { dotIgnoreFiles: defaultDotIgnoreFiles, environments: [{ name: "default", defaultNamespace, variables: {} }], outputs: [], - providers: [{ name: "some-provider" }], + providers: [{ name: "some-provider", dependencies: [] }], variables: {}, } @@ -67,7 +67,7 @@ describe("resolveProjectConfig", () => { dotIgnoreFiles: defaultDotIgnoreFiles, environments: [], outputs: [], - providers: [{ name: "some-provider" }], + providers: [{ name: "some-provider", dependencies: [] }], variables: {}, } @@ -98,7 +98,7 @@ describe("resolveProjectConfig", () => { }, }, ], - providers: [{ name: "some-provider" }], + providers: [{ name: "some-provider", dependencies: [] }], sources: [ { name: "${local.env.TEST_ENV_VAR}", @@ -188,10 +188,12 @@ describe("resolveProjectConfig", () => { providers: [ { name: "provider-a", + dependencies: [], someKey: "${local.env.TEST_ENV_VAR_A}", }, { name: "provider-b", + dependencies: [], environments: ["default"], someKey: "${local.env.TEST_ENV_VAR_B}", }, @@ -240,7 +242,7 @@ describe("resolveProjectConfig", () => { dotIgnoreFiles: defaultDotIgnoreFiles, environments: [], outputs: [], - providers: [{ name: "some-provider" }], + providers: [{ name: "some-provider", dependencies: [] }], variables: {}, } @@ -263,7 +265,7 @@ describe("resolveProjectConfig", () => { dotIgnoreFiles: defaultDotIgnoreFiles, environments: [], outputs: [], - providers: [{ name: "some-provider" }], + providers: [{ name: "some-provider", dependencies: [] }], variables: {}, } @@ -324,13 +326,16 @@ describe("resolveProjectConfig", () => { providers: [ { name: "provider-a", + dependencies: [], }, { name: "provider-b", environments: ["default"], + dependencies: [], }, { name: "provider-c", + dependencies: [], }, ], sources: [], @@ -386,6 +391,7 @@ describe("resolveProjectConfig", () => { providers: [ { name: "provider-a", + dependencies: [], }, { name: "provider-b", diff --git a/core/test/unit/src/garden.ts b/core/test/unit/src/garden.ts index ff9626eab9..bb00d3824c 100644 --- a/core/test/unit/src/garden.ts +++ b/core/test/unit/src/garden.ts @@ -109,6 +109,7 @@ describe("Garden", () => { name: "test-plugin", config: { name: "test-plugin", + dependencies: [], environments: ["local"], path: projectRoot, }, @@ -126,11 +127,20 @@ describe("Garden", () => { const configs = mapValues(providers, (p) => p.config) expect(configs).to.eql({ - "exec": emptyProvider(projectRoot, "exec").config, - "container": emptyProvider(projectRoot, "container").config, + "exec": { + name: "exec", + dependencies: [], + path: projectRoot, + }, + "container": { + name: "container", + dependencies: [], + path: projectRoot, + }, "test-plugin": testPluginProvider.config, "test-plugin-b": { name: "test-plugin-b", + dependencies: [], environments: ["local"], path: projectRoot, }, @@ -162,10 +172,19 @@ describe("Garden", () => { const configs = mapValues(providers, (p) => p.config) expect(configs).to.eql({ - "exec": emptyProvider(projectRoot, "exec").config, - "container": emptyProvider(projectRoot, "container").config, + "exec": { + name: "exec", + dependencies: [], + path: projectRoot, + }, + "container": { + name: "container", + dependencies: [], + path: projectRoot, + }, "test-plugin": { name: "test-plugin", + dependencies: [], path: projectRoot, }, }) @@ -1392,6 +1411,7 @@ describe("Garden", () => { name: "test-plugin", config: { name: "test-plugin", + dependencies: [], environments: ["local"], path: projectRoot, }, @@ -1407,11 +1427,20 @@ describe("Garden", () => { const configs = mapValues(providers, (p) => p.config) expect(configs).to.eql({ - "exec": emptyProvider(projectRoot, "exec").config, - "container": emptyProvider(projectRoot, "container").config, + "exec": { + name: "exec", + dependencies: [], + path: projectRoot, + }, + "container": { + name: "container", + dependencies: [], + path: projectRoot, + }, "test-plugin": testPluginProvider.config, "test-plugin-b": { name: "test-plugin-b", + dependencies: [], environments: ["local"], path: projectRoot, }, @@ -1437,6 +1466,7 @@ describe("Garden", () => { async configureProvider({ config }: ConfigureProviderParams) { expect(config).to.eql({ name: "test", + dependencies: [], path: projectConfig.path, foo: "bar", }) @@ -1454,6 +1484,7 @@ describe("Garden", () => { expect(provider.config).to.eql({ name: "test", + dependencies: [], path: projectConfig.path, foo: "bla", }) @@ -1686,6 +1717,43 @@ describe("Garden", () => { name: "test-a", }) + const testB = createGardenPlugin({ + name: "test-b", + }) + + const projectConfig: ProjectConfig = { + apiVersion: "garden.io/v0", + kind: "Project", + name: "test", + path: projectRootA, + defaultEnvironment: "default", + dotIgnoreFiles: defaultDotIgnoreFiles, + environments: [{ name: "default", defaultNamespace, variables: {} }], + providers: [ + { name: "test-a", foo: "${providers.test-b.outputs.foo}" }, + { name: "test-b", dependencies: ["test-a"] }, + ], + variables: {}, + } + + const plugins = [testA, testB] + const garden = await TestGarden.factory(projectRootA, { config: projectConfig, plugins }) + + await expectError( + () => garden.resolveProviders(garden.log), + (err) => + expect(err.message).to.equal(deline` + One or more circular dependencies found between providers or their + configurations:\n\ntest-a <- test-b <- test-a + `) + ) + }) + + it("should throw if provider configs have combined implicit and plugin circular dependencies", async () => { + const testA = createGardenPlugin({ + name: "test-a", + }) + const testB = createGardenPlugin({ name: "test-b", dependencies: ["test-a"], @@ -3555,19 +3623,3 @@ describe("Garden", () => { }) }) }) - -function emptyProvider(projectRoot: string, name: string) { - return { - name, - config: { - name, - path: projectRoot, - }, - dependencies: {}, - moduleConfigs: [], - status: { - ready: true, - outputs: {}, - }, - } -} diff --git a/docs/reference/config.md b/docs/reference/config.md index 2bf5225bff..24313da05c 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -84,6 +84,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -310,6 +313,22 @@ Example: environments: ``` +### `environments[].providers[].dependencies[]` + +[environments](#environments) > [providers](#environmentsproviders) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +environments: +``` + ### `environments[].providers[].environments[]` [environments](#environments) > [providers](#environmentsproviders) > environments @@ -390,6 +409,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/conftest-container.md b/docs/reference/providers/conftest-container.md index abc9e087e9..af7fa9cf12 100644 --- a/docs/reference/providers/conftest-container.md +++ b/docs/reference/providers/conftest-container.md @@ -24,6 +24,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -63,6 +66,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/conftest-kubernetes.md b/docs/reference/providers/conftest-kubernetes.md index 651c3bf4bd..65e366d74b 100644 --- a/docs/reference/providers/conftest-kubernetes.md +++ b/docs/reference/providers/conftest-kubernetes.md @@ -30,6 +30,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -69,6 +72,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/conftest.md b/docs/reference/providers/conftest.md index 9d2f51fe9d..6d25213173 100644 --- a/docs/reference/providers/conftest.md +++ b/docs/reference/providers/conftest.md @@ -26,6 +26,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -65,6 +68,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/exec.md b/docs/reference/providers/exec.md index c91c460fe0..de8b194064 100644 --- a/docs/reference/providers/exec.md +++ b/docs/reference/providers/exec.md @@ -26,6 +26,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -61,6 +64,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/hadolint.md b/docs/reference/providers/hadolint.md index b9715aeb12..f52be669b1 100644 --- a/docs/reference/providers/hadolint.md +++ b/docs/reference/providers/hadolint.md @@ -26,6 +26,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -63,6 +66,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/kubernetes.md b/docs/reference/providers/kubernetes.md index 0427878b6a..4afc05ed27 100644 --- a/docs/reference/providers/kubernetes.md +++ b/docs/reference/providers/kubernetes.md @@ -27,7 +27,10 @@ The values in the schema below are the default values. ```yaml providers: - - # If specified, this provider will only be used in the listed environments. Note that an empty array effectively + - # List other providers that should be resolved before this one. + dependencies: [] + + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -331,6 +334,24 @@ providers: | --------------- | ------- | -------- | | `array[object]` | `[]` | No | +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/local-kubernetes.md b/docs/reference/providers/local-kubernetes.md index c55f567254..94d00710fe 100644 --- a/docs/reference/providers/local-kubernetes.md +++ b/docs/reference/providers/local-kubernetes.md @@ -23,7 +23,10 @@ The values in the schema below are the default values. ```yaml providers: - - # If specified, this provider will only be used in the listed environments. Note that an empty array effectively + - # List other providers that should be resolved before this one. + dependencies: [] + + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -298,6 +301,24 @@ providers: | --------------- | ------- | -------- | | `array[object]` | `[]` | No | +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/maven-container.md b/docs/reference/providers/maven-container.md index e6574f5751..a69bb14783 100644 --- a/docs/reference/providers/maven-container.md +++ b/docs/reference/providers/maven-container.md @@ -24,6 +24,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -53,6 +56,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/openfaas.md b/docs/reference/providers/openfaas.md index 7e1f4ee1f5..5a4045f843 100644 --- a/docs/reference/providers/openfaas.md +++ b/docs/reference/providers/openfaas.md @@ -23,7 +23,10 @@ The values in the schema below are the default values. ```yaml providers: - - # If specified, this provider will only be used in the listed environments. Note that an empty array effectively + - # List other providers that should be resolved before this one. + dependencies: [] + + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -62,6 +65,24 @@ providers: | --------------- | ------- | -------- | | `array[object]` | `[]` | No | +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments diff --git a/docs/reference/providers/terraform.md b/docs/reference/providers/terraform.md index 20169969be..9f0a8c05fb 100644 --- a/docs/reference/providers/terraform.md +++ b/docs/reference/providers/terraform.md @@ -22,6 +22,9 @@ providers: - # The name of the provider plugin to use. name: + # List other providers that should be resolved before this one. + dependencies: [] + # If specified, this provider will only be used in the listed environments. Note that an empty array effectively # disables the provider. To use a provider in all environments, omit this field. environments: @@ -70,6 +73,24 @@ providers: - name: "local-kubernetes" ``` +### `providers[].dependencies[]` + +[providers](#providers) > dependencies + +List other providers that should be resolved before this one. + +| Type | Default | Required | +| --------------- | ------- | -------- | +| `array[string]` | `[]` | No | + +Example: + +```yaml +providers: + - dependencies: + - exec +``` + ### `providers[].environments[]` [providers](#providers) > environments