-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add no-relabeling option to backupPVC configmap #8288
Conversation
a9b915d
to
7b27670
Compare
7b27670
to
564d791
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8288 +/- ##
=======================================
Coverage 59.18% 59.19%
=======================================
Files 367 367
Lines 30838 30850 +12
=======================================
+ Hits 18253 18263 +10
- Misses 11124 11125 +1
- Partials 1461 1462 +1 ☔ View full report in Codecov by Sentry. |
pkg/exposer/csi_snapshot.go
Outdated
if value, exists := csiExposeParam.BackupPVCConfig[csiExposeParam.StorageClass]; exists { | ||
if value.StorageClass != "" { | ||
backupPVCStorageClass = value.StorageClass | ||
} | ||
|
||
backupPVCReadOnly = value.ReadOnly | ||
spcNoRelabeling = value.SPCNoRelabeling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spcNoRelabeling
is only required to set when backupPVCReadOnly=true
. However, here we allow users to set it separately.
This would be a security risk strictly speaking, so would we change it to:
if backupPVCReadOnly == true {
spcNoRelabeling = value.SPCNoRelabeling
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 sounds good, lets just document this behavior explicitly (i.e. SPCNoRelabeling
key would be ignored if backupPVCReadOnly
is not set to true.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll do that and update the doc as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lyndon-Li Slight modification to your suggestion -- if SPCNoRelabeling
is true and backupPVCReadOnly
is false, then we log a warning and ignore. Also, docs updated to make it clear that the no relabeling field is ignored if readOnly=false
.
Signed-off-by: Scott Seago <[email protected]>
564d791
to
b1035dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Thank you for contributing to Velero!
Please add a summary of your change
The previous SELinux fix handled podified datamover problems for users who are not setting the readOnly field for their storageclass. When the readOnly field is set (needed for ceph shallow copy), SELinux relabeling can't happen, so datamover backup will fail. This fix adds the
spc_t
SELinux option in cases when users set this field in the backupPVC configmap. This should only be needed for storageclasses that are already settingreadOnly=true
, so a new map key won't be needed, just a new value in the struct.Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
make new-changelog
) or comment/kind changelog-not-required
on this PR.site/content/docs/main
.