Skip to content

Commit 77172a2

Browse files
Merge pull request #392 from devcontainers/samruddhikhandale/containerenv-bug
Bug fix: Merge metadata logic for containerEnv - devcontainer build
2 parents 8e8c92c + dac6ee3 commit 77172a2

6 files changed

Lines changed: 53 additions & 1 deletion

File tree

src/spec-configuration/containerFeaturesConfiguration.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,8 @@ ARG _DEV_CONTAINERS_IMAGE_USER=root
245245
USER $_DEV_CONTAINERS_IMAGE_USER
246246
247247
#{devcontainerMetadata}
248+
249+
#{containerEnvMetadata}
248250
`;
249251
}
250252

src/spec-node/containerFeatures.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { readLocalFile } from '../spec-utils/pfs';
1515
import { includeAllConfiguredFeatures } from '../spec-utils/product';
1616
import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, getFolderImageName, getEmptyContextFolder, SubstitutedConfig } from './utils';
1717
import { isEarlierVersion, parseVersion } from '../spec-common/commonUtils';
18-
import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, ImageMetadataEntry, imageMetadataLabel, MergedDevContainerConfig } from './imageMetadata';
18+
import { getContainerEnvMetadata, getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, ImageMetadataEntry, imageMetadataLabel, MergedDevContainerConfig } from './imageMetadata';
1919
import { supportsBuildContexts } from './dockerfileUtils';
2020
import { ContainerError } from '../spec-common/errors';
2121

@@ -293,6 +293,7 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
293293
.replace('#{containerEnv}', generateContainerEnvs(featuresConfig))
294294
.replace('#{copyFeatureBuildStages}', getCopyFeatureBuildStages(featuresConfig, buildStageScripts))
295295
.replace('#{devcontainerMetadata}', getDevcontainerMetadataLabel(imageMetadata, common.experimentalImageMetadata))
296+
.replace('#{containerEnvMetadata}', getContainerEnvMetadata(devContainerConfig.config.containerEnv))
296297
;
297298
const syntax = imageBuildInfo.dockerfile?.preamble.directives.syntax;
298299
const dockerfilePrefixContent = `${useBuildKitBuildContexts && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ?

src/spec-node/imageMetadata.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,3 +443,13 @@ function toLabelString(obj: object) {
443443
return JSON.stringify(obj)
444444
.replace(/(?=["\\$])/g, '\\');
445445
}
446+
447+
export function getContainerEnvMetadata(containerEnv: Record<string, string> | undefined): string {
448+
if (!containerEnv) {
449+
return '';
450+
}
451+
452+
const keys = Object.keys(containerEnv);
453+
const concatenatedEnv = keys.map(k => `ENV ${k}=${containerEnv![k]}`).join('\n');
454+
return concatenatedEnv;
455+
}

src/test/cli.build.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,5 +222,20 @@ describe('Dev Containers CLI', function () {
222222
await shellExec(`docker buildx rm ${builderName}`);
223223
}
224224
});
225+
226+
it('should follow the correct merge logic for containerEnv', async () => {
227+
const res = await shellExec(`${cli} build --workspace-folder ${__dirname}/configs/image-metadata-containerEnv --image-name "test-metadata"`);
228+
const response = JSON.parse(res.stdout);
229+
assert.equal(response.outcome, 'success');
230+
231+
const resRun = await shellExec(`docker run -it -d "test-metadata"`);
232+
const containerId: string = resRun.stdout.split('\n')[0];
233+
assert.ok(containerId, 'Container id not found.');
234+
235+
const result = await shellExec(`docker exec ${containerId} bash -c 'echo $JAVA_HOME'`);
236+
assert.equal('/usr/lib/jvm/msopenjdk-current\n', result.stdout);
237+
238+
await shellExec(`docker rm -f ${containerId}`);
239+
});
225240
});
226241
});

src/test/cli.up.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,5 +165,18 @@ describe('Dev Containers CLI', function () {
165165
});
166166
});
167167
});
168+
169+
it('should follow the correct merge logic for containerEnv', async () => {
170+
const res = await shellExec(`${cli} up --workspace-folder ${__dirname}/configs/image-metadata-containerEnv`);
171+
const response = JSON.parse(res.stdout);
172+
assert.equal(response.outcome, 'success');
173+
const containerId: string = response.containerId;
174+
assert.ok(containerId, 'Container id not found.');
175+
176+
const result = await shellExec(`docker exec ${containerId} bash -c 'echo $JAVA_HOME'`);
177+
assert.equal('/usr/lib/jvm/msopenjdk-current\n', result.stdout);
178+
179+
await shellExec(`docker rm -f ${containerId}`);
180+
});
168181
});
169182
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"image": "mcr.microsoft.com/devcontainers/base:ubuntu",
3+
"features": {
4+
"ghcr.io/devcontainers/features/java:1": {
5+
"version": "none"
6+
}
7+
},
8+
"containerEnv": {
9+
"JAVA_HOME": "/usr/lib/jvm/msopenjdk-current"
10+
}
11+
}

0 commit comments

Comments
 (0)