Skip to content

Commit

Permalink
User env variables for features (devcontainers/spec#91)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrmarti committed Oct 24, 2022
1 parent c3b89ee commit 01a956e
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 22 deletions.
9 changes: 7 additions & 2 deletions src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ echo '${optionsIndented}'
echo ===========================================================================
set -a
. ../devcontainer-features.builtin.env
. ./devcontainer-features.env
set +a
Expand All @@ -274,8 +275,12 @@ function escapeQuotesForShell(input: string) {
return input.replace(new RegExp(`'`, 'g'), `'\\''`);
}

export function getFeatureLayers(featuresConfig: FeaturesConfig) {
let result = '';
export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string) {
let result = `RUN \\
echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
`;

// Features version 1
const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId);
Expand Down
32 changes: 24 additions & 8 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { readLocalFile } from '../spec-utils/pfs';
import { includeAllConfiguredFeatures } from '../spec-utils/product';
import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, getFolderImageName, getEmptyContextFolder, SubstitutedConfig } from './utils';
import { isEarlierVersion, parseVersion } from '../spec-common/commonUtils';
import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, MergedDevContainerConfig } from './imageMetadata';
import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, ImageMetadataEntry, MergedDevContainerConfig } from './imageMetadata';
import { supportsBuildContexts } from './dockerfileUtils';

// Escapes environment variable keys.
Expand All @@ -34,7 +34,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
const { cliHost, output } = common;

const imageBuildInfo = await getImageBuildInfoFromImage(params, imageName, config.substitute, common.experimentalImageMetadata);
const extendImageDetails = await getExtendImageBuildInfo(params, config, imageName, imageBuildInfo, additionalFeatures);
const extendImageDetails = await getExtendImageBuildInfo(params, config, imageName, imageBuildInfo, undefined, additionalFeatures);
if (!extendImageDetails || !extendImageDetails.featureBuildInfo) {
// no feature extensions - return
return {
Expand Down Expand Up @@ -94,7 +94,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
};
}

export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, baseName: string, imageBuildInfo: ImageBuildInfo, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>) {
export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, baseName: string, imageBuildInfo: ImageBuildInfo, composeServiceUser: string | undefined, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>) {

// Creates the folder where the working files will be setup.
const dstFolder = await createFeaturesTempFolder(params.common);
Expand All @@ -109,7 +109,7 @@ export async function getExtendImageBuildInfo(params: DockerResolverParameters,
}

// Generates the end configuration.
const featureBuildInfo = await getFeaturesBuildOptions(params, config, featuresConfig, baseName, imageBuildInfo);
const featureBuildInfo = await getFeaturesBuildOptions(params, config, featuresConfig, baseName, imageBuildInfo, composeServiceUser);
if (!featureBuildInfo) {
return undefined;
}
Expand Down Expand Up @@ -191,7 +191,7 @@ function getImageBuildOptions(params: DockerResolverParameters, config: Substitu
dstFolder,
dockerfileContent: `
FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage
${getDevcontainerMetadataLabel(imageBuildInfo.metadata, config, { featureSets: [] }, params.common.experimentalImageMetadata)}
${getDevcontainerMetadataLabel(getDevcontainerMetadata(imageBuildInfo.metadata, config, { featureSets: [] }), params.common.experimentalImageMetadata)}
`,
overrideTarget: 'dev_containers_target_stage',
dockerfilePrefixContent: `
Expand All @@ -204,7 +204,7 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
};
}

async function getFeaturesBuildOptions(params: DockerResolverParameters, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig, baseName: string, imageBuildInfo: ImageBuildInfo): Promise<ImageBuildOptions | undefined> {
async function getFeaturesBuildOptions(params: DockerResolverParameters, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig, baseName: string, imageBuildInfo: ImageBuildInfo, composeServiceUser: string | undefined): Promise<ImageBuildOptions | undefined> {
const { common } = params;
const { cliHost, output } = common;
const { dstFolder } = featuresConfig;
Expand Down Expand Up @@ -239,17 +239,26 @@ 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 { containerUser, remoteUser } = findContainerUsers(imageMetadata, composeServiceUser, imageBuildInfo.user);
const builtinVariables = [
`_CONTAINER_USER=${containerUser}`,
`_REMOTE_USER=${remoteUser}`,
];
const envPath = cliHost.path.join(dstFolder, 'devcontainer-features.builtin.env');
await cliHost.writeFile(envPath, Buffer.from(builtinVariables.join('\n') + '\n'));

// When copying via buildkit, the content is accessed via '.' (i.e. in the context root)
// When copying via temp image, the content is in '/tmp/build-features'
const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features/';
const dockerfile = getContainerFeaturesBaseDockerFile()
.replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`)
.replace('{contentSourceRootPath}', contentSourceRootPath)
.replace('#{featureBuildStages}', getFeatureBuildStages(featuresConfig, buildStageScripts, contentSourceRootPath))
.replace('#{featureLayer}', getFeatureLayers(featuresConfig))
.replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser))
.replace('#{containerEnv}', generateContainerEnvs(featuresConfig))
.replace('#{copyFeatureBuildStages}', getCopyFeatureBuildStages(featuresConfig, buildStageScripts))
.replace('#{devcontainerMetadata}', getDevcontainerMetadataLabel(imageBuildInfo.metadata, devContainerConfig, featuresConfig, common.experimentalImageMetadata))
.replace('#{devcontainerMetadata}', getDevcontainerMetadataLabel(imageMetadata, common.experimentalImageMetadata))
;
const syntax = imageBuildInfo.dockerfile?.preamble.directives.syntax;
const dockerfilePrefixContent = `${useBuildKitBuildContexts && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ?
Expand Down Expand Up @@ -338,6 +347,13 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
};
}

export function findContainerUsers(imageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, composeServiceUser: string | undefined, imageUser: string) {
const reversed = imageMetadata.config.slice().reverse();
const containerUser = reversed.find(entry => entry.containerUser)?.containerUser || composeServiceUser || imageUser;
const remoteUser = reversed.find(entry => entry.remoteUser)?.remoteUser || containerUser;
return { containerUser, remoteUser };
}

function getFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScripts: Record<string, { hasAcquire: boolean; hasConfigure: boolean } | undefined>[], contentSourceRootPath: string) {
return ([] as string[]).concat(...featuresConfig.featureSets
.map((featureSet, i) => featureSet.features
Expand Down
2 changes: 1 addition & 1 deletion src/spec-node/dockerCompose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ export async function buildAndExtendDockerCompose(configWithRaw: SubstitutedConf
// determine whether we need to extend with features
const noBuildKitParams = { ...params, buildKitVersion: null }; // skip BuildKit -> can't set additional build contexts with compose
const imageBuildInfo = await getImageBuildInfoFromDockerfile(params, originalDockerfile, serviceInfo.build?.args || {}, serviceInfo.build?.target, configWithRaw.substitute, common.experimentalImageMetadata);
const extendImageBuildInfo = await getExtendImageBuildInfo(noBuildKitParams, configWithRaw, baseName, imageBuildInfo, additionalFeatures);
const extendImageBuildInfo = await getExtendImageBuildInfo(noBuildKitParams, configWithRaw, baseName, imageBuildInfo, composeService.user, additionalFeatures);

let overrideImageName: string | undefined;
let buildOverrideContent = '';
Expand Down
4 changes: 2 additions & 2 deletions src/spec-node/imageMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,11 @@ function internalGetImageMetadata0(imageDetails: ImageDetails | ContainerDetails
return [];
}

export function getDevcontainerMetadataLabel(baseImageMetadata: SubstitutedConfig<ImageMetadataEntry[]>, devContainerConfig: SubstitutedConfig<DevContainerConfig>, featuresConfig: FeaturesConfig, experimentalImageMetadata: boolean) {
export function getDevcontainerMetadataLabel(devContainerMetadata: SubstitutedConfig<ImageMetadataEntry[]>, experimentalImageMetadata: boolean) {
if (!experimentalImageMetadata) {
return '';
}
const metadata = getDevcontainerMetadata(baseImageMetadata, devContainerConfig, featuresConfig).raw;
const metadata = devContainerMetadata.raw;
if (!metadata.length) {
return '';
}
Expand Down
2 changes: 1 addition & 1 deletion src/spec-node/singleContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
}

const imageBuildInfo = await getImageBuildInfoFromDockerfile(buildParams, originalDockerfile, config.build?.args || {}, config.build?.target, configWithRaw.substitute, buildParams.common.experimentalImageMetadata);
const extendImageBuildInfo = await getExtendImageBuildInfo(buildParams, configWithRaw, baseName, imageBuildInfo, additionalFeatures);
const extendImageBuildInfo = await getExtendImageBuildInfo(buildParams, configWithRaw, baseName, imageBuildInfo, undefined, additionalFeatures);

let finalDockerfilePath = dockerfilePath;
const additionalBuildArgs: string[] = [];
Expand Down
71 changes: 69 additions & 2 deletions src/test/container-features/featureHelpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import * as path from 'path';
import { DevContainerFeature } from '../../spec-configuration/configuration';
import { OCIRef } from '../../spec-configuration/containerCollectionsOCI';
import { Feature, FeatureSet, getBackwardCompatibleFeatureId, getFeatureInstallWrapperScript, processFeatureIdentifier } from '../../spec-configuration/containerFeaturesConfiguration';
import { getSafeId } from '../../spec-node/containerFeatures';
import { getSafeId, findContainerUsers } from '../../spec-node/containerFeatures';
import { ImageMetadataEntry } from '../../spec-node/imageMetadata';
import { SubstitutedConfig } from '../../spec-node/utils';
import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';

export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
Expand Down Expand Up @@ -487,6 +489,7 @@ echo ''
echo ===========================================================================
set -a
. ../devcontainer-features.builtin.env
. ./devcontainer-features.env
set +a
Expand Down Expand Up @@ -566,6 +569,7 @@ echo ' VERSION=latest
echo ===========================================================================
set -a
. ../devcontainer-features.builtin.env
. ./devcontainer-features.env
set +a
Expand All @@ -576,4 +580,67 @@ chmod +x ./install.sh
const actual = getFeatureInstallWrapperScript(feature, set, options);
assert.equal(actual, expected);
});
});
});

describe('findContainerUsers', () => {
it('returns last metadata containerUser as containerUser and remoteUser', () => {
assert.deepEqual(findContainerUsers(configWithRaw([
{
containerUser: 'metadataTestUser1',
},
{
containerUser: 'metadataTestUser2',
},
]), 'composeTestUser', 'imageTestUser'), {
containerUser: 'metadataTestUser2',
remoteUser: 'metadataTestUser2',
});
});
it('returns compose service user as containerUser and remoteUser', () => {
assert.deepEqual(findContainerUsers(configWithRaw<ImageMetadataEntry[]>([
{
remoteEnv: { foo: 'bar' },
},
{
remoteEnv: { bar: 'baz' },
},
]), 'composeTestUser', 'imageTestUser'), {
containerUser: 'composeTestUser',
remoteUser: 'composeTestUser',
});
});
it('returns image user as containerUser and remoteUser', () => {
assert.deepEqual(findContainerUsers(configWithRaw<ImageMetadataEntry[]>([
{
remoteEnv: { foo: 'bar' },
},
{
remoteEnv: { bar: 'baz' },
},
]), undefined, 'imageTestUser'), {
containerUser: 'imageTestUser',
remoteUser: 'imageTestUser',
});
});
it('returns last metadata remoteUser', () => {
assert.deepEqual(findContainerUsers(configWithRaw([
{
remoteUser: 'metadataTestUser1',
},
{
remoteUser: 'metadataTestUser2',
},
]), 'composeTestUser', 'imageTestUser'), {
containerUser: 'composeTestUser',
remoteUser: 'metadataTestUser2',
});
});
});

function configWithRaw<T>(config: T): SubstitutedConfig<T> {
return {
config,
raw: config,
substitute: config => config,
};
}
16 changes: 12 additions & 4 deletions src/test/container-features/generateFeaturesConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ describe('validate generateFeaturesConfig()', function () {
// assert.strictEqual(actualEnvs, expectedEnvs);

// getFeatureLayers
const actualLayers = getFeatureLayers(featuresConfig);
const expectedLayers = `RUN cd /tmp/build-features/first_1 \\
const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser');
const expectedLayers = `RUN \\
echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
RUN cd /tmp/build-features/first_1 \\
&& chmod +x ./install.sh \\
&& ./install.sh
Expand Down Expand Up @@ -122,8 +126,12 @@ RUN cd /tmp/build-features/second_2 \\
// -- Test containerFeatures.ts helper functions

// getFeatureLayers
const actualLayers = getFeatureLayers(featuresConfig);
const expectedLayers = `
const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser');
const expectedLayers = `RUN \\
echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
RUN cd /tmp/build-features/color_3 \\
&& chmod +x ./devcontainer-features-install.sh \\
&& ./devcontainer-features-install.sh
Expand Down
4 changes: 2 additions & 2 deletions src/test/imageMetadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('Image Metadata', function () {
});

it('should create label for Dockerfile', () => {
const label = getDevcontainerMetadataLabel(configWithRaw([
const label = getDevcontainerMetadataLabel(getDevcontainerMetadata(configWithRaw([
{
id: 'baseFeature',
}
Expand All @@ -194,7 +194,7 @@ describe('Image Metadata', function () {
value: 'someValue',
included: true,
}
]), true);
])), true);
const expected = [
{
id: 'baseFeature',
Expand Down

0 comments on commit 01a956e

Please sign in to comment.