Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backup: store directories if not root/root 0755 #297

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mglae
Copy link
Contributor

@mglae mglae commented May 10, 2023

The LE backup code assume that all directories are of root/root 0755.

This was correct in the past but at least since using docker overlays directories may be restored with wrong modes or owners.

Add code to store directories with different modes or owners in the backup.

@antonlacon
Copy link
Contributor

I'm not familiar with docker or docker overlays. My understanding is the change adds the given item to the backup tarball if it's one of: not a directory with 755 permissions, not owned by root, or not in root's group. I don't understand why the current code is failing to have the correct mode or owner or how this resolves it.

Shouldn't the test for dir+permissions and root/root be part of the if/elif's starting at line 633 using itempaths a few lines below?

@mglae
Copy link
Contributor Author

mglae commented May 16, 2023

I don't understand why the current code is failing to have the correct mode or owner or how this resolves it.

Currently directories are only stored into backup if they are leaves without any files. Implicit directories (occurring first in path of files) are automatically created root/root 0755.

Only other directories have to be stored into backup.

Shouldn't the test for dir+permissions and root/root be part of the if/elif's starting at line 633 using itempaths a few lines below?

Then root backup directories with unusual modes like /storage/.ssh are not stored.

Side note: /storage/.ssh is not an issue in practice because mode is fixed via tmpfiles,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants