From 5879ee8411f706a7b59bd9f1ab275589dcede60c Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Thu, 29 Sep 2022 16:12:39 +0200 Subject: [PATCH] Use qualified id (microsoft/vscode-remote-release#7253) --- .../containerFeaturesConfiguration.ts | 9 --- .../containerFeaturesOCI.ts | 4 +- src/spec-node/containerFeatures.ts | 11 ++-- src/spec-node/devContainersSpecCLI.ts | 8 +-- src/spec-node/dockerCompose.ts | 4 +- src/spec-node/imageMetadata.ts | 18 +++--- src/spec-node/singleContainer.ts | 4 +- src/test/imageMetadata.test.ts | 55 +++++++++++++++---- 8 files changed, 67 insertions(+), 46 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 84a9b89fa..b8ce53b68 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/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. diff --git a/src/spec-configuration/containerFeaturesOCI.ts b/src/spec-configuration/containerFeaturesOCI.ts index ad523388c..1352cafa3 100644 --- a/src/spec-configuration/containerFeaturesOCI.ts +++ b/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 diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 143e18199..c046b355b 100644 --- a/src/spec-node/containerFeatures.ts +++ b/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: ` diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index 6e2d33e86..7a0768b3a 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/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); diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index 37c67be5d..e333132f7 100644 --- a/src/spec-node/dockerCompose.ts +++ b/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, }; diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 2a4dbc09e..95ffa8878 100644 --- a/src/spec-node/imageMetadata.ts +++ b/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[] = [ 'init', 'privileged', 'capAdd', @@ -198,12 +197,11 @@ function collectOrUndefined(entries: T[], property: K): No return values.length ? values : undefined; } -export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | Feature[]): SubstitutedConfig { - 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, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | undefined): SubstitutedConfig { + 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, featuresConfig: FeaturesConfig | Feature[], experimentalImageMetadata: boolean, output: Log): SubstitutedConfig { +export function getImageMetadataFromContainer(containerDetails: ContainerDetails, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | undefined, experimentalImageMetadata: boolean, output: Log): SubstitutedConfig { 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, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | Feature[], experimentalImageMetadata: boolean) { +export function getDevcontainerMetadataLabel(baseImageMetadata: SubstitutedConfig, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig, experimentalImageMetadata: boolean) { if (!experimentalImageMetadata) { return ''; } diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index 1df6c43af..1ac4244b3 100644 --- a/src/spec-node/singleContainer.ts +++ b/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 }; } diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index fa4044ecb..ece87eab6 100644 --- a/src/test/imageMetadata.test.ts +++ b/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: [], + } + } + })) + }; +} \ No newline at end of file