v1.13 backports 2023-10-24 #28761
Merged
v1.13 backports 2023-10-24 #28761
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
[ upstream commit 414814f ] [ backporter's note: kept SECLABEL instead of SECLABEL_IPV4 and SECLABEL_IPV6 as used on main. ] 4de20fc ("bpf: Pass security ID via skb->cb from lxc to host") switched these parts from using set_identity_mark() to set_identity_meta(). Thus we no longer have to respect the opt-in that controls whether the skb mark may be used to store the identity. Signed-off-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 42ef7f3 ] The previous command relies on awk(1) which could behave differently on Linux and macOS. This patch uses `kubectl -o go-template` to remove dependency on awk(1), makes sure key rotation operation works properly on both platform. Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 73d23a6 ] BPF Map Pressure does not just measure "usage", as the current iteration of the docs suggests. It actually measures the required map size against the configured/allocated map size. This metric is useful at values >= 1.0 as it conveys the scope of the problem to users of the metric. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 2218611 ] [ backporter's note: merge conflict in cilium/cmd/encrypt_status_test.go due to changes diff context. Kept the gopkg.in/check.v1 import instead of github.com/cilium/checkmate and removed SetUpSuite, getXfrmState and TestCountUniqueIPsecKeys from that file. ] Moving common IPsec helpers to their own package will allow us to call them from both the agent and the Cilium CLIs. Also move from checkmate to testing.T. Note the prototype of countUniqueIPsecKeys changed to now take the XFRM states in argument. That also allows us to make the corresponding test unprivileged. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 092d661 ] [ backporter's note: conflict in pkg/datapath/linux/ipsec/xfrm_collector_test.go. Resolved by removing the file as in the upstream commit. ] Remove the unit tests for xfrm_collector. They are simply not worth it. All xfrm_collector does is collect statistics via helper functions and feed them into Prometheus functions. The helper functions are covered by unit tests. Prometheus functions are covered by unit tests. We're really just passing the information from one to the other. But to cover this trivial logic in unit tests, we end up copying lots of information across the code and tests. We also need to pass the helper functions as a function pointer to xfrm_collector. Every new metric is going to need a new function pointer. I believe this is a waste of engineering time and very unlikely to catch mistakes. It doesn't even test the helper functions since we mock them out. So it's really unclear what we're trying to cover here. Removing it makes it easier to add new metrics in followup commits. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 9472595 ] This new metric matches the existing information given by cilium encrypt status, showing the number of IPsec keys currently in use on the node. The number of keys should only change during key rotation and it should go back to 1 after a configurable delay. This metric can help alert if something went wrong during a key rotation. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit ac529da ] Provide two new helper functions to count the number of XFRM policies and states on the node, broken down by directions. XFRM policies can have directions in, out, and fwd, whereas XFRM states can only be in or out. For XFRM states, the direction is not a built-in parameter, but something Cilium encodes in the packet mark. These helper functions will be used in the subsequent commit to expose new metrics. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 6db3b8f ] This commit adds new Prometheus metrics for the number of XFRM states and policies for each direction. These can be used to alert on XFRM leaks. We had one such leak in the past for XFRM out policies which led to a performance degradation when the node churn was high. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 58b67dc ] According to the policy API documentation, if a protocol is not specified in a PortProtocol type its protocol is supposed to be presumed as "ANY". The policy package inconsistently enforces this logic. As a result, empty Protocol fields are validated as "ANY", but, in the actual rule logic that splits the "ANY" protocol into the supported protocol types, the comparison was made only to the actual "ANY" protocol constant. This is fixed with an L4Proto method `IsAny` that does both the constant and empty string comparison. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Tobias Klauser <tobias@cilium.io>
[ upstream commit 6517050 ] This commit adds a new line to cilium encrypt status, with the list of interfaces on which decryption can happen: $ ks exec ds/cilium -c cilium-agent -- cilium encrypt status Encryption: IPsec Decryption interface(s): eth0, eth1, eth2 Keys in use: 1 Max Seq. Number: 0x6e/0xffffffff Errors: 0 This can be useful to check that Cilium is attached to all the interfaces it should be attached to (all those that can receive remote pod traffic). Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser
added
kind/backports
tklauser
requested review from
nathanjsweet,
tommyp1ckles,
jschwinger233,
julianwiedmann and
pchaigno
|
/test-backport-1.13 |
pchaigno
approved these changes
Oct 24, 2023
nathanjsweet
approved these changes
Oct 24, 2023
julianwiedmann
approved these changes
Oct 25, 2023
jschwinger233
approved these changes
Oct 25, 2023
|
/test-runtime |
tommyp1ckles
approved these changes
Oct 25, 2023
|
@sayboras looks like some test failures related to envoy (in runtime)? |
As the flaky unit test was introduced by the below backport, I am working on the fix now. Considered that it's only unit test, we probably don't need to revert. |
|
/test-runtime |
|
Re-run test-runtime due to flaky unit test, which should be fixed in #28776 |
tklauser
added
the
ready-to-merge
dylandreimerink
approved these changes
Oct 25, 2023
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.
SECLABELinstead ofSECLABEL_IPV4andSECLABEL_IPV6as used onmainOnce this PR is merged, you can update the PR labels via: