Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

User env variables for features #228

Merged
merged 1 commit into from
Oct 24, 2022
Merged

User env variables for features #228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/spec-configuration/containerFeaturesConfiguration.ts
View file
Open in desktop
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 && \\
chrmarti marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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