/test

/test

I investigated in more detail the issue and I believe this is not what we want.

This fix successfully equates the two cases:

1. apply of the policy:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: test
spec:
  endpointSelector:
    matchLabels:
      run: tmp-shell
  egress:
  - toCIDR:
    - 1.1.1.1/32
    toEndpoints: [] # <-- this
    toPorts:
      - ports:
          - port: "80"

2. apply of the policy with a nil toEndpoints selector, then update it with an empty non-nil toEndpoints selector, ending up with the same policy as in case 1

But it does that in a way that reverts what #29608 fixed. The failed tests reflects this (as an example, see TestMergeDenyAllL3 - case 1B, that was introduced in #29608).

In other words, as suggested in the issue, the correct behavior should be that the two scenarios above ("direct" apply of the final version of the policy and apply+update) both lead to the policy denying connections to 1.1.1.1.
Quoting from the issue:

Expected

The behaviour should be consistent regardless of the order of operation on a policy.
Following result is what a user should experience at all times for above policy with toEndpoints: [] in 1.16:

$ curl -m 1 1.1.1.1
curl: (28) Connection timed out after 1002 milliseconds

$ curl -m 1 1.1.1.2
curl: (28) Connection timed out after 1001 milliseconds

This is not happening with the proposed fix, that allows the curl 1.1.1.1 in both case 1) and 2).

To get the expected behavior I think the solution should be this one, where we change the logic to compare EgressCommonRule and IngressCommonRule instances, taking into account the different semantic of nil and empty slices and propagating the update in the scenario above as expected.

Closing in favor of #33506