mirror of
https://github.com/Dokploy/dokploy.git
synced 2026-10-15 13:21:11 +00:00
fix(github-webhook): apply preview limit only to new previews
The preview deployment limit was checked before looking up whether the pull request already had a preview, so once an application reached the limit every `synchronize` event was dropped and existing previews went stale while GitHub still saw a 200. The check now runs inside the branch that creates a preview, uses the schema default of 3 when no limit is configured (instead of 0, which blocked every app without an explicit value) and compares with `>=` so the configured limit is inclusive. Closes #4074
This commit is contained in:
@@ -12,6 +12,8 @@ const mocks = vi.hoisted(() => ({
|
||||
queueAdd: vi.fn(),
|
||||
verify: vi.fn(),
|
||||
shouldDeploy: vi.fn(),
|
||||
createPreviewDeployment: vi.fn(),
|
||||
findPreviewDeploymentByApplicationId: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("drizzle-orm", () => ({
|
||||
@@ -64,10 +66,11 @@ vi.mock("@dokploy/server", () => ({
|
||||
IS_CLOUD: false,
|
||||
shouldDeploy: mocks.shouldDeploy,
|
||||
checkUserRepositoryPermissions: vi.fn(),
|
||||
createPreviewDeployment: vi.fn(),
|
||||
createPreviewDeployment: mocks.createPreviewDeployment,
|
||||
createSecurityBlockedComment: vi.fn(),
|
||||
findGithubById: vi.fn(),
|
||||
findPreviewDeploymentByApplicationId: vi.fn(),
|
||||
findPreviewDeploymentByApplicationId:
|
||||
mocks.findPreviewDeploymentByApplicationId,
|
||||
findPreviewDeploymentsByPullRequestId: vi.fn(),
|
||||
getBitbucketHeaders: vi.fn(() => ({})),
|
||||
removePreviewDeployment: vi.fn(),
|
||||
@@ -321,3 +324,157 @@ describe("GitHub app webhook auto-deploy", () => {
|
||||
expect(res.json).toHaveBeenCalledWith({ message: "No apps to deploy" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("GitHub app webhook preview deployments", () => {
|
||||
const createApplication = (
|
||||
overrides: Record<string, unknown> = {},
|
||||
): Record<string, unknown> => ({
|
||||
applicationId: "application-id",
|
||||
name: "my-app",
|
||||
serverId: null,
|
||||
previewLabels: [],
|
||||
previewLimit: 3,
|
||||
previewDeployments: [],
|
||||
previewRequireCollaboratorPermissions: false,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const createPreviewDeployments = (total: number) =>
|
||||
Array.from({ length: total }, (_, index) => ({
|
||||
previewDeploymentId: `existing-preview-${index}`,
|
||||
}));
|
||||
|
||||
const createPullRequestRequest = (action: string) =>
|
||||
({
|
||||
headers: {
|
||||
"x-hub-signature-256": "sha256=test-signature",
|
||||
"x-github-event": "pull_request",
|
||||
},
|
||||
body: {
|
||||
installation: {
|
||||
id: 12345,
|
||||
},
|
||||
action,
|
||||
pull_request: {
|
||||
id: 987,
|
||||
number: 42,
|
||||
title: "feat: add preview",
|
||||
html_url: "https://github.com/agentHits/dokploy/pull/42",
|
||||
labels: [],
|
||||
user: {
|
||||
login: "agentHits",
|
||||
},
|
||||
head: {
|
||||
ref: "feature",
|
||||
sha: "abc123",
|
||||
},
|
||||
base: {
|
||||
ref: "main",
|
||||
},
|
||||
},
|
||||
repository: {
|
||||
name: "dokploy",
|
||||
owner: {
|
||||
login: "agentHits",
|
||||
},
|
||||
},
|
||||
},
|
||||
}) as unknown as NextApiRequest;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.githubFindFirst.mockResolvedValue({
|
||||
githubId: "github-provider-id",
|
||||
githubInstallationId: 12345,
|
||||
githubWebhookSecret: "webhook-secret",
|
||||
});
|
||||
mocks.verify.mockResolvedValue(true);
|
||||
mocks.queueAdd.mockResolvedValue({ id: "job-id" });
|
||||
mocks.createPreviewDeployment.mockResolvedValue({
|
||||
previewDeploymentId: "new-preview-id",
|
||||
});
|
||||
mocks.findPreviewDeploymentByApplicationId.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("redeploys an existing preview even when the limit is reached", async () => {
|
||||
mocks.applicationsFindMany.mockResolvedValue([
|
||||
createApplication({
|
||||
previewLimit: 2,
|
||||
previewDeployments: createPreviewDeployments(3),
|
||||
}),
|
||||
]);
|
||||
mocks.findPreviewDeploymentByApplicationId.mockResolvedValue({
|
||||
previewDeploymentId: "existing-preview-0",
|
||||
});
|
||||
const res = createResponse();
|
||||
|
||||
await handler(createPullRequestRequest("synchronize"), res);
|
||||
|
||||
expect(mocks.createPreviewDeployment).not.toHaveBeenCalled();
|
||||
expect(mocks.queueAdd).toHaveBeenCalledWith(
|
||||
"deployments",
|
||||
expect.objectContaining({
|
||||
applicationId: "application-id",
|
||||
applicationType: "application-preview",
|
||||
previewDeploymentId: "existing-preview-0",
|
||||
type: "deploy",
|
||||
}),
|
||||
expect.objectContaining({
|
||||
removeOnComplete: true,
|
||||
removeOnFail: true,
|
||||
}),
|
||||
);
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
});
|
||||
|
||||
it("does not create a new preview once the limit is reached", async () => {
|
||||
mocks.applicationsFindMany.mockResolvedValue([
|
||||
createApplication({
|
||||
previewLimit: 2,
|
||||
previewDeployments: createPreviewDeployments(2),
|
||||
}),
|
||||
]);
|
||||
const res = createResponse();
|
||||
|
||||
await handler(createPullRequestRequest("opened"), res);
|
||||
|
||||
expect(mocks.createPreviewDeployment).not.toHaveBeenCalled();
|
||||
expect(mocks.queueAdd).not.toHaveBeenCalled();
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
});
|
||||
|
||||
it("falls back to the default limit when none is configured", async () => {
|
||||
mocks.applicationsFindMany.mockResolvedValue([
|
||||
createApplication({
|
||||
previewLimit: null,
|
||||
previewDeployments: createPreviewDeployments(2),
|
||||
}),
|
||||
]);
|
||||
const res = createResponse();
|
||||
|
||||
await handler(createPullRequestRequest("opened"), res);
|
||||
|
||||
expect(mocks.createPreviewDeployment).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
applicationId: "application-id",
|
||||
branch: "feature",
|
||||
pullRequestId: 987,
|
||||
pullRequestNumber: 42,
|
||||
}),
|
||||
);
|
||||
expect(mocks.queueAdd).toHaveBeenCalledWith(
|
||||
"deployments",
|
||||
expect.objectContaining({
|
||||
applicationId: "application-id",
|
||||
applicationType: "application-preview",
|
||||
previewDeploymentId: "new-preview-id",
|
||||
type: "deploy",
|
||||
}),
|
||||
expect.objectContaining({
|
||||
removeOnComplete: true,
|
||||
removeOnFail: true,
|
||||
}),
|
||||
);
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -484,10 +484,6 @@ export default async function handler(
|
||||
if (!hasLabel) continue;
|
||||
}
|
||||
|
||||
const previewLimit = app?.previewLimit || 0;
|
||||
if (app?.previewDeployments?.length > previewLimit) {
|
||||
continue;
|
||||
}
|
||||
const previewDeploymentResult =
|
||||
await findPreviewDeploymentByApplicationId(app.applicationId, prId);
|
||||
|
||||
@@ -495,6 +491,15 @@ export default async function handler(
|
||||
previewDeploymentResult?.previewDeploymentId || "";
|
||||
|
||||
if (!previewDeploymentResult && shouldCreateDeployment) {
|
||||
// The limit only applies to new previews, existing ones must
|
||||
// still be redeployed when the pull request is updated.
|
||||
const previewLimit = app?.previewLimit ?? 3;
|
||||
if ((app?.previewDeployments?.length ?? 0) >= previewLimit) {
|
||||
console.warn(
|
||||
`⚠️ Preview deployment limit (${previewLimit}) reached for ${app.name}, skipping preview for pull request #${prNumber}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const previewDeployment = await createPreviewDeployment({
|
||||
applicationId: app.applicationId as string,
|
||||
branch: prBranch,
|
||||
|
||||
Reference in New Issue
Block a user