fix endpoint selection for wildcard to/fromEndpoint in CCNP

[fristonio]

See individual commits for more details.

• Fixes Empty to/fromEndpoints in CCNP matches world #12844
• Add unit tests.
• Update pre-flight CCNP checks to check for possible required policy upgrade in cluster.

Example of preflight warning.

Fix endpoint selection for a wildcard to/fromEndpoints in CCNP.
Cilium will only allow access from Cilium-managed endpoints in such cases instead of allowing traffic from any source. Preflight checks, when following the upgrade guide, have been extended to warn users of the new behavior.

[joestringer]

I was expecting this change to be made in the same (or similar) place where we inject the namespace selector for CNP EndpointSelectors, but I don't see that logic here so it's hard to compare against that logic. Were you aware of that other code? Wondering if there's a functional tradeoff from having it here vs. wherever the namespace selector CNP injection logic is.

[fristonio]

Hey Joe!
That was our initial approach towards this. But while working on the issue @aanm and @jrajahalme figured that if we introduce the changes where we inject the namespace selector for CNP we would essentially be modifying the semantics of CCNP. So if a user creates a CCNP with the following spec:

apiVersion: "cilium.io/v2"
kind: CiliumClusterwideNetworkPolicy
metadata:
  name: "test-all-endpoints"
spec:
  endpointSelector:
    matchLabels:
      app: nginx
  ingress:
  - fromEndpoints:
    - {}
    toPorts:
    - ports:
      - port: "80"
        protocol: TCP

The policy user gets using cilium policy get will look something like this, where we have changed the semantics itself for CCNP provided by the user by adding the matchExpression.

[
  {
    "endpointSelector": {
      "matchLabels": {
        "any:app": "nginx"
      }
    },
    "ingress": [
      {
        "fromEndpoints": [
          {
            "matchExpressions": [
              {
                "key": "k8s:io.kubernetes.pod.namespace",
                "operator": "Exists"
              }
            ]
          }
        ],
        "toPorts": [
          {
            "ports": [
              {
                "port": "80",
                "protocol": "TCP"
              }
            ]
          }
        ]
      }
    ],
    "labels": [
      {
        "key": "io.cilium.k8s.policy.derived-from",
        "value": "CiliumClusterwideNetworkPolicy",
        "source": "k8s"
      },
      {
        "key": "io.cilium.k8s.policy.name",
        "value": "test-all-endpoints",
        "source": "k8s"
      },
      {
        "key": "io.cilium.k8s.policy.uid",
        "value": "3f3e0474-3ae7-47e5-8fc2-9eb174ed0594",
        "source": "k8s"
      }
    ]
  }
]

This was the reason in the PR, changes were pushed down for a policy update when we resolve the policy for the endpoints rather than injecting it earlier during a parse.

On digging a bit deeper into how this(the current changes) might affect a user we figured that this implementation might also not be correct because we are adding semantics of K8s to a policy that might not even be related to k8s, because at this point we are resolving a general policy rule. So consider a case where user imports the below policy using cilium policy import $POLICY_FILE

[{
    "endpointSelector": {
      "matchLabels": {
        "any:app": "nginx"
      }
    },
    "ingress": [
      {
        "fromEndpoints": [
          {}
        ],
        "toPorts": [
          {
            "ports": [
              {
                "port": "80",
                "protocol": "TCP"
              }
            ]
          }
        ]
      }
    ],
    "labels": []
}]

Even though the policy does not contain any reference to k8s it will still only allow traffic from cilium managed k8s endpoints. So in a non-K8s environment this is essentially blocking all the traffic.

Now if we were to go back to doing this injection where we do namespace injection for CNP, we will also have the same issue we are trying to solve here for CCNP for all the policies imported via cilium policy import $POLICY_FILE. Where we are allowing all traffic for policies with wildcard imported using cilium policy import. So what we need is a way to identify all the cilium managed endpoints(irrespective of the environment like K8s) and then add a match for those in the section that is currently in the PR.

We are still discussing on slack what would be the best way to achieve this. I will mark the Pull Request draft for now.

[joestringer]

Now if we were to go back to doing this injection where we do namespace injection for CNP, we will also have the same issue we are trying to solve here for CCNP for all the policies imported via cilium policy import $POLICY_FILE.

I think this is an acceptable trade-off. In Kubernetes mode with CNP, we inject the namespace which means that toEndpoints / fromEndpoints matches only pods. With direct cilium policy import, there is no such behaviour so it's "namespace blind" in some sense. Outside of k8s, we're not even guaranteed to have a notion of a 'namespace'.

If there are users who actively care about this kind of case with non-k8s mode, I would encourage them to look into this topic themselves and come up with a proposal to modify the semantics of toEndpoints / fromEndpoints. Right now, I don't think there's sufficient user interest in direct policy import for us to address the underlying questions/semantics in that case.

[fristonio]

Yeah, that seems like a reasonable trade-off to me as well. So just to be clear we will the do namespace injection in to/fromEndpoint selector when we parse the policy from CNP/CCNP right? For direct policy imports, we are currently assuming allow-all as default behavior since we don't have a notion of namespace there?

@jrajahalme @aanm what do you think about this? We will still have the issue we were trying to address earlier, where we change the semantics of CCNP when doing parse.

[fristonio]

Pushed the updated fix.

After this patch, a CCNP created with the below configuration

$ cat ccnp.yaml
apiVersion: "cilium.io/v2"
kind: CiliumClusterwideNetworkPolicy
metadata:
  name: "test-from-endpoints"
spec:
  endpointSelector:
    matchLabels:
      app: test
  ingress:
  - fromEndpoints:
    - {}
    toPorts:
    - ports:
      - port: "80"
        protocol: TCP

Will end in the policy repository like this

$ cilium policy get
[
  {
    "endpointSelector": {
      "matchLabels": {
        "any:app": "test"
      }
    },
    "ingress": [
      {
        "fromEndpoints": [
          {
            "matchExpressions": [
              {
                "key": "k8s:io.kubernetes.pod.namespace",
                "operator": "Exists"
              }
            ]
          }
        ],
        "toPorts": [
          {
            "ports": [
              {
                "port": "80",
                "protocol": "TCP"
              }
            ]
          }
        ]
      }
    ],
    "labels": [
      {
        "key": "io.cilium.k8s.policy.derived-from",
        "value": "CiliumClusterwideNetworkPolicy",
        "source": "k8s"
      },
      {
        "key": "io.cilium.k8s.policy.name",
        "value": "test-from-endpoints",
        "source": "k8s"
      },
      {
        "key": "io.cilium.k8s.policy.uid",
        "value": "7e204037-e1ec-46aa-9884-41fa1ef339d5",
        "source": "k8s"
      }
    ]
  }
]

eventually ending up to allow Ingress from all the endpoints having a namespace label(CIlium managed k8s endpoints), something similar to below output.

$ sudo cilium bpf policy get 728
DIRECTION   LABELS (source:key[=value])                                    PORT/PROTO   PROXY PORT   BYTES   PACKETS
Ingress     reserved:host                                                  ANY          NONE         0       0
Ingress     reserved:remote-node                                           ANY          NONE         0       0
Ingress     k8s:io.cilium.k8s.policy.cluster=default                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.serviceaccount=cilium-etcd-sa
            k8s:io.cilium/app=etcd-operator
            k8s:io.kubernetes.pod.namespace
Ingress     k8s:app=etcd                                                   80/TCP       NONE         0       0
            k8s:etcd_cluster=cilium-etcd
            k8s:io.cilium.k8s.policy.cluster=default
            k8s:io.cilium.k8s.policy.serviceaccount=default
            k8s:io.cilium/app=etcd-operator
            k8s:io.kubernetes.pod.namespace
Ingress     k8s:io.cilium.k8s.policy.cluster=default                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.serviceaccount=kube-dns
            k8s:io.kubernetes.pod.namespace=kube-system
            k8s:k8s-app=kube-dns
Ingress     k8s:eks.amazonaws.com/component=kube-dns                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.cluster=default
            k8s:io.cilium.k8s.policy.serviceaccount=kube-dns
            k8s:io.kubernetes.pod.namespace=kube-system
            k8s:k8s-app=kube-dns
Ingress     k8s:io.cilium.k8s.policy.cluster=default                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.serviceaccount=coredns
            k8s:io.kubernetes.pod.namespace=kube-system
            k8s:k8s-app=kube-dns
Ingress     k8s:io.cilium.k8s.policy.cluster=default                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.serviceaccount=cilium-operator
            k8s:io.cilium/app=operator
            k8s:io.kubernetes.pod.namespace
            k8s:name=cilium-operator
Ingress     k8s:eks.amazonaws.com/component=coredns                        80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.cluster=default
            k8s:io.cilium.k8s.policy.serviceaccount=coredns
            k8s:io.kubernetes.pod.namespace=kube-system
            k8s:k8s-app=kube-dns
Ingress     k8s:io.cilium.k8s.policy.cluster=default                       80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.serviceaccount=cilium-etcd-operator
            k8s:io.cilium/app=etcd-operator
            k8s:io.kubernetes.pod.namespace
            k8s:name=cilium-etcd-operator
Ingress     k8s:app=test                                                   80/TCP       NONE         0       0
            k8s:io.cilium.k8s.policy.cluster=default
            k8s:io.cilium.k8s.policy.serviceaccount=default
            k8s:io.kubernetes.pod.namespace=default
Egress      reserved:unknown

[joestringer]

Looks good to me, just one minor nit.

[fristonio]

test-me-please

[christarazi]

LGTM; just one question below.

[pchaigno]

LGTM. Just one minor nit below.

[fristonio]

test-me-please

[fristonio]

test-me-please