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

Remove two GetImages functions from API #12572

Merged
merged 1 commit into from
Jan 15, 2022
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
54 changes: 41 additions & 13 deletions pkg/api/handlers/compat/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/containers/buildah"
"github.com/containers/common/libimage"
"github.com/containers/common/pkg/config"
"github.com/containers/common/pkg/filters"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/types"
"github.com/containers/podman/v3/libpod"
Expand Down Expand Up @@ -404,25 +405,52 @@ func GetImage(w http.ResponseWriter, r *http.Request) {
}

func GetImages(w http.ResponseWriter, r *http.Request) {
images, err := utils.GetImages(w, r)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Failed get images"))
decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
query := struct {
All bool
Digests bool
Filter string // Docker 1.24 compatibility
}{
// This is where you can override the golang default value for one of fields
}

if err := decoder.Decode(&query, r.URL.Query()); err != nil {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
return
}
if _, found := r.URL.Query()["digests"]; found && query.Digests {
utils.UnSupportedParameter("digests")
return
}

summaries := make([]*entities.ImageSummary, 0, len(images))
for _, img := range images {
// If the image is a manifest list, extract as much as we can.
if isML, _ := img.IsManifestList(r.Context()); isML {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For compat api we were ignoring manifest list I think merging this with common code would start printing manifest lists as well.

I have only looked at the code yet. But I'll need to try this out inoder to make it sure that we are not breaking any existing behavior.

Copy link
Member Author

@rhatdan rhatdan Dec 22, 2021

Choose a reason for hiding this comment

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

This filter
filterList = append(filterList, "manifest=false")

Should remove all manifest images for the DockerAPI.

continue
filterList, err := filters.FiltersFromRequest(r)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
return
}
if !utils.IsLibpodRequest(r) {
if len(query.Filter) > 0 { // Docker 1.24 compatibility
filterList = append(filterList, "reference="+query.Filter)
}
filterList = append(filterList, "manifest=false")
}

is, err := handlers.ImageToImageSummary(img)
Copy link
Member

Choose a reason for hiding this comment

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

By skipping the call to ImageToImageSummary:

  • ImageSummary.VirtualSize will be incorrect.
  • It appears ID will no longer be prefixed with sha256:
  • ImageSummary.RepoDigests will no longer be formatted rather the raw data will be returned.
  • ...

Copy link
Member Author

Choose a reason for hiding this comment

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

I have fixed the sha256.
VirtualSize seems to be the same between the two calls. (It is always just size).
I switched libpod output for RepoDigests to match Dockers.
I did not see any others.

if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Failed transform image summaries"))
return
imageEngine := abi.ImageEngine{Libpod: runtime}

listOptions := entities.ImageListOptions{All: query.All, Filter: filterList}
summaries, err := imageEngine.List(r.Context(), listOptions)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
return
}

if !utils.IsLibpodRequest(r) {
// docker adds sha256: in front of the ID
for _, s := range summaries {
s.ID = "sha256:" + s.ID
}
summaries = append(summaries, is)
}
utils.WriteResponse(w, http.StatusOK, summaries)
}
Expand Down
43 changes: 0 additions & 43 deletions pkg/api/handlers/libpod/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/containers/buildah"
"github.com/containers/common/libimage"
"github.com/containers/common/pkg/filters"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/types"
"github.com/containers/podman/v3/libpod"
Expand Down Expand Up @@ -103,48 +102,6 @@ func GetImage(w http.ResponseWriter, r *http.Request) {
utils.WriteResponse(w, http.StatusOK, inspect)
}

func GetImages(w http.ResponseWriter, r *http.Request) {
decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
query := struct {
All bool
Digests bool
Filter string // Docker 1.24 compatibility
}{
// This is where you can override the golang default value for one of fields
}

if err := decoder.Decode(&query, r.URL.Query()); err != nil {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
return
}
if _, found := r.URL.Query()["digests"]; found && query.Digests {
utils.UnSupportedParameter("digests")
return
}

filterList, err := filters.FiltersFromRequest(r)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
return
}
if !utils.IsLibpodRequest(r) && len(query.Filter) > 0 { // Docker 1.24 compatibility
filterList = append(filterList, "reference="+query.Filter)
}

imageEngine := abi.ImageEngine{Libpod: runtime}

listOptions := entities.ImageListOptions{All: query.All, Filter: filterList}
summaries, err := imageEngine.List(r.Context(), listOptions)
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
return
}

utils.WriteResponse(w, http.StatusOK, summaries)
}

func PruneImages(w http.ResponseWriter, r *http.Request) {
var (
err error
Expand Down
40 changes: 0 additions & 40 deletions pkg/api/handlers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,46 +184,6 @@ type ExecStartConfig struct {
Width uint16 `json:"w"`
}

func ImageToImageSummary(l *libimage.Image) (*entities.ImageSummary, error) {
options := &libimage.InspectOptions{WithParent: true, WithSize: true}
imageData, err := l.Inspect(context.TODO(), options)
if err != nil {
return nil, errors.Wrapf(err, "failed to obtain summary for image %s", l.ID())
}

containers, err := l.Containers()
if err != nil {
return nil, errors.Wrapf(err, "failed to obtain Containers for image %s", l.ID())
}
containerCount := len(containers)

isDangling, err := l.IsDangling(context.TODO())
if err != nil {
return nil, errors.Wrapf(err, "failed to check if image %s is dangling", l.ID())
}

is := entities.ImageSummary{
// docker adds sha256: in front of the ID
ID: "sha256:" + l.ID(),
ParentId: imageData.Parent,
RepoTags: imageData.RepoTags,
RepoDigests: imageData.RepoDigests,
Created: l.Created().Unix(),
Size: imageData.Size,
SharedSize: 0,
VirtualSize: imageData.VirtualSize,
Labels: imageData.Labels,
Containers: containerCount,
ReadOnly: l.IsReadOnly(),
Dangling: isDangling,
Names: l.Names(),
Digest: string(imageData.Digest),
ConfigDigest: "", // TODO: libpod/image didn't set it but libimage should
History: imageData.NamesHistory,
}
return &is, nil
}

func ImageDataToImageInspect(ctx context.Context, l *libimage.Image) (*ImageInspect, error) {
options := &libimage.InspectOptions{WithParent: true, WithSize: true}
info, err := l.Inspect(context.Background(), options)
Expand Down
40 changes: 0 additions & 40 deletions pkg/api/handlers/utils/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"strings"

"github.com/containers/common/libimage"
"github.com/containers/common/pkg/filters"
"github.com/containers/image/v5/docker"
storageTransport "github.com/containers/image/v5/storage"
"github.com/containers/image/v5/transports/alltransports"
Expand All @@ -16,7 +15,6 @@ import (
"github.com/containers/podman/v3/pkg/util"
"github.com/containers/storage"
"github.com/docker/distribution/reference"
"github.com/gorilla/schema"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -91,44 +89,6 @@ func ParseStorageReference(name string) (types.ImageReference, error) {
return imageRef, nil
}

// GetImages is a common function used to get images for libpod and other compatibility
// mechanisms
func GetImages(w http.ResponseWriter, r *http.Request) ([]*libimage.Image, error) {
decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
query := struct {
All bool
Digests bool
Filter string // Docker 1.24 compatibility
}{
// This is where you can override the golang default value for one of fields
}

if err := decoder.Decode(&query, r.URL.Query()); err != nil {
return nil, err
}
if _, found := r.URL.Query()["digests"]; found && query.Digests {
UnSupportedParameter("digests")
}

filterList, err := filters.FiltersFromRequest(r)
if err != nil {
return nil, err
}
if !IsLibpodRequest(r) && len(query.Filter) > 0 { // Docker 1.24 compatibility
filterList = append(filterList, "reference="+query.Filter)
}

if !query.All {
// Filter intermediate images unless we want to list *all*.
// NOTE: it's a positive filter, so `intermediate=false` means
// to display non-intermediate images.
filterList = append(filterList, "intermediate=false")
}
listOptions := &libimage.ListImagesOptions{Filters: filterList}
return runtime.LibimageRuntime().ListImages(r.Context(), nil, listOptions)
}

func GetImage(r *http.Request, name string) (*libimage.Image, error) {
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
image, _, err := runtime.LibimageRuntime().LookupImage(name, nil)
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/server/register_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
// $ref: "#/responses/LibpodImageSummaryResponse"
// 500:
// $ref: '#/responses/InternalError'
r.Handle(VersionedPath("/libpod/images/json"), s.APIHandler(libpod.GetImages)).Methods(http.MethodGet)
r.Handle(VersionedPath("/libpod/images/json"), s.APIHandler(compat.GetImages)).Methods(http.MethodGet)
// swagger:operation POST /libpod/images/load libpod ImageLoadLibpod
// ---
// tags:
Expand Down
11 changes: 6 additions & 5 deletions pkg/domain/infra/abi/images_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions)

summaries := []*entities.ImageSummary{}
for _, img := range images {
digests := make([]string, len(img.Digests()))
for j, d := range img.Digests() {
digests[j] = string(d)
repoDigests, err := img.RepoDigests()
if err != nil {
return nil, errors.Wrapf(err, "getting repoDigests from image %q", img.ID())
}
isDangling, err := img.IsDangling(ctx)
if err != nil {
Expand All @@ -37,11 +37,12 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions)

e := entities.ImageSummary{
ID: img.ID(),
// ConfigDigest: string(img.ConfigDigest),
// TODO: libpod/image didn't set it but libimage should
// ConfigDigest: string(img.ConfigDigest),
Created: img.Created().Unix(),
Dangling: isDangling,
Digest: string(img.Digest()),
RepoDigests: digests,
RepoDigests: repoDigests,
History: img.NamesHistory(),
Names: img.Names(),
ReadOnly: img.IsReadOnly(),
Expand Down