Skip to content

Commit bcbf397

Browse files
committed
Add more control
1 parent 04bf682 commit bcbf397

2 files changed

Lines changed: 85 additions & 7 deletions

File tree

src/spec-node/dockerfilePreprocessor.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { runCommandNoPty } from '../spec-common/commonUtils';
1111
import { Log, LogLevel, makeLog } from '../spec-utils/log';
1212

1313
function dockerfilePreprocessorToolDocs(): string {
14-
return "Set 'dockerfilePreprocessor.tool' and optional 'dockerfilePreprocessor.args' in devcontainer.json. Use 'outputMode' to choose whether the tool runs in 'single-file' mode or 'build-tree' mode. Use 'generatedDockerfile' for tools that write the final Dockerfile to a predictable workspace-relative path instead of the CLI-provided output argument.";
14+
return "Set 'dockerfilePreprocessor.tool' and optional 'dockerfilePreprocessor.args' in devcontainer.json. 'outputMode' controls CLI invocation shape: 'single-file' passes input/output and 'build-tree' passes input/output/workdir. When using 'build-tree', set 'generatedDockerfile' to the tool's produced Dockerfile path so the CLI can verify and synchronize outputs.";
1515
}
1616

1717
export function getDockerfilePreprocessedPath(dockerfilePath: string): string | undefined {
@@ -41,6 +41,18 @@ export async function preprocessDockerExtensionFile(
4141
data: { fileWithError: dockerfilePath },
4242
});
4343
}
44+
if (outputMode === 'single-file' && generatedDockerfile) {
45+
throw new ContainerError({
46+
description: `dockerfilePreprocessor.outputMode 'single-file' cannot be used with 'dockerfilePreprocessor.generatedDockerfile'. Omit generatedDockerfile in single-file mode. ${dockerfilePreprocessorToolDocs()}`,
47+
data: { fileWithError: dockerfilePath },
48+
});
49+
}
50+
if (outputMode === 'build-tree' && !generatedDockerfile) {
51+
throw new ContainerError({
52+
description: `dockerfilePreprocessor.outputMode 'build-tree' requires 'dockerfilePreprocessor.generatedDockerfile' to be set. ${dockerfilePreprocessorToolDocs()}`,
53+
data: { fileWithError: dockerfilePath },
54+
});
55+
}
4456

4557
const { cliHost, output } = params;
4658
const infoOutput = makeLog(output, LogLevel.Info);
@@ -75,7 +87,7 @@ export async function preprocessDockerExtensionFile(
7587
const directOutputArgs = outputMode === 'single-file'
7688
? [inputPath, outputPath]
7789
: [inputPath, outputPath, workdirPath];
78-
const invocationArgs = generatedDockerfile ? args : [...args, ...directOutputArgs];
90+
const invocationArgs = [...args, ...directOutputArgs];
7991

8092
try {
8193
infoOutput.write(`Preprocessing '${dockerfilePath}' -> '${cliOutputPath}'`);
@@ -109,6 +121,11 @@ export async function preprocessDockerExtensionFile(
109121
});
110122
}
111123

124+
if (generatedDockerfile && generatedOutputPath !== outputPath && !await cliHost.isFile(generatedOutputPath) && await cliHost.isFile(outputPath)) {
125+
infoOutput.write(`No generated Dockerfile found at '${generatedOutputPath}', copying from CLI output '${outputPath}' to keep generated output consistent.`);
126+
await cliHost.copyFile(outputPath, generatedOutputPath);
127+
}
128+
112129
if (!await cliHost.isFile(generatedOutputPath)) {
113130
throw new ContainerError({
114131
description: generatedDockerfile
@@ -120,7 +137,6 @@ export async function preprocessDockerExtensionFile(
120137

121138
if (generatedOutputPath !== outputPath) {
122139
await cliHost.copyFile(generatedOutputPath, outputPath);
123-
await cliHost.remove(generatedOutputPath);
124140
}
125141

126142
infoOutput.write(`Preprocessed Dockerfile written to '${cliOutputPath}'`);

src/test/dockerfilePreprocessor.test.ts

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('dockerfilePreprocessor', function () {
6666
assert.strictEqual(outputContent, 'FROM alpine:3.20\n');
6767
});
6868

69-
it('passes the CLI-owned output path to the tool', async function () {
69+
it('passes the CLI-owned output path to the tool in single-file mode', async function () {
7070
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devcontainer-preprocess-'));
7171
const inputPath = path.join(tmpDir, 'Dockerfile.in');
7272
const outputPath = path.join(tmpDir, '.devcontainer-preprocessed', 'Dockerfile');
@@ -78,7 +78,7 @@ describe('dockerfilePreprocessor', function () {
7878
const cliHost = await getCLIHost(process.cwd(), loadNativeModule, true);
7979
const result = await preprocessDockerExtensionFile(
8080
{ cliHost, output: nullLog },
81-
{ dockerfilePreprocessor: { tool: './write-output.sh', outputMode: 'build-tree' } },
81+
{ dockerfilePreprocessor: { tool: './write-output.sh', outputMode: 'single-file' } },
8282
inputPath
8383
);
8484

@@ -87,6 +87,68 @@ describe('dockerfilePreprocessor', function () {
8787
assert.strictEqual(outputContent, 'FROM busybox\n');
8888
});
8989

90+
it('requires generatedDockerfile when outputMode is build-tree', async () => {
91+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devcontainer-preprocess-'));
92+
const inputPath = path.join(tmpDir, 'Dockerfile.in');
93+
await fs.writeFile(inputPath, 'FROM alpine:3.20\n');
94+
95+
const cliHost = await getCLIHost(process.cwd(), loadNativeModule, true);
96+
await assert.rejects(
97+
preprocessDockerExtensionFile(
98+
{ cliHost, output: nullLog },
99+
{ dockerfilePreprocessor: { tool: 'true', outputMode: 'build-tree' } },
100+
inputPath
101+
),
102+
(err: unknown) => {
103+
assert.ok(err instanceof ContainerError);
104+
assert.match((err as ContainerError).description, /build-tree.*generatedDockerfile/i);
105+
return true;
106+
}
107+
);
108+
});
109+
110+
it('rejects generatedDockerfile when outputMode is single-file', async () => {
111+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devcontainer-preprocess-'));
112+
const inputPath = path.join(tmpDir, 'Dockerfile.in');
113+
await fs.writeFile(inputPath, 'FROM alpine:3.20\n');
114+
115+
const cliHost = await getCLIHost(process.cwd(), loadNativeModule, true);
116+
await assert.rejects(
117+
preprocessDockerExtensionFile(
118+
{ cliHost, output: nullLog },
119+
{ dockerfilePreprocessor: { tool: 'true', outputMode: 'single-file', generatedDockerfile: 'build/Dockerfile' } },
120+
inputPath
121+
),
122+
(err: unknown) => {
123+
assert.ok(err instanceof ContainerError);
124+
assert.match((err as ContainerError).description, /single-file.*generatedDockerfile/i);
125+
return true;
126+
}
127+
);
128+
});
129+
130+
it('build-tree mode keeps generatedDockerfile authoritative and syncs CLI output', async () => {
131+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devcontainer-preprocess-'));
132+
const inputPath = path.join(tmpDir, 'Dockerfile.in');
133+
const outputPath = path.join(tmpDir, '.devcontainer-preprocessed', 'Dockerfile');
134+
const generatedPath = path.join(tmpDir, 'build', 'Dockerfile');
135+
const scriptPath = path.join(tmpDir, 'write-output.sh');
136+
await fs.writeFile(inputPath, 'FROM alpine:3.20\n');
137+
await fs.writeFile(scriptPath, '#!/bin/sh\nset -eu\nprintf "FROM busybox\\n" > "$2"\n');
138+
await fs.chmod(scriptPath, 0o755);
139+
140+
const cliHost = await getCLIHost(process.cwd(), loadNativeModule, true);
141+
const result = await preprocessDockerExtensionFile(
142+
{ cliHost, output: nullLog },
143+
{ dockerfilePreprocessor: { tool: './write-output.sh', outputMode: 'build-tree', generatedDockerfile: 'build/Dockerfile' } },
144+
inputPath
145+
);
146+
147+
assert.strictEqual(result, outputPath);
148+
assert.strictEqual((await fs.readFile(generatedPath)).toString(), 'FROM busybox\n');
149+
assert.strictEqual((await fs.readFile(outputPath)).toString(), 'FROM busybox\n');
150+
});
151+
90152
it('throws when a preprocessor command fails', async () => {
91153
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devcontainer-preprocess-'));
92154
const inputPath = path.join(tmpDir, 'Dockerfile.in');
@@ -95,7 +157,7 @@ describe('dockerfilePreprocessor', function () {
95157
const cliHost = await getCLIHost(process.cwd(), loadNativeModule, true);
96158
await assert.rejects(preprocessDockerExtensionFile(
97159
{ cliHost, output: nullLog },
98-
{ dockerfilePreprocessor: { tool: 'this-command-should-not-exist-xyz123' } },
160+
{ dockerfilePreprocessor: { tool: 'this-command-should-not-exist-xyz123', outputMode: 'single-file' } },
99161
inputPath
100162
));
101163
});
@@ -109,7 +171,7 @@ describe('dockerfilePreprocessor', function () {
109171
await assert.rejects(
110172
preprocessDockerExtensionFile(
111173
{ cliHost, output: nullLog },
112-
{ dockerfilePreprocessor: { tool: 'true' } },
174+
{ dockerfilePreprocessor: { tool: 'true', outputMode: 'single-file' } },
113175
inputPath
114176
),
115177
(err: unknown) => {

0 commit comments

Comments
 (0)