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
Add annotation for descriptor #4106
Conversation
Thanks for the pr. Can you please describe the motivation and purpose for this change? |
@Jamstah Please review again. ^_^ |
OK, great, I'm caught up :) Can you please add a test for this? |
Test case added. |
I ran the tests without the code change and they passed - I don't think the new tests are actually testing that the change makes it back to the descriptor otherwise they would have failed. |
Signed-off-by: Tosone <i@tosone.cn>
After i remove the changes will get this error: === RUN TestManifestUnmarshal
manifest_test.go:159: manifest annotation not equal:
expected:
map[hot:potato]
actual:
map[]
--- FAIL: TestManifestUnmarshal (0.00s) === RUN TestOCIManifestIndexUnmarshal
index_test.go:146: manifest index annotation not equal:
expected:
map[com.example.favourite-colour:blue com.example.locale:en_GB]
actual:
map[]
--- FAIL: TestOCIManifestIndexUnmarshal (0.00s) Also length check added: if len(descriptor.Annotations) != 2 {
t.Fatalf("manifest index annotation length should be 2")
} |
Apologies, you're absolutely right. It turns out vscode was caching more aggressively than I expected. Changes look great. One (hopefully) last question: What does this fix from an external point of view? The changes all make sense, but is there something broken for users that this fixes? |
Yes, my new project sigma, need to unmarshal the manifest, in some scenario, the annotation has some information, and some of the code snippet like here. https://github.com/go-sigma/sigma/blob/main/pkg/daemon/tagpushed/tagpushed.go#L96 |
OK, so you're using the project as a library to parse manifests? Thanks :) |
yes. |
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.
Nice! Thanks!
distribution.Descriptor
defined a fieldannotation
, butdistribution.UnmarshalManifest
outputdescriptor
cannot get the annotation.We should fill the field correctly.