search

vmware-tanzu / velero

Backup and migrate Kubernetes applications and their persistent volumes
https://velero.io
Apache License 2.0
8.77k stars 1.41k forks source link

Issue 8249: disable selinux relabel for backupPod #8250

Closed Lyndon-Li closed 1 month ago

Lyndon-Li commented 1 month ago

Fix issue #8249, disable selinux relabel for backupPod of data mover

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.21%. Comparing base (025d66d) to head (e42de2b). Report is 8 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #8250 +/- ## ======================================= Coverage 59.20% 59.21% ======================================= Files 367 367 Lines 30838 30844 +6 ======================================= + Hits 18259 18265 +6 Misses 11119 11119 Partials 1460 1460 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Lyndon-Li commented 1 month ago

@sseago We may still have problem even for this PR. From here https://kubernetes.io/docs/concepts/security/pod-security-standards/ we can see that setting spec.securityContext.seLinuxOptions.type is a restricted operation and spc_t is not an allowed type. Therefore, if the pod security policy is set to Restricted in a cluster, the pod will not be able to start.

This echos the first question in my comment here https://github.com/vmware-tanzu/velero/pull/8243#issuecomment-2378470126. Therefore, I am afraid setting spc_t type is still not a safe solution.

Please hold on merging this PR until this question is clear.

sseago commented 1 month ago

Testing on OpenShift was successful with both filesystem and block volumes.

sseago commented 1 month ago

@Lyndon-Li oops. Sorry, I already merged just as your comment was being posted.

sseago commented 1 month ago

Should we revert?

sseago commented 1 month ago

@Lyndon-Li So I think the only path forward is a configurable option. If selinux is enabled, then we either need spc_t or we need to remove that readOnly=true. Lets add a new installer/node-agent flag to configure this behavior. 3 options: 1) no change from current behavior -- this will be the default, won't work for selinux envs 2) Add spc_t selinux option -- preferred behavior for selinux env unless they're running with Restricted pods 3) Remove spec.volumes readOnly field -- required if selinux is enabled and pods are Restricted, but might break shallow copy.

sseago commented 1 month ago

also, we'll be reverting this merge with https://github.com/vmware-tanzu/velero/pull/8253

Lyndon-Li commented 1 month ago

also, we'll be reverting this merge with #8253

Thanks!