-
Notifications
You must be signed in to change notification settings - Fork 7.5k
fix: handle OCI digest algorithm prefix in chart downloader #31601
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
base: main
Are you sure you want to change the base?
Conversation
OCI references with tag+digest (e.g., chart:1.0@sha256:abc...) failed with "invalid byte" error because the sha256: prefix was passed to hex.DecodeString(). Signed-off-by: Evans Mungai <mbuevans@gmail.com>
a8697d7 to
c5112b9
Compare
Signed-off-by: Evans Mungai <mbuevans@gmail.com>
pkg/repo/v1/repotest/server.go
Outdated
| TestUsername string | ||
| TestPassword string | ||
| Client *ociRegistry.Client | ||
| ManifestDigest string // Digest of the pushed oci-dependent-chart manifest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit odd to put ManifestDigest here in a object about server configuration. There's no other way to validate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! I'll see how best to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TerryHowe I addressed this
|
Thanks for pulling to PR together so fast @banjoh ! |
Signed-off-by: Evans Mungai <mbuevans@gmail.com>
TerryHowe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
| if result != tt.expected { | ||
| t.Errorf("stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if result != tt.expected { | |
| t.Errorf("stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected) | |
| } | |
| assert.Equal(t, tt.expected, result) |
Use testify?
| } | ||
|
|
||
| func TestStripDigestAlgorithm(t *testing.T) { | ||
| tests := []struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a map here? e.g.
tests := map[string]struct {
input string
expected string
}{
"sha256 prefixed digest": {
input: "...
Then below:
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
What this PR does / why we need it:
OCI references with tag+digest (e.g.,
chart:1.0@sha256:abc...) failed with "invalid byte" error because thesha256:prefix was passed tohex.DecodeString().Closed: #31600
Special notes for your reviewer:
If applicable:
docs neededlabel should be applied if so)