Enable kdmp & NFS jobs to run in restricted PSA mode #378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
OSS Scan Results:
Total issues: 196 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 137 |
|
License Evaluation Results:
Total License Issues: 17 |
lalat-das
changed the title
|
OSS Scan Results:
Total issues: 142 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 142 |
|
License Evaluation Results:
Total License Issues: 17 |
|
OSS Scan Results:
Total issues: 142 |
|
License Evaluation Results:
Total License Issues: 17 |
prashanthpx
reviewed
Jun 15, 2024
| // if the job event reason is Failedcreate due to fobidden podSecurity violation | ||
| // then return true | ||
| for _, event := range events.Items { | ||
| if event.Reason == "FailedCreate" && strings.Contains(event.Message, "violates PodSecurity") { |
There was a problem hiding this comment.
Is this message the same if there is no PodSecurity context and only container security context violation?
There was a problem hiding this comment.
Yes they are same. Thanks for asking I tried in 1.27 k8s at least in that version it is same.
pkg/drivers/utils/utils.go
Outdated
| @@ -966,3 +997,91 @@ func GetShortUID(uid string) string { | |||
| } | |||
| return uid[:8] | |||
| } | |||
|
|
|||
| // Add container security Context to job pod if the PSA is enabled. | |||
| // if static uids like kdmpJobUid or kdmpJobGid is used that means | |||
There was a problem hiding this comment.
nit: Sentence to start with caps
| // of the backup pod filesystem. | ||
| if !strings.Contains(job.Spec.Template.Spec.Containers[0].Command[0], kopiaBackupString) { | ||
| job.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{ | ||
| FSGroup: &uid, |
There was a problem hiding this comment.
If it is no OCP backup where runAsUser, runAsGroup and FsGroup was set and restoring to another vanila cluster, should we not add the same to kdmp job pods? If we don't add wouldn't the job pod runs as root?
There was a problem hiding this comment.
In case of a non-ocp based backup and restore user group is preserved anyway and it will be used.. May be I need to understand the comment better. Let me discus this in Monday.
| job.Spec.Template.Spec.Containers[0].SecurityContext.RunAsGroup = &gid | ||
| } | ||
| // Add RunAsNonRoot to true and drop all capabilities and seccomp profile and allowPrivilegeEscalation to false | ||
| job.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot = ptr.To(true) |
There was a problem hiding this comment.
For a normal bakcup where no security context was set, kdmp pod will run as root. With this check on the restore side, are making it run as non-root, what would be the user after restore? During backup they would be as root
There was a problem hiding this comment.
So you are saying a scenario where in backup is taken for an app where in security context is not there So job will run as root . During restore we will check if volume info In backup if there is a UID mentioned there won't be anything used in data export CR as annotation and we will not add any security context bloc.
prashanthpx
approved these changes
Jun 15, 2024
- if psa enabled then extracted the uid and gid used by the POD. here the pod is choosen based on whichever PVC is used by that pod - applied those uid and GID to all relevant job pod spec - for baseline and privilege mode no restriction on UID/GID and default setting of SElinux and secomp is adopted in the job POD spec Signed-off-by: Lalatendu Das <[email protected]>
- checked job's event for a filedCreate reason and specifically for violating pod security standard. - the check is added for all the kdmp & nfs related job pods. Signed-off-by: Lalatendu Das <[email protected]>
|
OSS Scan Results:
Total issues: 142 |
|
License Evaluation Results:
Total License Issues: 17 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: This PR brings changes of psa and azure chine changes to kdmp via vending from stork to kdmp
Which issue(s) this PR fixes (optional)
Closes # pb-6681
Special notes for your reviewer:
Unit tested backup and restore with 1.2.13 executor image with corresponding stork PR
Restore of the above non-psi and psi backups are as below
