Skip to content

Commit

Permalink
Fix devcontainers#543: build with correct image names
Browse files Browse the repository at this point in the history
- Method `extendImage` created image with the default name ignoring
  `--image-name` option.
- The image could not be tagged after building, because CLI exited
  with `ERROR: denied: requested access to the resource is denied`
  after pushing attempt.
- Added image names from `--image-names` option at building stage.
  • Loading branch information
invasy committed Jul 5, 2023
1 parent c263250 commit 63727c1
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 14 deletions.
12 changes: 7 additions & 5 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const getSafeId = (str: string) => str
.replace(/^[\d_]+/g, '_')
.toUpperCase();

export async function extendImage(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, imageName: string, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>, canAddLabelsToContainer: boolean) {
export async function extendImage(params: DockerResolverParameters, config: SubstitutedConfig<DevContainerConfig>, imageName: string, imageNames: string[] | undefined, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>, canAddLabelsToContainer: boolean) {
const { common } = params;
const { cliHost, output } = common;

Expand All @@ -50,8 +50,6 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
// Got feature extensions -> build the image
const dockerfilePath = cliHost.path.join(featureBuildInfo.dstFolder, 'Dockerfile.extended');
await cliHost.writeFile(dockerfilePath, Buffer.from(featureBuildInfo.dockerfilePrefixContent + featureBuildInfo.dockerfileContent));
const folderImageName = getFolderImageName(common);
const updatedImageName = `${imageName.startsWith(folderImageName) ? imageName : folderImageName}-features`;

const args: string[] = [];
if (!params.buildKitVersion &&
Expand Down Expand Up @@ -90,14 +88,18 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
for (const buildArg in featureBuildInfo.buildArgs) {
args.push('--build-arg', `${buildArg}=${featureBuildInfo.buildArgs[buildArg]}`);
}

const folderImageName = getFolderImageName(common);
const updatedImageName: string[] = imageNames ?? [`${imageName.startsWith(folderImageName) ? imageName : folderImageName}-features`];
updatedImageName.map(imageName => args.push('-t', imageName));

// Once this is step merged with the user Dockerfile (or working against the base image),
// the path will be the dev container context
// Set empty dir under temp path as the context for now to ensure we don't have dependencies on the features content
const emptyTempDir = getEmptyContextFolder(common);
cliHost.mkdirp(emptyTempDir);
args.push(
'--target', featureBuildInfo.overrideTarget,
'-t', updatedImageName,
'-f', dockerfilePath,
emptyTempDir
);
Expand All @@ -110,7 +112,7 @@ export async function extendImage(params: DockerResolverParameters, config: Subs
await dockerCLI(infoParams, ...args);
}
return {
updatedImageName: [ updatedImageName ],
updatedImageName: updatedImageName,
imageMetadata: getDevcontainerMetadata(imageBuildInfo.metadata, config, featuresConfig),
imageDetails: async () => imageBuildInfo.imageDetails,
};
Expand Down
10 changes: 2 additions & 8 deletions src/spec-node/devContainersSpecCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,14 +653,8 @@ async function doBuild({
}

await inspectDockerImage(params, config.image, true);
const { updatedImageName } = await extendImage(params, configWithRaw, config.image, additionalFeatures, false);

if (imageNames) {
await Promise.all(imageNames.map(imageName => dockerPtyCLI(params, 'tag', updatedImageName[0], imageName)));
imageNameResult = imageNames;
} else {
imageNameResult = updatedImageName;
}
const { updatedImageName } = await extendImage(params, configWithRaw, config.image, imageNames, additionalFeatures, false);
imageNameResult = updatedImageName;
}

return {
Expand Down
2 changes: 1 addition & 1 deletion src/spec-node/singleContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export async function buildNamedImageAndExtend(params: DockerResolverParameters,
return await buildAndExtendImage(params, configWithRaw as SubstitutedConfig<DevContainerFromDockerfileConfig>, imageNames, params.buildNoCache ?? false, additionalFeatures);
}
// image-based dev container - extend
return await extendImage(params, configWithRaw, imageNames[0], additionalFeatures, canAddLabelsToContainer);
return await extendImage(params, configWithRaw, imageNames[0], imageNames.slice(1), additionalFeatures, canAddLabelsToContainer);
}

async function buildAndExtendImage(buildParams: DockerResolverParameters, configWithRaw: SubstitutedConfig<DevContainerFromDockerfileConfig>, baseImageNames: string[], noCache: boolean, additionalFeatures: Record<string, string | boolean | Record<string, string | boolean>>) {
Expand Down

0 comments on commit 63727c1

Please sign in to comment.