diff --git a/internal/erofsutils/mount.go b/internal/erofsutils/mount.go index ef5718ca72..6c3ef9f648 100644 --- a/internal/erofsutils/mount.go +++ b/internal/erofsutils/mount.go @@ -19,7 +19,6 @@ package erofsutils import ( "bytes" "context" - "errors" "fmt" "io" "os" @@ -209,22 +208,6 @@ func MountsToLayer(mounts []mount.Mount) (string, error) { return layer, nil } -// StagedLayerBlob reports whether the layer directory's blob was staged from a -// layer content cache. A staged blob is a symlink into that operator-owned -// cache and is shared with every other snapshot of the layer, so it must never -// be written to. -func StagedLayerBlob(layer string) (bool, error) { - layerBlob := filepath.Join(layer, "layer.erofs") - fi, err := os.Lstat(layerBlob) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return false, nil - } - return false, fmt.Errorf("failed to stat layer blob %q: %w", layerBlob, err) - } - return fi.Mode()&os.ModeSymlink != 0, nil -} - // SupportGenerateFromTar checks if the installed version of mkfs.erofs supports // the tar mode (--tar option). func SupportGenerateFromTar() (bool, error) { diff --git a/internal/erofsutils/mount_test.go b/internal/erofsutils/mount_test.go index 6a6ae19fe4..67bf90c0a3 100644 --- a/internal/erofsutils/mount_test.go +++ b/internal/erofsutils/mount_test.go @@ -40,34 +40,6 @@ func TestCacheBlobPath(t *testing.T) { assert.Equal(t, enc[:2], filepath.Base(filepath.Dir(got)), "blob must live under a 2-char prefix dir") } -// TestStagedLayerBlob covers detection of a blob staged from the layer content -// cache. A staged blob is a symlink shared with every other snapshot of that -// layer, so the differ must refuse to write it. -func TestStagedLayerBlob(t *testing.T) { - layer := t.TempDir() - - // No blob yet. - staged, err := StagedLayerBlob(layer) - require.NoError(t, err) - assert.False(t, staged) - - // A regular layer blob, written by the erofs differ. - blob := filepath.Join(layer, "layer.erofs") - require.NoError(t, os.WriteFile(blob, nil, 0644)) - staged, err = StagedLayerBlob(layer) - require.NoError(t, err) - assert.False(t, staged) - - // A staged one. - cached := filepath.Join(t.TempDir(), "cached.erofs") - require.NoError(t, os.WriteFile(cached, []byte("shared blob"), 0644)) - require.NoError(t, os.Remove(blob)) - require.NoError(t, os.Symlink(cached, blob)) - staged, err = StagedLayerBlob(layer) - require.NoError(t, err) - assert.True(t, staged) -} - // TestMountsToLayerStagedBlob covers resolving the layer directory of a // snapshot whose blob is staged. Compare only reads the blob, so it resolves // like any other; the apply path is where a staged blob is refused. diff --git a/plugins/diff/erofs/differ.go b/plugins/diff/erofs/differ.go index 60b4be6435..4e1c8e44f2 100644 --- a/plugins/diff/erofs/differ.go +++ b/plugins/diff/erofs/differ.go @@ -127,6 +127,19 @@ func (s erofsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts [] } }() + // Read-only mounts belong to a snapshot the snapshotter populated itself, + // for example from a layer content cache. The layer is already there and is + // shared with every other snapshot of it. Applying would write to content + // this differ does not own. The unpacker reads the same mounts and skips + // such a layer before it reaches here. + // + // The error is ErrFailedPrecondition. The diff service falls through to the + // next differ only on ErrNotImplemented. Another differ would fail on this + // layer for the same reason. + if len(mounts) > 0 && mounts[len(mounts)-1].ReadOnly() { + return emptyDesc, fmt.Errorf("cannot apply to a read-only snapshot, its content is already populated: %w", errdefs.ErrFailedPrecondition) + } + var ( erofsLayerType string fastcopy bool @@ -179,18 +192,6 @@ func (s erofsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts [] return emptyDesc, err } - // A staged blob is shared with every other snapshot of that layer, so - // applying would truncate the cache entry through the symlink. The unpacker - // reads the read-only mounts the snapshotter hands out and skips such a - // layer before it reaches here. - staged, err := erofsutils.StagedLayerBlob(layer) - if err != nil { - return emptyDesc, err - } - if staged { - return emptyDesc, fmt.Errorf("layer %q is already populated from the layer content cache: %w", layer, errdefs.ErrFailedPrecondition) - } - ra, err := s.store.ReaderAt(ctx, desc) if err != nil { return emptyDesc, fmt.Errorf("failed to get reader from content store: %w", err) diff --git a/plugins/diff/erofs/differ_test.go b/plugins/diff/erofs/differ_test.go new file mode 100644 index 0000000000..56c62da693 --- /dev/null +++ b/plugins/diff/erofs/differ_test.go @@ -0,0 +1,90 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package erofs + +import ( + "context" + "testing" + + "github.com/containerd/errdefs" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/containerd/containerd/v2/core/mount" +) + +// TestApplyReadOnlyMounts covers the differ's refusal to apply into a snapshot +// the snapshotter has already populated, for example from a layer content +// cache. The snapshotter says so by handing out read-only mounts. +// +// The error must be ErrFailedPrecondition and never ErrNotImplemented: +// plugins/services/diff/local.go treats ErrNotImplemented as the signal to try +// the next differ. +func TestApplyReadOnlyMounts(t *testing.T) { + ctx := context.Background() + d := erofsDiff{} + + // All erofs mounts are read-only. + erofsMount := mount.Mount{ + Type: "erofs", + Source: "/var/lib/containerd/snapshots/1/layer.erofs", + Options: []string{"ro", "loop"}, + } + // These mounts are a populated snapshot stacked over its parents. Without + // an upperdir the overlay is read-only. + overlayMounts := []mount.Mount{ + erofsMount, + {Type: "erofs", Source: "/var/lib/containerd/snapshots/2/layer.erofs", Options: []string{"ro", "loop"}}, + { + Type: "format/mkdir/overlay", + Source: "overlay", + Options: []string{"lowerdir={{ overlay 0 1 }}", "ro"}, + }, + } + + for _, tc := range []struct { + name string + mounts []mount.Mount + }{ + {name: "erofs layer", mounts: []mount.Mount{erofsMount}}, + {name: "read-only overlay", mounts: overlayMounts}, + } { + t.Run(tc.name, func(t *testing.T) { + desc := ocispec.Descriptor{MediaType: ocispec.MediaTypeImageLayerGzip} + _, err := d.Apply(ctx, desc, tc.mounts) + require.Error(t, err) + assert.ErrorIs(t, err, errdefs.ErrFailedPrecondition) + assert.NotErrorIs(t, err, errdefs.ErrNotImplemented, + "must not fall through to the next differ") + }) + } + + // A writable snapshot is applied normally. The read-only guard does not + // fire. Apply reaches the media type check and fails there. + t.Run("writable mounts are not refused", func(t *testing.T) { + mounts := []mount.Mount{{ + Type: "bind", + Source: "/var/lib/containerd/snapshots/3/fs", + Options: []string{"rw", "rbind"}, + }} + desc := ocispec.Descriptor{MediaType: "application/vnd.oci.image.layer.v1.tar+unsupported"} + _, err := d.Apply(ctx, desc, mounts) + require.Error(t, err) + assert.NotErrorIs(t, err, errdefs.ErrFailedPrecondition) + }) +}