Skip to content

local: avoid writing to content root on readonly store#10894

Merged
AkihiroSuda merged 1 commit into
containerd:mainfrom
tonistiigi:fix-readonly-contentstore
Oct 28, 2024
Merged

local: avoid writing to content root on readonly store#10894
AkihiroSuda merged 1 commit into
containerd:mainfrom
tonistiigi:fix-readonly-contentstore

Conversation

@tonistiigi

@tonistiigi tonistiigi commented Oct 25, 2024

Copy link
Copy Markdown
Member

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.

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 <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the fix-readonly-contentstore branch from 88bc402 to 3cc2343 Compare October 25, 2024 01:03
}

func (s *store) ensureIngestRoot() error {
return os.MkdirAll(filepath.Join(s.root, "ingest"), 0777)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different from ingest/ref that uses 0755. Not sure if bug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 0777?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just copying the old code. Commented as I think it should be 0755, but unrelated to this specific issue.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked up history, and it looks like these permissions come all the way from the initial implementation, and were just copied through during refactors 9008471

@AkihiroSuda AkihiroSuda added this pull request to the merge queue Oct 28, 2024
@AkihiroSuda AkihiroSuda added the cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch label Oct 28, 2024
Merged via the queue into containerd:main with commit 1e6fdb5 Oct 28, 2024
@estesp estesp added cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch kind/enhancement size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants