Skip to content

Move Cidr translation from iptable to ipset. #582

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 22, 2020
Merged

Move Cidr translation from iptable to ipset. #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f747a4b
Move Cidr translation from iptable to ipset.
csfmomo Jun 16, 2020
15716a6
Move Cidr translation from iptable to ipset.
csfmomo Jun 17, 2020
46bb100
Move Cidr translation from iptable to ipset.
csfmomo Jun 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions npm/ipsm/ipsm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type ipsEntry struct {
operationFlag string
name string
set string
spec string
spec []string
}

// IpsetManager stores ipset states.
Expand Down Expand Up @@ -81,7 +81,7 @@ func (ipsMgr *IpsetManager) CreateList(listName string) error {
name: listName,
operationFlag: util.IpsetCreationFlag,
set: util.GetHashedName(listName),
spec: util.IpsetSetListFlag,
spec: []string{util.IpsetSetListFlag},
}
log.Printf("Creating List: %+v", entry)
if errCode, err := ipsMgr.Run(entry); err != nil && errCode != 1 {
Expand Down Expand Up @@ -132,7 +132,7 @@ func (ipsMgr *IpsetManager) AddToList(listName string, setName string) error {
entry := &ipsEntry{
operationFlag: util.IpsetAppendFlag,
set: util.GetHashedName(listName),
spec: util.GetHashedName(setName),
spec: []string{util.GetHashedName(setName)},
}

if errCode, err := ipsMgr.Run(entry); err != nil && errCode != 1 {
Expand Down Expand Up @@ -162,7 +162,7 @@ func (ipsMgr *IpsetManager) DeleteFromList(listName string, setName string) erro
entry := &ipsEntry{
operationFlag: util.IpsetDeletionFlag,
set: hashedListName,
spec: hashedSetName,
spec: []string{hashedSetName},
}

if _, err := ipsMgr.Run(entry); err != nil {
Expand All @@ -181,7 +181,7 @@ func (ipsMgr *IpsetManager) DeleteFromList(listName string, setName string) erro
}

// CreateSet creates an ipset.
func (ipsMgr *IpsetManager) CreateSet(setName, spec string) error {
func (ipsMgr *IpsetManager) CreateSet(setName string, spec []string) error {
if _, exists := ipsMgr.setMap[setName]; exists {
return nil
}
Expand Down Expand Up @@ -211,10 +211,6 @@ func (ipsMgr *IpsetManager) DeleteSet(setName string) error {
return nil
}

if len(ipsMgr.setMap[setName].elements) > 0 {
return nil
}

entry := &ipsEntry{
operationFlag: util.IpsetDestroyFlag,
set: util.GetHashedName(setName),
Expand All @@ -240,14 +236,21 @@ func (ipsMgr *IpsetManager) AddToSet(setName, ip, spec string) error {
return nil
}

if err := ipsMgr.CreateSet(setName, spec); err != nil {
if err := ipsMgr.CreateSet(setName, append([]string{util.IpsetNetHashFlag})); err != nil {
return err
}
var resultSpec []string
if strings.Contains(ip, util.IpsetNomatch) {
ip = strings.Trim(ip, util.IpsetNomatch)
resultSpec = append([]string{ip, util.IpsetNomatch})
} else {
resultSpec = append([]string{ip})
}

entry := &ipsEntry{
operationFlag: util.IpsetAppendFlag,
set: util.GetHashedName(setName),
spec: ip,
spec: resultSpec,
}

if errCode, err := ipsMgr.Run(entry); err != nil && errCode != 1 {
Expand Down Expand Up @@ -276,7 +279,7 @@ func (ipsMgr *IpsetManager) DeleteFromSet(setName, ip string) error {
entry := &ipsEntry{
operationFlag: util.IpsetDeletionFlag,
set: util.GetHashedName(setName),
spec: ip,
spec: append([]string{ip}),
}

if errCode, err := ipsMgr.Run(entry); err != nil {
Expand Down Expand Up @@ -342,13 +345,8 @@ func (ipsMgr *IpsetManager) Destroy() error {
// Run execute an ipset command to update ipset.
func (ipsMgr *IpsetManager) Run(entry *ipsEntry) (int, error) {
cmdName := util.Ipset
cmdArgs := []string{entry.operationFlag, util.IpsetExistFlag}
if len(entry.set) > 0 {
cmdArgs = append(cmdArgs, entry.set)
}
if len(entry.spec) > 0 {
cmdArgs = append(cmdArgs, entry.spec)
}
cmdArgs := append([]string{entry.operationFlag, util.IpsetExistFlag, entry.set}, entry.spec...)
cmdArgs = util.DropEmptyFields(cmdArgs)

log.Printf("Executing ipset command %s %v", cmdName, cmdArgs)
_, err := exec.Command(cmdName, cmdArgs...).Output()
Expand Down
21 changes: 15 additions & 6 deletions npm/ipsm/ipsm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestAddToList(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestAddToList failed @ ipsMgr.CreateSet")
}

Expand All @@ -98,7 +98,7 @@ func TestDeleteFromList(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestDeleteFromList failed @ ipsMgr.CreateSet")
}

Expand Down Expand Up @@ -127,9 +127,14 @@ func TestCreateSet(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("test-set", []string{util.IpsetNetHashFlag}); err != nil {
t.Errorf("TestCreateSet failed @ ipsMgr.CreateSet")
}

spec := append([]string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum})
if err := ipsMgr.CreateSet("test-set-with-maxelem", spec); err != nil {
t.Errorf("TestCreateSet failed @ ipsMgr.CreateSet when set maxelem")
}
}

func TestDeleteSet(t *testing.T) {
Expand All @@ -144,7 +149,7 @@ func TestDeleteSet(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestDeleteSet failed @ ipsMgr.CreateSet")
}

Expand All @@ -168,6 +173,10 @@ func TestAddToSet(t *testing.T) {
if err := ipsMgr.AddToSet("test-set", "1.2.3.4", util.IpsetNetHashFlag); err != nil {
t.Errorf("TestAddToSet failed @ ipsMgr.AddToSet")
}

if err := ipsMgr.AddToSet("test-set", "1.2.3.4/nomatch", util.IpsetNetHashFlag); err != nil {
t.Errorf("TestAddToSet with nomatch failed @ ipsMgr.AddToSet")
}
}

func TestDeleteFromSet(t *testing.T) {
Expand Down Expand Up @@ -203,7 +212,7 @@ func TestClean(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestClean failed @ ipsMgr.CreateSet")
}

Expand Down Expand Up @@ -248,7 +257,7 @@ func TestRun(t *testing.T) {
entry := &ipsEntry{
operationFlag: util.IpsetCreationFlag,
set: "test-set",
spec: util.IpsetNetHashFlag,
spec: append([]string{util.IpsetNetHashFlag}),
}
if _, err := ipsMgr.Run(entry); err != nil {
t.Errorf("TestRun failed @ ipsMgr.Run")
Expand Down
2 changes: 1 addition & 1 deletion npm/namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {

ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
// Create ipset for the namespace.
if err = ipsMgr.CreateSet(nsName, util.IpsetNetHashFlag); err != nil {
if err = ipsMgr.CreateSet(nsName, append([]string{util.IpsetNetHashFlag})); err != nil {
log.Errorf("Error: failed to create ipset for namespace %s.", nsName)
return err
}
Expand Down
2 changes: 1 addition & 1 deletion npm/npm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in

// Create ipset for the namespace.
kubeSystemNs := "ns-" + util.KubeSystemFlag
if err := allNs.ipsMgr.CreateSet(kubeSystemNs, util.IpsetNetHashFlag); err != nil {
if err := allNs.ipsMgr.CreateSet(kubeSystemNs, append([]string{util.IpsetNetHashFlag})); err != nil {
log.Logf("Error: failed to create ipset for namespace %s.", kubeSystemNs)
}

Expand Down
61 changes: 51 additions & 10 deletions npm/nwpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
package npm

import (
"strconv"

"github.com/Azure/azure-container-networking/log"
"github.com/Azure/azure-container-networking/npm/iptm"
"github.com/Azure/azure-container-networking/npm/ipsm"
"github.com/Azure/azure-container-networking/npm/util"
networkingv1 "k8s.io/api/networking/v1"
)
Expand Down Expand Up @@ -49,7 +52,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
}

if !npMgr.isAzureNpmChainCreated {
if err = allNs.ipsMgr.CreateSet(util.KubeSystemFlag, util.IpsetNetHashFlag); err != nil {
if err = allNs.ipsMgr.CreateSet(util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil {
log.Errorf("Error: failed to initialize kube-system ipset.")
return err
}
Expand All @@ -63,11 +66,12 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
}

var (
hashedSelector = HashSelector(&npObj.Spec.PodSelector)
addedPolicy *networkingv1.NetworkPolicy
sets, namedPorts, lists []string
iptEntries []*iptm.IptEntry
ipsMgr = allNs.ipsMgr
hashedSelector = HashSelector(&npObj.Spec.PodSelector)
addedPolicy *networkingv1.NetworkPolicy
sets, namedPorts, lists []string
ingressIPCidrs, egressIPCidrs [][]string
iptEntries []*iptm.IptEntry
ipsMgr = allNs.ipsMgr
)

// Remove the existing policy from processed (merged) network policy map
Expand All @@ -93,16 +97,16 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP

ns.rawNpMap[npObj.ObjectMeta.Name] = npObj

sets, namedPorts, lists, iptEntries = translatePolicy(npObj)
sets, namedPorts, lists, ingressIPCidrs, egressIPCidrs, iptEntries = translatePolicy(npObj)
for _, set := range sets {
log.Printf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
if err = ipsMgr.CreateSet(set, util.IpsetNetHashFlag); err != nil {
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetNetHashFlag})); err != nil {
log.Printf("Error creating ipset %s", set)
}
}
for _, set := range namedPorts {
log.Printf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
if err = ipsMgr.CreateSet(set, util.IpsetIPPortHashFlag); err != nil {
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetIPPortHashFlag})); err != nil {
log.Printf("Error creating ipset named port %s", set)
}
}
Expand All @@ -114,6 +118,8 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
if err = npMgr.InitAllNsList(); err != nil {
log.Printf("Error initializing all-namespace ipset list.")
}
createCidrsRule("in", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, ingressIPCidrs, ipsMgr)
createCidrsRule("out", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, egressIPCidrs, ipsMgr)
iptMgr := allNs.iptMgr
for _, iptEntry := range iptEntries {
if err = iptMgr.Add(iptEntry); err != nil {
Expand Down Expand Up @@ -154,7 +160,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
npMgr.nsMap[npNs] = ns
}

_, _, _, iptEntries := translatePolicy(npObj)
_, _, _, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(npObj)

iptMgr := allNs.iptMgr
for _, iptEntry := range iptEntries {
Expand All @@ -163,6 +169,9 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
}
}

removeCidrsRule("in", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, ingressIPCidrs, allNs.ipsMgr)
removeCidrsRule("out", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, egressIPCidrs, allNs.ipsMgr)

delete(ns.rawNpMap, npObj.ObjectMeta.Name)

hashedSelector := HashSelector(&npObj.Spec.PodSelector)
Expand All @@ -189,3 +198,35 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo

return nil
}

func createCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]string, ipsMgr *ipsm.IpsetManager) {
spec := append([]string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum})
for i, ipCidrSet := range ipsetEntries {
if ipCidrSet == nil || len(ipCidrSet) == 0 {
Copy link
Contributor

@jaer-tsun jaer-tsun Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translatepolicy should already remove empty sets

Copy link
Contributor Author

@csfmomo csfmomo Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it didn't.
For the existing return object, including sets, namedPorts, lists, they are []string type and called UniqueStrSlice to remove empty set. However, for ipsetEntries, it's [][]string type and didn't do drop empty entries before returning. I intentionally didn't do that to keep index order for simplicity for deleting ipset when network policy rule updates.

continue
}
setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress
log.Printf("Creating set: %v, hashedSet: %v", setName, util.GetHashedName(setName))
if err := ipsMgr.CreateSet(setName, spec); err != nil {
log.Printf("Error creating ipset %s", ipCidrSet)
}
for _, ipCidrEntry := range util.DropEmptyFields(ipCidrSet) {
if err := ipsMgr.AddToSet(setName, ipCidrEntry, util.IpsetNetHashFlag); err != nil {
log.Printf("Error adding ip cidrs %s into ipset %s", ipCidrEntry, ipCidrSet)
}
}
}
}

func removeCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]string, ipsMgr *ipsm.IpsetManager) {
for i, ipCidrSet := range ipsetEntries {
if ipCidrSet == nil || len(ipCidrSet) == 0 {
Copy link
Contributor

@jaer-tsun jaer-tsun Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this func will work as intended since the length of the set includes all the entries to be deleted

Copy link
Contributor Author

@csfmomo csfmomo Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I intend to do here is, delete ipset npm created when translate cidrs rule. Length of the set includes cidrs belong to this rule.
It works as intended. I test it by updating same network policy rules. It will remove and update the target ipset.
image

continue
}
setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress
log.Printf("Delete set: %v, hashedSet: %v", setName, util.GetHashedName(setName))
if err := ipsMgr.DeleteSet(setName); err != nil {
log.Printf("Error deleting ipset %s", ipCidrSet)
}
}
}
6 changes: 3 additions & 3 deletions npm/nwpolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestAddNetworkPolicy(t *testing.T) {
}

// Create ns-kube-system set
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestAddNetworkPolicy failed @ ipsMgr.CreateSet, adding kube-system set%+v", err)
}

Expand Down Expand Up @@ -161,7 +161,7 @@ func TestUpdateNetworkPolicy(t *testing.T) {
}()

// Create ns-kube-system set
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestUpdateNetworkPolicy failed @ ipsMgr.CreateSet, adding kube-system set%+v", err)
}

Expand Down Expand Up @@ -273,7 +273,7 @@ func TestDeleteNetworkPolicy(t *testing.T) {
}()

// Create ns-kube-system set
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, util.IpsetNetHashFlag); err != nil {
if err := ipsMgr.CreateSet("ns-"+util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil {
t.Errorf("TestDeleteNetworkPolicy failed @ ipsMgr.CreateSet, adding kube-system set%+v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion npm/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {
// Add pod namespace if it doesn't exist
if _, exists := npMgr.nsMap[podNs]; !exists {
log.Printf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs))
if err = ipsMgr.CreateSet(podNs, util.IpsetNetHashFlag); err != nil {
if err = ipsMgr.CreateSet(podNs, append([]string{util.IpsetNetHashFlag})); err != nil {
log.Printf("Error creating ipset %s", podNs)
}
}
Expand Down
2 changes: 2 additions & 0 deletions npm/testpolicies/complex-policy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ spec:
- to:
- ipBlock:
cidr: 10.0.0.0/24
except:
- 10.0.0.1/32
ports:
- protocol: TCP
port: 5978
Loading