Skip to content

Commit

Permalink
refactor: remove NoopContext
Browse files Browse the repository at this point in the history
  • Loading branch information
stefreak committed Dec 5, 2024
1 parent 3397b0e commit e220046
Show file tree
Hide file tree
Showing 8 changed files with 429 additions and 347 deletions.
14 changes: 9 additions & 5 deletions core/src/config/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import type { ActionState } from "../actions/types.js"
import type { ValidResultType } from "../tasks/base.js"
import { uuidv4 } from "../util/random.js"
import { s } from "./zod.js"
import { NoOpContext } from "./template-contexts/base.js"
import { getContextLookupReferences, visitAll } from "../template-string/static-analysis.js"
import type { ConfigContext } from "./template-contexts/base.js"

// TODO: dedupe from the joi schema below
export const baseProviderConfigSchemaZod = s.object({
Expand Down Expand Up @@ -165,18 +165,22 @@ export function providerFromConfig({
* Given a plugin and its provider config, return a list of dependency names based on declared dependencies,
* as well as implicit dependencies based on template strings.
*/
export function getAllProviderDependencyNames(plugin: GardenPluginSpec, config: GenericProviderConfig) {
export function getAllProviderDependencyNames(
plugin: GardenPluginSpec,
config: GenericProviderConfig,
context: ConfigContext
) {
return uniq([
...(plugin.dependencies || []).map((d) => d.name),
...(config.dependencies || []),
...getProviderTemplateReferences(config),
...getProviderTemplateReferences(config, context),
]).sort()
}

/**
* Given a provider config, return implicit dependencies based on template strings.
*/
export function getProviderTemplateReferences(config: GenericProviderConfig) {
export function getProviderTemplateReferences(config: GenericProviderConfig, context: ConfigContext) {
const deps: string[] = []

const generator = getContextLookupReferences(
Expand All @@ -188,7 +192,7 @@ export function getProviderTemplateReferences(config: GenericProviderConfig) {
path: [],
},
}),
new NoOpContext()
context
)
for (const finding of generator) {
const keyPath = finding.keyPath
Expand Down
6 changes: 0 additions & 6 deletions core/src/config/template-contexts/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,6 @@ export class GenericContext extends ConfigContext {
}
}

export class NoOpContext extends ConfigContext {
override resolve() {
return { resolved: CONTEXT_RESOLVE_KEY_AVAILABLE_LATER, partial: true }
}
}

export class EnvironmentContext extends ConfigContext {
@schema(
joi
Expand Down
22 changes: 19 additions & 3 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,13 @@ export class Garden {
providerNames = getNames(rawConfigs)
}

throwOnMissingSecretKeys(rawConfigs, this.secrets, "Provider", log)
throwOnMissingSecretKeys(
rawConfigs,
new RemoteSourceConfigContext(this, this.variables),
this.secrets,
"Provider",
log
)

// As an optimization, we return immediately if all requested providers are already resolved
const alreadyResolvedProviders = providerNames.map((name) => this.resolvedProviders[name]).filter(Boolean)
Expand Down Expand Up @@ -820,7 +826,11 @@ export class Garden {

validationGraph.addNode(plugin.name)

for (const dep of getAllProviderDependencyNames(plugin!, config!)) {
for (const dep of getAllProviderDependencyNames(
plugin!,
config!,
new RemoteSourceConfigContext(this, this.variables)
)) {
validationGraph.addNode(dep)
validationGraph.addDependency(plugin.name, dep)
}
Expand Down Expand Up @@ -1411,7 +1421,13 @@ export class Garden {
const groupedResources = groupBy(allResources, "kind")

for (const [kind, configs] of Object.entries(groupedResources)) {
throwOnMissingSecretKeys(configs, this.secrets, kind, this.log)
throwOnMissingSecretKeys(
configs,
new RemoteSourceConfigContext(this, this.variables),
this.secrets,
kind,
this.log
)
}

let rawModuleConfigs = [...((groupedResources.Module as ModuleConfig[]) || [])]
Expand Down
6 changes: 5 additions & 1 deletion core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type { Log } from "../logger/log-entry.js"
import { styles } from "../logger/styles.js"
import type { ObjectPath } from "../config/base.js"
import fsExtra from "fs-extra"
import { RemoteSourceConfigContext } from "../config/template-contexts/project.js"

const { readFile, writeFile, ensureDir } = fsExtra

Expand Down Expand Up @@ -116,7 +117,10 @@ export class ResolveProviderTask extends BaseTask<Provider> {

const pluginDeps = this.plugin.dependencies
const explicitDeps = (this.config.dependencies || []).map((name) => ({ name }))
const implicitDeps = getProviderTemplateReferences(this.config).map((name) => ({ name }))
const implicitDeps = getProviderTemplateReferences(
this.config,
new RemoteSourceConfigContext(this.garden, this.garden.variables)
).map((name) => ({ name }))
const allDeps = uniq([...pluginDeps, ...explicitDeps, ...implicitDeps])

const rawProviderConfigs = this.garden.getRawProviderConfigs()
Expand Down
19 changes: 14 additions & 5 deletions core/src/template-string/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
CONTEXT_RESOLVE_KEY_AVAILABLE_LATER,
CONTEXT_RESOLVE_KEY_NOT_FOUND,
GenericContext,
NoOpContext,
} from "../config/template-contexts/base.js"
import cloneDeep from "fast-copy"
import { difference, isPlainObject, isString } from "lodash-es"
Expand Down Expand Up @@ -803,10 +802,16 @@ export function getModuleTemplateReferences(config: ModuleConfig, context: Modul
*
* Prefix should be e.g. "Module" or "Provider" (used when generating error messages).
*/
export function throwOnMissingSecretKeys(configs: ObjectWithName[], secrets: StringMap, prefix: string, log?: Log) {
export function throwOnMissingSecretKeys(
configs: ObjectWithName[],
context: ConfigContext,
secrets: StringMap,
prefix: string,
log?: Log
) {
const allMissing: [string, ContextKeySegment[]][] = [] // [[key, missing keys]]
for (const config of configs) {
const missing = detectMissingSecretKeys(config, secrets)
const missing = detectMissingSecretKeys(config, context, secrets)
if (missing.length > 0) {
allMissing.push([config.name, missing])
}
Expand Down Expand Up @@ -853,7 +858,11 @@ export function throwOnMissingSecretKeys(configs: ObjectWithName[], secrets: Str
* Collects template references to secrets in obj, and returns an array of any secret keys referenced in it that
* aren't present (or have blank values) in the provided secrets map.
*/
export function detectMissingSecretKeys(obj: ObjectWithName, secrets: StringMap): ContextKeySegment[] {
export function detectMissingSecretKeys(
obj: ObjectWithName,
context: ConfigContext,
secrets: StringMap
): ContextKeySegment[] {
const referencedKeys: ContextKeySegment[] = []
const generator = getContextLookupReferences(
visitAll({
Expand All @@ -864,7 +873,7 @@ export function detectMissingSecretKeys(obj: ObjectWithName, secrets: StringMap)
path: [],
},
}),
new NoOpContext()
context
)
for (const finding of generator) {
const keyPath = finding.keyPath
Expand Down
10 changes: 7 additions & 3 deletions core/test/unit/src/config/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { GenericProviderConfig } from "../../../../src/config/provider.js"
import { getAllProviderDependencyNames } from "../../../../src/config/provider.js"
import { expectError } from "../../../helpers.js"
import { createGardenPlugin } from "../../../../src/plugin/plugin.js"
import { GenericContext } from "../../../../src/config/template-contexts/base.js"

describe("getProviderDependencies", () => {
const plugin = createGardenPlugin({
Expand All @@ -23,7 +24,10 @@ describe("getProviderDependencies", () => {
someKey: "${providers.other-provider.foo}",
anotherKey: "foo-${providers.another-provider.bar}",
}
expect(getAllProviderDependencyNames(plugin, config)).to.eql(["another-provider", "other-provider"])
expect(getAllProviderDependencyNames(plugin, config, new GenericContext({}))).to.eql([
"another-provider",
"other-provider",
])
})

it("should ignore template strings that don't reference providers", async () => {
Expand All @@ -32,7 +36,7 @@ describe("getProviderDependencies", () => {
someKey: "${providers.other-provider.foo}",
anotherKey: "foo-${some.other.ref}",
}
expect(getAllProviderDependencyNames(plugin, config)).to.eql(["other-provider"])
expect(getAllProviderDependencyNames(plugin, config, new GenericContext({}))).to.eql(["other-provider"])
})

it("should throw on provider-scoped template strings without a provider name", async () => {
Expand All @@ -41,7 +45,7 @@ describe("getProviderDependencies", () => {
someKey: "${providers}",
}

await expectError(() => getAllProviderDependencyNames(plugin, config), {
await expectError(() => getAllProviderDependencyNames(plugin, config, new GenericContext({})), {
contains:
"Invalid template key 'providers' in configuration for provider 'my-provider'. You must specify a provider name as well (e.g. \\${providers.my-provider}).",
})
Expand Down
Loading

0 comments on commit e220046

Please sign in to comment.