Skip to content
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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 5, 2022
Merged

Add annotations for upload blob. #2188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b03a64
Add annotations for upload blob.
Aug 12, 2022
eee2abd
Fix failing checks
Aug 22, 2022
f6a105f
Last linter errors
Aug 23, 2022
5c8b2dd
docgen
Aug 24, 2022
c937cf5
use options instead of casting to a v1.image
Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/cosign/cli/options/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type UploadBlobOptions struct {
ContentType string
Files FilesOptions
Registry RegistryOptions
Annotations map[string]string
}

var _ Interface = (*UploadBlobOptions)(nil)
Expand All @@ -35,6 +36,8 @@ func (o *UploadBlobOptions) AddFlags(cmd *cobra.Command) {

cmd.Flags().StringVar(&o.ContentType, "ct", "",
"content type to set")
cmd.Flags().StringToStringVarP(&o.Annotations, "annotation", "a", nil,
"annotations to set")
}

// UploadWASMOptions is the top level wrapper for the `upload wasm` command.
Expand Down
4 changes: 2 additions & 2 deletions cmd/cosign/cli/policy_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func initPolicy() *cobra.Command {
cremote.FileFromFlag(outfile),
}

return upload.BlobCmd(cmd.Context(), o.Registry, files, "", rootPath(o.ImageRef))
return upload.BlobCmd(cmd.Context(), o.Registry, files, nil, "", rootPath(o.ImageRef))
},
}

Expand Down Expand Up @@ -297,7 +297,7 @@ func signPolicy() *cobra.Command {
cremote.FileFromFlag(outfile),
}

return upload.BlobCmd(ctx, o.Registry, files, "", rootPath(o.ImageRef))
return upload.BlobCmd(ctx, o.Registry, files, nil, "", rootPath(o.ImageRef))
},
}

Expand Down
10 changes: 8 additions & 2 deletions cmd/cosign/cli/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ func uploadBlob() *cobra.Command {
cosign upload blob -f foo:MYOS/MYPLATFORM <IMAGE>

# upload two blobs named foo-darwin and foo-linux to the location specified by <IMAGE>, setting the os fields
cosign upload blob -f foo-darwin:darwin -f foo-linux:linux <IMAGE>`,
cosign upload blob -f foo-darwin:darwin -f foo-linux:linux <IMAGE>

# upload a blob named foo to the location specified by <IMAGE>, setting annotations mykey=myvalue.
cosign upload blob -a mykey=myvalue -f foo <IMAGE>

# upload two blobs named foo-darwin and foo-linux to the location specified by <IMAGE>, setting annotations
cosign upload blob -a mykey=myvalue -a myotherkey="my other value" -f foo-darwin:darwin -f foo-linux:linux <IMAGE>`,
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
if len(o.Files.Files) < 1 {
Expand All @@ -70,7 +76,7 @@ func uploadBlob() *cobra.Command {
return err
}

return upload.BlobCmd(cmd.Context(), o.Registry, files, o.ContentType, args[0])
return upload.BlobCmd(cmd.Context(), o.Registry, files, o.Annotations, o.ContentType, args[0])
},
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/cosign/cli/upload/blob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
cremote "github.com/sigstore/cosign/pkg/cosign/remote"
)

func BlobCmd(ctx context.Context, regOpts options.RegistryOptions, files []cremote.File, contentType, imageRef string) error {
func BlobCmd(ctx context.Context, regOpts options.RegistryOptions, files []cremote.File, annotations map[string]string, contentType, imageRef string) error {
ref, err := name.ParseReference(imageRef)
if err != nil {
return err
Expand All @@ -43,7 +43,7 @@ func BlobCmd(ctx context.Context, regOpts options.RegistryOptions, files []cremo
}
}

dgstAddr, err := cremote.UploadFiles(ref, files, mt, regOpts.GetRegistryClientOpts(ctx)...)
dgstAddr, err := cremote.UploadFiles(ref, files, annotations, mt, regOpts.GetRegistryClientOpts(ctx)...)
if err != nil {
return err
}
Expand Down
7 changes: 7 additions & 0 deletions doc/cosign_upload_blob.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions pkg/cosign/remote/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func DefaultMediaTypeGetter(b []byte) types.MediaType {
return types.MediaType(strings.Split(http.DetectContentType(b), ";")[0])
}

func UploadFiles(ref name.Reference, files []File, getMt MediaTypeGetter, remoteOpts ...remote.Option) (name.Digest, error) {
func UploadFiles(ref name.Reference, files []File, annotations map[string]string, getMt MediaTypeGetter, remoteOpts ...remote.Option) (name.Digest, error) {
var lastHash v1.Hash
var idx v1.ImageIndex = empty.Index

Expand All @@ -109,14 +109,16 @@ func UploadFiles(ref name.Reference, files []File, getMt MediaTypeGetter, remote
mt := getMt(b)
fmt.Fprintf(os.Stderr, "Uploading file from [%s] to [%s] with media type [%s]\n", f.Path(), ref.Name(), mt)

img, err := static.NewFile(b, static.WithLayerMediaType(mt))
img, err := static.NewFile(b, static.WithLayerMediaType(mt), static.WithAnnotations(annotations))
if err != nil {
return name.Digest{}, err
}

lastHash, err = img.Digest()
if err != nil {
return name.Digest{}, err
}

if err := remote.Write(ref, img, remoteOpts...); err != nil {
return name.Digest{}, err
}
Expand All @@ -128,8 +130,10 @@ func UploadFiles(ref name.Reference, files []File, getMt MediaTypeGetter, remote
if err != nil {
return name.Digest{}, err
}

blobURL := ref.Context().Registry.RegistryStr() + "/v2/" + ref.Context().RepositoryStr() + "/blobs/" + layerHash.String()
fmt.Fprintf(os.Stderr, "File [%s] is available directly at [%s]\n", f.Path(), blobURL)

if f.Platform() != nil {
idx = mutate.AppendManifests(idx, mutate.IndexAddendum{
Add: img,
Expand All @@ -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)
Copy link
Contributor

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?

Copy link
Contributor Author

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 :)

}
err := remote.WriteIndex(ref, idx, remoteOpts...)
if err != nil {
return name.Digest{}, err
Expand Down
217 changes: 217 additions & 0 deletions pkg/cosign/remote/index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,18 @@
package remote

import (
"fmt"
"io/ioutil"
"log"
"net/http/httptest"
"net/url"
"reflect"
"testing"

"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/registry"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
)

func TestFilesFromFlagList(t *testing.T) {
Expand Down Expand Up @@ -93,3 +101,212 @@ func TestFileFromFlag(t *testing.T) {
})
}
}

func TestUploadFiles(t *testing.T) {
var (
expectedRepo = "foo"
mt = DefaultMediaTypeGetter
err error
)
// Set up a fake registry (with NOP logger to avoid spamming test logs).
nopLog := log.New(ioutil.Discard, "", 0)
s := httptest.NewServer(registry.New(registry.Logger(nopLog)))
defer s.Close()
u, err := url.Parse(s.URL)
if err != nil {
t.Fatal(err)
}

tests := []struct {
name string
image string
fs []File
annotations map[string]string
wantErr bool
}{{
name: "one file",
image: "one",
fs: []File{
&file{path: "testdata/foo"},
},
wantErr: false,
}, {
name: "missing file",
image: "one-missing",
fs: []File{
&file{path: "testdata/missing"},
},
wantErr: true,
},
{
name: "two files with platform",
image: "two-platform",
fs: []File{
&file{path: "testdata/foo", platform: &v1.Platform{
OS: "darwin",
Architecture: "amd64",
}},
&file{path: "testdata/bar", platform: &v1.Platform{
OS: "linux",
Architecture: "amd64",
}},
},
wantErr: false,
}, {
name: "one file with annotations",
image: "one-annotations",
fs: []File{
&file{path: "testdata/foo"},
},
annotations: map[string]string{
"foo": "bar",
},
wantErr: false,
}, {
name: "two files with annotations",
image: "two-annotations",
fs: []File{
&file{path: "testdata/foo", platform: &v1.Platform{
OS: "darwin",
Architecture: "amd64",
}},
&file{path: "testdata/bar", platform: &v1.Platform{
OS: "linux",
Architecture: "amd64",
}},
},
annotations: map[string]string{
"foo": "bar",
"bar": "baz",
},
wantErr: false,
}}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ref, err := name.ParseReference(fmt.Sprintf("%s/%s/%s:latest", u.Host, expectedRepo, tt.image))
if err != nil {
t.Fatalf("ParseRef() = %v", err)
}
_, err = UploadFiles(ref, tt.fs, tt.annotations, mt)

if (err != nil) != tt.wantErr {
t.Errorf("UploadFiles() error = %v, wantErr %v", err, tt.wantErr)
}

if len(tt.fs) > 1 {
if !checkIndex(t, ref) {
t.Errorf("UploadFiles() index = %v", checkIndex(t, ref))
}
}

for _, f := range tt.fs {
if f.Platform() != nil {
if !checkPlatform(t, ref, f.Platform()) {
t.Errorf("UploadFiles() platform = %v was not found", checkPlatform(t, ref, f.Platform()))
}
}
}

if tt.annotations != nil {
if !checkAnnotations(t, ref, tt.annotations) {
t.Errorf("UploadFiles() annotations = %v was not found", checkAnnotations(t, ref, tt.annotations))
}
}
})
}
}

func checkPlatform(t *testing.T, ref name.Reference, p *v1.Platform) bool {
t.Helper()
d, err := remote.Get(ref)
if err != nil {
t.Fatalf("Get() = %v", err)
}

if d.MediaType.IsIndex() {
// if it is an index recurse into the index
idx, err := d.ImageIndex()
if err != nil {
t.Fatalf("ImageIndex() = %v", err)
}
manifest, err := idx.IndexManifest()
if err != nil {
t.Fatalf("IndexManifest() = %v", err)
}
for _, m := range manifest.Manifests {
if !m.MediaType.IsImage() {
return false
}
if m.Platform != nil {
if m.Platform.OS == p.OS && m.Platform.Architecture == p.Architecture {
return true
}
}
}
return false
} else if d.MediaType.IsImage() {
// if it is an image check the platform
if d.Platform != nil {
if d.Platform.OS == p.OS && d.Platform.Architecture == p.Architecture {
return true
}
}
}
return false
}

func checkIndex(t *testing.T, ref name.Reference) bool {
t.Helper()
d, err := remote.Get(ref)
if err != nil {
t.Fatalf("Get() = %v", err)
}
if d.MediaType.IsIndex() {
return true
}
return false
}

func checkAnnotations(t *testing.T, ref name.Reference, annotations map[string]string) bool {
t.Helper()
var found bool
d, err := remote.Get(ref)
if err != nil {
t.Fatalf("Get() = %v", err)
}

if d.MediaType.IsIndex() {
idx, err := remote.Index(ref)
if err != nil {
t.Fatalf("Index() = %v", err)
}
m, err := idx.IndexManifest()
if err != nil {
t.Fatalf("IndexManifest() = %v", err)
}
for annotationsKey := range annotations {
_, ok := m.Annotations[annotationsKey]
if ok {
found = true
}
}
return found
}

if d.MediaType.IsImage() {
i, err := remote.Image(ref)
if err != nil {
t.Fatalf("Image() = %v", err)
}
m, _ := i.Manifest()
for annotationsKey := range annotations {
_, ok := m.Annotations[annotationsKey]
if ok {
found = true
}
}
return found
}
return false
}
1 change: 1 addition & 0 deletions pkg/cosign/remote/testdata/bar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bar
1 change: 1 addition & 0 deletions pkg/cosign/remote/testdata/foo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo
3 changes: 3 additions & 0 deletions pkg/oci/static/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

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,
	})

Copy link
Contributor Author

@cldmnky cldmnky Oct 4, 2022 •
edited
Loading

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)

Copy link
Contributor Author

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!


// Set the Created date to time of execution
img, err = mutate.CreatedAt(img, v1.Time{Time: time.Now()})
if err != nil {
Expand Down
Loading