diff --git a/package.json b/package.json index 5052ef3..6b101e3 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "@crawlee/fs-storage": "4.0.0-beta.145", "dockerode": "^4.0.5", "express": "^5.1.0", + "json5": "^2.2.3", "tar-stream": "^3.1.7" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4f467fd..f1d9b00 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,6 +23,9 @@ importers: express: specifier: ^5.1.0 version: 5.2.1 + json5: + specifier: ^2.2.3 + version: 2.2.3 tar-stream: specifier: ^3.1.7 version: 3.2.0 diff --git a/requirements/actor-driver.md b/requirements/actor-driver.md index d261fd8..22f0938 100644 --- a/requirements/actor-driver.md +++ b/requirements/actor-driver.md @@ -37,6 +37,12 @@ - Actor build details are saved in `__BUILDS__` internal storage - Actor build log is saved in `__LOGS__` internal storage - Actor details are saved in `__ACTORS__` internal storage +- **The Dockerfile to build is resolved from the Actor's pushed source**, not Docker's implicit default. `.actor/actor.json` is parsed as JSON5; an unparseable file fails the build with a "Could not parse .actor/actor.json" message. Resolution order, stopping at the first hit: + 1. the `dockerfile` field of `.actor/actor.json`, relative to `.actor/` - a path escaping the Actor root fails with "points outside the Actor root directory"; a non-string value fails with `"dockerfile" must be a string`; a value naming no pushed file (including empty) falls through instead of failing. + 2. `.actor/Dockerfile` + 3. `Dockerfile` at the Actor root + 4. the platform's bundled default Dockerfile, for that build only - the pushed source itself is unchanged. + - Matching is case-insensitive, exact-case wins ties, and every outcome is stated in the build log. # Bind mount volumes with Actor source code diff --git a/requirements/test.md b/requirements/test.md index 61c7724..fb85f4c 100644 --- a/requirements/test.md +++ b/requirements/test.md @@ -29,7 +29,8 @@ which covers push/call/log-stream/storage-access against the runtime itself, not what a given Actor's own code does over the network. - Building the sample Actors' images requires pulling their base images - (`apify/actor-node:24`, `apify/actor-python:3.13`) at least once; CI must pre-pull both before + (`apify/actor-node:24`, `apify/actor-python:3.13`, and `python:3.11-slim` for the + `sample_actor_crawler` build case) at least once; CI must pre-pull all three before running the e2e suite so the timing of the actual push/call assertions is not dominated by image pulls. diff --git a/sample_actor_crawler/.actor/Dockerfile b/sample_actor_crawler/.actor/Dockerfile new file mode 100644 index 0000000..0de5fe3 --- /dev/null +++ b/sample_actor_crawler/.actor/Dockerfile @@ -0,0 +1,17 @@ +# Specify the base Docker image. See https://docs.apify.com/sdk/python/docs/overview/getting-started +FROM apify/actor-python:3.13 + +COPY --chown=myuser:myuser requirements.txt ./ + +RUN echo "Python version:" \ + && python --version \ + && echo "Pip version:" \ + && pip --version \ + && echo "Installing dependencies:" \ + && pip install -r requirements.txt \ + && echo "All installed Python packages:" \ + && pip freeze + +COPY --chown=myuser:myuser . ./ + +CMD ["python3", "-m", "src"] diff --git a/sample_actor_crawler/.actor/actor.json b/sample_actor_crawler/.actor/actor.json new file mode 100644 index 0000000..ef21c93 --- /dev/null +++ b/sample_actor_crawler/.actor/actor.json @@ -0,0 +1,8 @@ +{ + "actorSpecification": 1, + "name": "sample-actor-crawler", + "version": "0.0", + "buildTag": "latest", + "dockerfile": "./Dockerfile", + "input": "./input_schema.json" +} diff --git a/sample_actor_crawler/.actor/input_schema.json b/sample_actor_crawler/.actor/input_schema.json new file mode 100644 index 0000000..c8fcfd2 --- /dev/null +++ b/sample_actor_crawler/.actor/input_schema.json @@ -0,0 +1,22 @@ +{ + "title": "Sample Actor Crawler input", + "type": "object", + "schemaVersion": 1, + "properties": { + "startUrl": { + "title": "Start URL", + "type": "string", + "description": "The page the crawler starts from. Same-domain links found on it (and on the pages it discovers) are enqueued too, up to the crawl limit.", + "editor": "textfield", + "default": "https://crawlee.dev" + }, + "proxyConfiguration": { + "title": "Proxy configuration", + "type": "object", + "description": "Apify Proxy configuration used for every request. The only way to crawl direct with no proxy is an explicit `{\"useApifyProxy\": false}` -- omitting this field entirely is NOT equivalent; it falls through to the SDK's default proxy configuration (same as `useApifyProxy: true`) and requires a valid APIFY_PROXY_PASSWORD.", + "editor": "proxy", + "default": { "useApifyProxy": true, "apifyProxyGroups": ["RESIDENTIAL"] } + } + }, + "required": ["startUrl"] +} diff --git a/sample_actor_crawler/requirements.txt b/sample_actor_crawler/requirements.txt new file mode 100644 index 0000000..baaa54e --- /dev/null +++ b/sample_actor_crawler/requirements.txt @@ -0,0 +1,2 @@ +apify +crawlee[parsel] diff --git a/sample_actor_crawler/src/__init__.py b/sample_actor_crawler/src/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/sample_actor_crawler/src/__main__.py b/sample_actor_crawler/src/__main__.py new file mode 100644 index 0000000..8c4ab0b --- /dev/null +++ b/sample_actor_crawler/src/__main__.py @@ -0,0 +1,6 @@ +import asyncio + +from .main import main + +if __name__ == '__main__': + asyncio.run(main()) diff --git a/sample_actor_crawler/src/main.py b/sample_actor_crawler/src/main.py new file mode 100644 index 0000000..157b62e --- /dev/null +++ b/sample_actor_crawler/src/main.py @@ -0,0 +1,41 @@ +"""Sample Actor demonstrating a Parsel-based crawl through Apify Proxy.""" + +from crawlee.crawlers import ParselCrawler, ParselCrawlingContext +from crawlee.router import Router + +from apify import Actor + +router = Router[ParselCrawlingContext]() + + +@router.default_handler +async def request_handler(context: ParselCrawlingContext) -> None: + Actor.log.info(f"Scraping {context.request.url} ...") + + data = { + "url": context.request.url, + "title": context.selector.xpath("//title/text()").get(), + "headings": context.selector.xpath("//h1/text() | //h2/text() | //h3/text()").getall(), + } + await context.push_data(data) + + await context.enqueue_links(strategy="same-domain") + + +async def main() -> None: + async with Actor: + actor_input = await Actor.get_input() or {} + start_url = actor_input.get("startUrl", "https://crawlee.dev") + + proxy_configuration = await Actor.create_proxy_configuration( + actor_proxy_input=actor_input.get("proxyConfiguration") + ) + + crawler = ParselCrawler( + proxy_configuration=proxy_configuration, + request_handler=router, + # Crawl limit: 10 pages total, seed URL counted. + max_requests_per_crawl=10, + ) + + await crawler.run([start_url]) diff --git a/src/driver/docker-driver.ts b/src/driver/docker-driver.ts index d63f54b..d9d5eb9 100644 --- a/src/driver/docker-driver.ts +++ b/src/driver/docker-driver.ts @@ -31,6 +31,7 @@ import Docker from 'dockerode'; import * as tar from 'tar-stream'; import { CONTAINER_API_ALIAS } from '../config.js'; +import { normalizeEntryName } from './tar-entry-name.js'; import type { SourceFile } from '../storage/entities.js'; import { DriverTimedOutError, @@ -111,7 +112,7 @@ function buildTarball(sourceFiles: SourceFile[]): NodeJS.ReadableStream { const pack = tar.pack(); for (const file of sourceFiles) { const buffer = sourceFileToBuffer(file); - pack.entry({ name: file.name }, buffer); + pack.entry({ name: normalizeEntryName(file.name) }, buffer); } pack.finalize(); return pack; @@ -231,6 +232,7 @@ export class DockerDriver implements Driver { stream = await this.docker.buildImage(tarball, { t: imageTag, nocache: !ctx.useCache, + dockerfile: ctx.dockerfilePath, abortSignal: controller.signal, }); } catch (error) { diff --git a/src/driver/tar-entry-name.ts b/src/driver/tar-entry-name.ts new file mode 100644 index 0000000..4233903 --- /dev/null +++ b/src/driver/tar-entry-name.ts @@ -0,0 +1,9 @@ +import * as path from 'node:path'; + +/** Canonicalizes a source-file name to a tar entry name: POSIX separators, no leading `./`, then + * `path.posix.normalize` (a leading `../` is left alone). */ +export function normalizeEntryName(name: string): string { + const posixName = name.replace(/\\/g, '/'); + const withoutLeadingDotSlash = posixName.replace(/^(?:\.\/)+/, ''); + return path.posix.normalize(withoutLeadingDotSlash); +} diff --git a/src/driver/types.ts b/src/driver/types.ts index bef4aca..5738d42 100644 --- a/src/driver/types.ts +++ b/src/driver/types.ts @@ -6,6 +6,8 @@ export interface BuildContext { sourceFiles: SourceFile[]; useCache: boolean; timeoutSecs: number; + /** Tar-relative path to the Dockerfile to build, passed as dockerode's `dockerfile` build option. */ + dockerfilePath: string; } /** Host folder + image working directory, carried together so "both or neither" is enforced by the diff --git a/src/services/builds.ts b/src/services/builds.ts index 4ee5d53..1f93a77 100644 --- a/src/services/builds.ts +++ b/src/services/builds.ts @@ -1,10 +1,11 @@ import { generateId } from '../storage/ids.js'; -import type { BuildRecord, JobStatus } from '../storage/entities.js'; +import type { BuildRecord, JobStatus, SourceFile } from '../storage/entities.js'; import { getRegistries } from '../storage/registries.js'; import type { ActorRecord, ActorVersionRecord } from '../storage/entities.js'; import { recordTaggedBuild, updateActor } from './actors.js'; import type { Driver } from '../driver/types.js'; import { DriverTimedOutError } from '../driver/types.js'; +import { resolveDockerfileLocation } from './dockerfile-location.js'; import { appendLog, flushLog, markLogTerminal } from './logs.js'; import { isTerminalJobStatus, transitionJobStatus } from './job-status.js'; @@ -180,14 +181,32 @@ export async function runBuildInBackground( return; } + const dockerfileResolution = resolveDockerfileLocation(version.sourceFiles); + if (dockerfileResolution.outcome === 'failure') { + appendLog(record.id, `${dockerfileResolution.message}\n`); + await flushLog(record.id); + markLogTerminal(record.id); + await transitionJobStatus(builds, record.id, 'FAILED', { + finishedAt: new Date().toISOString(), + statusMessage: dockerfileResolution.message, + }); + return; + } + for (const line of dockerfileResolution.logLines) appendLog(record.id, line); + const sourceFiles: SourceFile[] = + dockerfileResolution.outcome === 'default' + ? [...version.sourceFiles, dockerfileResolution.extraSourceFile] + : version.sourceFiles; + try { const outcome = await driver.startBuild( { buildId: record.id, actorName: actor.name, - sourceFiles: version.sourceFiles, + sourceFiles, useCache: options.useCache, timeoutSecs: DEFAULT_BUILD_TIMEOUT_SECS, + dockerfilePath: dockerfileResolution.dockerfilePath, }, (chunk) => appendLog(record.id, chunk), ); diff --git a/src/services/default-dockerfile.ts b/src/services/default-dockerfile.ts new file mode 100644 index 0000000..57712e3 --- /dev/null +++ b/src/services/default-dockerfile.ts @@ -0,0 +1,15 @@ +/** Basename for the resolver's Dockerfile candidates and the bundled default's own tar-entry name. */ +export const DEFAULT_DOCKERFILE_NAME = 'Dockerfile'; + +/** Injected when an Actor's pushed source names no Dockerfile. Matches the Apify platform's default Dockerfile. */ +export const DEFAULT_DOCKERFILE_CONTENT = `# This is a default Dockerfile is used for Actors that don't have a Dockerfile. +FROM apify/actor-node:20 + +# Copy all files and directories from the directory to the Docker image +COPY . ./ + +# Install NPM packages, skip optional and development dependencies to keep the image small, +# avoid logging to much and show log the dependency tree +RUN npm install --quiet --only=prod --no-optional \\ + && (npm list || true) +`; diff --git a/src/services/dockerfile-location.ts b/src/services/dockerfile-location.ts new file mode 100644 index 0000000..da7ea76 --- /dev/null +++ b/src/services/dockerfile-location.ts @@ -0,0 +1,158 @@ +/** + * Resolves which Dockerfile a build should use from the version's `sourceFiles`. Candidate order, + * stopping at the first hit: 1) the `dockerfile` field of `.actor/actor.json`, relative to `.actor` 2) + * `.actor/Dockerfile` 3) `Dockerfile` at the Actor root 4) the bundled default. Matching is + * case-insensitive; the returned path is always the matched file's own name, never the candidate's + * casing - Docker's tar lookup is case-sensitive. An exact-case match wins over a case-differing one. + */ +import * as path from 'node:path'; +import JSON5 from 'json5'; + +import { normalizeEntryName } from '../driver/tar-entry-name.js'; +import type { SourceFile } from '../storage/entities.js'; +import { DEFAULT_DOCKERFILE_CONTENT, DEFAULT_DOCKERFILE_NAME } from './default-dockerfile.js'; + +const ACTOR_DIR = '.actor'; +const ACTOR_JSON_NAME = `${ACTOR_DIR}/actor.json`; + +/** Why Dockerfile resolution failed. */ +export type DockerfileResolutionFailureReason = + 'escapes-actor-root' | 'invalid-dockerfile-field' | 'unparseable-actor-json'; + +/** `resolveDockerfileLocation`'s outcomes: `resolved` (a candidate matched), `default` (nothing matched + * - `extraSourceFile` must be appended to this build's context only, never persisted), or `failure`. */ +export type DockerfileResolution = + | { outcome: 'resolved'; dockerfilePath: string; logLines: string[] } + | { outcome: 'default'; dockerfilePath: string; logLines: string[]; extraSourceFile: SourceFile } + | { outcome: 'failure'; reason: DockerfileResolutionFailureReason; message: string }; + +function sourceFileToText(file: SourceFile): string { + return file.format === 'BASE64' ? Buffer.from(file.content, 'base64').toString('utf8') : file.content; +} + +interface IndexedFile { + normalizedName: string; + lowerName: string; +} + +function indexSourceFiles(sourceFiles: SourceFile[]): IndexedFile[] { + return sourceFiles.map((file) => { + const normalizedName = normalizeEntryName(file.name); + return { normalizedName, lowerName: normalizedName.toLowerCase() }; + }); +} + +/** Exact-case match wins; otherwise the first match in `sourceFiles` order. */ +function findCaseInsensitive(indexed: IndexedFile[], candidate: string): IndexedFile | undefined { + const lowerCandidate = candidate.toLowerCase(); + let firstMatch: IndexedFile | undefined; + for (const file of indexed) { + if (file.lowerName !== lowerCandidate) continue; + if (file.normalizedName === candidate) return file; // exact case always wins immediately + firstMatch ??= file; + } + return firstMatch; +} + +/** `.actor/actor.json`'s own path is not case-folded, unlike the Dockerfile candidates. */ +function findExact(sourceFiles: SourceFile[], normalizedTarget: string): SourceFile | undefined { + return sourceFiles.find((file) => normalizeEntryName(file.name) === normalizedTarget); +} + +function escapesActorRootFailure(rawField: string): DockerfileResolution { + return { + outcome: 'failure', + reason: 'escapes-actor-root', + message: `Dockerfile path "${rawField}" in .actor/actor.json points outside the Actor root directory.`, + }; +} + +export function resolveDockerfileLocation(sourceFiles: SourceFile[]): DockerfileResolution { + const indexed = indexSourceFiles(sourceFiles); + const logLines: string[] = []; + + const actorJsonFile = findExact(sourceFiles, ACTOR_JSON_NAME); + let actorSpecification: unknown; + if (actorJsonFile) { + try { + actorSpecification = JSON5.parse(sourceFileToText(actorJsonFile)); + } catch (error) { + return { + outcome: 'failure', + reason: 'unparseable-actor-json', + message: `Could not parse .actor/actor.json: ${(error as Error).message}`, + }; + } + } + + if (actorSpecification !== null && typeof actorSpecification === 'object' && 'dockerfile' in actorSpecification) { + const field: unknown = actorSpecification.dockerfile; + if (typeof field !== 'string') { + return { + outcome: 'failure', + reason: 'invalid-dockerfile-field', + message: '.actor/actor.json has invalid format: "dockerfile" must be a string.', + }; + } + + if (field === '') { + logLines.push( + 'Warning: "" (from the "dockerfile" field in .actor/actor.json) is not in the pushed source; falling back to the default locations.\n', + ); + } else if (field.startsWith('/')) { + return escapesActorRootFailure(field); + } else { + const joined = normalizeEntryName(path.posix.join(ACTOR_DIR, field)); + if (joined === '..' || joined.startsWith('../')) { + return escapesActorRootFailure(field); + } + + const match = findCaseInsensitive(indexed, joined); + if (match) { + return { + outcome: 'resolved', + dockerfilePath: match.normalizedName, + logLines: [ + ...logLines, + `Using Dockerfile "${match.normalizedName}" (from the "dockerfile" field in .actor/actor.json).\n`, + ], + }; + } + + logLines.push( + `Warning: "${joined}" (from the "dockerfile" field in .actor/actor.json) is not in the pushed source; falling back to the default locations.\n`, + ); + } + } + + const actorDirCandidate = normalizeEntryName(`${ACTOR_DIR}/${DEFAULT_DOCKERFILE_NAME}`); + const actorDirMatch = findCaseInsensitive(indexed, actorDirCandidate); + if (actorDirMatch) { + return { + outcome: 'resolved', + dockerfilePath: actorDirMatch.normalizedName, + logLines: [ + ...logLines, + `Using Dockerfile "${actorDirMatch.normalizedName}" (found at .actor/Dockerfile).\n`, + ], + }; + } + + const rootCandidate = normalizeEntryName(DEFAULT_DOCKERFILE_NAME); + const rootMatch = findCaseInsensitive(indexed, rootCandidate); + if (rootMatch) { + return { + outcome: 'resolved', + dockerfilePath: rootMatch.normalizedName, + logLines: [...logLines, `Using Dockerfile "${rootMatch.normalizedName}" (found at the Actor root).\n`], + }; + } + + logLines.push(`${DEFAULT_DOCKERFILE_NAME} not found, using the default one.\n`); + return { + outcome: 'default', + dockerfilePath: DEFAULT_DOCKERFILE_NAME, + logLines, + extraSourceFile: { name: DEFAULT_DOCKERFILE_NAME, format: 'TEXT', content: DEFAULT_DOCKERFILE_CONTENT }, + }; +} diff --git a/test/e2e/actor-dev-loop.test.ts b/test/e2e/actor-dev-loop.test.ts index 783ef8d..2675e09 100644 --- a/test/e2e/actor-dev-loop.test.ts +++ b/test/e2e/actor-dev-loop.test.ts @@ -106,6 +106,21 @@ describe('full Actor dev loop via apify-cli (requires Docker)', () => { ); } + it( + 'sample_actor_crawler: push -> build succeeds (build-only - its Dockerfile lives at .actor/Dockerfile, ' + + 'the layout that used to fail with a daemon-side "Cannot locate specified Dockerfile" error)', + () => { + const env = apifyEnv(isolatedApifyHome); + const actorDir = join(REPO_ROOT, 'sample_actor_crawler'); + + const pushOutput = apify(['push', '--json'], { cwd: actorDir, env }); + const push = JSON.parse(pushOutput) as PushResult; + + expect(push.build.status).toBe('SUCCEEDED'); + }, + 5 * 60 * 1000, + ); + it('the run log contains the crawler per-page lines', () => { const env = apifyEnv(isolatedApifyHome); const callOutput = apify(['call', '--input', JSON.stringify({ maxPages: 1 }), '--json'], { diff --git a/test/e2e/helpers/docker.ts b/test/e2e/helpers/docker.ts index 70235ea..7626c0f 100644 --- a/test/e2e/helpers/docker.ts +++ b/test/e2e/helpers/docker.ts @@ -18,7 +18,7 @@ export function pullBaseImages(): void { // Pre-pulled here rather than left to the first build, per `test.md`'s documented CI requirement - // building an Actor image is the one step that still needs network, and doing it once up front // keeps the timing of the actual push/call assertions predictable. - for (const image of ['apify/actor-node:24', 'apify/actor-python:3.13']) { + for (const image of ['apify/actor-node:24', 'apify/actor-python:3.13', 'python:3.11-slim']) { execFileSync('docker', ['pull', image], { stdio: 'inherit' }); } } diff --git a/test/integration/helpers/test-server.ts b/test/integration/helpers/test-server.ts index d35b157..0ba8353 100644 --- a/test/integration/helpers/test-server.ts +++ b/test/integration/helpers/test-server.ts @@ -12,7 +12,7 @@ import { resetUsersForTests } from '../../../src/services/users.js'; import { resetApiFallbackStateForTests } from '../../../src/services/api-fallback.js'; import { createApiServer } from '../../../src/api/server.js'; import { resetLogsForTests, stopLogFlusher } from '../../../src/services/logs.js'; -import type { BuildOutcome, Driver, RunOutcome } from '../../../src/driver/types.js'; +import type { BuildContext, BuildOutcome, Driver, RunOutcome } from '../../../src/driver/types.js'; /** A driver that is always unavailable, so build/run creation fails fast and deterministically. */ export function unavailableDriver(): Driver { @@ -65,14 +65,19 @@ export function fixedRunOutcomeDriver(outcome: RunOutcome): Driver { }; } -/** Same idea as `fixedRunOutcomeDriver`, but for builds: `startBuild` either resolves with `outcome` or - * rejects with `error`, whichever the caller supplies (`error` wins if both are given, so a - * `DriverTimedOutError` can be asserted straight through to a `TIMED-OUT` status). */ -export function fixedBuildOutcomeDriver(outcome: BuildOutcome, error?: Error): Driver { +/** Same idea as `fixedRunOutcomeDriver`, but for builds; also records every `startBuild` ctx into + * `startBuildContexts`. */ +export function fixedBuildOutcomeDriver( + outcome: BuildOutcome, + error?: Error, +): Driver & { startBuildContexts: BuildContext[] } { + const startBuildContexts: BuildContext[] = []; return { available: true, + startBuildContexts, async init() {}, - async startBuild() { + async startBuild(ctx) { + startBuildContexts.push(ctx); if (error) throw error; return outcome; }, diff --git a/test/integration/job-lifecycle.test.ts b/test/integration/job-lifecycle.test.ts index 2350284..ed0f8a2 100644 --- a/test/integration/job-lifecycle.test.ts +++ b/test/integration/job-lifecycle.test.ts @@ -39,7 +39,14 @@ import { waitForRunFinish, } from '../../src/services/runs.js'; import { DriverTimedOutError, type Driver } from '../../src/driver/types.js'; -import type { ActorRecord, ActorVersionRecord, BuildRecord, RunRecord } from '../../src/storage/entities.js'; +import type { + ActorRecord, + ActorVersionRecord, + BuildRecord, + RunRecord, + SourceFile, +} from '../../src/storage/entities.js'; +import { DEFAULT_DOCKERFILE_CONTENT, DEFAULT_DOCKERFILE_NAME } from '../../src/services/default-dockerfile.js'; /** Creates an Actor via the real client (so it has a genuine owner) and returns the underlying * `ActorRecord` for direct service-layer calls. */ @@ -91,8 +98,7 @@ const VERSION: ActorVersionRecord = { sourceFiles: [], }; -/** Fails the test immediately (rather than hanging) if the driver is ever asked to start a run/build - - * used to assert the pre-start abort window really does prevent a container/build from ever starting. */ +/** Fails the test immediately if the driver is ever asked to start a run/build. */ function neverStartDriver(): Driver & { abortRunCalls: string[]; abortBuildCalls: string[] } { const abortRunCalls: string[] = []; const abortBuildCalls: string[] = []; @@ -102,7 +108,9 @@ function neverStartDriver(): Driver & { abortRunCalls: string[]; abortBuildCalls abortBuildCalls, async init() {}, async startBuild() { - throw new Error('startBuild must never be called once the record is already ABORTING'); + throw new Error( + 'startBuild must never be called once the build has already been finalised - either the record was already ABORTING, or resolveDockerfileLocation returned a "failure" outcome', + ); }, async abortBuild(buildId) { abortBuildCalls.push(buildId); @@ -320,6 +328,114 @@ describe('job lifecycle: TIMED-OUT mapping and abort/completion race guards', () }); }); + describe("Dockerfile resolution wiring: runBuildInBackground acts on resolveDockerfileLocation's outcome", () => { + it('a "failure" outcome marks the build FAILED with the resolver\'s message as statusMessage, and never calls driver.startBuild', async () => { + const driver = neverStartDriver(); + server = await startTestServer(driver); + const actor = await seedActor(server, 'dockerfile-failure-actor'); + + const escapingSourceFiles: SourceFile[] = [ + { + name: '.actor/actor.json', + format: 'TEXT', + content: JSON.stringify({ dockerfile: '../../evil/Dockerfile' }), + }, + ]; + const version: ActorVersionRecord = { ...VERSION, sourceFiles: escapingSourceFiles }; + + const record: BuildRecord = { + id: generateId(), + userId: actor.userId, + actorId: actor.id, + versionNumber: '0.0', + buildNumber: '0.0.1', + tag: 'latest', + status: 'READY', + startedAt: new Date().toISOString(), + }; + await getRegistries().builds.set(record.id, record); + + await runBuildInBackground(driver, actor, version, record, { tag: 'latest', useCache: true }); + + const final = await getRegistries().builds.get(record.id); + expect(final?.status).toBe('FAILED'); + expect(final?.statusMessage).toBe( + 'Dockerfile path "../../evil/Dockerfile" in .actor/actor.json points outside the Actor root directory.', + ); + }); + + it('a "default" outcome appends the bundled default Dockerfile to the driver\'s ctx.sourceFiles and sets ctx.dockerfilePath to "Dockerfile"', async () => { + const driver = fixedBuildOutcomeDriver({ imageId: 'x' }); + server = await startTestServer(driver); + const actor = await seedActor(server, 'dockerfile-default-actor'); + + const noDockerfileSourceFiles: SourceFile[] = [ + { name: 'main.js', format: 'TEXT', content: 'console.log(1);\n' }, + ]; + const version: ActorVersionRecord = { ...VERSION, sourceFiles: noDockerfileSourceFiles }; + + const record: BuildRecord = { + id: generateId(), + userId: actor.userId, + actorId: actor.id, + versionNumber: '0.0', + buildNumber: '0.0.1', + tag: 'latest', + status: 'READY', + startedAt: new Date().toISOString(), + }; + await getRegistries().builds.set(record.id, record); + + await runBuildInBackground(driver, actor, version, record, { tag: 'latest', useCache: true }); + + expect(driver.startBuildContexts).toHaveLength(1); + const ctx = driver.startBuildContexts[0]!; + expect(ctx.dockerfilePath).toBe(DEFAULT_DOCKERFILE_NAME); + expect(ctx.sourceFiles).toEqual([ + ...noDockerfileSourceFiles, + { name: 'Dockerfile', format: 'TEXT', content: DEFAULT_DOCKERFILE_CONTENT }, + ]); + expect(version.sourceFiles).toEqual(noDockerfileSourceFiles); + + const final = await getRegistries().builds.get(record.id); + expect(final?.status).toBe('SUCCEEDED'); + }); + + it('a "resolved" outcome passes the resolved path through to ctx.dockerfilePath, with sourceFiles unchanged', async () => { + const driver = fixedBuildOutcomeDriver({ imageId: 'x' }); + server = await startTestServer(driver); + const actor = await seedActor(server, 'dockerfile-resolved-actor'); + + const resolvedSourceFiles: SourceFile[] = [ + { name: '.actor/Dockerfile', format: 'TEXT', content: 'FROM node:20\n' }, + { name: 'main.js', format: 'TEXT', content: 'console.log(1);\n' }, + ]; + const version: ActorVersionRecord = { ...VERSION, sourceFiles: resolvedSourceFiles }; + + const record: BuildRecord = { + id: generateId(), + userId: actor.userId, + actorId: actor.id, + versionNumber: '0.0', + buildNumber: '0.0.1', + tag: 'latest', + status: 'READY', + startedAt: new Date().toISOString(), + }; + await getRegistries().builds.set(record.id, record); + + await runBuildInBackground(driver, actor, version, record, { tag: 'latest', useCache: true }); + + expect(driver.startBuildContexts).toHaveLength(1); + const ctx = driver.startBuildContexts[0]!; + expect(ctx.dockerfilePath).toBe('.actor/Dockerfile'); + expect(ctx.sourceFiles).toEqual(resolvedSourceFiles); + + const final = await getRegistries().builds.get(record.id); + expect(final?.status).toBe('SUCCEEDED'); + }); + }); + describe('imageWorkingDirectory is build-specific, not Actor-specific (human directive: "the workdir should be build specific, not actor specific")', () => { it('a successful build outcome carrying imageWorkingDirectory lands it on that BUILD record, and the Actor record has no such field at all', async () => { const driver = fixedBuildOutcomeDriver({ imageId: 'x', imageWorkingDirectory: '/usr/src/app' }); diff --git a/test/unit/docker-driver.test.ts b/test/unit/docker-driver.test.ts index bbaa040..d6e5633 100644 --- a/test/unit/docker-driver.test.ts +++ b/test/unit/docker-driver.test.ts @@ -446,7 +446,14 @@ describe('DockerDriver.startBuild - imageWorkingDirectory capture (actor-driver. driver.available = true; const outcome = await driver.startBuild( - { buildId: 'build-1', actorName: 'my-actor', sourceFiles: [], useCache: true, timeoutSecs: 60 }, + { + buildId: 'build-1', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: 'Dockerfile', + }, () => {}, ); @@ -463,7 +470,14 @@ describe('DockerDriver.startBuild - imageWorkingDirectory capture (actor-driver. const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); const outcome = await driver.startBuild( - { buildId: 'build-2', actorName: 'my-actor', sourceFiles: [], useCache: true, timeoutSecs: 60 }, + { + buildId: 'build-2', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: 'Dockerfile', + }, () => {}, ); @@ -480,7 +494,14 @@ describe('DockerDriver.startBuild - imageWorkingDirectory capture (actor-driver. driver.available = true; const outcome = await driver.startBuild( - { buildId: 'build-3', actorName: 'my-actor', sourceFiles: [], useCache: true, timeoutSecs: 60 }, + { + buildId: 'build-3', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: 'Dockerfile', + }, () => {}, ); @@ -493,7 +514,14 @@ describe('DockerDriver.startBuild - imageWorkingDirectory capture (actor-driver. driver.available = true; const outcome = await driver.startBuild( - { buildId: 'build-4', actorName: 'my-actor', sourceFiles: [], useCache: true, timeoutSecs: 60 }, + { + buildId: 'build-4', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: 'Dockerfile', + }, () => {}, ); @@ -501,6 +529,100 @@ describe('DockerDriver.startBuild - imageWorkingDirectory capture (actor-driver. }); }); +describe('DockerDriver.startBuild - dockerfile option (the resolved path is handed to dockerode as its `dockerfile` build option)', () => { + function stubDockerCapturingBuildImageOptions() { + const followProgress = vi.fn( + ( + _stream: NodeJS.ReadableStream, + onFinished: (err: Error | null, res: Array<{ error?: string }>) => void, + ) => { + onFinished(null, []); + }, + ); + const buildImage = vi.fn(async () => new PassThrough()); + const getImage = vi.fn(() => ({ inspect: async () => ({ Config: { WorkingDir: '' } }) })); + const docker = { buildImage, modem: { followProgress }, getImage } as unknown as Docker; + return { docker, buildImage }; + } + + it('passes ctx.dockerfilePath through verbatim as buildImage\'s "dockerfile" option', async () => { + const stub = stubDockerCapturingBuildImageOptions(); + const driver = new DockerDriver(stub.docker); + driver.available = true; + + await driver.startBuild( + { + buildId: 'build-dockerfile-option', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: '.actor/Dockerfile', + }, + () => {}, + ); + + expect(stub.buildImage).toHaveBeenCalledTimes(1); + const [, options] = stub.buildImage.mock.calls[0]!; + expect(options).toMatchObject({ dockerfile: '.actor/Dockerfile' }); + }); + + it('always sets the "dockerfile" option, even for the plain root-"Dockerfile" case that coincides with Docker\'s own implicit default', async () => { + const stub = stubDockerCapturingBuildImageOptions(); + const driver = new DockerDriver(stub.docker); + driver.available = true; + + await driver.startBuild( + { + buildId: 'build-dockerfile-option-2', + actorName: 'my-actor', + sourceFiles: [], + useCache: true, + timeoutSecs: 60, + dockerfilePath: 'Dockerfile', + }, + () => {}, + ); + + const [, options] = stub.buildImage.mock.calls[0]!; + expect(options).toMatchObject({ dockerfile: 'Dockerfile' }); + }); + + it('normalizes tar entry names (leading "./" stripped) so a resolved dockerfilePath always names an entry that actually exists in the tar', async () => { + const stub = stubDockerCapturingBuildImageOptions(); + const driver = new DockerDriver(stub.docker); + driver.available = true; + + await driver.startBuild( + { + buildId: 'build-dockerfile-option-3', + actorName: 'my-actor', + sourceFiles: [{ name: './.actor/Dockerfile', format: 'TEXT', content: 'FROM node:20\n' }], + useCache: true, + timeoutSecs: 60, + dockerfilePath: '.actor/Dockerfile', + }, + () => {}, + ); + + const [tarball] = stub.buildImage.mock.calls[0]!; + const extract = tar.extract(); + const entryNames: string[] = []; + await new Promise((resolve, reject) => { + extract.on('entry', (header, entryStream, next) => { + entryNames.push(header.name); + entryStream.resume(); + next(); + }); + extract.on('finish', resolve); + extract.on('error', reject); + (tarball as NodeJS.ReadableStream).pipe(extract); + }); + + expect(entryNames).toEqual(['.actor/Dockerfile']); + }); +}); + describe('DockerDriver.ensureProbeImage (actor-driver.md: registration needs no build of its own)', () => { /** A stub covering only what `ensureProbeImage` calls: `buildImage` and `modem.followProgress` * (invoking its `onFinished` callback synchronously, as a successful build with no progress lines). */ diff --git a/test/unit/dockerfile-location.test.ts b/test/unit/dockerfile-location.test.ts new file mode 100644 index 0000000..8ac7ea3 --- /dev/null +++ b/test/unit/dockerfile-location.test.ts @@ -0,0 +1,297 @@ +import { describe, expect, it } from 'vitest'; + +import { resolveDockerfileLocation } from '../../src/services/dockerfile-location.js'; +import { DEFAULT_DOCKERFILE_CONTENT, DEFAULT_DOCKERFILE_NAME } from '../../src/services/default-dockerfile.js'; +import type { SourceFile } from '../../src/storage/entities.js'; + +/** A `TEXT`-format `SourceFile`, the common case in every table row below. */ +function text(name: string, content: string): SourceFile { + return { name, format: 'TEXT', content }; +} + +/** `.actor/actor.json` as a `SourceFile`, given the object to serialize (or a raw string, for the + * JSON5-tolerance and unparseable-content cases below). */ +function actorJson(spec: Record | string): SourceFile { + return text('.actor/actor.json', typeof spec === 'string' ? spec : JSON.stringify(spec)); +} + +describe('resolveDockerfileLocation', () => { + it('A: resolves the "dockerfile" field to .actor/Dockerfile (sample_actor_crawler layout)', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: './Dockerfile' }), + text('.actor/Dockerfile', 'FROM python:3.11-slim\n'), + text('.actor/input_schema.json', '{}'), + text('main.py', 'print(1)\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + expect(result.logLines).toEqual([ + 'Using Dockerfile ".actor/Dockerfile" (from the "dockerfile" field in .actor/actor.json).\n', + ]); + }); + + it('B: resolves "../Dockerfile" to the root Dockerfile (sample_actor_ts/py layout) - byte-identical to today\'s implicit default', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: '../Dockerfile' }), + text('Dockerfile', 'FROM node:20\n'), + text('main.ts', 'console.log(1);\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('Dockerfile'); + }); + + it('C: warns and falls through to .actor/Dockerfile when the "dockerfile" field names no file', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: './Custom.Dockerfile' }), + text('.actor/Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + expect(result.logLines).toEqual([ + 'Warning: ".actor/Custom.Dockerfile" (from the "dockerfile" field in .actor/actor.json) is not in the pushed source; falling back to the default locations.\n', + 'Using Dockerfile ".actor/Dockerfile" (found at .actor/Dockerfile).\n', + ]); + }); + + describe('D: an escaping "dockerfile" field fails the build before any daemon call', () => { + it('a relative path with enough ".." segments to escape .actor/', () => { + const result = resolveDockerfileLocation([actorJson({ dockerfile: '../../evil/Dockerfile' })]); + + expect(result).toEqual({ + outcome: 'failure', + reason: 'escapes-actor-root', + message: + 'Dockerfile path "../../evil/Dockerfile" in .actor/actor.json points outside the Actor root directory.', + }); + }); + + it('an absolute path (leading "/")', () => { + const result = resolveDockerfileLocation([actorJson({ dockerfile: '/etc/passwd' })]); + + expect(result).toEqual({ + outcome: 'failure', + reason: 'escapes-actor-root', + message: 'Dockerfile path "/etc/passwd" in .actor/actor.json points outside the Actor root directory.', + }); + }); + + it('a path that normalizes to exactly ".." (the joined === ".." disjunct, not just startsWith("../"))', () => { + const result = resolveDockerfileLocation([actorJson({ dockerfile: '../..' })]); + + expect(result).toEqual({ + outcome: 'failure', + reason: 'escapes-actor-root', + message: 'Dockerfile path "../.." in .actor/actor.json points outside the Actor root directory.', + }); + }); + + it('a path that stays inside .actor/ (one level of ".." exactly cancels the join) is not treated as escaping', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: '../Dockerfile' }), + text('Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + }); + }); + + describe('E: case-insensitive matching', () => { + it('matches .actor/Dockerfile against a lowercase .actor/dockerfile source file, returning ITS OWN spelling', () => { + const result = resolveDockerfileLocation([text('.actor/dockerfile', 'FROM node:20\n')]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/dockerfile'); + }); + + it('an exact-case match wins even when a case-differing match appears earlier in sourceFiles order', () => { + const result = resolveDockerfileLocation([ + text('dockerfile', 'wrong case, listed first'), + text('Dockerfile', 'exact case, listed second - must still win'), + ]); + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('Dockerfile'); + }); + + it('with no exact-case match, the first case-differing match in sourceFiles order wins', () => { + const result = resolveDockerfileLocation([ + text('DOCKERFILE', 'all caps, listed first'), + text('dockerfile', 'all lowercase, listed second'), + ]); + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('DOCKERFILE'); + }); + }); + + describe('F: nothing resolves - the bundled default is injected', () => { + it('injects the bundled default Dockerfile, verbatim, as an extra Dockerfile-named SourceFile', () => { + const result = resolveDockerfileLocation([ + actorJson({ name: 'no-dockerfile-actor' }), + text('main.py', 'print(1)\n'), + ]); + + expect(result.outcome).toBe('default'); + if (result.outcome !== 'default') return; + expect(result.dockerfilePath).toBe(DEFAULT_DOCKERFILE_NAME); + expect(result.extraSourceFile).toEqual({ + name: 'Dockerfile', + format: 'TEXT', + content: DEFAULT_DOCKERFILE_CONTENT, + }); + expect(result.logLines).toEqual(['Dockerfile not found, using the default one.\n']); + }); + + it('also injects the default when there is no .actor/actor.json at all', () => { + const result = resolveDockerfileLocation([text('main.py', 'print(1)\n')]); + + expect(result.outcome).toBe('default'); + }); + + it('is reached only after both .actor/Dockerfile and a root Dockerfile have failed to match, in any case', () => { + const result = resolveDockerfileLocation([ + text('main.py', 'print(1)\n'), + text('README.md', '# not a Dockerfile\n'), + ]); + + expect(result.outcome).toBe('default'); + }); + }); + + describe('G: malformed "dockerfile" field', () => { + it('a non-string value (e.g. true) fails the build immediately, before any candidate is tried', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: true }), + text('Dockerfile', 'FROM node:20\n'), + ]); + + expect(result).toEqual({ + outcome: 'failure', + reason: 'invalid-dockerfile-field', + message: '.actor/actor.json has invalid format: "dockerfile" must be a string.', + }); + }); + + it('a number value fails the same way as a boolean', () => { + const result = resolveDockerfileLocation([actorJson({ dockerfile: 42 })]); + + expect(result.outcome).toBe('failure'); + if (result.outcome !== 'failure') return; + expect(result.reason).toBe('invalid-dockerfile-field'); + }); + + it('an empty string is treated as "names nothing" - warn and fall through, never the invalid-format failure', () => { + const result = resolveDockerfileLocation([ + actorJson({ dockerfile: '' }), + text('.actor/Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + expect(result.logLines[0]).toBe( + 'Warning: "" (from the "dockerfile" field in .actor/actor.json) is not in the pushed source; falling back to the default locations.\n', + ); + }); + + it('an empty string falls all the way through to the bundled default when no other candidate exists either', () => { + const result = resolveDockerfileLocation([actorJson({ dockerfile: '' }), text('main.py', 'print(1)\n')]); + + expect(result.outcome).toBe('default'); + }); + }); + + describe('JSON5 tolerance', () => { + it('parses a trailing comma and a comment in .actor/actor.json', () => { + const result = resolveDockerfileLocation([ + actorJson('{\n // a comment JSON.parse would reject\n "dockerfile": "./Dockerfile",\n}\n'), + text('.actor/Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + }); + + it('parses unquoted keys and single-quoted strings', () => { + const result = resolveDockerfileLocation([ + actorJson("{ dockerfile: '../Dockerfile' }"), + text('Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('Dockerfile'); + }); + + it('a genuinely unparseable .actor/actor.json fails the build with a clear message', () => { + const result = resolveDockerfileLocation([actorJson('{ this is not json at all')]); + + expect(result.outcome).toBe('failure'); + if (result.outcome !== 'failure') return; + expect(result.reason).toBe('unparseable-actor-json'); + expect(result.message).toContain('.actor/actor.json'); + }); + + it('tolerates .actor/actor.json delivered as a BASE64-encoded SourceFile', () => { + const spec = JSON.stringify({ dockerfile: './Dockerfile' }); + const result = resolveDockerfileLocation([ + { name: '.actor/actor.json', format: 'BASE64', content: Buffer.from(spec, 'utf8').toString('base64') }, + text('.actor/Dockerfile', 'FROM node:20\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + }); + }); + + describe('no-actor.json fallbacks', () => { + it('resolves .actor/Dockerfile with no actor.json present at all', () => { + const result = resolveDockerfileLocation([ + text('.actor/Dockerfile', 'FROM node:20\n'), + text('main.py', ''), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + }); + + it('resolves the root Dockerfile with no actor.json and no .actor/Dockerfile present', () => { + const result = resolveDockerfileLocation([text('Dockerfile', 'FROM node:20\n'), text('main.py', '')]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('Dockerfile'); + }); + + it('.actor/Dockerfile is preferred over a root Dockerfile when both exist and actor.json names neither', () => { + const result = resolveDockerfileLocation([ + text('.actor/Dockerfile', 'FROM node:20 # actor-dir\n'), + text('Dockerfile', 'FROM node:20 # root\n'), + ]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + }); + }); + + describe('.actor/actor.json parses to a non-object value', () => { + it('content is exactly "null" (JSON5-parseable, non-object) - falls through to candidate 2, no crash', () => { + const result = resolveDockerfileLocation([actorJson('null'), text('.actor/Dockerfile', 'FROM node:20\n')]); + + expect(result.outcome).toBe('resolved'); + if (result.outcome !== 'resolved') return; + expect(result.dockerfilePath).toBe('.actor/Dockerfile'); + }); + }); +}); diff --git a/test/unit/tar-entry-name.test.ts b/test/unit/tar-entry-name.test.ts new file mode 100644 index 0000000..1af1536 --- /dev/null +++ b/test/unit/tar-entry-name.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; + +import { normalizeEntryName } from '../../src/driver/tar-entry-name.js'; + +describe('normalizeEntryName', () => { + it('strips a leading "./"', () => { + expect(normalizeEntryName('./Dockerfile')).toBe('Dockerfile'); + }); + + it('converts backslashes to POSIX separators', () => { + expect(normalizeEntryName('.actor\\Dockerfile')).toBe('.actor/Dockerfile'); + }); + + it('collapses "a/./b" and "a/b/../c" via path.posix.normalize', () => { + expect(normalizeEntryName('.actor/./Dockerfile')).toBe('.actor/Dockerfile'); + expect(normalizeEntryName('.actor/sub/../Dockerfile')).toBe('.actor/Dockerfile'); + }); + + it('leaves an escaping "../" prefix alone (the escape check depends on this)', () => { + expect(normalizeEntryName('../evil/Dockerfile')).toBe('../evil/Dockerfile'); + }); +});