Use qualified id (microsoft/vscode-remote-release#7253)

src/spec-configuration/containerFeaturesConfiguration.ts

@@ -161,15 +161,6 @@ export interface CollapsedFeaturesConfig {
 	allFeatures: Feature[];
 }
 
-export function collapseFeaturesConfig(original: FeaturesConfig): CollapsedFeaturesConfig {
-	const collapsed = {
-		allFeatures: original.featureSets
-			.map(fSet => fSet.features)
-			.flat()
-	};
-	return collapsed;
-}
-
 export const multiStageBuildExploration = false;
 
 // Counter to ensure that no two folders are the same even if we are executing the same feature multiple times.

src/spec-configuration/containerFeaturesOCI.ts

@@ -4,7 +4,7 @@ import * as semver from 'semver';
 import { request } from '../spec-utils/httpRequest';
 import { Log, LogLevel } from '../spec-utils/log';
 import { mkdirpLocal, writeLocalFile } from '../spec-utils/pfs';
-import { FeatureSet } from './containerFeaturesConfiguration';
+import { Feature, FeatureSet } from './containerFeaturesConfiguration';
 
 export type HEADERS = { 'authorization'?: string; 'user-agent': string; 'content-type'?: string; 'accept'?: string };
 
@@ -60,7 +60,7 @@ export function getOCIFeatureSet(output: Log, identifier: string, options: boole
 
 	const featureRef = getFeatureRef(output, identifier);
 
-	const feat = {
+	const feat: Feature = {
 		id: featureRef.id,
 		included: true,
 		value: options

src/spec-node/containerFeatures.ts

@@ -10,7 +10,7 @@ import * as tar from 'tar';
 import { DevContainerConfig } from '../spec-configuration/configuration';
 import { dockerCLI, dockerPtyCLI, ImageDetails, toExecParameters, toPtyExecParameters } from '../spec-shutdown/dockerUtils';
 import { LogLevel, makeLog, toErrorText } from '../spec-utils/log';
-import { FeaturesConfig, getContainerFeaturesFolder, getContainerFeaturesBaseDockerFile, getFeatureInstallWrapperScript, getFeatureLayers, getFeatureMainValue, getFeatureValueObject, generateFeaturesConfig, getSourceInfoString, collapseFeaturesConfig, Feature, multiStageBuildExploration, V1_DEVCONTAINER_FEATURES_FILE_NAME } from '../spec-configuration/containerFeaturesConfiguration';
+import { FeaturesConfig, getContainerFeaturesFolder, getContainerFeaturesBaseDockerFile, getFeatureInstallWrapperScript, getFeatureLayers, getFeatureMainValue, getFeatureValueObject, generateFeaturesConfig, getSourceInfoString, Feature, multiStageBuildExploration, V1_DEVCONTAINER_FEATURES_FILE_NAME } from '../spec-configuration/containerFeaturesConfiguration';
 import { readLocalFile } from '../spec-utils/pfs';
 import { includeAllConfiguredFeatures } from '../spec-utils/product';
 import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, getFolderImageName, getEmptyContextFolder, SubstitutedConfig } from './utils';
@@ -42,7 +42,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
 			imageDetails: async () => imageBuildInfo.imageDetails,
 		};
 	}
-	const { featureBuildInfo, collapsedFeaturesConfig } = extendImageDetails;
+	const { featureBuildInfo, featuresConfig } = extendImageDetails;
 
 	// Got feature extensions -> build the image
 	const dockerfilePath = cliHost.path.join(featureBuildInfo.dstFolder, 'Dockerfile.extended');
@@ -88,7 +88,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
 	}
 	return {
 		updatedImageName: [ updatedImageName ],
-		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, config, collapsedFeaturesConfig?.allFeatures || []),
+		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, config, featuresConfig),
 		imageDetails: async () => imageBuildInfo.imageDetails,
 	};
 }
@@ -108,12 +108,11 @@ export async function getExtendImageBuildInfo(params: DockerResolverParameters,
 	}
 
 	// Generates the end configuration.
-	const collapsedFeaturesConfig = collapseFeaturesConfig(featuresConfig);
 	const featureBuildInfo = await getFeaturesBuildOptions(params, config, featuresConfig, baseName, imageBuildInfo);
 	if (!featureBuildInfo) {
 		return undefined;
 	}
-	return { featureBuildInfo, collapsedFeaturesConfig };
+	return { featureBuildInfo, featuresConfig };
 
 }
 
@@ -191,7 +190,7 @@ function getImageBuildOptions(params: DockerResolverParameters, config: Substitu
 		dstFolder,
 		dockerfileContent: `
 FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage
-${getDevcontainerMetadataLabel(imageBuildInfo.metadata, config, [], params.common.experimentalImageMetadata)}
+${getDevcontainerMetadataLabel(imageBuildInfo.metadata, config, { featureSets: [] }, params.common.experimentalImageMetadata)}
 `,
 		overrideTarget: 'dev_containers_target_stage',
 		dockerfilePrefixContent: `

src/spec-node/devContainersSpecCLI.ts

@@ -593,7 +593,7 @@ async function doRunUserCommands({
 		if (!container) {
 			bailOut(common.output, 'Dev container not found.');
 		}
-		const imageMetadata = getImageMetadataFromContainer(container, config, [], experimentalImageMetadata, output).config;
+		const imageMetadata = getImageMetadataFromContainer(container, config, undefined, experimentalImageMetadata, output).config;
 		const mergedConfig = mergeConfiguration(config.config, imageMetadata);
 		const containerProperties = await createContainerProperties(params, container.Id, workspaceConfig.workspaceFolder, mergedConfig.remoteUser);
 		const updatedConfig = containerSubstitute(cliHost.platform, config.config.configFilePath, containerProperties.env, mergedConfig);
@@ -740,12 +740,12 @@ async function readConfiguration({
 		if (includeMergedConfig) {
 			let imageMetadata: ImageMetadataEntry[];
 			if (container) {
-				imageMetadata = getImageMetadataFromContainer(container, configuration, featuresConfiguration || [], experimentalImageMetadata, output).config;
+				imageMetadata = getImageMetadataFromContainer(container, configuration, featuresConfiguration, experimentalImageMetadata, output).config;
 				const substitute2: SubstituteConfig = config => containerSubstitute(cliHost.platform, configuration.config.configFilePath, envListToObj(container.Config.Env), config);
 				imageMetadata = imageMetadata.map(substitute2);
 			} else {
 				const imageBuildInfo = await getImageBuildInfo(params, configs.config, experimentalImageMetadata);
-				imageMetadata = getDevcontainerMetadata(imageBuildInfo.metadata, configs.config, featuresConfiguration || []).config;
+				imageMetadata = getDevcontainerMetadata(imageBuildInfo.metadata, configs.config, featuresConfiguration).config;
 			}
 			mergedConfig = mergeConfiguration(configuration.config, imageMetadata);
 		}
@@ -920,7 +920,7 @@ export async function doExec({
 		if (!container) {
 			bailOut(common.output, 'Dev container not found.');
 		}
-		const imageMetadata = getImageMetadataFromContainer(container, config, [], experimentalImageMetadata, output).config;
+		const imageMetadata = getImageMetadataFromContainer(container, config, undefined, experimentalImageMetadata, output).config;
 		const mergedConfig = mergeConfiguration(config.config, imageMetadata);
 		const containerProperties = await createContainerProperties(params, container.Id, workspaceConfig.workspaceFolder, mergedConfig.remoteUser);
 		const updatedConfig = containerSubstitute(cliHost.platform, config.config.configFilePath, containerProperties.env, mergedConfig);

src/spec-node/dockerCompose.ts

@@ -68,7 +68,7 @@ async function _openDockerComposeDevContainer(params: DockerResolverParameters,
 			// 	collapsedFeaturesConfig = collapseFeaturesConfig(featuresConfig);
 		}
 
-		const configs = getImageMetadataFromContainer(container, configWithRaw, [], common.experimentalImageMetadata, common.output).config;
+		const configs = getImageMetadataFromContainer(container, configWithRaw, undefined, common.experimentalImageMetadata, common.output).config;
 		const mergedConfig = mergeConfiguration(configWithRaw.config, configs);
 		containerProperties = await createContainerProperties(params, container.Id, remoteWorkspaceFolder, mergedConfig.remoteUser);
 
@@ -273,7 +273,7 @@ ${cacheFromOverrideContent}
 	}
 
 	return {
-		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, configWithRaw, extendImageBuildInfo?.collapsedFeaturesConfig?.allFeatures || []),
+		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, configWithRaw, extendImageBuildInfo?.featuresConfig),
 		additionalComposeOverrideFiles,
 		overrideImageName,
 	};

src/spec-node/imageMetadata.ts

@@ -39,8 +39,7 @@ const pickConfigProperties: (keyof DevContainerConfig & keyof ImageMetadataEntry
 	'hostRequirements',
 ];
 
-const pickFeatureProperties: (keyof Feature & keyof ImageMetadataEntry)[] = [
-	'id',
+const pickFeatureProperties: Exclude<keyof Feature & keyof ImageMetadataEntry, 'id'>[] = [
 	'init',
 	'privileged',
 	'capAdd',
@@ -198,12 +197,11 @@ 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 | Feature[]): SubstitutedConfig<ImageMetadataEntry[]> {
-	const features = Array.isArray(featuresConfig) ? featuresConfig : ([] as Feature[]).concat(
-		...featuresConfig.featureSets
-			.map(x => x.features)
-	);
-	const raw = features.map(feature => pick(feature, pickFeatureProperties));
+export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | undefined): SubstitutedConfig<ImageMetadataEntry[]> {
+	const raw = featuresConfig?.featureSets.map(featureSet => featureSet.features.map(feature => ({
+		id: featureSet.sourceInformation.userFeatureId,
+		...pick(feature, pickFeatureProperties),
+	}))).flat() || [];
 	return {
 		config: [
 			...baseImageMetadata.config,
@@ -311,7 +309,7 @@ export async function internalGetImageBuildInfoFromDockerfile(inspectDockerImage
 
 export const imageMetadataLabel = 'devcontainer.metadata';
 
-export function getImageMetadataFromContainer(containerDetails: ContainerDetails, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | Feature[], experimentalImageMetadata: boolean, output: Log): SubstitutedConfig<ImageMetadataEntry[]> {
+export function getImageMetadataFromContainer(containerDetails: ContainerDetails, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | undefined, experimentalImageMetadata: boolean, output: Log): SubstitutedConfig<ImageMetadataEntry[]> {
 	if (!experimentalImageMetadata) {
 		return getDevcontainerMetadata({ config: [], raw: [], substitute: devContainerConfig.substitute }, devContainerConfig, featuresConfig);
 	}
@@ -353,7 +351,7 @@ function internalGetImageMetadata0(imageDetails: ImageDetails | ContainerDetails
 	return [];
 }
 
-export function getDevcontainerMetadataLabel(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig | Feature[], experimentalImageMetadata: boolean) {
+export function getDevcontainerMetadataLabel(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig, experimentalImageMetadata: boolean) {
 	if (!experimentalImageMetadata) {
 		return '';
 	}

src/spec-node/singleContainer.ts

@@ -37,7 +37,7 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter
 			// 	})());
 			// };
 			await startExistingContainer(params, idLabels, container);
-			const imageMetadata = getImageMetadataFromContainer(container, configWithRaw, [], common.experimentalImageMetadata, common.output).config;
+			const imageMetadata = getImageMetadataFromContainer(container, configWithRaw, undefined, common.experimentalImageMetadata, common.output).config;
 			mergedConfig = mergeConfiguration(config, imageMetadata);
 		} else {
 			const res = await buildNamedImageAndExtend(params, configWithRaw);
@@ -245,7 +245,7 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
 
 	return {
 		updatedImageName: baseImageNames,
-		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, configWithRaw, extendImageBuildInfo?.collapsedFeaturesConfig?.allFeatures || []),
+		imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, configWithRaw, extendImageBuildInfo?.featuresConfig),
 		imageDetails
 	};
 }

src/test/imageMetadata.test.ts

@@ -6,6 +6,7 @@
 import * as assert from 'assert';
 import * as path from 'path';
 import { URI } from 'vscode-uri';
+import { Feature, FeaturesConfig, FeatureSet } from '../spec-configuration/containerFeaturesConfiguration';
 import { experimentalImageMetadataDefault } from '../spec-node/devContainers';
 import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageMetadata, mergeConfiguration } from '../spec-node/imageMetadata';
 import { SubstitutedConfig } from '../spec-node/utils';
@@ -52,15 +53,15 @@ describe('Image Metadata', function () {
 				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].id, './localFeatureA-substituted');
 				assert.strictEqual(metadata[1].init, true);
-				assert.strictEqual(metadata[2].id, 'localFeatureB-substituted');
+				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.strictEqual(raw[1].id, './localFeatureA');
 				assert.strictEqual(raw[1].init, true);
-				assert.strictEqual(raw[2].id, 'localFeatureB');
+				assert.strictEqual(raw[2].id, './localFeatureB');
 				assert.strictEqual(raw[2].privileged, true);
 			});
 		});
@@ -72,18 +73,18 @@ describe('Image Metadata', function () {
 				configFilePath: URI.parse('file:///devcontainer.json'),
 				image: 'image',
 				remoteUser: 'testUser',
-			}), [
+			}), getFeaturesConfig([
 				{
 					id: 'someFeature',
 					value: 'someValue',
 					included: true,
 				}
-			]);
+			]));
 			assert.strictEqual(metadata.length, 2);
-			assert.strictEqual(metadata[0].id, 'someFeature-substituted');
+			assert.strictEqual(metadata[0].id, 'ghcr.io/my-org/my-repo/someFeature:1-substituted');
 			assert.strictEqual(metadata[1].remoteUser, 'testUser');
 			assert.strictEqual(raw.length, 2);
-			assert.strictEqual(raw[0].id, 'someFeature');
+			assert.strictEqual(raw[0].id, 'ghcr.io/my-org/my-repo/someFeature:1');
 			assert.strictEqual(raw[1].remoteUser, 'testUser');
 		});
 
@@ -96,19 +97,19 @@ describe('Image Metadata', function () {
 				configFilePath: URI.parse('file:///devcontainer.json'),
 				image: 'image',
 				remoteUser: 'testUser',
-			}), [
+			}), getFeaturesConfig([
 				{
 					id: 'someFeature',
 					value: 'someValue',
 					included: true,
 				}
-			], true);
+			]), true);
 			const expected = [
 				{
 					id: 'baseFeature',
 				},
 				{
-					id: 'someFeature',
+					id: 'ghcr.io/my-org/my-repo/someFeature:1',
 				},
 				{
 					remoteUser: 'testUser',
@@ -154,3 +155,35 @@ describe('Image Metadata', function () {
 		});
 	});
 });
+
+function getFeaturesConfig(features: Feature[]): FeaturesConfig {
+	return {
+		featureSets: features.map((feature): FeatureSet => ({
+			features: [feature],
+			sourceInformation: {
+				type: 'oci',
+				userFeatureId: `ghcr.io/my-org/my-repo/${feature.id}:1`,
+				userFeatureIdWithoutVersion: `ghcr.io/my-org/my-repo/${feature.id}`,
+				featureRef: {
+					registry: 'ghcr.io',
+					owner: 'my-org',
+					namespace: 'my-org/my-repo',
+					path: 'my-org/my-repo/test',
+					resource: 'ghcr.io/my-org/my-repo/test',
+					id: 'test',
+					version: '1.2.3',
+				},
+				manifest: {
+					schemaVersion: 1,
+					mediaType: '',
+					config: {
+						digest: '',
+						mediaType: '',
+						size: 0,
+					},
+					layers: [],
+				}
+			}
+		}))
+	};
+}