From 3cc2343de020fa448138ce0283e8a142c05b6856 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 24 Oct 2024 17:26:23 -0700 Subject: [PATCH] local: avoid writing to content root on readonly store A contentstore can be created on top of readonly path and should not fail unless there is an attempt to write into it. Currently this fails because new ingest directory is created always, meaning for example that you can't create a store to read blobs from OCI layout without it contaminating the OCI layout files. Signed-off-by: Tonis Tiigi --- plugins/content/local/store.go | 30 +++++++++++++++++++++-------- plugins/content/local/store_test.go | 8 ++++---- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/plugins/content/local/store.go b/plugins/content/local/store.go index 30ecb156b..794c82c25 100644 --- a/plugins/content/local/store.go +++ b/plugins/content/local/store.go @@ -68,8 +68,9 @@ type store struct { ls LabelStore integritySupported bool - locksMu sync.Mutex - locks map[string]*lock + locksMu sync.Mutex + locks map[string]*lock + ensureIngestRootOnce func() error } // NewStore returns a local content store @@ -83,18 +84,16 @@ 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) { - if err := os.MkdirAll(filepath.Join(root, "ingest"), 0777); err != nil { - return nil, err - } - supported, _ := fsverity.IsSupported(root) - return &store{ + s := &store{ root: root, ls: ls, integritySupported: supported, locks: map[string]*lock{}, - }, nil + } + s.ensureIngestRootOnce = sync.OnceValue(s.ensureIngestRoot) + return s, nil } func (s *store) Info(ctx context.Context, dgst digest.Digest) (content.Info, error) { @@ -301,6 +300,9 @@ func (s *store) Status(ctx context.Context, ref string) (content.Status, error) func (s *store) ListStatuses(ctx context.Context, fs ...string) ([]content.Status, error) { fp, err := os.Open(filepath.Join(s.root, "ingest")) if err != nil { + if os.IsNotExist(err) { + return nil, nil + } return nil, err } defer fp.Close() @@ -350,6 +352,9 @@ func (s *store) ListStatuses(ctx context.Context, fs ...string) ([]content.Statu func (s *store) WalkStatusRefs(ctx context.Context, fn func(string) error) error { fp, err := os.Open(filepath.Join(s.root, "ingest")) if err != nil { + if os.IsNotExist(err) { + return nil + } return err } defer fp.Close() @@ -536,6 +541,11 @@ func (s *store) writer(ctx context.Context, ref string, total int64, expected di ) foundValidIngest := false + + if err := s.ensureIngestRootOnce(); err != nil { + return nil, err + } + // ensure that the ingest path has been created. if err := os.Mkdir(path, 0755); err != nil { if !os.IsExist(err) { @@ -646,6 +656,10 @@ func (s *store) ingestPaths(ref string) (string, string, string) { return fp, rp, dp } +func (s *store) ensureIngestRoot() error { + return os.MkdirAll(filepath.Join(s.root, "ingest"), 0777) +} + func readFileString(path string) (string, error) { p, err := os.ReadFile(path) return string(p), err diff --git a/plugins/content/local/store_test.go b/plugins/content/local/store_test.go index 6e4219094..827c5e97f 100644 --- a/plugins/content/local/store_test.go +++ b/plugins/content/local/store_test.go @@ -108,10 +108,6 @@ func TestContentWriter(t *testing.T) { defer cleanup() defer testutil.DumpDirOnFailure(t, tmpdir) - if _, err := os.Stat(filepath.Join(tmpdir, "ingest")); os.IsNotExist(err) { - t.Fatal("ingest dir should be created", err) - } - cw, err := cs.Writer(ctx, content.WithRef("myref")) if err != nil { t.Fatal(err) @@ -120,6 +116,10 @@ func TestContentWriter(t *testing.T) { t.Fatal(err) } + if _, err := os.Stat(filepath.Join(tmpdir, "ingest")); os.IsNotExist(err) { + t.Fatal("ingest dir should be created", err) + } + // reopen, so we can test things cw, err = cs.Writer(ctx, content.WithRef("myref")) if err != nil {