From 2dcae94c88de1ec05b520b4e75b83a05faa2ab99 Mon Sep 17 00:00:00 2001 From: Kaniska Date: Fri, 5 Dec 2025 16:55:58 +0000 Subject: [PATCH 1/2] Adding check to skip addition of `#syntax` directive when docker engine version < `23.0.0` --- .github/workflows/test-docker-v20.yml | 44 ++++++++++++++++ src/spec-node/containerFeatures.ts | 14 ++--- src/spec-node/devContainers.ts | 13 ++++- src/spec-node/utils.ts | 76 +-------------------------- src/spec-shutdown/dockerUtils.ts | 18 +++++++ 5 files changed, 83 insertions(+), 82 deletions(-) create mode 100644 .github/workflows/test-docker-v20.yml diff --git a/.github/workflows/test-docker-v20.yml b/.github/workflows/test-docker-v20.yml new file mode 100644 index 000000000..8160028ef --- /dev/null +++ b/.github/workflows/test-docker-v20.yml @@ -0,0 +1,44 @@ +name: Docker v20 Tests for dockerfile frontend test + +on: + push: + branches: ['main', 'directive-syntax-further-changes'] + pull_request: + branches: ['main'] + +jobs: + test-docker-v20: + name: Docker v20.10.24 Compatibility + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v5 + with: + node-version: '18.x' + + - name: Install Docker v20.10.24 + run: | + sudo apt-get remove -y docker-ce docker-ce-cli containerd.io || true + curl -fsSL https://get.docker.com -o get-docker.sh + sudo VERSION=20.10.24 sh get-docker.sh + sudo systemctl restart docker + + - name: Verify Docker version, Install and Test + run: | + # Verify + docker version + DOCKER_VERSION=$(docker version --format '{{.Server.Version}}') + if [[ ! "$DOCKER_VERSION" =~ ^20\. ]]; then + echo "ERROR: Expected Docker v20.x but got $DOCKER_VERSION" + exit 1 + fi + yarn install --frozen-lockfile + yarn type-check + yarn package + yarn test-matrix --forbid-only src/test/cli.up.test.ts + env: + CI: true + + diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index f8912254a..cc957c592 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -11,7 +11,7 @@ import { LogLevel, makeLog } from '../spec-utils/log'; import { FeaturesConfig, getContainerFeaturesBaseDockerFile, getFeatureInstallWrapperScript, getFeatureLayers, getFeatureMainValue, getFeatureValueObject, generateFeaturesConfig, Feature, generateContainerEnvs } from '../spec-configuration/containerFeaturesConfiguration'; import { readLocalFile } from '../spec-utils/pfs'; import { includeAllConfiguredFeatures } from '../spec-utils/product'; -import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, getFolderImageName, getEmptyContextFolder, SubstitutedConfig, ensureDockerHubImageAccessible } from './utils'; +import { createFeaturesTempFolder, DockerResolverParameters, getCacheFolder, getFolderImageName, getEmptyContextFolder, SubstitutedConfig } from './utils'; import { isEarlierVersion, parseVersion, runCommandNoPty } from '../spec-common/commonUtils'; import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageBuildInfoFromImage, ImageBuildInfo, ImageMetadataEntry, imageMetadataLabel, MergedDevContainerConfig } from './imageMetadata'; import { supportsBuildContexts } from './dockerfileUtils'; @@ -195,7 +195,6 @@ export interface ImageBuildOptions { async function getImageBuildOptions(params: DockerResolverParameters, config: SubstitutedConfig, dstFolder: string, baseName: string, imageBuildInfo: ImageBuildInfo): Promise { const syntax = imageBuildInfo.dockerfile?.preamble.directives.syntax; - const dockerHubAccessible = syntax ? await ensureDockerHubImageAccessible(params, 'docker/dockerfile', '1.4') : false; return { dstFolder, dockerfileContent: ` @@ -203,7 +202,7 @@ FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage ${getDevcontainerMetadataLabel(getDevcontainerMetadata(imageBuildInfo.metadata, config, { featureSets: [] }, [], getOmitDevcontainerPropertyOverride(params.common)))} `, overrideTarget: 'dev_containers_target_stage', - dockerfilePrefixContent: `${dockerHubAccessible && syntax ? `# syntax=${syntax}` : ''} + dockerfilePrefixContent: `${syntax ? `# syntax=${syntax}` : ''} ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder `, buildArgs: { @@ -242,7 +241,10 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont const useBuildKitBuildContexts = buildKitVersionParsed ? !isEarlierVersion(buildKitVersionParsed, minRequiredVersion) : false; const buildContentImageName = 'dev_container_feature_content_temp'; const disableSELinuxLabels = useBuildKitBuildContexts && await isUsingSELinuxLabels(params); - + // Access Docker engine version + const dockerEngineVersionParsed = params.dockerEngineVersion?.versionMatch ? parseVersion(params.dockerEngineVersion.versionMatch) : undefined; + const minDockerEngineVersion = [23, 0, 0]; + const skipDefaultSyntax = dockerEngineVersionParsed ? !isEarlierVersion(dockerEngineVersionParsed, minDockerEngineVersion) : false; const omitPropertyOverride = params.common.skipPersistingCustomizationsFromFeatures ? ['customizations'] : []; const imageMetadata = getDevcontainerMetadata(imageBuildInfo.metadata, devContainerConfig, featuresConfig, omitPropertyOverride, getOmitDevcontainerPropertyOverride(params.common)); const { containerUser, remoteUser } = findContainerUsers(imageMetadata, composeServiceUser, imageBuildInfo.user); @@ -265,9 +267,9 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont ; const syntax = imageBuildInfo.dockerfile?.preamble.directives.syntax; const omitSyntaxDirective = common.omitSyntaxDirective; // Can be removed when https://github.com/moby/buildkit/issues/4556 is fixed - const dockerHubAccessible = !omitSyntaxDirective ? await ensureDockerHubImageAccessible(params, 'docker/dockerfile', '1.4') : false; const dockerfilePrefixContent = `${omitSyntaxDirective ? '' : - useBuildKitBuildContexts && dockerHubAccessible && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ? '# syntax=docker/dockerfile:1.4' : + skipDefaultSyntax ? (syntax ? `# syntax=${syntax}` : '') : + useBuildKitBuildContexts && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ? '# syntax=docker/dockerfile:1.4' : syntax ? `# syntax=${syntax}` : ''} ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder `; diff --git a/src/spec-node/devContainers.ts b/src/spec-node/devContainers.ts index 493d0dc22..a6d546566 100644 --- a/src/spec-node/devContainers.ts +++ b/src/spec-node/devContainers.ts @@ -17,7 +17,7 @@ import { LogLevel, LogDimensions, toErrorText, createCombinedLog, createTerminal import { dockerComposeCLIConfig } from './dockerCompose'; import { Mount } from '../spec-configuration/containerFeaturesConfiguration'; import { getPackageConfig, PackageConfiguration } from '../spec-utils/product'; -import { dockerBuildKitVersion, isPodman } from '../spec-shutdown/dockerUtils'; +import { dockerBuildKitVersion, dockerEngineVersion, isPodman } from '../spec-shutdown/dockerUtils'; import { Event } from '../spec-utils/event'; @@ -205,6 +205,16 @@ export async function createDockerParams(options: ProvisionOptions, disposables: output, platformInfo })); + + const dockerEngineVer = await dockerEngineVersion({ + cliHost, + dockerCLI: dockerPath, + dockerComposeCLI, + env: cliHost.env, + output, + platformInfo + }); + return { common, parsedAuthority, @@ -225,6 +235,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables: updateRemoteUserUIDDefault, additionalCacheFroms: options.additionalCacheFroms, buildKitVersion, + dockerEngineVersion: dockerEngineVer, isTTY: process.stdout.isTTY || options.logFormat === 'json', experimentalLockfile, experimentalFrozenLockfile, diff --git a/src/spec-node/utils.ts b/src/spec-node/utils.ts index 0dbed5171..902888866 100644 --- a/src/spec-node/utils.ts +++ b/src/spec-node/utils.ts @@ -28,7 +28,6 @@ import { ImageMetadataEntry, MergedDevContainerConfig } from './imageMetadata'; import { getImageIndexEntryForPlatform, getManifest, getRef } from '../spec-configuration/containerCollectionsOCI'; import { requestEnsureAuthenticated } from '../spec-configuration/httpOCIRegistry'; import { configFileLabel, findDevContainer, hostFolderLabel } from './singleContainer'; -import { requestResolveHeaders } from '../spec-utils/httpRequest'; export { getConfigFilePath, getDockerfilePath, isDockerFileConfig } from '../spec-configuration/configuration'; export { uriToFsPath, parentURI } from '../spec-configuration/configurationCommonUtils'; @@ -37,12 +36,6 @@ export type BindMountConsistency = 'consistent' | 'cached' | 'delegated' | undef export type GPUAvailability = 'all' | 'detect' | 'none'; -// Constants for DockerHub registry + image access check -const DEVCONTAINER_USER_AGENT = 'devcontainer'; -const DOCKER_MANIFEST_ACCEPT_HEADER = 'application/vnd.docker.distribution.manifest.v2+json'; -const DOCKERFILE_FRONTEND_CHECK_MAX_RETRIES = 5; -const DOCKERFILE_FRONTEND_CHECK_RETRY_INTERVAL_MS = 2000; - // Generic retry function export async function retry(fn: () => Promise, options: { retryIntervalMilliseconds: number; maxRetries: number; output: Log }): Promise { const { retryIntervalMilliseconds, maxRetries, output } = options; @@ -124,6 +117,7 @@ export interface DockerResolverParameters { updateRemoteUserUIDDefault: UpdateRemoteUserUIDDefault; additionalCacheFroms: string[]; buildKitVersion: { versionString: string; versionMatch?: string } | undefined; + dockerEngineVersion: { versionString: string; versionMatch?: string } | undefined; isTTY: boolean; experimentalLockfile?: boolean; experimentalFrozenLockfile?: boolean; @@ -609,71 +603,3 @@ export function runAsyncHandler(handler: () => Promise) { } })(); } - -// Helper functions to construct DockerHub URLs -function getDockerHubAuthUrl(imageName: string, version: string): string { - return `https://auth.docker.io/token?service=registry.docker.io&scope=repository:${imageName}:pull&tag=${version}`; -} - -function getDockerHubRegistryUrl(imageName: string, version: string): string { - return `https://registry-1.docker.io/v2/${imageName}/manifests/${version}`; -} - -async function checkDockerHubImageAccessible(params: DockerResolverParameters, imageName: string, version: string): Promise { - const { output } = params.common; - - const authUrl = getDockerHubAuthUrl(imageName, version); - const registryUrl = getDockerHubRegistryUrl(imageName, version); - - const tokenRes = await requestResolveHeaders({ - type: 'GET', - url: authUrl, - headers: { 'user-agent': DEVCONTAINER_USER_AGENT } - }, output); - if (!tokenRes || tokenRes.statusCode !== 200) { - throw new Error('Token fetch failed: status ' + (tokenRes?.statusCode ?? 'unknown')); - } - - let body: any; - try { - body = JSON.parse(tokenRes.resBody.toString()); - } catch (e) { - throw new Error('Token parse failed: ' + (e instanceof Error ? e.message : String(e))); - } - const token: string | undefined = body?.token || body?.access_token; - if (!token) { - throw new Error('Token missing in auth response'); - } - - const manifestRes = await requestResolveHeaders({ - type: 'GET', - url: registryUrl, - headers: { - 'user-agent': DEVCONTAINER_USER_AGENT, - 'authorization': `Bearer ${token}`, - 'accept': DOCKER_MANIFEST_ACCEPT_HEADER - } - }, output); - if (!manifestRes || manifestRes.statusCode !== 200) { - throw new Error('Manifest fetch failed: status ' + (manifestRes?.statusCode ?? 'unknown')); - } -} - -export async function ensureDockerHubImageAccessible(params: DockerResolverParameters, imageName: string, version: string): Promise { - const { output } = params.common; - try { - await retry( - async () => { await checkDockerHubImageAccessible(params, imageName, version); }, - { maxRetries: DOCKERFILE_FRONTEND_CHECK_MAX_RETRIES, retryIntervalMilliseconds: DOCKERFILE_FRONTEND_CHECK_RETRY_INTERVAL_MS, output } - ); - output.write('Dockerfile frontend is accessible in DockerHub registry.', LogLevel.Info); - return true; - } catch (err) { - output.write( - 'Dockerfile frontend check failed after retries: ' + - (err instanceof Error ? err.message : String(err)), - LogLevel.Warning - ); - return false; - } -} diff --git a/src/spec-shutdown/dockerUtils.ts b/src/spec-shutdown/dockerUtils.ts index 10205c420..3aa4d8166 100644 --- a/src/spec-shutdown/dockerUtils.ts +++ b/src/spec-shutdown/dockerUtils.ts @@ -259,6 +259,24 @@ export async function dockerBuildKitVersion(params: DockerCLIParameters | Partia } } +export async function dockerEngineVersion(params: DockerCLIParameters | PartialExecParameters | DockerResolverParameters): Promise<{ versionString: string; versionMatch?: string } | undefined> { + try { + const execParams = { + ...toExecParameters(params), + print: true, + }; + const result = await dockerCLI(execParams, 'version', '--format', '{{.Server.Version}}'); + const versionString = result.stdout.toString().trim(); + const versionMatch = versionString.match(/(?[0-9]+)\.(?[0-9]+)\.(?[0-9]+)/); + if (!versionMatch) { + return { versionString }; + } + return { versionString, versionMatch: versionMatch[0] }; + } catch { + return undefined; + } +} + export async function dockerCLI(params: DockerCLIParameters | PartialExecParameters | DockerResolverParameters, ...args: string[]) { const partial = toExecParameters(params); return runCommandNoPty({ From 27ee594f4edd4b22c3f3691b77d215a48ca5d620 Mon Sep 17 00:00:00 2001 From: Kaniska Date: Fri, 5 Dec 2025 17:14:19 +0000 Subject: [PATCH 2/2] Updated workflow runner-image --- .github/workflows/test-docker-v20.yml | 29 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test-docker-v20.yml b/.github/workflows/test-docker-v20.yml index 8160028ef..e38817420 100644 --- a/.github/workflows/test-docker-v20.yml +++ b/.github/workflows/test-docker-v20.yml @@ -8,8 +8,8 @@ on: jobs: test-docker-v20: - name: Docker v20.10.24 Compatibility - runs-on: ubuntu-latest + name: Docker v20.10 Compatibility + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v6 @@ -18,11 +18,22 @@ jobs: with: node-version: '18.x' - - name: Install Docker v20.10.24 + - name: Install Docker v20.10 run: | sudo apt-get remove -y docker-ce docker-ce-cli containerd.io || true - curl -fsSL https://get.docker.com -o get-docker.sh - sudo VERSION=20.10.24 sh get-docker.sh + sudo apt-get update + sudo apt-get install -y \ + ca-certificates \ + curl \ + gnupg \ + lsb-release + sudo mkdir -p /etc/apt/keyrings + curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo gpg --dearmor -o /etc/apt/keyrings/docker.gpg + echo \ + "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu \ + $(lsb_release -cs) stable" | sudo tee /etc/apt/sources.list.d/docker.list > /dev/null + sudo apt-get update + sudo apt-get install -y docker-ce=5:20.10.* docker-ce-cli=5:20.10.* containerd.io sudo systemctl restart docker - name: Verify Docker version, Install and Test @@ -30,8 +41,8 @@ jobs: # Verify docker version DOCKER_VERSION=$(docker version --format '{{.Server.Version}}') - if [[ ! "$DOCKER_VERSION" =~ ^20\. ]]; then - echo "ERROR: Expected Docker v20.x but got $DOCKER_VERSION" + if [[ ! "$DOCKER_VERSION" =~ ^20\.10\. ]]; then + echo "ERROR: Expected Docker v20.10.x but got $DOCKER_VERSION" exit 1 fi yarn install --frozen-lockfile @@ -39,6 +50,4 @@ jobs: yarn package yarn test-matrix --forbid-only src/test/cli.up.test.ts env: - CI: true - - + CI: true \ No newline at end of file