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

Disable xattrs on rsync #694

Merged
merged 5 commits into from
Mar 6, 2025
Merged

Disable xattrs on rsync #694

merged 5 commits into from
Mar 6, 2025

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Mar 4, 2025

For systems with selinux enabled, this makes impossible to generate the proper files as its being blocked

This fixes it as tested on my Fedora with selinux.

Fixes #kairos-io/AuroraBoot#196

For systems with selinux enabled, this makes impossible to generate the
proper files as its being blocked

Signed-off-by: Itxaka <[email protected]>
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.25%. Comparing base (09c138e) to head (9cd602f).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #694      +/-   ##
==========================================
+ Coverage   48.19%   48.25%   +0.06%     
==========================================
  Files          48       48              
  Lines        6161     6169       +8     
==========================================
+ Hits         2969     2977       +8     
  Misses       2912     2912              
  Partials      280      280              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Itxaka Itxaka marked this pull request as ready for review March 4, 2025 14:44
@Itxaka Itxaka requested a review from a team March 4, 2025 14:44
@jimmykarily
Copy link
Contributor

I don't know much about selinux but xattrs is supposed to maintain "SELinux contexts" or "labels". I suppose we don't need it for as long as selinux is not supported right? Otherwise those metadata should be maintained?

@Itxaka
Copy link
Member Author

Itxaka commented Mar 5, 2025

I don't know much about selinux but xattrs is supposed to maintain "SELinux contexts" or "labels". I suppose we don't need it for as long as selinux is not supported right? Otherwise those metadata should be maintained?

I think the issue here is, when building or rsyncing stuff in a host that has selinux, it autosets those selinux contexts to the extracted data, and then on copying those attrs cannot be maintained.

For building images, it will always fail when rsyncing stuff into a FAT partition as its impossible to save thos attributes in FAT.

Also, the question is, are those attrs the ones originally from the extracted OCI artifact, or are they new and set by the host on data extraction? If they are set on data extraction, due to the host that extracts it having selinux, then that data is useless, as its new and not appropiate to the final system. AFAIK we dont save any selinux content on OCI artifact creation so thats why this only happens to fail in selinux hosts that build images.

IMHO, this is only a workaround for now. When we start dealing with selinux we will need to rework this properly to maintain the original selinux context if any and to deal with hosts that have selinux enabled and try to label things when we extract it :(

Itxaka added 2 commits March 5, 2025 09:20
Some flags already include the rest, explain what they do

Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka requested review from jimmykarily and a team March 5, 2025 08:21
@Itxaka
Copy link
Member Author

Itxaka commented Mar 5, 2025

I don't know much about selinux but xattrs is supposed to maintain "SELinux contexts" or "labels". I suppose we don't need it for as long as selinux is not supported right? Otherwise those metadata should be maintained?

I think the issue here is, when building or rsyncing stuff in a host that has selinux, it autosets those selinux contexts to the extracted data, and then on copying those attrs cannot be maintained.

For building images, it will always fail when rsyncing stuff into a FAT partition as its impossible to save thos attributes in FAT.

Also, the question is, are those attrs the ones originally from the extracted OCI artifact, or are they new and set by the host on data extraction? If they are set on data extraction, due to the host that extracts it having selinux, then that data is useless, as its new and not appropiate to the final system. AFAIK we dont save any selinux content on OCI artifact creation so thats why this only happens to fail in selinux hosts that build images.

IMHO, this is only a workaround for now. When we start dealing with selinux we will need to rework this properly to maintain the original selinux context if any and to deal with hosts that have selinux enabled and try to label things when we extract it :(

Ther eis also the possibility of having this wrong in the context of, you dont need to keep the original selinux labels as on first boot it should relabel everything.
I think that the original labels migth not be useful, as the selinux workflow is to relabel everything on first boot, but I have no idea if thats true and how that works with our immutable system....

@Itxaka Itxaka merged commit 11cdddc into main Mar 6, 2025
14 checks passed
@Itxaka Itxaka deleted the xattrs_rsync branch March 6, 2025 09:33
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