Skip to content

Implement crane index subcommand#1561

Merged
jonjohnsonjr merged 2 commits intogoogle:mainfrom
jonjohnsonjr:copy-platforms
Feb 8, 2023
Merged

Implement crane index subcommand#1561
jonjohnsonjr merged 2 commits intogoogle:mainfrom
jonjohnsonjr:copy-platforms

Conversation

@jonjohnsonjr
Copy link
Copy Markdown
Collaborator

@jonjohnsonjr jonjohnsonjr commented Feb 8, 2023

crane index filter allows platform-based filtering of manifests in an index. This is primarily useful for folks who want to use only some entries in a base image without having to copy irrelevant platforms.

crane index append allows appending manifests to an existing index or creating a new index from scratch.

Fixes #1144
Fixes #1143
Fixes #1557

See also:

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 8, 2023 •
edited
Loading

Codecov Report

Merging #1561 (9b77df4) into main (e04520b) will increase coverage by 0.15%.
The diff coverage is 82.14%.

@@            Coverage Diff             @@
##             main    #1561      +/-   ##
==========================================
+ Coverage   72.75%   72.91%   +0.15%     
==========================================
  Files         118      118              
  Lines        9236     9408     +172     
==========================================
+ Hits         6720     6860     +140     
- Misses       1831     1856      +25     
- Partials      685      692       +7
Impacted Files Coverage Δ
pkg/v1/config.go 31.57% <0.00%> (-43.43%) ⬇️
pkg/v1/remote/write.go 62.84% <62.50%> (+0.15%) ⬆️
pkg/v1/remote/descriptor.go 72.34% <67.56%> (-1.51%) ⬇️
internal/cmd/edit.go 54.12% <85.00%> (+0.64%) ⬆️
pkg/registry/manifest.go 94.04% <100.00%> (+1.83%) ⬆️
pkg/registry/registry.go 100.00% <100.00%> (ø)
pkg/v1/platform.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tianon
Copy link
Copy Markdown
Contributor

tianon commented Feb 8, 2023

I gave Jon badly worded feedback that made him split the one command he'd created into two separate commands 😭

"filter" for something that mutates seems like an odd word choice - his original choice was crane amend, and I thought that was potentially a large source of user confusion when it'll appear right next to crane append (and "append" vs "amend" is not great - they're very related words but the two commands will do very different things and even operate on entirely different sets of objects)

IMO something like crane index amend would be totally reasonable, but it feels a little weird next to crane manifest, crane config, and crane blob that are all single-object-type getters and it feels like overkill to crane index just for a single subcommand so I don't know what to recommend 😞

I'm sorry I made this harder Jon 😭 ❤️ 🙇

@imjasonh
Copy link
Copy Markdown
Contributor

imjasonh commented Feb 8, 2023

Don't beat yourself up @tianon 😄, I recommended index filter and index append because they feel like two separate operations to me. One removes manifests from an index, one adds them.

I think I like having crane index as a place to do index-specific operations. A lot of crane implicitly deals with images (pull, append, export), but I don't think it's worth having a crane image sub-tree. Agnostic stuff like manifest, digest, blob also make sense at the root and should stay there.

Since index append is different than (image) append, I think that's a good place to start filling out an index-specific sub-command.

imjasonh
imjasonh reviewed Feb 8, 2023
View reviewed changes
@jonjohnsonjr
Implement crane index subcommand
4e3c889 
crane index filter allows platform-based filtering of manifests in an
index. This is primarily useful for folks who want to use only some
entries in a base image without having to copy irrelevant platforms.

crane index append allows appending manifests to an existing index or
creating a new index from scratch.
@jonjohnsonjr
Reuse v1.Platform parser and Stringer
4bfc05f 
There is some special handling around "all" that I left in place, but at
least we can drop some of this code.
@jonjohnsonjr
Copy link
Copy Markdown
Collaborator Author

jonjohnsonjr commented Feb 8, 2023

@thesayyn FYI, would be interested in your feedback.

I'm not sure if I'm ready to go full-on CEL integration :P

@jonjohnsonjr jonjohnsonjr merged commit 2ceebaa into google:main Feb 8, 2023
@jonjohnsonjr jonjohnsonjr deleted the copy-platforms branch February 8, 2023 22:03
@thesayyn
Copy link
Copy Markdown
Contributor

thesayyn commented Feb 16, 2023

sorry, I was off last week due to the earthquake. this looks as good as it could get without having a DSL or using CEL.

@thesayyn thesayyn mentioned this pull request Feb 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@tianon tianon tianon left review comments

@imjasonh imjasonh imjasonh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@imjasonh imjasonh

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

question: how can I copy image between registries for multiple OS/arch ? RFC: crane amend crane: how to copy a sub-set of a image platforms?

5 participants

@jonjohnsonjr @codecov-commenter @tianon @imjasonh @thesayyn