mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-12 00:06:14 +00:00
test: auto-pull e2e integration tests; fix PR comment table formatting
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
0307b7fda6
commit
41cc2072ac
@@ -180,6 +180,9 @@ jobs:
|
||||
DENO_PATH: deno
|
||||
BUN_PATH: bun
|
||||
NODE_BIN_PATH: node
|
||||
# The auto-pull poller's SSRF guard rejects localhost git remotes;
|
||||
# the tests' Gitea runs on localhost.
|
||||
ALLOW_LOCAL_GIT_REMOTES: "true"
|
||||
run: |
|
||||
./target/debug/windmill &
|
||||
echo "Waiting for Windmill to be ready..."
|
||||
|
||||
@@ -2521,6 +2521,13 @@ async fn validate_git_url(url: &str) -> Result<()> {
|
||||
let host = extract_host_from_git_url(url)
|
||||
.ok_or_else(|| Error::BadRequest("Could not parse hostname from git URL".to_string()))?;
|
||||
|
||||
// CI/dev escape hatch: integration tests run their git remote (a Gitea
|
||||
// container) on localhost, which the network-target checks below reject.
|
||||
// Scheme and option-injection validation above still applies.
|
||||
if std::env::var("ALLOW_LOCAL_GIT_REMOTES").is_ok_and(|v| v == "true" || v == "1") {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if host == "localhost" || host.ends_with(".local") || host == "[::1]" {
|
||||
return Err(Error::BadRequest(
|
||||
"Git URLs targeting localhost or local network are not allowed".to_string(),
|
||||
|
||||
@@ -1215,7 +1215,7 @@ async fn maybe_post_git_sync_check(
|
||||
.map(|s| &s[..s.len().min(7)])
|
||||
.unwrap_or("latest");
|
||||
let body = format!(
|
||||
"{marker}\n### Windmill deploy preview\n\n | | |\n|---|---|\n | **Workspace** | `{workspace_id}` |\n | **Status** | {title} |\n | **Commit** | `{head}` |\n\n <details><summary>Details</summary>\n\n{summary}\n\n</details>"
|
||||
"{marker}\n### Windmill deploy preview\n\n| | |\n|---|---|\n| **Workspace** | `{workspace_id}` |\n| **Status** | {title} |\n| **Commit** | `{head}` |\n\n<details><summary>Details</summary>\n\n{summary}\n\n</details>"
|
||||
);
|
||||
if let Err(e) = windmill_common::git_sync_ee::upsert_pr_comment(
|
||||
db,
|
||||
|
||||
@@ -19,7 +19,12 @@ def unique_name(prefix: str = "git-sync-test") -> str:
|
||||
return f"{prefix}-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
|
||||
class TestGitSync(unittest.TestCase):
|
||||
class GitSyncTestBase(unittest.TestCase):
|
||||
"""Shared fixture + helpers for git sync e2e tests (no tests of its own).
|
||||
|
||||
setUpClass binds the client and cleanup lists on each concrete subclass,
|
||||
so every test class gets its own Windmill client and cleanup scope."""
|
||||
|
||||
_client: WindmillClient
|
||||
_gitea: GiteaClient
|
||||
_repos_to_cleanup: list
|
||||
@@ -167,6 +172,56 @@ class TestGitSync(unittest.TestCase):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def _wait_until(self, predicate, timeout: int, interval: int = 5, message: str = ""):
|
||||
"""Poll predicate() until it returns truthy or timeout (seconds) elapses.
|
||||
Exceptions from the predicate count as 'not yet' (transient API errors)."""
|
||||
start = time.time()
|
||||
last_error = None
|
||||
while time.time() - start < timeout:
|
||||
try:
|
||||
if predicate():
|
||||
return
|
||||
last_error = None
|
||||
except Exception as e:
|
||||
last_error = e
|
||||
time.sleep(interval)
|
||||
suffix = f" (last error: {last_error})" if last_error else ""
|
||||
self.fail(f"Timed out after {timeout}s: {message}{suffix}")
|
||||
|
||||
def _find_repo_settings(self, resource_path: str) -> dict:
|
||||
"""Return this workspace's stored git sync settings for the given resource."""
|
||||
settings = self._client.get_workspace_settings()
|
||||
for repo in (settings.get("git_sync") or {}).get("repositories", []):
|
||||
if resource_path in repo.get("git_repo_resource_path", ""):
|
||||
return repo
|
||||
return None
|
||||
|
||||
def _deploy_seed_script(self, name_prefix: str) -> str:
|
||||
"""Deploy a script and wait for its push to git. Returns the script path."""
|
||||
initial_count = self._client.count_deployment_callback_jobs()
|
||||
script_path = f"u/admin/{unique_name(name_prefix)}"
|
||||
self._client.create_script(
|
||||
path=script_path,
|
||||
content=ts_script("return 'seed'"),
|
||||
language="bun",
|
||||
)
|
||||
self._client.wait_for_sync_jobs(initial_count, min_new=1)
|
||||
time.sleep(3)
|
||||
return script_path
|
||||
|
||||
def _repo_script_file(self, repo_name: str, script_path: str, branch: str = None) -> str:
|
||||
"""Find the .ts file for a deployed script in the repo."""
|
||||
repo_dir = self._clone_repo(repo_name, branch=branch)
|
||||
files = self._list_repo_files(repo_dir)
|
||||
matching = [f for f in files if script_path in f and f.endswith(".ts")]
|
||||
self.assertTrue(
|
||||
len(matching) > 0,
|
||||
f"Expected '{script_path}' .ts file in repo files: {files}",
|
||||
)
|
||||
return matching[0]
|
||||
|
||||
|
||||
class TestGitSync(GitSyncTestBase):
|
||||
# ──────────────────────────────────────────────────
|
||||
# Core happy-path tests
|
||||
# ──────────────────────────────────────────────────
|
||||
@@ -735,3 +790,242 @@ class TestGitSync(unittest.TestCase):
|
||||
len(fork_branches) > 0,
|
||||
f"Expected a wm-fork branch in the repo after forking, got: {branches}",
|
||||
)
|
||||
|
||||
|
||||
class TestGitSyncAutoPull(GitSyncTestBase):
|
||||
"""Auto-pull (git → Windmill, EE): polling, fork-branch routing, settings
|
||||
semantics. Webhook delivery and PR features need a GitHub App and are
|
||||
covered by manual verification instead."""
|
||||
|
||||
# The poller visits repos about once a minute; a pull then runs as a job.
|
||||
# Two poll cycles + job execution, with slack for a loaded CI runner.
|
||||
PULL_TIMEOUT = 240
|
||||
|
||||
def _seed_wmill_yaml(self, repo_name: str, branch: str = "main"):
|
||||
"""Commit a minimal wmill.yaml: the pull CLI requires one in the repo.
|
||||
Real setups get it from the init/settings-push flow; pushes alone
|
||||
don't write it."""
|
||||
self._gitea.create_file(
|
||||
repo_name,
|
||||
"wmill.yaml",
|
||||
"defaultTs: bun\n"
|
||||
"includes:\n"
|
||||
' - "**"\n'
|
||||
"excludes: []\n"
|
||||
"codebases: []\n"
|
||||
"skipVariables: true\n"
|
||||
"skipResources: true\n"
|
||||
"skipResourceTypes: true\n"
|
||||
"skipSecrets: true\n"
|
||||
"includeSchedules: false\n"
|
||||
"includeTriggers: false\n",
|
||||
branch=branch,
|
||||
)
|
||||
|
||||
def _configure_auto_pull(self, resource_path: str, sync_forks: bool = False):
|
||||
"""Single sync repo with auto-pull enabled in polling mode."""
|
||||
auto_pull = {"enabled": True, "mode": "polling"}
|
||||
if sync_forks:
|
||||
auto_pull["sync_forks"] = True
|
||||
self._client.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": auto_pull,
|
||||
}],
|
||||
})
|
||||
|
||||
def test_polling_applies_remote_commit(self):
|
||||
"""A commit pushed to the tracked branch is deployed into the workspace
|
||||
by the poller, the pull status is recorded, and the resulting no-op
|
||||
push callback does not add a commit (no sync loop)."""
|
||||
repo_name, _ = self._create_test_repo()
|
||||
resource_path = self._setup_git_sync_resource(repo_name)
|
||||
self._configure_single_repo_sync(resource_path, include_type=["script"])
|
||||
|
||||
# Seed the repo through a normal deploy, then find the script's file.
|
||||
script_path = self._deploy_seed_script("autopull")
|
||||
script_file = self._repo_script_file(repo_name, script_path)
|
||||
self._seed_wmill_yaml(repo_name)
|
||||
|
||||
self._configure_auto_pull(resource_path)
|
||||
|
||||
# External commit on the tracked branch (not [WM]-prefixed).
|
||||
self._gitea.create_file(
|
||||
repo_name, script_file, ts_script("return 'pulled from git'")
|
||||
)
|
||||
|
||||
self._wait_until(
|
||||
lambda: "pulled from git" in self._client.get_script_content(script_path),
|
||||
timeout=self.PULL_TIMEOUT,
|
||||
message=f"workspace script {script_path} was not updated from git",
|
||||
)
|
||||
|
||||
# Pull status is recorded on the repo settings.
|
||||
repo_settings = self._find_repo_settings(resource_path)
|
||||
status = (repo_settings.get("auto_pull") or {}).get("last_pull_status") or {}
|
||||
self.assertTrue(
|
||||
status.get("success"),
|
||||
f"Expected successful last_pull_status, got: {repo_settings.get('auto_pull')}",
|
||||
)
|
||||
|
||||
# The pull-caused deploy triggers a push callback; since the workspace
|
||||
# now matches the repo it must not create a commit (loop safety).
|
||||
time.sleep(10)
|
||||
repo_dir = self._clone_repo(repo_name)
|
||||
last_msg = self._get_last_commit_message(repo_dir)
|
||||
self.assertIn(
|
||||
script_file,
|
||||
last_msg,
|
||||
f"Expected the external commit to stay the branch head (no [WM] "
|
||||
f"loop commit), got: {last_msg!r}",
|
||||
)
|
||||
|
||||
# A whole-config resave without server-owned fields (what a UI/CLI
|
||||
# round-trip sends) must not clobber the recorded pull state.
|
||||
self._configure_auto_pull(resource_path)
|
||||
repo_settings = self._find_repo_settings(resource_path)
|
||||
status = (repo_settings.get("auto_pull") or {}).get("last_pull_status") or {}
|
||||
self.assertTrue(
|
||||
status.get("success"),
|
||||
f"Expected last_pull_status to survive a config resave, got: "
|
||||
f"{repo_settings.get('auto_pull')}",
|
||||
)
|
||||
|
||||
def test_fork_branch_commit_deploys_into_fork(self):
|
||||
"""With sync_forks on the parent, a commit on a fork's wm-fork/** branch
|
||||
is deployed into the fork workspace and leaves the parent untouched."""
|
||||
repo_name, _ = self._create_test_repo()
|
||||
resource_path = self._setup_git_sync_resource(repo_name)
|
||||
self._configure_single_repo_sync(resource_path, include_type=["script"])
|
||||
|
||||
script_path = self._deploy_seed_script("forkpull")
|
||||
script_file = self._repo_script_file(repo_name, script_path)
|
||||
# Seed before the fork branch is created so the branch inherits it.
|
||||
self._seed_wmill_yaml(repo_name)
|
||||
|
||||
self._configure_auto_pull(resource_path, sync_forks=True)
|
||||
|
||||
# Create the fork (branch first, then workspace), like the UI does.
|
||||
fork_id = f"wm-fork-{uuid.uuid4().hex[:8]}"
|
||||
self._fork_workspaces_to_cleanup.append(fork_id)
|
||||
job_ids = self._client.create_workspace_fork_branch(fork_id, f"Fork {fork_id}")
|
||||
if job_ids:
|
||||
self._client.wait_for_jobs_by_ids(job_ids, timeout=90)
|
||||
time.sleep(3)
|
||||
self._client.create_workspace_fork(fork_id, f"Fork {fork_id}")
|
||||
|
||||
fork_branch = f"wm-fork/main/{fork_id[len('wm-fork-'):]}"
|
||||
self._gitea.create_file(
|
||||
repo_name, script_file, ts_script("return 'fork only'"),
|
||||
branch=fork_branch,
|
||||
)
|
||||
|
||||
fork_client = WindmillClient(workspace=fork_id)
|
||||
self._wait_until(
|
||||
lambda: "fork only" in fork_client.get_script_content(script_path),
|
||||
timeout=self.PULL_TIMEOUT,
|
||||
message=f"fork workspace {fork_id} did not receive the fork-branch commit",
|
||||
)
|
||||
|
||||
# The parent workspace must not see the fork-branch content.
|
||||
self.assertNotIn(
|
||||
"fork only",
|
||||
self._client.get_script_content(script_path),
|
||||
"Parent workspace received a commit from a fork branch",
|
||||
)
|
||||
|
||||
def test_settings_normalization_and_redaction(self):
|
||||
"""Webhook mode on a token repo is persisted as polling; server-owned
|
||||
webhook fields are never exposed; legacy repos gain no auto_pull key."""
|
||||
repo_name, _ = self._create_test_repo()
|
||||
resource_path = self._setup_git_sync_resource(repo_name)
|
||||
|
||||
# Token-based repos can't register webhooks: a webhook-mode save is
|
||||
# normalized to polling and that normalization is persisted.
|
||||
self._client.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": "webhook"},
|
||||
}],
|
||||
})
|
||||
repo_settings = self._find_repo_settings(resource_path)
|
||||
auto_pull = repo_settings.get("auto_pull") or {}
|
||||
self.assertEqual(
|
||||
auto_pull.get("mode"),
|
||||
"polling",
|
||||
f"Expected webhook mode to normalize to polling on a token repo: {auto_pull}",
|
||||
)
|
||||
self.assertNotIn("webhook_secret", auto_pull)
|
||||
self.assertIsNone(auto_pull.get("webhook_id"))
|
||||
|
||||
# A repo saved without auto_pull stays without it (legacy round-trip).
|
||||
self._configure_single_repo_sync(resource_path, include_type=["script"])
|
||||
repo_settings = self._find_repo_settings(resource_path)
|
||||
self.assertIsNone(
|
||||
repo_settings.get("auto_pull"),
|
||||
f"Legacy repo config unexpectedly gained auto_pull: {repo_settings}",
|
||||
)
|
||||
|
||||
def test_fork_rejects_parent_only_git_sync_settings(self):
|
||||
"""Fork workspaces cannot enable auto-pull, promotion mode, or
|
||||
fork-PR creation on their own git sync settings."""
|
||||
repo_name, _ = self._create_test_repo()
|
||||
resource_path = self._setup_git_sync_resource(repo_name)
|
||||
self._configure_single_repo_sync(resource_path, include_type=["script"])
|
||||
|
||||
fork_id = f"wm-fork-{uuid.uuid4().hex[:8]}"
|
||||
self._fork_workspaces_to_cleanup.append(fork_id)
|
||||
self._client.create_workspace_fork(fork_id, f"Fork {fork_id}")
|
||||
fork_client = WindmillClient(workspace=fork_id)
|
||||
|
||||
base_repo = {"git_repo_resource_path": f"$res:{resource_path}"}
|
||||
rejected = [
|
||||
{**base_repo, "auto_pull": {"enabled": True, "mode": "polling"}},
|
||||
{**base_repo, "use_individual_branch": True},
|
||||
{**base_repo, "fork_open_prs": True},
|
||||
]
|
||||
for repo in rejected:
|
||||
response = fork_client.edit_git_sync_repository(
|
||||
f"$res:{resource_path}", repo
|
||||
)
|
||||
self.assertEqual(
|
||||
response.status_code,
|
||||
400,
|
||||
f"Expected 400 saving {repo} on a fork, got "
|
||||
f"{response.status_code}: {response.content.decode()}",
|
||||
)
|
||||
|
||||
def test_webhook_receiver_ignores_unknown_deliveries(self):
|
||||
"""An unsolicited webhook delivery (no registered hook) is not an error
|
||||
and enqueues nothing."""
|
||||
initial_count = self._client.count_deployment_callback_jobs()
|
||||
response = self._client._client.post(
|
||||
f"/api/w/{self._client._workspace}/github_app/webhook",
|
||||
json={"ref": "refs/heads/main", "after": "0" * 40},
|
||||
headers={
|
||||
"X-GitHub-Event": "push",
|
||||
"X-GitHub-Hook-ID": "999999999",
|
||||
"X-Hub-Signature-256": "sha256=" + "0" * 64,
|
||||
},
|
||||
)
|
||||
self.assertLess(
|
||||
response.status_code,
|
||||
500,
|
||||
f"Webhook receiver errored on unknown delivery: "
|
||||
f"{response.status_code} {response.content.decode()}",
|
||||
)
|
||||
time.sleep(3)
|
||||
self.assertEqual(
|
||||
self._client.count_deployment_callback_jobs(),
|
||||
initial_count,
|
||||
"Unknown webhook delivery enqueued a job",
|
||||
)
|
||||
|
||||
@@ -466,6 +466,29 @@ class WindmillClient:
|
||||
raise Exception(response.content.decode())
|
||||
return response.content.decode()
|
||||
|
||||
def edit_git_sync_repository(self, git_repo_resource_path: str, repository: dict):
|
||||
"""POST the per-repository git sync settings endpoint. Returns the raw
|
||||
response so callers can assert on rejections (e.g. fork guards)."""
|
||||
return self._client.post(
|
||||
f"/api/w/{self._workspace}/workspaces/edit_git_sync_repository",
|
||||
json={
|
||||
"git_repo_resource_path": git_repo_resource_path,
|
||||
"repository": repository,
|
||||
},
|
||||
)
|
||||
|
||||
def get_workspace_settings(self) -> dict:
|
||||
response = self._client.get(f"/api/w/{self._workspace}/workspaces/get_settings")
|
||||
if response.status_code // 100 != 2:
|
||||
raise Exception(response.content.decode())
|
||||
return response.json()
|
||||
|
||||
def get_script_content(self, path: str) -> str:
|
||||
response = self._client.get(f"/api/w/{self._workspace}/scripts/get/p/{path}")
|
||||
if response.status_code // 100 != 2:
|
||||
raise Exception(response.content.decode())
|
||||
return response.json().get("content", "")
|
||||
|
||||
def get_completed_jobs(self, job_kinds: str = None, success: bool = None):
|
||||
params = {"per_page": 1000}
|
||||
if job_kinds:
|
||||
|
||||
Reference in New Issue
Block a user