Leverage distribution/reference for local index #505
Conversation
This has 2 upside: - it enforces bundle names in the local index are valid repo names - it uses a proven image reference parser instead of relying on raw strings package Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
|
Hey Simon, we intentionally forked pkg/reference so we could remove the default docker.io domain on normalized names. We should still keep it in and use that instead. See #485 |
|
Hmm, ok. We have to check on the implication for push/pull to a registry though |
|
I keep this one as is, for after Monday's discussion (e.g.: should we remove the custom registry and adopt image registries instead leveraging #442 until we get the proper manifest-list based OCI representation) |
| func (i Index) Add(name, version string, digest string) { | ||
| if tags, ok := i[name]; ok { | ||
| tags[version] = digest | ||
| func (i Index) Add(ref reference.NamedTagged, digest string) { |
There was a problem hiding this comment.
I'm not too fond of the idea of having such a tight dependency on reference in this package unless there was some gain in utility. The only two changes this package makes is by using ref.Tag() and reference.FamiliarName(), which could've been achieved by using idx.Add(reference.FamiliarName(ref), ref.Tag()) prior to this PR. From an external consumer's point of view, relying on strings rather than references makes the package more easily consumable, testable and maintainable. Most of the changes proposed in this package just add another dependency to the package without gaining much in the way of maintainability/cleanliness IMO.
For example, we've talked about potentially restricting version specifiers to a semver tag, which would be much easier to parse as strings rather than a reference which would require de-serializing into a tag, then parsed into a string to be passed along to a semver checker.
Any opposing thoughts on this?
FYI we're already kinda going ahead with that. We've hidden all repo-related commands and removed them from the documentation to focus primarily on the CNAB runtime while we figure out what we want to do with the bundle registry API. See #491 for more context on that discussion |
yeah fair enough. For pushing and pulling Docker images we should probably use distribution's reference package, but for bundles we don't want the tight dependency on DockerHub as the default registry of choice. There's some discussion on that over in #426 (comment) if you have an opinion :) |
This has 2 upside: