mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-21 08:02:26 +00:00
fix(cli): use wmill.yaml key consistently for workspace-specific items (#8900)
* fix(cli): use wmill.yaml key (not branch name) for workspace-specific filenames
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): resolve --workspace as config key even with --base-url
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(integration): assert workspace config key drives filename suffix in git-sync
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Revert "test(integration): assert workspace config key drives filename suffix in git-sync"
This reverts commit 528993fe29.
* fix(cli): filter reserved keys and preserve validation-skip on non-config --workspace
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -180,20 +180,25 @@ export async function findResourceFile(path: string) {
|
||||
let contentBasePathJSON = splitPath[0] + "." + splitPath[1] + ".json";
|
||||
let contentBasePathYAML = splitPath[0] + "." + splitPath[1] + ".yaml";
|
||||
|
||||
// Check for branch-specific metadata files first
|
||||
// Check for workspace-specific metadata files first, using the wmill.yaml
|
||||
// config key for the current git branch as the filename suffix (falls back
|
||||
// to the branch name when no matching workspace entry exists).
|
||||
const currentBranch = getCurrentGitBranch();
|
||||
const wsName = currentBranch
|
||||
? await specificItems.resolveWsNameForGitBranch(currentBranch)
|
||||
: null;
|
||||
|
||||
const candidates = [contentBasePathJSON, contentBasePathYAML];
|
||||
|
||||
if (currentBranch) {
|
||||
// Add branch-specific candidates at the beginning (higher priority)
|
||||
if (wsName) {
|
||||
// Add workspace-specific candidates at the beginning (higher priority)
|
||||
const branchSpecificJSON = specificItems.toWorkspaceSpecificPath(
|
||||
contentBasePathJSON,
|
||||
currentBranch
|
||||
wsName
|
||||
);
|
||||
const branchSpecificYAML = specificItems.toWorkspaceSpecificPath(
|
||||
contentBasePathYAML,
|
||||
currentBranch
|
||||
wsName
|
||||
);
|
||||
candidates.unshift(branchSpecificJSON, branchSpecificYAML);
|
||||
}
|
||||
|
||||
@@ -46,6 +46,7 @@ import {
|
||||
} from "../../utils/utils.ts";
|
||||
import {
|
||||
getEffectiveSettings,
|
||||
getWorkspaceNames,
|
||||
mergeConfigWithConfigFile,
|
||||
parseSyncBehavior,
|
||||
SyncOptions,
|
||||
@@ -122,6 +123,27 @@ function resolveWsNameFromBranch(opts: SyncOptions, branchName: string): string
|
||||
return match ? match[0] : branchName;
|
||||
}
|
||||
|
||||
// Resolve wsNameForConfig from CLI flags. Prefers --branch → matching config key,
|
||||
// then --workspace → matching config key (incl. when --base-url is set). Returns
|
||||
// undefined when no flag-based resolution applies; callers then fall back to
|
||||
// inferWsNameFromProfile on the resolved workspace profile.
|
||||
export function resolveWsNameForConfigFromFlags(
|
||||
opts: SyncOptions & { branch?: string; workspace?: string },
|
||||
): string | undefined {
|
||||
if (opts.branch) {
|
||||
return resolveWsNameFromBranch(opts, opts.branch);
|
||||
}
|
||||
if (opts.workspace) {
|
||||
// Use getWorkspaceNames so reserved keys (e.g. commonSpecificItems) are filtered out,
|
||||
// matching the behavior of findWorkspaceByGitBranch / inferWsNameFromProfile.
|
||||
const validKeys = getWorkspaceNames(opts.workspaces);
|
||||
if (validKeys.includes(opts.workspace)) {
|
||||
return opts.workspace;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Warn if --workspace overrides auto-detected branch or if workspace not in config.
|
||||
function warnWorkspaceOverride(opts: SyncOptions, wsNameForConfig: string | undefined): void {
|
||||
if (!wsNameForConfig || !opts.workspaces) return;
|
||||
@@ -2181,27 +2203,29 @@ export async function pull(
|
||||
|
||||
// Resolve workspace name for config lookups.
|
||||
// --branch resolves git branch → workspace name (deprecated but still supported).
|
||||
// --workspace (without --base-url) selects a workspace config entry by name.
|
||||
// When --base-url is used with --workspace, --workspace is a profile selector only;
|
||||
// --branch should still drive config lookups.
|
||||
// --workspace selects a workspace config entry by name when it matches one,
|
||||
// regardless of --base-url. If it doesn't match any entry it's treated as a
|
||||
// profile/credential selector only.
|
||||
const hasExplicitCredentials = !!opts.baseUrl;
|
||||
let wsNameForConfig: string | undefined;
|
||||
|
||||
if (opts.branch) {
|
||||
if (!hasExplicitCredentials && !branchDeprecationWarned) {
|
||||
log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead.");
|
||||
branchDeprecationWarned = true;
|
||||
}
|
||||
wsNameForConfig = resolveWsNameFromBranch(opts, opts.branch);
|
||||
} else if (opts.workspace && !hasExplicitCredentials) {
|
||||
// --workspace without --base-url: use as workspace config name
|
||||
wsNameForConfig = opts.workspace;
|
||||
warnWorkspaceOverride(opts, wsNameForConfig);
|
||||
if (opts.branch && !hasExplicitCredentials && !branchDeprecationWarned) {
|
||||
log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead.");
|
||||
branchDeprecationWarned = true;
|
||||
}
|
||||
|
||||
// Validate workspace configuration early (skipped when override is used)
|
||||
wsNameForConfig = resolveWsNameForConfigFromFlags(opts);
|
||||
|
||||
if (!opts.branch && opts.workspace && !hasExplicitCredentials) {
|
||||
// Warn if override doesn't match a config key, or mismatches the auto-detected branch
|
||||
warnWorkspaceOverride(opts, opts.workspace);
|
||||
}
|
||||
|
||||
// Validate workspace configuration early. Skip when ANY explicit flag is set
|
||||
// (even a --workspace value that doesn't match a config key — the user opted
|
||||
// out of branch-based auto-detection).
|
||||
try {
|
||||
await validateBranchConfiguration(opts, wsNameForConfig);
|
||||
await validateBranchConfiguration(opts, wsNameForConfig ?? opts.workspace);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message.includes("overrides")) {
|
||||
log.error(error.message);
|
||||
@@ -2723,20 +2747,23 @@ export async function push(
|
||||
const hasExplicitCredentials = !!opts.baseUrl;
|
||||
let wsNameForConfig: string | undefined;
|
||||
|
||||
if (opts.branch) {
|
||||
if (!hasExplicitCredentials && !branchDeprecationWarned) {
|
||||
log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead.");
|
||||
branchDeprecationWarned = true;
|
||||
}
|
||||
wsNameForConfig = resolveWsNameFromBranch(opts, opts.branch);
|
||||
} else if (opts.workspace && !hasExplicitCredentials) {
|
||||
wsNameForConfig = opts.workspace;
|
||||
warnWorkspaceOverride(opts, wsNameForConfig);
|
||||
if (opts.branch && !hasExplicitCredentials && !branchDeprecationWarned) {
|
||||
log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead.");
|
||||
branchDeprecationWarned = true;
|
||||
}
|
||||
|
||||
// Validate workspace configuration early (skipped when override is used)
|
||||
wsNameForConfig = resolveWsNameForConfigFromFlags(opts);
|
||||
|
||||
if (!opts.branch && opts.workspace && !hasExplicitCredentials) {
|
||||
// Warn if override doesn't match a config key, or mismatches the auto-detected branch
|
||||
warnWorkspaceOverride(opts, opts.workspace);
|
||||
}
|
||||
|
||||
// Validate workspace configuration early. Skip when ANY explicit flag is set
|
||||
// (even a --workspace value that doesn't match a config key — the user opted
|
||||
// out of branch-based auto-detection).
|
||||
try {
|
||||
await validateBranchConfiguration(opts, wsNameForConfig);
|
||||
await validateBranchConfiguration(opts, wsNameForConfig ?? opts.workspace);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message.includes("overrides")) {
|
||||
log.error(error.message);
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
import {
|
||||
fromWorkspaceSpecificPath,
|
||||
isWorkspaceSpecificFile,
|
||||
resolveWsNameForGitBranch,
|
||||
} from "../../core/specific_items.ts";
|
||||
import { getCurrentGitBranch } from "../../utils/git.ts";
|
||||
import { requireLogin } from "../../core/auth.ts";
|
||||
@@ -584,14 +585,17 @@ function checkIfValidTrigger(kind: string | undefined): kind is TriggerType {
|
||||
}
|
||||
}
|
||||
|
||||
function extractTriggerKindFromPath(filePath: string): string | undefined {
|
||||
async function extractTriggerKindFromPath(filePath: string): Promise<string | undefined> {
|
||||
let pathToAnalyze = filePath;
|
||||
|
||||
// If this is a branch-specific file, convert it to the base path first
|
||||
// If this is a workspace-specific file, convert it to the base path first.
|
||||
// Resolve the wmill.yaml config key for the current branch (falls back to
|
||||
// the branch name when no matching workspace entry exists).
|
||||
if (isWorkspaceSpecificFile(filePath)) {
|
||||
const currentBranch = getCurrentGitBranch();
|
||||
if (currentBranch) {
|
||||
pathToAnalyze = fromWorkspaceSpecificPath(filePath, currentBranch);
|
||||
const wsName = await resolveWsNameForGitBranch(currentBranch);
|
||||
pathToAnalyze = fromWorkspaceSpecificPath(filePath, wsName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -615,7 +619,7 @@ async function push(opts: GlobalOptions, filePath: string, remotePath: string) {
|
||||
|
||||
console.log(colors.bold.yellow("Pushing trigger..."));
|
||||
|
||||
const triggerKind = extractTriggerKindFromPath(filePath);
|
||||
const triggerKind = await extractTriggerKindFromPath(filePath);
|
||||
if (!checkIfValidTrigger(triggerKind)) {
|
||||
throw new Error("Invalid trigger kind: " + triggerKind);
|
||||
}
|
||||
|
||||
@@ -4,10 +4,22 @@ import { isFileResource, isFilesetResource } from "../utils/utils.ts";
|
||||
import {
|
||||
SyncOptions,
|
||||
findWorkspaceByGitBranch,
|
||||
readConfigFile,
|
||||
WorkspaceEntryConfig,
|
||||
} from "./conf.ts";
|
||||
import { TRIGGER_TYPES } from "../types.ts";
|
||||
|
||||
/**
|
||||
* Resolve the effective workspace name (wmill.yaml config key) for a given
|
||||
* git branch. Falls back to the branch name itself when no matching workspace
|
||||
* entry exists (legacy behavior).
|
||||
*/
|
||||
export async function resolveWsNameForGitBranch(branchName: string): Promise<string> {
|
||||
const config = await readConfigFile({ warnIfMissing: false });
|
||||
const match = findWorkspaceByGitBranch(config.workspaces, branchName);
|
||||
return match ? match[0] : branchName;
|
||||
}
|
||||
|
||||
export interface SpecificItemsConfig {
|
||||
variables?: string[];
|
||||
resources?: string[];
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
|
||||
import { execSync } from "node:child_process";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { stringify as yamlStringify } from "yaml";
|
||||
|
||||
import { resolveWsNameForGitBranch } from "../src/core/specific_items.ts";
|
||||
import { findResourceFile } from "../src/commands/script/script.ts";
|
||||
import { resolveWsNameForConfigFromFlags } from "../src/commands/sync/sync.ts";
|
||||
import type { SyncOptions } from "../src/core/conf.ts";
|
||||
|
||||
// Integration tests covering the bug where workspace-specific filenames used
|
||||
// the raw git branch name instead of the wmill.yaml workspace config key.
|
||||
// A per-item helper (findResourceFile) now resolves the effective wsName from
|
||||
// wmill.yaml before falling back to the branch name.
|
||||
|
||||
async function withGitRepoAndConfig(
|
||||
config: unknown,
|
||||
branch: string,
|
||||
fn: (tempDir: string) => Promise<void>,
|
||||
): Promise<void> {
|
||||
const tempDir = await mkdtemp(path.join(os.tmpdir(), "wmill_wskey_"));
|
||||
const originalCwd = process.cwd();
|
||||
try {
|
||||
// Set up a minimal git repo on the target branch so getCurrentGitBranch()
|
||||
// returns the expected value. An initial commit is needed so HEAD points
|
||||
// somewhere and `git rev-parse --abbrev-ref HEAD` succeeds.
|
||||
execSync(`git init -q -b ${branch}`, { cwd: tempDir });
|
||||
execSync(`git config user.email test@example.com`, { cwd: tempDir });
|
||||
execSync(`git config user.name test`, { cwd: tempDir });
|
||||
|
||||
await writeFile(
|
||||
path.join(tempDir, "wmill.yaml"),
|
||||
yamlStringify(config),
|
||||
);
|
||||
|
||||
execSync(`git add wmill.yaml && git commit -q -m init`, { cwd: tempDir });
|
||||
|
||||
process.chdir(tempDir);
|
||||
await fn(tempDir);
|
||||
} finally {
|
||||
process.chdir(originalCwd);
|
||||
await rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("resolveWsNameForGitBranch", () => {
|
||||
test("returns the wmill.yaml config key for a branch matched via gitBranch field", async () => {
|
||||
await withGitRepoAndConfig(
|
||||
{
|
||||
workspaces: {
|
||||
myKey: { gitBranch: "main", workspaceId: "prod" },
|
||||
},
|
||||
},
|
||||
"main",
|
||||
async () => {
|
||||
const wsName = await resolveWsNameForGitBranch("main");
|
||||
expect(wsName).toEqual("myKey");
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("returns the wmill.yaml config key when the branch equals the key and gitBranch is not set", async () => {
|
||||
await withGitRepoAndConfig(
|
||||
{
|
||||
workspaces: {
|
||||
staging: { workspaceId: "stg_workspace" },
|
||||
},
|
||||
},
|
||||
"staging",
|
||||
async () => {
|
||||
const wsName = await resolveWsNameForGitBranch("staging");
|
||||
expect(wsName).toEqual("staging");
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("falls back to the branch name when no matching workspace entry exists", async () => {
|
||||
await withGitRepoAndConfig(
|
||||
{
|
||||
workspaces: {
|
||||
production: { gitBranch: "main" },
|
||||
},
|
||||
},
|
||||
"feature-x",
|
||||
async () => {
|
||||
const wsName = await resolveWsNameForGitBranch("feature-x");
|
||||
expect(wsName).toEqual("feature-x");
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveWsNameForConfigFromFlags", () => {
|
||||
test("--workspace matching a config key resolves, even with --base-url", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
workspace: "test",
|
||||
baseUrl: "http://127.0.0.1:8080/",
|
||||
workspaces: {
|
||||
test: { gitBranch: "main" },
|
||||
prod: { gitBranch: "main" },
|
||||
},
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toEqual("test");
|
||||
});
|
||||
|
||||
test("--workspace not in config with --base-url returns undefined (treat as ad-hoc credential)", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
workspace: "adhocWorkspaceId",
|
||||
baseUrl: "https://other.windmill.dev/",
|
||||
workspaces: {
|
||||
test: { gitBranch: "main" },
|
||||
},
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined();
|
||||
});
|
||||
|
||||
test("--workspace matching config key resolves even without --base-url", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
workspace: "prod",
|
||||
workspaces: { test: {}, prod: {} },
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toEqual("prod");
|
||||
});
|
||||
|
||||
test("--branch takes precedence and looks up by gitBranch", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
branch: "main",
|
||||
workspace: "someOtherKey",
|
||||
workspaces: {
|
||||
test: { gitBranch: "main" },
|
||||
prod: { gitBranch: "release" },
|
||||
},
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toEqual("test");
|
||||
});
|
||||
|
||||
test("no flags returns undefined", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
workspaces: { test: {} },
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined();
|
||||
});
|
||||
|
||||
test("reserved key 'commonSpecificItems' is not accepted as a config key", () => {
|
||||
const opts: SyncOptions & { branch?: string; workspace?: string } = {
|
||||
workspace: "commonSpecificItems",
|
||||
workspaces: {
|
||||
test: {},
|
||||
commonSpecificItems: { variables: ["f/**"] } as any,
|
||||
},
|
||||
} as any;
|
||||
expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("findResourceFile picks the wsName-named file, not the branch-named file", () => {
|
||||
test("finds the workspace-specific resource file using the config key as suffix", async () => {
|
||||
await withGitRepoAndConfig(
|
||||
{
|
||||
workspaces: {
|
||||
myKey: { gitBranch: "main", workspaceId: "prod" },
|
||||
},
|
||||
},
|
||||
"main",
|
||||
async (tempDir) => {
|
||||
// Only the config-key-named file exists on disk. A wmill without this
|
||||
// fix would look for `f/foo.main.resource.yaml` (the branch name) and
|
||||
// either miss this file or pick a stale branch-named file.
|
||||
await mkdir(path.join(tempDir, "f"), { recursive: true });
|
||||
await writeFile(
|
||||
path.join(tempDir, "f/foo.myKey.resource.yaml"),
|
||||
"value: {}\nresource_type: text\n",
|
||||
);
|
||||
|
||||
const found = await findResourceFile("f/foo.resource.file.txt");
|
||||
expect(found).toEqual("f/foo.myKey.resource.yaml");
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user