Adds "docker-list-tags" command to list tags #716

Sep 10, 2019 · 2019-09-10T16:34:46Z

Adds new command docker-list-tags to list the tags in a docker V2 registry's repository.

Discussion in #276. I expect some changes will be needed in the naming so opening this early to facilitate discussion.

If there is any additional testing or docs required I will gladly update.

mtrmac

Thanks! This is pretty good.

cmd/skopeo/tags.go

cmd/skopeo/tags_test.go

docs/skopeo-docker-list-tags.1.md

docs/skopeo.1.md

docs/skopeo-docker-list-tags.1.md

Sep 10, 2019 · 2019-09-10T17:03:45Z

Thanks! This is pretty good.

Thanks for the super fast response. Will review and make changes as recommended.

cmd/skopeo/tags.go

cmd/skopeo/tags_test.go

rhatdan · 2019-09-11T15:09:17Z

Need bash completions.

Sep 11, 2019 · 2019-09-11T16:41:42Z

@rhatdan yes, as discussed in the issue, I don't really care about the name. The contention seems to be with others' opinions on naming scope. I'm happy to use 'list-tags', 'list-registry-tags', 'list-repository-tags', or whatever as long as its descriptive of what the command does.

How about 'list-repository-tags'? @mtrmac is that ok? I'd prefer not to make the change more than once, so if we can get agreement here first then I'll change it.

Thanks again. I very much appreciate the prompt reviews and suggestions.

mtrmac · 2019-09-11T20:03:39Z

PLEASE just call this list-tags, do not add docker-. If this only works currently with docker API, then just document it. Lets not make the skopeo CLI any more complicated then it already is.

Well, the thing is: Are you willing to bet that no new protocols and server kinds which could use a “list tags” operation will be added during the lifetime of Skopeo? Considering the long discussions about file-at-a-time sync, I’m really not.

And if there are going to be new protocols and server kinds, then the CLI will need to accommodate them. If we don’t add the protocol anywhere in the CLI now, we’ll end up requiring an extra option, or a non-trivial command name, in the future, for the new protocols/server kinds. If skopeo list-tags were created now for docker/distribution, any future protocol would have to use skopeo list-tags --fedora or skopeo list-fedora-tags in the future, no matter how prevalent the future protocol became = no matter how confusing it were to have the shorter command not apply to the prevalent protocol.

Being explicit about the protocol now allows us to symmetrically handle future protocols without expressing a preference (who knows, the next one may well turn out to be a vendor-named one as well).

Also, note that this command is pretty likely to be primarily used in scripts / automation. So, in a sense, the user-friendliness does not matter nearly as much as stable behavior. (So, I’m absolutely unwilling to do, or design this to eventually require, heuristic guessing about the protocol in Skopeo — we can’t know that it will be possible to safely guess that with future unknown protocols anyway.)

(And, as far as user-friendliness goes, with completions, list-* are all exactly as easy to type.)

If Podman wants to provide easier-to-type code with more heuristics, it can of course do that; looking at how the semantics of the currently-existing heuristics in there turned out to be unclear and inconsistent, I wouldn’t recommend it but I can’t and won’t try to stop it.)

How about 'list-repository-tags'

I’m not happy about that name but I guess I could settle on that if @rhatdan can; it’s still very generic, but at least I can console myself with the thought that calling image servers “registries” does not make any semantic sense, so at least there’s some chance that the new protocol won’t use this jargon and we won’t collide.

rhatdan · 2019-09-12T01:23:45Z

I am not tighing the list-tags to only container-registries. If other containers stores supported tags, then list-tags should show there as well.

I could see skopeo list-tags listing tags for an images stored in container-storage and docker-daemon. Perhaps.

The question you raise is do I see some time in the future where list-tags would talk about something totally different on a a different type of image store, and my answer would be no.

mtrmac · 2019-09-12T20:26:10Z

I am not tighing the list-tags to only container-registries. If other containers stores supported tags, then list-tags should show there as well.

I could see skopeo list-tags listing tags for an images stored in container-storage and docker-daemon. Perhaps.

Then the CLI must be explicit about which “container store” the user is asking about.

rhatdan · 2019-09-12T22:36:44Z

Yes I agree, the container store needs to be specified.

Sep 13, 2019 · 2019-09-13T19:24:42Z

Yes I agree, the container store needs to be specified.

As in: list-tags docker://host.io:port/repo and later list-tags container-storage://path?

I'm fixing up integration tests now, and will get the updates pushed here shortly

mtrmac · 2019-09-13T20:08:07Z

Yes I agree, the container store needs to be specified.

As in: list-tags docker://host.io:port/repo and later list-tags container-storage://path?

docker://host.io:port/repo is defined to be the same as docker://host.io:port/repo:latest; and use of that syntax implies that other image references (like dir:path) can probably be accepted as well.

So, if the semantics of the parameter here is different, I think it should use a different syntax, maybe list-docker tags or list-tags --docker; of the two, the former is easier to use with shell completions.

rhatdan · 2019-09-13T20:35:06Z

Why is the additional option needed?
Isn't this enough for the tool to interpret what the user wants?

skopeo list-tags  docker://host.io:port/repo
skopeo list-tags container-storage

rhatdan · 2019-10-31T12:31:19Z

@zhill @mtrmac @vrothberg What is the state of this PR?

Oct 31, 2019 · 2019-10-31T18:36:38Z

I have pushed updates and am ready for more feedback re:naming. I had let it sit for a while b/c my cycles to operate on feedback were limited. I'd like to get a clear direction on what to name the command and how params should work if not the way it is currently implemented. I can then rebase and get the PR ready to merge

rhatdan · 2019-10-31T18:42:43Z

@giuseppe @mtrmac Do we need to add this as a topic for the F2F?

docs/skopeo-list-repository-tags.1.md

completions/bash/skopeo

cmd/skopeo/tags_test.go

docs/skopeo.1.md

mtrmac · 2019-10-31T19:52:16Z

@rhatdan From my POV this is now mostly up to you to state goals and/or ask ask for a design, and if both, to make them consistent with each other. E.g. #716 (comment) asks a question that I think answered in the immediately preceding comment already.

Or maybe just say that you are fine with list-repository-tags $docker_reference as it is now.

Nov 5, 2019 · 2019-11-05T00:08:36Z

Thanks, I'll get the updates done ASAP, but cycles are scarce until the end of the week. I appreciate your time reviewing and helping move this forward.

mtrmac

OK, I guess the discussion period was long enough. Last chance to object to skopeo list-repository-tags $docker-reference.

(@vrothberg WDYT?)

These two are still outstanding:
#716 (comment)
#716 (comment)
that, and the mis-placed Error marshalling json output error are the only things I care somewhat strongly enough; the rest are very low-priority and non-blocking.

cmd/skopeo/tags.go

cmd/skopeo/tags_test.go

integration/list_repository_tags_test.go

Jan 9, 2020 · 2020-01-09T17:14:22Z

Thanks for the review (again). I'll get updates in today. I've been very cycle constrained lately but have time to focus on this and get it over the finish line. Thanks!

vrothberg · 2020-01-09T17:14:32Z

(@vrothberg WDYT?)

Thanks for the ping! I will have a look at this PR tomorrow morning with a fresh pair of eyes.

vrothberg · 2020-01-09T17:15:11Z

@zhill, can you also rebase the PR so we can merge it more easily?

Jan 9, 2020 · 2020-01-09T17:17:48Z

Yes, working on that now. I'll rebase and squash it down to a single commit now that we're close (didn't want to lose history previously). Thanks again folks! I know its been a long PR due to my intermittent availability, the patience is appreciated.

vrothberg

Just a couple of nits mainly related to naming

cmd/skopeo/tags.go

vrothberg · 2020-01-10T10:04:24Z

cmd/skopeo/tags.go

+	}
+
+	return cli.Command{
+		Name:  "list-repository-tags",


I have a preference to name the command list-tags and require the input to follow transport:.... convention. Maybe we want to support something similar for other transports in the future. Embedding "repository" into the command name and making it docker-transport only would close that door in some way.

The code wouldn't need to change much as we can error out for other transports for now. But I am late to the game and may miss context.

@mtrmac WDYT?

My worry is that it’s not certain that things can be generalized for all transports.

It’s already a bit conceptually dubious with docker://, where docker://foo means “the docker://foo:latest image”, not “the docker://foo repository”.

In general, the whole idea of “repository” and “tag” does generalize at all well — compare #716 (comment) (besides syntax problems) suggesting that the whole local storage is a single “repository” and the hostname/[ns/…/]repo:tag value is a “tag”: it’s not at all an unreasonable position to take, but it makes the concept of “tag” meaningless and rules out the possibility of any transport-agnostic consumer of the output of this command.

(Compare #682 (comment) and #524 (comment) dealing with registry names, all of this ends up with several docker:// syntaxes with different meanings and rules.)

My default heuristic in such cases is to intentionally make the command extremely specific and limited; if we ever generalize it, we can easily add a general command, and maintain the specific one as a trivial shim. Going the other way from a prematurely-general interface to a specific one, or changing the prematurely-general interface to be general in a different way, requires better ability to forecast the future.

But, overall, sure:

list-tags docker://… and list-docker-tags … are equivalent in the information that is being provided

The “docker://repo means :latest” quibble is pedantic but does not make a difference in practice (apart from our ability to cleanly reject tagged/digested input, but this PR does not reject it anyway)

list-repository-tags … is already an specifics-losing compromise that I’m not at all strongly attached to

As long as the input is clearly namespaced and therefore unambiguous (i.e. not skopeo list heuristically-guess-what-I-mean), I think any of the three options works, and we’ve taken 4 months to decide and it’s not going to get any easier with 4 more months.

@vrothberg So, to settle this once for all, skopeo list-tags docker://…?

cmd/skopeo/tags.go

docs/skopeo-list-repository-tags.1.md

docs/skopeo.1.md

Jan 14, 2020 · 2020-01-14T06:14:59Z

I've made the changes and rebased along with squashing to a single commit for easier review and merge.

cmd/skopeo/tags.go

mtrmac · 2020-01-14T17:20:51Z

cmd/skopeo/tags.go

+	}
+
+	return cli.Command{
+		Name:  "list-repository-tags",


My worry is that it’s not certain that things can be generalized for all transports.

It’s already a bit conceptually dubious with docker://, where docker://foo means “the docker://foo:latest image”, not “the docker://foo repository”.

In general, the whole idea of “repository” and “tag” does generalize at all well — compare #716 (comment) (besides syntax problems) suggesting that the whole local storage is a single “repository” and the hostname/[ns/…/]repo:tag value is a “tag”: it’s not at all an unreasonable position to take, but it makes the concept of “tag” meaningless and rules out the possibility of any transport-agnostic consumer of the output of this command.

(Compare #682 (comment) and #524 (comment) dealing with registry names, all of this ends up with several docker:// syntaxes with different meanings and rules.)

My default heuristic in such cases is to intentionally make the command extremely specific and limited; if we ever generalize it, we can easily add a general command, and maintain the specific one as a trivial shim. Going the other way from a prematurely-general interface to a specific one, or changing the prematurely-general interface to be general in a different way, requires better ability to forecast the future.

But, overall, sure:

list-tags docker://… and list-docker-tags … are equivalent in the information that is being provided

The “docker://repo means :latest” quibble is pedantic but does not make a difference in practice (apart from our ability to cleanly reject tagged/digested input, but this PR does not reject it anyway)

list-repository-tags … is already an specifics-losing compromise that I’m not at all strongly attached to

As long as the input is clearly namespaced and therefore unambiguous (i.e. not skopeo list heuristically-guess-what-I-mean), I think any of the three options works, and we’ve taken 4 months to decide and it’s not going to get any easier with 4 more months.

@vrothberg So, to settle this once for all, skopeo list-tags docker://…?

Jan 14, 2020 · 2020-01-14T19:15:27Z

Once the naming decision is made I'll update with a new commit for that change. Thanks @mtrmac and @vrothberg !

vrothberg · 2020-01-15T09:31:45Z

@vrothberg So, to settle this once for all, skopeo list-tags docker://…?

Ay! I am in favor of that. Thanks!

rhatdan · 2020-01-15T17:31:44Z

I am also.

mtrmac

Thanks! A few comments after a quick skim, I’ll do a complete review later.

cmd/skopeo/list_tags.go

cmd/skopeo/list_tags_test.go

mtrmac

A full review now.

integration/list_tags_test.go

docs/skopeo-list-tags.1.md

cmd/skopeo/list_tags.go

Jan 23, 2020 · 2020-01-23T08:14:00Z

apologies for going dark, some other stuff has come up but I'll get to this ASAP. I don't see any issues with making the suggested changes and I'll get those in as soon as I can but may be few more days.

containers#276 Example: skopeo list-tags docker://docker.io/library/centos Returns response: { Repository": "docker.io/library/centos", "Tags": [ "6", "7", ... ] } Signed-off-by: Zach Hill <[email protected]>

Signed-off-by: Zach Hill <[email protected]>

…t already tested by the upstream container/images repo or local unit tests Signed-off-by: Zach Hill <[email protected]>

Signed-off-by: Zach Hill <[email protected]>

Feb 24, 2020 · 2020-02-24T19:12:42Z

I'll update the branch to latest, but I think I resolved all the comments. Note that I did remove the integration test since there wasn't one for the inspect command and such a test was dependent on an external registry and didn't really provide a lot of value that was't covered by the unit tests. I can add it back in if you feel it provides real test value moving forward.

mtrmac

Thanks! Sorry it took so long.

Note that I did remove the integration test since there wasn't one for the inspect command and such a test was dependent on an external registry and didn't really provide a lot of value that was't covered by the unit tests. I can add it back in if you feel it provides real test value moving forward.

If you don’t think a test would be useful enough to keep this code working that it’s worth the trouble, sure, up to you.

force-pushed the tag_list branch from b4c851c to e1b5007 Compare September 10, 2019 16:45

mtrmac reviewed Sep 10, 2019

View reviewed changes

rhatdan reviewed Sep 11, 2019

View reviewed changes

cmd/skopeo/tags.go Outdated Show resolved Hide resolved

cmd/skopeo/tags.go Outdated Show resolved Hide resolved

cmd/skopeo/tags_test.go Outdated Show resolved Hide resolved

cmd/skopeo/tags_test.go Outdated Show resolved Hide resolved

force-pushed the tag_list branch from 5d9e927 to a58d82f Compare September 24, 2019 06:43

mtrmac reviewed Oct 31, 2019

View reviewed changes

docs/skopeo-list-repository-tags.1.md Outdated Show resolved Hide resolved

completions/bash/skopeo Outdated Show resolved Hide resolved

cmd/skopeo/tags_test.go Outdated Show resolved Hide resolved

docs/skopeo.1.md Outdated Show resolved Hide resolved

force-pushed the tag_list branch 3 times, most recently from af52650 to 07ee608 Compare December 13, 2019 17:33

mtrmac reviewed Jan 9, 2020

View reviewed changes

vrothberg reviewed Jan 10, 2020

View reviewed changes

force-pushed the tag_list branch from 0d2fb3b to 7fc4aa7 Compare January 14, 2020 05:13

mtrmac reviewed Jan 14, 2020

View reviewed changes

force-pushed the tag_list branch from 0e3e2d1 to 9d9028a Compare January 16, 2020 23:14

mtrmac reviewed Jan 16, 2020

View reviewed changes

mtrmac reviewed Jan 18, 2020

View reviewed changes

February 12, 2020 00:03

Adds "list-tags" command to list tags with no known tag required. Fixes

cce44c4

containers#276 Example: skopeo list-tags docker://docker.io/library/centos Returns response: { Repository": "docker.io/library/centos", "Tags": [ "6", "7", ... ] } Signed-off-by: Zach Hill <[email protected]>

Fix formatting on test

b0cfab1

Signed-off-by: Zach Hill <[email protected]>

Fix inconsistency in manpage example for list-tags

5554964

Signed-off-by: Zach Hill <[email protected]>

Updates based on code review to simplify logic and tests

36fdc06

Signed-off-by: Zach Hill <[email protected]>

Remove the list_tags integration test since it does not cover much no…

a9854e1

…t already tested by the upstream container/images repo or local unit tests Signed-off-by: Zach Hill <[email protected]>

force-pushed the tag_list branch from e8236e4 to a9854e1 Compare February 12, 2020 19:12

Fix wrong import of docker reference

07c89b4

Signed-off-by: Zach Hill <[email protected]>

Merge branch 'master' into tag_list

0a91c00

mtrmac approved these changes Feb 25, 2020

View reviewed changes

mtrmac merged commit d7a2bd7 into containers:master Feb 25, 2020

arturrez mentioned this pull request Mar 18, 2020

list-tags is broken for skopeo 0.1.41 #859

Closed

xelalexv mentioned this pull request Jun 19, 2020

Omitting tag during sync results in an error when using skopeo, works using docker xelalexv/dregsy#13

Closed

github-actions bot added the locked - please file new issue/PR label Sep 30, 2023

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds "docker-list-tags" command to list tags #716

Adds "docker-list-tags" command to list tags #716

Sep 10, 2019

mtrmac left a comment

Sep 10, 2019

rhatdan commented Sep 11, 2019

Sep 11, 2019

mtrmac commented Sep 11, 2019

rhatdan commented Sep 12, 2019

mtrmac commented Sep 12, 2019

rhatdan commented Sep 12, 2019

Sep 13, 2019

mtrmac commented Sep 13, 2019

rhatdan commented Sep 13, 2019

rhatdan commented Oct 31, 2019

Oct 31, 2019

rhatdan commented Oct 31, 2019

mtrmac commented Oct 31, 2019

Nov 5, 2019

mtrmac left a comment

Jan 9, 2020

vrothberg commented Jan 9, 2020

vrothberg commented Jan 9, 2020

Jan 9, 2020

vrothberg left a comment

vrothberg Jan 10, 2020

mtrmac Jan 14, 2020

Jan 14, 2020

mtrmac Jan 14, 2020

Jan 14, 2020

vrothberg commented Jan 15, 2020

rhatdan commented Jan 15, 2020

mtrmac left a comment

mtrmac left a comment

Jan 23, 2020

Feb 24, 2020

mtrmac left a comment

Adds "docker-list-tags" command to list tags #716

Adds "docker-list-tags" command to list tags #716

Conversation

mtrmac left a comment

Choose a reason for hiding this comment

rhatdan commented Sep 11, 2019

mtrmac commented Sep 11, 2019

rhatdan commented Sep 12, 2019

mtrmac commented Sep 12, 2019

rhatdan commented Sep 12, 2019

mtrmac commented Sep 13, 2019

rhatdan commented Sep 13, 2019

rhatdan commented Oct 31, 2019

rhatdan commented Oct 31, 2019

mtrmac commented Oct 31, 2019

mtrmac left a comment

Choose a reason for hiding this comment

vrothberg commented Jan 9, 2020

vrothberg commented Jan 9, 2020

vrothberg left a 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

vrothberg commented Jan 15, 2020

rhatdan commented Jan 15, 2020

mtrmac left a comment

Choose a reason for hiding this comment

mtrmac left a comment

Choose a reason for hiding this comment

mtrmac left a comment

Choose a reason for hiding this comment