[sha512] common: enable sha512 support #376
Conversation
github-actions
bot
added
storage
There was a problem hiding this comment.
Just an extremely brief skim.
I strongly suspect that the existing approach of accepting arbitraryRepo:arbitraryTag and sha256:digestValue in the same strings does not scale and is not sustainable . Doesn’t this break existing deployments where the repo name is sha512?
I don’t know what to actually do here, mind.
common/libimage/pull.go
Outdated
| imageName = "sha256:" + storageName[1:] | ||
| // Use the configured digest algorithm for the image name | ||
| digestAlgorithm := supportedDigests.Get() | ||
| imageName = digestAlgorithm.String() + ":" + storageName[1:] |
There was a problem hiding this comment.
(See below about getImageID)
| return "sha256:" + name, nil | ||
| // Use the configured digest algorithm for the image ID | ||
| digestAlgorithm := supportedDigests.Get() | ||
| return digestAlgorithm.String() + ":" + name, nil |
There was a problem hiding this comment.
getImageID knows which algorithm was used, and it’s not necessarily this one. Applies in several other places.
| // Test SHA512 digest computation | ||
| sha512Digest1 := digest.SHA512.FromBytes(manifest1) | ||
| sha512Digest2 := digest.SHA512.FromBytes(manifest2) | ||
| sha512Digest3 := digest.SHA512.FromBytes(manifest3) | ||
|
|
||
| // Verify that SHA512 and SHA256 produce different digests | ||
| assert.NotEqual(t, digest1, sha512Digest1, "SHA512 and SHA256 digests should be different") | ||
| assert.NotEqual(t, digest2, sha512Digest2, "SHA512 and SHA256 digests should be different") | ||
| assert.NotEqual(t, digest3, sha512Digest3, "SHA512 and SHA256 digests should be different") | ||
|
|
||
| // Verify algorithm strings | ||
| assert.Equal(t, "sha256", digest1.Algorithm().String(), "original digest should be SHA256") | ||
| assert.Equal(t, "sha512", sha512Digest1.Algorithm().String(), "SHA512 digest should have correct algorithm") | ||
|
|
There was a problem hiding this comment.
This is not testing anything in this package?!
lsm5
force-pushed
the
common-agile-digest
branch
2 times, most recently
from
5c325c0 to
7ef06bb
Compare
Added a and Let me know if that still doesn't address your concern. Accounted for getImageID too and cleaned up the unnecessary tests. PTAL. Thanks again! |
- Replace hardcoded SHA256 with configurable digest algorithms using storage/pkg/supported-digests - Add centralized digest validation utilities in image/pkg/digestvalidation - Implement parameterized digest computation in image/copy/single.go - Rename DigestIfCanonicalUnknown to DigestIfConfiguredUnknown for clarity Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
- Create common/pkg/digestutils package with comprehensive digest handling functions * IsDigestReference() for robust digest detection using go-digest library * ExtractAlgorithmFromDigest() for parsing digest strings with validation * HasDigestPrefix(), GetDigestPrefix(), TrimDigestPrefix() for scalable prefix handling - Update libimage to use digestutils instead of hardcoded SHA256/SHA512 checks * Replace strings.HasPrefix() calls in filters.go, image.go, runtime.go * Support only SHA256 and SHA512 algorithms as per container-libs requirements - Update existing libimage tests to work with new digestutils functions Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
lsm5
force-pushed
the
common-agile-digest
branch
from
7ef06bb to
64cf4e0
Compare
2 participants
Depends on (and includes changes from) #374 and #375 . Maybe better to let those go in separately before this one.
(cursor assisted).