Corrected lease implementation

Signed-off-by: Brandon Lum <lumjjb@gmail.com>

cmd/ctr/commands/images/crypt_utils.go

@@ -24,12 +24,10 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/images"
 	imgenc "github.com/containerd/containerd/images/encryption"
-	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/pkg/encryption"
 	encconfig "github.com/containerd/containerd/pkg/encryption/config"
 	encutils "github.com/containerd/containerd/pkg/encryption/utils"
@@ -254,17 +252,16 @@ func cryptImage(client *containerd.Client, ctx gocontext.Context, name, newName
 		newSpec  ocispec.Descriptor
 	)
 
-	ls := client.LeasesService()
+	ctx, done, err := client.WithLease(ctx)
-	l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
 	if err != nil {
 		return images.Image{}, err
 	}
-	defer ls.Delete(ctx, l, leases.SynchronousDelete)
+	defer done(ctx)
 
 	if encrypt {
-		newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
+		newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
 	} else {
-		newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
+		newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
 	}
 	if err != nil {
 		return image, err

docs/encryption.md

@@ -75,10 +75,10 @@ The current draft of exposed interfaces we believe will be used by consumers are
 /* Functions */
 
 // EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
-func EncryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
+func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
 
 // DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
-func DecryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
+func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
 
 // CheckAuthorization checks whether a user has the right keys to be allowed to access an image (every layer)
 // It takes decrypting of the layers only as far as decrypting the asymmetrically encrypted data

image_enc_test.go

@@ -20,13 +20,11 @@ import (
 	"context"
 	"runtime"
 	"testing"
-	"time"
 
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	imgenc "github.com/containerd/containerd/images/encryption"
-	"github.com/containerd/containerd/leases"
 	encconfig "github.com/containerd/containerd/pkg/encryption/config"
 	"github.com/containerd/containerd/pkg/encryption/utils"
 	"github.com/containerd/containerd/platforms"
@@ -86,7 +84,11 @@ func TestImageEncryption(t *testing.T) {
 	defer client.Close()
 
 	s := client.ImageService()
-	ls := client.LeasesService()
+	ctx, done, err := client.WithLease(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer done(ctx)
 
 	image, err := s.Get(ctx, imageName)
 	if err != nil {
@@ -136,14 +138,8 @@ func TestImageEncryption(t *testing.T) {
 		},
 	}
 
-	l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
-	if err != nil {
-		t.Fatal("Unable to create lease for encryption")
-	}
-	defer ls.Delete(ctx, l, leases.SynchronousDelete)
-
 	// Perform encryption of image
-	encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
+	encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -159,8 +155,6 @@ func TestImageEncryption(t *testing.T) {
 	if _, err := s.Create(ctx, image); err != nil {
 		t.Fatalf("Unable to create image: %v", err)
 	}
-	// Force deletion of lease early to check for proper referencing
-	ls.Delete(ctx, l, leases.SynchronousDelete)
 
 	cc = &encconfig.CryptoConfig{
 		DecryptConfig: &encconfig.DecryptConfig{
@@ -175,13 +169,7 @@ func TestImageEncryption(t *testing.T) {
 		return true
 	}
 
-	l, err = ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
+	decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), encSpec, cc, lf)
-	if err != nil {
-		t.Fatal("Unable to create lease for decryption")
-	}
-	defer ls.Delete(ctx, l, leases.SynchronousDelete)
-
-	decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, encSpec, cc, lf)
 	if err != nil {
 		t.Fatal(err)
 	}

images/encryption/encryption.go

@@ -30,7 +30,6 @@ import (
 
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
-	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/platforms"
 	digest "github.com/opencontainers/go-digest"
 	specs "github.com/opencontainers/image-spec/specs-go"
@@ -149,7 +148,7 @@ func decryptLayer(cc *encconfig.CryptoConfig, dataReader content.ReaderAt, desc
 }
 
 // cryptLayer handles the changes due to encryption or decryption of a layer
-func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, cryptoOp cryptoOp) (ocispec.Descriptor, error) {
+func cryptLayer(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, cryptoOp cryptoOp) (ocispec.Descriptor, error) {
 	var (
 		resultReader io.Reader
 		newDesc      ocispec.Descriptor
@@ -171,33 +170,13 @@ func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leas
 	}
 	// some operations, such as changing recipients, may not touch the layer at all
 	if resultReader != nil {
-		if ls == nil {
-			return ocispec.Descriptor{}, errors.New("Unexpected write to object without lease")
-		}
-
-		var rsrc leases.Resource
 		var ref string
-
 		// If we have the digest, write blob with checks
 		haveDigest := newDesc.Digest.String() != ""
 		if haveDigest {
 			ref = fmt.Sprintf("layer-%s", newDesc.Digest.String())
-			rsrc = leases.Resource{
-				ID:   newDesc.Digest.String(),
-				Type: "content",
-			}
 		} else {
 			ref = fmt.Sprintf("blob-%d-%d", rand.Int(), rand.Int())
-			rsrc = leases.Resource{
-				ID:   ref,
-				Type: "ingests",
-			}
-
-		}
-
-		// Add resource to lease and write blob
-		if err := ls.AddResource(ctx, l, rsrc); err != nil {
-			return ocispec.Descriptor{}, errors.Wrap(err, "Unable to add resource to lease")
 		}
 
 		if haveDigest {
@@ -205,20 +184,16 @@ func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leas
 				return ocispec.Descriptor{}, errors.Wrap(err, "failed to write config")
 			}
 		} else {
-			newDesc.Digest, newDesc.Size, err = ingestReaderLeaseContent(ctx, cs, ls, l, ref, resultReader)
+			newDesc.Digest, newDesc.Size, err = ingestReader(ctx, cs, ref, resultReader)
 			if err != nil {
 				return ocispec.Descriptor{}, err
 			}
-			// Delete the ingest lease since the blob is protected now by the content lease
-			if err := ls.DeleteResource(ctx, l, rsrc); err != nil {
-				return ocispec.Descriptor{}, errors.Wrap(err, "Unable to delete resource from lease")
-			}
 		}
 	}
 	return newDesc, err
 }
 
-func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls leases.Manager, l leases.Lease, ref string, r io.Reader) (digest.Digest, int64, error) {
+func ingestReader(ctx context.Context, cs content.Ingester, ref string, r io.Reader) (digest.Digest, int64, error) {
 	cw, err := content.OpenWriter(ctx, cs, content.WithRef(ref))
 	if err != nil {
 		return "", 0, errors.Wrap(err, "failed to open writer")
@@ -234,16 +209,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease
 		return "", 0, errors.Wrap(err, "failed to get state")
 	}
 
-	rsrc := leases.Resource{
+	if err := cw.Commit(ctx, st.Offset, ""); err != nil {
-		ID:   cw.Digest().String(),
-		Type: "content",
-	}
-
-	if err := ls.AddResource(ctx, l, rsrc); err != nil {
-		return "", 0, errors.Wrapf(err, "Unable to add resource to lease")
-	}
-
-	if err := cw.Commit(ctx, st.Offset, cw.Digest()); err != nil {
 		if !errdefs.IsAlreadyExists(err) {
 			return "", 0, errors.Wrapf(err, "failed commit on ref %q", ref)
 		}
@@ -253,7 +219,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease
 }
 
 // Encrypt or decrypt all the Children of a given descriptor
-func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp, thisPlatform *ocispec.Platform) (ocispec.Descriptor, bool, error) {
+func cryptChildren(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp, thisPlatform *ocispec.Platform) (ocispec.Descriptor, bool, error) {
 	children, err := images.Children(ctx, cs, desc)
 	if err != nil {
 		if errdefs.IsNotFound(err) {
@@ -274,7 +240,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
 		case images.MediaTypeDockerSchema2LayerGzip, images.MediaTypeDockerSchema2Layer,
 			ocispec.MediaTypeImageLayerGzip, ocispec.MediaTypeImageLayer:
 			if cryptoOp == cryptoOpEncrypt && lf(child) {
-				nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp)
+				nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp)
 				if err != nil {
 					return ocispec.Descriptor{}, false, err
 				}
@@ -286,7 +252,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
 		case images.MediaTypeDockerSchema2LayerGzipEnc, images.MediaTypeDockerSchema2LayerEnc:
 			// this one can be decrypted but also its recipients list changed
 			if lf(child) {
-				nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp)
+				nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp)
 				if err != nil || cryptoOp == cryptoOpUnwrapOnly {
 					return ocispec.Descriptor{}, false, err
 				}
@@ -332,19 +298,6 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
 
 		ref := fmt.Sprintf("manifest-%s", newDesc.Digest.String())
 
-		if ls == nil {
-			return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease")
-		}
-
-		rsrc := leases.Resource{
-			ID:   newDesc.Digest.String(),
-			Type: "content",
-		}
-
-		if err := ls.AddResource(ctx, l, rsrc); err != nil {
-			return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease")
-		}
-
 		if err := content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil {
 			return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write config")
 		}
@@ -355,7 +308,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
 }
 
 // cryptManifest encrypts or decrypts the children of a top level manifest
-func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
+func cryptManifest(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
 	p, err := content.ReadBlob(ctx, cs, desc)
 	if err != nil {
 		return ocispec.Descriptor{}, false, err
@@ -365,7 +318,7 @@ func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l l
 		return ocispec.Descriptor{}, false, err
 	}
 	platform := platforms.DefaultSpec()
-	newDesc, modified, err := cryptChildren(ctx, cs, ls, l, desc, cc, lf, cryptoOp, &platform)
+	newDesc, modified, err := cryptChildren(ctx, cs, desc, cc, lf, cryptoOp, &platform)
 	if err != nil || cryptoOp == cryptoOpUnwrapOnly {
 		return ocispec.Descriptor{}, false, err
 	}
@@ -373,7 +326,7 @@ func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l l
 }
 
 // cryptManifestList encrypts or decrypts the children of a top level manifest list
-func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
+func cryptManifestList(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
 	// read the index; if any layer is encrypted and any manifests change we will need to rewrite it
 	b, err := content.ReadBlob(ctx, cs, desc)
 	if err != nil {
@@ -388,7 +341,7 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
 	var newManifests []ocispec.Descriptor
 	modified := false
 	for _, manifest := range index.Manifests {
-		newManifest, m, err := cryptChildren(ctx, cs, ls, l, manifest, cc, lf, cryptoOp, manifest.Platform)
+		newManifest, m, err := cryptChildren(ctx, cs, manifest, cc, lf, cryptoOp, manifest.Platform)
 		if err != nil || cryptoOp == cryptoOpUnwrapOnly {
 			return ocispec.Descriptor{}, false, err
 		}
@@ -423,19 +376,6 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
 
 		ref := fmt.Sprintf("index-%s", newDesc.Digest.String())
 
-		if ls == nil {
-			return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease")
-		}
-
-		rsrc := leases.Resource{
-			ID:   newDesc.Digest.String(),
-			Type: "content",
-		}
-
-		if err := ls.AddResource(ctx, l, rsrc); err != nil {
-			return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease")
-		}
-
 		if err = content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil {
 			return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write index")
 		}
@@ -447,28 +387,28 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
 
 // cryptImage is the dispatcher to encrypt/decrypt an image; it accepts either an OCI descriptor
 // representing a manifest list or a single manifest
-func cryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
+func cryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
 	if cc == nil {
 		return ocispec.Descriptor{}, false, errors.Wrapf(errdefs.ErrInvalidArgument, "CryptoConfig must not be nil")
 	}
 	switch desc.MediaType {
 	case ocispec.MediaTypeImageIndex, images.MediaTypeDockerSchema2ManifestList:
-		return cryptManifestList(ctx, cs, ls, l, desc, cc, lf, cryptoOp)
+		return cryptManifestList(ctx, cs, desc, cc, lf, cryptoOp)
 	case ocispec.MediaTypeImageManifest, images.MediaTypeDockerSchema2Manifest:
-		return cryptManifest(ctx, cs, ls, l, desc, cc, lf, cryptoOp)
+		return cryptManifest(ctx, cs, desc, cc, lf, cryptoOp)
 	default:
 		return ocispec.Descriptor{}, false, errors.Errorf("CryptImage: Unhandled media type: %s", desc.MediaType)
 	}
 }
 
 // EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
-func EncryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
+func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
-	return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpEncrypt)
+	return cryptImage(ctx, cs, desc, cc, lf, cryptoOpEncrypt)
 }
 
 // DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
-func DecryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
+func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
-	return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpDecrypt)
+	return cryptImage(ctx, cs, desc, cc, lf, cryptoOpDecrypt)
 }
 
 // CheckAuthorization checks whether a user has the right keys to be allowed to access an image (every layer)
@@ -478,11 +418,12 @@ func CheckAuthorization(ctx context.Context, cs content.Store, desc ocispec.Desc
 	cc := encconfig.CryptoConfig{
 		DecryptConfig: dc,
 	}
+
 	lf := func(desc ocispec.Descriptor) bool {
 		return true
 	}
-	// We shouldn't need to create any objects in CheckAuthorization, so no lease required.
+
-	_, _, err := cryptImage(ctx, cs, nil, leases.Lease{}, desc, &cc, lf, cryptoOpUnwrapOnly)
+	_, _, err := cryptImage(ctx, cs, desc, &cc, lf, cryptoOpUnwrapOnly)
 	if err != nil {
 		return errors.Wrapf(err, "you are not authorized to use this image")
 	}