[WIP] Modify SCC admission plugin to not mutate a pod when it it's not required #13053
Conversation
| providers, errs := oscc.CreateProvidersFromConstraints(a.GetNamespace(), matchedConstraints, c.client) | ||
| if a.GetOperation() == kadmission.Update { | ||
| for _, constraint := range matchedConstraints { | ||
| if NewSccChecker(constraint).allowsPod(pod) { |
There was a problem hiding this comment.
this needs to be the "allows pod as-is" check
| // 2. filter to ones that do not forbid the pod - TODO | ||
| var filteredConstraints []*kapi.SecurityContextConstraints | ||
| for _, constraint := range matchedConstraints { | ||
| if NewSccChecker(constraint).allowsPod(pod) { |
There was a problem hiding this comment.
this needs to be the "could allow pod" check (pod spec doesn't indicate a runAsUser, for example, and SCC requires a runAsUser in a range, so defaulting could make the pod spec acceptable)
There was a problem hiding this comment.
In other words, "could allow pod" check is weaker than "allows pod as-is" because the former suppose that on step later we'll generate default values for a pod spec. Is it correct?
There was a problem hiding this comment.
each constraint should have implementations of each of those checks. for example, the runasrange constraint:
couldAllowPod would return false if the pod explicitly requests a uid outside the range, true otherwise
allowsPodAsIs would return true if the pod explicitly requests a uid inside the range, false otherwise
openshift-merge-robot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: php-coder Assign the PR to them by writing No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: php-coder Assign the PR to them by writing No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
removed
the
approved
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: php-coder Assign the PR to them by writing No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/unsassign |
|
@php-coder PR needs rebase |
openshift-merge-robot
added
the
needs-rebase
stevekuznetsov
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
@php-coder: Your pull request title starts with "[WIP]", so the do-not-merge/work-in-progress label will be added. This label will ensure that your pull request will not be merged. Remove the prefix from your pull request title to trigger the removal of the label and allow for your pull request to be merged. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/close |
@liggitt @pweil- could you look at this and provide a feedback?
Bugzilla issue: https://bugzilla.redhat.com/show_bug.cgi?id=1383707