Merge pull request #352 from devcontainers/joshspicer/issue/106

Allow prebuilding image without a Dockerfile
devcontainers · Jan 4, 2023 · b61f8dd · b61f8dd
2 parents e7233d1 + 5c54607
commit b61f8dd
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 17 deletions.
diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
@@ -17,6 +17,7 @@ import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, get
 import { isEarlierVersion, parseVersion } from '../spec-common/commonUtils';
 import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, ImageMetadataEntry, imageMetadataLabel, MergedDevContainerConfig } from './imageMetadata';
 import { supportsBuildContexts } from './dockerfileUtils';
+import { ContainerError } from '../spec-common/errors';
 
 // Escapes environment variable keys.
 //
@@ -53,15 +54,35 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
 	const updatedImageName = `${imageName.startsWith(folderImageName) ? imageName : folderImageName}-features`;
 
 	const args: string[] = [];
+	if (!params.buildKitVersion &&
+		(params.buildxPlatform || params.buildxPush)) {
+		throw new ContainerError({ description: '--platform or --push require BuildKit enabled.', data: { fileWithError: dockerfilePath } });
+	}
 	if (params.buildKitVersion) {
-		args.push(
-			'buildx', 'build',
-			'--load', // (short for --output=docker, i.e. load into normal 'docker images' collection)
-		);
+		args.push('buildx', 'build');
+
+		// --platform
+		if (params.buildxPlatform) {
+			output.write('Setting BuildKit platform(s): ' + params.buildxPlatform, LogLevel.Trace);
+			args.push('--platform', params.buildxPlatform);
+		}
+
+		// --push/--output
+		if (params.buildxPush) {
+			args.push('--push');
+		} else {
+			if (params.buildxOutput) {
+				args.push('--output', params.buildxOutput);
+			} else {
+				args.push('--load'); // (short for --output=docker, i.e. load into normal 'docker images' collection)
+			}
+		}
+
 		for (const buildContext in featureBuildInfo.buildKitContexts) {
 			args.push('--build-context', `${buildContext}=${featureBuildInfo.buildKitContexts[buildContext]}`);
 		}
 	} else {
+		// Not using buildx
 		args.push(
 			'build',
 		);

diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts
@@ -457,12 +457,6 @@ async function doBuild({
 			await inspectDockerImage(params, config.image, true);
 			const { updatedImageName } = await extendImage(params, configWithRaw, config.image, additionalFeatures, false);
 
-			if (buildxPlatform || buildxPush) {
-				throw new ContainerError({ description: '--platform or --push require dockerfilePath.' });
-			}
-			if (buildxOutput) {
-				throw new ContainerError({ description: '--output requires dockerfilePath.' });
-			}
 			if (imageNames) {
 				await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName)));
 				imageNameResult = imageNames;

diff --git a/src/spec-node/dockerfileUtils.ts b/src/spec-node/dockerfileUtils.ts
@@ -35,7 +35,7 @@ export interface Stage {
 }
 
 export interface From {
-	platfrom?: string;
+	platform?: string;
 	image: string;
 	label?: string;
 }

diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts
@@ -179,6 +179,7 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
 	if (buildParams.buildKitVersion) {
 		args.push('buildx', 'build');
 		if (buildParams.buildxPlatform) {
+			output.write('Setting BuildKit platform(s): ' + buildParams.buildxPlatform, LogLevel.Trace);
 			args.push('--platform', buildParams.buildxPlatform);
 		}
 		if (buildParams.buildxPush) {

diff --git a/src/test/cli.build.test.ts b/src/test/cli.build.test.ts
@@ -76,13 +76,20 @@ describe('Dev Containers CLI', function () {
 			assert.equal(success, false, 'expect non-successful call');
 		});
 
-		it('should succeed with supported --platform', async () => {
+		it('should succeed (dockerfile) with supported --platform', async () => {
 			const testFolder = `${__dirname}/configs/dockerfile-with-target`;
 			const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --platform linux/amd64`);
 			const response = JSON.parse(res.stdout);
 			assert.equal(response.outcome, 'success');
 		});
 
+		it('should succeed (image) with supported --platform', async () => {
+			const testFolder = `${__dirname}/configs/image-with-features`;
+			const res = await shellExec(`${cli} build --workspace-folder ${testFolder} --platform linux/amd64`);
+			const response = JSON.parse(res.stdout);
+			assert.equal(response.outcome, 'success');
+		});
+
 		it('should fail --platform without dockerfile', async () => {
 			let success = false;
 			const testFolder = `${__dirname}/configs/image`;

diff --git a/src/test/configs/dockerfile-with-features/.devcontainer.json b/src/test/configs/dockerfile-with-features/.devcontainer.json
@@ -6,8 +6,8 @@
 		}
 	},
 	"features": {
-		"docker-in-docker": "latest",
-		"codspace/myfeatures/helloworld": {
+		"ghcr.io/devcontainers/features/docker-in-docker:2": {},
+		"ghcr.io/devcontainers/feature-starter/hello:1": {
 			"greeting": "howdy"
 		}
 	}

diff --git a/src/test/configs/image-with-features/.devcontainer.json b/src/test/configs/image-with-features/.devcontainer.json
@@ -1,8 +1,8 @@
 {
-	"image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:0-16-bullseye",
+	"image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:0-16",
 	"features": {
-		"ghcr.io/devcontainers/features/docker-in-docker:1": {},
-		"codspace/myfeatures/helloworld": {
+		"ghcr.io/devcontainers/features/docker-in-docker:2": {},
+		"ghcr.io/devcontainers/feature-starter/hello:1": {
 			"greeting": "howdy"
 		}
 	}