mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: keep instance groups when editing auto-invite (#11217)
* fix: keep instance groups when editing auto-invite and push them from sync Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: compare settings.yaml without undeclared instance groups on push Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: read a missing auto_invite as empty when diffing settings.yaml on push Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: update ee-repo-ref to f2fced19fcae81de7f6dac545010ce404c052e1b This commit updates the EE repository reference after PR #813 was merged in windmill-ee-private. Previous ee-repo-ref: cf4258c1232720ca3b82db2b22ea1fb4bca9533e New ee-repo-ref: f2fced19fcae81de7f6dac545010ce404c052e1b Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com> Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
This commit is contained in:
co-authored by
Claude Opus 5
windmill-internal-app[bot]
Ruben Fiszel
parent
5639187fec
commit
df61dea5fa
-15
@@ -1,15 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings SET auto_invite = $1 WHERE workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "255ba68caa78d0e814ea817693a319f769907025f9c7d1150d45e8ecb1bff4ab"
|
||||
}
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings SET auto_invite = (COALESCE(auto_invite, '{}'::jsonb) - 'domain') || $1::jsonb WHERE workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "a1bcf143135ecb9f32709a56a353d3246dc238920c3e155caa9cf767246fa6fb"
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
7e338e4dabf91689bfd7fb0333c6534040b17b59
|
||||
f2fced19fcae81de7f6dac545010ce404c052e1b
|
||||
|
||||
@@ -1166,3 +1166,47 @@ async fn test_create_service_account_drops_orphaned_group_memberships(
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(feature = "private")]
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_edit_auto_invite_preserves_instance_groups(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
|
||||
sqlx::query(
|
||||
r#"UPDATE workspace_settings
|
||||
SET auto_invite = '{"instance_groups": ["eng"], "instance_groups_roles": {"eng": "developer"}}'
|
||||
WHERE workspace_id = 'test-workspace'"#,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace/workspaces");
|
||||
|
||||
// enable, then disable
|
||||
for body in [
|
||||
json!({"operator": false, "invite_all": true, "auto_add": false}),
|
||||
json!({}),
|
||||
] {
|
||||
let resp = authed(client().post(format!("{base}/edit_auto_invite")))
|
||||
.json(&body)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{body}: {}", resp.text().await?);
|
||||
|
||||
let auto_invite: serde_json::Value = sqlx::query_scalar(
|
||||
"SELECT auto_invite FROM workspace_settings WHERE workspace_id = 'test-workspace'",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(auto_invite["instance_groups"], json!(["eng"]), "{body}");
|
||||
assert_eq!(
|
||||
auto_invite["instance_groups_roles"],
|
||||
json!({"eng": "developer"}),
|
||||
"{body}"
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2546,9 +2546,9 @@ export function preservePendingScriptLocks(
|
||||
}
|
||||
|
||||
// `sync push` never applies the workspace's display name from settings.yaml and
|
||||
// applies its color only when the local file carries one (see
|
||||
// pushWorkspaceSettings), so on a push the fields it would not apply must
|
||||
// compare equal, or the row is listed on every run.
|
||||
// applies its color and auto_invite.instance_groups only when the local file
|
||||
// carries them (see pushWorkspaceSettings), so on a push the fields it would not
|
||||
// apply must compare equal, or the row is listed on every run.
|
||||
const isWorkspaceSettingsFile = (p: string) =>
|
||||
/^settings(\.[^./\\]+)?\.(yaml|json)$/.test(p);
|
||||
function stripUnappliedSettingsFields(local: any, remote: any) {
|
||||
@@ -2558,6 +2558,20 @@ function stripUnappliedSettingsFields(local: any, remote: any) {
|
||||
delete local?.color;
|
||||
delete remote?.color;
|
||||
}
|
||||
// push reads a missing auto_invite as {} on both sides
|
||||
if (local) local.auto_invite ??= {};
|
||||
if (remote) remote.auto_invite ??= {};
|
||||
const localInvite = local?.auto_invite;
|
||||
const remoteInvite = remote?.auto_invite;
|
||||
if (localInvite?.instance_groups == null) {
|
||||
for (const invite of [localInvite, remoteInvite]) {
|
||||
delete invite?.instance_groups;
|
||||
delete invite?.instance_groups_roles;
|
||||
}
|
||||
} else {
|
||||
localInvite.instance_groups_roles ??= {};
|
||||
if (remoteInvite) remoteInvite.instance_groups_roles ??= {};
|
||||
}
|
||||
}
|
||||
|
||||
export async function compareDynFSElement(
|
||||
|
||||
@@ -239,8 +239,19 @@ export async function pushWorkspaceSettings(
|
||||
});
|
||||
}
|
||||
|
||||
// Handle auto_invite using grouped format
|
||||
if (!deepEqual(localSettings.auto_invite, settings.auto_invite)) {
|
||||
// Handle auto_invite using grouped format. The domain invite and the instance groups
|
||||
// are applied by separate endpoints, each rewriting only its own keys.
|
||||
const {
|
||||
instance_groups: localGroups,
|
||||
instance_groups_roles: localGroupRoles,
|
||||
...localDomainInvite
|
||||
} = localSettings.auto_invite ?? {};
|
||||
const {
|
||||
instance_groups: remoteGroups,
|
||||
instance_groups_roles: remoteGroupRoles,
|
||||
...remoteDomainInvite
|
||||
} = settings.auto_invite ?? {};
|
||||
if (!deepEqual(localDomainInvite, remoteDomainInvite)) {
|
||||
log.debug(`Updating auto invite...`);
|
||||
|
||||
const localAutoInvite = localSettings.auto_invite;
|
||||
@@ -278,6 +289,20 @@ export async function pushWorkspaceSettings(
|
||||
}
|
||||
}
|
||||
|
||||
// Only when settings.yaml declares instance_groups: clearing a group removes the
|
||||
// workspace members it granted, so an absent key must never clear it.
|
||||
if (
|
||||
localGroups != undefined &&
|
||||
(!deepEqual(localGroups, remoteGroups) ||
|
||||
!deepEqual(localGroupRoles ?? {}, remoteGroupRoles ?? {}))
|
||||
) {
|
||||
log.debug(`Updating instance groups...`);
|
||||
await wmill.editInstanceGroups({
|
||||
workspace,
|
||||
requestBody: { groups: localGroups, roles: localGroupRoles ?? {} },
|
||||
});
|
||||
}
|
||||
|
||||
if (!deepEqual(localSettings.ai_config, settings.ai_config)) {
|
||||
log.debug(`Updating copilot settings...`);
|
||||
await wmill.editCopilotConfig({
|
||||
|
||||
@@ -320,3 +320,29 @@ test("push: settings.yaml differing only by name or an unset color is not a chan
|
||||
});
|
||||
expect(await diff(otherColor, remote, skips)).toEqual(["edited settings.yaml"]);
|
||||
});
|
||||
|
||||
// A push applies auto_invite.instance_groups only when the local file declares
|
||||
// them (see pushWorkspaceSettings).
|
||||
test("push: settings.yaml without instance_groups is not a change", async () => {
|
||||
const remote = local({
|
||||
"settings.yaml":
|
||||
"name: prod\nauto_invite:\n enabled: false\n instance_groups:\n - eng\n instance_groups_roles:\n eng: developer\n",
|
||||
});
|
||||
const undeclared = local({
|
||||
"settings.yaml": "name: prod\nauto_invite:\n enabled: false\n",
|
||||
});
|
||||
const skips = { includeSettings: true };
|
||||
expect(await diff(undeclared, remote, skips)).toEqual([]);
|
||||
|
||||
const groupsOnlyRemote = local({
|
||||
"settings.yaml": "name: prod\nauto_invite:\n instance_groups:\n - eng\n",
|
||||
});
|
||||
const noAutoInvite = local({ "settings.yaml": "name: prod\n" });
|
||||
expect(await diff(noAutoInvite, groupsOnlyRemote, skips)).toEqual([]);
|
||||
|
||||
const otherGroups = local({
|
||||
"settings.yaml":
|
||||
"name: prod\nauto_invite:\n enabled: false\n instance_groups: []\n",
|
||||
});
|
||||
expect(await diff(otherGroups, remote, skips)).toEqual(["edited settings.yaml"]);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* Regression guard: `sync push` (pushWorkspaceSettings) applies the domain invite and
|
||||
* the instance groups of `auto_invite` through their own endpoints, and never clears
|
||||
* instance groups that settings.yaml does not declare.
|
||||
*/
|
||||
|
||||
import { expect, test, describe, beforeEach, mock } from "bun:test";
|
||||
|
||||
let editAutoInviteCalls: unknown[] = [];
|
||||
let editInstanceGroupsCalls: unknown[] = [];
|
||||
const remoteAutoInvite = {
|
||||
enabled: true,
|
||||
domain: "*",
|
||||
operator: false,
|
||||
mode: "invite",
|
||||
instance_groups: ["eng"],
|
||||
instance_groups_roles: { eng: "developer" },
|
||||
};
|
||||
|
||||
// Every wmill.* call reachable from pushWorkspaceSettings is stubbed: bun shares one
|
||||
// mocked module across test files, and names missing from whichever mock loads first
|
||||
// stay missing for the others.
|
||||
mock.module("../gen/services.gen.ts", () => ({
|
||||
getSettings: async () => ({ auto_invite: remoteAutoInvite }),
|
||||
getWorkspaceName: async () => "phoenix",
|
||||
changeWorkspaceName: async () => {},
|
||||
changeWorkspaceColor: async () => {},
|
||||
editWebhook: async () => {},
|
||||
editAutoInvite: async (a: unknown) => {
|
||||
editAutoInviteCalls.push(a);
|
||||
},
|
||||
editInstanceGroups: async (a: unknown) => {
|
||||
editInstanceGroupsCalls.push(a);
|
||||
},
|
||||
editErrorHandler: async () => {},
|
||||
editSuccessHandler: async () => {},
|
||||
editCopilotConfig: async () => {},
|
||||
editLargeFileStorageConfig: async () => {},
|
||||
editWorkspaceGitSyncConfig: async () => {},
|
||||
editWorkspaceDefaultApp: async () => {},
|
||||
editDefaultScripts: async () => {},
|
||||
workspaceMuteCriticalAlertsUi: async () => {},
|
||||
updateOperatorSettings: async () => {},
|
||||
editDataTableConfig: async () => {},
|
||||
editSlackCommand: async () => {},
|
||||
setWorkspaceSlackOauthConfig: async () => {},
|
||||
deleteWorkspaceSlackOauthConfig: async () => {},
|
||||
}));
|
||||
|
||||
const { pushWorkspaceSettings } = await import("../src/core/settings.ts");
|
||||
|
||||
describe("pushWorkspaceSettings auto_invite", () => {
|
||||
beforeEach(() => {
|
||||
editAutoInviteCalls = [];
|
||||
editInstanceGroupsCalls = [];
|
||||
});
|
||||
|
||||
test("an instance-group-only change updates the groups and leaves the domain invite", async () => {
|
||||
await pushWorkspaceSettings("phoenix", "settings", undefined, {
|
||||
name: "phoenix",
|
||||
auto_invite: { ...remoteAutoInvite, instance_groups_roles: { eng: "admin" } },
|
||||
});
|
||||
expect(editAutoInviteCalls.length).toBe(0);
|
||||
expect(editInstanceGroupsCalls).toEqual([
|
||||
{
|
||||
workspace: "phoenix",
|
||||
requestBody: { groups: ["eng"], roles: { eng: "admin" } },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
test("a settings.yaml without instance_groups does not clear them", async () => {
|
||||
const { instance_groups: _g, instance_groups_roles: _r, ...domainInvite } =
|
||||
remoteAutoInvite;
|
||||
await pushWorkspaceSettings("phoenix", "settings", undefined, {
|
||||
name: "phoenix",
|
||||
auto_invite: { ...domainInvite, operator: true },
|
||||
});
|
||||
expect(editAutoInviteCalls.length).toBe(1);
|
||||
expect(editInstanceGroupsCalls.length).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -31,6 +31,7 @@ mock.module("../gen/services.gen.ts", () => ({
|
||||
editWebhookCalls.push(a);
|
||||
},
|
||||
editAutoInvite: async () => {},
|
||||
editInstanceGroups: async () => {},
|
||||
editErrorHandler: async () => {},
|
||||
editSuccessHandler: async () => {},
|
||||
editCopilotConfig: async () => {},
|
||||
|
||||
Reference in New Issue
Block a user