Skip to content

hash: use generic instantiation#1538

Merged
jonjohnsonjr merged 2 commits intogoogle:mainfrom
howardjohn:hash/use-generic-instantiation
Jan 24, 2023
Merged

hash: use generic instantiation#1538
jonjohnsonjr merged 2 commits intogoogle:mainfrom
howardjohn:hash/use-generic-instantiation

Conversation

@howardjohn
Copy link
Copy Markdown
Contributor

This enables doing something like
opencontainers/go-digest#71 (comment) to override the hashing method (example: replace with sha256simd).

When an importer does not overriden, the library should behave exactly the same.

See #1330

This enables doing something like
opencontainers/go-digest#71 (comment)
to override the hashing method (example: replace with sha256simd).

When an importer does not overriden, the library should behave exactly
the same.
@jonjohnsonjr
Copy link
Copy Markdown
Collaborator

Oh that's wonderful. A few more places:

registry/manifest.go
113:		rd := sha256.Sum256(m.blob)
140:		rd := sha256.Sum256(m.blob)
156:		rd := sha256.Sum256(b.Bytes())

registry/registry_test.go
50:	h := sha256.Sum256([]byte(s))
legacy/tarball/write.go
66:	rawDigest := sha256.Sum256([]byte(s))

@howardjohn
Copy link
Copy Markdown
Contributor Author

legacy/tarball/write.go

Thanks, missed those ones! Updated a bit. Note the ignored errors match what the old function did internally so I think its safe

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 24, 2023

Codecov Report

Merging #1538 (62bb5a5) into main (efc62d8) will decrease coverage by 0.80%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1538      +/-   ##
==========================================
- Coverage   74.07%   73.27%   -0.80%     
==========================================
  Files         112      117       +5     
  Lines        8385     9015     +630     
==========================================
+ Hits         6211     6606     +395     
- Misses       1570     1748     +178     
- Partials      604      661      +57     
Impacted Files Coverage Δ
pkg/v1/stream/layer.go 88.05% <ø> (+0.38%) ⬆️
pkg/legacy/tarball/write.go 67.18% <100.00%> (ø)
pkg/registry/manifest.go 92.21% <100.00%> (-1.93%) ⬇️
pkg/v1/hash.go 89.09% <100.00%> (ø)
pkg/v1/random/image.go 78.57% <100.00%> (ø)
pkg/crane/append.go 55.17% <0.00%> (-15.67%) ⬇️
internal/retry/retry.go 86.20% <0.00%> (-13.80%) ⬇️
pkg/crane/options.go 79.16% <0.00%> (-6.55%) ⬇️
pkg/v1/remote/transport/useragent.go 41.17% <0.00%> (-5.89%) ⬇️
pkg/v1/remote/transport/transport.go 95.83% <0.00%> (-4.17%) ⬇️
... and 50 more

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

@jonjohnsonjr jonjohnsonjr merged commit d872232 into google:main Jan 24, 2023
func sha256String(s string) string {
h := sha256.Sum256([]byte(s))
return hex.EncodeToString(h[:])
h, _, _ := v1.SHA256(bytes.NewReader([]byte(s)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strings.NewReader could be better here

@imjasonh
Copy link
Copy Markdown
Contributor

We might want a lint check that crypto/sha256 doesn't sneak back in in a future change, and to steer folks toward crypto.SHA256. I know I'm going to forget about this, old habits die hard.

jonjohnsonjr pushed a commit to jonjohnsonjr/go-containerregistry that referenced this pull request Jan 25, 2023
* hash: use generic instantiation

This enables doing something like
opencontainers/go-digest#71 (comment)
to override the hashing method (example: replace with sha256simd).

When an importer does not overriden, the library should behave exactly
the same.

* catch a few more usages
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