Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions apps/jobs/src/policy-checker/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { alias, eq } from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import * as schema from "@ctrlplane/db/schema";
import { Channel, getQueue } from "@ctrlplane/events";
import { logger } from "@ctrlplane/logger";

const triggerPolicyEvaluation = async () => {
Expand All @@ -11,10 +14,27 @@ const triggerPolicyEvaluation = async () => {

while (hasMore) {
try {
const releaseTargets = await db.query.releaseTarget.findMany({
limit: PAGE_SIZE,
offset,
});
const ct = alias(schema.computedPolicyTargetReleaseTarget, "ct");

const releaseTargets = await db
.select()
.from(schema.policy)
.innerJoin(
schema.policyTarget,
eq(schema.policyTarget.policyId, schema.policy.id),
)
.innerJoin(ct, eq(ct.policyTargetId, schema.policyTarget.id))
.innerJoin(
schema.releaseTarget,
eq(ct.releaseTargetId, schema.releaseTarget.id),
)
.innerJoin(
schema.policyRuleGradualRollout,
eq(schema.policyRuleGradualRollout.policyId, schema.policy.id),
)
.limit(PAGE_SIZE)
.offset(offset)
.then((rows) => rows.map((row) => row.release_target));
Comment on lines +17 to +37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Non-deterministic pagination + join fan-out risks missing or duplicate work

The query paginates with limit/offset but has no orderBy, so PostgreSQL can change row order between calls, causing duplicates or gaps.
Because the joins create a one-to-many relation (multiple policyTargets per policy, etc.) a single release_target can emerge multiple times.

-      const releaseTargets = await db
-        .select()
+      const releaseTargets = await db
+        .selectDistinctOn([schema.releaseTarget.id])
         .from(schema.policy)
 ...
-        .offset(offset)
+        .orderBy(schema.releaseTarget.id)
+        .offset(offset)
         .then((rows) => rows.map((row) => row.release_target));

This makes pagination deterministic and collapses duplicates.

🤖 Prompt for AI Agents
In apps/jobs/src/policy-checker/index.ts around lines 17 to 37, the query uses
limit and offset for pagination without an orderBy clause, causing
non-deterministic results and potential duplicates due to join fan-out. To fix
this, add a deterministic orderBy clause on a unique or stable column (e.g.,
primary key) to ensure consistent row ordering, and modify the query to collapse
duplicates by using distinct or grouping on release_target to avoid processing
the same release_target multiple times.


if (releaseTargets.length === 0) {
hasMore = false;
Expand All @@ -26,15 +46,15 @@ const triggerPolicyEvaluation = async () => {
);
totalProcessed += releaseTargets.length;

// await getQueue(Channel.EvaluateReleaseTarget).addBulk(
// releaseTargets.map((rt) => ({
// name: `${rt.resourceId}-${rt.environmentId}-${rt.deploymentId}`,
// data: rt,
// priority: 10,
// })),
// );

offset += PAGE_SIZE;

await getQueue(Channel.EvaluateReleaseTarget).addBulk(
releaseTargets.map((rt) => ({
name: `${rt.resourceId}-${rt.environmentId}-${rt.deploymentId}`,
data: rt,
priority: 10,
})),
);
} catch (error) {
logger.error("Error during policy evaluation:", error);
throw error;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import type { Swagger } from "atlassian-openapi";

export const openapi: Swagger.SwaggerV3 = {
openapi: "3.0.0",
info: {
title: "Ctrlplane API",
version: "1.0.0",
},
components: {
schemas: {
Release: {
type: "object",
properties: {
resource: { $ref: "#/components/schemas/Resource" },
environment: { $ref: "#/components/schemas/Environment" },
deployment: { $ref: "#/components/schemas/Deployment" },
version: { $ref: "#/components/schemas/DeploymentVersion" },
variables: { type: "object", additionalProperties: true },
},
},
},
},
paths: {
"/v1/deployment-versions/{deploymentVersionId}/releases": {
get: {
summary: "Get all releases for a deployment version",
parameters: [
{
name: "deploymentVersionId",
in: "path",
required: true,
schema: { type: "string", format: "uuid" },
},
],
responses: {
200: {
description: "OK",
content: {
"application/json": {
schema: {
type: "array",
items: { $ref: "#/components/schemas/Release" },
},
},
},
},
404: {
description: "Not Found",
content: {
"application/json": {
schema: {
type: "object",
properties: {
error: { type: "string" },
},
required: ["error"],
},
},
},
},
500: {
description: "Internal Server Error",
content: {
"application/json": {
schema: {
type: "object",
properties: {
error: { type: "string" },
},
required: ["error"],
},
},
},
},
},
},
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import type { Tx } from "@ctrlplane/db";
import { NextResponse } from "next/server";
import { NOT_FOUND } from "http-status";

import { eq, sql } from "@ctrlplane/db";
import * as SCHEMA from "@ctrlplane/db/schema";
import { Permission } from "@ctrlplane/validators/auth";

import { authn, authz } from "~/app/api/v1/auth";
import { request } from "~/app/api/v1/middleware";

export const GET = request()
.use(authn)
.use(
authz(({ can, params }) =>
can.perform(Permission.DeploymentVersionGet).on({
type: "deploymentVersion",
id: params.deploymentVersionId ?? "",
}),
),
)
.handle<{ db: Tx }, { params: Promise<{ deploymentVersionId: string }> }>(
async ({ db }, { params }) => {
const { deploymentVersionId } = await params;

const deploymentVersion = await db.query.deploymentVersion.findFirst({
where: eq(SCHEMA.deploymentVersion.id, deploymentVersionId),
with: { metadata: true },
});

if (deploymentVersion == null)
return NextResponse.json(
{ error: "Deployment version not found" },
{ status: NOT_FOUND },
);

const variableReleaseSubquery = db
.select({
variableSetReleaseId: SCHEMA.variableSetRelease.id,
variables: sql<Record<string, any>>`COALESCE(jsonb_object_agg(
${SCHEMA.variableValueSnapshot.key},
${SCHEMA.variableValueSnapshot.value}
) FILTER (WHERE ${SCHEMA.variableValueSnapshot.id} IS NOT NULL), '{}'::jsonb)`.as(
"variables",
),
})
.from(SCHEMA.variableSetRelease)
.leftJoin(
SCHEMA.variableSetReleaseValue,
eq(
SCHEMA.variableSetRelease.id,
SCHEMA.variableSetReleaseValue.variableSetReleaseId,
),
)
.leftJoin(
SCHEMA.variableValueSnapshot,
eq(
SCHEMA.variableSetReleaseValue.variableValueSnapshotId,
SCHEMA.variableValueSnapshot.id,
),
)
.groupBy(SCHEMA.variableSetRelease.id)
.as("variableRelease");

const releases = await db
.select()
.from(SCHEMA.release)
.innerJoin(
SCHEMA.versionRelease,
eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id),
)
.innerJoin(
variableReleaseSubquery,
eq(
SCHEMA.release.variableReleaseId,
variableReleaseSubquery.variableSetReleaseId,
),
)
.innerJoin(
SCHEMA.releaseTarget,
eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id),
)
.innerJoin(
SCHEMA.resource,
eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id),
)
.innerJoin(
SCHEMA.environment,
eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id),
)
.innerJoin(
SCHEMA.deployment,
eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id),
)
.where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
.limit(500);

Comment on lines +65 to +97
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add deterministic ordering & cursor-friendly pagination to the releases query

Today the query limits the result set to 500 rows, but returns them in an undefined order.
This can cause:

  • Non-deterministic responses across identical requests (hard to cache & page).
  • Starvation of older releases if new ones constantly push them out of the first 500.

Consider ordering by a monotonic column (createdAt, id, etc.) and exposing limit/cursor query-params so consumers can reliably traverse the full history.

-      .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
-      .limit(500);
+      .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
+      .orderBy(sql`${SCHEMA.release.createdAt} DESC`)  // deterministic
+      .limit(limit)                                    // value from searchParams
+      .offset(offset);                                 // cursor/offset

This small change prevents pagination gaps and improves cacheability.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const releases = await db
.select()
.from(SCHEMA.release)
.innerJoin(
SCHEMA.versionRelease,
eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id),
)
.innerJoin(
variableReleaseSubquery,
eq(
SCHEMA.release.variableReleaseId,
variableReleaseSubquery.variableSetReleaseId,
),
)
.innerJoin(
SCHEMA.releaseTarget,
eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id),
)
.innerJoin(
SCHEMA.resource,
eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id),
)
.innerJoin(
SCHEMA.environment,
eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id),
)
.innerJoin(
SCHEMA.deployment,
eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id),
)
.where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
.limit(500);
const releases = await db
.select()
.from(SCHEMA.release)
.innerJoin(
SCHEMA.versionRelease,
eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id),
)
.innerJoin(
variableReleaseSubquery,
eq(
SCHEMA.release.variableReleaseId,
variableReleaseSubquery.variableSetReleaseId,
),
)
.innerJoin(
SCHEMA.releaseTarget,
eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id),
)
.innerJoin(
SCHEMA.resource,
eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id),
)
.innerJoin(
SCHEMA.environment,
eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id),
)
.innerJoin(
SCHEMA.deployment,
eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id),
)
.where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
.orderBy(sql`${SCHEMA.release.createdAt} DESC`) // deterministic order
.limit(limit) // from request params
.offset(offset); // for cursor/offset pagination
🤖 Prompt for AI Agents
In
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/route.ts
between lines 65 and 97, the releases query limits results to 500 without any
ordering, causing non-deterministic and inconsistent pagination. Fix this by
adding an orderBy clause on a monotonic column like createdAt or id to ensure
deterministic ordering. Additionally, modify the API to accept limit and cursor
query parameters to enable cursor-based pagination, allowing consumers to
reliably page through the full release history without gaps or starvation.

const fullReleases = releases.map((release) => ({
resource: release.resource,
environment: release.environment,
deployment: release.deployment,
version: {
...deploymentVersion,
metadata: Object.fromEntries(
deploymentVersion.metadata.map((m) => [m.key, m.value]),
),
},
variables: release.variableRelease.variables,
}));

return NextResponse.json(fullReleases);
},
);
4 changes: 4 additions & 0 deletions apps/webservice/src/app/api/v1/policies/[policyId]/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ export const openapi: Swagger.SwaggerV3 = {
required: ["roleId"],
},
},
gradualRollout: {
$ref: "#/components/schemas/GradualRollout",
nullable: true,
},
},
},
},
Expand Down
18 changes: 18 additions & 0 deletions apps/webservice/src/app/api/v1/policies/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ export const openapi: Swagger.SwaggerV3 = {
},
required: ["roleId", "requiredApprovalsCount"],
},
GradualRollout: {
type: "object",
properties: {
deployRate: { type: "number" },
windowSizeMinutes: { type: "number" },
name: { type: "string" },
description: { type: "string" },
},
required: ["deployRate", "windowSizeMinutes", "name"],
},
Policy: {
type: "object",
properties: {
Expand Down Expand Up @@ -99,6 +109,10 @@ export const openapi: Swagger.SwaggerV3 = {
type: "array",
items: { $ref: "#/components/schemas/VersionRoleApproval" },
},
gradualRollout: {
$ref: "#/components/schemas/GradualRollout",
nullable: true,
},
},
required: [
"id",
Expand All @@ -111,6 +125,7 @@ export const openapi: Swagger.SwaggerV3 = {
"denyWindows",
"versionUserApprovals",
"versionRoleApprovals",
"gradualRollout",
],
},
},
Expand Down Expand Up @@ -166,6 +181,9 @@ export const openapi: Swagger.SwaggerV3 = {
$ref: "#/components/schemas/VersionRoleApproval",
},
},
gradualRollout: {
$ref: "#/components/schemas/GradualRollout",
},
},
required: ["name", "workspaceId", "targets"],
},
Expand Down
Loading