-
Notifications
You must be signed in to change notification settings - Fork 11
chore: init kafka ts consumer #663
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
Conversation
WalkthroughAdds a new event-queue app under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Operator as Operator/Runtime
participant App as Event Queue App
participant Env as Config (env)
participant Kafka as Kafka Broker
participant Logger as @ctrlplane/logger
Operator->>App: module load / start()
App->>Env: load & validate NODE_ENV, KAFKA_BROKERS
Env-->>App: brokers[]
App->>Kafka: connect(clientId="ctrlplane-events")
App->>Kafka: subscribe(topic="ctrlplane-events", fromBeginning=true)
loop On each message
Kafka-->>App: message(topic, partition, value)
App->>Logger: log "Received event" + metadata
end
sequenceDiagram
autonumber
actor Dev as Dev/CI
participant GH as GitHub Actions
participant Docker as Buildx
participant Registry as Docker Hub
Dev->>GH: push PR / push to main (paths: apps/event-queue/**)
GH->>Docker: build image (matrix linux/amd64)
alt branch != main
Docker-->>GH: build only (push=false)
else main && DOCKERHUB_LOGIN == "true"
GH->>GH: docker/metadata-action@v5 (id: meta) -> generate tags
GH->>Docker: build & push with tags
Docker->>Registry: push tags
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (15)
apps/event-queue/README.md (1)
5-9: Document required environment variables and sample .env for dev.Running locally will fail or be confusing without Kafka-related envs. Please add minimal env docs and a sample.
Proposed addition:
## Running ```bash pnpm dev
+## Configuration
+
+Set these env vars (via ../../.env in the monorepo root or a local .env):
+
++# required +KAFKA_BROKERS=localhost:9092 +KAFKA_CLIENT_ID=ctrlplane-event-queue +KAFKA_GROUP_ID=event-queue-consumer +KAFKA_TOPIC=ctrlplane-events +</blockquote></details> <details> <summary>apps/event-queue/tsconfig.json (1)</summary><blockquote> `12-14`: **Exclude build output to prevent recursive type-checking and accidental emits.** dist should be excluded so tsc doesn’t crawl emitted JS/TS declaration files. Apply: ```diff - "include": ["src", "*.ts"], - "exclude": ["node_modules"] + "include": ["src", "*.ts"], + "exclude": ["node_modules", "dist"]apps/event-queue/Dockerfile (3)
10-11: Duplicate global Turbo install (npm and pnpm).You install Turbo twice (npm and pnpm). Keep one to reduce layers and avoid version drift.
Apply:
-RUN npm install -g turbo RUN npm install -g corepack@latest RUN corepack enable pnpm @@ -RUN pnpm add -g turbo +RUN pnpm add -g turbo(Or drop the pnpm one and use the npm global; just pick one.)
Also applies to: 18-18
20-23: Potential credential leakage by copying .npmrc into the runtime image.Copying .npmrc (and later copying the whole repo again) may bake tokens into the final layer. Prefer multi-stage builds and avoid copying .npmrc into the final stage.
Minimal mitigation in single stage:
COPY .npmrc .npmrc @@ COPY . . +RUN rm -f .npmrcBetter: multi-stage build where .npmrc exists only in the builder stage and never in the final runtime image. Also consider a .dockerignore that excludes .npmrc.
Also applies to: 35-35
46-46: Consider removing EXPOSE if the service isn’t serving HTTP.If the consumer only connects out to Kafka, exposing 3123 is unnecessary. If you keep it for observability/health endpoints, fine.
-EXPOSE 3123 +# EXPOSE 3123 # uncomment if you actually serve on this portapps/event-queue/package.json (2)
8-9: Lint script should target files."eslint" without a target can be a no-op depending on CLI defaults. Safer to lint the workspace.
- "lint": "eslint", + "lint": "eslint .",
1-4: Pin Node engine to match Docker base and ESM runtime.Helps CI and local dev stay aligned (Node 20 Alpine in Dockerfile).
{ "name": "@ctrlplane/event-queue", "private": true, "type": "module", + "engines": { + "node": ">=20.6.0" + },Also applies to: 9-10
apps/event-queue/src/config.ts (1)
7-18: Consider surfacing additional Kafka settings via env (client/group/topic, SSL/SASL) for parity with deploymentsNot a blocker, but making clientId, groupId, topic, and security knobs configurable will ease multi-env rollouts and local testing without code changes.
If helpful, I can provide a follow-up patch adding:
- KAFKA_CLIENT_ID (default: event-queue)
- KAFKA_GROUP_ID (default: event-queue)
- KAFKA_TOPIC (default: ctrlplane-events)
- KAFKA_FROM_BEGINNING (default: false)
- KAFKA_SSL, KAFKA_SASL_MECHANISM, KAFKA_SASL_USERNAME, KAFKA_SASL_PASSWORD
apps/event-queue/src/index.ts (2)
7-13: Avoid module-level Kafka client/consumer instantiation; construct inside start()Creating the client/consumer at import time introduces side effects, complicates testing, and makes re-initialization harder. Instantiate inside start() to keep lifecycle explicit.
Apply this diff to move construction inside start():
-const kafka = new Kafka({ - clientId: "event-queue", - brokers: env.KAFKA_BROKERS, -}); - -const consumer = kafka.consumer({ groupId: "event-queue" }); +// Construct within start() to avoid module side effectsAnd pair with the edit in the next comment (lines 14-27) to create the instances locally in start().
14-27: Consolidate lifecycle in start(), add graceful shutdown, and keep logs sane
- Move Kafka/consumer construction into start().
- Register SIGINT/SIGTERM handlers to disconnect cleanly.
- Truncate logged message payloads to avoid PII/log bloat while keeping observability.
Apply this diff:
-export const start = async () => { - await consumer.connect(); - await consumer.subscribe({ topic: "ctrlplane-events", fromBeginning: true }); - - await consumer.run({ - eachMessage: async ({ topic, partition, message }) => { - logger.info("Received event", { - topic, - partition, - message: message.value?.toString() ?? "No message", - }); - }, - }); -}; +export const start = async () => { + const kafka = new Kafka({ + clientId: "event-queue", + brokers: env.KAFKA_BROKERS, + }); + const consumer = kafka.consumer({ groupId: "event-queue" }); + + // Graceful shutdown + const shutdown = async (signal: string) => { + try { + logger.info(`Shutting down on ${signal}`); + await consumer.stop(); + await consumer.disconnect(); + } catch (err) { + logger.error("Error during Kafka consumer shutdown", { err }); + } finally { + process.exit(0); + } + }; + process.once("SIGINT", () => void shutdown("SIGINT")); + process.once("SIGTERM", () => void shutdown("SIGTERM")); + + await consumer.connect(); + await consumer.subscribe({ topic: "ctrlplane-events", fromBeginning: true }); + + await consumer.run({ + eachMessage: async ({ topic, partition, message }) => { + const raw = message.value?.toString("utf8"); + const preview = + raw && raw.length > 1024 ? `${raw.slice(0, 1024)}…(truncated)` : raw ?? "No message"; + logger.info("Received event", { + topic, + partition, + message: preview, + }); + }, + }); +};.github/workflows/apps-event-queue.yaml (5)
61-63: Prefix SHA tags to prevent ambiguous short tags and improve discoverabilityBare short SHAs like
abc1234are easy to collide across images/repos. Prefixing aids clarity.- tags: | - type=sha,format=short,prefix= + tags: | + type=sha,format=short,prefix=sha-
64-72: Enable BuildKit layer cache for faster PR buildsCache significantly speeds up repeated builds, especially for monorepos.
- name: Build uses: docker/build-push-action@v6 if: github.ref != 'refs/heads/main' with: push: false file: apps/event-queue/Dockerfile platforms: ${{ matrix.platform }} tags: ${{ steps.meta.outputs.tags }} + cache-from: type=gha + cache-to: type=gha,mode=max
73-80: Also cache on the push job; consider enabling provenanceMirror caching on push; optionally add SBOM/provenance if your org requires it.
- name: Build and Push uses: docker/build-push-action@v6 if: github.ref == 'refs/heads/main' && env.DOCKERHUB_LOGIN == 'true' with: push: true file: apps/event-queue/Dockerfile platforms: ${{ matrix.platform }} tags: ${{ steps.meta.outputs.tags }} + cache-from: type=gha + cache-to: type=gha,mode=max + provenance: true
3-20: Optional: add workflow concurrency to cancel superseded PR buildsPrevents wasting CI minutes when new commits arrive.
on: pull_request: branches: ["*"] @@ - pnpm-lock.yaml push: branches: ["main"] @@ - pnpm-lock.yaml + +concurrency: + group: event-queue-${{ github.workflow }}-${{ github.ref || github.run_id }} + cancel-in-progress: true
80-80: Add a trailing newlineMinor formatting fix flagged by yamllint.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/apps-event-queue.yaml(1 hunks)apps/event-queue/Dockerfile(1 hunks)apps/event-queue/README.md(1 hunks)apps/event-queue/esling.config.js(1 hunks)apps/event-queue/package.json(1 hunks)apps/event-queue/src/config.ts(1 hunks)apps/event-queue/src/index.ts(1 hunks)apps/event-queue/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/event-queue/src/config.tsapps/event-queue/src/index.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/event-queue/src/config.tsapps/event-queue/src/index.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/event-queue/src/config.tsapps/event-queue/tsconfig.jsonapps/event-queue/esling.config.jsapps/event-queue/package.jsonapps/event-queue/README.mdapps/event-queue/src/index.ts
🧠 Learnings (2)
📚 Learning: 2025-03-09T08:56:53.603Z
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#363
File: apps/webservice/tsconfig.json:14-15
Timestamp: 2025-03-09T08:56:53.603Z
Learning: In the ctrlplane project, they use Next.js standalone output mode, which means they intentionally exclude `.next/types` in their tsconfig.json despite including `.next/types/**/*.ts` in their include pattern.
Applied to files:
apps/event-queue/tsconfig.jsonapps/event-queue/esling.config.js
📚 Learning: 2025-07-22T22:34:20.754Z
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T22:34:20.754Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,md,yml,yaml} : Formatting: Prettier is used with `ctrlplane/prettier-config`
Applied to files:
apps/event-queue/tsconfig.jsonapps/event-queue/esling.config.js
🧬 Code graph analysis (1)
apps/event-queue/src/index.ts (2)
apps/event-queue/src/config.ts (1)
env(7-18)packages/logger/src/index.ts (1)
logger(48-48)
🪛 Hadolint (2.12.0)
apps/event-queue/Dockerfile
[error] 48-48: Use absolute WORKDIR
(DL3000)
🪛 actionlint (1.7.7)
.github/workflows/apps-event-queue.yaml
58-58: the runner of "docker/metadata-action@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/apps-event-queue.yaml
[error] 80-80: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (2)
apps/event-queue/tsconfig.json (1)
3-11: ESM resolution confirmed
The base tsconfig (base.json) is already configured for ESM on Node 20:
"module": "Preserve""moduleResolution": "Bundler"These settings leave import/export syntax intact and use the bundler-style resolution introduced in TypeScript 5.x, which aligns with modern ESM workflows on Node 20. No additional overrides are needed in
apps/event-queue/tsconfig.json.apps/event-queue/src/config.ts (1)
1-6: LGTM on env scaffolding and dotenv initializationdotenv is loaded before env parsing and zod schemas are defined—good ordering. Typed runtime config via @t3-oss/env-core is a solid choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/apps-event-queue.yaml (2)
70-70: Double-check Dockerfile actually builds the event-queue app.A prior review found apps/event-queue/Dockerfile was copying/building workspace-engine instead of event-queue. Since this workflow builds that Dockerfile in both steps, ensure it now targets the correct app paths and entrypoint.
#!/bin/bash # Sanity check the Dockerfile still in use by this workflow set -euo pipefail DF="apps/event-queue/Dockerfile" test -f "$DF" || { echo "Missing $DF"; exit 1; } echo "Scanning $DF for wrong app references..." rg -n -i 'workspace-engine' "$DF" && { echo "Found workspace-engine references — fix before merging."; exit 2; } || echo "OK: no workspace-engine refs." # Show key instructions to eyeball rg -n -C2 -e 'WORKDIR|COPY|RUN|ENTRYPOINT|CMD' "$DF" || trueAlso applies to: 79-79
57-64: Bump docker/metadata-action to v5 (actionlint failure).actionlint flags v4 as incompatible with current runners. Upgrade to v5 to avoid CI breakage.
- - name: Extract metadata (tags, labels) for Docker - id: meta - uses: docker/metadata-action@v4 + - name: Extract metadata (tags, labels) for Docker + id: meta + uses: docker/metadata-action@v5 with: images: ctrlplane/event-queue tags: | type=sha,format=short,prefix=
🧹 Nitpick comments (6)
.github/workflows/apps-event-queue.yaml (6)
36-38: Drop QEMU; you only build linux/amd64.QEMU emulation isn’t needed for single-arch amd64, and removing it will speed up jobs.
- - name: Set up QEMU - uses: docker/setup-qemu-action@v3
25-28: Tighten permissions: id-token not used.OIDC (id-token: write) isn’t consumed in this workflow (Docker Hub auth uses username/token). Prefer least privilege.
permissions: contents: read - id-token: write
65-81: Consolidate duplicate build steps; add cache + labels.You can collapse Build and Build and Push into one step keyed by an expression, add GHA cache to speed builds, and apply image labels from metadata.
- - name: Build - uses: docker/build-push-action@v6 - if: github.ref != 'refs/heads/main' - with: - push: false - file: apps/event-queue/Dockerfile - platforms: ${{ matrix.platform }} - tags: ${{ steps.meta.outputs.tags }} - - - name: Build and Push - uses: docker/build-push-action@v6 - if: github.ref == 'refs/heads/main' && env.DOCKERHUB_LOGIN == 'true' - with: - push: true - file: apps/event-queue/Dockerfile - platforms: ${{ matrix.platform }} - tags: ${{ steps.meta.outputs.tags }} + - name: Build (and Push on main when logged in) + uses: docker/build-push-action@v6 + with: + push: ${{ github.ref == 'refs/heads/main' && env.DOCKERHUB_LOGIN == 'true' }} + file: apps/event-queue/Dockerfile + platforms: ${{ matrix.platform }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max
61-64: Optional: publish a stable tag on main (e.g., latest).Right now only short-SHA tags are produced. Consider adding a stable tag for main pushes to simplify deployments.
Example (if you keep metadata-action):
- Add an extra tag: type=raw,value=latest (enabled only on main).
- Or add type=ref,event=branch to include branch tags for non-main.
4-11: Trigger simplification (minor).For pull_request, branches: ["*"] is redundant; omitting it defaults to all base branches. Not required to change.
81-81: Add trailing newline to satisfy linters.YAMLlint reports “no new line character at the end of file.”
tags: ${{ steps.meta.outputs.tags }} +
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/apps-event-queue.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/apps-event-queue.yaml
59-59: the runner of "docker/metadata-action@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/apps-event-queue.yaml
[error] 81-81: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/apps-event-queue.yaml (1)
57-60: Resolved: Upgraded docker/metadata-action to v5This addresses the earlier actionlint warning about v4 incompatibility. Good fix.
🧹 Nitpick comments (10)
.github/workflows/apps-event-queue.yaml (5)
36-38: QEMU not needed for single-arch (linux/amd64) buildsYou only build linux/amd64, so QEMU adds overhead without benefit. Safe to remove.
- - name: Set up QEMU - uses: docker/setup-qemu-action@v3
65-73: Speed up builds with BuildKit cache and add OCI labelsEnabling GHA cache drastically reduces build times. Also surface labels from metadata for better provenance.
- name: Build uses: docker/build-push-action@v6 if: github.ref != 'refs/heads/main' with: push: false file: apps/event-queue/Dockerfile platforms: ${{ matrix.platform }} - tags: ${{ steps.meta.outputs.tags }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max
74-81: Mirror cache + labels on the push job; optionally pass secrets for private depsConsistent cache setup on the push path keeps main builds fast. If you follow the Dockerfile recommendation to stop copying .npmrc into the image, plumb a BuildKit secret here.
- name: Build and Push uses: docker/build-push-action@v6 if: github.ref == 'refs/heads/main' && env.DOCKERHUB_LOGIN == 'true' with: push: true file: apps/event-queue/Dockerfile platforms: ${{ matrix.platform }} - tags: ${{ steps.meta.outputs.tags }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max + # If using Dockerfile secret mount for .npmrc (recommended), uncomment: + # secrets: | + # npmrc=${{ secrets.NPMRC }}
25-28: Drop id-token permission if not using OIDCNo OIDC-based registry login here. Trim unnecessary permissions.
permissions: contents: read - id-token: write
81-81: Add trailing newline to satisfy yamllintThe file is missing a newline at EOF; some linters treat this as an error.
apps/event-queue/Dockerfile (5)
48-48: Use an absolute WORKDIR to satisfy hadolint DL3000 and avoid ambiguityRelative paths can be brittle. Switch to an absolute path rooted at /app.
-WORKDIR apps/event-queue/dist/ +WORKDIR /app/apps/event-queue/dist/
10-13: Avoid installing Turbo twice and mixing npm/pnpm for globalsYou install Turbo globally via npm and again via pnpm. Keep one (pnpm is fine) to reduce layers and avoid PATH confusion.
-RUN npm install -g turbo @@ -RUN pnpm add -g turbo +RUN pnpm add -g turboAlso applies to: 18-18
16-16: Nit: copying .gitignore needlessly invalidates cacheThis file is irrelevant to the build and can cause extra cache churn.
-COPY .gitignore .gitignore
46-47: Consider dropping EXPOSE for a background consumerIf the service doesn’t listen on a port, EXPOSE 3123 is misleading. Remove unless you actually bind a port at runtime.
-EXPOSE 3123
5-5: Optional: move build tools to a builder stage to slim the runtime imagepython3/make/g++ are only needed for builds. A multi-stage build (builder + runtime) will reduce image size and CVE surface. I can draft a multi-stage Dockerfile if helpful.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/workflows/apps-event-queue.yaml(1 hunks)apps/event-queue/Dockerfile(1 hunks)apps/event-queue/eslint.config.js(1 hunks)apps/event-queue/package.json(1 hunks)apps/event-queue/src/config.ts(1 hunks)apps/event-queue/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/event-queue/src/config.ts
- apps/event-queue/package.json
- apps/event-queue/src/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/event-queue/eslint.config.js
🧠 Learnings (2)
📚 Learning: 2025-03-09T08:56:53.603Z
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#363
File: apps/webservice/tsconfig.json:14-15
Timestamp: 2025-03-09T08:56:53.603Z
Learning: In the ctrlplane project, they use Next.js standalone output mode, which means they intentionally exclude `.next/types` in their tsconfig.json despite including `.next/types/**/*.ts` in their include pattern.
Applied to files:
apps/event-queue/eslint.config.js
📚 Learning: 2025-07-22T22:34:20.754Z
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T22:34:20.754Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,md,yml,yaml} : Formatting: Prettier is used with `ctrlplane/prettier-config`
Applied to files:
apps/event-queue/eslint.config.js
🪛 Hadolint (2.12.0)
apps/event-queue/Dockerfile
[error] 48-48: Use absolute WORKDIR
(DL3000)
🪛 YAMLlint (1.37.1)
.github/workflows/apps-event-queue.yaml
[error] 81-81: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (1)
apps/event-queue/eslint.config.js (1)
1-8: ESLint flat config looks goodImport order, ignore patterns, and spreading baseConfig/requireJsSuffix are consistent with the monorepo setup.
| COPY package.json package.json | ||
| COPY .npmrc .npmrc | ||
| COPY pnpm-*.yaml . | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Do not bake .npmrc into the image; mount it as a BuildKit secret during install
Copying .npmrc into the image risks leaking tokens in layers. Use a secret mount for pnpm install instead.
-COPY .npmrc .npmrc
@@
-RUN pnpm install --frozen-lockfile
+RUN --mount=type=secret,id=npmrc,dst=/root/.npmrc pnpm install --frozen-lockfileFollow-up (workflow): pass the secret to docker/build-push-action (see my workflow comment adding secrets: npmrc=${{ secrets.NPMRC }}).
Also applies to: 33-33
| RUN addgroup --system --gid 1001 nodejs | ||
| RUN adduser --system --uid 1001 nodejs | ||
| USER nodejs | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Alpine adduser/addgroup flags; current build will fail
BusyBox on Alpine does not support the long options --system/--gid/--uid as used; this will break the build. Use -S, -g, -u, and ensure ownership before switching users.
-RUN addgroup --system --gid 1001 nodejs
-RUN adduser --system --uid 1001 nodejs
-USER nodejs
+RUN addgroup -S -g 1001 nodejs \
+ && adduser -S -G nodejs -u 1001 nodejs \
+ && chown -R 1001:1001 /app
+USER nodejs🤖 Prompt for AI Agents
In apps/event-queue/Dockerfile around lines 39 to 42, the addgroup/adduser
commands use non‑existent long BusyBox flags (--system/--gid/--uid) which break
Alpine builds; replace them with Alpine/BusyBox flags (addgroup -S -g 1001
nodejs and adduser -S -u 1001 -G nodejs nodejs), ensure the user is added to the
group, fix ownership of relevant app directories (chown -R nodejs:nodejs
<paths>) before switching users, and then keep the final USER nodejs line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/event-queue/Dockerfile (3)
41-44: Alpine adduser/addgroup flags are invalid; build will fail — switch to BusyBox-compatible flags and fix ownership.Use
-S/-g/-uand ensure the user is in the group and owns /app before switching users.Apply:
-RUN addgroup --system --gid 1001 nodejs -RUN adduser --system --uid 1001 nodejs -USER nodejs +RUN addgroup -S -g 1001 nodejs \ + && adduser -S -G nodejs -u 1001 nodejs \ + && chown -R 1001:1001 /app +USER nodejs
50-50: Fix hadolint DL3000: WORKDIR must be absolute.This is a linter error and can also cause path ambiguity at runtime.
-WORKDIR apps/event-queue/dist/ +WORKDIR /app/apps/event-queue/dist/
21-21: Do not bake .npmrc into the image; use a BuildKit secret for install and ensure it’s excluded from COPY.Copying
.npmrccan leak registry tokens through image layers. Mount it only for the install step and make sureCOPY . .does not re-add it.Apply:
-COPY .npmrc .npmrc @@ -RUN pnpm install --frozen-lockfile +RUN --mount=type=secret,id=npmrc,dst=/root/.npmrc pnpm install --frozen-lockfileAlso add
.npmrcto.dockerignore(and keepnode_modules,.gitetc. out):.npmrc node_modules .gitIn the workflow, pass the secret to buildx (e.g., docker/build-push-action) as
secrets: npmrc=${{ secrets.NPMRC }}.Also applies to: 35-35, 37-37
🧹 Nitpick comments (4)
apps/event-queue/Dockerfile (4)
10-13: Avoid global Turbo installs; use the workspace version via pnpm.Reduces image bloat, aligns with repo-managed versions, and removes duplicate global installs.
-RUN npm install -g turbo @@ -RUN pnpm add -g turbo @@ -RUN turbo build --filter=...@ctrlplane/event-queue +RUN pnpm turbo run build --filter=...@ctrlplane/event-queueAlso applies to: 18-18, 39-39
48-48: EXPOSE likely unnecessary for a Kafka consumer — confirm and remove if unused.If the app doesn’t open an HTTP/GRPC port, exposing 3123 is misleading.
-EXPOSE 3123Can you confirm whether the event-queue service starts a server on port 3123? If not, let’s drop this line.
16-16: Nit: copying .gitignore into the image has no effect.Docker doesn’t use
.gitignore; remove to trim a layer.-COPY .gitignore .gitignore
1-52: Optional: switch to a multi-stage build to shed toolchains and shrink the final image.Right now, the runtime image carries build deps (python3, make, g++, turbo). A multi-stage build keeps those only in the builder.
Example skeleton:
# ---- builder ---- ARG NODE_VERSION=20 FROM node:${NODE_VERSION}-alpine AS builder RUN apk add --no-cache libc6-compat python3 make g++ curl ENV PNPM_HOME="/pnpm" PATH="$PNPM_HOME:$PATH" RUN npm install -g corepack@latest && corepack enable pnpm WORKDIR /app COPY turbo.json package.json pnpm-*.yaml ./ COPY tooling/prettier/package.json tooling/prettier/package.json COPY tooling/eslint/package.json tooling/eslint/package.json COPY tooling/typescript/package.json tooling/typescript/package.json COPY packages/logger/package.json packages/logger/package.json COPY packages/events/package.json packages/events/package.json COPY packages/secrets/package.json packages/secrets/package.json COPY packages/validators/package.json packages/validators/package.json COPY apps/event-queue/package.json apps/event-queue/package.json # mount npmrc secret for install RUN --mount=type=secret,id=npmrc,dst=/root/.npmrc pnpm install --frozen-lockfile COPY . . # use workspace turbo RUN pnpm turbo run build --filter=...@ctrlplane/event-queue # optional: prune to production deps for the app RUN pnpm -C apps/event-queue prune --prod # ---- runner ---- FROM node:${NODE_VERSION}-alpine AS runner WORKDIR /app ENV NODE_ENV=production NODE_MAX_OLD_SPACE_SIZE=4096 # create non-root user RUN addgroup -S -g 1001 nodejs \ && adduser -S -G nodejs -u 1001 nodejs # copy only what’s needed COPY --from=builder /app/apps/event-queue/dist/ /app/apps/event-queue/dist/ COPY --from=builder /app/apps/event-queue/node_modules/ /app/apps/event-queue/node_modules/ COPY --from=builder /app/package.json /app/package.json RUN chown -R 1001:1001 /app USER nodejs WORKDIR /app/apps/event-queue/dist/ CMD ["node", "index.js"]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
apps/event-queue/Dockerfile(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
apps/event-queue/Dockerfile
[error] 50-50: Use absolute WORKDIR
(DL3000)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/event-queue/Dockerfile (3)
53-55: Fix DL3000: use an absolute WORKDIR (and keep CMD path stable).Hadolint flags the relative WORKDIR. Use an absolute path rooted at /app to avoid ambiguity and satisfy DL3000.
Apply:
-WORKDIR apps/event-queue/dist/ +WORKDIR /app/apps/event-queue/dist/ CMD ["node", "index.js"]
21-23: Don’t bake .npmrc into the image; mount it as a BuildKit secret for install.Copying .npmrc risks leaking tokens in image layers. Use a secret mount on the install step instead. You’ll also want to pass the secret in your CI workflow.
Apply:
-COPY .npmrc .npmrc @@ -RUN pnpm install --frozen-lockfile +RUN --mount=type=secret,id=npmrc,dst=/root/.npmrc \ + --mount=type=cache,id=pnpm-store,target=/pnpm/store \ + pnpm install --frozen-lockfileFollow-up:
- In your GitHub Actions build step, pass the secret to docker/build-push-action, e.g.,
secrets: npmrc=${{ secrets.NPMRC }}.Also applies to: 38-38
44-46: Alpine adduser/addgroup flags are invalid; build will fail.BusyBox on Alpine doesn’t support the long flags used here. Replace with Alpine-compatible flags and fix ownership before switching users.
Apply:
-RUN addgroup --system --gid 1001 nodejs -RUN adduser --system --uid 1001 nodejs -USER nodejs +RUN addgroup -S -g 1001 nodejs \ + && adduser -S -G nodejs -u 1001 nodejs \ + && chown -R 1001:1001 /app +USER nodejs
🧹 Nitpick comments (6)
apps/event-queue/Dockerfile (6)
10-11: Remove duplicate turbo installation and prefer invoking via pnpm dlx (or fix command form).Turbo is installed twice (npm and pnpm). Keep one source of truth and simplify invocation. Also, the canonical CLI is
turbo run build.Option A (minimal change — keep npm global turbo, remove pnpm global install, fix command):
-RUN pnpm add -g turbo @@ -RUN turbo build --filter=...@ctrlplane/event-queue +RUN turbo run build --filter=...@ctrlplane/event-queueOption B (no global install — use pnpm dlx):
-RUN npm install -g turbo -# RUN pnpm add -g turbo # remove @@ -RUN turbo build --filter=...@ctrlplane/event-queue +RUN pnpm dlx turbo@latest run build --filter=...@ctrlplane/event-queuePlease confirm which approach you prefer and that
turbo buildwasn’t relying on a repo-local alias. If not, switch toturbo run build.Also applies to: 18-18, 42-42
51-51: Confirm whether EXPOSE 3123 is actually needed for a Kafka consumer.If the service doesn’t bind to a port, EXPOSE is misleading and can be dropped.
16-16: Unnecessary COPY of .gitignore.This file isn’t used at runtime or build-time. Removing it saves a layer and a few bytes.
Apply:
-COPY .gitignore .gitignore
38-38: Speed up installs with a persistent pnpm store cache.In addition to the secret mount, using a cache mount for pnpm store significantly speeds up rebuilds.
If you keep the current structure, ensure the install line uses:
-RUN pnpm install --frozen-lockfile +RUN --mount=type=cache,id=pnpm-store,target=/pnpm/store pnpm install --frozen-lockfileNote: If you applied the secret+cache combined line in my earlier comment, you’re covered.
5-13: Minor cleanup opportunities (optional).
- corepack: Node 20 ships with Corepack; installing it via npm is usually unnecessary.
corepack enableis sufficient.- PNPM_HOME and PATH tweaks are optional if you rely on
corepack pnpm.Potential cleanup:
-RUN npm install -g corepack@latest -RUN corepack enable pnpm +RUN corepack enable
24-36: Streamline Dockerfile’s manifest-copy layerAll declared workspace dependencies for apps/event-queue—events and logger—are already being copied, so you won’t lose any cache hits. However, the Dockerfile is also copying manifests for packages that aren’t actual dependencies of event-queue, which only adds build noise and maintenance overhead.
Please trim the extra COPY directives in apps/event-queue/Dockerfile (lines 24–36) so they only include:
- tooling/prettier
- tooling/eslint
- tooling/typescript
- packages/logger
- packages/events
- apps/event-queue
Remove these unused package entries from that block:
- packages/db
- packages/job-dispatch
- packages/rule-engine
- packages/secrets
- packages/validators
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/event-queue/Dockerfile(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
apps/event-queue/Dockerfile
[error] 53-53: Use absolute WORKDIR
(DL3000)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
| USER nodejs | ||
|
|
||
| ENV NODE_ENV=production | ||
| ENV NODE_MAX_OLD_SPACE_SIZE=4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
NODE_MAX_OLD_SPACE_SIZE has no effect; use NODE_OPTIONS.
Node reads memory flags from NODE_OPTIONS or CLI args. The current env var won’t change heap size.
Apply:
-ENV NODE_MAX_OLD_SPACE_SIZE=4096
+ENV NODE_OPTIONS="--max-old-space-size=4096"📝 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.
| ENV NODE_MAX_OLD_SPACE_SIZE=4096 | |
| ENV NODE_OPTIONS="--max-old-space-size=4096" |
🤖 Prompt for AI Agents
In apps/event-queue/Dockerfile around lines 49-49, the Dockerfile sets ENV
NODE_MAX_OLD_SPACE_SIZE=4096 which Node ignores; replace it with an ENV that
sets NODE_OPTIONS to include the memory flag (e.g. ENV
NODE_OPTIONS="--max-old-space-size=4096"), or if NODE_OPTIONS may already exist,
append the flag to the existing value (preserve any existing NODE_OPTIONS and
add --max-old-space-size=4096).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/event-worker/Dockerfile (2)
53-57: Fix memory flag mismatch: undefined env var in CMD can break Node startupYou replaced NODE_MAX_OLD_SPACE_SIZE with NODE_OPTIONS, but CMD still expands ${NODE_MAX_OLD_SPACE_SIZE}. In shell-form CMD this becomes "--max-old-space-size=" (empty), which can cause Node to error. Rely on NODE_OPTIONS and switch to exec-form CMD for proper signal handling.
Apply this diff:
-ENV NODE_OPTIONS="--max-old-space-size=4096" +ENV NODE_OPTIONS="--max-old-space-size=4096" @@ -CMD node -r ./apps/event-worker/dist/instrumentation-node.js --max-old-space-size=${NODE_MAX_OLD_SPACE_SIZE} apps/event-worker/dist/index.js +CMD ["node", "-r", "./apps/event-worker/dist/instrumentation-node.js", "apps/event-worker/dist/index.js"]Alternative (if you want to keep the CLI flag instead of NODE_OPTIONS): restore the variable and remove NODE_OPTIONS.
-ENV NODE_OPTIONS="--max-old-space-size=4096" +ENV NODE_MAX_OLD_SPACE_SIZE=4096 @@ -CMD node -r ./apps/event-worker/dist/instrumentation-node.js --max-old-space-size=${NODE_MAX_OLD_SPACE_SIZE} apps/event-worker/dist/index.js +CMD ["node", "-r", "./apps/event-worker/dist/instrumentation-node.js", "--max-old-space-size=${NODE_MAX_OLD_SPACE_SIZE}", "apps/event-worker/dist/index.js"]
25-25: Avoid baking .npmrc into the final image (possible token leak)Copying .npmrc into the image can leak private registry tokens in image layers. Use BuildKit secrets or a multi-stage build to keep credentials out of the final image.
Example (BuildKit secret):
# syntax=docker/dockerfile:1.7 # ... RUN --mount=type=secret,id=npmrc,target=/root/.npmrc pnpm install --frozen-lockfileOr multi-stage: perform install/build in a builder stage that has .npmrc, then copy only dist/ into the final runtime image.
🧹 Nitpick comments (3)
apps/event-worker/Dockerfile (3)
14-14: Remove duplicate global installs of turboYou're installing turbo globally twice (npm and pnpm). Keep one. Since you already enable pnpm, prefer the pnpm global install.
Apply this diff:
-RUN npm install -g turboAlso applies to: 22-22
46-46: Verify turbo filter syntaxDouble-check the filter. Turbo commonly uses "@pkg..." to include the package and its deps. The current pattern has the ellipsis before the package name.
Potential correction:
-RUN turbo build --filter=...@ctrlplane/event-worker +RUN turbo build --filter=@ctrlplane/event-worker...Please confirm which form your turbo version expects.
10-13: Trim unnecessary runtime tools to reduce attack surfaceIf vcluster isn’t required at runtime for event-worker, consider moving it to a builder stage or removing it to shrink the image and minimize CVE surface.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/event-worker/Dockerfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (1)
apps/event-worker/Dockerfile (1)
52-52: Good: run as non-root with NODE_ENV=productionRunning as a non-root user and setting NODE_ENV=production is a solid baseline for runtime security and performance.
Summary by CodeRabbit
New Features
Documentation
Chores