mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 16:05:42 +00:00
fix(git-sync): license-gate preserved auto-pull; attach strips parent-only settings
- edit_git_sync_repository re-checks the runtime Enterprise gate against the EFFECTIVE repo state after preservation: the older-client arm copies the existing auto_pull back, which the request-side check never saw - attach_dev_workspace now mirrors the fork-creation copy on the attached workspace's own git sync: promotion repos dropped, auto_pull/fork PRs/PR error stripped, and any managed webhook deleted after commit (the attached workspace is parent-managed and must not keep pulling its old tracked branch) - integration test: attaching an auto-pull-enabled workspace strips it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm
This commit is contained in:
co-authored by
Claude Fable 5
parent
5787c2c355
commit
1aa8e1665e
@@ -3218,6 +3218,13 @@ async fn edit_git_sync_repository(
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
// The request-side license gate above only saw the submitted config; the
|
||||
// preservation can resurrect an enabled auto_pull (None arm), so re-check
|
||||
// the effective state before it gets written and reconciled.
|
||||
#[cfg(feature = "enterprise")]
|
||||
if updated.auto_pull.as_ref().is_some_and(|a| a.enabled) {
|
||||
check_auto_pull_license().await?;
|
||||
}
|
||||
*existing_repo = updated;
|
||||
} else {
|
||||
// Repository doesn't exist, add it as a new repository
|
||||
@@ -6291,6 +6298,45 @@ async fn attach_dev_workspace(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// The attached workspace is now parent-managed like any fork: its own
|
||||
// auto-pull (and webhook), fork PRs, and promotion repos must not stay
|
||||
// live — they'd keep pulling/pushing against its pre-attach tracked
|
||||
// branch. Mirror the fork-creation copy: keep sync repos only, strip the
|
||||
// parent-only fields, and delete any managed webhook after commit.
|
||||
#[allow(unused_mut)]
|
||||
let mut stripped_webhooks: Vec<(String, i64)> = Vec::new();
|
||||
if let Some(git_sync) = sqlx::query_scalar!(
|
||||
"SELECT git_sync FROM workspace_settings WHERE workspace_id = $1",
|
||||
&dev_w_id
|
||||
)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?
|
||||
.flatten()
|
||||
{
|
||||
if let Ok(mut settings) = serde_json::from_value::<WorkspaceGitSyncSettings>(git_sync) {
|
||||
settings
|
||||
.repositories
|
||||
.retain(|r| !r.use_individual_branch.unwrap_or(false));
|
||||
for r in settings.repositories.iter_mut() {
|
||||
if let Some(hook) = r.auto_pull.as_ref().and_then(|a| a.webhook_id) {
|
||||
stripped_webhooks.push((r.git_repo_resource_path.clone(), hook));
|
||||
}
|
||||
r.auto_pull = None;
|
||||
r.fork_open_prs = false;
|
||||
r.open_pr_error = None;
|
||||
}
|
||||
let serialized =
|
||||
serde_json::to_value(&settings).map_err(|e| Error::internal_err(e.to_string()))?;
|
||||
sqlx::query!(
|
||||
"UPDATE workspace_settings SET git_sync = $1 WHERE workspace_id = $2",
|
||||
serialized,
|
||||
&dev_w_id
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
}
|
||||
}
|
||||
|
||||
if req.lock_prod_deploy || req.lock_prod_forking {
|
||||
lock_prod_workspace(
|
||||
&mut tx,
|
||||
@@ -6316,6 +6362,17 @@ async fn attach_dev_workspace(
|
||||
// The dev workspace's parent just changed (none -> prod); drop its cached fork->parent mapping
|
||||
// so per-workspace job tags route to the prod family immediately rather than after the TTL.
|
||||
windmill_queue::tags::invalidate_fork_parent_cache(&dev_w_id);
|
||||
// Best-effort: the hooks captured before the strip above are unreachable now
|
||||
// (their auto_pull is gone), so remove them from GitHub.
|
||||
#[cfg(all(feature = "enterprise", feature = "private"))]
|
||||
for (path, hook_id) in stripped_webhooks {
|
||||
if let Ok(url) = windmill_common::git_sync_ee::resolve_repo_url(&db, &dev_w_id, &path).await
|
||||
{
|
||||
let _ =
|
||||
windmill_common::git_sync_ee::delete_repo_webhook(&db, &dev_w_id, &url, hook_id)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
// Drop the cached ancestor chains too — the workspace existed BEFORE the attach, so a
|
||||
// cached empty chain reads as "not a fork" and its ducklake jobs would write the shared
|
||||
// lake until the TTL. Descendants' chains also gained the new root.
|
||||
|
||||
@@ -1004,6 +1004,58 @@ class TestGitSyncAutoPull(GitSyncTestBase):
|
||||
f"{response.status_code}: {response.content.decode()}",
|
||||
)
|
||||
|
||||
def test_attach_dev_workspace_strips_auto_pull(self):
|
||||
"""Attaching a workspace as a dev workspace strips its own auto-pull:
|
||||
dev/fork sync is parent-managed, so a pre-attach auto-pull must not
|
||||
keep pulling the old tracked branch."""
|
||||
candidate_id = f"it-dev-cand-{uuid.uuid4().hex[:8]}"
|
||||
self._fork_workspaces_to_cleanup.append(candidate_id)
|
||||
candidate = WindmillClient(workspace=candidate_id)
|
||||
|
||||
repo_name, _ = self._create_test_repo()
|
||||
resource_path = f"u/admin/git_sync_{repo_name.replace('-', '_')}"
|
||||
candidate.create_resource(
|
||||
path=resource_path,
|
||||
resource_type="git_repository",
|
||||
value={
|
||||
"url": self._gitea.get_docker_clone_url(repo_name),
|
||||
"branch": "main",
|
||||
"is_github_app": False,
|
||||
},
|
||||
update_if_exists=True,
|
||||
)
|
||||
candidate.configure_git_sync({
|
||||
"repositories": [{
|
||||
"git_repo_resource_path": f"$res:{resource_path}",
|
||||
"use_individual_branch": False,
|
||||
"group_by_folder": False,
|
||||
"settings": {"include_type": ["script"], "include_path": ["**"]},
|
||||
"auto_pull": {"enabled": True, "mode": "polling"},
|
||||
}],
|
||||
})
|
||||
|
||||
response = self._client._client.post(
|
||||
f"/api/w/{self._client._workspace}/workspaces/attach_dev_workspace",
|
||||
json={"dev_workspace_id": candidate_id, "dev_workspace_label": "dev"},
|
||||
)
|
||||
self.assertEqual(
|
||||
response.status_code // 100,
|
||||
2,
|
||||
f"attach_dev_workspace failed: {response.content.decode()}",
|
||||
)
|
||||
|
||||
repo_settings = None
|
||||
for repo in (candidate.get_workspace_settings().get("git_sync") or {}).get(
|
||||
"repositories", []
|
||||
):
|
||||
if resource_path in repo.get("git_repo_resource_path", ""):
|
||||
repo_settings = repo
|
||||
self.assertIsNotNone(repo_settings, "candidate lost its sync repo on attach")
|
||||
self.assertIsNone(
|
||||
repo_settings.get("auto_pull"),
|
||||
f"auto_pull survived the attach: {repo_settings}",
|
||||
)
|
||||
|
||||
def test_webhook_receiver_ignores_unknown_deliveries(self):
|
||||
"""An unsolicited webhook delivery (no registered hook) is not an error
|
||||
and enqueues nothing."""
|
||||
|
||||
Reference in New Issue
Block a user