Skip to content

Commit

Permalink
Last mount source wins (microsoft/vscode-remote-release#7368)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrmarti committed Oct 26, 2022
1 parent 36c62b9 commit f4da653
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 7 deletions.
8 changes: 7 additions & 1 deletion src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,16 @@ export interface Mount {
external?: boolean;
}

const normalizedMountKeys: Record<string, string> = {
src: 'source',
destination: 'target',
dst: 'target',
};

export function parseMount(str: string): Mount {
return str.split(',')
.map(s => s.split('='))
.reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}) as Mount;
.reduce((acc, [key, value]) => ({ ...acc, [(normalizedMountKeys[key] || key)]: value }), {}) as Mount;
}

export type SourceInformation = LocalCacheSourceInformation | GithubSourceInformation | DirectTarballSourceInformation | FilePathSourceInformation | OCISourceInformation;
Expand Down
21 changes: 16 additions & 5 deletions src/spec-node/imageMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { ContainerError } from '../spec-common/errors';
import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration';
import { Feature, FeaturesConfig, Mount } from '../spec-configuration/containerFeaturesConfiguration';
import { Feature, FeaturesConfig, Mount, parseMount } from '../spec-configuration/containerFeaturesConfiguration';
import { ContainerDetails, DockerCLIParameters, ImageDetails } from '../spec-shutdown/dockerUtils';
import { Log } from '../spec-utils/log';
import { getBuildInfoForService, readDockerComposeConfig } from './dockerCompose';
Expand Down Expand Up @@ -125,7 +125,7 @@ export function mergeConfiguration(config: DevContainerConfig, imageMetadata: Im
capAdd: unionOrUndefined(imageMetadata.map(entry => entry.capAdd)),
securityOpt: unionOrUndefined(imageMetadata.map(entry => entry.securityOpt)),
entrypoints: collectOrUndefined(imageMetadata, 'entrypoint'),
mounts: concatOrUndefined(imageMetadata.map(entry => entry.mounts)),
mounts: mergeMounts(imageMetadata),
customizations: Object.keys(customizations).length ? customizations : undefined,
onCreateCommands: collectOrUndefined(imageMetadata, 'onCreateCommand'),
updateContentCommands: collectOrUndefined(imageMetadata, 'updateContentCommand'),
Expand Down Expand Up @@ -181,9 +181,20 @@ function parseBytes(str: string) {
return 0;
}

function concatOrUndefined<T>(entries: (T[] | undefined)[]): T[] | undefined {
const values = ([] as T[]).concat(...entries.filter(entry => !!entry) as T[][]);
return values.length ? values : undefined;
function mergeMounts(imageMetadata: ImageMetadataEntry[]): (Mount | string)[] | undefined {
const seen = new Set<string>();
const mounts = imageMetadata.map(entry => entry.mounts)
.filter(Boolean)
.flat()
.map(mount => ({
obj: typeof mount === 'string' ? parseMount(mount) : mount!,
orig: mount!,
}))
.reverse()
.filter(mount => !seen.has(mount.obj.target) && seen.add(mount.obj.target))
.reverse()
.map(mount => mount.orig);
return mounts.length ? mounts : undefined;
}

function unionOrUndefined<T>(entries: (T[] | undefined)[]): T[] | undefined {
Expand Down
42 changes: 41 additions & 1 deletion src/test/imageMetadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +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 { Feature, FeaturesConfig, FeatureSet, Mount } from '../spec-configuration/containerFeaturesConfiguration';
import { experimentalImageMetadataDefault } from '../spec-node/devContainers';
import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageMetadata, getImageMetadataFromContainer, imageMetadataLabel, mergeConfiguration } from '../spec-node/imageMetadata';
import { SubstitutedConfig } from '../spec-node/utils';
Expand Down Expand Up @@ -244,6 +244,46 @@ describe('Image Metadata', function () {
assert.strictEqual(merged.remoteEnv?.ENV3, 'devcontainer.json');
assert.strictEqual(merged.remoteEnv?.ENV4, 'feature1');
});

it('should deduplicate mounts', () => {
const merged = mergeConfiguration({
configFilePath: URI.parse('file:///devcontainer.json'),
image: 'image',
}, [
{
mounts: [
'source=source1,dst=target1,type=volume',
'source=source2,target=target2,type=volume',
'source=source3,destination=target3,type=volume',
],
},
{
mounts: [
{
source: 'source4',
target: 'target1',
type: 'volume'
},
],
},
{
mounts: [
{
source: 'source5',
target: 'target3',
type: 'volume'
},
],
},
]);
assert.strictEqual(merged.mounts?.length, 3);
assert.strictEqual(typeof merged.mounts?.[0], 'string');
assert.strictEqual(merged.mounts?.[0], 'source=source2,target=target2,type=volume');
assert.strictEqual(typeof merged.mounts?.[1], 'object');
assert.strictEqual((merged.mounts?.[1] as Mount).source, 'source4');
assert.strictEqual(typeof merged.mounts?.[2], 'object');
assert.strictEqual((merged.mounts?.[2] as Mount).source, 'source5');
});
});
});

Expand Down

0 comments on commit f4da653

Please sign in to comment.