Skip to content

Commit

Permalink
feat(config): allow explicitly declaring provider dependencies
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
edvald committed Aug 19, 2020
1 parent d6aee9f commit 79f3826
Show file tree
Hide file tree
Showing 18 changed files with 357 additions and 42 deletions.
10 changes: 9 additions & 1 deletion core/src/config/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { environmentStatusSchema } from "./status"

export interface BaseProviderConfig {
name: string
dependencies?: string[]
environments?: string[]
}

Expand All @@ -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())
Expand Down Expand Up @@ -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()
}

/**
Expand Down
4 changes: 3 additions & 1 deletion core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion core/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ContainerProviderConfig>
Expand Down Expand Up @@ -246,6 +246,7 @@ export const gardenPlugin = createGardenPlugin({
},
},
],
configSchema: providerConfigBaseSchema(),
tools: [
{
name: "docker",
Expand Down
15 changes: 7 additions & 8 deletions core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -85,24 +85,23 @@ 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)
)
}

// 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) {
Expand Down
2 changes: 1 addition & 1 deletion core/test/unit/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 11 additions & 5 deletions core/test/unit/src/config/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe("resolveProjectConfig", () => {
dotIgnoreFiles: defaultDotIgnoreFiles,
environments: [{ name: "default", defaultNamespace, variables: {} }],
outputs: [],
providers: [{ name: "some-provider" }],
providers: [{ name: "some-provider", dependencies: [] }],
variables: {},
}

Expand Down Expand Up @@ -67,7 +67,7 @@ describe("resolveProjectConfig", () => {
dotIgnoreFiles: defaultDotIgnoreFiles,
environments: [],
outputs: [],
providers: [{ name: "some-provider" }],
providers: [{ name: "some-provider", dependencies: [] }],
variables: {},
}

Expand Down Expand Up @@ -98,7 +98,7 @@ describe("resolveProjectConfig", () => {
},
},
],
providers: [{ name: "some-provider" }],
providers: [{ name: "some-provider", dependencies: [] }],
sources: [
{
name: "${local.env.TEST_ENV_VAR}",
Expand Down Expand Up @@ -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}",
},
Expand Down Expand Up @@ -240,7 +242,7 @@ describe("resolveProjectConfig", () => {
dotIgnoreFiles: defaultDotIgnoreFiles,
environments: [],
outputs: [],
providers: [{ name: "some-provider" }],
providers: [{ name: "some-provider", dependencies: [] }],
variables: {},
}

Expand All @@ -263,7 +265,7 @@ describe("resolveProjectConfig", () => {
dotIgnoreFiles: defaultDotIgnoreFiles,
environments: [],
outputs: [],
providers: [{ name: "some-provider" }],
providers: [{ name: "some-provider", dependencies: [] }],
variables: {},
}

Expand Down Expand Up @@ -324,13 +326,16 @@ describe("resolveProjectConfig", () => {
providers: [
{
name: "provider-a",
dependencies: [],
},
{
name: "provider-b",
environments: ["default"],
dependencies: [],
},
{
name: "provider-c",
dependencies: [],
},
],
sources: [],
Expand Down Expand Up @@ -386,6 +391,7 @@ describe("resolveProjectConfig", () => {
providers: [
{
name: "provider-a",
dependencies: [],
},
{
name: "provider-b",
Expand Down
96 changes: 74 additions & 22 deletions core/test/unit/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe("Garden", () => {
name: "test-plugin",
config: {
name: "test-plugin",
dependencies: [],
environments: ["local"],
path: projectRoot,
},
Expand All @@ -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,
},
Expand Down Expand Up @@ -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,
},
})
Expand Down Expand Up @@ -1392,6 +1411,7 @@ describe("Garden", () => {
name: "test-plugin",
config: {
name: "test-plugin",
dependencies: [],
environments: ["local"],
path: projectRoot,
},
Expand All @@ -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,
},
Expand All @@ -1437,6 +1466,7 @@ describe("Garden", () => {
async configureProvider({ config }: ConfigureProviderParams) {
expect(config).to.eql({
name: "test",
dependencies: [],
path: projectConfig.path,
foo: "bar",
})
Expand All @@ -1454,6 +1484,7 @@ describe("Garden", () => {

expect(provider.config).to.eql({
name: "test",
dependencies: [],
path: projectConfig.path,
foo: "bla",
})
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -3555,19 +3623,3 @@ describe("Garden", () => {
})
})
})

function emptyProvider(projectRoot: string, name: string) {
return {
name,
config: {
name,
path: projectRoot,
},
dependencies: {},
moduleConfigs: [],
status: {
ready: true,
outputs: {},
},
}
}
Loading

0 comments on commit 79f3826

Please sign in to comment.