Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(modules): another fix for the experimental partial module resolution #6105

Merged
merged 1 commit into from
May 29, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 56 additions & 5 deletions core/src/resolve-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ export class ModuleResolver {
}
}

// Clean up after our little hack
for (const config of Object.values(this.rawConfigsByKey)) {
delete config["_templateDeps"]
}

return { skipped, resolvedModules: Object.values(resolvedModules), resolvedConfigs: Object.values(resolvedConfigs) }
}

Expand Down Expand Up @@ -431,10 +436,17 @@ export class ModuleResolver {
return true
}

// Is it depended on (at the module level) by something set in the filter?
// Is it depended on (at the module level) by something set in the filter or in a template string?
const dependantKeys = fullGraph.dependantsOf(config.name)
for (const key of dependantKeys) {
if (this.matchFilter(`build.${key}`, actionsFilter)) {
const dep = this.rawConfigsByKey[key]
if (!dep) {
continue
}
if (dep["_templateDeps"]?.includes(config.name)) {
return true
}
if (this.moduleMatchesFilter(dep, actionsFilter)) {
return true
}
}
Expand Down Expand Up @@ -470,17 +482,17 @@ export class ModuleResolver {
return true
}

for (const s of config.serviceConfigs) {
for (const s of getServiceNames(config)) {
if (match(`deploy.${s}`)) {
return true
}
}
for (const t of config.taskConfigs) {
for (const t of getTaskNames(config)) {
if (match(`run.${t}`)) {
return true
}
}
for (const t of config.testConfigs) {
for (const t of getTestNames(config)) {
if (match(`test.${config.name}-${t}`)) {
return true
}
Expand Down Expand Up @@ -516,6 +528,10 @@ export class ModuleResolver {
const templateRefs = getModuleTemplateReferences(rawConfig, configContext)
const templateDeps = <string[]>templateRefs.filter((d) => d[1] !== rawConfig.name).map((d) => d[1])

// This is a bit of a hack, but we need to store the template dependencies on the raw config so we can check
// them later when deciding whether to resolve a module inline or not.
rawConfig["_templateDeps"] = templateDeps

// Try resolving template strings if possible
let buildDeps: string[] = []
const resolvedDeps = resolveTemplateStrings({
Expand Down Expand Up @@ -746,6 +762,8 @@ export class ModuleResolver {
}
}

delete config["_templateDeps"]

return config
}

Expand Down Expand Up @@ -1230,3 +1248,36 @@ function missingBuildDependency(moduleName: string, dependencyName: string) {
`configured in module ${styles.highlight(moduleName)}`,
})
}

// We're hardcoding the module types here because the schemas are frozen, so it's an okay shortcut to support
// partial resolution.
function getServiceNames(config: ModuleConfig) {
const names = config.serviceConfigs.map((s) => s.name)
// These all have a services list field
if (["container", "jib-container", "exec"].includes(config.type)) {
names.push(...(config.spec.services || []).map((s) => s.name).filter(Boolean))
thsig marked this conversation as resolved.
Show resolved Hide resolved
}
// These all map to a single service
if (["helm", "kubernetes", "terraform", "pulumi", "configmap", "persistentvolumeclaim"].includes(config.type)) {
names.push(config.name)
}
return names
}

function getTaskNames(config: ModuleConfig) {
const names = config.taskConfigs.map((t) => t.name)
// These all have a tasks list field
if (["exec", "kubernetes", "helm", "container", "jib-container"].includes(config.type)) {
names.push(...(config.spec.tasks || []).map((t) => t.name).filter(Boolean))
}
return names
}

function getTestNames(config: ModuleConfig) {
const names = config.testConfigs.map((t) => t.name)
// These all have a tests list field
if (["exec", "kubernetes", "helm", "container", "jib-container"].includes(config.type)) {
names.push(...(config.spec.tests || []).map((t) => config.name + "-" + t.name).filter(Boolean))
}
return names
}