-
Notifications
You must be signed in to change notification settings - Fork 130
feat(toolchain): build args support for remote builds #2583
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
feat(toolchain): build args support for remote builds #2583
Conversation
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.
- serialize args on rust side
- implement target
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.
PR Summary
Adds Docker build argument support for remote builds in Rivet's CI system, enabling dynamic configuration during build time through multiple components.
- Added new
cloud/packages/ci-manager/src/common.tswith utility functions for handling build argument escaping and conversion - Introduced
cloud/packages/ci-runner/entry.shto safely process build arguments using UNIT_SEP_CHAR (0x1F) delimiter - Modified
cloud/packages/ci-manager/src/types.tsto add requiredbuildArgs: Record<string, string>to BuildInfo interface - Enhanced
/buildsendpoint incloud/packages/ci-manager/src/server.tswith validation for build argument format and sanitization - Updated Kaniko execution in Docker and Rivet executors to properly handle space-containing build arguments
9 files reviewed, 7 comments
Edit PR Review Bot Settings | Greptile
55281e5 to
c2676b7
Compare
8675015 to
fbbfba0
Compare
| `--destination=${args.destination}`, | ||
| `--upload-tar=${args.outputUrl}`, | ||
| `--dockerfile=${args.dockerfilePath}`, | ||
| ...(args.buildTarget ? [`--target='${args.buildTarget}'`] : []), |
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.
The --target argument should not include single quotes around the value. This will cause the quotes to be passed literally to the Docker build process, which will fail to find the target.
Change:
...(args.buildTarget ? [`--target='${args.buildTarget}'`] : []),To:
...(args.buildTarget ? [`--target=${args.buildTarget}`] : []),| ...(args.buildTarget ? [`--target='${args.buildTarget}'`] : []), | |
| ...(args.buildTarget ? [`--target=${args.buildTarget}`] : []), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Merge activity
|
<!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes RVT-4787 <!-- If there are frontend changes, please include screenshots. -->

Changes
RVT-4787