Skip to content
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

Document digest specification? #6

Closed
wking opened this issue Dec 17, 2016 · 2 comments
Closed

Document digest specification? #6

wking opened this issue Dec 17, 2016 · 2 comments

Comments

@wking
Copy link
Contributor

wking commented Dec 17, 2016

There are already some docs for the very-closely-related OCI digests, but if this spec wants to keep mixed-case hashes around (#3), it's probably worth documenting the local choices independently. For example, is there a way to identify hex hashes vs. base64 hashes (counting characters and comparing with the algorithm's hash size?)? Are hex hashes allowed to be mixed case? We probably don't want to dig into those ambiguities in this issue, I'm just asking maintainers if they plan on providing a specification for the digests that this package will parse/generate.

@stevvooe
Copy link
Contributor

@wking I think we can add a reference to Docker Registry HTTP API V2 and OCI Image Spec.

Encoding is a different issue. Basically, it is a per-alg detail. The addition of alt encoding schemes isn't a part of this package. In practice, this hasn't been an issue but we may want to narrow this package's validation.

Let me update this in the ecosystem before getting too deep here.

wking pushed a commit to wking/go-digest that referenced this issue Jan 6, 2017
CONTRIBUTING: Make leader-issues optional
@stevvooe
Copy link
Contributor

stevvooe commented Mar 7, 2017

After some thought, it is best that this package supports a superset of what is valid in the OCI specification. This package will validate the minimum safe level of digest format. Anything further than that can be done in the specification specific validator.

Thank you for you submission!

@stevvooe stevvooe closed this as completed Mar 7, 2017
wking added a commit to wking/go-digest that referenced this issue Mar 7, 2017
The docs for 'Algorithm' (now 'algorithm') made it clear that the
algorithm identifier was intended to cover both the hash and encoding
algorithms.  Stephen confirmed this interpretation in recent comments
[1,2] as well.  The idea is that a future algorithm may chose a
non-hex encoding like base 64 [1].

The previous implementation, on the other hand, baked the hex encoding
into key locations (e.g. in NewDigestFromBytes and Digest.Validate).
This commit makes the encoding internal to Agorithm instances, adding
a new Encoding interface and an Algorithm.Encoding method.  In order
support external algorithms with different encoding, I've defined an
Algorithm interface with the public API and made renamed the local
implementation of that interface to 'algorithm'.  And now that
Algorithm is a stand-alone interface, I've made the
identifier-to-Algorithm registry public by renaming 'algorithms' to
'Algorithm'.

I've also split out the flag binding into its own AlgorithmFlag
structure, because the old Algorithm.Set pointer assignment no longer
works now that Algorithm is an interface.  Having a separate interface
for the flag.Value also simplifies the Algorithm interface, which
makes it easier to write external Algorithm implementations while
still benefiting from the flag.Value helper.

API changes:

* Additions

  * Algorithms, a newly-public registry of known algorithm
    identifiers.
  * Algorithm.HashSize, which allows us to avoid a
    previously-hardcoded hex assumption in Digest.Validate().
  * Algorithm.Encoding, which allows us to implement NewDigester and
    avoid a previously-hardcoded hex assumption in NewDigestFromBytes.
  * Digest.Hash, as a better name for Digest.Hex.
  * Encoding, a new interface backing Algorithm.Encoding.
  * Hex, the lowercase base 16 Encoding.
  * NewDigester, as a more flexible replacement for NewDigest.
  * NewDigestFromHash, as a better name for NewDigestFromHex.

* Adjustments

  * Algorithm.Hash now returns nil on unavailable algorithms, to
    mirror the old API for Algorithm.Digester.

* Deprecations

  * NewDigest, because NewDigester is a more flexible form of the same
    thing that is just about as compact.
  * NewDigestFromHex, because NewDigestFromHash is a better name for
    this function.
  * Digest.Hex, because Hash is a better name for this method.

* Removals

  * Algorithm.Set, because it is not possible to support this now that
    Algorithm is an interface.

I also switched to black-box testing (with digest_test package names)
because I was getting Hex-undefined errors with white-box testing, and
we don't actually need any white-box access in our test suite.

[1]: opencontainers#3 (comment)
[2]: opencontainers#6 (comment)
wking added a commit to wking/go-digest that referenced this issue Mar 7, 2017
The docs for 'Algorithm' (now 'algorithm') made it clear that the
algorithm identifier was intended to cover both the hash and encoding
algorithms.  Stephen confirmed this interpretation in recent comments
[1,2] as well.  The idea is that a future algorithm may chose a
non-hex encoding like base 64 [1].

The previous implementation, on the other hand, baked the hex encoding
into key locations (e.g. in NewDigestFromBytes and Digest.Validate).
This commit makes the encoding internal to Agorithm instances, adding
a new Encoding interface and an Algorithm.Encoding method.  In order
support external algorithms with different encoding, I've defined an
Algorithm interface with the public API and made renamed the local
implementation of that interface to 'algorithm'.  And now that
Algorithm is a stand-alone interface, I've made the
identifier-to-Algorithm registry public by renaming 'algorithms' to
'Algorithm'.

I've also split out the flag binding into its own AlgorithmFlag
structure, because the old Algorithm.Set pointer assignment no longer
works now that Algorithm is an interface.  Having a separate interface
for the flag.Value also simplifies the Algorithm interface, which
makes it easier to write external Algorithm implementations while
still benefiting from the flag.Value helper.

API changes:

* Additions

  * Algorithms, a newly-public registry of known algorithm
    identifiers.
  * Algorithm.HashSize, which allows us to avoid a
    previously-hardcoded hex assumption in Digest.Validate().
  * Algorithm.Encoding, which allows us to implement NewDigester and
    avoid a previously-hardcoded hex assumption in NewDigestFromBytes.
  * Digest.Hash, as a better name for Digest.Hex.
  * Encoding, a new interface backing Algorithm.Encoding.
  * Hex, the lowercase base 16 Encoding.
  * NewDigester, as a more flexible replacement for NewDigest.
  * NewDigestFromHash, as a better name for NewDigestFromHex.

* Adjustments

  * Algorithm.Hash now returns nil on unavailable algorithms, to
    mirror the old API for Algorithm.Digester.

* Deprecations

  * NewDigest, because NewDigester is a more flexible form of the same
    thing that is just about as compact.
  * NewDigestFromHex, because NewDigestFromHash is a better name for
    this function.
  * Digest.Hex, because Hash is a better name for this method.

* Removals

  * Algorithm.Set, because it is not possible to support this now that
    Algorithm is an interface.

I also switched to black-box testing (with digest_test package names)
because I was getting Hex-undefined errors with white-box testing, and
we don't actually need any white-box access in our test suite.

[1]: opencontainers#3 (comment)
[2]: opencontainers#6 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
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

No branches or pull requests

2 participants