mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-18 00:03:11 +00:00
feat(cli): sync variable expiry and the workspace expiration handler
`wmill sync` carries `value_expires_at` on variables in both directions, and pushes the workspace handler alongside the error and success handlers in the grouped settings format. Dropping the key from a variable's yaml clears the remote expiry, so the file stays canonical rather than accumulating fields that can only ever be added. The handler follows the same rule, with the caveat recorded where the type is declared: an omitted handler only reaches the clear when some other setting differs, because the push short-circuits on a comparison that walks the local object's keys and so never sees a missing one. That gate is shared with the two existing handlers and is left as it is here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5782757d0c
commit
67da34668f
@@ -97,6 +97,7 @@ export interface VariableFile {
|
||||
description: string;
|
||||
account?: number;
|
||||
is_oauth?: boolean;
|
||||
value_expires_at?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -153,7 +154,16 @@ export async function pushVariable(
|
||||
}
|
||||
|
||||
if (variable) {
|
||||
if (isSuperset(localVariable, variable)) {
|
||||
// The server reads an absent `value_expires_at` as "leave the stored date alone", so
|
||||
// dropping the key from the file has to be sent as an explicit `null`. `isSuperset`
|
||||
// only compares keys the file still has, so a removed expiry is also invisible to it:
|
||||
// without both halves the clear never applies and every later push re-reports it.
|
||||
const clearsValueExpiresAt =
|
||||
localVariable.value_expires_at === undefined &&
|
||||
variable.value_expires_at !== undefined &&
|
||||
variable.value_expires_at !== null;
|
||||
|
||||
if (!clearsValueExpiresAt && isSuperset(localVariable, variable)) {
|
||||
log.debug(`Variable ${remotePath} is up-to-date`);
|
||||
return;
|
||||
}
|
||||
@@ -181,6 +191,7 @@ export async function pushVariable(
|
||||
...localVariable,
|
||||
is_secret: nextIsSecret,
|
||||
...(wsSpecific !== undefined ? { ws_specific: wsSpecific } : {}),
|
||||
...(clearsValueExpiresAt ? { value_expires_at: null } : {}),
|
||||
},
|
||||
});
|
||||
} else {
|
||||
|
||||
@@ -34,12 +34,21 @@ export interface SuccessHandlerConfig {
|
||||
extra_args?: any;
|
||||
}
|
||||
|
||||
export interface VariableExpirationHandlerConfig {
|
||||
path?: string;
|
||||
extra_args?: any;
|
||||
muted_on_user_path?: boolean;
|
||||
}
|
||||
|
||||
export interface SimplifiedSettings {
|
||||
// Grouped format (current). Explicit `null` on error_handler / success_handler
|
||||
// signals "clear the remote value"; `undefined` means "not managed by git".
|
||||
// Grouped format (current). On push the YAML is canonical: a handler that is
|
||||
// absent or explicitly `null` clears the remote value, a present object upserts.
|
||||
// An omitted handler reaches that clear only when some other setting differs:
|
||||
// `isSuperset` compares local's own keys, so it never sees the missing one.
|
||||
auto_invite?: AutoInviteConfig;
|
||||
error_handler?: ErrorHandlerConfig | null;
|
||||
success_handler?: SuccessHandlerConfig | null;
|
||||
variable_expiration_handler?: VariableExpirationHandlerConfig | null;
|
||||
|
||||
// Other fields
|
||||
webhook?: string;
|
||||
@@ -150,6 +159,17 @@ export function migrateToGroupedFormat(settings: any): SimplifiedSettings {
|
||||
};
|
||||
}
|
||||
|
||||
// Handle variable_expiration_handler: same null-clears semantics. It has no flat
|
||||
// legacy form — the setting postdates the grouped format.
|
||||
if (settings.variable_expiration_handler === null) {
|
||||
result.variable_expiration_handler = null;
|
||||
} else if (
|
||||
settings.variable_expiration_handler &&
|
||||
typeof settings.variable_expiration_handler === "object"
|
||||
) {
|
||||
result.variable_expiration_handler = settings.variable_expiration_handler;
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -192,6 +212,9 @@ export async function pushWorkspaceSettings(
|
||||
auto_invite: remoteSettings.auto_invite as AutoInviteConfig | undefined,
|
||||
error_handler: remoteSettings.error_handler as ErrorHandlerConfig | undefined,
|
||||
success_handler: remoteSettings.success_handler as SuccessHandlerConfig | undefined,
|
||||
variable_expiration_handler: remoteSettings.variable_expiration_handler as
|
||||
| VariableExpirationHandlerConfig
|
||||
| undefined,
|
||||
webhook: remoteSettings.webhook,
|
||||
ai_config: remoteSettings.ai_config,
|
||||
large_file_storage: remoteSettings.large_file_storage,
|
||||
@@ -311,6 +334,26 @@ export async function pushWorkspaceSettings(
|
||||
});
|
||||
}
|
||||
|
||||
// Handle variable_expiration_handler using grouped format. Same semantics as error_handler.
|
||||
if (
|
||||
!deepEqual(
|
||||
localSettings.variable_expiration_handler,
|
||||
settings.variable_expiration_handler
|
||||
)
|
||||
) {
|
||||
log.debug(`Updating variable expiration handler...`);
|
||||
const localVariableExpirationHandler = localSettings.variable_expiration_handler;
|
||||
await wmill.editVariableExpirationHandler({
|
||||
workspace,
|
||||
requestBody: {
|
||||
path: localVariableExpirationHandler?.path,
|
||||
extra_args: localVariableExpirationHandler?.extra_args,
|
||||
muted_on_user_path:
|
||||
localVariableExpirationHandler?.muted_on_user_path ?? false,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
if (
|
||||
!deepEqual(localSettings.large_file_storage, settings.large_file_storage)
|
||||
) {
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { deployItem } from "../windmill-utils-internal/src/deploy.ts";
|
||||
|
||||
// Workspace deploy builds the variable body field by field, so a field the variable
|
||||
// carries but the body forgets is dropped on promotion — silently, since the deploy
|
||||
// still reports success. The clear half matters as much as the carry: the server reads
|
||||
// an absent `value_expires_at` as "leave the stored date alone", so a source with no
|
||||
// expiry has to say so explicitly or the target keeps a date the source no longer has.
|
||||
function recordingProvider(
|
||||
captured: [string, any][],
|
||||
exists: boolean,
|
||||
value_expires_at: string | null,
|
||||
) {
|
||||
return {
|
||||
existsVariable: async () => exists,
|
||||
getVariable: async () => ({
|
||||
path: "f/x/v",
|
||||
value: "secret",
|
||||
is_secret: true,
|
||||
description: "d",
|
||||
value_expires_at,
|
||||
}),
|
||||
createVariable: async (p: any) =>
|
||||
void captured.push(["createVariable", p.requestBody]),
|
||||
updateVariable: async (p: any) =>
|
||||
void captured.push(["updateVariable", p.requestBody]),
|
||||
} as any;
|
||||
}
|
||||
|
||||
test("deployItem: carries value_expires_at, and clears it explicitly", async () => {
|
||||
const captured: [string, any][] = [];
|
||||
|
||||
// The field is written out once per branch, so exercise both: a variable absent from
|
||||
// the target (create) and one already there (update).
|
||||
for (const exists of [false, true]) {
|
||||
await deployItem(
|
||||
recordingProvider(captured, exists, "2027-03-15T08:00:00Z"),
|
||||
"variable" as any,
|
||||
"f/x/v",
|
||||
"src",
|
||||
"dst",
|
||||
);
|
||||
await deployItem(
|
||||
recordingProvider(captured, exists, null),
|
||||
"variable" as any,
|
||||
"f/x/v",
|
||||
"src",
|
||||
"dst",
|
||||
);
|
||||
}
|
||||
|
||||
expect(captured.map(([fn]) => fn)).toEqual([
|
||||
"createVariable",
|
||||
"createVariable",
|
||||
"updateVariable",
|
||||
"updateVariable",
|
||||
]);
|
||||
expect(captured.map(([, body]) => body.value_expires_at)).toEqual([
|
||||
"2027-03-15T08:00:00Z",
|
||||
null,
|
||||
"2027-03-15T08:00:00Z",
|
||||
null,
|
||||
]);
|
||||
});
|
||||
@@ -133,6 +133,68 @@ describe("variable", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("push clears value_expires_at when the key is dropped from the file", async () => {
|
||||
await withTestBackend(async (backend, tempDir) => {
|
||||
await setupWorkspaceProfile(backend);
|
||||
|
||||
const uniqueId = Date.now();
|
||||
const path = `f/test/expiry_var_${uniqueId}`;
|
||||
|
||||
const createResp = await backend.apiRequest!(
|
||||
`/api/w/${backend.workspace}/variables/create`,
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
path,
|
||||
value: "original_value",
|
||||
is_secret: false,
|
||||
description: "Expiring",
|
||||
value_expires_at: "2027-03-15T08:00:00Z",
|
||||
}),
|
||||
}
|
||||
);
|
||||
expect(createResp.status).toBeLessThan(300);
|
||||
await createResp.text();
|
||||
|
||||
await writeFile(
|
||||
join(tempDir, "wmill.yaml"),
|
||||
`defaultTs: bun\nincludes:\n - "**"\nexcludes: []\n`,
|
||||
"utf-8"
|
||||
);
|
||||
await mkdir(join(tempDir, "f", "test"), { recursive: true });
|
||||
// No value_expires_at: the server reads an absent field as "leave the stored
|
||||
// date alone", so the clear only lands if the CLI sends an explicit null.
|
||||
await writeFile(
|
||||
join(tempDir, `${path}.variable.yaml`),
|
||||
`value: "original_value"\nis_secret: false\ndescription: "Expiring"\n`,
|
||||
"utf-8"
|
||||
);
|
||||
|
||||
const pushResult = await backend.runCLICommand(
|
||||
["sync", "push", "--yes", "--includes", `${path}**`],
|
||||
tempDir
|
||||
);
|
||||
expect(pushResult.code).toEqual(0);
|
||||
|
||||
const apiResp = await backend.apiRequest!(
|
||||
`/api/w/${backend.workspace}/variables/get/${path}`
|
||||
);
|
||||
expect(apiResp.status).toEqual(200);
|
||||
const varData = await apiResp.json();
|
||||
expect(varData.value_expires_at ?? null).toBeNull();
|
||||
|
||||
// A clear that does not apply leaves the same diff pending forever, so the
|
||||
// push reporting success is not on its own evidence that it landed.
|
||||
const secondPush = await backend.runCLICommand(
|
||||
["sync", "push", "--yes", "--includes", `${path}**`],
|
||||
tempDir
|
||||
);
|
||||
expect(secondPush.code).toEqual(0);
|
||||
expect(secondPush.stdout).toContain("0 changes to apply");
|
||||
});
|
||||
});
|
||||
|
||||
test("add creates secret by default and preserves fields on update", async () => {
|
||||
await withTestBackend(async (backend, tempDir) => {
|
||||
await setupWorkspaceProfile(backend);
|
||||
|
||||
@@ -598,6 +598,10 @@ export async function deployItem(
|
||||
value: variable.value ?? "",
|
||||
is_secret: variable.is_secret,
|
||||
description: variable.description ?? "",
|
||||
// Sent as an explicit null when the source has no expiry: an absent key means
|
||||
// "leave the stored date alone", which would strand the target on a date the
|
||||
// source has already moved or cleared.
|
||||
value_expires_at: variable.value_expires_at ?? null,
|
||||
},
|
||||
alreadyEncrypted: false,
|
||||
});
|
||||
@@ -609,6 +613,7 @@ export async function deployItem(
|
||||
value: variable.value ?? "",
|
||||
is_secret: variable.is_secret,
|
||||
description: variable.description ?? "",
|
||||
value_expires_at: variable.value_expires_at ?? null,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user