-
Notifications
You must be signed in to change notification settings - Fork 557
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 annotations for upload blob. #2188
Conversation
566a58b
to
028a730
Compare
Codecov Report
@@ Coverage Diff @@
## main #2188 +/- ##
==========================================
+ Coverage 26.03% 27.22% +1.19%
==========================================
Files 131 131
Lines 7689 7709 +20
==========================================
+ Hits 2002 2099 +97
+ Misses 5445 5341 -104
- Partials 242 269 +27
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Add tests for UploadFile() Signed-off-by: Magnus Bengtsson <[email protected]>
Signed-off-by: Magnus Bengtsson <[email protected]>
f16de30
to
eee2abd
Compare
Signed-off-by: Magnus Bengtsson <[email protected]>
Signed-off-by: Magnus Bengtsson <[email protected]>
Signed-off-by: Magnus Bengtsson <[email protected]>
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
@priyawadhwa @dlorenc @mattmoor how do you want to proceed with 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.
sorry for the delayed review! left a couple comments :)
@@ -46,6 +46,9 @@ func NewFile(payload []byte, opts ...Option) (oci.File, error) { | |||
return nil, err | |||
} | |||
|
|||
// Add annotations from options | |||
img = mutate.Annotations(img, o.Annotations).(v1.Image) |
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.
Instead of calling mutate again, I think you can add the annotations in the addendum on line 42?
img, err := mutate.Append(base, mutate.Addendum{
Layer: layer,
Annotations: annotations,
})
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.
Hmmm when using addendum the annotations are not correctly added to the image as it seems. I do believe that the image needs to be annotated (as for CreatedAt)
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.
sorry for the delayed review! left a couple comments :)
No worries!
@@ -141,6 +145,9 @@ func UploadFiles(ref name.Reference, files []File, getMt MediaTypeGetter, remote | |||
} | |||
|
|||
if len(files) > 1 { | |||
if annotations != nil { | |||
idx = mutate.Annotations(idx, annotations).(v1.ImageIndex) |
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 we're already including annotations when the file is initially created in line 112, do we need to add them in again here?
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.
Since this is only run if we have an image index, the annotations are (also) added to the image index, the annotations on line 112 are added to the the image it self. So when accessing a multiplatform image index the annotations needs to be on the index, for a plain image it's enough for them to be on the image.
The tests should handle 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.
Looks good, thank you!
This PR adds support for adding annotations to images and index manifests when uploading blobs (#2149)
Summary
We use
cosign upload blob
to upload and sign releases of a cli tool. We. would like to add a bit more metadata to our artifacts when we upload them, such as release name and other things.Our custom tool is then able to query or registry and display the annotations on the artifacts for our users.
Added tests for the
UploadFiles()
function since the permutations increased. The tests should cover the change.Release Note
Added an
--annotations
flag to thecosign upload blob
command that sets (unsigned) annotations on the OCI images written by the command.Documentation