From ae56e3cc20d99b04c71a18a95a739c5f4bf9d27e Mon Sep 17 00:00:00 2001 From: Kohei Tokunaga Date: Mon, 13 May 2024 22:17:05 +0900 Subject: [PATCH 1/2] store: Enable to recieve TOCDigest from runtime Signed-off-by: Kohei Tokunaga --- cmd/stargz-store/main.go | 5 +- fs/layer/layer.go | 2 + .../demo-store/etc/stargz-store/config.toml | 1 - store/fs.go | 10 +- store/manager.go | 208 ++++++++---------- 5 files changed, 101 insertions(+), 125 deletions(-) diff --git a/cmd/stargz-store/main.go b/cmd/stargz-store/main.go index e7f0058af..afc3f7f84 100644 --- a/cmd/stargz-store/main.go +++ b/cmd/stargz-store/main.go @@ -132,9 +132,8 @@ func main() { Fatalf("failed to prepare mountpoint %q", mountPoint) } } - if !config.Config.DisableVerification { - log.G(ctx).Warnf("content verification is not supported; switching to non-verification mode") - config.Config.DisableVerification = true + if config.Config.DisableVerification { + log.G(ctx).Fatalf("content verification can't be disabled") } mt, err := getMetadataStore(*rootDir, config) if err != nil { diff --git a/fs/layer/layer.go b/fs/layer/layer.go index c53330c70..c856d4801 100644 --- a/fs/layer/layer.go +++ b/fs/layer/layer.go @@ -108,6 +108,7 @@ type Info struct { FetchedSize int64 // layer fetched size in bytes PrefetchSize int64 // layer prefetch size in bytes ReadTime time.Time // last time the layer was read + TOCDigest digest.Digest } // Resolver resolves the layer location and provieds the handler of that layer. @@ -415,6 +416,7 @@ func (l *layer) Info() Info { FetchedSize: l.blob.FetchedSize(), PrefetchSize: l.prefetchedSize(), ReadTime: readTime, + TOCDigest: l.verifiableReader.Metadata().TOCDigest(), } } diff --git a/script/demo-store/etc/stargz-store/config.toml b/script/demo-store/etc/stargz-store/config.toml index 6873abe3f..8d1185971 100644 --- a/script/demo-store/etc/stargz-store/config.toml +++ b/script/demo-store/etc/stargz-store/config.toml @@ -1,5 +1,4 @@ metrics_address = "127.0.0.1:8234" -disable_verification = true [[resolver.host."registry2-store:5000".mirrors]] host = "registry2-store:5000" insecure = true diff --git a/store/fs.go b/store/fs.go index 171e48378..93881d694 100644 --- a/store/fs.go +++ b/store/fs.go @@ -384,12 +384,20 @@ func (n *layernode) Lookup(ctx context.Context, name string, out *fuse.EntryOut) return nil, syscall.EIO } log.G(ctx).WithField(remoteSnapshotLogKey, prepareFailed). - WithField("layerdigest", n.digest). + WithField("digest", n.digest). WithError(err). Debugf("error resolving layer (context error: %v)", cErr) log.G(ctx).WithError(err).Warnf("failed to mount layer %q: %q", name, n.digest) return nil, syscall.EIO } + if err := l.Verify(n.digest); err != nil { + log.G(ctx).WithField(remoteSnapshotLogKey, prepareFailed). + WithField("digest", n.digest). + WithError(err). + Debugf("failed to verify layer") + log.G(ctx).WithError(err).Warnf("failed to mount layer %q: %q", name, n.digest) + return nil, syscall.EIO + } if name == blobLink { sAttr := layerToAttr(l, &out.Attr) cn := &blobnode{l: l} diff --git a/store/manager.go b/store/manager.go index d7a62119e..e3992dcbb 100644 --- a/store/manager.go +++ b/store/manager.go @@ -26,7 +26,6 @@ import ( "github.com/containerd/containerd/reference" "github.com/containerd/log" - "github.com/containerd/stargz-snapshotter/estargz" "github.com/containerd/stargz-snapshotter/estargz/zstdchunked" "github.com/containerd/stargz-snapshotter/fs/config" "github.com/containerd/stargz-snapshotter/fs/layer" @@ -83,8 +82,6 @@ func NewLayerManager(ctx context.Context, root string, hosts source.RegistryHost noprefetch: cfg.NoPrefetch, noBackgroundFetch: cfg.NoBackgroundFetch, backgroundTaskManager: tm, - allowNoVerification: cfg.AllowNoVerification, - disableVerification: cfg.DisableVerification, metricsController: c, resolveLock: new(namedmutex.NamedMutex), layer: make(map[string]map[string]layer.Layer), @@ -102,18 +99,18 @@ type LayerManager struct { noprefetch bool noBackgroundFetch bool backgroundTaskManager *task.BackgroundTaskManager - allowNoVerification bool - disableVerification bool metricsController *layermetrics.Controller resolveLock *namedmutex.NamedMutex - layer map[string]map[string]layer.Layer - refcounter map[string]map[string]int + layer map[string]map[string]layer.Layer // keyed by image ref and TOC Digest + refcounter map[string]map[string]int // keyed by image ref and TOC Digest + + resolveLayerCache map[string]map[string]error // keyed by image ref and layer digest (not TOCDigest) mu sync.Mutex } -func (r *LayerManager) cacheLayer(refspec reference.Spec, dgst digest.Digest, l layer.Layer) (_ layer.Layer, added bool) { +func (r *LayerManager) cacheLayer(refspec reference.Spec, tocDigest digest.Digest, l layer.Layer) (_ layer.Layer, added bool) { r.mu.Lock() defer r.mu.Unlock() if r.layer == nil { @@ -122,35 +119,39 @@ func (r *LayerManager) cacheLayer(refspec reference.Spec, dgst digest.Digest, l if r.layer[refspec.String()] == nil { r.layer[refspec.String()] = make(map[string]layer.Layer) } - if cl, ok := r.layer[refspec.String()][dgst.String()]; ok { + if cl, ok := r.layer[refspec.String()][tocDigest.String()]; ok && cl.Info().TOCDigest == tocDigest { return cl, false // already exists } - r.layer[refspec.String()][dgst.String()] = l + r.layer[refspec.String()][tocDigest.String()] = l return l, true } -func (r *LayerManager) getCachedLayer(refspec reference.Spec, dgst digest.Digest) layer.Layer { +func (r *LayerManager) getCachedLayer(refspec reference.Spec, tocDigest digest.Digest) layer.Layer { r.mu.Lock() defer r.mu.Unlock() if r.layer == nil || r.layer[refspec.String()] == nil { return nil } - if l, ok := r.layer[refspec.String()][dgst.String()]; ok { + if l, ok := r.layer[refspec.String()][tocDigest.String()]; ok && l.Info().TOCDigest == tocDigest { return l } return nil } -func (r *LayerManager) getLayerInfo(ctx context.Context, refspec reference.Spec, dgst digest.Digest) (Layer, error) { +func (r *LayerManager) getLayerInfo(ctx context.Context, refspec reference.Spec, tocDigest digest.Digest) (Layer, error) { manifest, config, err := r.refPool.loadRef(ctx, refspec) if err != nil { return Layer{}, fmt.Errorf("failed to get manifest and config: %w", err) } - return genLayerInfo(ctx, dgst, manifest, config) + gotL := r.getCachedLayer(refspec, tocDigest) + if gotL == nil { + return Layer{TOCDigest: tocDigest}, nil + } + return genLayerInfo(ctx, gotL.Info().Digest, manifest, config, tocDigest) } -func (r *LayerManager) getLayer(ctx context.Context, refspec reference.Spec, dgst digest.Digest) (layer.Layer, error) { - gotL := r.getCachedLayer(refspec, dgst) +func (r *LayerManager) getLayer(ctx context.Context, refspec reference.Spec, tocDigest digest.Digest) (layer.Layer, error) { + gotL := r.getCachedLayer(refspec, tocDigest) if gotL != nil { return gotL, nil } @@ -160,58 +161,43 @@ func (r *LayerManager) getLayer(ctx context.Context, refspec reference.Spec, dgs result layer.Layer resultChan = make(chan layer.Layer) errChan = make(chan error) + wg sync.WaitGroup ) manifest, _, err := r.refPool.loadRef(ctx, refspec) if err != nil { return nil, fmt.Errorf("failed to get manifest and config: %w", err) } - var target ocispec.Descriptor - var preResolve []ocispec.Descriptor - var found bool for _, l := range manifest.Layers { - if l.Digest == dgst { - l := l - found = true - target = l - continue - } - preResolve = append(preResolve, l) - } - if !found { - return nil, fmt.Errorf("unknown digest %v for ref %q", target, refspec.String()) - } - for _, l := range append([]ocispec.Descriptor{target}, preResolve...) { l := l - // Check if layer is already resolved before creating goroutine. - gotL := r.getCachedLayer(refspec, l.Digest) - if gotL != nil { - // Layer already resolved - if l.Digest.String() != target.Digest.String() { - continue // This is not the target layer; nop - } - result = gotL - continue - } - // Resolve the layer + wg.Add(1) go func() { + defer wg.Done() // Avoids to get canceled by client. ctx := context.Background() - gotL, err := r.resolveLayer(ctx, refspec, l) - if l.Digest.String() != target.Digest.String() { - return // This is not target layer - } - if err != nil { - errChan <- fmt.Errorf("failed to resolve layer %q / %q: %w", refspec, l.Digest, err) + if err := r.resolveLayer(ctx, refspec, l); err != nil { + log.G(ctx).Debugf("failed to resolve layer %q / %q: %v", refspec, l.Digest, err) return } + + gotL := r.getCachedLayer(refspec, tocDigest) // Try to get the layer we want. + if gotL == nil { + return // Layer not found + } + // Log this as preparation success log.G(ctx).WithField(remoteSnapshotLogKey, prepareSucceeded).Debugf("successfully resolved layer") resultChan <- gotL }() } + allDone := make(chan struct{}) + go func() { + wg.Wait() + close(allDone) + }() + if result != nil { return result, nil } @@ -226,23 +212,41 @@ func (r *LayerManager) getLayer(ctx context.Context, refspec reference.Spec, dgs case <-time.After(30 * time.Second): log.G(ctx).Debug("failed to resolve layer (timeout)") return nil, fmt.Errorf("failed to resolve layer (timeout)") + case <-allDone: + log.G(ctx).Debug("fetched all layer but no layer with the expected TOC acquired") + return nil, fmt.Errorf("layer with TOCDigest %v not found", tocDigest) } return l, nil } -func (r *LayerManager) resolveLayer(ctx context.Context, refspec reference.Spec, target ocispec.Descriptor) (layer.Layer, error) { +func (r *LayerManager) resolveLayer(ctx context.Context, refspec reference.Spec, target ocispec.Descriptor) (retErr error) { key := refspec.String() + "/" + target.Digest.String() // Wait if resolving this layer is already running. r.resolveLock.Lock(key) defer r.resolveLock.Unlock(key) - gotL := r.getCachedLayer(refspec, target.Digest) - if gotL != nil { - // layer already resolved - return gotL, nil + // Check if this resolving has already done. + r.mu.Lock() + if r.resolveLayerCache != nil && r.resolveLayerCache[refspec.String()] != nil { + if err, ok := r.resolveLayerCache[refspec.String()][target.Digest.String()]; ok { + r.mu.Unlock() + return err + } } + r.mu.Unlock() + defer func() { + r.mu.Lock() + if r.resolveLayerCache == nil { + r.resolveLayerCache = make(map[string]map[string]error) + } + if r.resolveLayerCache[refspec.String()] == nil { + r.resolveLayerCache[refspec.String()] = make(map[string]error) + } + r.resolveLayerCache[refspec.String()][target.Digest.String()] = retErr + r.mu.Unlock() + }() // Resolve this layer. var esgzOpts []metadata.Option @@ -258,35 +262,8 @@ func (r *LayerManager) resolveLayer(ctx context.Context, refspec reference.Spec, } l, err := r.resolver.Resolve(ctx, r.hosts, refspec, target, esgzOpts...) if err != nil { - return nil, err + return err } - - // Verify layer's content - labels := target.Annotations - if labels == nil { - labels = make(map[string]string) - } - if r.disableVerification { - // Skip if verification is disabled completely - l.SkipVerify() - log.G(ctx).Debugf("Verification forcefully skipped") - } else if tocDigest, ok := labels[estargz.TOCJSONDigestAnnotation]; ok { - // Verify this layer using the TOC JSON digest passed through label. - dgst, err := digest.Parse(tocDigest) - if err != nil { - log.G(ctx).WithError(err).Debugf("failed to parse passed TOC digest %q", dgst) - return nil, fmt.Errorf("invalid TOC digest: %v: %w", tocDigest, err) - } - if err := l.Verify(dgst); err != nil { - log.G(ctx).WithError(err).Debugf("invalid layer") - return nil, fmt.Errorf("invalid stargz layer: %w", err) - } - log.G(ctx).Debugf("verified") - } else { - // Verification must be done. Don't mount this layer. - return nil, fmt.Errorf("digest of TOC JSON must be passed") - } - // Prefetch this layer. We prefetch several layers in parallel. The first // Check() for this layer waits for the prefetch completion. if !r.noprefetch { @@ -316,17 +293,17 @@ func (r *LayerManager) resolveLayer(ctx context.Context, refspec reference.Spec, } // Cache this layer. - cachedL, added := r.cacheLayer(refspec, target.Digest, l) + cachedL, added := r.cacheLayer(refspec, l.Info().TOCDigest, l) if added { r.metricsController.Add(key, cachedL) } else { l.Done() // layer is already cached. use the cached one instead. discard this layer. } - return cachedL, nil + return nil } -func (r *LayerManager) release(ctx context.Context, refspec reference.Spec, dgst digest.Digest) (int, error) { +func (r *LayerManager) release(ctx context.Context, refspec reference.Spec, tocDigest digest.Digest) (int, error) { r.refPool.release(refspec) r.mu.Lock() @@ -334,35 +311,36 @@ func (r *LayerManager) release(ctx context.Context, refspec reference.Spec, dgst if r.refcounter == nil || r.refcounter[refspec.String()] == nil { return 0, fmt.Errorf("ref %q not tracked", refspec.String()) - } else if _, ok := r.refcounter[refspec.String()][dgst.String()]; !ok { - return 0, fmt.Errorf("layer %q/%q not tracked", refspec.String(), dgst.String()) + } else if _, ok := r.refcounter[refspec.String()][tocDigest.String()]; !ok { + return 0, fmt.Errorf("layer %q/%q not tracked", refspec.String(), tocDigest.String()) } - r.refcounter[refspec.String()][dgst.String()]-- - i := r.refcounter[refspec.String()][dgst.String()] + r.refcounter[refspec.String()][tocDigest.String()]-- + i := r.refcounter[refspec.String()][tocDigest.String()] if i <= 0 { // No reference to this layer. release it. - delete(r.refcounter, dgst.String()) + delete(r.refcounter, tocDigest.String()) if len(r.refcounter[refspec.String()]) == 0 { delete(r.refcounter, refspec.String()) + delete(r.resolveLayerCache, refspec.String()) // no reference to this image. So reset the resolve status as well. } if r.layer == nil || r.layer[refspec.String()] == nil { return 0, fmt.Errorf("layer of reference %q is not registered (ref=%d)", refspec, i) } - l, ok := r.layer[refspec.String()][dgst.String()] + l, ok := r.layer[refspec.String()][tocDigest.String()] if !ok { - return 0, fmt.Errorf("layer of digest %q/%q is not registered (ref=%d)", refspec, dgst, i) + return 0, fmt.Errorf("layer of digest %q/%q is not registered (ref=%d)", refspec, tocDigest, i) } l.Done() - delete(r.layer[refspec.String()], dgst.String()) + delete(r.layer[refspec.String()], tocDigest.String()) if len(r.layer[refspec.String()]) == 0 { delete(r.layer, refspec.String()) } - log.G(ctx).WithField("refcounter", i).Infof("layer %v/%v is released due to no reference", refspec, dgst) + log.G(ctx).WithField("refcounter", i).Infof("layer %v/%v is released due to no reference", refspec, tocDigest) } return i, nil } -func (r *LayerManager) use(refspec reference.Spec, dgst digest.Digest) int { +func (r *LayerManager) use(refspec reference.Spec, tocDigest digest.Digest) int { r.refPool.use(refspec) r.mu.Lock() @@ -374,12 +352,12 @@ func (r *LayerManager) use(refspec reference.Spec, dgst digest.Digest) int { if r.refcounter[refspec.String()] == nil { r.refcounter[refspec.String()] = make(map[string]int) } - if _, ok := r.refcounter[refspec.String()][dgst.String()]; !ok { - r.refcounter[refspec.String()][dgst.String()] = 1 + if _, ok := r.refcounter[refspec.String()][tocDigest.String()]; !ok { + r.refcounter[refspec.String()][tocDigest.String()] = 1 return 1 } - r.refcounter[refspec.String()][dgst.String()]++ - return r.refcounter[refspec.String()][dgst.String()] + r.refcounter[refspec.String()][tocDigest.String()]++ + return r.refcounter[refspec.String()][tocDigest.String()] } func colon2dash(s string) string { @@ -389,18 +367,16 @@ func colon2dash(s string) string { // Layer represents the layer information. Format is compatible to the one required by // "additional layer store" of github.com/containers/storage. type Layer struct { - CompressedDigest digest.Digest `json:"compressed-diff-digest,omitempty"` - CompressedSize int64 `json:"compressed-size,omitempty"` - UncompressedDigest digest.Digest `json:"diff-digest,omitempty"` - UncompressedSize int64 `json:"diff-size,omitempty"` - CompressionType int `json:"compression,omitempty"` - ReadOnly bool `json:"-"` + UncompressedSize int64 `json:"diff-size,omitempty"` + CompressionType int `json:"compression,omitempty"` + TOCDigest digest.Digest `json:"toc-digest,omitempty"` + Flags map[string]string `json:"flags,omitempty"` } // Defined in https://github.com/containers/storage/blob/b64e13a1afdb0bfed25601090ce4bbbb1bc183fc/pkg/archive/archive.go#L108-L119 const gzipTypeMagicNum = 2 -func genLayerInfo(ctx context.Context, dgst digest.Digest, manifest ocispec.Manifest, config ocispec.Image) (Layer, error) { +func genLayerInfo(ctx context.Context, dgst digest.Digest, manifest ocispec.Manifest, config ocispec.Image, tocDigest digest.Digest) (Layer, error) { if len(manifest.Layers) != len(config.RootFS.DiffIDs) { return Layer{}, fmt.Errorf( "len(manifest.Layers) != len(config.Rootfs): %d != %d", @@ -417,22 +393,14 @@ func genLayerInfo(ctx context.Context, dgst digest.Digest, manifest ocispec.Mani if layerIndex == -1 { return Layer{}, fmt.Errorf("layer %q not found in the manifest", dgst.String()) } - var uncompressedSize int64 - var err error - if uncompressedSizeStr, ok := manifest.Layers[layerIndex].Annotations[estargz.StoreUncompressedSizeAnnotation]; ok { - uncompressedSize, err = strconv.ParseInt(uncompressedSizeStr, 10, 64) - if err != nil { - log.G(ctx).WithError(err).Warnf("layer %q has invalid uncompressed size; exposing incomplete layer info", dgst.String()) - } - } else { - log.G(ctx).Warnf("layer %q doesn't have uncompressed size; exposing incomplete layer info", dgst.String()) + + layerFlags := map[string]string{ + "expected-layer-diffid": config.RootFS.DiffIDs[layerIndex].String(), } return Layer{ - CompressedDigest: manifest.Layers[layerIndex].Digest, - CompressedSize: manifest.Layers[layerIndex].Size, - UncompressedDigest: config.RootFS.DiffIDs[layerIndex], - UncompressedSize: uncompressedSize, - CompressionType: gzipTypeMagicNum, - ReadOnly: true, + UncompressedSize: -1, // means unknown + CompressionType: gzipTypeMagicNum, + TOCDigest: tocDigest, + Flags: layerFlags, }, nil } From a1573c23d90b2ca1538bdf656bcd89d70dfb0701 Mon Sep 17 00:00:00 2001 From: Kohei Tokunaga Date: Mon, 13 May 2024 22:17:31 +0900 Subject: [PATCH 2/2] enable test Signed-off-by: Kohei Tokunaga --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index a855f8005..6faa69596 100644 --- a/Dockerfile +++ b/Dockerfile @@ -17,8 +17,8 @@ ARG RUNC_VERSION=v1.1.12 ARG CNI_PLUGINS_VERSION=v1.4.1 ARG NERDCTL_VERSION=1.7.6 -ARG PODMAN_VERSION=v5.0.2 -ARG CRIO_VERSION=v1.30.0 +ARG PODMAN_VERSION=v5.1.1 +ARG CRIO_VERSION=main ARG CONMON_VERSION=v2.1.11 ARG COMMON_VERSION=v0.58.2 ARG CRIO_TEST_PAUSE_IMAGE_NAME=registry.k8s.io/pause:3.6