Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #543: build with correct image names #586

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

invasy
Copy link

@invasy invasy commented Jul 5, 2023

Closes #543

Problem

  • 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.

Solution

  • Added image names from --image-names option at building stage.

- 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.
@invasy invasy requested a review from a team as a code owner July 5, 2023 21:46
@invasy
Copy link
Author

invasy commented Jul 5, 2023

@microsoft-github-policy-service agree

@@ -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`];
Copy link
Member

@joshspicer joshspicer Jul 26, 2023

Choose a reason for hiding this comment

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

Looking quickly at CI, I think imageName might need a null check here?

Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Hi @invasy 👋

Apologies on missing out on this PR, would you be still interested in checking this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"--image-name" is ignored when devcontainer.json is using "image" statement
4 participants