From 4aa0e550f305089f688adecaa127695308a874e1 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Mon, 28 Nov 2022 09:30:57 +0100 Subject: [PATCH 1/8] Include config (#299) --- src/spec-node/containerFeatures.ts | 2 +- .../.devcontainer/devcontainer.json | 1 + src/test/configs/image/.devcontainer.json | 1 + src/test/imageMetadata.test.ts | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 37586ea76..50cb419c2 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -39,7 +39,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs // no feature extensions - return return { updatedImageName: [imageName], - imageMetadata: imageBuildInfo.metadata, + imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, config, extendImageDetails?.featuresConfig), imageDetails: async () => imageBuildInfo.imageDetails, labels: extendImageDetails?.labels, }; diff --git a/src/test/configs/compose-image-without-features-minimal/.devcontainer/devcontainer.json b/src/test/configs/compose-image-without-features-minimal/.devcontainer/devcontainer.json index 8f240d13a..0d78429ce 100644 --- a/src/test/configs/compose-image-without-features-minimal/.devcontainer/devcontainer.json +++ b/src/test/configs/compose-image-without-features-minimal/.devcontainer/devcontainer.json @@ -2,6 +2,7 @@ "dockerComposeFile": "docker-compose.yml", "service": "app", "workspaceFolder": "/workspace", + "postCreateCommand": "touch /postCreateCommand.txt", "remoteEnv": { "TEST": "ENV" } diff --git a/src/test/configs/image/.devcontainer.json b/src/test/configs/image/.devcontainer.json index d04e6f2c7..0cb2f1701 100644 --- a/src/test/configs/image/.devcontainer.json +++ b/src/test/configs/image/.devcontainer.json @@ -1,5 +1,6 @@ { "image": "ubuntu:latest", + "postCreateCommand": "touch /postCreateCommand.txt", "remoteEnv": { "LOCAL_PATH": "${localEnv:PATH}", "CONTAINER_PATH": "${containerEnv:PATH}" diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 8e70a7d25..b4dc52ac1 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -156,6 +156,7 @@ describe('Image Metadata', function () { const metadata = internalGetImageMetadata0(details, true, nullLog); assert.strictEqual(metadata.length, 1); assert.ok(metadata[0].remoteEnv); + await shellExec(`docker exec ${response.containerId} test -f /postCreateCommand.txt`); await shellExec(`docker rm -f ${response.containerId}`); }); }); From 6f76c9518230915ca256320573c891f2f305c537 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 28 Nov 2022 07:21:55 -0800 Subject: [PATCH 2/8] Emit response from registry on failed `postUploadSessionId` (#298) * requestResolveHeaders() also returns responseBody * print responseBody for "postUploadSessionId" on error * missing semicolon * one more semicolon --- .../containerCollectionsOCIPush.ts | 9 ++++-- src/spec-utils/httpRequest.ts | 30 +++++++++++-------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/spec-configuration/containerCollectionsOCIPush.ts b/src/spec-configuration/containerCollectionsOCIPush.ts index 575597240..eabf1df57 100644 --- a/src/spec-configuration/containerCollectionsOCIPush.ts +++ b/src/spec-configuration/containerCollectionsOCIPush.ts @@ -311,7 +311,7 @@ async function postUploadSessionId(output: Log, ociRef: OCIRef | OCICollectionRe const url = `https://${ociRef.registry}/v2/${ociRef.path}/blobs/uploads/`; output.write(`Generating Upload URL -> ${url}`, LogLevel.Trace); - const { statusCode, resHeaders } = await requestResolveHeaders({ type: 'POST', url, headers }, output); + const { statusCode, resHeaders, resBody } = await requestResolveHeaders({ type: 'POST', url, headers }, output); output.write(`${url}: ${statusCode}`, LogLevel.Trace); if (statusCode === 202) { const locationHeader = resHeaders['location'] || resHeaders['Location']; @@ -320,8 +320,13 @@ async function postUploadSessionId(output: Log, ociRef: OCIRef | OCICollectionRe return undefined; } return locationHeader; + } else { + // Any other statusCode besides 202 is unexpected + // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#error-codes + const displayResBody = resBody ? ` -> ${resBody}` : ''; + output.write(`${url}: Unexpected status code '${statusCode}'${displayResBody}`, LogLevel.Error); + return undefined; } - return undefined; } export async function calculateManifestAndContentDigest(output: Log, dataLayer: OCILayer, annotations: { [key: string]: string } | undefined) { diff --git a/src/spec-utils/httpRequest.ts b/src/spec-utils/httpRequest.ts index dee65e8cc..62c16be1a 100644 --- a/src/spec-utils/httpRequest.ts +++ b/src/spec-utils/httpRequest.ts @@ -65,9 +65,10 @@ export function headRequest(options: { url: string; headers: Record; data?: Buffer }, _output?: Log) { - return new Promise<{ statusCode: number; resHeaders: Record }>((resolve, reject) => { + return new Promise<{ statusCode: number; resHeaders: Record; resBody: Buffer }>((resolve, reject) => { const parsed = new url.URL(options.url); const reqOptions: RequestOptions = { hostname: parsed.hostname, @@ -79,16 +80,21 @@ export function requestResolveHeaders(options: { type: string; url: string; head }; const req = https.request(reqOptions, res => { res.on('error', reject); - const result = { - statusCode: res.statusCode!, - resHeaders: res.headers! as Record - }; - resolve(result); + + // Resolve response body + const chunks: Buffer[] = []; + res.on('data', chunk => chunks.push(chunk as Buffer)); + res.on('end', () => { + resolve({ + statusCode: res.statusCode!, + resHeaders: res.headers! as Record, + resBody: Buffer.concat(chunks) + }); + }); + if (options.data) { + req.write(options.data); + } + req.end(); }); - req.on('error', reject); - if (options.data) { - req.write(options.data); - } - req.end(); }); } \ No newline at end of file From 4030a28e140d928a1125dde723451025ca8848b1 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 28 Nov 2022 07:22:04 -0800 Subject: [PATCH 3/8] downcase OCI identifiers and validate input of getRef() (#293) * validate input of getRef() * update tests using getRef() * add unit tests * Refactor getRef() * add more test cases * dregex match entire string --- .../containerCollectionsOCI.ts | 55 ++++++-- .../containerFeaturesConfiguration.ts | 5 +- .../containerFeaturesOCI.ts | 10 +- .../containerTemplatesOCI.ts | 3 + .../publishCommandImpl.ts | 12 +- src/spec-node/featuresCLI/info.ts | 8 ++ src/spec-node/featuresCLI/publish.ts | 15 ++- src/spec-node/templatesCLI/publish.ts | 5 + src/spec-node/utils.ts | 3 + .../containerFeaturesOCI.test.ts | 119 ++++++++++++++++++ .../containerFeaturesOCIPush.test.ts | 3 + .../featuresCLICommands.test.ts | 3 + 12 files changed, 220 insertions(+), 21 deletions(-) diff --git a/src/spec-configuration/containerCollectionsOCI.ts b/src/spec-configuration/containerCollectionsOCI.ts index 7f95785d0..9a23f0e8a 100644 --- a/src/spec-configuration/containerCollectionsOCI.ts +++ b/src/spec-configuration/containerCollectionsOCI.ts @@ -58,13 +58,38 @@ interface OCITagList { tags: string[]; } -export function getRef(output: Log, resourceAndVersion: string): OCIRef { - - // ex: ghcr.io/codspace/features/ruby:1 - // ex: ghcr.io/codspace/templates/ruby:1 - const splitOnColon = resourceAndVersion.split(':'); - const resource = splitOnColon[0]; - const version = splitOnColon[1] ? splitOnColon[1] : 'latest'; +// Following Spec: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests +// Alternative Spec: https://docs.docker.com/registry/spec/api/#overview +// +// Entire path ('namespace' in spec terminology) for the given repository +// (eg: devcontainers/features/go) +const regexForPath = /^[a-z0-9]+([._-][a-z0-9]+)*(\/[a-z0-9]+([._-][a-z0-9]+)*)*$/; +// MUST be either (a) the digest of the manifest or (b) a tag +// MUST be at most 128 characters in length and MUST match the following regular expression: +const regexForReference = /^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$/; + +// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests +// Attempts to parse the given string into an OCIRef +export function getRef(output: Log, input: string): OCIRef | undefined { + // Normalize input by downcasing entire string + input = input.toLowerCase(); + + const indexOfLastColon = input.lastIndexOf(':'); + + let resource = ''; + let version = ''; // TODO: Support parsing out manifest digest (...@sha256:...) + + // 'If' condition is true in the following cases: + // 1. The final colon is before the first slash (a port) : eg: ghcr.io:8081/codspace/features/ruby + // 2. There is no version : eg: ghcr.io/codspace/features/ruby + // In both cases, assume 'latest' tag. + if (indexOfLastColon === -1 || indexOfLastColon < input.indexOf('/')) { + resource = input; + version = 'latest'; + } else { + resource = input.substring(0, indexOfLastColon); + version = input.substring(indexOfLastColon + 1); + } const splitOnSlash = resource.split('/'); @@ -79,10 +104,22 @@ export function getRef(output: Log, resourceAndVersion: string): OCIRef { output.write(`id: ${id}`, LogLevel.Trace); output.write(`version: ${version}`, LogLevel.Trace); output.write(`owner: ${owner}`, LogLevel.Trace); - output.write(`namespace: ${namespace}`, LogLevel.Trace); + output.write(`namespace: ${namespace}`, LogLevel.Trace); // TODO: We assume 'namespace' includes at least one slash (eg: 'devcontainers/features') output.write(`registry: ${registry}`, LogLevel.Trace); output.write(`path: ${path}`, LogLevel.Trace); + // Validate results of parse. + + if (!regexForPath.exec(path)) { + output.write(`Parsed path '${path}' for input '${input}' failed validation.`, LogLevel.Error); + return undefined; + } + + if (!regexForReference.test(version)) { + output.write(`Parsed version '${version}' for input '${input}' failed validation.`, LogLevel.Error); + return undefined; + } + return { id, version, @@ -229,7 +266,7 @@ export async function getPublishedVersions(ref: OCIRef, output: Log, sorted: boo let authToken = await fetchRegistryAuthToken(output, ref.registry, ref.path, process.env, 'pull'); if (!authToken) { - output.write(`(!) ERR: Failed to publish ${collectionType}: ${ref.resource}`, LogLevel.Error); + output.write(`(!) ERR: Failed to get published versions for ${collectionType}: ${ref.resource}`, LogLevel.Error); return undefined; } diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 873b87027..906ec6d52 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -12,7 +12,7 @@ import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal, import { Log, LogLevel } from '../spec-utils/log'; import { request } from '../spec-utils/httpRequest'; import { computeFeatureInstallationOrder } from './containerFeaturesOrder'; -import { fetchOCIFeature, getOCIFeatureSet, fetchOCIFeatureManifestIfExistsFromUserIdentifier } from './containerFeaturesOCI'; +import { fetchOCIFeature, tryGetOCIFeatureSet, fetchOCIFeatureManifestIfExistsFromUserIdentifier } from './containerFeaturesOCI'; import { OCIManifest, OCIRef } from './containerCollectionsOCI'; // v1 @@ -769,8 +769,7 @@ export async function processFeatureIdentifier(output: Log, configPath: string, // (6) Oci Identifier if (type === 'oci' && manifest) { - let newFeaturesSet: FeatureSet = getOCIFeatureSet(output, userFeature.id, userFeature.options, manifest, originalUserFeatureId); - return newFeaturesSet; + return tryGetOCIFeatureSet(output, userFeature.id, userFeature.options, manifest, originalUserFeatureId); } output.write(`Github feature.`); diff --git a/src/spec-configuration/containerFeaturesOCI.ts b/src/spec-configuration/containerFeaturesOCI.ts index f6117b280..91bc1c5f9 100644 --- a/src/spec-configuration/containerFeaturesOCI.ts +++ b/src/spec-configuration/containerFeaturesOCI.ts @@ -2,9 +2,12 @@ import { Log, LogLevel } from '../spec-utils/log'; import { Feature, FeatureSet } from './containerFeaturesConfiguration'; import { fetchOCIManifestIfExists, getBlob, getRef, OCIManifest } from './containerCollectionsOCI'; -export function getOCIFeatureSet(output: Log, identifier: string, options: boolean | string | Record, manifest: OCIManifest, originalUserFeatureId: string): FeatureSet { - +export function tryGetOCIFeatureSet(output: Log, identifier: string, options: boolean | string | Record, manifest: OCIManifest, originalUserFeatureId: string): FeatureSet | undefined { const featureRef = getRef(output, identifier); + if (!featureRef) { + output.write(`Unable to parse '${identifier}'`, LogLevel.Error); + return undefined; + } const feat: Feature = { id: featureRef.id, @@ -30,6 +33,9 @@ export function getOCIFeatureSet(output: Log, identifier: string, options: boole export async function fetchOCIFeatureManifestIfExistsFromUserIdentifier(output: Log, env: NodeJS.ProcessEnv, identifier: string, manifestDigest?: string, authToken?: string): Promise { const featureRef = getRef(output, identifier); + if (!featureRef) { + return undefined; + } return await fetchOCIManifestIfExists(output, env, featureRef, manifestDigest, authToken); } diff --git a/src/spec-configuration/containerTemplatesOCI.ts b/src/spec-configuration/containerTemplatesOCI.ts index 84a332400..98a9f93c9 100644 --- a/src/spec-configuration/containerTemplatesOCI.ts +++ b/src/spec-configuration/containerTemplatesOCI.ts @@ -122,6 +122,9 @@ export async function fetchTemplate(output: Log, selectedTemplate: SelectedTempl async function fetchOCITemplateManifestIfExistsFromUserIdentifier(output: Log, env: NodeJS.ProcessEnv, identifier: string, manifestDigest?: string, authToken?: string): Promise { const templateRef = getRef(output, identifier); + if (!templateRef) { + return undefined; + } return await fetchOCIManifestIfExists(output, env, templateRef, manifestDigest, authToken); } diff --git a/src/spec-node/collectionCommonUtils/publishCommandImpl.ts b/src/spec-node/collectionCommonUtils/publishCommandImpl.ts index e5d3bbf61..2976f837f 100644 --- a/src/spec-node/collectionCommonUtils/publishCommandImpl.ts +++ b/src/spec-node/collectionCommonUtils/publishCommandImpl.ts @@ -44,7 +44,7 @@ export async function doPublishCommand(version: string, ociRef: OCIRef, outputDi const publishedVersions = await getPublishedVersions(ociRef, output); if (!publishedVersions) { - process.exit(1); + return false; } const semanticVersions: string[] | undefined = getSermanticVersions(version, publishedVersions, output); @@ -53,11 +53,12 @@ export async function doPublishCommand(version: string, ociRef: OCIRef, outputDi output.write(`Publishing versions: ${semanticVersions.toString()}...`, LogLevel.Info); const pathToTgz = path.join(outputDir, getArchiveName(ociRef.id, collectionType)); if (! await pushOCIFeatureOrTemplate(output, ociRef, pathToTgz, semanticVersions, collectionType)) { - output.write(`(!) ERR: Failed to publish ${collectionType}: ${ociRef.resource}`, LogLevel.Error); - process.exit(1); + output.write(`(!) ERR: Failed to publish ${collectionType}: '${ociRef.resource}'`, LogLevel.Error); + return false; } - output.write(`Published ${collectionType}: ${ociRef.id}...`, LogLevel.Info); } + output.write(`Published ${collectionType}: ${ociRef.id}...`, LogLevel.Info); + return true; } export async function doPublishMetadata(collectionRef: OCICollectionRef, outputDir: string, output: Log, collectionType: string) { @@ -67,7 +68,8 @@ export async function doPublishMetadata(collectionRef: OCICollectionRef, outputD const pathToCollectionFile = path.join(outputDir, OCICollectionFileName); if (! await pushCollectionMetadata(output, collectionRef, pathToCollectionFile, collectionType)) { output.write(`(!) ERR: Failed to publish collection metadata: ${OCICollectionFileName}`, LogLevel.Error); - process.exit(1); + return false; } output.write('Published collection metadata...', LogLevel.Info); + return true; } diff --git a/src/spec-node/featuresCLI/info.ts b/src/spec-node/featuresCLI/info.ts index 23a911e4c..49681e2db 100644 --- a/src/spec-node/featuresCLI/info.ts +++ b/src/spec-node/featuresCLI/info.ts @@ -40,6 +40,14 @@ async function featuresInfo({ }, pkg, new Date(), disposables, true); const featureOciRef = getRef(output, featureId); + if (!featureOciRef) { + if (outputFormat === 'json') { + output.raw(JSON.stringify({}), LogLevel.Info); + } else { + output.raw(`Failed to parse Feature identifier '${featureId}'\n`, LogLevel.Error); + } + process.exit(1); + } const publishedVersions = await getPublishedVersions(featureOciRef, output, true); if (!publishedVersions || publishedVersions.length === 0) { diff --git a/src/spec-node/featuresCLI/publish.ts b/src/spec-node/featuresCLI/publish.ts index 249f9d6c8..34027f133 100644 --- a/src/spec-node/featuresCLI/publish.ts +++ b/src/spec-node/featuresCLI/publish.ts @@ -77,7 +77,15 @@ async function featuresPublish({ const resource = `${registry}/${namespace}/${f.id}`; const featureRef = getRef(output, resource); - await doPublishCommand(f.version, featureRef, outputDir, output, collectionType); + if (!featureRef) { + output.write(`(!) Could not parse provided Feature identifier: '${resource}'`, LogLevel.Error); + process.exit(1); + } + + if (! await doPublishCommand(f.version, featureRef, outputDir, output, collectionType)) { + output.write(`(!) ERR: Failed to publish '${resource}'`, LogLevel.Error); + process.exit(1); + } } const featureCollectionRef: OCICollectionRef = { @@ -86,7 +94,10 @@ async function featuresPublish({ version: 'latest' }; - await doPublishMetadata(featureCollectionRef, outputDir, output, collectionType); + if (! await doPublishMetadata(featureCollectionRef, outputDir, output, collectionType)) { + output.write(`(!) ERR: Failed to publish '${featureCollectionRef.registry}/${featureCollectionRef.path}'`, LogLevel.Error); + process.exit(1); + } // Cleanup await rmLocal(outputDir, { recursive: true, force: true }); diff --git a/src/spec-node/templatesCLI/publish.ts b/src/spec-node/templatesCLI/publish.ts index e876f1fb8..94bd90788 100644 --- a/src/spec-node/templatesCLI/publish.ts +++ b/src/spec-node/templatesCLI/publish.ts @@ -78,6 +78,11 @@ async function templatesPublish({ const resource = `${registry}/${namespace}/${t.id}`; const templateRef = getRef(output, resource); + if (!templateRef) { + output.write(`(!) Could not parse provided Template identifier: '${resource}'`, LogLevel.Error); + process.exit(1); + } + await doPublishCommand(t.version, templateRef, outputDir, output, collectionType); } diff --git a/src/spec-node/utils.ts b/src/spec-node/utils.ts index aa81a09b8..d0814884b 100644 --- a/src/spec-node/utils.ts +++ b/src/spec-node/utils.ts @@ -215,6 +215,9 @@ export async function inspectDockerImage(params: DockerResolverParameters | Dock export async function inspectImageInRegistry(output: Log, name: string, authToken?: string): Promise { const resourceAndVersion = qualifyImageName(name); const ref = getRef(output, resourceAndVersion); + if (!ref) { + throw new Error(`Could not parse image name '${name}'`); + } const auth = authToken ?? await fetchRegistryAuthToken(output, ref.registry, ref.path, process.env, 'pull'); const registryServer = ref.registry === 'docker.io' ? 'registry-1.docker.io' : ref.registry; diff --git a/src/test/container-features/containerFeaturesOCI.test.ts b/src/test/container-features/containerFeaturesOCI.test.ts index f47bac425..fafcdfb8d 100644 --- a/src/test/container-features/containerFeaturesOCI.test.ts +++ b/src/test/container-features/containerFeaturesOCI.test.ts @@ -4,9 +4,122 @@ import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log'; export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace)); +describe('getRef()', async function () { + this.timeout('120s'); + + it('valid getRef() with a tag', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker:latest'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + }); + + it('valid getRef() without a version tag', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() automatically downcases', async () => { + const feat = getRef(output, 'ghcr.io/DeVContainERS/templates/Docker-FROM-Docker'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() with a registry that contains a port.', async () => { + const feat = getRef(output, 'docker.io:8001/devcontainers/templates/docker-from-docker:latest'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'docker.io:8001'); + assert.equal(feat.resource, 'docker.io:8001/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() really short path and no version', async () => { + const feat = getRef(output, 'docker.io:8001/a/b/c'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'c'); + assert.equal(feat.namespace, 'a/b'); + assert.equal(feat.owner, 'a'); + assert.equal(feat.registry, 'docker.io:8001'); + assert.equal(feat.resource, 'docker.io:8001/a/b/c'); + assert.equal(feat.path, 'a/b/c'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('invalid getRef() with duplicate version tags', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker:latest:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with invalid character in namespace', async () => { + const feat = getRef(output, 'ghcr.io/devco%ntainers/templates/docker-from-docker:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with invalid character in feature name', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from@docker:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with missing path with version tag', async () => { + const feat = getRef(output, 'ghcr.io/:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with missing path without version tag', async () => { + const feat = getRef(output, 'ghcr.io'); + assert.isUndefined(feat); + }); + + it('invalid getRef() multiple slashes in sequence', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers//templates/docker-from-docker:latest'); + assert.isUndefined(feat); + }); + +}); + describe('Test OCI Pull', () => { it('Parse OCI identifier', async () => { const feat = getRef(output, 'ghcr.io/codspace/features/ruby:1'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } output.write(`feat: ${JSON.stringify(feat)}`); assert.equal(feat.id, 'ruby'); @@ -20,6 +133,9 @@ describe('Test OCI Pull', () => { it('Get a manifest by tag', async () => { const featureRef = getRef(output, 'ghcr.io/codspace/features/ruby:1.0.13'); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const manifest = await getManifest(output, process.env, 'https://ghcr.io/v2/codspace/features/ruby/manifests/1.0.13', featureRef); assert.isNotNull(manifest); assert.exists(manifest); @@ -42,6 +158,9 @@ describe('Test OCI Pull', () => { it('Download a feature', async () => { const featureRef = getRef(output, 'ghcr.io/codspace/features/ruby:1.0.13'); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const blobResult = await getBlob(output, process.env, 'https://ghcr.io/v2/codspace/features/ruby/blobs/sha256:8f59630bd1ba6d9e78b485233a0280530b3d0a44338f472206090412ffbd3efb', '/tmp', '/tmp/featureTest', featureRef); assert.isDefined(blobResult); assert.isArray(blobResult?.files); diff --git a/src/test/container-features/containerFeaturesOCIPush.test.ts b/src/test/container-features/containerFeaturesOCIPush.test.ts index 32ebe28ec..2ccfdec7e 100644 --- a/src/test/container-features/containerFeaturesOCIPush.test.ts +++ b/src/test/container-features/containerFeaturesOCIPush.test.ts @@ -46,6 +46,9 @@ describe('Test OCI Push', () => { it('Can check whether a blob exists', async () => { const ociFeatureRef = getRef(output, 'ghcr.io/codspace/features/go:1'); + if (!ociFeatureRef) { + assert.fail('getRef() for the Feature should not be undefined'); + } const { registry, resource } = ociFeatureRef; const sessionAuth = await fetchRegistryAuthToken(output, registry, resource, process.env, 'pull'); if (!sessionAuth) { diff --git a/src/test/container-features/featuresCLICommands.test.ts b/src/test/container-features/featuresCLICommands.test.ts index f265280fc..c25cc85e7 100644 --- a/src/test/container-features/featuresCLICommands.test.ts +++ b/src/test/container-features/featuresCLICommands.test.ts @@ -478,6 +478,9 @@ describe('test function getPublishedVersions', async () => { it('should list published versions', async () => { const resource = 'ghcr.io/devcontainers/features/node'; const featureRef = getRef(output, resource); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const versionsList = await getPublishedVersions(featureRef, output) ?? []; assert.includeMembers(versionsList, ['1', '1.0', '1.0.0', 'latest']); }); From 3142fb4a125cde62d969b91e6e82c9634e7298c8 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Mon, 28 Nov 2022 17:31:43 +0100 Subject: [PATCH 4/8] 0.25.0 --- CHANGELOG.md | 11 ++++++++++- package.json | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40c2be13e..1cb8c47c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,18 @@ Notable changes. ## November 2022 +### [0.25.0] + +- `features test`: Respect image label metadata. (https://github.com/devcontainers/cli/pull/288) +- Surface first error (https://github.com/microsoft/vscode-remote-release/issues/7382) +- `templates publish`: Exit for "Failed to PUT manifest for tag x" error. (https://github.com/devcontainers/cli/pull/296) +- Respect devcontainer.json when using image without features. (https://github.com/devcontainers/cli/issues/299) +- Emit response from registry on failed `postUploadSessionId` (https://github.com/devcontainers/cli/pull/298) +- downcase OCI identifiers and validate input of getRef() (https://github.com/devcontainers/cli/pull/293) + ### [0.24.1] -- `features test`: Respects testing scenarios where 'remoteUser' is non-root (https://github.com/devcontainers/cli/pull/286) +- `features test`: Respects testing scenarios where 'remoteUser' is non-root (https://github.com/devcontainers/cli/pull/286) ### [0.24.0] diff --git a/package.json b/package.json index 9349d1d41..82388b184 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@devcontainers/cli", "description": "Dev Containers CLI", - "version": "0.24.1", + "version": "0.25.0", "bin": { "devcontainer": "devcontainer.js" }, From 1aeb8489f0f006c83a94377c7d122f9871dc93f1 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 28 Nov 2022 14:49:10 -0800 Subject: [PATCH 5/8] v0.25.1 - Fix regression in `requestResolveHeaders` (#302) * fix request headers * remove debug line * v0.25.1 --- CHANGELOG.md | 3 +++ package.json | 2 +- src/spec-configuration/containerCollectionsOCIPush.ts | 6 +++--- src/spec-utils/httpRequest.ts | 11 +++++++---- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb8c47c6..49a1024a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Notable changes. ## November 2022 +### [0.25.1] +- Fix regression in https://github.com/devcontainers/cli/pull/298 + ### [0.25.0] - `features test`: Respect image label metadata. (https://github.com/devcontainers/cli/pull/288) diff --git a/package.json b/package.json index 82388b184..afaf9d986 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@devcontainers/cli", "description": "Dev Containers CLI", - "version": "0.25.0", + "version": "0.25.1", "bin": { "devcontainer": "devcontainer.js" }, diff --git a/src/spec-configuration/containerCollectionsOCIPush.ts b/src/spec-configuration/containerCollectionsOCIPush.ts index eabf1df57..b97f555bb 100644 --- a/src/spec-configuration/containerCollectionsOCIPush.ts +++ b/src/spec-configuration/containerCollectionsOCIPush.ts @@ -161,14 +161,14 @@ async function putManifestWithTags(output: Log, manifestStr: string, ociRef: OCI data: Buffer.from(manifestStr), }; - let { statusCode, resHeaders } = await requestResolveHeaders(options); + let { statusCode, resHeaders } = await requestResolveHeaders(options, output); // Retry logic: when request fails with HTTP 429: too many requests if (statusCode === 429) { output.write(`Failed to PUT manifest for tag ${tag} due to too many requests. Retrying...`, LogLevel.Warning); await delay(2000); - let response = await requestResolveHeaders(options); + let response = await requestResolveHeaders(options, output); statusCode = response.statusCode; resHeaders = response.resHeaders; } @@ -212,7 +212,7 @@ async function putBlob(output: Log, pathToBlob: string, blobPutLocationUriPath: output.write(`Crafted blob url: ${url}`, LogLevel.Trace); - const { statusCode } = await requestResolveHeaders({ type: 'PUT', url, headers, data: await readLocalFile(pathToBlob) }); + const { statusCode } = await requestResolveHeaders({ type: 'PUT', url, headers, data: await readLocalFile(pathToBlob) }, output); if (statusCode !== 201) { output.write(`${statusCode}: Failed to upload blob '${pathToBlob}' to '${url}'`, LogLevel.Error); return false; diff --git a/src/spec-utils/httpRequest.ts b/src/spec-utils/httpRequest.ts index 62c16be1a..44058ae44 100644 --- a/src/spec-utils/httpRequest.ts +++ b/src/spec-utils/httpRequest.ts @@ -91,10 +91,13 @@ export function requestResolveHeaders(options: { type: string; url: string; head resBody: Buffer.concat(chunks) }); }); - if (options.data) { - req.write(options.data); - } - req.end(); }); + + if (options.data) { + req.write(options.data); + } + + req.on('error', reject); + req.end(); }); } \ No newline at end of file From d5bc1f28c858ee205eded34598701ce64bbcc176 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 29 Nov 2022 09:26:37 -0800 Subject: [PATCH 6/8] `getCollectionRef()` to centralize validating Collection identifier (#303) * create getCollectionRef() to centralize validating Collection identifier * missing a quote * another quote --- .../containerCollectionsOCI.ts | 52 ++++++++++++++---- src/spec-node/featuresCLI/publish.ts | 12 ++--- src/spec-node/templatesCLI/publish.ts | 12 ++--- .../containerFeaturesOCI.test.ts | 54 ++++++++++++++++++- 4 files changed, 107 insertions(+), 23 deletions(-) diff --git a/src/spec-configuration/containerCollectionsOCI.ts b/src/spec-configuration/containerCollectionsOCI.ts index 9a23f0e8a..302a27c20 100644 --- a/src/spec-configuration/containerCollectionsOCI.ts +++ b/src/spec-configuration/containerCollectionsOCI.ts @@ -13,7 +13,9 @@ export const DEVCONTAINER_COLLECTION_LAYER_MEDIATYPE = 'application/vnd.devconta export type HEADERS = { 'authorization'?: string; 'user-agent': string; 'content-type'?: string; 'accept'?: string }; -// ghcr.io/devcontainers/features/go:1.0.0 +// Represents the unique OCI identifier for a Feature or Template. +// eg: ghcr.io/devcontainers/features/go:1.0.0 +// Constructed by 'getRef()' export interface OCIRef { registry: string; // 'ghcr.io' owner: string; // 'devcontainers' @@ -24,11 +26,14 @@ export interface OCIRef { version?: string; // '1.0.0' } -// ghcr.io/devcontainers/features:latest +// Represents the unique OCI identifier for a Collection's Metadata artifact. +// eg: ghcr.io/devcontainers/features:latest +// Constructed by 'getCollectionRef()' export interface OCICollectionRef { registry: string; // 'ghcr.io' path: string; // 'devcontainers/features' - version: 'latest'; // 'latest' + resource: string; // 'ghcr.io/devcontainers/features' + version: 'latest'; // 'latest' (always) } export interface OCILayer { @@ -100,13 +105,15 @@ export function getRef(output: Log, input: string): OCIRef | undefined { const path = `${namespace}/${id}`; - output.write(`resource: ${resource}`, LogLevel.Trace); - output.write(`id: ${id}`, LogLevel.Trace); - output.write(`version: ${version}`, LogLevel.Trace); - output.write(`owner: ${owner}`, LogLevel.Trace); - output.write(`namespace: ${namespace}`, LogLevel.Trace); // TODO: We assume 'namespace' includes at least one slash (eg: 'devcontainers/features') - output.write(`registry: ${registry}`, LogLevel.Trace); - output.write(`path: ${path}`, LogLevel.Trace); + output.write(`> input: ${input}`, LogLevel.Trace); + output.write(`>`, LogLevel.Trace); + output.write(`> resource: ${resource}`, LogLevel.Trace); + output.write(`> id: ${id}`, LogLevel.Trace); + output.write(`> version: ${version}`, LogLevel.Trace); + output.write(`> owner: ${owner}`, LogLevel.Trace); + output.write(`> namespace: ${namespace}`, LogLevel.Trace); // TODO: We assume 'namespace' includes at least one slash (eg: 'devcontainers/features') + output.write(`> registry: ${registry}`, LogLevel.Trace); + output.write(`> path: ${path}`, LogLevel.Trace); // Validate results of parse. @@ -131,6 +138,31 @@ export function getRef(output: Log, input: string): OCIRef | undefined { }; } +export function getCollectionRef(output: Log, registry: string, namespace: string): OCICollectionRef | undefined { + // Normalize input by downcasing entire string + registry = registry.toLowerCase(); + namespace = namespace.toLowerCase(); + + const path = namespace; + const resource = `${registry}/${path}`; + + output.write(`> Inputs: registry='${registry}' namespace='${namespace}'`, LogLevel.Trace); + output.write(`>`, LogLevel.Trace); + output.write(`> resource: ${resource}`, LogLevel.Trace); + + if (!regexForPath.exec(path)) { + output.write(`Parsed path '${path}' from input failed validation.`, LogLevel.Error); + return undefined; + } + + return { + registry, + path, + resource, + version: 'latest' + }; +} + // Validate if a manifest exists and is reachable about the declared feature/template. // Specification: https://github.com/opencontainers/distribution-spec/blob/v1.0.1/spec.md#pulling-manifests export async function fetchOCIManifestIfExists(output: Log, env: NodeJS.ProcessEnv, ref: OCIRef | OCICollectionRef, manifestDigest?: string, authToken?: string): Promise { diff --git a/src/spec-node/featuresCLI/publish.ts b/src/spec-node/featuresCLI/publish.ts index 34027f133..1946c3708 100644 --- a/src/spec-node/featuresCLI/publish.ts +++ b/src/spec-node/featuresCLI/publish.ts @@ -12,7 +12,7 @@ import { loadNativeModule } from '../../spec-common/commonUtils'; import { PackageCommandInput } from '../collectionCommonUtils/package'; import { OCICollectionFileName } from '../collectionCommonUtils/packageCommandImpl'; import { publishOptions } from '../collectionCommonUtils/publish'; -import { getRef, OCICollectionRef } from '../../spec-configuration/containerCollectionsOCI'; +import { getCollectionRef, getRef, OCICollectionRef } from '../../spec-configuration/containerCollectionsOCI'; import { doPublishCommand, doPublishMetadata } from '../collectionCommonUtils/publishCommandImpl'; const collectionType = 'feature'; @@ -88,11 +88,11 @@ async function featuresPublish({ } } - const featureCollectionRef: OCICollectionRef = { - registry: registry, - path: namespace, - version: 'latest' - }; + const featureCollectionRef: OCICollectionRef | undefined = getCollectionRef(output, registry, namespace); + if (!featureCollectionRef) { + output.write(`(!) Could not parse provided collection identifier with registry '${registry}' and namespace '${namespace}'`, LogLevel.Error); + process.exit(1); + } if (! await doPublishMetadata(featureCollectionRef, outputDir, output, collectionType)) { output.write(`(!) ERR: Failed to publish '${featureCollectionRef.registry}/${featureCollectionRef.path}'`, LogLevel.Error); diff --git a/src/spec-node/templatesCLI/publish.ts b/src/spec-node/templatesCLI/publish.ts index 94bd90788..ca5dca783 100644 --- a/src/spec-node/templatesCLI/publish.ts +++ b/src/spec-node/templatesCLI/publish.ts @@ -12,7 +12,7 @@ import { loadNativeModule } from '../../spec-common/commonUtils'; import { PackageCommandInput } from '../collectionCommonUtils/package'; import { OCICollectionFileName } from '../collectionCommonUtils/packageCommandImpl'; import { packageTemplates } from './packageImpl'; -import { getRef, OCICollectionRef } from '../../spec-configuration/containerCollectionsOCI'; +import { getCollectionRef, getRef, OCICollectionRef } from '../../spec-configuration/containerCollectionsOCI'; import { doPublishCommand, doPublishMetadata } from '../collectionCommonUtils/publishCommandImpl'; const collectionType = 'template'; @@ -86,11 +86,11 @@ async function templatesPublish({ await doPublishCommand(t.version, templateRef, outputDir, output, collectionType); } - const templateCollectionRef: OCICollectionRef = { - registry: registry, - path: namespace, - version: 'latest' - }; + const templateCollectionRef: OCICollectionRef | undefined = getCollectionRef(output, registry, namespace); + if (!templateCollectionRef) { + output.write(`(!) Could not parse provided collection identifier with registry '${registry}' and namespace '${namespace}'`, LogLevel.Error); + process.exit(1); + } await doPublishMetadata(templateCollectionRef, outputDir, output, collectionType); diff --git a/src/test/container-features/containerFeaturesOCI.test.ts b/src/test/container-features/containerFeaturesOCI.test.ts index fafcdfb8d..b3db8ecbf 100644 --- a/src/test/container-features/containerFeaturesOCI.test.ts +++ b/src/test/container-features/containerFeaturesOCI.test.ts @@ -1,9 +1,61 @@ import { assert } from 'chai'; -import { getRef, getManifest, getBlob } from '../../spec-configuration/containerCollectionsOCI'; +import { getRef, getManifest, getBlob, getCollectionRef } from '../../spec-configuration/containerCollectionsOCI'; import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log'; export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace)); +describe('getCollectionRef()', async function () { + this.timeout('120s'); + + + it('valid getCollectionRef()', async () => { + const collectionRef = getCollectionRef(output, 'ghcr.io', 'devcontainers/templates'); + if (!collectionRef) { + assert.fail('collectionRef should not be undefined'); + } + assert.ok(collectionRef); + assert.equal(collectionRef.registry, 'ghcr.io'); + assert.equal(collectionRef.path, 'devcontainers/templates'); + assert.equal(collectionRef.resource, 'ghcr.io/devcontainers/templates'); + assert.equal(collectionRef.version, 'latest'); + }); + + it('valid getCollectionRef() that was originally uppercase', async () => { + const collectionRef = getCollectionRef(output, 'GHCR.IO', 'DEVCONTAINERS/TEMPLATES'); + if (!collectionRef) { + assert.fail('collectionRef should not be undefined'); + } + assert.ok(collectionRef); + assert.equal(collectionRef.registry, 'ghcr.io'); + assert.equal(collectionRef.path, 'devcontainers/templates'); + assert.equal(collectionRef.resource, 'ghcr.io/devcontainers/templates'); + assert.equal(collectionRef.version, 'latest'); + }); + + it('valid getCollectionRef() with port in registry', async () => { + const collectionRef = getCollectionRef(output, 'ghcr.io:8001', 'devcontainers/templates'); + if (!collectionRef) { + assert.fail('collectionRef should not be undefined'); + } + assert.ok(collectionRef); + assert.equal(collectionRef.registry, 'ghcr.io:8001'); + assert.equal(collectionRef.path, 'devcontainers/templates'); + assert.equal(collectionRef.resource, 'ghcr.io:8001/devcontainers/templates'); + assert.equal(collectionRef.version, 'latest'); + }); + + it('invalid getCollectionRef() with an invalid character in path', async () => { + const collectionRef = getCollectionRef(output, 'ghcr.io', 'devcont%ainers/templates'); + assert.isUndefined(collectionRef); + }); + + it('invalid getCollectionRef() with too many slashes in path', async () => { + const collectionRef = getCollectionRef(output, 'ghcr.io', 'devcontainers//templates'); + assert.isUndefined(collectionRef); + }); + +}); + describe('getRef()', async function () { this.timeout('120s'); From 1289e4e9fd2e804df2cb5b58180129525c847dd6 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 29 Nov 2022 10:10:36 -0800 Subject: [PATCH 7/8] v0.25.2 (#304) --- CHANGELOG.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49a1024a3..e9159588c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Notable changes. ## November 2022 +### [0.25.2] + +- Fix Feature/Template publishing issue when a capital letter is in the repo name (https://github.com/devcontainers/cli/pull/303) + ### [0.25.1] - Fix regression in https://github.com/devcontainers/cli/pull/298 diff --git a/package.json b/package.json index afaf9d986..c9549c82d 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@devcontainers/cli", "description": "Dev Containers CLI", - "version": "0.25.1", + "version": "0.25.2", "bin": { "devcontainer": "devcontainer.js" }, From fd95a6f9c24dc97587c31c22c4ac1368b425aad2 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 23 Nov 2022 10:05:42 -0800 Subject: [PATCH 8/8] Invoke newt to build devcontainers with cloudbuild --- src/spec-node/devContainers.ts | 9 +-- src/spec-node/devContainersSpecCLI.ts | 10 +-- src/spec-node/singleContainer.ts | 91 +++++---------------------- src/spec-shutdown/dockerUtils.ts | 6 +- 4 files changed, 21 insertions(+), 95 deletions(-) diff --git a/src/spec-node/devContainers.ts b/src/spec-node/devContainers.ts index 4f554fcef..46f62688f 100644 --- a/src/spec-node/devContainers.ts +++ b/src/spec-node/devContainers.ts @@ -17,7 +17,6 @@ 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 } from '../spec-shutdown/dockerUtils'; export const experimentalImageMetadataDefault = true; @@ -142,13 +141,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables: env: cliHost.env, output: common.output, }, dockerPath, dockerComposePath); - const buildKitVersion = options.useBuildKit === 'never' ? null : (await dockerBuildKitVersion({ - cliHost, - dockerCLI: dockerPath, - dockerComposeCLI, - env: cliHost.env, - output - })); + const buildKitVersion = options.useBuildKit === 'never' ? null : 'v0.10.0'; return { common, parsedAuthority, diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index 2a5bfa677..38486a3fe 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -397,15 +397,7 @@ async function doBuild({ // Build the base image and extend with features etc. let { updatedImageName } = await buildNamedImageAndExtend(params, configWithRaw as SubstitutedConfig, additionalFeatures, false, imageNames); - - if (imageNames) { - if (!buildxPush && !buildxOutput) { - await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName))); - } - imageNameResult = imageNames; - } else { - imageNameResult = updatedImageName; - } + imageNameResult = updatedImageName; } else if ('dockerComposeFile' in config) { if (buildxPlatform || buildxPush) { diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index 751457755..396345485 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -4,13 +4,13 @@ *--------------------------------------------------------------------------------------------*/ -import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, logUMask, SubstitutedConfig, checkDockerSupportForGPU } from './utils'; +import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, SubstitutedConfig, checkDockerSupportForGPU } from './utils'; import { ContainerProperties, setupInContainer, ResolverProgress, ResolverParameters } from '../spec-common/injectHeadless'; import { ContainerError, toErrorText } from '../spec-common/errors'; import { ContainerDetails, listContainers, DockerCLIParameters, inspectContainers, dockerCLI, dockerPtyCLI, toPtyExecParameters, ImageDetails, toExecParameters } from '../spec-shutdown/dockerUtils'; import { DevContainerConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig } from '../spec-configuration/configuration'; import { LogLevel, Log, makeLog } from '../spec-utils/log'; -import { extendImage, getExtendImageBuildInfo, updateRemoteUserUID } from './containerFeatures'; +import { extendImage, getExtendImageBuildInfo } from './containerFeatures'; import { getDevcontainerMetadata, getImageBuildInfoFromDockerfile, getImageMetadataFromContainer, ImageMetadataEntry, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata'; import { ensureDockerfileHasFinalStageName } from './dockerfileUtils'; @@ -40,24 +40,7 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter const imageMetadata = getImageMetadataFromContainer(container, configWithRaw, undefined, idLabels, common.experimentalImageMetadata, common.output).config; mergedConfig = mergeConfiguration(config, imageMetadata); } else { - const res = await buildNamedImageAndExtend(params, configWithRaw, additionalFeatures, true); - const imageMetadata = res.imageMetadata.config; - mergedConfig = mergeConfiguration(config, imageMetadata); - const { containerUser } = mergedConfig; - const updatedImageName = await updateRemoteUserUID(params, mergedConfig, res.updatedImageName[0], res.imageDetails, findUserArg(config.runArgs) || containerUser); - - // collapsedFeaturesConfig = async () => res.collapsedFeaturesConfig; - - try { - await spawnDevContainer(params, config, mergedConfig, updatedImageName, idLabels, workspaceConfig.workspaceMount, res.imageDetails, containerUser, res.labels || {}); - } finally { - // In 'finally' because 'docker run' can fail after creating the container. - // Trying to get it here, so we can offer 'Rebuild Container' as an action later. - container = await findDevContainer(params, idLabels); - } - if (!container) { - return bailOut(common.output, 'Dev container not found.'); - } + return bailOut(common.output, 'Code removed.'); } containerProperties = await createContainerProperties(params, container.Id, workspaceConfig.workspaceFolder, mergedConfig.remoteUser); @@ -110,7 +93,7 @@ async function setupContainer(container: ContainerDetails, params: DockerResolve function getDefaultName(config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, params: DockerResolverParameters) { return 'image' in config ? config.image : getFolderImageName(params.common); } -export async function buildNamedImageAndExtend(params: DockerResolverParameters, configWithRaw: SubstitutedConfig, additionalFeatures: Record>, canAddLabelsToContainer: boolean, argImageNames?: string[]): Promise<{ updatedImageName: string[]; imageMetadata: SubstitutedConfig; imageDetails: () => Promise; labels?: Record }> { +export async function buildNamedImageAndExtend(params: DockerResolverParameters, configWithRaw: SubstitutedConfig, additionalFeatures: Record>, canAddLabelsToContainer: boolean, argImageNames?: string[]): Promise<{ updatedImageName: string[]; imageMetadata: SubstitutedConfig; labels?: Record }> { const { config } = configWithRaw; const imageNames = argImageNames ?? [getDefaultName(config, params)]; params.common.progress(ResolverProgress.BuildingImage); @@ -167,90 +150,46 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config additionalBuildArgs.push('--build-context', `${buildContext}=${featureBuildInfo.buildKitContexts[buildContext]}`); } for (const buildArg in featureBuildInfo.buildArgs) { - additionalBuildArgs.push('--build-arg', `${buildArg}=${featureBuildInfo.buildArgs[buildArg]}`); + additionalBuildArgs.push('--param', `${buildArg}=${featureBuildInfo.buildArgs[buildArg]}`); } } - const args: string[] = []; - if (!buildParams.buildKitVersion && - (buildParams.buildxPlatform || buildParams.buildxPush)) { - throw new ContainerError({ description: '--platform or --push require BuildKit enabled.', data: { fileWithError: dockerfilePath } }); - } - if (buildParams.buildKitVersion) { - args.push('buildx', 'build'); - if (buildParams.buildxPlatform) { - args.push('--platform', buildParams.buildxPlatform); - } - if (buildParams.buildxPush) { - args.push('--push'); - } else { - if (buildParams.buildxOutput) { - args.push('--output', buildParams.buildxOutput); - } else { - args.push('--load'); // (short for --output=docker, i.e. load into normal 'docker images' collection) - } - } - args.push('--build-arg', 'BUILDKIT_INLINE_CACHE=1'); - } else { - args.push('build'); - } - args.push('-f', finalDockerfilePath); + const args: string[] = ['build']; + args.push('--dockerfile', finalDockerfilePath); - baseImageNames.map(imageName => args.push('-t', imageName)); + baseImageNames.map(imageName => args.push('--image-name', imageName)); const target = extendImageBuildInfo?.featureBuildInfo ? extendImageBuildInfo.featureBuildInfo.overrideTarget : config.build?.target; if (target) { args.push('--target', target); } - if (noCache) { - args.push('--no-cache'); - // `docker build --pull` pulls local image: https://github.com/devcontainers/cli/issues/60 - if (buildParams.buildKitVersion || !extendImageBuildInfo) { - args.push('--pull'); - } - } else { - const configCacheFrom = config.build?.cacheFrom; - if (buildParams.additionalCacheFroms.length || (configCacheFrom && (configCacheFrom === 'string' || configCacheFrom.length))) { - await logUMask(buildParams); - } - buildParams.additionalCacheFroms.forEach(cacheFrom => args.push('--cache-from', cacheFrom)); - if (config.build && config.build.cacheFrom) { - if (typeof config.build.cacheFrom === 'string') { - args.push('--cache-from', config.build.cacheFrom); - } else { - for (let index = 0; index < config.build.cacheFrom.length; index++) { - const cacheFrom = config.build.cacheFrom[index]; - args.push('--cache-from', cacheFrom); - } - } - } + if (!noCache) { + args.push('--cache'); } const buildArgs = config.build?.args; if (buildArgs) { for (const key in buildArgs) { - args.push('--build-arg', `${key}=${buildArgs[key]}`); + args.push('--param', `${key}=${buildArgs[key]}`); } } args.push(...additionalBuildArgs); - args.push(await uriToWSLFsPath(getDockerContextPath(cliHost, config), cliHost)); + args.push('--build-path', await uriToWSLFsPath(getDockerContextPath(cliHost, config), cliHost)); try { + buildParams.dockerCLI = 'newt'; if (buildParams.isTTY) { const infoParams = { ...toPtyExecParameters(buildParams), output: makeLog(output, LogLevel.Info) }; - await dockerPtyCLI(infoParams, ...args); + await dockerPtyCLI(infoParams, process.cwd(), ...args); } else { const infoParams = { ...toExecParameters(buildParams), output: makeLog(output, LogLevel.Info), print: 'continuous' as 'continuous' }; - await dockerCLI(infoParams, ...args); + await dockerCLI(infoParams, process.cwd(), ...args); } } catch (err) { throw new ContainerError({ description: 'An error occurred building the image.', originalError: err, data: { fileWithError: dockerfilePath } }); } - const imageDetails = () => inspectDockerImage(buildParams, baseImageNames[0], false); - return { updatedImageName: baseImageNames, imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, configWithRaw, extendImageBuildInfo?.featuresConfig), - imageDetails }; } diff --git a/src/spec-shutdown/dockerUtils.ts b/src/spec-shutdown/dockerUtils.ts index da31d7a7a..1ce75f95d 100644 --- a/src/spec-shutdown/dockerUtils.ts +++ b/src/spec-shutdown/dockerUtils.ts @@ -247,10 +247,11 @@ export async function dockerBuildKitVersion(params: DockerCLIParameters | Partia } } -export async function dockerCLI(params: DockerCLIParameters | PartialExecParameters | DockerResolverParameters, ...args: string[]) { +export async function dockerCLI(params: DockerCLIParameters | PartialExecParameters | DockerResolverParameters, cwd?: string, ...args: string[]) { const partial = toExecParameters(params); return runCommandNoPty({ ...partial, + cwd: cwd, args: (partial.args || []).concat(args), }); } @@ -286,10 +287,11 @@ export async function isPodman(params: DockerCLIParameters | DockerResolverParam } } -export async function dockerPtyCLI(params: PartialPtyExecParameters | DockerResolverParameters | DockerCLIParameters, ...args: string[]) { +export async function dockerPtyCLI(params: PartialPtyExecParameters | DockerResolverParameters | DockerCLIParameters, cwd?: string, ...args: string[]) { const partial = toPtyExecParameters(params); return runCommand({ ...partial, + cwd: cwd, args: (partial.args || []).concat(args), }); }