Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces support for an optional entrypointPrefix in the build system and establishes a new CI/CD workflow for publishing Docker images to Google Cloud Artifact Registry. It adds a new entrypointPrefix field to the BuildLayer and BuildManifest schemas, propagates this configuration through the build image generation logic via a new serializeEntrypoint helper, and creates a parameterized GitHub Actions workflow that builds and pushes Docker images for both the webapp and worker packages (coordinator, docker-provider, kubernetes-provider, supervisor) to Google Cloud Artifact Registry. The workflow is integrated into the main publish flow as a dependent job. A debug logging statement is removed from the managed-run-controller entrypoint. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
✏️ Tip: You can disable this entire section by setting Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @pnispel, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (layer.image.entrypointPrefix) { | ||
| $manifest.image.entrypointPrefix = [...layer.image.entrypointPrefix]; | ||
| } |
There was a problem hiding this comment.
🚩 entrypointPrefix replaces rather than concatenates, unlike other image layer properties
In applyLayerToManifest, when a layer has image.entrypointPrefix, the manifest's existing entrypointPrefix is completely replaced ($manifest.image.entrypointPrefix = [...layer.image.entrypointPrefix]). This contrasts with image.pkgs (line 207-209) and image.instructions (line 203-204), which both concatenate across layers.
If two different build extensions each call context.addLayer() with an entrypointPrefix, the second silently overwrites the first. This may be intentional (you'd want exactly one entrypoint prefix, not a merged one), but it's an inconsistency worth documenting. A user with two extensions that both set entrypointPrefix would get surprising behavior where only the last-processed extension's prefix takes effect.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function serializeEntrypoint(entrypointPrefix: string[], entrypoint: string) { | ||
| return JSON.stringify(["dumb-init", ...entrypointPrefix, "node", entrypoint]); | ||
| } |
There was a problem hiding this comment.
🚩 entrypointPrefix items are passed as arguments to dumb-init, affecting process execution chain
The serialized entrypoint ["dumb-init", ...entrypointPrefix, "node", entrypoint] means dumb-init will execute the first element of entrypointPrefix as the child process (with remaining items as its arguments), rather than executing node directly. For example, ["dumb-init", "my-wrapper", "node", "entry.js"] makes dumb-init run my-wrapper node entry.js. This is likely the intended use case (wrapping the node process with a custom tool), but users need to understand that their prefix command must pass through to node — it's not just prepending flags. The naming "entrypointPrefix" could lead users to think they're adding flags to dumb-init or node, when they're actually inserting an intermediate command in the exec chain.
Was this helpful? React with 👍 or 👎 to provide feedback.
| secrets: | | ||
| sentry_auth_token= |
There was a problem hiding this comment.
🚩 GCP workflow sentry_auth_token is set to empty string
At line 100, the build secret sentry_auth_token= is set to an empty value. This means Sentry source map uploads won't work for GCP-published images. Compare with the existing publish-webapp.yml which likely passes a real secret. This may be intentional if Sentry integration isn't needed for GCP builds, or it could be a placeholder that was forgotten.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Closes #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯