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

Fixup search #9070

Merged
merged 1 commit into from
Jan 26, 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
28 changes: 17 additions & 11 deletions libpod/image/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) {
searchImageInRegistryHelper := func(index int, registry string) {
defer sem.Release(1)
defer wg.Done()
searchOutput := searchImageInRegistry(term, registry, options)
data[index] = searchOutputData{data: searchOutput}
searchOutput, err := searchImageInRegistry(term, registry, options)
data[index] = searchOutputData{data: searchOutput, err: err}
}

ctx := context.Background()
Expand All @@ -116,13 +116,21 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) {

wg.Wait()
results := []SearchResult{}
var lastError error
for _, d := range data {
if d.err != nil {
return nil, d.err
if lastError != nil {
logrus.Errorf("%v", lastError)
}
lastError = d.err
continue
}
results = append(results, d.data...)
}
return results, nil
if len(results) > 0 {
return results, nil
}
return results, lastError
}

// getRegistries returns the list of registries to search, depending on an optional registry specification
Expand All @@ -140,7 +148,7 @@ func getRegistries(registry string) ([]string, error) {
return registries, nil
}

func searchImageInRegistry(term string, registry string, options SearchOptions) []SearchResult {
func searchImageInRegistry(term string, registry string, options SearchOptions) ([]SearchResult, error) {
// Max number of queries by default is 25
limit := maxQueries
if options.Limit > 0 {
Expand All @@ -156,16 +164,14 @@ func searchImageInRegistry(term string, registry string, options SearchOptions)
if options.ListTags {
results, err := searchRepositoryTags(registry, term, sc, options)
if err != nil {
logrus.Errorf("error listing registry tags %q: %v", registry, err)
return []SearchResult{}
return []SearchResult{}, err
}
return results
return results, nil
}

results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit)
if err != nil {
logrus.Errorf("error searching registry %q: %v", registry, err)
return []SearchResult{}
return []SearchResult{}, err
}
index := registry
arr := strings.Split(registry, ".")
Expand Down Expand Up @@ -219,7 +225,7 @@ func searchImageInRegistry(term string, registry string, options SearchOptions)
}
paramsArr = append(paramsArr, params)
}
return paramsArr
return paramsArr, nil
}

func searchRepositoryTags(registry, term string, sc *types.SystemContext, options SearchOptions) ([]SearchResult, error) {
Expand Down
84 changes: 31 additions & 53 deletions pkg/api/handlers/compat/images_search.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
package compat

import (
"fmt"
"net/http"
"strconv"

"github.com/containers/image/v5/types"
"github.com/containers/podman/v2/libpod/image"
"github.com/containers/podman/v2/libpod"
"github.com/containers/podman/v2/libpod/define"
"github.com/containers/podman/v2/pkg/api/handlers/utils"
"github.com/containers/podman/v2/pkg/auth"
"github.com/docker/docker/api/types/registry"
"github.com/containers/podman/v2/pkg/domain/entities"
"github.com/containers/podman/v2/pkg/domain/infra/abi"
"github.com/gorilla/schema"
"github.com/pkg/errors"
)

func SearchImages(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value("runtime").(*libpod.Runtime)
decoder := r.Context().Value("decoder").(*schema.Decoder)
query := struct {
Term string `json:"term"`
Limit int `json:"limit"`
NoTrunc bool `json:"noTrunc"`
Filters map[string][]string `json:"filters"`
TLSVerify bool `json:"tlsVerify"`
ListTags bool `json:"listTags"`
}{
// This is where you can override the golang default value for one of fields
}
Expand All @@ -29,67 +34,40 @@ func SearchImages(w http.ResponseWriter, r *http.Request) {
return
}

filter := image.SearchFilter{}
if len(query.Filters) > 0 {
if len(query.Filters["stars"]) > 0 {
stars, err := strconv.Atoi(query.Filters["stars"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.Stars = stars
}
if len(query.Filters["is-official"]) > 0 {
isOfficial, err := strconv.ParseBool(query.Filters["is-official"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.IsOfficial = types.NewOptionalBool(isOfficial)
}
if len(query.Filters["is-automated"]) > 0 {
isAutomated, err := strconv.ParseBool(query.Filters["is-automated"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.IsAutomated = types.NewOptionalBool(isAutomated)
}
}
options := image.SearchOptions{
Filter: filter,
Limit: query.Limit,
}

if _, found := r.URL.Query()["tlsVerify"]; found {
options.InsecureSkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
}

_, authfile, key, err := auth.GetCredentials(r)
if err != nil {
utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String()))
return
}
defer auth.RemoveAuthfile(authfile)
options.Authfile = authfile

results, err := image.SearchImages(query.Term, options)
filters := []string{}
for key, val := range query.Filters {
filters = append(filters, fmt.Sprintf("%s=%s", key, val[0]))
}

options := entities.ImageSearchOptions{
Authfile: authfile,
Limit: query.Limit,
NoTrunc: query.NoTrunc,
ListTags: query.ListTags,
Filters: filters,
}
if _, found := r.URL.Query()["tlsVerify"]; found {
options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
}
ir := abi.ImageEngine{Libpod: runtime}
reports, err := ir.Search(r.Context(), query.Term, options)
if err != nil {
utils.BadRequest(w, "term", query.Term, err)
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
return
}

compatResults := make([]registry.SearchResult, 0, len(results))
for _, result := range results {
compatResult := registry.SearchResult{
Name: result.Name,
Description: result.Description,
StarCount: result.Stars,
IsAutomated: result.Automated == "[OK]",
IsOfficial: result.Official == "[OK]",
if !utils.IsLibpodRequest(r) {
if len(reports) == 0 {
utils.ImageNotFound(w, query.Term, define.ErrNoSuchImage)
return
}
compatResults = append(compatResults, compatResult)
}

utils.WriteResponse(w, http.StatusOK, compatResults)
utils.WriteResponse(w, http.StatusOK, reports)
}
86 changes: 0 additions & 86 deletions pkg/api/handlers/libpod/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,92 +589,6 @@ func UntagImage(w http.ResponseWriter, r *http.Request) {
utils.WriteResponse(w, http.StatusCreated, "")
}

func SearchImages(w http.ResponseWriter, r *http.Request) {
decoder := r.Context().Value("decoder").(*schema.Decoder)
query := struct {
Term string `json:"term"`
Limit int `json:"limit"`
NoTrunc bool `json:"noTrunc"`
Filters map[string][]string `json:"filters"`
TLSVerify bool `json:"tlsVerify"`
ListTags bool `json:"listTags"`
}{
// 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, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
return
}

filter := image.SearchFilter{}
if len(query.Filters) > 0 {
if len(query.Filters["stars"]) > 0 {
stars, err := strconv.Atoi(query.Filters["stars"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.Stars = stars
}
if len(query.Filters["is-official"]) > 0 {
isOfficial, err := strconv.ParseBool(query.Filters["is-official"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.IsOfficial = types.NewOptionalBool(isOfficial)
}
if len(query.Filters["is-automated"]) > 0 {
isAutomated, err := strconv.ParseBool(query.Filters["is-automated"][0])
if err != nil {
utils.InternalServerError(w, err)
return
}
filter.IsAutomated = types.NewOptionalBool(isAutomated)
}
}
options := image.SearchOptions{
Limit: query.Limit,
NoTrunc: query.NoTrunc,
ListTags: query.ListTags,
Filter: filter,
}

if _, found := r.URL.Query()["tlsVerify"]; found {
options.InsecureSkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
}

_, authfile, key, err := auth.GetCredentials(r)
if err != nil {
utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String()))
return
}
defer auth.RemoveAuthfile(authfile)
options.Authfile = authfile

searchResults, err := image.SearchImages(query.Term, options)
if err != nil {
utils.BadRequest(w, "term", query.Term, err)
return
}
// Convert from image.SearchResults to entities.ImageSearchReport. We don't
// want to leak any low-level packages into the remote client, which
// requires converting.
reports := make([]entities.ImageSearchReport, len(searchResults))
for i := range searchResults {
reports[i].Index = searchResults[i].Index
reports[i].Name = searchResults[i].Name
reports[i].Description = searchResults[i].Description
reports[i].Stars = searchResults[i].Stars
reports[i].Official = searchResults[i].Official
reports[i].Automated = searchResults[i].Automated
reports[i].Tag = searchResults[i].Tag
}

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

// ImagesBatchRemove is the endpoint for batch image removal.
func ImagesBatchRemove(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value("runtime").(*libpod.Runtime)
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 @@ -1019,7 +1019,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
// $ref: "#/responses/DocsSearchResponse"
// 500:
// $ref: '#/responses/InternalError'
r.Handle(VersionedPath("/libpod/images/search"), s.APIHandler(libpod.SearchImages)).Methods(http.MethodGet)
r.Handle(VersionedPath("/libpod/images/search"), s.APIHandler(compat.SearchImages)).Methods(http.MethodGet)
// swagger:operation GET /libpod/images/{name:.*}/get libpod libpodExportImage
// ---
// tags:
Expand Down
13 changes: 3 additions & 10 deletions test/e2e/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ registries = ['{{.Host}}:{{.Port}}']`
})

It("podman search doesn't attempt HTTP if force secure is true", func() {
SkipIfRemote("FIXME This should work on podman-remote")
if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
}
Expand All @@ -324,15 +323,11 @@ registries = ['{{.Host}}:{{.Port}}']`
registryFileTmpl.Execute(&buffer, registryEndpoints[5])
podmanTest.setRegistriesConfigEnv(buffer.Bytes())
ioutil.WriteFile(fmt.Sprintf("%s/registry5.conf", tempdir), buffer.Bytes(), 0644)
if IsRemote() {
podmanTest.RestartRemoteService()
defer podmanTest.RestartRemoteService()
}

search := podmanTest.Podman([]string{"search", image, "--tls-verify=true"})
search.WaitWithDefaultTimeout()

Expect(search.ExitCode()).To(Equal(0))
Expect(search.ExitCode()).To(Equal(125))
Expect(search.OutputToString()).Should(BeEmpty())
match, _ := search.ErrorGrepString("error")
Expect(match).Should(BeTrue())
Expand All @@ -342,7 +337,6 @@ registries = ['{{.Host}}:{{.Port}}']`
})

It("podman search doesn't attempt HTTP if registry is not listed as insecure", func() {
SkipIfRemote("FIXME This should work on podman-remote")
if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
}
Expand Down Expand Up @@ -376,7 +370,7 @@ registries = ['{{.Host}}:{{.Port}}']`
search := podmanTest.Podman([]string{"search", image})
search.WaitWithDefaultTimeout()

Expect(search.ExitCode()).To(Equal(0))
Expect(search.ExitCode()).To(Equal(125))
Expect(search.OutputToString()).Should(BeEmpty())
match, _ := search.ErrorGrepString("error")
Expect(match).Should(BeTrue())
Expand All @@ -386,7 +380,6 @@ registries = ['{{.Host}}:{{.Port}}']`
})

It("podman search doesn't attempt HTTP if one registry is not listed as insecure", func() {
SkipIfRemote("FIXME This should work on podman-remote")
if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
}
Expand Down Expand Up @@ -431,7 +424,7 @@ registries = ['{{.Host}}:{{.Port}}']`
search := podmanTest.Podman([]string{"search", "my-alpine"})
search.WaitWithDefaultTimeout()

Expect(search.ExitCode()).To(Equal(0))
Expect(search.ExitCode()).To(Equal(125))
Expect(search.OutputToString()).Should(BeEmpty())
match, _ := search.ErrorGrepString("error")
Expect(match).Should(BeTrue())
Expand Down
12 changes: 10 additions & 2 deletions test/python/docker/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,16 @@ def test_list_images(self):

def test_search_image(self):
"""Search for image"""
for r in self.client.images.search("libpod/alpine"):
self.assertIn("quay.io/libpod/alpine", r["Name"])
for r in self.client.images.search("alpine"):
self.assertIn("alpine", r["Name"])

def test_search_bogus_image(self):
"""Search for bogus image should throw exception"""
try:
r = self.client.images.search("bogus/bogus")
except:
return
self.assertTrue(len(r)==0)

def test_remove_image(self):
"""Remove image"""
Expand Down