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.

Sign up for GitHub

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

Add docker.io to unqualified image name #12321

Closed
wants to merge 1 commit into from
Closed

Add docker.io to unqualified image name #12321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/api/handlers/compat/images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {
// without this early check this function would return 200 but reported error via body stream soon after
// it's better to let caller know early via HTTP status code that request cannot be processed
_, err := shortnames.Resolve(runtime.SystemContext(), fromImage)
if err != nil && shortnames.IsShortName(fromImage) {
fromImage = fmt.Sprintf("%s/%s", "docker.io", fromImage)
_, err = shortnames.Resolve(runtime.SystemContext(), fromImage)
}
Copy link
Member

Choose a reason for hiding this comment

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

I was suggesting something like:

if err != nil && shortnames.IsShortName(fromImage) {
		names, err = shortnames.Resolve(runtime.SystemContext(), fromImage)
                if len(names) == 1 {
                      fromImage=names[0]
               } else {
		       fromImage = fmt.Sprintf("%s/%s", "docker.io", fromImage)
              }
}

Copy link
Member

Choose a reason for hiding this comment

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

That way if I have a shortname mapping, Podman service will use it, if there is not one then fail over to docker.io.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, my bad; I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but if a software ask for ubi8, shortname.Resolve shouldn't fail in the first place, so it wouldn't go in that condition, no ?

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what you are asking. shortnames.IsShortName("ubi8") should return true.
Then I would want to pull registry.access.redhat.com/ubi8 not docker.io/ubi8.

Copy link
Contributor Author

@mscherer mscherer Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, but the code just before (line 257):

_, err := shortnames.Resolve(runtime.SystemContext(), fromImage)

will return nil in err if fromImage is a short name alias, since it can be resolved.
If there is no error, fromImage will not be modified.

Copy link
Member

Choose a reason for hiding this comment

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

Your right I missed that
LGTM

Copy link
Member

Choose a reason for hiding this comment

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

This is ignoring errors in short-name resolution which I think is something we cannot do. Let's go one step back and first define the problem we want to solve. This looks like a way to big shotgun to me.

if err != nil {
utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrap(err, "failed to resolve image name"))
return
Expand Down