-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Support docker-like references #6
Conversation
/// - `docker.io/library/ubuntu@sha256:1234567890` is resolved as `docker.io/library/ubuntu@sha256:1234567890` | ||
/// - `docker.io/library/ubuntu:24.04` is resolved as `docker.io/library/ubuntu:24.04` | ||
#[arg(verbatim_doc_comment)] | ||
pub image: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
[Optional, probably more relevant for the CLI and super minor] It might be worth making the "base" configurable via env var. If I'm a company who has a list of image names it may be simpler for me to set an env var specifying the base rather than have to program my CI jobs to concatenate it together with the base name. It also means that I can create a CI job template that sets the var and that makes calls to this do the right thing rather than making my engineers have to remember to use the FQN.
^^ I'm a little dubious that this is super useful, but I think it's worth explicitly rejecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's an incredible idea! made that quick change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about this more over lunch, and it would be similar to also have CLI options that can do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this is behavior that we probably don't want in flags; it's not really intended to be used most of the time, it's just an escape hatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
Overview
Adds support for docker-like references.
The following all work:
Previously,
circe
would error on these cases; now it doesn't, and works as you'd expect.In effect, partially-qualified references are expanded to
docker.io/library/{name}
by default.The choice of
docker.io
being the default was chosen mainly for compatibility with the behavior in FOSSA CLI. I considered adding more hosts, but decided it'd be better to push users to fully qualify their references instead of having them lean on potentially surprising fallback behavior.Acceptance criteria
Docker-style pulls are possible.
Testing plan
Automated tests cover this mostly; did some basic manual tests below:
Metrics
None
Risks
Allowing users to provide partially qualified references introduces potential for confusion into the system, but since this is the same behavior as
docker
(with which most container users are familiar) I think this is minimized.Checklist