Search repository tags using --list-tags #7836

QiWang19 · 2020-09-29T20:46:04Z

For fix of BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1684263
Add --list-tags to podman search to return a table the repository tags.

Signed-off-by: Qi Wang [email protected]

docs/source/markdown/podman-search.1.md

TomSweeneyRedHat · 2020-09-29T23:20:28Z

docs/source/markdown/podman-search.1.md

@@ -65,6 +68,10 @@ Example if limit is 10 and two registries are being searched, the total
 number of results will be 20, 10 from each (if there are at least 10 matches in each).
 The order of the search results is the order in which the API endpoint returns the results.

+**--list-tags**
+
+Search tags available in the repository. Note: the result only contains the Image name and its tag.


As we can use search on registry.redhat.io/ and get all of the images in the repository:

Suggested change

Search tags available in the repository. Note: the result only contains the Image name and its tag.

List the available tags in the repository for each image found. Note: the result contains the Image name and its tag, one line for every tag associated with the image.

TomSweeneyRedHat · 2020-09-29T23:22:35Z

libpod/image/search.go

+		dockerPrefix := fmt.Sprintf("%s://", docker.Transport.Name())
+		imageRef, err := alltransports.ParseImageName(fmt.Sprintf("%s/%s", registry, term))
+		if err == nil && imageRef.Transport().Name() != docker.Transport.Name() {
+			logrus.Errorf("reference %q must be a docker reference", term)


@rhatdan got a less Docker way to say this? Perhaps:

Suggested change

logrus.Errorf("reference %q must be a docker reference", term)

logrus.Errorf("reference %q must be a valid container image reference", term)

I think "docker" is more accurate in containers-transports(5) terms as all of the transports refer to a container image.

@vrothberg sometimes I really don't like it when you're right... Let's leave it then, thx for the follow up.

TomSweeneyRedHat · 2020-09-29T23:25:52Z

test/e2e/search_test.go

+		search = podmanTest.Podman([]string{"search", "--filter=is-official", "--list-tags", "docker.io/library/alpine"})
+		search.WaitWithDefaultTimeout()
+		Expect(search.ExitCode()).To(Not(Equal(0)))
+	})


need to add a search for a registry too with a backslash only, no image name. For that and for this test, add a test to make sure the returned lines are greater than 1 and perhaps greater than 2 or 3.

I see you added the test with lines >2, ty! Could you also add a test that lists the tags for docker.io/library/ (no image, just a backslash). According to the doc, it's allowed: podman search registry.fedoraproject.org/ and if it is NOT allowed with the --list-tags option, it would be good to test that.

Test added. It's not allowed, no tags will be returned for docker.io/library/

I'm fine with not allowing it, but we really should add a note to the --list-tags option in the man page saying that, and preferably raise some kind of error message telling the user that they need to specify an image or at least return an "image not found" kind of message.

Hope I'm not being too painful @QiWang19 !

the error would be "library/" must be a docker reference". Add to the doc "the search term should be repository name". Would this work?
$ bin/podman search --list-tags docker.io/library/
ERRO[0000] error listing registry tags "docker.io": reference "library/" must be a docker reference

OK, first off, I'd still like to see a test for docker.io/library/ (or some other registry) and then test for the appropriate error message. I'm finding the man page to be very confusing at the moment, will try to suggest a rewrite.

search does not return error, it logs errors to standard logger and returns the 0 entry of the result.

Sounds good and it looks like you've added a test for that, TY!

libpod/image/search.go

vrothberg · 2020-09-30T12:45:06Z

libpod/image/search.go

+		dockerPrefix := fmt.Sprintf("%s://", docker.Transport.Name())
+		imageRef, err := alltransports.ParseImageName(fmt.Sprintf("%s/%s", registry, term))
+		if err == nil && imageRef.Transport().Name() != docker.Transport.Name() {
+			logrus.Errorf("reference %q must be a docker reference", term)


I think "docker" is more accurate in containers-transports(5) terms as all of the transports refer to a container image.

vrothberg · 2020-09-30T12:46:52Z

pkg/api/handlers/libpod/images.go

@@ -591,6 +591,7 @@ func SearchImages(w http.ResponseWriter, r *http.Request) {
 		NoTrunc   bool     `json:"noTrunc"`
 		Filters   []string `json:"filters"`
 		TLSVerify bool     `json:"tlsVerify"`
+		ListTags  bool     `json:"listTags"`


We also need to extend the swagger docs in pkg/api/server/register_images.go

QiWang19 · 2020-09-30T21:50:48Z

@containers/podman-maintainers PTAL

rhatdan · 2020-10-01T10:03:06Z

cmd/podman/images/search.go

@@ -86,6 +86,7 @@ func searchFlags(flags *pflag.FlagSet) {
 	flags.BoolVar(&searchOptions.NoTrunc, "no-trunc", false, "Do not truncate the output")
 	flags.StringVar(&searchOptions.Authfile, "authfile", auth.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
 	flags.BoolVar(&searchOptions.TLSVerifyCLI, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
+	flags.BoolVar(&searchOptions.ListTags, "list-tags", false, "Search the tags of the input registry")


List the tags ...

rhatdan · 2020-10-01T10:03:43Z

cmd/podman/images/search.go

@@ -126,6 +127,19 @@ func imageSearch(cmd *cobra.Command, args []string) error {
 		return err
 	}

+	if searchOptions.ListTags {
+		if len(searchOptions.Filters) != 0 {
+			return errors.Errorf("filters are not applicable to search tags result")


applicable to list tags result

rhatdan · 2020-10-01T10:05:21Z

libpod/image/search.go

@@ -207,6 +217,45 @@ func searchImageInRegistry(term string, registry string, options SearchOptions)
 	return paramsArr
 }

+func searchRepositoryTags(registry, term string, sc *types.SystemContext, options SearchOptions) []SearchResult {


This should return real errors, and not just write them to the screen.

rhatdan · 2020-10-01T10:05:32Z

libpod/image/search.go

+	dockerPrefix := fmt.Sprintf("%s://", docker.Transport.Name())
+	imageRef, err := alltransports.ParseImageName(fmt.Sprintf("%s/%s", registry, term))
+	if err == nil && imageRef.Transport().Name() != docker.Transport.Name() {
+		logrus.Errorf("reference %q must be a docker reference", term)


Return this error

rhatdan · 2020-10-01T18:11:53Z

/approve

openshift-ci-robot · 2020-10-01T18:11:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: QiWang19, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2020-10-01T18:13:00Z

LGTM

rhatdan · 2020-10-01T18:13:20Z

@containers/podman-maintainers PTAL

TomSweeneyRedHat · 2020-10-01T19:35:17Z

Looks like there are still comments from @vrothberg and me that have not yet been addressed....

QiWang19 · 2020-10-01T19:38:16Z

Looks like there are still comments from @vrothberg and me that have not yet been addressed....

fixed that in pkg/api/server/register_images.go

TomSweeneyRedHat · 2020-10-05T23:48:34Z

docs/source/markdown/podman-search.1.md

+
+List the available tags in the repository for each image found. Note: --list-tags requires the search term to be a full
+repository name. If not specified the registry server, default registries will be searched through (in /etc/containers/registries.conf).  The result contains the Image name and its tag, one line for every tag associated with the image.
+


This is all very contradictory. You're saying at the start that you'll list all tags for each image found in the repository, and then that the full name must be used. I think from our conversations that the image name with the repository must be supplied. I.e. you must do docker.io/library/alpine and you can't do docker.io/library/ or alpine. Given that assumption:

Suggested change

List the available tags in the repository for the specified image. **Note:** --list-tags requires the search term to be a fully

specified image name. The result contains the Image name and its tag, one line for every tag associated with the image.

I think the docker.io/library/alpine refer to a repository name, if append a tag to it, it will refer to an image?

@QiWang19 yeah, it's confusing and I may not have helped matters. When you push a container image to a registry, it creates a repository from it. How's this?

List the available tags in the repository for the specified image. **Note:** --list-tags requires the search term to be a fully specified image. The result contains the Image name and its tag, one line for every tag found in the repository.

👍
nicely done @QiWang19

For fix of BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1684263 Add --list-tags to podman search to return a table the repository tags. Signed-off-by: Qi Wang <[email protected]>

TomSweeneyRedHat

LGTM

rhatdan · 2020-10-10T11:03:28Z

/lgtm
/hold

rhatdan · 2020-10-12T10:56:22Z

/hold cancel

TomSweeneyRedHat reviewed Sep 29, 2020

View reviewed changes

docs/source/markdown/podman-search.1.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Sep 29, 2020

View reviewed changes

docs/source/markdown/podman-search.1.md Show resolved Hide resolved

TomSweeneyRedHat reviewed Sep 29, 2020

View reviewed changes

vrothberg reviewed Sep 30, 2020

View reviewed changes

QiWang19 force-pushed the search-tags branch from 00b21f1 to a58c522 Compare September 30, 2020 18:46

rhatdan reviewed Oct 1, 2020

View reviewed changes

QiWang19 force-pushed the search-tags branch from a58c522 to 9cfa149 Compare October 1, 2020 13:42

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2020

QiWang19 force-pushed the search-tags branch 5 times, most recently from 39e8d00 to 600c1dd Compare October 5, 2020 17:47

TomSweeneyRedHat reviewed Oct 5, 2020

View reviewed changes

QiWang19 force-pushed the search-tags branch 3 times, most recently from 9b70587 to b8c5af2 Compare October 7, 2020 17:47

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2020

QiWang19 force-pushed the search-tags branch from b8c5af2 to 4e4c089 Compare October 8, 2020 20:13

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2020

Search repository tags using --list-tags

66798e9

For fix of BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1684263 Add --list-tags to podman search to return a table the repository tags. Signed-off-by: Qi Wang <[email protected]>

QiWang19 force-pushed the search-tags branch from 4e4c089 to 66798e9 Compare October 9, 2020 15:49

TomSweeneyRedHat reviewed Oct 10, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2020

openshift-ci-robot assigned rhatdan Oct 10, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 10, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2020

openshift-merge-robot merged commit 212011f into containers:master Oct 12, 2020

QiWang19 deleted the search-tags branch October 12, 2020 14:43

edsantiago mentioned this pull request Oct 19, 2020

Cirrus CI: agent stopped responding #8068

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

	Search tags available in the repository. Note: the result only contains the Image name and its tag.
	List the available tags in the repository for each image found. Note: the result contains the Image name and its tag, one line for every tag associated with the image.

	logrus.Errorf("reference %q must be a docker reference", term)
	logrus.Errorf("reference %q must be a valid container image reference", term)


		List the available tags in the repository for each image found. Note: --list-tags requires the search term to be a full
		repository name. If not specified the registry server, default registries will be searched through (in /etc/containers/registries.conf). The result contains the Image name and its tag, one line for every tag associated with the image.


	List the available tags in the repository for the specified image. Note: --list-tags requires the search term to be a fully
	specified image name. The result contains the Image name and its tag, one line for every tag associated with the image.

Search repository tags using --list-tags #7836

Search repository tags using --list-tags #7836

Conversation

QiWang19 commented Sep 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomSweeneyRedHat Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QiWang19 commented Sep 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Oct 1, 2020

openshift-ci-robot commented Oct 1, 2020

rhatdan commented Oct 1, 2020

rhatdan commented Oct 1, 2020

TomSweeneyRedHat commented Oct 1, 2020

QiWang19 commented Oct 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

rhatdan commented Oct 10, 2020

rhatdan commented Oct 12, 2020

TomSweeneyRedHat Sep 30, 2020 •

edited

Loading