Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init #202

Closed
wants to merge 4 commits into from
Closed

init #202

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
1 change: 1 addition & 0 deletions apps/jobs/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ COPY packages/validators/package.json ./packages/validators/package.json
COPY packages/logger/package.json ./packages/logger/package.json
COPY packages/job-dispatch/package.json ./packages/job-dispatch/package.json
COPY packages/secrets/package.json ./packages/secrets/package.json
COPY packages/events/package.json ./packages/events/package.json

COPY apps/jobs/package.json ./apps/jobs/package.json

Expand Down
1 change: 1 addition & 0 deletions apps/jobs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"dependencies": {
"@ctrlplane/db": "workspace:*",
"@ctrlplane/events": "workspace:*",
"@ctrlplane/job-dispatch": "workspace:*",
"@ctrlplane/logger": "workspace:*",
"@ctrlplane/validators": "workspace:*",
Expand Down
51 changes: 10 additions & 41 deletions apps/jobs/src/expired-env-checker/index.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,22 @@
import _ from "lodash";
import { isPresent } from "ts-is-present";

import { eq, inArray, lte } from "@ctrlplane/db";
import { inArray, lte } from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import * as SCHEMA from "@ctrlplane/db/schema";
import { logger } from "@ctrlplane/logger";

type QueryRow = {
environment: SCHEMA.Environment;
deployment: SCHEMA.Deployment;
};

const groupByEnvironment = (rows: QueryRow[]) =>
_.chain(rows)
.groupBy((e) => e.environment.id)
.map((env) => ({
...env[0]!.environment,
deployments: env.map((e) => e.deployment),
}))
.value();
import { handleHookEvent } from "@ctrlplane/events";
import { EnvironmentEvent } from "@ctrlplane/validators/events";

export const run = async () => {
const expiredEnvironments = await db
.select()
.from(SCHEMA.environment)
.innerJoin(
SCHEMA.deployment,
eq(SCHEMA.deployment.systemId, SCHEMA.environment.systemId),
)
.where(lte(SCHEMA.environment.expiresAt, new Date()))
.then(groupByEnvironment);
.where(lte(SCHEMA.environment.expiresAt, new Date()));
if (expiredEnvironments.length === 0) return;

const targetPromises = expiredEnvironments
.filter((env) => isPresent(env.targetFilter))
.map(async (env) => {
const targets = await db
.select()
.from(SCHEMA.target)
.where(SCHEMA.targetMatchesMetadata(db, env.targetFilter));

return { environmentId: env.id, targets };
});
const associatedTargets = await Promise.all(targetPromises);

for (const { environmentId, targets } of associatedTargets)
logger.info(
`[${targets.length}] targets are associated with expired environment [${environmentId}]`,
);
const events = expiredEnvironments.map((env) => ({
type: EnvironmentEvent.Deleted,
createdAt: new Date().toISOString(),
payload: { environmentId: env.id },
}));
await Promise.all(events.map(handleHookEvent));

Comment on lines +14 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding error handling for event processing

While concurrent event processing is efficient, a failure in handleHookEvent for one environment could cause others to fail silently. Consider adding error handling and possibly a transaction to ensure atomicity.

Here's a suggested improvement:

-  await Promise.all(events.map(handleHookEvent));
+  try {
+    await Promise.all(
+      events.map(async (event) => {
+        try {
+          await handleHookEvent(event);
+        } catch (error) {
+          console.error(`Failed to process event for environment ${event.payload.environmentId}:`, error);
+          throw error; // Re-throw to trigger the outer catch
+        }
+      })
+    );
+  } catch (error) {
+    console.error('Failed to process some environment deletion events:', error);
+    throw error; // Prevent environment deletion if event processing fails
+  }
📝 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 events = expiredEnvironments.map((env) => ({
type: EnvironmentEvent.Deleted,
createdAt: new Date().toISOString(),
payload: { environmentId: env.id },
}));
await Promise.all(events.map(handleHookEvent));
const events = expiredEnvironments.map((env) => ({
type: EnvironmentEvent.Deleted,
createdAt: new Date().toISOString(),
payload: { environmentId: env.id },
}));
try {
await Promise.all(
events.map(async (event) => {
try {
await handleHookEvent(event);
} catch (error) {
console.error(`Failed to process event for environment ${event.payload.environmentId}:`, error);
throw error; // Re-throw to trigger the outer catch
}
})
);
} catch (error) {
console.error('Failed to process some environment deletion events:', error);
throw error; // Prevent environment deletion if event processing fails
}

const envIds = expiredEnvironments.map((env) => env.id);
await db
Expand Down
20 changes: 20 additions & 0 deletions packages/db/src/schema/hook.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { pgTable, text, uuid } from "drizzle-orm/pg-core";

import { runbook } from "./runbook.js";

export const runhook = pgTable("runhook", {
id: uuid("id").primaryKey().defaultRandom(),
scopeType: text("scope_type").notNull(),
scopeId: uuid("scope_id").notNull(),
runbookId: uuid("runbook_id")
.references(() => runbook.id, { onDelete: "cascade" })
.notNull(),
});

export const runhookEvent = pgTable("runhook_event", {
id: uuid("id").primaryKey().defaultRandom(),
runhookId: uuid("runhook_id")
.references(() => runhook.id, { onDelete: "cascade" })
.notNull(),
eventType: text("event_type").notNull(),
});
Comment on lines +14 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding essential fields for event tracking.

The runhook_event table appears to be missing some common fields typically needed for event tracking:

  1. Timestamp fields (created_at, processed_at)
  2. Event payload or metadata field
  3. Event status field
  4. Index on runhookId for efficient lookups

Consider enhancing the schema like this:

 export const runhookEvent = pgTable("runhook_event", {
   id: uuid("id").primaryKey().defaultRandom(),
   runhookId: uuid("runhook_id")
     .references(() => runhook.id, { onDelete: "cascade" })
     .notNull(),
   eventType: text("event_type").notNull(),
+  payload: jsonb("payload"),
+  status: text("status").notNull().default("pending"),
+  createdAt: timestamp("created_at").notNull().defaultNow(),
+  processedAt: timestamp("processed_at"),
+  error: text("error"),
-});
+}, (table) => ({
+  runhookIdx: index("runhook_event_runhook_idx").on(table.runhookId),
+  createdAtIdx: index("runhook_event_created_at_idx").on(table.createdAt),
+}));

Also, consider adding documentation for valid event types:

+/**
+ * Represents events associated with runhooks.
+ * 
+ * @property eventType - The type of event.
+ *                     Valid values: [list valid values here]
+ * @property payload - Additional event data in JSON format
+ * @property status - Event processing status (pending, processing, completed, failed)
+ */
 export const runhookEvent = pgTable("runhook_event", {
📝 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
export const runhookEvent = pgTable("runhook_event", {
id: uuid("id").primaryKey().defaultRandom(),
runhookId: uuid("runhook_id")
.references(() => runhook.id, { onDelete: "cascade" })
.notNull(),
eventType: text("event_type").notNull(),
});
/**
* Represents events associated with runhooks.
*
* @property eventType - The type of event.
* Valid values: [list valid values here]
* @property payload - Additional event data in JSON format
* @property status - Event processing status (pending, processing, completed, failed)
*/
export const runhookEvent = pgTable("runhook_event", {
id: uuid("id").primaryKey().defaultRandom(),
runhookId: uuid("runhook_id")
.references(() => runhook.id, { onDelete: "cascade" })
.notNull(),
eventType: text("event_type").notNull(),
payload: jsonb("payload"),
status: text("status").notNull().default("pending"),
createdAt: timestamp("created_at").notNull().defaultNow(),
processedAt: timestamp("processed_at"),
error: text("error"),
}, (table) => ({
runhookIdx: index("runhook_event_runhook_idx").on(table.runhookId),
createdAtIdx: index("runhook_event_created_at_idx").on(table.createdAt),
}));

1 change: 1 addition & 0 deletions packages/db/src/schema/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ export * from "./runbook-variables.js";
export * from "./rbac.js";
export * from "./runbook.js";
export * from "./job-agent.js";
export * from "./hook.js";
10 changes: 10 additions & 0 deletions packages/events/eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import baseConfig, { requireJsSuffix } from "@ctrlplane/eslint-config/base";

/** @type {import('typescript-eslint').Config} */
export default [
{
ignores: ["dist/**"],
},
...requireJsSuffix,
...baseConfig,
];
38 changes: 38 additions & 0 deletions packages/events/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"name": "@ctrlplane/events",
"private": true,
"version": "0.1.0",
"type": "module",
"exports": {
".": {
"types": "./src/index.ts",
"default": "./dist/index.js"
}
},
"license": "MIT",
"scripts": {
"build": "tsc",
"dev": "tsc --watch",
"clean": "rm -rf .turbo node_modules",
"format": "prettier --check . --ignore-path ../../.gitignore",
"lint": "eslint",
"typecheck": "tsc --noEmit --emitDeclarationOnly false"
},
"dependencies": {
"@ctrlplane/db": "workspace:*",
"@ctrlplane/job-dispatch": "workspace:*",
"@ctrlplane/validators": "workspace:*",
"ts-is-present": "^1.2.2",
"zod": "catalog:"
},
"devDependencies": {
"@ctrlplane/eslint-config": "workspace:*",
"@ctrlplane/prettier-config": "workspace:*",
"@ctrlplane/tsconfig": "workspace:*",
"@types/node": "catalog:node20",
"eslint": "catalog:",
"prettier": "catalog:",
"typescript": "catalog:"
},
"prettier": "@ctrlplane/prettier-config"
}
111 changes: 111 additions & 0 deletions packages/events/src/environments/environment-delete.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import type { EnvironmentDeletedEvent } from "@ctrlplane/validators/events";
import type { TargetCondition } from "@ctrlplane/validators/targets";
import { isPresent } from "ts-is-present";

import {
and,
eq,
inArray,
isNotNull,
ne,
or,
takeFirstOrNull,
} from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import * as SCHEMA from "@ctrlplane/db/schema";
import { dispatchRunbook } from "@ctrlplane/job-dispatch";
import { ComparisonOperator } from "@ctrlplane/validators/conditions";
import { TargetFilterType } from "@ctrlplane/validators/targets";

const handleTargets = async (event: EnvironmentDeletedEvent) => {
const environment = await db
.select()
.from(SCHEMA.environment)
.where(eq(SCHEMA.environment.id, event.payload.environmentId))
.then(takeFirstOrNull);

Comment on lines +20 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add Null Check for 'environment' Before Accessing Its Properties

While you check if environment?.targetFilter == null, there is a possibility that environment itself could be null if the database query does not find a matching record. Accessing environment.id later without confirming that environment is not null may lead to runtime errors.

Consider adding a null check for environment after retrieving it from the database to ensure it exists before accessing its properties.

Apply this diff to add the null check:

const environment = await db
  .select()
  .from(SCHEMA.environment)
  .where(eq(SCHEMA.environment.id, event.payload.environmentId))
  .then(takeFirstOrNull);

+ if (environment == null) return;

if (environment.targetFilter == null) return;
📝 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 handleTargets = async (event: EnvironmentDeletedEvent) => {
const environment = await db
.select()
.from(SCHEMA.environment)
.where(eq(SCHEMA.environment.id, event.payload.environmentId))
.then(takeFirstOrNull);
const handleTargets = async (event: EnvironmentDeletedEvent) => {
const environment = await db
.select()
.from(SCHEMA.environment)
.where(eq(SCHEMA.environment.id, event.payload.environmentId))
.then(takeFirstOrNull);
if (environment == null) return;

if (environment?.targetFilter == null) return;

const targets = await db
.select()
.from(SCHEMA.target)
.where(SCHEMA.targetMatchesMetadata(db, environment.targetFilter));
if (targets.length === 0) return;

const checks = and(
isNotNull(SCHEMA.environment.targetFilter),
ne(SCHEMA.environment.id, environment.id),
);
const system = await db.query.system.findFirst({
where: eq(SCHEMA.system.id, environment.systemId),
with: { environments: { where: checks }, deployments: true },
});
if (system == null) return;

const envFilters = system.environments
.map((e) => e.targetFilter)
.filter(isPresent);

const removedFromSystemFilter: TargetCondition = {
type: TargetFilterType.Comparison,
operator: ComparisonOperator.Or,
not: true,
conditions: envFilters,
};

const removedFromSystemTargets =
envFilters.length > 0
? await db
.select()
.from(SCHEMA.target)
.where(
and(
SCHEMA.targetMatchesMetadata(db, removedFromSystemFilter),
inArray(
SCHEMA.target.id,
targets.map((t) => t.id),
),
),
)
: targets;
if (removedFromSystemTargets.length === 0) return;

const runhooks = await db
.select()
.from(SCHEMA.runhook)
.innerJoin(
SCHEMA.runhookEvent,
eq(SCHEMA.runhookEvent.runhookId, SCHEMA.runhook.id),
)
.where(
or(
...system.deployments.map((deployment) =>
and(
eq(SCHEMA.runhook.scopeType, "deployment"),
eq(SCHEMA.runhook.scopeId, deployment.id),
eq(SCHEMA.runhookEvent.eventType, "environment.deleted"),
),
),
),
)
.then((r) => r.map((rh) => rh.runhook));
if (runhooks.length === 0) return;

const handleLifecycleHooksForTargets = removedFromSystemTargets.flatMap((t) =>
runhooks.map((rh) => {
const values: Record<string, string> = {
targetId: t.id,
deploymentId: rh.scopeId,
environmentId: environment.id,
systemId: system.id,
};

return dispatchRunbook(db, rh.runbookId, values);
}),
);

await Promise.all(handleLifecycleHooksForTargets);
};

export const handleEnvironmentDeleted = (event: EnvironmentDeletedEvent) =>
handleTargets(event);
10 changes: 10 additions & 0 deletions packages/events/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { HookEvent } from "@ctrlplane/validators/events";

import { isEnvironmentDeletedEvent } from "@ctrlplane/validators/events";

import { handleEnvironmentDeleted } from "./environments/environment-delete.js";

export const handleHookEvent = (event: HookEvent) => {
if (isEnvironmentDeletedEvent(event)) handleEnvironmentDeleted(event);
throw new Error(`Unknown event type: ${event.type}`);
};
9 changes: 9 additions & 0 deletions packages/events/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "@ctrlplane/tsconfig/internal-package.json",
"compilerOptions": {
"outDir": "dist",
"baseUrl": "."
},
"include": ["*.ts", "src"],
"exclude": ["node_modules"]
}
23 changes: 23 additions & 0 deletions packages/validators/src/events/hook-events/environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { z } from "zod";

const environmentBaseEvent = z.object({
createdAt: z.string().datetime(),
payload: z.object({
environmentId: z.string().uuid(),
}),
});

export enum EnvironmentEvent {
Deleted = "environment.deleted",
Created = "environment.created",
}

export const environmentDeletedEvent = environmentBaseEvent.extend({
type: z.literal(EnvironmentEvent.Deleted),
});
export type EnvironmentDeletedEvent = z.infer<typeof environmentDeletedEvent>;

export const environmentCreatedEvent = environmentBaseEvent.extend({
type: z.literal(EnvironmentEvent.Created),
});
export type EnvironmentCreatedEvent = z.infer<typeof environmentCreatedEvent>;
Comment on lines +15 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider extending event payloads with event-specific data.

The current implementation uses the same payload structure for both events. Consider adding event-specific fields to capture relevant data:

 export const environmentDeletedEvent = environmentBaseEvent.extend({
   type: z.literal(EnvironmentEvent.Deleted),
+  payload: z.object({
+    environmentId: z.string().uuid(),
+    deletedBy: z.string().uuid(),
+    reason: z.string().optional(),
+  }),
 });

 export const environmentCreatedEvent = environmentBaseEvent.extend({
   type: z.literal(EnvironmentEvent.Created),
+  payload: z.object({
+    environmentId: z.string().uuid(),
+    createdBy: z.string().uuid(),
+    configuration: z.record(z.unknown()),
+  }),
 });

Committable suggestion skipped: line range outside the PR's diff.

27 changes: 27 additions & 0 deletions packages/validators/src/events/hook-events/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { z } from "zod";

import type {
EnvironmentCreatedEvent,
EnvironmentDeletedEvent,
} from "./environment.js";
import {
environmentCreatedEvent,
environmentDeletedEvent,
EnvironmentEvent,
} from "./environment.js";

export * from "./environment.js";

export const HookEvent = z.union([
environmentDeletedEvent,
environmentCreatedEvent,
]);
export type HookEvent = z.infer<typeof HookEvent>;

// typeguards
export const isEnvironmentDeletedEvent = (
event: HookEvent,
): event is EnvironmentDeletedEvent => event.type === EnvironmentEvent.Deleted;
export const isEnvironmentCreatedEvent = (
event: HookEvent,
): event is EnvironmentCreatedEvent => event.type === EnvironmentEvent.Created;
2 changes: 2 additions & 0 deletions packages/validators/src/events/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { z } from "zod";

export * from "./hook-events/index.js";

export enum Channel {
JobSync = "job-sync",
DispatchJob = "dispatch-job",
Expand Down
Loading
Loading