store: add support for multiple hashAlgorithms.#2479
store: add support for multiple hashAlgorithms.#2479sgotti wants to merge 1 commit intorkt:masterfrom
Conversation
8bcebd3 to
83048b9
Compare
Actually the store just uses sha512 hash keys (half sized) since the appc/spec only uses this kind of hash. This patch adds a hashAlgorithm interface and implements two hash algorithms (the current sha512 with half sized keys like the one currently used, and a sha256) and converts the store to use this interface. This will be useful for future implementations where multiple hash types should be supported (like OCI image digests). Related to rkt#2313
83048b9 to
78d85e2
Compare
| "strings" | ||
| ) | ||
|
|
||
| type HashAlgorithm interface { |
There was a problem hiding this comment.
Could you add a top-level description of this interface?
|
Thanks! There's a couple godoc's missing (even when they're an interface implementation I think it's worth godoc'ing them". I am a bit worried about the reflect call. |
|
@s-urbaniak Thanks for the review, I'll be quite busy in the next days, but I'm working on it and I'm also trying to make it handle multiple prefix separators (for example the OCI image spec uses a About the reflection I don't think it'll have a big performance impact, another solution will be to define different hash types wrapping the |
|
@sgotti no problem, I think documenting the reasoning behind the reflection should be OK. |
|
Visited for triaging. I'm not sure about the status of this and I know it will be tangled among other OCI rework, but I guess this may be given some priority at this point. @s-urbaniak can you please shepherd this in in together with your other OCI stuff? @sgotti I tentatively mile-stoned this for next 1.9. Feel free to provide us an estimated schedule on your side, and we'll try to plan this accordingly. |
|
ack 👍 On Wed, Jun 8, 2016 at 11:02 AM, Luca Bruno [email protected]
|
|
@lucab sorry for the delay. It was an attempt to generalize multiple hash algorithms for future OCI image spec. I'll prefer to sync with @s-urbaniak on the work on oci image spec implementation with rkt before going ahead with this (see also #2541) |
|
Moving it to 1.11 milestone |
|
@sgotti or @s-urbaniak can you reassess? |
|
The new store I'm working on needs a different but also simpler way to support multiple Hash algorithms. I think this could be closed. |
|
@sgotti as per request, closing this then |
I'm not sure how in future multiple image formats (like to OCI image spec) will be implemented.
Based on #2313 my initial idea was to use different kind of stores (for example ACIStore and OCIStore) since they have different peculiarities and logics.
This patch is the first part of a store refactor to support multiple kind of stores and adds support for multiple hash algorithms.
/cc @philips
store: add support for multiple hashAlgorithms.
Actually the store just uses sha512 hash keys (half sized) since the
appc/spec only uses this kind of hash.
This patch adds a hashAlgorithm interface and implements two
hash algorithms (the current sha512 with half sized keys like the one
currently used, and a sha256) and converts the store to use this
interface.
This will be useful for future implementations where multiple hash types
should be supported (like OCI image digests).
Related to #2313