Allow `cosign save` to reuse save directory to help dedupe shared layers. #3239

amartin120 · 2023-09-13T18:55:05Z

Summary

When using cosign save with dozens, or in my case, hundreds of images, the storage consumption can really add up with each image requiring their own directory. There's potentially a case where images can share layers so this change allows for the cosign save function to be able to reuse a directory from a previous save and just build on the index.json along with an additional annotation for each entry to aide with the corresponding cosign load.

From there, I've modified cosign load to have a new optional flag --registry where you can specify a registry to load all of the images to from a single directory. In cases where you are only dealing with a single image, the load command would function exactly how it does today where you just provide the remote reference via the cli.

Release Note

Updated cosign save to append the index.json if pointed at a --dir used in a previous save.
Added the org.opencontainers.image.ref.name annotation to the index.json for clarity and to help with cosign load in cases where the directory is reused.
Updated cosign load to have an optional flag called --registry that can be used in cases where the index.json from a save contains multiple images. This handles being able to control where the images get loaded to in the case where providing a single ref via the CLI no longer makes sense. If you don't provide the --registry flag, the cosign load command will continue to function as it does today.

Documentation

Example usage:
cosign save image1:v1.0 --dir ~/saves
cosign save image2:v1.0 --dir ~/saves

cat ~/saves/index.json

{
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 1581,
         "digest": "sha256:c5b6da2ffbc9f63e8d250d5985f722830dfb5cc8958e8f64f7e14edc18b584fa",
         "annotations": {
            "kind": "dev.cosignproject.cosign/image",
            "org.opencontainers.image.ref.name": "image1:v1.0"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 558,
         "digest": "sha256:077bc8e8cfe1577f89959c749da1fbe941bde8b35b7ccdfe4f5832fb5ade2ec1",
         "annotations": {
            "kind": "dev.cosignproject.cosign/sigs",
            "org.opencontainers.image.ref.name": "image1:v1.0"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 468,
         "digest": "sha256:7e6fb78b8f35c859c9200567229a4350c17e057d4d13c80121a3f603748a1cb0",
         "annotations": {
            "kind": "dev.cosignproject.cosign/atts",
            "org.opencontainers.image.ref.name": "image1:v1.0"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
         "size": 967,
         "digest": "sha256:7e42b1a631859c6740835659a022ed96097955dde2707f32e7a089f5b5f64c06",
         "annotations": {
            "kind": "dev.cosignproject.cosign/imageIndex",
            "org.opencontainers.image.ref.name": "image2:v1.0"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 558,
         "digest": "sha256:88145843e5dcd7a0d515cf874997b1d4eaa742a1594dc75c6bd111f109cf4455",
         "annotations": {
            "kind": "dev.cosignproject.cosign/sigs",
            "org.opencontainers.image.ref.name": "image2:v1.0"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 469,
         "digest": "sha256:6fa5a413e06af7babaeae39bb06c92b563463080fd46003b078050787c3ef6f0",
         "annotations": {
            "kind": "dev.cosignproject.cosign/atts",
            "org.opencontainers.image.ref.name": "image2:v1.0"
         }
      }
   ]
}

cosign load --dir ~/saves --registry your.registry.com

adjust load to handle multi-image index from save fix image index bug - save and load fix image ref trunc issue export oci.layout constants to use with oci.remote slightly clean up test Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

pkg/oci/layout/write.go

Co-authored-by: Jacob Blain Christen <dweomer5@gmail.com> Signed-off-by: Adam Martin <42001113+amartin120@users.noreply.github.com>

pkg/oci/layout/write.go

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

codecov · 2023-10-03T15:03:20Z

Codecov Report

Merging #3239 (e2de0a4) into main (1ee8bf9) will decrease coverage by 0.01%.
Report is 86 commits behind head on main.
The diff coverage is 12.10%.

❗ Current head e2de0a4 differs from pull request most recent head 861bfe6. Consider uploading reports for the commit 861bfe6 to get more accurate results

@@            Coverage Diff             @@
##             main    #3239      +/-   ##
==========================================
- Coverage   30.37%   30.36%   -0.01%     
==========================================
  Files         155      155              
  Lines        9835    10117     +282     
==========================================
+ Hits         2987     3072      +85     
- Misses       6402     6587     +185     
- Partials      446      458      +12

Files	Coverage Δ
cmd/cosign/cli/options/registry.go	`0.00% <ø> (ø)`
cmd/cosign/cli/save.go	`0.00% <0.00%> (ø)`
pkg/oci/layout/index.go	`28.57% <66.66%> (ø)`
cmd/cosign/cli/options/load.go	`0.00% <0.00%> (ø)`
pkg/oci/layout/write.go	`37.68% <41.66%> (+0.18%)`	⬆️
cmd/cosign/cli/load.go	`0.00% <0.00%> (ø)`
pkg/oci/remote/write.go	`8.92% <0.00%> (-5.26%)`	⬇️

... and 20 files with indirect coverage changes

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

priyawadhwa

Hey @amartin120 thanks for contributing this! I had a couple thoughts:

Instead of a --registry flag on load, maybe it would make sense to take a regex on the image name? That way you could say something like "load all the images" or "load all the images from a specific registry"
Would you mind adding an e2e test similar to this one to test this new functionality?

I wasn't able to find anything specific in the oci-layout spec which says that storing multiple images in index.json is valid. If you have any pointers for that it would be helpful. Otherwise though, I read through this github thread and it seems reasonable to me.

Let me know if you have any questions. Thank you!

amartin120 · 2023-10-04T21:03:23Z

Hello @priyawadhwa. Thanks for the feedback!

Instead of a --registry flag on load, maybe it would make sense to take a regex on the image name? That way you could say something like "load all the images" or "load all the images from a specific registry"

I'm looking at this from a scenario where I'm ...

saving images from "Registry A"
tar/compressing the directory used as the output
walking it over to an air-gapped environment
loading it all to "Registry B" and maybe even a subsequent call to load "Registry C".

So with that I kind of need to specify the registry when the load command is called. Does that help?

I'll get to work on those e2e tests a.s.a.p.

priyawadhwa · 2023-10-05T23:34:04Z

@amartin120 thanks for the explanation, I think I misunderstood before what the --registry flag was for!

We have this environment variable we use to signal pushing signatures to a different registry called COSIGN_REPOSITORY -- https://github.com/sigstore/cosign#specifying-registry

WDYT of reusing that instead of the flag?

amartin120 · 2023-10-06T02:12:02Z

I'll take a look at COSIGN_REPOSITORY. Thanks for the info!

amartin120 · 2023-10-09T18:09:57Z

@priyawadhwa I apologize for the delay. COSIGN_REPOSITORY seems like a good option for a single repository. However, this PR allows for loading multiple images in a single cosign load execution which could load to multiple repositories, just in another registry.

We could still reuse COSIGN_REPOSITORY if we could just put a registry name in there instead of a full repository designation. Would this still make sense over a separate registry flag?

Hopefully I'm understanding things correctly here. Thanks for your patience.

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

priyawadhwa · 2023-10-25T23:41:53Z

@amartin120 no worries, I think we can just go ah we can keep the flag since it seems more accurately named and more intuitive. I can take a look at this once CI is green!

amartin120 · 2023-10-26T02:05:06Z

@priyawadhwa Thank you. I may need some help with the CI. Both of the failing stages look like timeouts as opposed to true failures. Am I reading the logs right?

priyawadhwa · 2023-10-26T21:11:24Z

@amartin120 I think the linter is picking up some things that need to be fixed! If you go through the files on this PR it should be visible in the Github UI:

Error: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
  Error: ineffectual assignment to err (ineffassign)
  Error: ineffectual assignment to err (ineffassign)
  Error: ineffectual assignment to err (ineffassign)
  Error: SA[40](https://github.com/sigstore/cosign/actions/runs/6551101939/job/18094664470?pr=3239#step:4:42)06: this value of `err` is never used (staticcheck)
  Error: SA4006: this value of `err` is never used (staticcheck)
  Error: SA4006: this value of `err` is never used (staticcheck)

amartin120 · 2023-10-27T13:32:54Z

Strange. Those were not showing on the PR when I responded originally. Maybe a Github hiccup at the time? Maybe I was just blind? The logs just showed timeouts. Oh well. I'll work on this today now that I can see them. Thanks again for the patience.

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

dweomer · 2023-11-02T21:59:47Z

Oh, hey! All checks are all passing. Any objections to merging this @priyawadhwa?

priyawadhwa

Thanks for your patience, I left a few comments!

priyawadhwa · 2023-11-03T22:27:35Z

cmd/cosign/cli/load.go

+		Short:   "Load a signed image on disk to a remote registry",
+		Long:    "Load a signed image on disk to a remote registry",
+		Example: `  cosign load --dir <path to directory> <IMAGE> OR cosign load --dir <path to directory> --registry <REGISTRY>`,
+		//Args:             cobra.ExactArgs(1),


nit: we should probably delete this

cmd/cosign/cli/load.go

priyawadhwa · 2023-11-03T22:37:42Z

pkg/oci/remote/write.go

+// WriteSignedImageIndexImagesBulk writes the images within the image index.
+// Bulk version.  Uses targetRegistry for multiple images/sigs/atts.
+// This includes the signed image and associated signatures in the image index
+func WriteSignedImageIndexImagesBulk(targetRegistry string, sii oci.SignedImageIndex, opts ...Option) error {


I wonder if we could refactor the copy command here, which will handle copying over the image and any signatures/attestations associated with it, rather than having to do it manually here.

Right now the copy takes a source image and a destination image as a string. We could look into pulling out the copy logic into a reusable function which takes a SignedEntity and passing through to that here?

Good thoughts. Let me look into this more.

My only concern for the copy command logic is the usabiltiy in an air-gapped situation where cosign may not be able to reach the source image in its original registry/repo. Will the copy mechanics work if the SignedEntity is what's on local disk via cosign save?

Apologies if I'm way off here. I'm just making sure that I understand.

pkg/oci/remote/write.go

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

multi-image index from save

3e8f793

adjust load to handle multi-image index from save fix image index bug - save and load fix image ref trunc issue export oci.layout constants to use with oci.remote slightly clean up test Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

dweomer reviewed Sep 13, 2023

View reviewed changes

pkg/oci/layout/write.go Outdated Show resolved Hide resolved

dweomer reviewed Sep 13, 2023

View reviewed changes

pkg/oci/layout/write.go Outdated Show resolved Hide resolved

dweomer reviewed Sep 13, 2023

View reviewed changes

pkg/oci/layout/write.go Outdated Show resolved Hide resolved

amartin120 and others added 2 commits September 13, 2023 15:45

clean up truth table to be less complex

c51b9fa

Co-authored-by: Jacob Blain Christen <dweomer5@gmail.com> Signed-off-by: Adam Martin <42001113+amartin120@users.noreply.github.com>

clean up truth table to be less complex

70b1f26

Co-authored-by: Jacob Blain Christen <dweomer5@gmail.com> Signed-off-by: Adam Martin <42001113+amartin120@users.noreply.github.com>

dweomer reviewed Sep 13, 2023

View reviewed changes

pkg/oci/layout/write.go Outdated Show resolved Hide resolved

amartin120 added 2 commits September 13, 2023 16:02

handle nil on getImageRef

d58278e

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

use org.opencontainers.image.ref.name instead of title

7f55559

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

updated docs via docgen

274c2a3

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

amartin120 force-pushed the dedupe-save-load branch from 642f107 to 274c2a3 Compare October 3, 2023 15:24

priyawadhwa reviewed Oct 3, 2023

View reviewed changes

amartin120 added 4 commits October 10, 2023 10:33

add e2e test for bulk save/load

cbe9715

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

fix bulk save/load e2e typos

156ac84

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

fix registry option for bulk save/load e2e

7a6d72c

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

correct lint issues for bulk save/load changes

d1e6ba2

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

correct more lint issues

e2de0a4

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

priyawadhwa reviewed Nov 3, 2023

View reviewed changes

amartin120 added 2 commits November 7, 2023 11:05

remove leftover debugging

b7d6ce4

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

move the load flag validation to a better spot

861bfe6

Signed-off-by: Adam Martin <adam.martin@rancherfederal.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `cosign save` to reuse save directory to help dedupe shared layers. #3239

Allow `cosign save` to reuse save directory to help dedupe shared layers. #3239

amartin120 commented Sep 13, 2023 •

edited

codecov bot commented Oct 3, 2023 •

edited

priyawadhwa left a comment

amartin120 commented Oct 4, 2023 •

edited

priyawadhwa commented Oct 5, 2023

amartin120 commented Oct 6, 2023 •

edited

amartin120 commented Oct 9, 2023

priyawadhwa commented Oct 25, 2023

amartin120 commented Oct 26, 2023

priyawadhwa commented Oct 26, 2023

amartin120 commented Oct 27, 2023 •

edited

dweomer commented Nov 2, 2023

priyawadhwa left a comment

priyawadhwa Nov 3, 2023

priyawadhwa Nov 3, 2023

amartin120 Nov 7, 2023

amartin120 Nov 13, 2023 •

edited

Allow cosign save to reuse save directory to help dedupe shared layers. #3239

Are you sure you want to change the base?

Allow cosign save to reuse save directory to help dedupe shared layers. #3239

Conversation

amartin120 commented Sep 13, 2023 • edited

Summary

Release Note

Documentation

codecov bot commented Oct 3, 2023 • edited

Codecov Report

priyawadhwa left a comment

Choose a reason for hiding this comment

amartin120 commented Oct 4, 2023 • edited

priyawadhwa commented Oct 5, 2023

amartin120 commented Oct 6, 2023 • edited

amartin120 commented Oct 9, 2023

priyawadhwa commented Oct 25, 2023

amartin120 commented Oct 26, 2023

priyawadhwa commented Oct 26, 2023

amartin120 commented Oct 27, 2023 • edited

dweomer commented Nov 2, 2023

priyawadhwa left a comment

Choose a reason for hiding this comment

priyawadhwa Nov 3, 2023

Choose a reason for hiding this comment

priyawadhwa Nov 3, 2023

Choose a reason for hiding this comment

amartin120 Nov 7, 2023

Choose a reason for hiding this comment

amartin120 Nov 13, 2023 • edited

Choose a reason for hiding this comment

Allow `cosign save` to reuse save directory to help dedupe shared layers. #3239

Allow `cosign save` to reuse save directory to help dedupe shared layers. #3239

amartin120 commented Sep 13, 2023 •

edited

codecov bot commented Oct 3, 2023 •

edited

amartin120 commented Oct 4, 2023 •

edited

amartin120 commented Oct 6, 2023 •

edited

amartin120 commented Oct 27, 2023 •

edited

amartin120 Nov 13, 2023 •

edited