Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
"integrity": "sha256:ce078b7bf7d9ef3bcb9813b32103795d8d72172446890b64772cbe1dec6baafd"
}
}
}
}
2 changes: 1 addition & 1 deletion scripts/updateUID.Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
ARG BASE_IMAGE
ARG BASE_IMAGE=placeholder
FROM $BASE_IMAGE

USER root
Expand Down
2 changes: 2 additions & 0 deletions src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string

#{nonBuildKitFeatureContentFallback}

ARG _DEV_CONTAINERS_BASE_IMAGE=scratch

FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize
USER root
COPY --from=dev_containers_feature_content_source ${path.posix.join(contentSourceRootPath, 'devcontainer-features.builtin.env')} /tmp/build-features/
Expand Down
4 changes: 2 additions & 2 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ ${getDevcontainerMetadataLabel(getDevcontainerMetadata(imageBuildInfo.metadata,
`,
overrideTarget: 'dev_containers_target_stage',
dockerfilePrefixContent: `${syntax ? `# syntax=${syntax}` : ''}
ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
`,
buildArgs: {
_DEV_CONTAINERS_BASE_IMAGE: baseName,
Expand Down Expand Up @@ -274,7 +274,7 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
skipDefaultSyntax ? (syntax ? `# syntax=${syntax}` : '') :
useBuildKitBuildContexts && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ? '# syntax=docker/dockerfile:1.4' :
syntax ? `# syntax=${syntax}` : ''}
ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
`;

// Build devcontainer-features.env and devcontainer-features-install.sh file(s) for each features source folder
Expand Down
52 changes: 50 additions & 2 deletions src/test/container-features/generateFeaturesConfig.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { assert } from 'chai';
import { generateFeaturesConfig, getFeatureLayers, FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration';
import { generateFeaturesConfig, getFeatureLayers, getContainerFeaturesBaseDockerFile, FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration';
import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';
import * as path from 'path';
import * as process from 'process';
import * as os from 'os';
import * as crypto from 'crypto';
import { mkdirpLocal } from '../../spec-utils/pfs';
import { mkdirpLocal, readLocalFile } from '../../spec-utils/pfs';
import { DevContainerConfig } from '../../spec-configuration/configuration';
import { URI } from 'vscode-uri';
import { getLocalCacheFolder } from '../../spec-node/utils';
Expand Down Expand Up @@ -135,4 +135,52 @@ RUN chmod -R 0755 /tmp/dev-container-features/hello_1 \\
const javaSettings = java?.features[0]?.customizations?.vscode?.settings;
assert.isObject(javaSettings);
});
});

// returns the names of any ARGs used in a `FROM` that lack a default declared before them.
function findFromArgsWithoutDefault(dockerfile: string): string[] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not also want to use this for other features?

const argsWithDefault = new Set<string>();
const offenders: string[] = [];

for (const raw of dockerfile.split('\n')) {
const line = raw.trim();

// ARG NAME=value -> has a default
const argWithDefault = /^ARG\s+([A-Za-z0-9_]+)\s*=\s*\S+/.exec(line);
if (argWithDefault) {
argsWithDefault.add(argWithDefault[1]);
continue;
}
// ARG NAME -> no default (does NOT satisfy the linter)
if (/^ARG\s+[A-Za-z0-9_]+\s*$/.test(line)) {
continue;
}
// FROM $NAME or FROM ${NAME...}
const fromMatch = /^FROM\s+\$\{?([A-Za-z0-9_]+)/.exec(line);
if (fromMatch && !argsWithDefault.has(fromMatch[1])) {
offenders.push(fromMatch[1]);
}
}
return offenders;
}

describe('validate generated Dockerfiles avoid InvalidDefaultArgInFrom', function () {

it('feature base Dockerfile declares _DEV_CONTAINERS_BASE_IMAGE with a default before FROM', function () {
const dockerfile = getContainerFeaturesBaseDockerFile('/tmp/build-features');
assert.match(dockerfile, /ARG _DEV_CONTAINERS_BASE_IMAGE=\S+/, 'ARG should have a default value');
assert.match(dockerfile, /FROM \$_DEV_CONTAINERS_BASE_IMAGE\b/, 'FROM should reference the ARG directly');
assert.notMatch(dockerfile, /FROM \$\{_DEV_CONTAINERS_BASE_IMAGE:-/, 'should not rely on ${VAR:-default} shell fallback');
assert.deepStrictEqual(findFromArgsWithoutDefault(dockerfile), []);
});

it('validate that updateUID.Dockerfile declares BASE_IMAGE with a default before FROM', async function () {
const content = (await readLocalFile('scripts/updateUID.Dockerfile')).toString();
assert.match(content, /ARG BASE_IMAGE=\S+/, 'BASE_IMAGE ARG should have a default value');
assert.deepStrictEqual(
findFromArgsWithoutDefault(content),
[],
'updateUID.Dockerfile FROM references an ARG without a default'
);
});
});