From b2c7adcf98adff0bf3f64a9621e3bc0a533f0472 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari <48932219+adityachoudhari26@users.noreply.github.com> Date: Sun, 27 Oct 2024 13:06:19 -0700 Subject: [PATCH] fix: Get externalid from webhook (#177) --- apps/event-worker/package.json | 1 - apps/event-worker/src/github-utils.ts | 10 ---- apps/event-worker/src/job-dispatch/github.ts | 53 +------------------ apps/event-worker/src/job-sync/github.ts | 53 ------------------- .../api/github/webhook/workflow/handler.ts | 45 ++++++++++++---- pnpm-lock.yaml | 3 -- 6 files changed, 35 insertions(+), 130 deletions(-) delete mode 100644 apps/event-worker/src/job-sync/github.ts diff --git a/apps/event-worker/package.json b/apps/event-worker/package.json index 444126f6..3ed7bae9 100644 --- a/apps/event-worker/package.json +++ b/apps/event-worker/package.json @@ -28,7 +28,6 @@ "ioredis": "^5.4.1", "lodash": "^4.17.21", "ms": "^2.1.3", - "p-retry": "^6.2.0", "semver": "^7.6.2", "zod": "catalog:" }, diff --git a/apps/event-worker/src/github-utils.ts b/apps/event-worker/src/github-utils.ts index 7149b95e..99373473 100644 --- a/apps/event-worker/src/github-utils.ts +++ b/apps/event-worker/src/github-utils.ts @@ -1,18 +1,8 @@ import { createAppAuth } from "@octokit/auth-app"; import { Octokit } from "@octokit/rest"; -import { JobStatus as JStatus } from "@ctrlplane/validators/jobs"; - import { env } from "./config.js"; -export const convertStatus = (status: string): JStatus => { - if (status === "success" || status === "neutral") return JStatus.Completed; - if (status === "queued" || status === "requested" || status === "waiting") - return JStatus.InProgress; - if (status === "timed_out" || status === "stale") return JStatus.Failure; - return status as JStatus; -}; - export const getInstallationOctokit = (installationId: number) => env.GITHUB_BOT_APP_ID && env.GITHUB_BOT_PRIVATE_KEY && diff --git a/apps/event-worker/src/job-dispatch/github.ts b/apps/event-worker/src/job-dispatch/github.ts index 73e9e9f8..c60b4ab8 100644 --- a/apps/event-worker/src/job-dispatch/github.ts +++ b/apps/event-worker/src/job-dispatch/github.ts @@ -1,5 +1,4 @@ import type { Job } from "@ctrlplane/db/schema"; -import pRetry from "p-retry"; import { and, eq, takeFirstOrNull } from "@ctrlplane/db"; import { db } from "@ctrlplane/db/client"; @@ -15,7 +14,7 @@ import { logger } from "@ctrlplane/logger"; import { configSchema } from "@ctrlplane/validators/github"; import { JobStatus } from "@ctrlplane/validators/jobs"; -import { convertStatus, getInstallationOctokit } from "../github-utils.js"; +import { getInstallationOctokit } from "../github-utils.js"; export const dispatchGithubJob = async (je: Job) => { logger.info(`Dispatching github job ${je.id}...`); @@ -100,54 +99,4 @@ export const dispatchGithubJob = async (je: Job) => { authorization: `Bearer ${installationToken.token}`, }, }); - - let runId: number | null = null; - let status: string | null = null; - - try { - const { runId: runId_, status: status_ } = await pRetry( - async () => { - const runs = await octokit.actions.listWorkflowRuns({ - owner: parsed.data.owner, - repo: parsed.data.repo, - workflow_id: parsed.data.workflowId, - branch: ghOrg.branch, - }); - - const run = runs.data.workflow_runs.find((run) => - run.name?.includes(je.id), - ); - - if (run == null) throw new Error("Run not found"); - - logger.info(`Run found for job ${je.id}`, { run }); - - return { runId: run.id, status: run.status, url: run.html_url }; - }, - { retries: 15, minTimeout: 1000 }, - ); - - runId = runId_; - status = status_; - } catch (error) { - logger.error(`Job ${je.id} dispatch to GitHub failed`, { error }); - await db.update(job).set({ - status: JobStatus.ExternalRunNotFound, - message: `Run ID not found for job ${je.id}`, - }); - return; - } - - logger.info(`Job ${je.id} dispatched to GitHub`, { - runId, - status, - }); - - await db - .update(job) - .set({ - externalId: runId.toString(), - status: convertStatus(status ?? JobStatus.InProgress), - }) - .where(eq(job.id, je.id)); }; diff --git a/apps/event-worker/src/job-sync/github.ts b/apps/event-worker/src/job-sync/github.ts deleted file mode 100644 index fd6426ce..00000000 --- a/apps/event-worker/src/job-sync/github.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { Job } from "@ctrlplane/db/schema"; - -import { eq } from "@ctrlplane/db"; -import { db } from "@ctrlplane/db/client"; -import { job } from "@ctrlplane/db/schema"; -import { configSchema } from "@ctrlplane/validators/github"; -import { JobStatus } from "@ctrlplane/validators/jobs"; - -import { convertStatus, getInstallationOctokit } from "../github-utils.js"; - -export const syncGithubJob = async (je: Job) => { - if (je.externalId == null) { - await db.update(job).set({ - status: JobStatus.ExternalRunNotFound, - message: `Run ID not found for job ${je.id}`, - }); - return; - } - - const runId = Number(je.externalId); - - const config = je.jobAgentConfig; - const parsed = configSchema.safeParse(config); - - if (!parsed.success) { - await db.update(job).set({ - status: JobStatus.InvalidJobAgent, - message: parsed.error.message, - }); - return; - } - - const octokit = getInstallationOctokit(parsed.data.installationId); - if (octokit == null) { - await db.update(job).set({ - status: JobStatus.InvalidJobAgent, - message: "GitHub bot not configured", - }); - return; - } - - const { data: workflowState } = await octokit.actions.getWorkflowRun({ - owner: parsed.data.owner, - repo: parsed.data.repo, - run_id: runId, - }); - - const status = convertStatus(workflowState.status ?? JobStatus.Pending); - - await db.update(job).set({ status }).where(eq(job.id, je.id)); - - return status === JobStatus.Completed; -}; diff --git a/apps/webservice/src/app/api/github/webhook/workflow/handler.ts b/apps/webservice/src/app/api/github/webhook/workflow/handler.ts index 938c7358..599cdbf2 100644 --- a/apps/webservice/src/app/api/github/webhook/workflow/handler.ts +++ b/apps/webservice/src/app/api/github/webhook/workflow/handler.ts @@ -21,31 +21,54 @@ const convertStatus = ( ): schema.JobStatus => status === JobStatus.Completed ? JobStatus.Completed : JobStatus.InProgress; +const extractUuid = (str: string) => { + const uuidRegex = + /\b[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\b/; + const match = uuidRegex.exec(str); + return match ? match[0] : null; +}; + +const getJob = async (externalId: number, name: string) => { + const jobFromExternalId = await db + .select() + .from(schema.job) + .where(eq(schema.job.externalId, externalId.toString())) + .then(takeFirstOrNull); + + if (jobFromExternalId != null) return jobFromExternalId; + + const uuid = extractUuid(name); + if (uuid == null) return null; + + return db + .select() + .from(schema.job) + .where(eq(schema.job.id, uuid)) + .then(takeFirstOrNull); +}; + export const handleWorkflowWebhookEvent = async (event: WorkflowRunEvent) => { const { id, status: externalStatus, conclusion, repository, + name, } = event.workflow_run; + const job = await getJob(id, name); + if (job == null) return; + const status = conclusion != null ? convertConclusion(conclusion) : convertStatus(externalStatus); - const job = await db + const externalId = id.toString(); + await db .update(schema.job) - .set({ status }) - .where(eq(schema.job.externalId, id.toString())) - .returning() - .then(takeFirstOrNull); - - // Addressing a race condition: When the job is created externally on GitHub, - // it triggers a webhook event. However, our system hasn't updated the job with - // the externalRunId yet, as it depends on the job's instantiation. Therefore, - // the first event lacks the run ID, so we skip it and wait for the next event. - if (job == null) return; + .set({ status, externalId }) + .where(eq(schema.job.id, job.id)); const existingUrlMetadata = await db .select() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6ceda454..b846016c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -243,9 +243,6 @@ importers: ms: specifier: ^2.1.3 version: 2.1.3 - p-retry: - specifier: ^6.2.0 - version: 6.2.0 semver: specifier: ^7.6.2 version: 7.6.3