Skip to content

Conversation

@huntergregory
Copy link
Contributor

@huntergregory huntergregory commented May 17, 2022

Fixes a niche bug in NPM v2, where netpol behavior becomes inaccurate if 2+ network policies have the exact same multi-value pod selector, and one of those policies is deleted.

Currently, can mitigate by re-adding one of the existing policies with that pod selector.

Fix

NestedLabelOfPod ipsets must have a different ipset name for each policy.

Example

Using this network policy

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: nestedlabel-policy
  namespace: x
spec:
  ingress:
  - from:
    ports:
    - port: 81
      protocol: TCP
  podSelector:
    matchExpressions:
    - key: pod
      operator: In
      values:
      - a
      - b
  policyTypes:
  - Ingress

and the agnhost pods from here. You'll need to label a node with the vm=0.

The ./connect-agnhosts.sh script can be found here.

$ kubectl apply -f nestedlabel-policy.yaml 
networkpolicy.networking.k8s.io/nestedlabel-policy created

$ ./connect-agnhosts.sh x a x b -r 81
TIMEOUT
command terminated with exit code 1
Connection failed from x/a to x/b via tcp on port 81.
# CORRECTLY BLOCK TRAFFIC

$ kubectl exec -it -n kube-system azure-npm-84v79 -- ipset -L azure-npm-2682470511
Name: azure-npm-2682470511 # nestedlabel-pod:a:b
Type: list:set
Revision: 3
Header: size 8
Size in memory: 176
References: 1
Number of entries: 2
Members:
azure-npm-3922407721 # podlabel-pod:a
azure-npm-3872074864 # podlabel-pod b
# IPSET HAS 2 MEMBERS

$ kubectl apply -f test-policies/nestedlabel-policy-copy.yaml 
networkpolicy.networking.k8s.io/nestedlabel-policy-copy created

$ kubectl exec -it -n kube-system azure-npm-84v79 -- ipset -L azure-npm-2682470511
Name: azure-npm-2682470511 # nestedlabel-pod:a:b
Type: list:set
Revision: 3
Header: size 8
Size in memory: 176
References: 2
Number of entries: 2
Members:
azure-npm-3922407721 # podlabel-pod a
azure-npm-3872074864 # podlabel-pod b
# IPSET HAS SAME 2 MEMBERS

$ kubectl delete netpol -n x nestedlabel-policy-copy
networkpolicy.networking.k8s.io "nestedlabel-policy-copy" deleted

$ kubectl exec -it -n kube-system azure-npm-84v79 -- ipset -L azure-npm-2682470511
Name: azure-npm-2682470511
Type: list:set
Revision: 3
Header: size 8
Size in memory: 80
References: 1
Number of entries: 0
Members:
# IPSET HAS NO MEMBERS NOW

$ ./connect-agnhosts.sh x a x b -r 81
Successfully connected from x/a to x/b via tcp on port 81.
# INCORRECTLY ALLOW TRAFFIC after deleting the network policy with the same multi-label pod selector

@huntergregory huntergregory added the npm Related to NPM. label May 17, 2022
@huntergregory huntergregory requested a review from a team as a code owner May 17, 2022 01:04
@huntergregory huntergregory requested review from matmerr and removed request for a team May 17, 2022 01:04
@huntergregory
Copy link
Contributor Author

/azp run

1 similar comment
@huntergregory
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@huntergregory huntergregory changed the title fix: [NPM] fix nested label translate bug fix: [NPM] niche multi-value pod selector bug May 19, 2022
@huntergregory huntergregory enabled auto-merge (squash) May 19, 2022 01:29
@tamilmani1989 tamilmani1989 disabled auto-merge May 19, 2022 16:42
@tamilmani1989 tamilmani1989 merged commit cb85674 into master May 19, 2022
@tamilmani1989 tamilmani1989 deleted the npm-translation-bug branch May 19, 2022 16:42
matmerr pushed a commit to matmerr/azure-container-networking that referenced this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

npm Related to NPM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants