Add flag to skip persisting 'customizations' from referenced Features (…

…#262)
devcontainers · Nov 2, 2022 · 26723c0 · 26723c0
1 parent 409367d
commit 26723c0
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 4 deletions.
diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts
@@ -62,6 +62,7 @@ export interface ResolverParameters {
 	skipFeatureAutoMapping: boolean;
 	skipPostAttach: boolean;
 	experimentalImageMetadata: boolean;
+	skipPersistingCustomizationsFromFeatures: boolean;
 }
 
 export interface PostCreate {

diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
@@ -187,6 +187,7 @@ export interface ImageBuildOptions {
 }
 
 function getImageBuildOptions(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, dstFolder: string, baseName: string, imageBuildInfo: ImageBuildInfo) {
+
 	return {
 		dstFolder,
 		dockerfileContent: `
@@ -239,7 +240,8 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
 	const useBuildKitBuildContexts = buildKitVersionParsed ? !isEarlierVersion(buildKitVersionParsed, minRequiredVersion) : false;
 	const buildContentImageName = 'dev_container_feature_content_temp';
 
-	const imageMetadata = getDevcontainerMetadata(imageBuildInfo.metadata, devContainerConfig, featuresConfig);
+	const omitPropertyOverride = params.common.skipPersistingCustomizationsFromFeatures ? ['customizations'] : [];
+	const imageMetadata = getDevcontainerMetadata(imageBuildInfo.metadata, devContainerConfig, featuresConfig, omitPropertyOverride);
 	const { containerUser, remoteUser } = findContainerUsers(imageMetadata, composeServiceUser, imageBuildInfo.user);
 	const builtinVariables = [
 		`_CONTAINER_USER=${containerUser}`,

diff --git a/src/spec-node/devContainers.ts b/src/spec-node/devContainers.ts
@@ -57,6 +57,7 @@ export interface ProvisionOptions {
 	skipFeatureAutoMapping: boolean;
 	skipPostAttach: boolean;
 	experimentalImageMetadata: boolean;
+	skipPersistingCustomizationsFromFeatures: boolean;
 }
 
 export async function launch(options: ProvisionOptions, disposables: (() => Promise<unknown> | undefined)[]) {
@@ -131,6 +132,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
 		skipFeatureAutoMapping: options.skipFeatureAutoMapping,
 		skipPostAttach: options.skipPostAttach,
 		experimentalImageMetadata: options.experimentalImageMetadata,
+		skipPersistingCustomizationsFromFeatures: options.skipPersistingCustomizationsFromFeatures,
 	};
 
 	const dockerPath = options.dockerPath || 'docker';

diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts
@@ -224,6 +224,7 @@ async function provision({
 		skipFeatureAutoMapping,
 		skipPostAttach,
 		experimentalImageMetadata,
+		skipPersistingCustomizationsFromFeatures: false,
 	};
 
 	const result = await doProvision(options);
@@ -287,6 +288,7 @@ function buildOptions(y: Argv) {
 		'additional-features': { type: 'string', description: 'Additional features to apply to the dev container (JSON as per "features" section in devcontainer.json)' },
 		'skip-feature-auto-mapping': { type: 'boolean', default: false, hidden: true, description: 'Temporary option for testing.' },
 		'experimental-image-metadata': { type: 'boolean', default: experimentalImageMetadataDefault, hidden: true, description: 'Temporary option for testing.' },
+		'skip-persisting-customizations-from-features': { type: 'boolean', default: false, hidden: true, description: 'Do not save customizations from referenced Features as image metadata' },
 	});
 }
 
@@ -321,6 +323,7 @@ async function doBuild({
 	'additional-features': additionalFeaturesJson,
 	'skip-feature-auto-mapping': skipFeatureAutoMapping,
 	'experimental-image-metadata': experimentalImageMetadata,
+	'skip-persisting-customizations-from-features': skipPersistingCustomizationsFromFeatures,
 }: BuildArgs) {
 	const disposables: (() => Promise<unknown> | undefined)[] = [];
 	const dispose = async () => {
@@ -365,6 +368,7 @@ async function doBuild({
 			skipFeatureAutoMapping,
 			skipPostAttach: true,
 			experimentalImageMetadata,
+			skipPersistingCustomizationsFromFeatures: skipPersistingCustomizationsFromFeatures,
 		}, disposables);
 
 		const { common, dockerCLI, dockerComposeCLI } = params;
@@ -606,6 +610,7 @@ async function doRunUserCommands({
 			skipFeatureAutoMapping,
 			skipPostAttach,
 			experimentalImageMetadata,
+			skipPersistingCustomizationsFromFeatures: false,
 		}, disposables);
 
 		const { common } = params;
@@ -936,6 +941,7 @@ export async function doExec({
 			buildxOutput: undefined,
 			skipPostAttach: false,
 			experimentalImageMetadata,
+			skipPersistingCustomizationsFromFeatures: false,
 		}, disposables);
 
 		const { common } = params;

diff --git a/src/spec-node/featuresCLI/testCommandImpl.ts b/src/spec-node/featuresCLI/testCommandImpl.ts
@@ -375,6 +375,7 @@ async function launchProject(params: DockerResolverParameters, workspaceFolder:
 		remoteEnv: common.remoteEnv,
 		skipFeatureAutoMapping: common.skipFeatureAutoMapping,
 		experimentalImageMetadata: common.experimentalImageMetadata,
+		skipPersistingCustomizationsFromFeatures: common.skipPersistingCustomizationsFromFeatures,
 		log: text => quiet ? null : process.stderr.write(text),
 	};
 
@@ -472,5 +473,6 @@ async function generateDockerParams(workspaceFolder: string, args: FeaturesTestC
 		skipFeatureAutoMapping: false,
 		skipPostAttach: false,
 		experimentalImageMetadata: false,
+		skipPersistingCustomizationsFromFeatures: false,
 	}, disposables);
 }
diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts
@@ -214,11 +214,14 @@ function collectOrUndefined<T, K extends keyof T>(entries: T[], property: K): No
 	return values.length ? values : undefined;
 }
 
-export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | undefined): SubstitutedConfig<ImageMetadataEntry[]> {
+export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | undefined, omitPropertyOverride: string[] = []): SubstitutedConfig<ImageMetadataEntry[]> {
+	const effectivePickFeatureProperties = pickFeatureProperties.filter(property => !omitPropertyOverride.includes(property));
+
 	const raw = featuresConfig?.featureSets.map(featureSet => featureSet.features.map(feature => ({
 		id: featureSet.sourceInformation.userFeatureId,
-		...pick(feature, pickFeatureProperties),
+		...pick(feature, effectivePickFeatureProperties),
 	}))).flat() || [];
+
 	return {
 		config: [
 			...baseImageMetadata.config,

diff --git a/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json b/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json
@@ -1,5 +1,13 @@
 {
     "id": "localFeatureA",
     "version": "1.0.0",
-    "init": true
+    "init": true,
+    "customizations": {
+        "vscode": {
+            "extensions": [
+                "extensionA",
+                "extensionB"
+            ]
+        }
+    }
 }
diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts
@@ -60,6 +60,35 @@ describe('Image Metadata', function () {
 				assert.strictEqual(raw.length, 3);
 				assert.strictEqual(raw[0].id, 'baseFeature');
 				assert.strictEqual(raw[1].id, './localFeatureA');
+				assert.ok(raw[1].customizations);
+				assert.strictEqual(raw[1].customizations.vscode.extensions.length, 2);
+				assert.strictEqual(raw[1].init, true);
+				assert.strictEqual(raw[2].id, './localFeatureB');
+				assert.strictEqual(raw[2].privileged, true);
+			});
+		});
+
+		buildKitOptions.forEach(({ text, options }) => {
+			it(`should omit appending Feature customizations with --skip-persisting-customizations-from-features [${text}]`, async () => {
+				if (!experimentalImageMetadataDefault) {
+					return;
+				}
+				const buildKitOption = (options?.useBuildKit ?? false) ? '' : ' --buildkit=never';
+				const res = await shellExec(`${cli} build --skip-persisting-customizations-from-features --workspace-folder ${testFolder} --image-name skip-persisting-test${buildKitOption}`);
+				const response = JSON.parse(res.stdout);
+				assert.strictEqual(response.outcome, 'success');
+				const details = JSON.parse((await shellExec(`docker inspect skip-persisting-test`)).stdout)[0] as ImageDetails;
+				const { config: metadata, raw } = getImageMetadata(details, testSubstitute, true, nullLog);
+				assert.strictEqual(metadata.length, 3);
+				assert.strictEqual(metadata[0].id, 'baseFeature-substituted');
+				assert.strictEqual(metadata[1].id, './localFeatureA-substituted');
+				assert.strictEqual(metadata[1].init, true);
+				assert.strictEqual(metadata[2].id, './localFeatureB-substituted');
+				assert.strictEqual(metadata[2].privileged, true);
+				assert.strictEqual(raw.length, 3);
+				assert.strictEqual(raw[0].id, 'baseFeature');
+				assert.strictEqual(raw[1].id, './localFeatureA');
+				assert.ok(!raw[1].customizations); // Customizations have not been persisted due to the flag
 				assert.strictEqual(raw[1].init, true);
 				assert.strictEqual(raw[2].id, './localFeatureB');
 				assert.strictEqual(raw[2].privileged, true);