Rewrite only ubuntu: image references on Incus - #105
Merged
Conversation
TranslateImage rewrote every reference carrying a remote into images:<remote>/<alias>/cloud. That is right for ubuntu:, whose simplestreams endpoint does not serve Incus clients, and wrong for everything else: lxc: was redirected to images:lxc/... on a server with no such path, a private or self-hosted remote was silently replaced by the public one, and a /cloud variant was appended to image families that do not publish it. Only ubuntu: is rewritten now. An explicit ubuntu:24.04/cloud is not doubled. The existing test asserted the old behaviour for lxc:, so it encoded the bug rather than catching it; it now asserts the reference is left alone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #91.
The problem
TranslateImagerewrote every reference carrying a remote intoimages:<remote>/<alias>/cloud:That is correct for
ubuntu:— Canonical's simplestreams endpoint does not serve Incus clients, so the reference has to be redirected to the linuxcontainers server. It is wrong for everything else:lxc:alpine/3.18images:lxc/alpine/3.18/cloudlocal:my-golden-imageimages:local/my-golden-image/cloudmyremote:custom/thingimages:myremote/custom/thing/cloudThe
/cloudsuffix is a variant preference, not a fact about the alias namespace, and most image families do not publish it. Rewriting the remote is a separate error from choosing a variant, and this did both at once.The fix
Only
ubuntu:is rewritten. Everything else passes through as written —images:,lxc:, private and self-hosted remotes, and references with no remote at all. LXD is unaffected, as before.An explicit
ubuntu:24.04/cloudis no longer doubled into.../cloud/cloud.The bug was tested in
TestTranslateImageasserted the old behaviour:So the defect had a passing test describing it. That case now asserts the reference is left alone, with a comment explaining why. New table-driven tests cover the full matrix: LXD passthrough, the ubuntu rewrite, every other remote untouched, and the no-double-variant case.
Note on the second symptom in the issue
The issue also recorded that an already-translated
images:ubuntu/24.04/cloudfailed against a live daemon withThe requested image couldn't be found for fingerprint "ubuntu/24.04/cloud", even though the equivalentincus launchworked. That is not fixed here and I could not reproduce it as a DART bug: the reference passes throughTranslateImageuntouched in both the old and new code, and the failure comes from the image server's alias lookup. The only local daemon on this machine is snap LXD, so the reference resolves againstimages.linuxcontainers.orgrather than an Incus remote — a different path than the one the issue describes. Worth keeping #91 open on that point, or splitting it out, since the translation half is now settled.Build, vet, gofmt, and the full test suite are green; the docs site builds. The docs section describing the old rewrite is replaced by what it does now.