Fixes for imagetools create cross-registry authentication failures
#298
Conversation
Test Results862 tests 862 ✅ 8m 7s ⏱️ Results for commit 41d27e2. ♻️ This comment has been updated with latest results. |
…party package logging
…h `imagetools create`
…platforms are considered Print the resulting manifest for the image always
|
This worked in I since closed the PR as all changes were made in this PR instead. The failed dev build is from #304 and is unrelated to these changes. |
bschwedler
left a comment
bschwedler
left a comment
There was a problem hiding this comment.
The process, although convoluted, makes sense to me.
I am curious how much extra time pulling/pushing all the images locally adds.
| manifest = target.merge(sources=sources, dry_run=dry_run) | ||
| if dry_run: | ||
| stdout_console.print_json(manifest.model_dump_json(indent=2, exclude_unset=True, exclude_none=True)) | ||
| stdout_console.print_json(manifest.model_dump_json(indent=2, exclude_unset=True, exclude_none=True)) |
There was a problem hiding this comment.
Do we want to output this for every operation?
There was a problem hiding this comment.
It helps my sanity significantly to see that the result is multipltaform. We could disable it with --quiet or something if we wanted.
| if self.os: | ||
| s += self.os + "/" | ||
| if self.architecture: | ||
| s += self.architecture |
There was a problem hiding this comment.
If architecture is None, this could conceivably end with a trailing slash.
There was a problem hiding this comment.
It would be a much larger problem I think if zero platforms were listed in an inspection, but I added s.rstrip("/") since I agree it's good practice.
| self.app_name = "bakery" | ||
| self.application_storage: Path = Path(typer.get_app_dir(app_name=self.app_name)).resolve() | ||
| self.temporary_storage: Path = Path(tempfile.gettempdir()) | ||
| self.log_level: str | int = logging.INFO |
| # Return the final manifest for the merged image as a sanity check. | ||
| return python_on_whales.docker.buildx.imagetools.inspect(self.tags[0]) |
There was a problem hiding this comment.
IIUC this makes the call against the remote repository, which in our case is the image's final destination.
Is there any merit to comparing the results we get from that to manifest from the create call against the temp registry?
There was a problem hiding this comment.
Yes, this is against the remote. I'm not sure if there would be. I suppose you could compare them and output an error if they don't match, but I would also hope that Docker's ops at this sort of level would just work.
Fixes #294, #289