initial commit #1
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
aojea
force-pushed
the
initial
branch
2 times, most recently
from
ebfe1db to
e570e76
Compare
Change-Id: I466f51dd29f6e4a93849102da27d4a0fba3afb42
| if !cache.WaitForCacheSync(ctx.Done(), podController.HasSynced) { | ||
| return fmt.Errorf("timed out waiting for caches to sync") |
There was a problem hiding this comment.
Is this exploitable? I don't think so...
There was a problem hiding this comment.
WaitForCacheSync returns false only if the context is cancelled
|
|
||
| namespace, name, err := cache.SplitMetaNamespaceKey(key) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
This can only fail in the event of programmer error. I would just remove the error check (since we don't recover properly anyway if an error does occur here).
There was a problem hiding this comment.
I prefer to keep it this way for consistency
| return true, nil | ||
| }) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
context.Background() can't be cancelled, so it should not be possible to get an error here...
There was a problem hiding this comment.
yeah, but better to be correct
There was a problem hiding this comment.
I'm just trying to enumerate possible ways this could fail and possibly allow the bug to be exploited.
| } | ||
| patchBytes, err := json.Marshal(patch) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
can only fail in the event of a programmer error, and would introduce an exploitable scenario if it did
|
|
||
| patchBytes, err := json.Marshal(patch) | ||
| if err != nil { | ||
| return err |
|
|
||
| c.queue.Forget(key) | ||
| utilruntime.HandleError(err) | ||
| klog.Infof("Dropping network policy %q out of the queue: %v", key, err) |
There was a problem hiding this comment.
Is this exploitable? I guess you'd have to do something like ensure a long network outage occurred while the NetworkPolicy you wanted to later subvert was being created... Seems unlikely.
|
I assume the lack of README is (temporarily) intentional? |
Co-authored-by: Dan Winship <[email protected]>
Co-authored-by: Dan Winship <[email protected]>
Co-authored-by: Dan Winship <[email protected]>
Change-Id: I13797a962aa2521b148516d93956c62487b4dd3a
Change-Id: Id1b256e250c66d7b3fe01f1fb278d1ffaae807cc
added README, this will no longer be required after kubernetes/kubernetes#130035 is implemented, it is being backported to all stable branches |
Change-Id: I0c28b4de7961358cf0d5ac1694b6c035fba64a74
|
lgtm but the tests are failing |
Change-Id: I466f51dd29f6e4a93849102da27d4a0fba3afb42