Skip to content

Conversation

thaJeztah
Copy link
Member

The "shortid" syntax was added in moby/moby@d26a3b3 (moby/moby#799), and allowed for matching an image on its ID prefix (this is before images were content-addressable). With the introduction of content-addressable references, this syntax became problematic, and Docker deprecated this syntax in 2016 (Docker v1.13.0) through commit; moby/moby@5fc7159 (moby/moby#27207):

The repository:shortid syntax for referencing images is very little used,
collides with tag references, and can be confused with digest references.

Support for this syntax was removed in 2017 (Docker 17.12) through commit: moby/moby@a942c92 (moby/moby#35790)

containerd uses a fork of the reference package with this syntax removed, and does not support this syntax:
containerd/containerd@901bcb2 (containerd/containerd#3728)

This patch removes the deprecated syntax, the ParseAnyReferenceWithSet function, and the ShortIdentifierRegexp regex.

As there are no external consumers for this function, nor the regexp, I'm skipping a deprecation cycle for this;

The "shortid" syntax was added in moby/moby@d26a3b3,
and allowed for matching an image on its ID prefix (this is before images were
content-addressable). With the introduction of content-addressable references,
this syntax became problematic, and Docker deprecated this syntax in 2016
(Docker v1.13.0) through commit; moby/moby@5fc7159

> The `repository:shortid` syntax for referencing images is very little used,
> collides with tag references, and can be confused with digest references.

Support for this syntax was removed in 2017 (Docker 17.12) through commit:
moby/moby@a942c92

containerd uses a fork of the reference package with this syntax removed, and
does not support this syntax:
containerd/containerd@901bcb2

This patch removes the deprecated syntax, the ParseAnyReferenceWithSet function,
and the ShortIdentifierRegexp regex.

As there are no external consumers for this function, nor the regexp, I'm
skipping a deprecation cycle for this;

- https://grep.app/search?q=.ShortIdentifierRegexp
- https://grep.app/search?q=.ParseAnyReferenceWithSet%28

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2022

Codecov Report

Base: 57.06% // Head: 56.54% // Decreases project coverage by -0.51% ⚠️

Coverage data is based on head (8cf5d11) compared to base (c47a966).
Patch coverage: 70.23% of modified lines in pull request are covered.

❗ Current head 8cf5d11 differs from pull request most recent head 6d4f62d. Consider uploading reports for the commit 6d4f62d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3774      +/-   ##
==========================================
- Coverage   57.06%   56.54%   -0.52%     
==========================================
  Files         105      104       -1     
  Lines       10806    10642     -164     
==========================================
- Hits         6166     6018     -148     
+ Misses       3952     3943       -9     
+ Partials      688      681       -7     
Impacted Files Coverage Δ
configuration/configuration.go 64.38% <ø> (ø)
context/trace.go 93.47% <ø> (ø)
contrib/token-server/main.go 0.00% <0.00%> (ø)
manifest/ocischema/manifest.go 74.19% <ø> (ø)
manifest/schema1/manifest.go 33.82% <ø> (ø)
manifest/schema1/reference_builder.go 94.00% <ø> (ø)
manifest/schema2/manifest.go 80.00% <ø> (-0.40%) ⬇️
reference/normalize.go 79.04% <ø> (+0.59%) ⬆️
reference/reference.go 79.08% <ø> (ø)
reference/regexp.go 90.90% <ø> (ø)
... and 37 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thaJeztah
Copy link
Member Author

Also opened cncf/cncf-fuzzing#235 to remove fuzzing for these

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants