From b90675b297a22ea2c908b72426d80d9436dca1f6 Mon Sep 17 00:00:00 2001 From: abcxff <79597906+abcxff@users.noreply.github.com> Date: Wed, 11 Jun 2025 18:11:24 -0400 Subject: [PATCH] feat(toolchain): build args support for remote builds --- cloud/packages/ci-manager/src/build-store.ts | 32 ++++--- cloud/packages/ci-manager/src/common.ts | 44 ++++++++++ .../ci-manager/src/executors/docker.ts | 20 +++-- .../ci-manager/src/executors/rivet.ts | 13 ++- .../packages/ci-manager/src/kaniko-runner.ts | 4 +- cloud/packages/ci-manager/src/server.ts | 52 ++++++----- cloud/packages/ci-manager/src/types.ts | 88 ++++++++++++++----- cloud/packages/ci-runner/Dockerfile | 5 +- cloud/packages/ci-runner/entry.sh | 18 ++++ examples/system-test-actor/rivet.jsonc | 10 ++- .../toolchain/src/util/docker/build.rs | 2 +- .../toolchain/src/util/docker/build_remote.rs | 23 ++++- 12 files changed, 225 insertions(+), 86 deletions(-) create mode 100644 cloud/packages/ci-manager/src/common.ts create mode 100644 cloud/packages/ci-runner/entry.sh diff --git a/cloud/packages/ci-manager/src/build-store.ts b/cloud/packages/ci-manager/src/build-store.ts index 2d5612010d..ec97718dfe 100644 --- a/cloud/packages/ci-manager/src/build-store.ts +++ b/cloud/packages/ci-manager/src/build-store.ts @@ -20,7 +20,13 @@ export class BuildStore { await mkdir(this.tempDir, { recursive: true }); } - createBuild(buildName: string, dockerfilePath: string, environmentId: string): string { + createBuild( + buildName: string, + dockerfilePath: string, + environmentId: string, + buildArgs: Record, + buildTarget: string | undefined + ): string { const id = randomUUID(); const contextPath = join(this.tempDir, id, "context.tar.gz"); const outputPath = join(this.tempDir, id, "output.tar.gz"); @@ -32,6 +38,8 @@ export class BuildStore { dockerfilePath, environmentId, contextPath, + buildArgs, + buildTarget, outputPath, events: [], createdAt: new Date(), @@ -86,7 +94,7 @@ export class BuildStore { } } - getContextPath(id: string): string | undefined { + getContextPath(id: string): string | undefined{ return this.builds.get(id)?.contextPath; } @@ -118,17 +126,15 @@ export class BuildStore { } // Remove build directory and all files - if (build.contextPath) { - const buildDir = dirname(build.contextPath); - try { - await rm(buildDir, { recursive: true, force: true }); - console.log(`Removed build directory: ${buildDir}`); - } catch (error) { - console.warn( - `Failed to remove build directory ${buildDir}:`, - error, - ); - } + const buildDir = dirname(build.contextPath); + try { + await rm(buildDir, { recursive: true, force: true }); + console.log(`Removed build directory: ${buildDir}`); + } catch (error) { + console.warn( + `Failed to remove build directory ${buildDir}:`, + error, + ); } // Remove from memory diff --git a/cloud/packages/ci-manager/src/common.ts b/cloud/packages/ci-manager/src/common.ts new file mode 100644 index 0000000000..a86bff5265 --- /dev/null +++ b/cloud/packages/ci-manager/src/common.ts @@ -0,0 +1,44 @@ +// Keep in sync with ci-runner/entry.sh +export const UNIT_SEP_CHAR = "\x1F"; +export const NO_SEP_CHAR_REGEX = /^[^\x1F]+$/; + +interface KanikoArguments { + contextUrl: string; + outputUrl: string; + destination: string; + dockerfilePath: string; + buildArgs: Record; + buildTarget?: string; +} + +// SAFETY: buildArgs keys never have equal signs or spaces +function convertBuildArgsToArgs( + buildArgs: Record, +): string[] { + return Object.entries(buildArgs).flatMap(([key, value]) => [ + `--build-arg`, + `${key}=${value}`, + ]); +} + +export function serializeKanikoArguments(args: KanikoArguments): string { + // SAFETY: Nothing needed to be escaped, as values are already sanitized, + // and are joined by IFS=UNIT_SEP_CHAR (see entry.sh of ci-runner). + const preparedArgs = [ + ...convertBuildArgsToArgs(args.buildArgs), + `--context=${args.contextUrl}`, + `--destination=${args.destination}`, + `--upload-tar=${args.outputUrl}`, + `--dockerfile=${args.dockerfilePath}`, + ...(args.buildTarget ? [`--target='${args.buildTarget}'`] : []), + "--no-push", + "--single-snapshot", + "--verbosity=info", + ].map(arg => { + // Args should never contain UNIT_SEP_CHAR, but we can + // escape it if they do. + return arg.replaceAll(UNIT_SEP_CHAR, "\\" + UNIT_SEP_CHAR) + }); + + return preparedArgs.join(UNIT_SEP_CHAR); +} \ No newline at end of file diff --git a/cloud/packages/ci-manager/src/executors/docker.ts b/cloud/packages/ci-manager/src/executors/docker.ts index 0adceab46d..b707f40940 100644 --- a/cloud/packages/ci-manager/src/executors/docker.ts +++ b/cloud/packages/ci-manager/src/executors/docker.ts @@ -1,5 +1,6 @@ import { spawn } from "node:child_process"; import { BuildStore } from "../build-store"; +import { serializeKanikoArguments, UNIT_SEP_CHAR } from "../common"; export async function runDockerBuild( buildStore: BuildStore, @@ -19,13 +20,16 @@ export async function runDockerBuild( "--rm", "--network=host", "-e", - `CONTEXT_URL=${contextUrl}`, - "-e", - `OUTPUT_URL=${outputUrl}`, - "-e", - `DESTINATION=${buildId}:latest`, - "-e", - `DOCKERFILE_PATH=${build.dockerfilePath}`, + `KANIKO_ARGS=${ + serializeKanikoArguments({ + contextUrl, + outputUrl, + destination: `${buildId}:latest`, + dockerfilePath: build.dockerfilePath, + buildArgs: build.buildArgs, + buildTarget: build.buildTarget, + }) + }`, "ci-runner", ]; @@ -36,7 +40,7 @@ export async function runDockerBuild( return new Promise((resolve, reject) => { const dockerProcess = spawn("docker", kanikoArgs, { - stdio: ["pipe", "pipe", "pipe"], + stdio: ["pipe", "pipe", "pipe"] }); buildStore.setContainerProcess(buildId, dockerProcess); diff --git a/cloud/packages/ci-manager/src/executors/rivet.ts b/cloud/packages/ci-manager/src/executors/rivet.ts index 936498566e..021f6c36da 100644 --- a/cloud/packages/ci-manager/src/executors/rivet.ts +++ b/cloud/packages/ci-manager/src/executors/rivet.ts @@ -1,5 +1,6 @@ import { RivetClient } from "@rivet-gg/api"; import { BuildStore } from "../build-store"; +import { serializeKanikoArguments } from "../common"; export async function runRivetBuild( buildStore: BuildStore, @@ -49,10 +50,14 @@ export async function runRivetBuild( build: kanikoBuildId, runtime: { environment: { - CONTEXT_URL: contextUrl, - OUTPUT_URL: outputUrl, - DESTINATION: `${buildId}:latest`, - DOCKERFILE_PATH: build.dockerfilePath!, + KANIKO_ARGS: serializeKanikoArguments({ + contextUrl, + outputUrl, + destination: `${buildId}:latest`, + dockerfilePath: build.dockerfilePath, + buildArgs: build.buildArgs, + buildTarget: build.buildTarget, + }) }, }, network: { diff --git a/cloud/packages/ci-manager/src/kaniko-runner.ts b/cloud/packages/ci-manager/src/kaniko-runner.ts index 86a6dd24b8..1de7535642 100644 --- a/cloud/packages/ci-manager/src/kaniko-runner.ts +++ b/cloud/packages/ci-manager/src/kaniko-runner.ts @@ -37,10 +37,10 @@ async function validateBuildUpload( buildId: string, ): Promise { const build = buildStore.getBuild(buildId); - if (!build || !build.outputPath) { + if (!build) { buildStore.updateStatus(buildId, { type: "failure", - data: { reason: "Build not found or missing output path" }, + data: { reason: "Build not found" }, }); return; } diff --git a/cloud/packages/ci-manager/src/server.ts b/cloud/packages/ci-manager/src/server.ts index c4e3ad3f6b..ae8b6e5c20 100644 --- a/cloud/packages/ci-manager/src/server.ts +++ b/cloud/packages/ci-manager/src/server.ts @@ -17,14 +17,16 @@ import { uploadOCIBundleToRivet, type RivetUploadConfig, } from "./rivet-uploader"; +import { UNIT_SEP_CHAR } from "./common"; +import { BuildRequestSchema } from "./types"; async function processRivetUpload( buildStore: BuildStore, buildId: string, ): Promise { const build = buildStore.getBuild(buildId); - if (!build || !build.outputPath) { - throw new Error(`Build ${buildId} not found or missing output path`); + if (!build) { + throw new Error(`Build ${buildId} not found`); } try { @@ -63,7 +65,7 @@ async function processRivetUpload( const uploadResult = await uploadOCIBundleToRivet( conversionResult.bundleTarPath, - build.buildName!, + build.buildName, `${buildId}:latest`, // Match kaniko destination format rivetConfig, new Date().toISOString(), // Use timestamp as version for now @@ -100,30 +102,31 @@ export async function createServer(port: number = 3000) { app.post("/builds", async (c) => { try { - const formData = await c.req.formData(); - const buildName = formData.get("buildName") as string; - const dockerfilePath = formData.get("dockerfilePath") as string; - const environmentId = formData.get("environmentId") as string; - const contextFile = formData.get("context") as File; - - if (!buildName) { - return c.json({ error: "buildName is required" }, 400); - } - - if (!dockerfilePath) { - return c.json({ error: "dockerfilePath is required" }, 400); - } - - if (!environmentId) { - return c.json({ error: "environmentId is required" }, 400); - } - - if (!contextFile) { - return c.json({ error: "context file is required" }, 400); + const body = await c.req.parseBody(); + const parseResult = BuildRequestSchema.safeParse(body); + if (!parseResult.success) { + return c.json( + { error: "Invalid build request format" }, + 400, + ); } + const { + buildName, + dockerfilePath, + environmentId, + buildArgs, + buildTarget, + context: contextFile + } = parseResult.data; // Create the build - const buildId = buildStore.createBuild(buildName, dockerfilePath, environmentId); + const buildId = buildStore.createBuild( + buildName, + dockerfilePath, + environmentId, + buildArgs, + buildTarget + ); const contextPath = buildStore.getContextPath(buildId); if (!contextPath) { @@ -169,6 +172,7 @@ export async function createServer(port: number = 3000) { return c.json({ buildId }); } catch (error) { + console.error("Error processing build request:", error); return c.json({ error: "Failed to process build request" }, 500); } }); diff --git a/cloud/packages/ci-manager/src/types.ts b/cloud/packages/ci-manager/src/types.ts index f66f4b2281..1e5966c722 100644 --- a/cloud/packages/ci-manager/src/types.ts +++ b/cloud/packages/ci-manager/src/types.ts @@ -1,43 +1,83 @@ import { z } from "zod"; +import { NO_SEP_CHAR_REGEX, UNIT_SEP_CHAR } from "./common"; export const StatusSchema = z.discriminatedUnion("type", [ - z.object({ type: z.literal("starting"), data: z.object({}) }), - z.object({ type: z.literal("running"), data: z.object({}) }), - z.object({ type: z.literal("finishing"), data: z.object({}) }), - z.object({ type: z.literal("converting"), data: z.object({}) }), - z.object({ type: z.literal("uploading"), data: z.object({}) }), - z.object({ type: z.literal("failure"), data: z.object({ reason: z.string() }) }), - z.object({ type: z.literal("success"), data: z.object({ buildId: z.string() }) }), + z.object({ type: z.literal("starting"), data: z.object({}) }), + z.object({ type: z.literal("running"), data: z.object({}) }), + z.object({ type: z.literal("finishing"), data: z.object({}) }), + z.object({ type: z.literal("converting"), data: z.object({}) }), + z.object({ type: z.literal("uploading"), data: z.object({}) }), + z.object({ type: z.literal("failure"), data: z.object({ reason: z.string() }) }), + z.object({ type: z.literal("success"), data: z.object({ buildId: z.string() }) }), ]); export type Status = z.infer; +const ILLEGAL_BUILD_ARG_KEY = /[\s'"\\]/g; +const BuildArgsSchema = z.string() + .transform((str) => JSON.parse(str)) + .pipe(z.array(z.string())) + .refine((arr) => { + // Check each key=value pair to ensure keys have no spaces + return arr.every(item => { + const [key] = item.split('='); + if (!key) return false; + if (ILLEGAL_BUILD_ARG_KEY.test(key)) return false; + if (item.includes(UNIT_SEP_CHAR)) return false; + return true; + }); + }, { message: "Argument key/value contains invalid character" }) + .transform((arr) => { + const result: Record = Object.create(null); + // Convert array of strings to an object + for (const item of arr) { + const [key, ...valueParts] = item.split('='); + const value = valueParts.join('='); + + if (key && value !== undefined) { + result[key] = value; + } + } + + return result; + }); + export const BuildRequestSchema = z.object({ - buildName: z.string(), - dockerfilePath: z.string(), - environmentId: z.string(), + buildName: z.string() + .regex(NO_SEP_CHAR_REGEX, "buildName cannot contain special characters"), + dockerfilePath: z.string() + .regex(NO_SEP_CHAR_REGEX, "dockerfilePath cannot contain special characters"), + environmentId: z.string() + .regex(NO_SEP_CHAR_REGEX, "environmentId cannot contain special characters"), + buildArgs: BuildArgsSchema, + buildTarget: z.string() + .regex(NO_SEP_CHAR_REGEX, "buildTarget cannot contain special characters") + .optional(), + context: z.instanceof(File) }); export type BuildRequest = z.infer; export const BuildEventSchema = z.discriminatedUnion("type", [ - z.object({ type: z.literal("status"), data: StatusSchema }), - z.object({ type: z.literal("log"), data: z.object({ line: z.string() }) }), + z.object({ type: z.literal("status"), data: StatusSchema }), + z.object({ type: z.literal("log"), data: z.object({ line: z.string() }) }), ]); export type BuildEvent = z.infer; export interface BuildInfo { - id: string; - status: Status; - buildName?: string; - dockerfilePath?: string; - environmentId?: string; - contextPath?: string; - outputPath?: string; - events: BuildEvent[]; - containerProcess?: any; - createdAt: Date; - downloadedAt?: Date; - cleanupTimeout?: NodeJS.Timeout; + id: string; + status: Status; + buildName: string; + dockerfilePath: string; + environmentId: string; + contextPath: string; + buildArgs: Record; + buildTarget?: string; + outputPath: string; + events: BuildEvent[]; + containerProcess?: any; + createdAt: Date; + downloadedAt?: Date; + cleanupTimeout?: NodeJS.Timeout; } diff --git a/cloud/packages/ci-runner/Dockerfile b/cloud/packages/ci-runner/Dockerfile index bd7a621b00..8bbf748c20 100644 --- a/cloud/packages/ci-runner/Dockerfile +++ b/cloud/packages/ci-runner/Dockerfile @@ -5,5 +5,6 @@ FROM ghcr.io/rivet-gg/executor@sha256:439d4dbb0f3f8c1c6c2195e144d29195b4930b8716 COPY --from=builder /bin/sh /bin/sh COPY --from=builder /lib/ld-musl-x86_64.so.1 /lib/ld-musl-x86_64.so.1 -# HACK: Use env vars to interpolate args bc Rivet doesn't support passing args -ENTRYPOINT ["/bin/sh", "-c", "/kaniko/executor --context=${CONTEXT_URL} --destination=${DESTINATION} --upload-tar=${OUTPUT_URL} --dockerfile=${DOCKERFILE_PATH} --no-push --single-snapshot --verbosity=info"] +COPY entry.sh ~/entry.sh + +ENTRYPOINT ["/bin/sh", "~/entry.sh"] diff --git a/cloud/packages/ci-runner/entry.sh b/cloud/packages/ci-runner/entry.sh new file mode 100644 index 0000000000..7b0d42e192 --- /dev/null +++ b/cloud/packages/ci-runner/entry.sh @@ -0,0 +1,18 @@ +#!/bin/sh + +# Kaniko args are passed as a single string and +# are split by the entrypoint script. + +# Keep in sync with ci-manager/common.ts +UNIT_SEP_CHAR=$'\x1F' + +# We split the string by the unit separator character +# into an array of words ($@), which preserves any +# spaces in the arguments. +export IFS=$UNIT_SEP_CHAR +set -- $KANIKO_ARGS + +# For build args, values containing spaces are not natively supported by +# Kaniko, they recommend setting IFS to null before command. +export IFS='' +/kaniko/executor $@ \ No newline at end of file diff --git a/examples/system-test-actor/rivet.jsonc b/examples/system-test-actor/rivet.jsonc index cc40dad97c..d36dd27efc 100644 --- a/examples/system-test-actor/rivet.jsonc +++ b/examples/system-test-actor/rivet.jsonc @@ -1,8 +1,10 @@ { - "builds": { - // "ws-isolate": { - // "script": "src/isolate/main.ts" - // }, + "actors": { + "ws-isolate": { + "script": "src/isolate/main.ts" + } + }, + "containers": { "ws-container": { "dockerfile": "Dockerfile" } diff --git a/packages/toolchain/toolchain/src/util/docker/build.rs b/packages/toolchain/toolchain/src/util/docker/build.rs index 94d025e422..06823a358e 100644 --- a/packages/toolchain/toolchain/src/util/docker/build.rs +++ b/packages/toolchain/toolchain/src/util/docker/build.rs @@ -17,7 +17,7 @@ pub struct BuildImageOutput { /// Builds an image and archives it to a path. pub async fn build_image( - ctx: &ToolchainCtx, + _ctx: &ToolchainCtx, task: task::TaskCtx, build_path: &Path, dockerfile: &Path, diff --git a/packages/toolchain/toolchain/src/util/docker/build_remote.rs b/packages/toolchain/toolchain/src/util/docker/build_remote.rs index 5171710300..055b399799 100644 --- a/packages/toolchain/toolchain/src/util/docker/build_remote.rs +++ b/packages/toolchain/toolchain/src/util/docker/build_remote.rs @@ -8,7 +8,6 @@ use uuid::Uuid; use crate::{ config::{self}, project::environment::TEMPEnvironment, - tasks::env, toolchain_ctx::ToolchainCtx, util::{docker::push, task}, }; @@ -34,8 +33,8 @@ pub async fn build_remote( environment: TEMPEnvironment, build_path: &Path, dockerfile: &Path, - _build_arg_flags: &Option>, - _build_target: &Option, + build_arg_flags: &Option>, + build_target: &Option, ) -> Result { task.log("[Remote Build] Starting remote build process"); @@ -58,6 +57,8 @@ pub async fn build_remote( &environment, build_path, dockerfile, + build_arg_flags, + build_target, &ci_runner_actor_id, &ci_runner_endpoint, ) @@ -415,6 +416,8 @@ async fn upload_build_context( environment: &TEMPEnvironment, build_path: &Path, dockerfile: &Path, + build_arg_flags: &Option>, + build_target: &Option, _ci_manager_actor_id: &str, ci_manager_endpoint: &str, ) -> Result { @@ -431,10 +434,16 @@ async fn upload_build_context( // Prepare multipart form data task.log("[Remote Build] Uploading build context..."); + // Serialize build args if provided + let serialized_build_args = serde_json::to_string( + build_arg_flags.as_deref().unwrap_or(&[]) + ).context("Failed to serialize build args")?; + // Create FormData-like structure using reqwest let form = reqwest::multipart::Form::new() .text("buildName", build_name) .text("environmentId", environment.slug.to_string()) + .text("buildArgs", serialized_build_args) .text( "dockerfilePath", dockerfile @@ -449,6 +458,12 @@ async fn upload_build_context( .file_name("context.tar.gz") .mime_str("application/gzip")?, ); + + let form = if let Some(target) = build_target { + form.text("buildTarget", target.clone()) + } else { + form + }; // Submit build let encoded_server_url = server_url.replace(":", "%3A").replace("/", "%2F"); @@ -576,7 +591,7 @@ async fn poll_build_status( bail!("timed out polling status") } -async fn get_build_by_tags( +async fn _get_build_by_tags( ctx: &ToolchainCtx, task: task::TaskCtx, environment: &TEMPEnvironment,