podman-search: refactor code to libpod/image/search.go

Refactor the image-search logic from cmd/podman/search.go to
libpod/image/search.go and update podman-search and the Varlink API to
use it.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

3 files changed, 308 insertions, 298 deletions

diff --git a/cmd/podman/search.go b/cmd/podman/search.go
index 02f171a68..a772827a6 100644
--- a/cmd/podman/search.go
+++ b/cmd/podman/search.go
@@ -1,22 +1,14 @@
 package main
 
 import (
-	"context"
-	"reflect"
-	"strconv"
 	"strings"
-	"sync"
 
-	"github.com/containers/image/docker"
 	"github.com/containers/image/types"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/cmd/podman/formats"
-	"github.com/containers/libpod/libpod/common"
-	sysreg "github.com/containers/libpod/pkg/registries"
+	"github.com/containers/libpod/libpod/image"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	"golang.org/x/sync/semaphore"
 )
 
 const (
@@ -56,30 +48,6 @@ func init() {
 	flags.BoolVar(&searchCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries (default: true)")
 }
 
-type searchParams struct {
-	Index       string
-	Name        string
-	Description string
-	Stars       int
-	Official    string
-	Automated   string
-}
-
-type searchOpts struct {
-	filter                []string
-	limit                 int
-	noTrunc               bool
-	format                string
-	authfile              string
-	insecureSkipTLSVerify types.OptionalBool
-}
-
-type searchFilterParams struct {
-	stars       int
-	isAutomated *bool
-	isOfficial  *bool
-}
-
 func searchCmd(c *cliconfig.SearchValues) error {
 	args := c.InputArgs
 	if len(args) > 1 {
@@ -90,37 +58,24 @@ func searchCmd(c *cliconfig.SearchValues) error {
 	}
 	term := args[0]
 
-	// Check if search term has a registry in it
-	registry, err := sysreg.GetRegistry(term)
-	if err != nil {
-		return errors.Wrapf(err, "error getting registry from %q", term)
-	}
-	if registry != "" {
-		term = term[len(registry)+1:]
-	}
-
-	format := genSearchFormat(c.Format)
-	opts := searchOpts{
-		format:   format,
-		noTrunc:  c.NoTrunc,
-		limit:    c.Limit,
-		filter:   c.Filter,
-		authfile: getAuthFile(c.Authfile),
+	searchOptions := image.SearchOptions{
+		NoTrunc:  c.NoTrunc,
+		Limit:    c.Limit,
+		Filter:   c.Filter,
+		Authfile: getAuthFile(c.Authfile),
 	}
 	if c.Flag("tls-verify").Changed {
-		opts.insecureSkipTLSVerify = types.NewOptionalBool(!c.TlsVerify)
-	}
-	registries, err := getRegistries(registry)
-	if err != nil {
-		return err
+		searchOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!c.TlsVerify)
 	}
 
-	filter, err := parseSearchFilter(&opts)
+	results, err := image.SearchImages(term, searchOptions)
 	if err != nil {
 		return err
 	}
-
-	return generateSearchOutput(term, registries, opts, *filter)
+	format := genSearchFormat(c.Format)
+	out := formats.StdoutTemplateArray{Output: searchToGeneric(results), Template: format, Fields: results[0].HeaderMap()}
+	formats.Writer(out).Out()
+	return nil
 }
 
 func genSearchFormat(format string) string {
@@ -132,205 +87,9 @@ func genSearchFormat(format string) string {
 	return "table {{.Index}}\t{{.Name}}\t{{.Description}}\t{{.Stars}}\t{{.Official}}\t{{.Automated}}\t"
 }
 
-func searchToGeneric(params []searchParams) (genericParams []interface{}) {
+func searchToGeneric(params []image.SearchResult) (genericParams []interface{}) {
 	for _, v := range params {
 		genericParams = append(genericParams, interface{}(v))
 	}
 	return genericParams
 }
-
-func (s *searchParams) headerMap() map[string]string {
-	v := reflect.Indirect(reflect.ValueOf(s))
-	values := make(map[string]string, v.NumField())
-
-	for i := 0; i < v.NumField(); i++ {
-		key := v.Type().Field(i).Name
-		value := key
-		values[key] = strings.ToUpper(splitCamelCase(value))
-	}
-	return values
-}
-
-// getRegistries returns the list of registries to search, depending on an optional registry specification
-func getRegistries(registry string) ([]string, error) {
-	var registries []string
-	if registry != "" {
-		registries = append(registries, registry)
-	} else {
-		var err error
-		registries, err = sysreg.GetRegistries()
-		if err != nil {
-			return nil, errors.Wrapf(err, "error getting registries to search")
-		}
-	}
-	return registries, nil
-}
-
-func getSearchOutput(term string, registry string, opts searchOpts, filter searchFilterParams) ([]searchParams, error) {
-	// Max number of queries by default is 25
-	limit := maxQueries
-	if opts.limit != 0 {
-		limit = opts.limit
-	}
-
-	sc := common.GetSystemContext("", opts.authfile, false)
-	sc.DockerInsecureSkipTLSVerify = opts.insecureSkipTLSVerify
-	// FIXME: Set this more globally.  Probably no reason not to have it in
-	// every types.SystemContext, and to compute the value just once in one
-	// place.
-	sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath()
-	results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit)
-	if err != nil {
-		logrus.Errorf("error searching registry %q: %v", registry, err)
-		return []searchParams{}, nil
-	}
-	index := registry
-	arr := strings.Split(registry, ".")
-	if len(arr) > 2 {
-		index = strings.Join(arr[len(arr)-2:], ".")
-	}
-
-	// limit is the number of results to output
-	// if the total number of results is less than the limit, output all
-	// if the limit has been set by the user, output those number of queries
-	limit = maxQueries
-	if len(results) < limit {
-		limit = len(results)
-	}
-	if opts.limit != 0 && opts.limit < len(results) {
-		limit = opts.limit
-	}
-
-	paramsArr := []searchParams{}
-	for i := 0; i < limit; i++ {
-		if len(opts.filter) > 0 {
-			// Check whether query matches filters
-			if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) {
-				continue
-			}
-		}
-		official := ""
-		if results[i].IsOfficial {
-			official = "[OK]"
-		}
-		automated := ""
-		if results[i].IsAutomated {
-			automated = "[OK]"
-		}
-		description := strings.Replace(results[i].Description, "\n", " ", -1)
-		if len(description) > 44 && !opts.noTrunc {
-			description = description[:descriptionTruncLength] + "..."
-		}
-		name := registry + "/" + results[i].Name
-		if index == "docker.io" && !strings.Contains(results[i].Name, "/") {
-			name = index + "/library/" + results[i].Name
-		}
-		params := searchParams{
-			Index:       index,
-			Name:        name,
-			Description: description,
-			Official:    official,
-			Automated:   automated,
-			Stars:       results[i].StarCount,
-		}
-		paramsArr = append(paramsArr, params)
-	}
-	return paramsArr, nil
-}
-
-func generateSearchOutput(term string, registries []string, opts searchOpts, filter searchFilterParams) error {
-	// searchOutputData is used as a return value for searching in parallel.
-	type searchOutputData struct {
-		data []searchParams
-		err  error
-	}
-
-	// Let's follow Firefox by limiting parallel downloads to 6.
-	sem := semaphore.NewWeighted(6)
-	wg := sync.WaitGroup{}
-	wg.Add(len(registries))
-	data := make([]searchOutputData, len(registries))
-
-	getSearchOutputHelper := func(index int, registry string) {
-		defer sem.Release(1)
-		defer wg.Done()
-		searchOutput, err := getSearchOutput(term, registry, opts, filter)
-		data[index] = searchOutputData{data: searchOutput, err: err}
-	}
-
-	ctx := context.Background()
-	for i := range registries {
-		sem.Acquire(ctx, 1)
-		go getSearchOutputHelper(i, registries[i])
-	}
-
-	wg.Wait()
-	searchOutput := []searchParams{}
-	for _, d := range data {
-		if d.err != nil {
-			return d.err
-		}
-		searchOutput = append(searchOutput, d.data...)
-	}
-	if len(searchOutput) == 0 {
-		return nil
-	}
-	out := formats.StdoutTemplateArray{Output: searchToGeneric(searchOutput), Template: opts.format, Fields: searchOutput[0].headerMap()}
-	return formats.Writer(out).Out()
-}
-
-func parseSearchFilter(opts *searchOpts) (*searchFilterParams, error) {
-	filterParams := &searchFilterParams{}
-	ptrTrue := true
-	ptrFalse := false
-	for _, filter := range opts.filter {
-		arr := strings.Split(filter, "=")
-		switch arr[0] {
-		case "stars":
-			if len(arr) < 2 {
-				return nil, errors.Errorf("invalid `stars` filter %q, should be stars=<value>", filter)
-			}
-			stars, err := strconv.Atoi(arr[1])
-			if err != nil {
-				return nil, errors.Wrapf(err, "incorrect value type for stars filter")
-			}
-			filterParams.stars = stars
-			break
-		case "is-automated":
-			if len(arr) == 2 && arr[1] == "false" {
-				filterParams.isAutomated = &ptrFalse
-			} else {
-				filterParams.isAutomated = &ptrTrue
-			}
-			break
-		case "is-official":
-			if len(arr) == 2 && arr[1] == "false" {
-				filterParams.isOfficial = &ptrFalse
-			} else {
-				filterParams.isOfficial = &ptrTrue
-			}
-			break
-		default:
-			return nil, errors.Errorf("invalid filter type %q", filter)
-		}
-	}
-	return filterParams, nil
-}
-
-func matchesStarFilter(filter searchFilterParams, result docker.SearchResult) bool {
-	return result.StarCount >= filter.stars
-}
-
-func matchesAutomatedFilter(filter searchFilterParams, result docker.SearchResult) bool {
-	if filter.isAutomated != nil {
-		return result.IsAutomated == *filter.isAutomated
-	}
-	return true
-}
-
-func matchesOfficialFilter(filter searchFilterParams, result docker.SearchResult) bool {
-	if filter.isOfficial != nil {
-		return result.IsOfficial == *filter.isOfficial
-	}
-	return true
-}
diff --git a/libpod/image/search.go b/libpod/image/search.go
new file mode 100644
index 000000000..e9612ef47
--- /dev/null
+++ b/libpod/image/search.go
@@ -0,0 +1,280 @@
+package image
+
+import (
+	"context"
+	"reflect"
+	"strconv"
+	"strings"
+	"sync"
+
+	"github.com/containers/image/docker"
+	"github.com/containers/image/types"
+	"github.com/containers/libpod/libpod/common"
+	sysreg "github.com/containers/libpod/pkg/registries"
+	"github.com/fatih/camelcase"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sync/semaphore"
+)
+
+const (
+	descriptionTruncLength = 44
+	maxQueries             = 25
+	maxParallelSearches    = int64(6)
+)
+
+// SearchResult is holding image-search related data.
+type SearchResult struct {
+	// Index is the image index (e.g., "docker.io" or "quay.io")
+	Index string
+	// Name is the canoncical name of the image (e.g., "docker.io/library/alpine").
+	Name string
+	// Description of the image.
+	Description string
+	// Stars is the number of stars of the image.
+	Stars int
+	// Official indicates if it's an official image.
+	Official string
+	// Automated indicates if the image was created by an automated build.
+	Automated string
+}
+
+// SearchOptions are used to control the behaviour of SearchImages.
+type SearchOptions struct {
+	// Filter allows to filter the results.
+	Filter []string
+	// Limit limits the number of queries per index (default: 25). Must be
+	// greater than 0 to overwrite the default value.
+	Limit int
+	// NoTrunc avoids the output to be truncated.
+	NoTrunc bool
+	// Authfile is the path to the authentication file.
+	Authfile string
+	// InsecureSkipTLSVerify allows to skip TLS verification.
+	InsecureSkipTLSVerify types.OptionalBool
+}
+
+type searchFilterParams struct {
+	stars       int
+	isAutomated *bool
+	isOfficial  *bool
+}
+
+func splitCamelCase(src string) string {
+	entries := camelcase.Split(src)
+	return strings.Join(entries, " ")
+}
+
+// HeaderMap returns the headers of a SearchResult.
+func (s *SearchResult) HeaderMap() map[string]string {
+	v := reflect.Indirect(reflect.ValueOf(s))
+	values := make(map[string]string, v.NumField())
+
+	for i := 0; i < v.NumField(); i++ {
+		key := v.Type().Field(i).Name
+		value := key
+		values[key] = strings.ToUpper(splitCamelCase(value))
+	}
+	return values
+}
+
+// SearchImages searches images based on term and the specified SearchOptions
+// in all registries.
+func SearchImages(term string, options SearchOptions) ([]SearchResult, error) {
+	filter, err := parseSearchFilter(&options)
+	if err != nil {
+		return nil, err
+	}
+
+	// Check if search term has a registry in it
+	registry, err := sysreg.GetRegistry(term)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error getting registry from %q", term)
+	}
+	if registry != "" {
+		term = term[len(registry)+1:]
+	}
+
+	registries, err := getRegistries(registry)
+	if err != nil {
+		return nil, err
+	}
+
+	// searchOutputData is used as a return value for searching in parallel.
+	type searchOutputData struct {
+		data []SearchResult
+		err  error
+	}
+
+	// Let's follow Firefox by limiting parallel downloads to 6.
+	sem := semaphore.NewWeighted(maxParallelSearches)
+	wg := sync.WaitGroup{}
+	wg.Add(len(registries))
+	data := make([]searchOutputData, len(registries))
+
+	searchImageInRegistryHelper := func(index int, registry string) {
+		defer sem.Release(1)
+		defer wg.Done()
+		searchOutput, err := searchImageInRegistry(term, registry, options, filter)
+		data[index] = searchOutputData{data: searchOutput, err: err}
+	}
+
+	ctx := context.Background()
+	for i := range registries {
+		sem.Acquire(ctx, 1)
+		go searchImageInRegistryHelper(i, registries[i])
+	}
+
+	wg.Wait()
+	results := []SearchResult{}
+	for _, d := range data {
+		if d.err != nil {
+			return nil, d.err
+		}
+		results = append(results, d.data...)
+	}
+	return results, nil
+}
+
+// getRegistries returns the list of registries to search, depending on an optional registry specification
+func getRegistries(registry string) ([]string, error) {
+	var registries []string
+	if registry != "" {
+		registries = append(registries, registry)
+	} else {
+		var err error
+		registries, err = sysreg.GetRegistries()
+		if err != nil {
+			return nil, errors.Wrapf(err, "error getting registries to search")
+		}
+	}
+	return registries, nil
+}
+
+func searchImageInRegistry(term string, registry string, options SearchOptions, filter *searchFilterParams) ([]SearchResult, error) {
+	// Max number of queries by default is 25
+	limit := maxQueries
+	if options.Limit > 0 {
+		limit = options.Limit
+	}
+
+	sc := common.GetSystemContext("", options.Authfile, false)
+	sc.DockerInsecureSkipTLSVerify = options.InsecureSkipTLSVerify
+	// FIXME: Set this more globally.  Probably no reason not to have it in
+	// every types.SystemContext, and to compute the value just once in one
+	// place.
+	sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath()
+	results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit)
+	if err != nil {
+		logrus.Errorf("error searching registry %q: %v", registry, err)
+		return []SearchResult{}, nil
+	}
+	index := registry
+	arr := strings.Split(registry, ".")
+	if len(arr) > 2 {
+		index = strings.Join(arr[len(arr)-2:], ".")
+	}
+
+	// limit is the number of results to output
+	// if the total number of results is less than the limit, output all
+	// if the limit has been set by the user, output those number of queries
+	limit = maxQueries
+	if len(results) < limit {
+		limit = len(results)
+	}
+	if options.Limit != 0 && options.Limit < len(results) {
+		limit = options.Limit
+	}
+
+	paramsArr := []SearchResult{}
+	for i := 0; i < limit; i++ {
+		if len(options.Filter) > 0 {
+			// Check whether query matches filters
+			if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) {
+				continue
+			}
+		}
+		official := ""
+		if results[i].IsOfficial {
+			official = "[OK]"
+		}
+		automated := ""
+		if results[i].IsAutomated {
+			automated = "[OK]"
+		}
+		description := strings.Replace(results[i].Description, "\n", " ", -1)
+		if len(description) > 44 && !options.NoTrunc {
+			description = description[:descriptionTruncLength] + "..."
+		}
+		name := registry + "/" + results[i].Name
+		if index == "docker.io" && !strings.Contains(results[i].Name, "/") {
+			name = index + "/library/" + results[i].Name
+		}
+		params := SearchResult{
+			Index:       index,
+			Name:        name,
+			Description: description,
+			Official:    official,
+			Automated:   automated,
+			Stars:       results[i].StarCount,
+		}
+		paramsArr = append(paramsArr, params)
+	}
+	return paramsArr, nil
+}
+
+func parseSearchFilter(options *SearchOptions) (*searchFilterParams, error) {
+	filterParams := &searchFilterParams{}
+	ptrTrue := true
+	ptrFalse := false
+	for _, filter := range options.Filter {
+		arr := strings.Split(filter, "=")
+		switch arr[0] {
+		case "stars":
+			if len(arr) < 2 {
+				return nil, errors.Errorf("invalid `stars` filter %q, should be stars=<value>", filter)
+			}
+			stars, err := strconv.Atoi(arr[1])
+			if err != nil {
+				return nil, errors.Wrapf(err, "incorrect value type for stars filter")
+			}
+			filterParams.stars = stars
+			break
+		case "is-automated":
+			if len(arr) == 2 && arr[1] == "false" {
+				filterParams.isAutomated = &ptrFalse
+			} else {
+				filterParams.isAutomated = &ptrTrue
+			}
+			break
+		case "is-official":
+			if len(arr) == 2 && arr[1] == "false" {
+				filterParams.isOfficial = &ptrFalse
+			} else {
+				filterParams.isOfficial = &ptrTrue
+			}
+			break
+		default:
+			return nil, errors.Errorf("invalid filter type %q", filter)
+		}
+	}
+	return filterParams, nil
+}
+
+func matchesStarFilter(filter *searchFilterParams, result docker.SearchResult) bool {
+	return result.StarCount >= filter.stars
+}
+
+func matchesAutomatedFilter(filter *searchFilterParams, result docker.SearchResult) bool {
+	if filter.isAutomated != nil {
+		return result.IsAutomated == *filter.isAutomated
+	}
+	return true
+}
+
+func matchesOfficialFilter(filter *searchFilterParams, result docker.SearchResult) bool {
+	if filter.isOfficial != nil {
+		return result.IsOfficial == *filter.isOfficial
+	}
+	return true
+}
diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go
index 6e4974dc4..e80a47684 100644
--- a/pkg/varlinkapi/images.go
+++ b/pkg/varlinkapi/images.go
@@ -13,7 +13,6 @@ import (
 
 	"github.com/containers/buildah"
 	"github.com/containers/buildah/imagebuildah"
-	"github.com/containers/image/docker"
 	dockerarchive "github.com/containers/image/docker/archive"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/transports/alltransports"
@@ -22,7 +21,6 @@ import (
 	"github.com/containers/libpod/cmd/podman/varlink"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/image"
-	sysreg "github.com/containers/libpod/pkg/registries"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/containers/libpod/utils"
 	"github.com/containers/storage/pkg/archive"
@@ -437,53 +435,26 @@ func (i *LibpodAPI) RemoveImage(call iopodman.VarlinkCall, name string, force bo
 // SearchImages searches all registries configured in /etc/containers/registries.conf for an image
 // Requires an image name and a search limit as int
 func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool) error {
-	sc := image.GetSystemContext("", "", false)
-	if tlsVerify != nil {
-		sc.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!*tlsVerify)
+	searchOptions := image.SearchOptions{
+		Limit:                 1000,
+		InsecureSkipTLSVerify: types.NewOptionalBool(!*tlsVerify),
 	}
-	var registries []string
-
-	// Check if search query has a registry in it
-	registry, err := sysreg.GetRegistry(query)
+	results, err := image.SearchImages(query, searchOptions)
 	if err != nil {
-		return call.ReplyErrorOccurred(fmt.Sprintf("error getting registry from %q: %q", query, err))
-	}
-	if registry != "" {
-		registries = append(registries, registry)
-		query = query[len(registry)+1:]
-	} else {
-		registries, err = sysreg.GetRegistries()
-		if err != nil {
-			return call.ReplyErrorOccurred(fmt.Sprintf("unable to get system registries: %q", err))
-		}
+		return call.ReplyErrorOccurred(err.Error())
 	}
+
 	var imageResults []iopodman.ImageSearchResult
-	for _, reg := range registries {
-		var lim = 1000
-		if limit != nil {
-			lim = int(*limit)
-		}
-		results, err := docker.SearchRegistry(getContext(), sc, reg, query, lim)
-		if err != nil {
-			// If we are searching multiple registries, don't make something like an
-			// auth error fatal. Unfortunately we cannot differentiate between auth
-			// errors and other possibles errors
-			if len(registries) > 1 {
-				continue
-			}
-			return call.ReplyErrorOccurred(err.Error())
-		}
-		for _, result := range results {
-			i := iopodman.ImageSearchResult{
-				Registry:     reg,
-				Description:  result.Description,
-				Is_official:  result.IsOfficial,
-				Is_automated: result.IsAutomated,
-				Name:         result.Name,
-				Star_count:   int64(result.StarCount),
-			}
-			imageResults = append(imageResults, i)
+	for _, result := range results {
+		i := iopodman.ImageSearchResult{
+			Registry:     result.Index,
+			Description:  result.Description,
+			Is_official:  result.Official == "[OK]",
+			Is_automated: result.Automated == "[OK]",
+			Name:         result.Name,
+			Star_count:   int64(result.Stars),
 		}
+		imageResults = append(imageResults, i)
 	}
 	return call.ReplySearchImages(imageResults)
 }