Skip addition of #syntax directive when docker engine version >= 23.0.0 (#1118)

* Adding check to skip addition of `#syntax` directive when docker engine version < `23.0.0`

* Updated workflow runner-image

Author: Kaniska244

.github/workflows/test-docker-v20.yml

@@ -0,0 +1,53 @@
+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 Compatibility
+    runs-on: ubuntu-22.04
+    
+    steps:
+    - uses: actions/checkout@v6
+    
+    - uses: actions/setup-node@v5
+      with:
+        node-version: '18.x'
+    
+    - name: Install Docker v20.10
+      run: |
+        sudo apt-get remove -y docker-ce docker-ce-cli containerd.io || true
+        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
+      run: |
+        # Verify
+        docker version
+        DOCKER_VERSION=$(docker version --format '{{.Server.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
+        yarn type-check
+        yarn package
+        yarn test-matrix --forbid-only src/test/cli.up.test.ts
+      env:
+        CI: true

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,15 +195,14 @@ export interface ImageBuildOptions {
 
 async function getImageBuildOptions(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, dstFolder: string, baseName: string, imageBuildInfo: ImageBuildInfo): Promise<ImageBuildOptions> {
     const syntax = imageBuildInfo.dockerfile?.preamble.directives.syntax;
-    const dockerHubAccessible = syntax ? await ensureDockerHubImageAccessible(params, 'docker/dockerfile', '1.4') : false;
     return {
         dstFolder,
         dockerfileContent: `
 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
 `;

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,

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<T>(fn: () => Promise<T>, options: { retryIntervalMilliseconds: number; maxRetries: number; output: Log }): Promise<T> {
 	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<void>) {
 		}
 	})();
 }
-
-// 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<void> {
-  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<boolean> {
-  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;
-  }
-}

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(/(?<major>[0-9]+)\.(?<minor>[0-9]+)\.(?<patch>[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({