digest: allow separators in algorithm field #33
Conversation
|
Oh interesting to bring that back. When might it be needed? |
In the case where we want some parameterized algorithm agility. I explained this much better in opencontainers/image-spec#654. One example was |
|
This regexp still differs from what we have defined in distribution, but what we defined in distribution my be too strict. Is there any reason to disallow the algorithm/component part from starting with a number? |
|
cool cool. @stevvooe should this pr include a bit of docs explaining that? |
|
oh i see the image-spec explanation. |
|
Change to not allow a number as the first character should be in different PR, if we want that |
|
LGTM
…On Tue, Apr 25, 2017 at 1:59 PM Derek McGowan ***@***.***> wrote:
Change to not allow a number as the first character should be in different
PR, if we want that
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEF6VB9rVTACRekwai0szHXL8vdJ8LPks5rzjRzgaJpZM4NGxtv>
.
|
In the past, we have had support for separators in the algorithm field to allow parameterization of digest algorithms. A classic example is `tarsum+sha256`. While this particular case is deprecated, support for this case in the future must be allow in case we bring this back. This will ensure that implementations these as valid digest, but correctly report error when the algorithm is unsupported, rather than this being treated as an invalid format. We also widen the character set allowed in the hex encoded portion of the digest to allow support for future encodings that are not hex-based. Signed-off-by: Stephen J Day <stephen.day@docker.com>
stevvooe
force-pushed
the
future-proof-algorithm-field
branch
from
e7722e0
to
5ab10f5
Compare
|
@vbatts Brought this into line with opencontainers/image-spec#654. |
|
@opencontainers/go-digest-maintainers PTAL |
digest.go
Outdated
| } | ||
|
|
||
| // NewDigestFromEncoded returns a Digest from alg and a the encoded digest. | ||
| func NewDigestFromEncoded(alg Algorithm, hex string) Digest { |
|
This change looks reasonable. From my understanding we are stating with this change that any given digest algorithm value can only have one valid output format, which is crucial for the usefulness of this package. The existing algorithm could then be implied to be Is it worth keeping deprecated functions pre-1.0 of this package? It is an easy translation to use this newer form. |
We made a change to call the _hex_ portion the _encoded_ portion and this makes a few updates to make that clearer for prospective users of this package. Signed-off-by: Stephen J Day <stephen.day@docker.com>
stevvooe
force-pushed
the
future-proof-algorithm-field
branch
from
0a8e2cf
to
55f6758
Compare
We can do a full pass on that later. I think this up to the community. |
|
LGTM |
Signed-off-by: Stephen J Day <stephen.day@docker.com>
stevvooe
force-pushed
the
future-proof-algorithm-field
branch
from
d15bd73
to
678a95e
Compare
|
Updated per opencontainers/image-spec#654. @opencontainers/go-digest-maintainers PTAL |
|
LGTM |
1 similar comment
|
LGTM |
In the past, we have had support for separators in the algorithm field
to allow parameterization of digest algorithms. A classic example is
tarsum+sha256. While this particular case is deprecated, support forthis case in the future must be allow in case we bring this back. This
will ensure that implementations these as valid digest, but correctly
report error when the algorithm is unsupported, rather than this being
treated as an invalid format.
Signed-off-by: Stephen J Day stephen.day@docker.com