Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for 'except' feature of network policy rule #543

Conversation

jimmy-zh
Copy link
Collaborator

@jimmy-zh jimmy-zh commented Sep 27, 2018

Add support for except feature of network policy rule.

Solve #326 based on #529

I have completed the following test cases:

  1. Deploy and test network policy rules that do not include except rule.
  2. Deploy and test network policy rules including multiple except rules.
  3. Perform a rolling update on kube-router from master to this PR.

@jimmy-zh jimmy-zh force-pushed the add-support-for-except-feature-in-NPC branch from c16d568 to eb63d8d Compare September 27, 2018 11:47
@jimmy-zh jimmy-zh force-pushed the add-support-for-except-feature-in-NPC branch from eb63d8d to f1a3e2d Compare September 27, 2018 11:50
@sohel2020
Copy link

sohel2020 commented Sep 27, 2018

@jimmy-zh
getting following error
core@ip-10-112-18-209 ~ $ sudo journalctl -u kube-router -f -- Logs begin at Thu 2018-09-27 13:50:29 UTC. -- Sep 27 13:55:12 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:12.558988 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:13 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:13.703657 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:14 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:14.814110 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:20 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:20.290971 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:23 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:23.326877 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:24 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:24.444111 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid Sep 27 13:55:26 ip-10-112-18-209.ec2.internal bash[5776]: E0927 13:55:26.523563 5776 network_policy_controller.go:585] failed to refresh dstIpBlockIpSet: ipset v6.20.1: The value of the CIDR parameter of the IP address is invalid

@jimmy-zh
Copy link
Collaborator Author

@sohel2020
Did you use CIDR like *.*.*.*/0 in network policy rules? This kind of network address with zero prefix size cannot be stored in the ipset of hash:net type.

The CIDR with zero prefix size is supported both in k8s network policy rule defination and iptables -s or -d option while it isn't supported in ipset (#529). I also just realized that.

@murali-reddy @roffe
Any suggestions? Please check this. Thank you!

@murali-reddy
Copy link
Member

@jimmy-zh thanks for your PR. With this i think network policies should be complaint with network policies spec.

I am stealing idea from somewhere, don't know if it actually works. split /0 to two rules

0.0.0.0/1 and 128.0.0.1/1

@jimmy-zh
Copy link
Collaborator Author

@murali-reddy thanks for your idea. Splitting /0 into 0.0.0.0/1 and 128.0.0.0/1 is also the only way I can think of now. I will use this skill to make some modifications to this PR before we find a more elegant approach.

@jimmy-zh
Copy link
Collaborator Author

@murali-reddy I have modified this PR to support the CIDR with zero prefix size and verified the effectiveness of the above skill. Please check this when you are free. Thanks.

@murali-reddy
Copy link
Member

thanks @jimmy-zh i tested exceptfunctionality with zero prefix size successfully.

@sohel2020 would be be able to quickly verify with current patch?

@sohel2020
Copy link

sohel2020 commented Oct 1, 2018

@murali-reddy Now how should write the following snippet ?

egress:
  - to:
    - ipBlock:
        cidr: 0.0.0.0/0
        except:
        - 10.125.0.0/19
        - 10.190.0.0/15
    - ipBlock:
        cidr: 10.125.13.88/32
    - ipBlock:
        cidr: 10.190.4.36/32

@murali-reddy
Copy link
Member

@sohel2020 you don't have to change any thing in network policies. Just try out which ever you are planning to test out. Its only implementation detail that had to be adopted to overcome ipset issue.

@sohel2020
Copy link

@jimmy-zh Thanks for this PR. @murali-reddy It works as expected.

@murali-reddy murali-reddy merged commit a47e0f4 into cloudnativelabs:master Oct 3, 2018
@murali-reddy
Copy link
Member

murali-reddy commented Oct 3, 2018

@jimmy-zh thanks for your great contributions. AFAICS, network policy implementation in kube-router is complete? Do you still see any gaps?

@jimmy-zh
Copy link
Collaborator Author

@murali-reddy Sorry for the late reply. It seems that the named port (#338) has not been implemented. So far, I have not observed any other gaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants