Fix kube-apiserver policy matching feature with tunneling enabled#18527
Conversation
a7cdab0 to
dcda34e
Compare
There was a problem hiding this comment.
It would be good to get a look from the @cilium/sig-encryption team on this change.
f63ec15 to
1fb0de3
Compare
When running in tunneling mode with a kube-apiserver in the cluster, the node with the kube-apiserver will transmit packets towards remote pods with identity 6 (remote-node). The recipient node will receive the packet and trust the identity as "remote-node" and not "kube-apiserver". If the receiving pod has a policy to only allow "from kube-apiserver" then this policy would not work, and thus drop the packet. To fix this, we switch the source identity in bpf_overlay from "remote-node" to the identity of the destination IP via the ipcache, which we expect to be the "kube-apiserver" identity. If the ipcache entry doesn't exist or the identity in the ipcache is "remote-node" already, then this is a no-op (besides the extra ipcache lookup). Going forward, there are two future changes that are needed in order: 1) A subsequent change to make any packet associated with the kube-apiserver IP have ID 7 (rather than ID 6; without needing the logic added by this commit). 2) Once v1.12.y is released, another subsequent change to remove this specific code added in this commit because we can expect all of this traffic to be transmitted with "kube-apiserver" identity (7), **after** (1) is implemented. For a hitless upgrade, we would only support a path from Cilium v1.11.x (this commit would need to be backported to v1.11.x) to v1.12.y. This is because only v1.11.x or later can successfully receive this traffic and handle policy correctly, as guaranteed by the above. Co-authored-by: Joe Stringer <joe@cilium.io> Signed-off-by: Chris Tarazi <chris@isovalent.com>
1fb0de3 to
a9316a6
Compare
|
/test Job 'Cilium-PR-K8s-1.21-kernel-5.4' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment |
joestringer
left a comment
There was a problem hiding this comment.
Oh, one small nit: Please update Documentation/policy/language.rst to remove the text about the known issue. (Won't need re-running the full CI, can add a small patch at the end before merging or follow up with the change in another PR after this)
Additional sanity check coverage that the kube-apiserver policy matching feature still works with a duplicate policy added and the previous removed. Updates: cilium#17829 Signed-off-by: Chris Tarazi <chris@isovalent.com>
Now that the limitation has been fixed, remove it from the docs. Signed-off-by: Chris Tarazi <chris@isovalent.com>
a9316a6 to
d1d3203
Compare
|
I've added an extra commit to address Joe's comment above. The CI failures were legitimate (related to test setup). They should be fixed now. |
|
/test Job 'Cilium-PR-K8s-1.22-kernel-4.19' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment Job 'Cilium-PR-K8s-1.23-kernel-net-next' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment |
|
@joamaki ping :) |
See commit msgs.
Fixes: #18049
Updates: #17962
Updates: #17829