From 01fa05731f1abaa2ef5d230ab0b7cbde9eed57c1 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 7 Jan 2026 14:53:24 -0300 Subject: [PATCH 1/4] Add Parent option handling in Commit method and tests Signed-off-by: Manuel de Brito Fontes --- contrib/snapshotservice/service.go | 3 + contrib/snapshotservice/service_test.go | 147 ++++++++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 contrib/snapshotservice/service_test.go diff --git a/contrib/snapshotservice/service.go b/contrib/snapshotservice/service.go index 17bda7a6390e..b03afafe3ba1 100644 --- a/contrib/snapshotservice/service.go +++ b/contrib/snapshotservice/service.go @@ -85,6 +85,9 @@ func (s service) Commit(ctx context.Context, cr *snapshotsapi.CommitSnapshotRequ if cr.Labels != nil { opts = append(opts, snapshots.WithLabels(cr.Labels)) } + if cr.Parent != "" { + opts = append(opts, snapshots.WithParent(cr.Parent)) + } if err := s.sn.Commit(ctx, cr.Name, cr.Key, opts...); err != nil { return nil, errgrpc.ToGRPC(err) } diff --git a/contrib/snapshotservice/service_test.go b/contrib/snapshotservice/service_test.go new file mode 100644 index 000000000000..b2b1a4cef69e --- /dev/null +++ b/contrib/snapshotservice/service_test.go @@ -0,0 +1,147 @@ +/* + 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 snapshotservice + +import ( + "context" + "testing" + + snapshotsapi "github.com/containerd/containerd/api/services/snapshots/v1" + "github.com/containerd/containerd/v2/core/mount" + "github.com/containerd/containerd/v2/core/snapshots" +) + +// mockSnapshotter is a mock implementation of snapshots.Snapshotter +// that captures the options passed to Commit for testing. +type mockSnapshotter struct { + commitOpts []snapshots.Opt +} + +func (m *mockSnapshotter) Stat(ctx context.Context, key string) (snapshots.Info, error) { + return snapshots.Info{}, nil +} + +func (m *mockSnapshotter) Update(ctx context.Context, info snapshots.Info, fieldpaths ...string) (snapshots.Info, error) { + return snapshots.Info{}, nil +} + +func (m *mockSnapshotter) Usage(ctx context.Context, key string) (snapshots.Usage, error) { + return snapshots.Usage{}, nil +} + +func (m *mockSnapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount, error) { + return nil, nil +} + +func (m *mockSnapshotter) Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) { + return nil, nil +} + +func (m *mockSnapshotter) View(ctx context.Context, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) { + return nil, nil +} + +func (m *mockSnapshotter) Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) error { + m.commitOpts = opts + return nil +} + +func (m *mockSnapshotter) Remove(ctx context.Context, key string) error { + return nil +} + +func (m *mockSnapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error { + return nil +} + +func (m *mockSnapshotter) Close() error { + return nil +} + +// TestCommitParentOption verifies that the Parent field from CommitSnapshotRequest +// is correctly passed to the snapshotter via WithParent option. +func TestCommitParentOption(t *testing.T) { + for _, tc := range []struct { + name string + parent string + labels map[string]string + expectedParent string + expectedLabels map[string]string + }{ + { + name: "WithParent", + parent: "parent-snapshot", + expectedParent: "parent-snapshot", + }, + { + name: "WithoutParent", + parent: "", + expectedParent: "", + }, + { + name: "WithLabelsAndParent", + parent: "parent-snapshot", + labels: map[string]string{"test-label": "test-value"}, + expectedParent: "parent-snapshot", + expectedLabels: map[string]string{"test-label": "test-value"}, + }, + { + name: "WithLabelsOnly", + parent: "", + labels: map[string]string{"key": "value"}, + expectedParent: "", + expectedLabels: map[string]string{"key": "value"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mock := &mockSnapshotter{} + svc := FromSnapshotter(mock) + + req := &snapshotsapi.CommitSnapshotRequest{ + Name: "test-snapshot", + Key: "test-key", + Parent: tc.parent, + Labels: tc.labels, + } + + _, err := svc.Commit(context.Background(), req) + if err != nil { + t.Fatalf("Commit failed: %v", err) + } + + // Apply all opts to check the resulting Info + info := &snapshots.Info{} + for _, opt := range mock.commitOpts { + if err := opt(info); err != nil { + t.Fatalf("failed to apply opt: %v", err) + } + } + + if info.Parent != tc.expectedParent { + t.Errorf("expected parent %q, got %q", tc.expectedParent, info.Parent) + } + + if tc.expectedLabels != nil { + for k, v := range tc.expectedLabels { + if info.Labels[k] != v { + t.Errorf("expected label %q=%q, got %q", k, v, info.Labels[k]) + } + } + } + }) + } +} From 9018c75d5d720175d438fa1c8ee08803c451737c Mon Sep 17 00:00:00 2001 From: ningmingxiao Date: Fri, 9 Jan 2026 11:28:54 +0800 Subject: [PATCH 2/4] cri: fix create container panic if originalAnnotations is nil when restore container Signed-off-by: ningmingxiao --- internal/cri/server/container_checkpoint_linux.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/cri/server/container_checkpoint_linux.go b/internal/cri/server/container_checkpoint_linux.go index b54963ae8676..b07491d423aa 100644 --- a/internal/cri/server/container_checkpoint_linux.go +++ b/internal/cri/server/container_checkpoint_linux.go @@ -253,6 +253,9 @@ func (c *criService) CRImportCheckpoint( } originalAnnotations := containerStatus.GetAnnotations() + if originalAnnotations == nil { + originalAnnotations = make(map[string]string) + } originalLabels := containerStatus.GetLabels() sandboxUID := sandboxConfig.GetMetadata().GetUid() From 5f0f0dcaac13a5370a28a42ada9fd6be246ee807 Mon Sep 17 00:00:00 2001 From: ningmingxiao Date: Sun, 26 Oct 2025 22:39:28 +0800 Subject: [PATCH 3/4] content: ensure root directory exists before checking fs-verity support Currently, fs-verity support detection fails on fresh containerd installations because the content store root directory (io.containerd.content.v1.content) doesn't exist yet. This directory is only created when pulling images, causing checker to always be false on new hosts. The IsSupported() function attempts to create a temporary directory within rootPath to test fs-verity support, but fails when rootPath doesn't exist, returning an error that is silently ignored. Fix this by ensuring the root directory exists before performing the fs-verity support check in NewLabeledStore(). Signed-off-by: ningmingxiao --- plugins/content/local/store.go | 16 +++++++++-- plugins/content/local/store_test.go | 42 ++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/plugins/content/local/store.go b/plugins/content/local/store.go index 794c82c25e3b..1b9abdd3b06c 100644 --- a/plugins/content/local/store.go +++ b/plugins/content/local/store.go @@ -18,8 +18,10 @@ package local import ( "context" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "strconv" @@ -84,8 +86,18 @@ func NewStore(root string) (content.Store, error) { // require labels and should use `NewStore`. `NewLabeledStore` is primarily // useful for tests or standalone implementations. func NewLabeledStore(root string, ls LabelStore) (content.Store, error) { - supported, _ := fsverity.IsSupported(root) - + if _, err := os.Stat(root); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("failed to stat %q: %w", root, err) + } + if err := os.MkdirAll(root, 0755); err != nil { + return nil, fmt.Errorf("failed to mkdir %q: %w", root, err) + } + } + supported, err := fsverity.IsSupported(root) + if err != nil { + log.L.WithError(err).WithField("path", root).Warnf("failed check for fsverity support") + } s := &store{ root: root, ls: ls, diff --git a/plugins/content/local/store_test.go b/plugins/content/local/store_test.go index 54b082dcd6e4..0ce3a9c9c9c3 100644 --- a/plugins/content/local/store_test.go +++ b/plugins/content/local/store_test.go @@ -25,6 +25,7 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "reflect" "runtime" @@ -94,15 +95,50 @@ func (mls *memoryLabelStore) Update(d digest.Digest, update map[string]string) ( func TestContent(t *testing.T) { testsuite.ContentSuite(t, "fs", func(ctx context.Context, root string) (context.Context, content.Store, func() error, error) { cs, err := NewLabeledStore(root, newMemoryLabelStore()) - if err != nil { - return nil, nil, nil, err - } + assert.NoError(t, err) return ctx, cs, func() error { return nil }, nil }) } +func TestContentRootDir(t *testing.T) { + // test dir exist + dirExist := t.TempDir() + _, err := NewLabeledStore(dirExist, newMemoryLabelStore()) + assert.NoError(t, err) + // test dir doesn't exist + dir := filepath.Join(t.TempDir(), "test_dir001") + _, err = NewLabeledStore(dir, newMemoryLabelStore()) + assert.NoError(t, err) + _, err = os.Stat(dir) + assert.NoError(t, err) +} + +func TestInvalidPermissionRootDir(t *testing.T) { + // test dir permissions are invalid + if os.Getuid() != 0 { + t.Skip("skipping test that requires root") + } + _, err := exec.LookPath("chattr") + if err != nil { + t.Skip("skipping test that requires chattr command") + } + dirBadPermission := t.TempDir() + cmd := exec.Command("chattr", "+i", dirBadPermission) + _, err = cmd.CombinedOutput() + assert.NoError(t, err) + defer func() { + cmd := exec.Command("chattr", "-i", dirBadPermission) + _, err = cmd.CombinedOutput() + assert.NoError(t, err) + }() + _, err = fsverity.IsSupported(dirBadPermission) + if err == nil { + t.Fatal(fmt.Errorf("err can't be nil")) + } +} + func TestContentWriter(t *testing.T) { ctx, tmpdir, cs, cleanup := contentStoreEnv(t) defer cleanup() From b0bd04b0469b28ed0cb1f4d21682e34caf7e45f3 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Fri, 9 Jan 2026 14:44:17 +0200 Subject: [PATCH 4/4] cri,nri: pass container user (uid, gids) to plugins. Signed-off-by: Krisztian Litkey --- internal/cri/nri/nri_api_linux.go | 13 +++++++++++++ internal/nri/container.go | 2 ++ 2 files changed, 15 insertions(+) diff --git a/internal/cri/nri/nri_api_linux.go b/internal/cri/nri/nri_api_linux.go index 374dcae1a714..b34ebbb4de39 100644 --- a/internal/cri/nri/nri_api_linux.go +++ b/internal/cri/nri/nri_api_linux.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "maps" + "slices" eventtypes "github.com/containerd/containerd/api/events" containerd "github.com/containerd/containerd/v2/client" @@ -1043,6 +1044,18 @@ func (c *criContainer) GetRlimits() []*api.POSIXRlimit { return rlimits } +func (c *criContainer) GetUser() *api.User { + if c.spec.Process == nil { + return nil + } + + return &api.User{ + Uid: c.spec.Process.User.UID, + Gid: c.spec.Process.User.GID, + AdditionalGids: slices.Clone(c.spec.Process.User.AdditionalGids), + } +} + // // conversion to/from CRI types // diff --git a/internal/nri/container.go b/internal/nri/container.go index ab1cd14eab7c..6dc9cb57f6e4 100644 --- a/internal/nri/container.go +++ b/internal/nri/container.go @@ -48,6 +48,7 @@ type Container interface { GetLinuxContainer() LinuxContainer GetCDIDevices() []*nri.CDIDevice GetRlimits() []*nri.POSIXRlimit + GetUser() *nri.User } type LinuxContainer interface { @@ -85,6 +86,7 @@ func commonContainerToNRI(ctr Container) *nri.Container { FinishedAt: status.FinishedAt, ExitCode: status.ExitCode, Rlimits: ctr.GetRlimits(), + User: ctr.GetUser(), } }