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

Prevent permission issues after ingesting compat layer update tarballs #144

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bedroge
Copy link
Collaborator

@bedroge bedroge commented Feb 2, 2023

Fixes / prevents #143.

@@ -182,7 +182,7 @@ function ingest_compat_tarball() {
echo_yellow "Removing the existing layer, and adding the new one from the tarball..."
cvmfs_server transaction "${repo}"
rm -rf "/cvmfs/${repo}/${basedir}/${version}/compat/${os}/${arch}/"
Copy link
Contributor

Choose a reason for hiding this comment

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

@bedroge We should probably check with CVMFS devs if there's a recommend approach for this type of "replace" operation...

Ideally we don't need to unpack the tarball at all, without having to publish the removal first

Copy link
Collaborator Author

@bedroge bedroge Feb 2, 2023

Choose a reason for hiding this comment

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

As discussed during the monthly meeting, we could ingest the new tarball/layer to a directory with a revision number in its name (e.g. 2021.12-1), and then switch a (variant) symlink 2021.12 to that actual version. In case of issues, we can easily revert to the older version. And if things are okay, we could even remove the old (and possibly insecure) version after a while.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That does require a change in the tarball itself, as we can't simply ingest it to the versions base dir. We would either need to modify the way the tarball is created, or somehow strip the first directory (2021.12) from every file/dir in the tarball and replace it by / ingest it to 2021.12-<n>.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we strongly prefer using cmvfs_server ingest, we should change the way the tarball is created.

The ingest script could then also fail hard if the top-level directory isn't what's expected (2021.12.<n+1> if the current is 2021.12.<n>).

I would prefer using a versioning scheme like 2021.12-001, 2021.12-002 (since that sorts better)

@bedroge
Copy link
Collaborator Author

bedroge commented Nov 14, 2023

This does solve the permission issues in case we already have an existing compat layer in place, but I just ran into similar permission issues with a tarball for a compat layer that didn't exist yet. The default behavior of cvmfs_server ingest is to preserve the owner as well, which then breaks the nested catalog creation. There's an open PR to solve this: cvmfs/cvmfs#3362.

@boegel
Copy link
Contributor

boegel commented Jan 20, 2024

@bedroge Is this still relevant?

@bedroge
Copy link
Collaborator Author

bedroge commented Jan 25, 2024

@bedroge Is this still relevant?

I think this is indeed still needed for replacing compat layer dirs (i.e. for cases where we cannot use cvmfs_server ingest). For cases where we can use ingest, we could make use of the functionality from PR cvmfs/cvmfs#3362, though that doesn't seem to be part of a release yet.

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

Successfully merging this pull request may close these issues.

2 participants