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

compat API: allow enforcing short-names resolution to Docker Hub #12435

Merged
merged 1 commit into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ validate.completions:
.PHONY: run-docker-py-tests
run-docker-py-tests:
touch test/__init__.py
pytest test/python/docker/
env CONTAINERS_CONF=$(CURDIR)/test/apiv2/containers.conf pytest test/python/docker/
-rm test/__init__.py

.PHONY: localunit
Expand Down Expand Up @@ -594,8 +594,8 @@ remotesystem:
.PHONY: localapiv2
localapiv2:
env PODMAN=./bin/podman ./test/apiv2/test-apiv2
env PODMAN=./bin/podman ${PYTHON} -m unittest discover -v ./test/apiv2/python
env PODMAN=./bin/podman ${PYTHON} -m unittest discover -v ./test/python/docker
env CONTAINERS_CONF=$(CURDIR)/test/apiv2/containers.conf PODMAN=./bin/podman ${PYTHON} -m unittest discover -v ./test/apiv2/python
env CONTAINERS_CONF=$(CURDIR)/test/apiv2/containers.conf PODMAN=./bin/podman ${PYTHON} -m unittest discover -v ./test/python/docker

.PHONY: remoteapiv2
remoteapiv2:
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ require (
github.com/containernetworking/cni v1.0.1
github.com/containernetworking/plugins v1.0.1
github.com/containers/buildah v1.23.1
github.com/containers/common v0.46.1-0.20211122213330-d4e7724a0c58
github.com/containers/common v0.46.1-0.20211125160015-ccf46abecd91
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/image/v5 v5.17.0
github.com/containers/image/v5 v5.17.1-0.20211129144953-4f6d0b45be6c
github.com/containers/ocicrypt v1.1.2
github.com/containers/psgo v1.7.1
github.com/containers/storage v1.37.1-0.20211122214631-59ba58582415
Expand Down
9 changes: 4 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ github.com/containerd/containerd v1.5.0-beta.3/go.mod h1:/wr9AVtEM7x9c+n0+stptlo
github.com/containerd/containerd v1.5.0-beta.4/go.mod h1:GmdgZd2zA2GYIBZ0w09ZvgqEq8EfBp/m3lcVZIvPHhI=
github.com/containerd/containerd v1.5.0-rc.0/go.mod h1:V/IXoMqNGgBlabz3tHD2TWDoTJseu1FGOKuoA4nNb2s=
github.com/containerd/containerd v1.5.1/go.mod h1:0DOxVqwDy2iZvrZp2JUx/E+hS0UNTVn7dJnIOwtYR4g=
github.com/containerd/containerd v1.5.4/go.mod h1:sx18RgvW6ABJ4iYUw7Q5x7bgFOAB9B6G7+yO0XBc4zw=
github.com/containerd/containerd v1.5.5/go.mod h1:oSTh0QpT1w6jYcGmbiSbxv9OSQYaa88mPyWIuU79zyo=
github.com/containerd/containerd v1.5.7 h1:rQyoYtj4KddB3bxG6SAqd4+08gePNyJjRqvOIfV3rkM=
github.com/containerd/containerd v1.5.7/go.mod h1:gyvv6+ugqY25TiXxcZC3L5yOeYgEw0QMhscqVp1AR9c=
Expand Down Expand Up @@ -263,14 +262,14 @@ github.com/containernetworking/plugins v1.0.1/go.mod h1:QHCfGpaTwYTbbH+nZXKVTxNB
github.com/containers/buildah v1.23.1 h1:Tpc9DsRuU+0Oofewpxb6OJVNQjCu7yloN/obUqzfDTY=
github.com/containers/buildah v1.23.1/go.mod h1:4WnrN0yrA7ab0ppgunixu2WM1rlD2rG8QLJAKbEkZlQ=
github.com/containers/common v0.44.2/go.mod h1:7sdP4vmI5Bm6FPFxb3lvAh1Iktb6tiO1MzjUzhxdoGo=
github.com/containers/common v0.46.1-0.20211122213330-d4e7724a0c58 h1:d99ZfYePYt1gU5dPvtIdnORNtv/7mkAZUHhCJzR5D5k=
github.com/containers/common v0.46.1-0.20211122213330-d4e7724a0c58/go.mod h1:GrXYaGvQtdKA+fCQLudCTOSGRwZ06MVmRnC7KlI+syY=
github.com/containers/common v0.46.1-0.20211125160015-ccf46abecd91 h1:h9SrSLSQkvluH/sEJ8X1rlBqCoGJtLvSOu4OGK0Qtuw=
github.com/containers/common v0.46.1-0.20211125160015-ccf46abecd91/go.mod h1:PHwsa3UBgbvn2/MwpTQvyHXvVpuwfBrlDBx3GpIRPDQ=
github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
github.com/containers/image/v5 v5.16.0/go.mod h1:XgTpfAPLRGOd1XYyCU5cISFr777bLmOerCSpt/v7+Q4=
github.com/containers/image/v5 v5.16.1/go.mod h1:mCvIFdzyyP1B0NBcZ80OIuaYqFn/OpFpaOMOMn1kU2M=
github.com/containers/image/v5 v5.17.0 h1:KS5pro80CCsSp5qDBTMmSAWQo+xcBX19zUPExmYX2OQ=
github.com/containers/image/v5 v5.17.0/go.mod h1:GnYVusVRFPMMTAAUkrcS8NNSpBp8oyrjOUe04AAmRr4=
github.com/containers/image/v5 v5.17.1-0.20211129144953-4f6d0b45be6c h1:WfMOQlq3CDvVe5ONUGfj9/MajskqUHnbo24j24Xg2ZM=
github.com/containers/image/v5 v5.17.1-0.20211129144953-4f6d0b45be6c/go.mod h1:boW5ckkT0wu9obDEiOIxrtWQmz1znMuHiVMQPcpHnk0=
github.com/containers/libtrust v0.0.0-20190913040956-14b96171aa3b h1:Q8ePgVfHDplZ7U33NwHZkrVELsZP5fYj9pM5WBZB2GE=
github.com/containers/libtrust v0.0.0-20190913040956-14b96171aa3b/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
github.com/containers/ocicrypt v1.0.1/go.mod h1:MeJDzk1RJHv89LjsH0Sp5KTY3ZYkjXO/C+bKAeWFIrc=
Expand Down
7 changes: 7 additions & 0 deletions pkg/api/handlers/compat/containers_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
return
}

imageName, err := utils.NormalizeToDockerHub(r, body.Config.Image)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a thought (I didn’t review the PR otherwise at all!)

Several (but by no means all) instances might be a tiny bit shorter with a

if err := utils.NormalizeOverwriteToDockerHub(r, &body.Config.Image); err != nil { … }

with the obvious wrapper

func NormalizeOverwriteToDockerHub(r …, inPlace *string) error {
   v, err := NormalizeToDockerHub(r, *inPlace)
   if err != nil { return err }
   *inPlace = v
   return nil
}

(*grumble* about Go’s error handling ergonomics)

if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
body.Config.Image = imageName

newImage, resolvedName, err := runtime.LibimageRuntime().LookupImage(body.Config.Image, nil)
if err != nil {
if errors.Cause(err) == storage.ErrImageUnknown {
Expand Down
77 changes: 51 additions & 26 deletions pkg/api/handlers/compat/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/containers/common/libimage"
"github.com/containers/common/pkg/config"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/shortnames"
"github.com/containers/image/v5/types"
"github.com/containers/podman/v3/libpod"
"github.com/containers/podman/v3/pkg/api/handlers"
Expand Down Expand Up @@ -56,14 +55,20 @@ func ExportImage(w http.ResponseWriter, r *http.Request) {
defer os.Remove(tmpfile.Name())

name := utils.GetName(r)
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, name)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}

imageEngine := abi.ImageEngine{Libpod: runtime}

saveOptions := entities.ImageSaveOptions{
Format: "docker-archive",
Output: tmpfile.Name(),
}

if err := imageEngine.Save(r.Context(), name, nil, saveOptions); err != nil {
if err := imageEngine.Save(r.Context(), possiblyNormalizedName, nil, saveOptions); err != nil {
if errors.Cause(err) == storage.ErrImageUnknown {
utils.ImageNotFound(w, name, errors.Wrapf(err, "failed to find image %s", name))
return
Expand All @@ -87,9 +92,6 @@ func ExportImage(w http.ResponseWriter, r *http.Request) {
}

func CommitContainer(w http.ResponseWriter, r *http.Request) {
var (
destImage string
)
decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)

Expand All @@ -98,12 +100,12 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
Changes string `schema:"changes"`
Comment string `schema:"comment"`
Container string `schema:"container"`
Pause bool `schema:"pause"`
Repo string `schema:"repo"`
Tag string `schema:"tag"`
// fromSrc string # fromSrc is currently unused
Pause bool `schema:"pause"`
Repo string `schema:"repo"`
Tag string `schema:"tag"`
}{
// This is where you can override the golang default value for one of fields
Tag: "latest",
}

if err := decoder.Decode(&query, r.URL.Query()); err != nil {
Expand All @@ -116,7 +118,6 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
return
}
sc := runtime.SystemContext()
tag := "latest"
options := libpod.ContainerCommitOptions{
Pause: true,
}
Expand All @@ -133,9 +134,6 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
return
}

if len(query.Tag) > 0 {
tag = query.Tag
}
options.Message = query.Comment
options.Author = query.Author
options.Pause = query.Pause
Expand All @@ -146,17 +144,23 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
return
}

// I know mitr hates this ... but doing for now
var destImage string
if len(query.Repo) > 1 {
destImage = fmt.Sprintf("%s:%s", query.Repo, tag)
destImage = fmt.Sprintf("%s:%s", query.Repo, query.Tag)
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, destImage)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
destImage = possiblyNormalizedName
}

commitImage, err := ctr.Commit(r.Context(), destImage, options)
if err != nil && !strings.Contains(err.Error(), "is not running") {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "CommitFailure"))
return
}
utils.WriteResponse(w, http.StatusOK, handlers.IDResponse{ID: commitImage.ID()}) // nolint
utils.WriteResponse(w, http.StatusCreated, handlers.IDResponse{ID: commitImage.ID()}) // nolint
}

func CreateImageFromSrc(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -195,12 +199,22 @@ func CreateImageFromSrc(w http.ResponseWriter, r *http.Request) {
}
}

reference := query.Repo
if query.Repo != "" {
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, reference)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
reference = possiblyNormalizedName
}

platformSpecs := strings.Split(query.Platform, "/")
opts := entities.ImageImportOptions{
Source: source,
Changes: query.Changes,
Message: query.Message,
Reference: query.Repo,
Reference: reference,
OS: platformSpecs[0],
}
if len(platformSpecs) > 1 {
Expand Down Expand Up @@ -250,13 +264,9 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {
return
}

fromImage := mergeNameAndTagOrDigest(query.FromImage, query.Tag)

// 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)
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, mergeNameAndTagOrDigest(query.FromImage, query.Tag))
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrap(err, "failed to resolve image name"))
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}

Expand Down Expand Up @@ -291,7 +301,7 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {

pullResChan := make(chan pullResult)
go func() {
pulledImages, err := runtime.LibimageRuntime().Pull(r.Context(), fromImage, config.PullPolicyAlways, pullOptions)
pulledImages, err := runtime.LibimageRuntime().Pull(r.Context(), possiblyNormalizedName, config.PullPolicyAlways, pullOptions)
pullResChan <- pullResult{images: pulledImages, err: err}
}()

Expand Down Expand Up @@ -371,7 +381,13 @@ func GetImage(w http.ResponseWriter, r *http.Request) {
// 404 no such
// 500 internal
name := utils.GetName(r)
newImage, err := utils.GetImage(r, name)
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, name)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}

newImage, err := utils.GetImage(r, possiblyNormalizedName)
if err != nil {
// Here we need to fiddle with the error message because docker-py is looking for "No
// such image" to determine on how to raise the correct exception.
Expand Down Expand Up @@ -483,7 +499,16 @@ func ExportImages(w http.ResponseWriter, r *http.Request) {
return
}

images := query.Names
images := make([]string, len(query.Names))
for i, img := range query.Names {
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, img)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
images[i] = possiblyNormalizedName
}

tmpfile, err := ioutil.TempFile("", "api.tar")
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "unable to create tempfile"))
Expand Down
50 changes: 38 additions & 12 deletions pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
SecurityOpt string `schema:"securityopt"`
ShmSize int `schema:"shmsize"`
Squash bool `schema:"squash"`
Tag []string `schema:"t"`
Tags []string `schema:"t"`
Target string `schema:"target"`
Timestamp int64 `schema:"timestamp"`
Ulimits string `schema:"ulimits"`
Expand All @@ -144,6 +144,9 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
}

// convert tag formats
tags := query.Tags

// convert addcaps formats
var addCaps = []string{}
if _, found := r.URL.Query()["addcaps"]; found {
Expand Down Expand Up @@ -240,8 +243,13 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}

var output string
if len(query.Tag) > 0 {
output = query.Tag[0]
if len(tags) > 0 {
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, tags[0])
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
output = possiblyNormalizedName
}
format := buildah.Dockerv2ImageManifest
registry := query.Registry
Expand All @@ -257,9 +265,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
}
}
var additionalTags []string
if len(query.Tag) > 1 {
additionalTags = query.Tag[1:]
var additionalTags []string // nolint
for i := 1; i < len(tags); i++ {
possiblyNormalizedTag, err := utils.NormalizeToDockerHub(r, tags[i])
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
additionalTags = append(additionalTags, possiblyNormalizedTag)
}

var buildArgs = map[string]string{}
Expand Down Expand Up @@ -404,6 +417,22 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
defer auth.RemoveAuthfile(authfile)

fromImage := query.From
if fromImage != "" {
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, fromImage)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}
fromImage = possiblyNormalizedName
}

systemContext := &types.SystemContext{
AuthFilePath: authfile,
DockerAuthConfig: creds,
}
utils.PossiblyEnforceDockerHub(r, systemContext)

// Channels all mux'ed in select{} below to follow API build protocol
stdout := channel.NewWriter(make(chan []byte))
defer stdout.Close()
Expand Down Expand Up @@ -458,7 +487,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
Err: auxout,
Excludes: excludes,
ForceRmIntermediateCtrs: query.ForceRm,
From: query.From,
From: fromImage,
IgnoreUnrecognizedInstructions: query.Ignore,
Isolation: isolation,
Jobs: &jobs,
Expand All @@ -481,10 +510,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
RusageLogFile: query.RusageLogFile,
Squash: query.Squash,
Target: query.Target,
SystemContext: &types.SystemContext{
AuthFilePath: authfile,
DockerAuthConfig: creds,
},
SystemContext: systemContext,
}

for _, platformSpec := range query.Platform {
Expand Down Expand Up @@ -590,7 +616,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
logrus.Warnf("Failed to json encode error %v", err)
}
flush()
for _, tag := range query.Tag {
for _, tag := range tags {
m.Stream = fmt.Sprintf("Successfully tagged %s\n", tag)
if err := enc.Encode(m); err != nil {
logrus.Warnf("Failed to json encode error %v", err)
Expand Down
10 changes: 8 additions & 2 deletions pkg/api/handlers/compat/images_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ func HistoryImage(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
name := utils.GetName(r)

newImage, _, err := runtime.LibimageRuntime().LookupImage(name, nil)
possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, name)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusNotFound, errors.Wrapf(err, "failed to find image %s", name))
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error normalizing image"))
return
}

newImage, _, err := runtime.LibimageRuntime().LookupImage(possiblyNormalizedName, nil)
if err != nil {
utils.ImageNotFound(w, possiblyNormalizedName, errors.Wrapf(err, "failed to find image %s", possiblyNormalizedName))
return
}
history, err := newImage.History(r.Context())
Expand Down
Loading