-
Notifications
You must be signed in to change notification settings - Fork 109
Remove deprecated StoreTransport.GetStoreImage, GetImage #840
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,29 +45,6 @@ type StoreTransport interface { | |
| SetStore(storage.Store) | ||
| // GetStoreIfSet returns the default store for this transport, or nil if not set/determined yet. | ||
| GetStoreIfSet() storage.Store | ||
| // GetImage retrieves the image from the transport's store that's named | ||
| // by the reference. | ||
| // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, | ||
| // this ignores that ID; and repeated calls of GetStoreImage with the same named reference | ||
| // can return different images, with no way for the caller to "freeze" the storage.Image identity | ||
| // without discarding the name entirely. | ||
| // | ||
| // Use storage.ResolveReference instead; note that if the image is not found, ResolveReference returns | ||
| // c/image/v5/storage.ErrNoSuchImage, not c/storage.ErrImageUnknown. | ||
| GetImage(types.ImageReference) (*storage.Image, error) | ||
| // GetStoreImage retrieves the image from a specified store that's named | ||
| // by the reference. | ||
| // | ||
| // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, | ||
| // this ignores that ID; and repeated calls of GetStoreImage with the same named reference | ||
| // can return different images, with no way for the caller to "freeze" the storage.Image identity | ||
| // without discarding the name entirely. | ||
| // | ||
| // Also, a StoreTransport reference already contains a store, so providing another one is redundant. | ||
| // | ||
| // Use storage.ResolveReference instead; note that if the image is not found, ResolveReference returns | ||
| // c/image/v5/storage.ErrNoSuchImage, not c/storage.ErrImageUnknown. | ||
| GetStoreImage(storage.Store, types.ImageReference) (*storage.Image, error) | ||
| // ParseStoreReference parses a reference, overriding any store | ||
| // specification that it may contain. | ||
| ParseStoreReference(store storage.Store, reference string) (*storageReference, error) | ||
|
|
@@ -306,46 +283,6 @@ func (s *storageTransport) ParseReference(reference string) (types.ImageReferenc | |
| return s.ParseStoreReference(store, reference) | ||
| } | ||
|
|
||
| // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, | ||
| // this ignores that ID; and repeated calls of GetStoreImage with the same named reference | ||
| // can return different images, with no way for the caller to "freeze" the storage.Image identity | ||
| // without discarding the name entirely. | ||
| // | ||
| // Also, a StoreTransport reference already contains a store, so providing another one is redundant. | ||
| // | ||
| // Use storage.ResolveReference instead; note that if the image is not found, ResolveReference returns | ||
| // c/image/v5/storage.ErrNoSuchImage, not c/storage.ErrImageUnknown. | ||
| func (s storageTransport) GetStoreImage(store storage.Store, ref types.ImageReference) (*storage.Image, error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I defer to @mtrmac but he is committed to having a stable image API so that means not removing them. If the deprecation notice was not right than you also cannot assume callers knew this and fixed it in the meantime.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes; we are at c/image/v5 and I’m stubborn about not moving to v6 unless absolutely unavoidable. |
||
| dref := ref.DockerReference() | ||
| if dref != nil { | ||
| if img, err := store.Image(dref.String()); err == nil { | ||
| return img, nil | ||
| } | ||
| } | ||
| if sref, ok := ref.(*storageReference); ok { | ||
| tmpRef := *sref | ||
| if img, err := tmpRef.resolveImage(nil); err == nil { | ||
| return img, nil | ||
| } | ||
| } | ||
| return nil, storage.ErrImageUnknown | ||
| } | ||
|
|
||
| // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, | ||
| // this ignores that ID; and repeated calls of GetStoreImage with the same named reference | ||
| // can return different images, with no way for the caller to "freeze" the storage.Image identity | ||
| // without discarding the name entirely. | ||
| // | ||
| // Use storage.ResolveReference instead; note that if the image is not found, ResolveReference returns | ||
| // c/image/v5/storage.ErrNoSuchImage, not c/storage.ErrImageUnknown. | ||
| func (s *storageTransport) GetImage(ref types.ImageReference) (*storage.Image, error) { | ||
| store, err := s.GetStore() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return s.GetStoreImage(store, ref) | ||
| } | ||
|
|
||
| func (s storageTransport) ValidatePolicyConfigurationScope(scope string) error { | ||
| // Check that there's a store location prefix. Values we're passed are | ||
| // expected to come from PolicyConfigurationIdentity or | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This did not actually stop using the old function… it does not even compile.
Absolutely non-blocking: It wouldn’t hurt to tighten the code (and field documentation!) here,
l.instances[""]is, AFAICS, always an image ID. (Annoyingly,instances[""]andinstances[realDigest]have different value formats!).So, instead of
ParseStoreReference, this could useNewStoreReference(store, nil, theID). And then this call to essentially obtainimg.IDis not really necessary.