From 7a4dbf84b6b1fd362deb0a130de495d9fd042538 Mon Sep 17 00:00:00 2001 From: vakr Date: Fri, 27 Aug 2021 14:49:14 -0700 Subject: [PATCH 01/26] adding initial dataplane interface --- npm/pkg/dataplane/dataplane.go | 86 +++++++++++++ npm/pkg/dataplane/ipsets/ipset.go | 127 ++++++++++++++++++++ npm/pkg/dataplane/ipsets/ipsetmanager.go | 51 ++++++++ npm/pkg/dataplane/policies/policy.go | 52 ++++++++ npm/pkg/dataplane/policies/policymanager.go | 31 +++++ 5 files changed, 347 insertions(+) create mode 100644 npm/pkg/dataplane/dataplane.go create mode 100644 npm/pkg/dataplane/ipsets/ipset.go create mode 100644 npm/pkg/dataplane/ipsets/ipsetmanager.go create mode 100644 npm/pkg/dataplane/policies/policy.go create mode 100644 npm/pkg/dataplane/policies/policymanager.go diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go new file mode 100644 index 0000000000..37f8fc13b9 --- /dev/null +++ b/npm/pkg/dataplane/dataplane.go @@ -0,0 +1,86 @@ +package dataplane + +import ( + "runtime" + + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" +) + +type DataPlane struct { + policies.PolicyManager + ipsets.IPSetManager + DataPlaneInterface + OsType OsType + networkID string + // key is PodKey + endpointCache map[string]interface{} +} + +func NewDataPlane() *DataPlane { + return &DataPlane{ + OsType: detectOsType(), + PolicyManager: policies.NewPolicyManager(), + IPSetManager: ipsets.NewIPSetManager(string(detectOsType())), + } +} + +type OsType string + +const ( + Windows OsType = "windows" + Linux OsType = "linux" +) + +type DataPlaneInterface interface { + NewDataplane() (*DataPlane, error) + + InitializeDataplane() error + ResetDataplane() error + + // ACLPolicy related functions + // Add Policy takes in the custom NPMNetworkPolicy object + // and adds it to the dataplane + AddPolicies(policies *policies.NPMNetworkPolicy) error + // Delete Policy takes in name of the policy, looks up cache for policy obj + // and deletes it from the dataplane + RemovePolicies(policyName string) error + // Update Policy takes in the custom NPMNetworkPolicy object + // calculates the diff between the old and new policy + // and updates the dataplane + UpdatePolicies(policies *policies.NPMNetworkPolicy) error + + // IPSet related functions + CreateIPSet(Set *ipsets.IPSet) error + DeleteSet(name string) error + DeleteList(name string) error + + AddToSet(setNames []string, ip, podKey string) error + RemoveFromSet(setNames []string, ip, podkey string) error + AddToList(listName string, setNames []string) error + RemoveFromList(listName string, setNames []string) error + + // UpdatePod helps in letting DP know about a new pod + // this function will have two responsibilities, + // 1. proactively get endpoint info of pod + // 2. check if any of the existing policies applies to this pod + // and update ACLs on this pod's endpoint + UpdatePod(pod interface{}) error + + // Called after all the ipsets operations are done + // this call acts as a signal to the dataplane to update the kernel + ApplyDataplane() error +} + +// Detects the OS type +func detectOsType() OsType { + os := runtime.GOOS + switch os { + case "linux": + return Linux + case "windows": + return Windows + default: + panic("Unsupported OS type: " + os) + } +} diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go new file mode 100644 index 0000000000..13789e8a7c --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -0,0 +1,127 @@ +package ipsets + +import ( + "fmt" + + "github.com/Azure/azure-container-networking/npm/util" +) + +type IPSet struct { + Name string + HashedName string + Type SetType + // IpPodKey is used for setMaps to store Ips and ports as keys + // and podKey as value + IpPodKey map[string]string + // This is used for listMaps to store child IP Sets + MemberIPSets map[string]*IPSet + NetPolReferCount int + IpsetReferCount int +} + +type SetType int32 + +const ( + Unknown SetType = 0 + NameSpace SetType = 1 + KeyLabelOfNameSpace SetType = 2 + KeyValueLabelOfNameSpace SetType = 3 + KeyLabelOfPod SetType = 4 + KeyValueLabelOfPod SetType = 5 + NamedPorts SetType = 6 + NestedLabelOfPod SetType = 7 + CIDRBlocks SetType = 8 +) + +var SetType_name = map[int32]string{ + 0: "Unknown", + 1: "NameSpace", + 2: "KeyLabelOfNameSpace", + 3: "KeyValueLabelOfNameSpace", + 4: "KeyLabelOfPod", + 5: "KeyValueLabelOfPod", + 6: "NamedPorts", + 7: "NestedLabelOfPod", + 8: "CIDRBlocks", +} + +var SetType_value = map[string]int32{ + "Unknown": 0, + "NameSpace": 1, + "KeyLabelOfNameSpace": 2, + "KeyValueLabelOfNameSpace": 3, + "KeyLabelOfPod": 4, + "KeyValueLabelOfPod": 5, + "NamedPorts": 6, + "NestedLabelOfPod": 7, + "CIDRBlocks": 8, +} + +func (x SetType) String() string { + return SetType_name[int32(x)] +} + +func GetSetType(x string) SetType { + return SetType(SetType_value[x]) +} + +type SetKind string + +const ( + ListSet SetKind = "list" + HashSet SetKind = "set" +) + +func NewIPSet(name string, setType SetType) *IPSet { + return &IPSet{ + Name: name, + HashedName: util.GetHashedName(name), + IpPodKey: make(map[string]string), + Type: setType, + MemberIPSets: make(map[string]*IPSet, 0), + NetPolReferCount: 0, + IpsetReferCount: 0, + } +} + +func GetSetContents(set *IPSet) ([]string, error) { + contents := make([]string, 0) + setType := getSetKind(set) + switch setType { + case HashSet: + for podIp := range set.IpPodKey { + contents = append(contents, podIp) + } + return contents, nil + case ListSet: + for _, memberSet := range set.MemberIPSets { + contents = append(contents, memberSet.HashedName) + } + return contents, nil + default: + return contents, fmt.Errorf("Unknown set type %s", setType) + } +} + +func getSetKind(set *IPSet) SetKind { + switch set.Type { + case CIDRBlocks: + return HashSet + case NameSpace: + return HashSet + case NamedPorts: + return HashSet + case KeyLabelOfPod: + return HashSet + case KeyValueLabelOfPod: + return HashSet + case KeyLabelOfNameSpace: + return ListSet + case KeyValueLabelOfNameSpace: + return ListSet + case NestedLabelOfPod: + return ListSet + default: + return "unknown" + } +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go new file mode 100644 index 0000000000..73624c4f66 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -0,0 +1,51 @@ +package ipsets + +import ( + "sync" + + "github.com/Azure/azure-container-networking/npm/util/errors" +) + +type IPSetMap struct { + cache map[string]*IPSet + sync.Mutex +} +type IPSetManager struct { + listMap *IPSetMap + setMap *IPSetMap +} + +func newIPSetMap() *IPSetMap { + return &IPSetMap{ + cache: make(map[string]*IPSet), + } +} + +func (m *IPSetMap) exists(name string) bool { + m.Lock() + defer m.Unlock() + _, ok := m.cache[name] + return ok +} + +func NewIPSetManager(os string) IPSetManager { + return IPSetManager{ + listMap: newIPSetMap(), + setMap: newIPSetMap(), + } +} + +func (iMgr *IPSetManager) getSetCache(set *IPSet) (*IPSetMap, error) { + kind := getSetKind(set) + + var m *IPSetMap + switch kind { + case ListSet: + m = iMgr.listMap + case HashSet: + m = iMgr.setMap + default: + return nil, errors.Errorf(errors.CreateIPSet, false, "unknown Set kind") + } + return m, nil +} diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go new file mode 100644 index 0000000000..9000d42b77 --- /dev/null +++ b/npm/pkg/dataplane/policies/policy.go @@ -0,0 +1,52 @@ +package policies + +import ( + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + networkingv1 "k8s.io/api/networking/v1" +) + +type NPMNetworkPolicy struct { + Name string + PodSelectorIPSets []*ipsets.IPSet + OtherIPSets []*ipsets.IPSet + ACLs []*AclPolicy + // Making this a podKey instead should be + // use NPMPod obj + Pods []string + RawNP *networkingv1.NetworkPolicy +} + +type AclPolicy struct { // Iptable rules + PolicyID string + Comment string + SrcList []SetInfo + DstList []SetInfo + Target Verdict + Direction Direction + SrcPorts []Ports + DstPorts []Ports + Protocol string +} + +type SetInfo struct { + IpSet *ipsets.IPSet + Included bool + MatchType string // match type can be “src”, “src,dst” or “dst,dst” etc +} + +type Ports struct { + Port int64 + EndPort int64 +} + +type Verdict string +type Direction string + +const ( + Ingress Direction = "IN" + Egress Direction = "OUT" + Both Direction = "BOTH" + + Allowed Verdict = "ALLOW" + Dropped Verdict = "DROP" +) diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go new file mode 100644 index 0000000000..6cfbfe0959 --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -0,0 +1,31 @@ +package policies + +import "sync" + +type PolicyMap struct { + sync.Mutex + cache map[string]*NPMNetworkPolicy +} + +type PolicyManager struct { + policyMap *PolicyMap +} + +func NewPolicyManager() PolicyManager { + return PolicyManager{ + policyMap: &PolicyMap{ + cache: make(map[string]*NPMNetworkPolicy), + }, + } +} + +func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { + pMgr.policyMap.Lock() + defer pMgr.policyMap.Unlock() + + if policy, ok := pMgr.policyMap.cache[name]; ok { + return policy, nil + } + + return nil, nil +} From 4385d469b62205b8adcd92de7fe164ae18c465a0 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 30 Aug 2021 11:55:59 -0700 Subject: [PATCH 02/26] adding some skeleton code for crud functions --- npm/pkg/dataplane/dataplane.go | 60 ++++- npm/pkg/dataplane/dataplane_test.go | 23 ++ npm/pkg/dataplane/ipsets/ipset.go | 12 + npm/pkg/dataplane/ipsets/ipsetmanager.go | 255 +++++++++++++++++++- npm/pkg/dataplane/policies/policymanager.go | 27 +++ 5 files changed, 365 insertions(+), 12 deletions(-) create mode 100644 npm/pkg/dataplane/dataplane_test.go diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 37f8fc13b9..be40d8eb7a 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -8,9 +8,9 @@ import ( ) type DataPlane struct { - policies.PolicyManager - ipsets.IPSetManager DataPlaneInterface + policyMgr policies.PolicyManager + ipsetMgr ipsets.IPSetManager OsType OsType networkID string // key is PodKey @@ -19,9 +19,9 @@ type DataPlane struct { func NewDataPlane() *DataPlane { return &DataPlane{ - OsType: detectOsType(), - PolicyManager: policies.NewPolicyManager(), - IPSetManager: ipsets.NewIPSetManager(string(detectOsType())), + OsType: detectOsType(), + policyMgr: policies.NewPolicyManager(), + ipsetMgr: ipsets.NewIPSetManager(string(detectOsType())), } } @@ -55,7 +55,7 @@ type DataPlaneInterface interface { DeleteSet(name string) error DeleteList(name string) error - AddToSet(setNames []string, ip, podKey string) error + AddToSet(setNames []*ipsets.IPSet, ip, podKey string) error RemoveFromSet(setNames []string, ip, podkey string) error AddToList(listName string, setNames []string) error RemoveFromList(listName string, setNames []string) error @@ -84,3 +84,51 @@ func detectOsType() OsType { panic("Unsupported OS type: " + os) } } + +func (dp *DataPlane) CreateIPSet(Set *ipsets.IPSet) error { + return dp.ipsetMgr.CreateIPSet(Set) +} + +func (dp *DataPlane) DeleteSet(name string) error { + return dp.ipsetMgr.DeleteSet(name) +} + +func (dp *DataPlane) DeleteList(name string) error { + return dp.ipsetMgr.DeleteList(name) +} + +func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSet, ip, podKey string) error { + return dp.ipsetMgr.AddToSet(setNames, ip, podKey) +} + +func (dp *DataPlane) RemoveFromSet(setNames []string, ip, podKey string) error { + return dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) +} + +func (dp *DataPlane) AddToList(listName string, setNames []string) error { + return dp.ipsetMgr.AddToList(listName, setNames) +} + +func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { + return dp.ipsetMgr.RemoveFromList(listName, setNames) +} + +func (dp *DataPlane) UpdatePod(pod interface{}) error { + return nil +} + +func (dp *DataPlane) ApplyDataplane() error { + return nil +} + +func (dp *DataPlane) AddPolicies(policies *policies.NPMNetworkPolicy) error { + return dp.policyMgr.AddPolicies(policies) +} + +func (dp *DataPlane) RemovePolicies(policyName string) error { + return dp.policyMgr.RemovePolicies(policyName) +} + +func (dp *DataPlane) UpdatePolicies(policies *policies.NPMNetworkPolicy) error { + return dp.policyMgr.UpdatePolicies(policies) +} diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go new file mode 100644 index 0000000000..86dc913823 --- /dev/null +++ b/npm/pkg/dataplane/dataplane_test.go @@ -0,0 +1,23 @@ +package dataplane + +import ( + "testing" + + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" +) + +func TestNewDataPlane(t *testing.T) { + + dp := NewDataPlane() + + if dp == nil { + t.Error("NewDataPlane() returned nil") + } + set := ipsets.NewIPSet("test", ipsets.NameSpace) + + err := dp.CreateIPSet(set) + if err != nil { + t.Error("CreateIPSet() returned error") + } + +} diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 13789e8a7c..cbca9645c1 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -125,3 +125,15 @@ func getSetKind(set *IPSet) SetKind { return "unknown" } } + +func (set *IPSet) AddMemberIPSet(memberIPSet *IPSet) { + set.MemberIPSets[memberIPSet.Name] = memberIPSet +} + +func (set *IPSet) IncIpsetReferCount() { + set.IpsetReferCount++ +} + +func (set *IPSet) DecIpsetReferCount() { + set.IpsetReferCount-- +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 73624c4f66..9244e04557 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -1,8 +1,12 @@ package ipsets import ( + "fmt" + "net" "sync" + "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util/errors" ) @@ -11,8 +15,9 @@ type IPSetMap struct { sync.Mutex } type IPSetManager struct { - listMap *IPSetMap - setMap *IPSetMap + listMap *IPSetMap + setMap *IPSetMap + dirtyCaches *IPSetMap } func newIPSetMap() *IPSetMap { @@ -22,16 +27,15 @@ func newIPSetMap() *IPSetMap { } func (m *IPSetMap) exists(name string) bool { - m.Lock() - defer m.Unlock() _, ok := m.cache[name] return ok } func NewIPSetManager(os string) IPSetManager { return IPSetManager{ - listMap: newIPSetMap(), - setMap: newIPSetMap(), + listMap: newIPSetMap(), + setMap: newIPSetMap(), + dirtyCaches: newIPSetMap(), } } @@ -49,3 +53,242 @@ func (iMgr *IPSetManager) getSetCache(set *IPSet) (*IPSetMap, error) { } return m, nil } + +func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { + m, err := iMgr.getSetCache(set) + if err != nil { + return err + } + + m.Lock() + defer m.Unlock() + // Check if the Set already exists + if m.exists(set.Name) { + // ipset already exists + // we should calculate a diff if the members are different + return nil + } + + // Call the dataplane specifc fucntion here to + // create the Set + + // append the cache if dataplane specific function + // return nil as error + m.cache[set.Name] = set + + return nil +} + +func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error { + + // check if the IP is IPV$ family + if net.ParseIP(ip).To4() == nil { + return errors.Errorf(errors.AppendIPSet, false, "IPV6 not supported") + } + + for _, updatedSet := range addToSets { + iMgr.setMap.Lock() + defer iMgr.setMap.Unlock() + set, exists := iMgr.setMap.cache[updatedSet.Name] // check if the Set exists + if !exists { + set = NewIPSet(updatedSet.Name, updatedSet.Type) + err := iMgr.CreateIPSet(set) + if err != nil { + return err + } + } + + if getSetKind(set) != HashSet { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a hash set", set.Name)) + } + cachedPodKey, ok := set.IpPodKey[ip] + if ok { + if cachedPodKey != podKey { + log.Logf("AddToSet: PodOwner has changed for Ip: %s, setName:%s, Old podKey: %s, new podKey: %s. Replace context with new PodOwner.", + ip, set.Name, cachedPodKey, podKey) + + set.IpPodKey[ip] = podKey + } + return nil + } + + // Now actually add the IP to the Set + // err := addToSet(setName, ip) + // some more error handling here + + // update the IP ownership with podkey + set.IpPodKey[ip] = podKey + + // Update metrics of the IpSet + metrics.NumIPSetEntries.Inc() + metrics.IncIPSetInventory(set.Name) + } + + return nil +} + +func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey string) error { + iMgr.setMap.Lock() + defer iMgr.setMap.Unlock() + for _, setName := range removeFromSets { + set, exists := iMgr.setMap.cache[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) + } + + if getSetKind(set) != HashSet { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) + } + + // in case the IP belongs to a new Pod, then ignore this Delete call as this might be stale + cachedPodKey := set.IpPodKey[ip] + if cachedPodKey != podKey { + log.Logf("DeleteFromSet: PodOwner has changed for Ip: %s, setName:%s, Old podKey: %s, new podKey: %s. Ignore the delete as this is stale update", + ip, setName, cachedPodKey, podKey) + + return nil + } + + // Now actually delete the IP from the Set + // err := deleteFromSet(setName, ip) + // some more error handling here + + // update the IP ownership with podkey + delete(set.IpPodKey, ip) + + // Update metrics of the IpSet + metrics.NumIPSetEntries.Dec() + metrics.DecIPSetInventory(setName) + } + + return nil +} + +func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { + + for _, setName := range setNames { + + if listName == setName { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("list %s cannot be added to itself", listName)) + } + + iMgr.listMap.Lock() + defer iMgr.listMap.Unlock() + set, exists := iMgr.setMap.cache[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) + } + + // Nested IPSets are only supported for windows + // Check if we want to actually use that support + if getSetKind(set) != HashSet { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) + } + + list, exists := iMgr.listMap.cache[listName] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) + } + + if getSetKind(list) != ListSet { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) + } + + // check if Set is a member of List + listSet, exists := list.MemberIPSets[setName] + if exists { + if listSet == set { + // Set is already a member of List + return nil + } + // Update the ipset in list + list.MemberIPSets[setName] = set + return nil + } + + // Now actually add the Set to the List + // err := addToList(listName, setName) + // some more error handling here + + // update the Ipset member list of list + list.AddMemberIPSet(set) + set.IncIpsetReferCount() + + // Update metrics of the IpSet + metrics.NumIPSetEntries.Inc() + metrics.IncIPSetInventory(setName) + } + + return nil +} + +func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) error { + iMgr.listMap.Lock() + defer iMgr.listMap.Unlock() + for _, setName := range setNames { + set, exists := iMgr.setMap.cache[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) + } + + if getSetKind(set) != HashSet { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) + } + + // Nested IPSets are only supported for windows + //Check if we want to actually use that support + if getSetKind(set) != HashSet { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) + } + + list, exists := iMgr.listMap.cache[listName] // check if the Set exists + if !exists { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) + } + + if getSetKind(list) != ListSet { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) + } + + // check if Set is a member of List + _, exists = list.MemberIPSets[setName] + if !exists { + return nil + } + + // Now actually delete the Set from the List + // err := deleteFromList(listName, setName) + // some more error handling here + + // delete IPSet from the list + delete(list.MemberIPSets, setName) + set.DecIpsetReferCount() + + // Update metrics of the IpSet + metrics.NumIPSetEntries.Dec() + metrics.DecIPSetInventory(setName) + } + + return nil +} + +func (iMgr *IPSetManager) DeleteList(name string) error { + iMgr.listMap.Lock() + defer iMgr.listMap.Unlock() + delete(iMgr.listMap.cache, name) + + return nil +} + +func (iMgr *IPSetManager) DeleteSet(name string) error { + iMgr.setMap.Lock() + defer iMgr.setMap.Unlock() + delete(iMgr.setMap.cache, name) + + return nil +} + +func (iMgr *IPSetManager) ApplyIPSets() error { + + return nil +} diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 6cfbfe0959..54a5241cee 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -29,3 +29,30 @@ func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { return nil, nil } + +func (pMgr *PolicyManager) AddPolicies(policy *NPMNetworkPolicy) error { + pMgr.policyMap.Lock() + defer pMgr.policyMap.Unlock() + + pMgr.policyMap.cache[policy.Name] = policy + + return nil +} + +func (pMgr *PolicyManager) RemovePolicies(name string) error { + pMgr.policyMap.Lock() + defer pMgr.policyMap.Unlock() + + delete(pMgr.policyMap.cache, name) + + return nil +} + +func (pMgr *PolicyManager) UpdatePolicies(policy *NPMNetworkPolicy) error { + pMgr.policyMap.Lock() + defer pMgr.policyMap.Unlock() + + // check and update + + return nil +} From 013aa672b0097df90d55bd5f35d2783689a9e594 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 30 Aug 2021 14:30:24 -0700 Subject: [PATCH 03/26] adding dirty cache initial support for ipset manager --- npm/pkg/dataplane/ipsets/ipset.go | 58 +++++++-- npm/pkg/dataplane/ipsets/ipsetmanager.go | 147 ++++++++++++++--------- 2 files changed, 135 insertions(+), 70 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index cbca9645c1..2e3797985a 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -14,9 +14,11 @@ type IPSet struct { // and podKey as value IpPodKey map[string]string // This is used for listMaps to store child IP Sets - MemberIPSets map[string]*IPSet - NetPolReferCount int - IpsetReferCount int + MemberIPSets map[string]*IPSet + // Using a map here to emulate set of netpol names + SelectorReference map[string]struct{} + NetPolReference map[string]struct{} + IpsetReferCount int } type SetType int32 @@ -74,13 +76,14 @@ const ( func NewIPSet(name string, setType SetType) *IPSet { return &IPSet{ - Name: name, - HashedName: util.GetHashedName(name), - IpPodKey: make(map[string]string), - Type: setType, - MemberIPSets: make(map[string]*IPSet, 0), - NetPolReferCount: 0, - IpsetReferCount: 0, + Name: name, + HashedName: util.GetHashedName(name), + IpPodKey: make(map[string]string), + Type: setType, + MemberIPSets: make(map[string]*IPSet), + SelectorReference: make(map[string]struct{}), + NetPolReference: make(map[string]struct{}), + IpsetReferCount: 0, } } @@ -137,3 +140,38 @@ func (set *IPSet) IncIpsetReferCount() { func (set *IPSet) DecIpsetReferCount() { set.IpsetReferCount-- } + +func (set *IPSet) AddSelectorReference(netPolName string) { + set.SelectorReference[netPolName] = struct{}{} +} + +func (set *IPSet) DeleteSelectorReference(netPolName string) { + delete(set.SelectorReference, netPolName) +} + +func (set *IPSet) AddNetPolReference(netPolName string) { + set.NetPolReference[netPolName] = struct{}{} +} + +func (set *IPSet) DeleteNetPolReference(netPolName string) { + delete(set.NetPolReference, netPolName) +} + +func (set *IPSet) CanBeDeleted() bool { + if len(set.SelectorReference) > 0 { + return false + } + if len(set.NetPolReference) > 0 { + return false + } + if set.IpsetReferCount > 0 { + return false + } + if len(set.MemberIPSets) > 0 { + return false + } + if len(set.IpPodKey) > 0 { + return false + } + return true +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 9244e04557..63384488bb 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -10,60 +10,55 @@ import ( "github.com/Azure/azure-container-networking/npm/util/errors" ) -type IPSetMap struct { - cache map[string]*IPSet - sync.Mutex -} type IPSetManager struct { - listMap *IPSetMap - setMap *IPSetMap - dirtyCaches *IPSetMap -} - -func newIPSetMap() *IPSetMap { - return &IPSetMap{ - cache: make(map[string]*IPSet), - } + setMap map[string]*IPSet + dirtyCaches map[string]struct{} + sync.Mutex } -func (m *IPSetMap) exists(name string) bool { - _, ok := m.cache[name] +func (iMgr *IPSetManager) exists(name string) bool { + _, ok := iMgr.setMap[name] return ok } func NewIPSetManager(os string) IPSetManager { return IPSetManager{ - listMap: newIPSetMap(), - setMap: newIPSetMap(), - dirtyCaches: newIPSetMap(), + setMap: make(map[string]*IPSet), + dirtyCaches: make(map[string]struct{}), } } -func (iMgr *IPSetManager) getSetCache(set *IPSet) (*IPSetMap, error) { - kind := getSetKind(set) - - var m *IPSetMap - switch kind { - case ListSet: - m = iMgr.listMap - case HashSet: - m = iMgr.setMap - default: - return nil, errors.Errorf(errors.CreateIPSet, false, "unknown Set kind") +func (iMgr *IPSetManager) updateDirtyCache(setName string) error { + set, exists := iMgr.setMap[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", set.Name)) } - return m, nil -} -func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { - m, err := iMgr.getSetCache(set) - if err != nil { - return err + // If set is not referenced in netpol then ignore the update + if len(set.NetPolReference) == 0 && len(set.SelectorReference) == 0 { + return nil + } + + iMgr.dirtyCaches[set.Name] = struct{}{} + if getSetKind(set) == ListSet { + // TODO check if we will need to add all the member ipsets + // also to the dirty cache list + for _, member := range set.MemberIPSets { + iMgr.dirtyCaches[member.Name] = struct{}{} + } } + return nil +} + +func (iMgr *IPSetManager) clearDirtyCache() { + iMgr.dirtyCaches = make(map[string]struct{}) +} - m.Lock() - defer m.Unlock() +func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { + iMgr.Lock() + defer iMgr.Unlock() // Check if the Set already exists - if m.exists(set.Name) { + if iMgr.exists(set.Name) { // ipset already exists // we should calculate a diff if the members are different return nil @@ -74,7 +69,7 @@ func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { // append the cache if dataplane specific function // return nil as error - m.cache[set.Name] = set + iMgr.setMap[set.Name] = set return nil } @@ -85,11 +80,11 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error if net.ParseIP(ip).To4() == nil { return errors.Errorf(errors.AppendIPSet, false, "IPV6 not supported") } + iMgr.Lock() + defer iMgr.Unlock() for _, updatedSet := range addToSets { - iMgr.setMap.Lock() - defer iMgr.setMap.Unlock() - set, exists := iMgr.setMap.cache[updatedSet.Name] // check if the Set exists + set, exists := iMgr.setMap[updatedSet.Name] // check if the Set exists if !exists { set = NewIPSet(updatedSet.Name, updatedSet.Type) err := iMgr.CreateIPSet(set) @@ -118,6 +113,7 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error // update the IP ownership with podkey set.IpPodKey[ip] = podKey + iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet metrics.NumIPSetEntries.Inc() @@ -128,10 +124,10 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error } func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey string) error { - iMgr.setMap.Lock() - defer iMgr.setMap.Unlock() + iMgr.Lock() + defer iMgr.Unlock() for _, setName := range removeFromSets { - set, exists := iMgr.setMap.cache[setName] // check if the Set exists + set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } @@ -155,6 +151,7 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri // update the IP ownership with podkey delete(set.IpPodKey, ip) + iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet metrics.NumIPSetEntries.Dec() @@ -166,15 +163,15 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { + iMgr.Lock() + defer iMgr.Unlock() + for _, setName := range setNames { if listName == setName { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("list %s cannot be added to itself", listName)) } - - iMgr.listMap.Lock() - defer iMgr.listMap.Unlock() - set, exists := iMgr.setMap.cache[setName] // check if the Set exists + set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) } @@ -185,7 +182,7 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } - list, exists := iMgr.listMap.cache[listName] // check if the Set exists + list, exists := iMgr.setMap[listName] // check if the Set exists if !exists { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } @@ -219,14 +216,16 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { metrics.IncIPSetInventory(setName) } + iMgr.updateDirtyCache(listName) + return nil } func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) error { - iMgr.listMap.Lock() - defer iMgr.listMap.Unlock() + iMgr.Lock() + defer iMgr.Unlock() for _, setName := range setNames { - set, exists := iMgr.setMap.cache[setName] // check if the Set exists + set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } @@ -241,7 +240,7 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } - list, exists := iMgr.listMap.cache[listName] // check if the Set exists + list, exists := iMgr.setMap[listName] // check if the Set exists if !exists { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } @@ -268,27 +267,55 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err metrics.NumIPSetEntries.Dec() metrics.DecIPSetInventory(setName) } + iMgr.updateDirtyCache(listName) return nil } func (iMgr *IPSetManager) DeleteList(name string) error { - iMgr.listMap.Lock() - defer iMgr.listMap.Unlock() - delete(iMgr.listMap.cache, name) + iMgr.Lock() + defer iMgr.Unlock() + set, exists := iMgr.setMap[name] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) + } + + if !set.CanBeDeleted() { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) + } + delete(iMgr.setMap, name) return nil } func (iMgr *IPSetManager) DeleteSet(name string) error { - iMgr.setMap.Lock() - defer iMgr.setMap.Unlock() - delete(iMgr.setMap.cache, name) + iMgr.Lock() + defer iMgr.Unlock() + set, exists := iMgr.setMap[name] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) + } + if !set.CanBeDeleted() { + return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) + } + delete(iMgr.setMap, name) return nil } func (iMgr *IPSetManager) ApplyIPSets() error { + iMgr.Lock() + defer iMgr.Unlock() + for setName := range iMgr.dirtyCaches { + set, exists := iMgr.setMap[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) + } + + fmt.Printf(set.Name) + + } + iMgr.clearDirtyCache() return nil } From 3e32783bcbc4f6547cc65946ac8b172407d0b705 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 30 Aug 2021 14:40:04 -0700 Subject: [PATCH 04/26] correcting some go lints --- npm/pkg/dataplane/dataplane.go | 4 ++-- npm/pkg/dataplane/dataplane_test.go | 1 - npm/pkg/dataplane/ipsets/ipset.go | 18 +++++++++--------- npm/pkg/dataplane/ipsets/ipsetmanager.go | 20 +++++++++----------- npm/pkg/dataplane/policies/policy.go | 6 +++--- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index be40d8eb7a..eb6673cecf 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -85,8 +85,8 @@ func detectOsType() OsType { } } -func (dp *DataPlane) CreateIPSet(Set *ipsets.IPSet) error { - return dp.ipsetMgr.CreateIPSet(Set) +func (dp *DataPlane) CreateIPSet(set *ipsets.IPSet) error { + return dp.ipsetMgr.CreateIPSet(set) } func (dp *DataPlane) DeleteSet(name string) error { diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 86dc913823..24483e4e99 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -7,7 +7,6 @@ import ( ) func TestNewDataPlane(t *testing.T) { - dp := NewDataPlane() if dp == nil { diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 2e3797985a..2105786a56 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -12,7 +12,7 @@ type IPSet struct { Type SetType // IpPodKey is used for setMaps to store Ips and ports as keys // and podKey as value - IpPodKey map[string]string + IPPodKey map[string]string // This is used for listMaps to store child IP Sets MemberIPSets map[string]*IPSet // Using a map here to emulate set of netpol names @@ -35,7 +35,7 @@ const ( CIDRBlocks SetType = 8 ) -var SetType_name = map[int32]string{ +var SetTypeName = map[int32]string{ 0: "Unknown", 1: "NameSpace", 2: "KeyLabelOfNameSpace", @@ -47,7 +47,7 @@ var SetType_name = map[int32]string{ 8: "CIDRBlocks", } -var SetType_value = map[string]int32{ +var SetTypeValue = map[string]int32{ "Unknown": 0, "NameSpace": 1, "KeyLabelOfNameSpace": 2, @@ -60,11 +60,11 @@ var SetType_value = map[string]int32{ } func (x SetType) String() string { - return SetType_name[int32(x)] + return SetTypeName[int32(x)] } func GetSetType(x string) SetType { - return SetType(SetType_value[x]) + return SetType(SetTypeValue[x]) } type SetKind string @@ -78,7 +78,7 @@ func NewIPSet(name string, setType SetType) *IPSet { return &IPSet{ Name: name, HashedName: util.GetHashedName(name), - IpPodKey: make(map[string]string), + IPPodKey: make(map[string]string), Type: setType, MemberIPSets: make(map[string]*IPSet), SelectorReference: make(map[string]struct{}), @@ -92,8 +92,8 @@ func GetSetContents(set *IPSet) ([]string, error) { setType := getSetKind(set) switch setType { case HashSet: - for podIp := range set.IpPodKey { - contents = append(contents, podIp) + for podIP := range set.IPPodKey { + contents = append(contents, podIP) } return contents, nil case ListSet: @@ -170,7 +170,7 @@ func (set *IPSet) CanBeDeleted() bool { if len(set.MemberIPSets) > 0 { return false } - if len(set.IpPodKey) > 0 { + if len(set.IPPodKey) > 0 { return false } return true diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 63384488bb..8cd62f3c05 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -28,15 +28,15 @@ func NewIPSetManager(os string) IPSetManager { } } -func (iMgr *IPSetManager) updateDirtyCache(setName string) error { +func (iMgr *IPSetManager) updateDirtyCache(setName string) { set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", set.Name)) + return } // If set is not referenced in netpol then ignore the update if len(set.NetPolReference) == 0 && len(set.SelectorReference) == 0 { - return nil + return } iMgr.dirtyCaches[set.Name] = struct{}{} @@ -47,7 +47,7 @@ func (iMgr *IPSetManager) updateDirtyCache(setName string) error { iMgr.dirtyCaches[member.Name] = struct{}{} } } - return nil + return } func (iMgr *IPSetManager) clearDirtyCache() { @@ -75,7 +75,6 @@ func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { } func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error { - // check if the IP is IPV$ family if net.ParseIP(ip).To4() == nil { return errors.Errorf(errors.AppendIPSet, false, "IPV6 not supported") @@ -96,13 +95,13 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error if getSetKind(set) != HashSet { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a hash set", set.Name)) } - cachedPodKey, ok := set.IpPodKey[ip] + cachedPodKey, ok := set.IPPodKey[ip] if ok { if cachedPodKey != podKey { log.Logf("AddToSet: PodOwner has changed for Ip: %s, setName:%s, Old podKey: %s, new podKey: %s. Replace context with new PodOwner.", ip, set.Name, cachedPodKey, podKey) - set.IpPodKey[ip] = podKey + set.IPPodKey[ip] = podKey } return nil } @@ -112,7 +111,7 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error // some more error handling here // update the IP ownership with podkey - set.IpPodKey[ip] = podKey + set.IPPodKey[ip] = podKey iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet @@ -137,7 +136,7 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri } // in case the IP belongs to a new Pod, then ignore this Delete call as this might be stale - cachedPodKey := set.IpPodKey[ip] + cachedPodKey := set.IPPodKey[ip] if cachedPodKey != podKey { log.Logf("DeleteFromSet: PodOwner has changed for Ip: %s, setName:%s, Old podKey: %s, new podKey: %s. Ignore the delete as this is stale update", ip, setName, cachedPodKey, podKey) @@ -150,7 +149,7 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri // some more error handling here // update the IP ownership with podkey - delete(set.IpPodKey, ip) + delete(set.IPPodKey, ip) iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet @@ -162,7 +161,6 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri } func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { - iMgr.Lock() defer iMgr.Unlock() diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 9000d42b77..ed348dc79e 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -9,14 +9,14 @@ type NPMNetworkPolicy struct { Name string PodSelectorIPSets []*ipsets.IPSet OtherIPSets []*ipsets.IPSet - ACLs []*AclPolicy + ACLs []*ACLPolicy // Making this a podKey instead should be // use NPMPod obj Pods []string RawNP *networkingv1.NetworkPolicy } -type AclPolicy struct { // Iptable rules +type ACLPolicy struct { // Iptable rules PolicyID string Comment string SrcList []SetInfo @@ -29,7 +29,7 @@ type AclPolicy struct { // Iptable rules } type SetInfo struct { - IpSet *ipsets.IPSet + IPSet *ipsets.IPSet Included bool MatchType string // match type can be “src”, “src,dst” or “dst,dst” etc } From a907ea25f27166f0d3810b7596ddf635315ce0aa Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 30 Aug 2021 16:24:04 -0700 Subject: [PATCH 05/26] Adding some skeleton around os specific files --- npm/pkg/dataplane/dataplane.go | 6 +- npm/pkg/dataplane/dataplane_linux.go | 10 ++++ npm/pkg/dataplane/dataplane_windows.go | 22 +++++++ npm/pkg/dataplane/ipsets/ipset.go | 12 ++-- .../dataplane/ipsets/ipsetmanager_linux.go | 1 + .../dataplane/ipsets/ipsetmanager_windows.go | 59 +++++++++++++++++++ 6 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 npm/pkg/dataplane/dataplane_linux.go create mode 100644 npm/pkg/dataplane/dataplane_windows.go create mode 100644 npm/pkg/dataplane/ipsets/ipsetmanager_linux.go create mode 100644 npm/pkg/dataplane/ipsets/ipsetmanager_windows.go diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index eb6673cecf..785b2bb9cd 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -33,8 +33,6 @@ const ( ) type DataPlaneInterface interface { - NewDataplane() (*DataPlane, error) - InitializeDataplane() error ResetDataplane() error @@ -85,6 +83,10 @@ func detectOsType() OsType { } } +func (dp *DataPlane) InitializeDataplane() error { + return dp.initializeDataplane() +} + func (dp *DataPlane) CreateIPSet(set *ipsets.IPSet) error { return dp.ipsetMgr.CreateIPSet(set) } diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go new file mode 100644 index 0000000000..12b707521b --- /dev/null +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -0,0 +1,10 @@ +package dataplane + +import ( + "fmt" +) + +func (dp *DataPlane) initializeDataplane() error { + fmt.Printf("Initializing dataplane for linux") + return nil +} \ No newline at end of file diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go new file mode 100644 index 0000000000..cec34a1a0d --- /dev/null +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -0,0 +1,22 @@ +package dataplane + +import ( + "fmt" + + "github.com/Microsoft/hcsshim/hcn" +) + +const ( + // Windows specific constants + AZURENETWORKNAME = "azure" +) + +func (dp *DataPlane) initializeDataplane() error { + fmt.Printf("Initializing dataplane for windows") + + // Get Network ID + network, err := hcn.GetNetworkByName(AZURENETWORKNAME) + + + return nil +} \ No newline at end of file diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 2105786a56..e2b7ed9d7b 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -75,19 +75,23 @@ const ( ) func NewIPSet(name string, setType SetType) *IPSet { - return &IPSet{ + set := &IPSet{ Name: name, HashedName: util.GetHashedName(name), - IPPodKey: make(map[string]string), Type: setType, - MemberIPSets: make(map[string]*IPSet), SelectorReference: make(map[string]struct{}), NetPolReference: make(map[string]struct{}), IpsetReferCount: 0, } + if getSetKind(set) == HashSet { + set.IPPodKey = make(map[string]string) + } else { + set.MemberIPSets = make(map[string]*IPSet) + } + return set } -func GetSetContents(set *IPSet) ([]string, error) { +func (set *IPSet) GetSetContents() ([]string, error) { contents := make([]string, 0) setType := getSetKind(set) switch setType { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go new file mode 100644 index 0000000000..bcd83858a0 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go @@ -0,0 +1 @@ +package ipsets diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go new file mode 100644 index 0000000000..9253a56456 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -0,0 +1,59 @@ +package ipsets + +// SetPolicyTypes associated with SetPolicy. Value is IPSET. +type SetPolicyType string + +const ( + SetPolicyTypeIpSet SetPolicyType = "IPSET" + SetPolicyTypeNestedIpSet SetPolicyType = "NESTEDIPSET" +) + +// SetPolicySetting creates IPSets on network +type SetPolicySetting struct { + Id string + Name string + Type SetPolicyType + Values string +} + +func isValidIPSet(set *IPSet) error { + if set.Name == "" { + return fmt.Errorf("IPSet " + set.Name + " is missing Name") + } + + if set.Type == "" { + return fmt.Errorf("IPSet " + set.Type + " is missing Type") + } + + if set.HashedName == "" { + return fmt.Errorf("IPSet " + set.HashedName + " is missing HashedName") + } + + return nil +} + +func getSetPolicyType(set *IPSet) SetPolicyType { + setKind := getSetKind(set) + switch setKind { + case ListSet: + return SetPolicyTypeNestedIpSet + case HashSet: + return SetPolicyTypeIpSet + default: + return "Unknown" + } +} + +func convertToSetPolicy(set *IPSet) (*SetPolicySetting, error) { + err := isValidIPSet(set) + if err != nil { + return err + } + + return &SetPolicySetting{ + Id: set.HashedName, + Name: set.Name, + Type: getSetPolicyType(set), + Values: set.GetSetContents(), + } +} From 58d7782d847bef51240c01bb4399f625f8777695 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 31 Aug 2021 12:58:26 -0700 Subject: [PATCH 06/26] removing interface and adding some network logic --- npm/pkg/dataplane/dataplane.go | 67 ++++++------------- npm/pkg/dataplane/dataplane_linux.go | 8 ++- npm/pkg/dataplane/dataplane_windows.go | 28 +++++++- .../dataplane/ipsets/ipsetmanager_windows.go | 22 ++++-- npm/util/const.go | 2 + npm/util/util.go | 14 ++++ 6 files changed, 85 insertions(+), 56 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 785b2bb9cd..a951d65b87 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -3,25 +3,32 @@ package dataplane import ( "runtime" + "github.com/Azure/azure-container-networking/npm" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" ) type DataPlane struct { - DataPlaneInterface policyMgr policies.PolicyManager ipsetMgr ipsets.IPSetManager OsType OsType networkID string // key is PodKey - endpointCache map[string]interface{} + endpointCache map[string]*NPMEndpoint +} + +type NPMEndpoint struct { + Name string + ID string + NetpolRef []string } func NewDataPlane() *DataPlane { return &DataPlane{ - OsType: detectOsType(), - policyMgr: policies.NewPolicyManager(), - ipsetMgr: ipsets.NewIPSetManager(string(detectOsType())), + OsType: detectOsType(), + policyMgr: policies.NewPolicyManager(), + ipsetMgr: ipsets.NewIPSetManager(string(detectOsType())), + endpointCache: make(map[string]*NPMEndpoint), } } @@ -32,44 +39,6 @@ const ( Linux OsType = "linux" ) -type DataPlaneInterface interface { - InitializeDataplane() error - ResetDataplane() error - - // ACLPolicy related functions - // Add Policy takes in the custom NPMNetworkPolicy object - // and adds it to the dataplane - AddPolicies(policies *policies.NPMNetworkPolicy) error - // Delete Policy takes in name of the policy, looks up cache for policy obj - // and deletes it from the dataplane - RemovePolicies(policyName string) error - // Update Policy takes in the custom NPMNetworkPolicy object - // calculates the diff between the old and new policy - // and updates the dataplane - UpdatePolicies(policies *policies.NPMNetworkPolicy) error - - // IPSet related functions - CreateIPSet(Set *ipsets.IPSet) error - DeleteSet(name string) error - DeleteList(name string) error - - AddToSet(setNames []*ipsets.IPSet, ip, podKey string) error - RemoveFromSet(setNames []string, ip, podkey string) error - AddToList(listName string, setNames []string) error - RemoveFromList(listName string, setNames []string) error - - // UpdatePod helps in letting DP know about a new pod - // this function will have two responsibilities, - // 1. proactively get endpoint info of pod - // 2. check if any of the existing policies applies to this pod - // and update ACLs on this pod's endpoint - UpdatePod(pod interface{}) error - - // Called after all the ipsets operations are done - // this call acts as a signal to the dataplane to update the kernel - ApplyDataplane() error -} - // Detects the OS type func detectOsType() OsType { os := runtime.GOOS @@ -83,8 +52,12 @@ func detectOsType() OsType { } } -func (dp *DataPlane) InitializeDataplane() error { - return dp.initializeDataplane() +func (dp *DataPlane) InitializeDataPlane() error { + return dp.initializeDataPlane() +} + +func (dp *DataPlane) ResetDataPlane() error { + return dp.resetDataPlane() } func (dp *DataPlane) CreateIPSet(set *ipsets.IPSet) error { @@ -115,11 +88,11 @@ func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { return dp.ipsetMgr.RemoveFromList(listName, setNames) } -func (dp *DataPlane) UpdatePod(pod interface{}) error { +func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { return nil } -func (dp *DataPlane) ApplyDataplane() error { +func (dp *DataPlane) ApplyDataPlane() error { return nil } diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go index 12b707521b..39711006e8 100644 --- a/npm/pkg/dataplane/dataplane_linux.go +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -4,7 +4,11 @@ import ( "fmt" ) -func (dp *DataPlane) initializeDataplane() error { +func (dp *DataPlane) initializeDataPlane() error { fmt.Printf("Initializing dataplane for linux") return nil -} \ No newline at end of file +} + +func (dp *DataPlane) resetDataPlane() error { + return nil +} diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index cec34a1a0d..387927e8fc 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -11,12 +11,36 @@ const ( AZURENETWORKNAME = "azure" ) -func (dp *DataPlane) initializeDataplane() error { +func (dp *DataPlane) initializeDataPlane() error { fmt.Printf("Initializing dataplane for windows") // Get Network ID network, err := hcn.GetNetworkByName(AZURENETWORKNAME) + if err != nil { + return err + } + dp.networkID = network.Id + endpoints, err := hcn.ListEndpointsOfNetwork(dp.networkID) + if err != nil { + return err + } + + for _, endpoint := range endpoints { + fmt.Println(endpoint.Policies) + ep := &NPMEndpoint{ + Name: endpoint.Name, + ID: endpoint.Id, + NetpolRef: []string{}, + } + + dp.endpointCache[ep.Name] = ep + } + + return nil +} + +func (dp *DataPlane) resetDataPlane() error { return nil -} \ No newline at end of file +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 9253a56456..2147291298 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -1,5 +1,11 @@ package ipsets +import ( + "fmt" + + "github.com/Azure/azure-container-networking/npm/util" +) + // SetPolicyTypes associated with SetPolicy. Value is IPSET. type SetPolicyType string @@ -21,8 +27,8 @@ func isValidIPSet(set *IPSet) error { return fmt.Errorf("IPSet " + set.Name + " is missing Name") } - if set.Type == "" { - return fmt.Errorf("IPSet " + set.Type + " is missing Type") + if set.Type == Unknown { + return fmt.Errorf("IPSet " + set.Type.String() + " is missing Type") } if set.HashedName == "" { @@ -47,13 +53,19 @@ func getSetPolicyType(set *IPSet) SetPolicyType { func convertToSetPolicy(set *IPSet) (*SetPolicySetting, error) { err := isValidIPSet(set) if err != nil { - return err + return nil, err + } + + setContents, err := set.GetSetContents() + if err != nil { + return nil, err } - return &SetPolicySetting{ + setPolicy := &SetPolicySetting{ Id: set.HashedName, Name: set.Name, Type: getSetPolicyType(set), - Values: set.GetSetContents(), + Values: util.SliceToString(setContents), } + return setPolicy, nil } diff --git a/npm/util/const.go b/npm/util/const.go index ff34c34fd6..7310172566 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -145,6 +145,8 @@ const ( NamespacePrefix string = "ns-" NegationPrefix string = "not-" + + SetPolicyDelimiter string = "," ) //NPM telemetry constants. diff --git a/npm/util/util.go b/npm/util/util.go index 09d7b7d0d1..001a073e72 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -341,3 +341,17 @@ func CompareSlices(list1, list2 []string) bool { } return true } + +func SliceToString(list []string) string { + returnStr := "" + + for _, s := range list { + if returnStr != "" { + returnStr = returnStr + SetPolicyDelimiter + s + } else { + returnStr = s + } + } + + return returnStr +} From 5c5403ecd2fa6f8b7d8e8e8d436fee3d0537d5c5 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 1 Sep 2021 13:13:31 -0700 Subject: [PATCH 07/26] adding apply ipsets logic --- npm/pkg/dataplane/dataplane.go | 2 +- npm/pkg/dataplane/ipsets/ipset.go | 10 ++ npm/pkg/dataplane/ipsets/ipsetmanager.go | 15 +-- .../dataplane/ipsets/ipsetmanager_linux.go | 21 ++++ .../dataplane/ipsets/ipsetmanager_windows.go | 117 +++++++++++++++++- 5 files changed, 151 insertions(+), 14 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index a951d65b87..b28faae79a 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -93,7 +93,7 @@ func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { } func (dp *DataPlane) ApplyDataPlane() error { - return nil + return dp.ipsetMgr.ApplyIPSets(dp.networkID) } func (dp *DataPlane) AddPolicies(policies *policies.NPMNetworkPolicy) error { diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index e2b7ed9d7b..590dc25755 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -179,3 +179,13 @@ func (set *IPSet) CanBeDeleted() bool { } return true } + +func (set *IPSet) UsedByNetPol() bool { + if len(set.SelectorReference) <= 0 { + return false + } + if len(set.NetPolReference) <= 0 { + return false + } + return true +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 8cd62f3c05..4337070181 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -301,19 +301,16 @@ func (iMgr *IPSetManager) DeleteSet(name string) error { return nil } -func (iMgr *IPSetManager) ApplyIPSets() error { +func (iMgr *IPSetManager) ApplyIPSets(networkID string) error { iMgr.Lock() defer iMgr.Unlock() - for setName := range iMgr.dirtyCaches { - set, exists := iMgr.setMap[setName] // check if the Set exists - if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) - } - - fmt.Printf(set.Name) - + // Call the appropriate apply ipsets + err := iMgr.applyIPSets(networkID) + if err != nil { + return err } + iMgr.clearDirtyCache() return nil } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go index bcd83858a0..dd508e7e27 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go @@ -1 +1,22 @@ package ipsets + +import ( + "fmt" + + "github.com/Azure/azure-container-networking/npm/util" + "github.com/Azure/azure-container-networking/npm/util/errors" +) + +func (iMgr *IPSetManager) applyIPSets(networkID string) error { + for setName := range iMgr.dirtyCaches { + set, exists := iMgr.setMap[setName] // check if the Set exists + if !exists { + return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) + } + + fmt.Printf(set.Name) + + } + return nil +} + diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 2147291298..cd5b41be7b 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -1,9 +1,13 @@ package ipsets import ( + "encoding/json" "fmt" "github.com/Azure/azure-container-networking/npm/util" + "github.com/Azure/azure-container-networking/npm/util/errors" + "github.com/Microsoft/hcsshim/hcn" + "k8s.io/klog" ) // SetPolicyTypes associated with SetPolicy. Value is IPSET. @@ -22,6 +26,95 @@ type SetPolicySetting struct { Values string } +func (iMgr *IPSetManager) applyIPSets(networkID string) error { + network, err := hcn.GetNetworkByID(networkID) + if err != nil { + return err + } + + setPolNames, err := getAllSetPolicyNames(network.Policies) + if err != nil { + return err + } + + setPolSettings, err := iMgr.calculateNewSetPolicies(setPolNames) + if err != nil { + return err + } + + policyNetworkRequest := hcn.PolicyNetworkRequest{ + Policies: []hcn.NetworkPolicy{}, + } + + for _, policy := range network.Policies { + if policy.Type != "SetPolicy" { + policyNetworkRequest.Policies = append(policyNetworkRequest.Policies, policy) + } + } + + for setPol := range setPolSettings { + rawSettings, err := json.Marshal(setPolSettings[setPol]) + if err != nil { + return err + } + policyNetworkRequest.Policies = append( + policyNetworkRequest.Policies, + hcn.NetworkPolicy{ + Type: "SetPolicy", + Settings: rawSettings, + }, + ) + } + + err = network.AddPolicy(policyNetworkRequest) + if err != nil { + return err + } + + return nil +} + +func (iMgr *IPSetManager) calculateNewSetPolicies(existingSets []string) (map[string]SetPolicySetting, error) { + // some of this below logic can be abstracted a step above + dirtySets := iMgr.dirtyCaches + + for _, setName := range existingSets { + dirtySets[setName] = struct{}{} + } + + setsToUpdate := make(map[string]SetPolicySetting) + for setName := range dirtySets { + set, exists := iMgr.setMap[setName] // check if the Set exists + if !exists { + return nil, errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) + } + if !set.UsedByNetPol() { + continue + } + + setPol, err := convertToSetPolicy(set) + if err != nil { + return nil, err + } + setsToUpdate[setName] = setPol + if getSetKind(set) == ListSet { + for _, memberSet := range set.MemberIPSets { + // TODO check whats the name here, hashed or normal + if _, ok := setsToUpdate[memberSet.Name]; ok { + continue + } + setPol, err = convertToSetPolicy(memberSet) + if err != nil { + return nil, err + } + setsToUpdate[memberSet.Name] = setPol + } + } + } + + return setsToUpdate, nil +} + func isValidIPSet(set *IPSet) error { if set.Name == "" { return fmt.Errorf("IPSet " + set.Name + " is missing Name") @@ -50,18 +143,18 @@ func getSetPolicyType(set *IPSet) SetPolicyType { } } -func convertToSetPolicy(set *IPSet) (*SetPolicySetting, error) { +func convertToSetPolicy(set *IPSet) (SetPolicySetting, error) { err := isValidIPSet(set) if err != nil { - return nil, err + return SetPolicySetting{}, err } setContents, err := set.GetSetContents() if err != nil { - return nil, err + return SetPolicySetting{}, err } - setPolicy := &SetPolicySetting{ + setPolicy := SetPolicySetting{ Id: set.HashedName, Name: set.Name, Type: getSetPolicyType(set), @@ -69,3 +162,19 @@ func convertToSetPolicy(set *IPSet) (*SetPolicySetting, error) { } return setPolicy, nil } + +func getAllSetPolicyNames(networkPolicies []hcn.NetworkPolicy) ([]string, error) { + setPols := []string{} + for _, netpol := range networkPolicies { + if netpol.Type == "SetPolicy" { + var set SetPolicySetting + err := json.Unmarshal(netpol.Settings, &set) + if err != nil { + klog.Error(err.Error()) + continue + } + setPols = append(setPols, set.Name) + } + } + return setPols, nil +} From 6c5afdc7de586869de60071763292e0f39196a9d Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 1 Sep 2021 22:14:17 -0700 Subject: [PATCH 08/26] Addressing some comments and also adding comments to code --- npm/pkg/dataplane/dataplane.go | 63 ++++++++++--------- npm/pkg/dataplane/dataplane_windows.go | 10 +-- npm/pkg/dataplane/ipsets/ipset.go | 26 +++----- npm/pkg/dataplane/ipsets/ipsetmanager.go | 2 +- .../dataplane/ipsets/ipsetmanager_linux.go | 1 - npm/pkg/dataplane/policies/policy.go | 15 ++++- 6 files changed, 62 insertions(+), 55 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index b28faae79a..276a1f9f60 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -1,8 +1,6 @@ package dataplane import ( - "runtime" - "github.com/Azure/azure-container-networking/npm" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" @@ -11,99 +9,108 @@ import ( type DataPlane struct { policyMgr policies.PolicyManager ipsetMgr ipsets.IPSetManager - OsType OsType networkID string // key is PodKey endpointCache map[string]*NPMEndpoint } type NPMEndpoint struct { - Name string - ID string - NetpolRef []string + Name string + ID string + NetPolReference map[string]struct{} } func NewDataPlane() *DataPlane { return &DataPlane{ - OsType: detectOsType(), policyMgr: policies.NewPolicyManager(), - ipsetMgr: ipsets.NewIPSetManager(string(detectOsType())), + ipsetMgr: ipsets.NewIPSetManager(), endpointCache: make(map[string]*NPMEndpoint), } } -type OsType string - -const ( - Windows OsType = "windows" - Linux OsType = "linux" -) - -// Detects the OS type -func detectOsType() OsType { - os := runtime.GOOS - switch os { - case "linux": - return Linux - case "windows": - return Windows - default: - panic("Unsupported OS type: " + os) - } -} - +// InitializeDataPlane helps in setting up dataplane for NPM +// in linux this function should be adding required chains and rules +// in windows this function will help gather network and endpoint details func (dp *DataPlane) InitializeDataPlane() error { return dp.initializeDataPlane() } +// ResetDataPlane helps in cleaning up dataplane sets and policies programmed +// by NPM, retunring a clean slate func (dp *DataPlane) ResetDataPlane() error { return dp.resetDataPlane() } +// CreateIPSet takes in a set object and updates local cache with this set func (dp *DataPlane) CreateIPSet(set *ipsets.IPSet) error { return dp.ipsetMgr.CreateIPSet(set) } +// DeleteSet checks for members and references of the given "set" type ipset +// if not used then will delete it from cache func (dp *DataPlane) DeleteSet(name string) error { return dp.ipsetMgr.DeleteSet(name) } +// DeleteList sanity checks and deletes a list ipset func (dp *DataPlane) DeleteList(name string) error { return dp.ipsetMgr.DeleteList(name) } +// AddToSet takes in a list of IPset objects along with IP member +// and then updates it local cache func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSet, ip, podKey string) error { return dp.ipsetMgr.AddToSet(setNames, ip, podKey) } +// RemoveFromSet takes in list of setnames from which a given IP member should be +// removed and will update the local cache func (dp *DataPlane) RemoveFromSet(setNames []string, ip, podKey string) error { return dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) } +// AddToList takes a list name and list of sets which are to be added as members +// to given list func (dp *DataPlane) AddToList(listName string, setNames []string) error { return dp.ipsetMgr.AddToList(listName, setNames) } +// RemoveFromList takes a list name and list of sets which are to be removed as members +// to given list func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { return dp.ipsetMgr.RemoveFromList(listName, setNames) } +// UpdatePod is to be called by pod_controller ONLY when a new pod is CREATED. +// this function has two responsibilties in windows +// 1. Will call into dataplane and updates endpoint references of this pod. +// 2. Will check for exisitng applicable network policies and applies it on endpoint +// In Linux, this function currently is a no-op func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { return nil } +// ApplyDataPlane all the IPSet operations just update cache and update a dirty ipset structure, +// they do not change apply changes into dataplane. This function needs to be called at the +// end of IPSet operations of a given controller event, it will check for the dirty ipset list +// and accordingly makes changes in dataplane. This function helps emulate a single call to +// dataplane instead of multiple ipset operations calls ipset operations calls to dataplane func (dp *DataPlane) ApplyDataPlane() error { return dp.ipsetMgr.ApplyIPSets(dp.networkID) } +// AddPolicies takes in a translated NPMNetworkPolicy object and applies on dataplane func (dp *DataPlane) AddPolicies(policies *policies.NPMNetworkPolicy) error { return dp.policyMgr.AddPolicies(policies) } +// RemovePolicies takes in network policy name and removes it from dataplane and cache func (dp *DataPlane) RemovePolicies(policyName string) error { return dp.policyMgr.RemovePolicies(policyName) } +// UpdatePolicies takes in updated policy object, calculates the delta and applies changes +// onto dataplane accordingly func (dp *DataPlane) UpdatePolicies(policies *policies.NPMNetworkPolicy) error { return dp.policyMgr.UpdatePolicies(policies) } diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 387927e8fc..72aeb315e3 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -8,14 +8,14 @@ import ( const ( // Windows specific constants - AZURENETWORKNAME = "azure" + AzureNetworkName = "azure" ) func (dp *DataPlane) initializeDataPlane() error { fmt.Printf("Initializing dataplane for windows") // Get Network ID - network, err := hcn.GetNetworkByName(AZURENETWORKNAME) + network, err := hcn.GetNetworkByName(AzureNetworkName) if err != nil { return err } @@ -30,9 +30,9 @@ func (dp *DataPlane) initializeDataPlane() error { for _, endpoint := range endpoints { fmt.Println(endpoint.Policies) ep := &NPMEndpoint{ - Name: endpoint.Name, - ID: endpoint.Id, - NetpolRef: []string{}, + Name: endpoint.Name, + ID: endpoint.Id, + NetPolReference: make(map[string]struct{}), } dp.endpointCache[ep.Name] = ep diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 590dc25755..f8c60785e7 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -162,29 +162,21 @@ func (set *IPSet) DeleteNetPolReference(netPolName string) { } func (set *IPSet) CanBeDeleted() bool { - if len(set.SelectorReference) > 0 { - return false - } - if len(set.NetPolReference) > 0 { - return false - } - if set.IpsetReferCount > 0 { - return false - } - if len(set.MemberIPSets) > 0 { - return false - } - if len(set.IPPodKey) > 0 { + if len(set.SelectorReference) > 0 || + len(set.NetPolReference) > 0 || + set.IpsetReferCount > 0 || + len(set.MemberIPSets) > 0 || + len(set.IPPodKey) > 0 { + // return false if this IPSet has any references and memebrs. return false } return true } +// UsedByNetPol check if an IPSet is refered in network policies. func (set *IPSet) UsedByNetPol() bool { - if len(set.SelectorReference) <= 0 { - return false - } - if len(set.NetPolReference) <= 0 { + if len(set.SelectorReference) == 0 && + len(set.NetPolReference) == 0 { return false } return true diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 4337070181..125850d928 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -21,7 +21,7 @@ func (iMgr *IPSetManager) exists(name string) bool { return ok } -func NewIPSetManager(os string) IPSetManager { +func NewIPSetManager() IPSetManager { return IPSetManager{ setMap: make(map[string]*IPSet), dirtyCaches: make(map[string]struct{}), diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go index dd508e7e27..9de813f516 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go @@ -3,7 +3,6 @@ package ipsets import ( "fmt" - "github.com/Azure/azure-container-networking/npm/util" "github.com/Azure/azure-container-networking/npm/util/errors" ) diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index ed348dc79e..ddffe68516 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -6,10 +6,13 @@ import ( ) type NPMNetworkPolicy struct { - Name string + Name string + // PodSelectorIPSets holds all the IPSets generated from Pod Selector PodSelectorIPSets []*ipsets.IPSet - OtherIPSets []*ipsets.IPSet - ACLs []*ACLPolicy + // OtherIPSets holds all IPSets generated from policy + // except for pod selector IPSets + OtherIPSets []*ipsets.IPSet + ACLs []*ACLPolicy // Making this a podKey instead should be // use NPMPod obj Pods []string @@ -28,6 +31,12 @@ type ACLPolicy struct { // Iptable rules Protocol string } +// SetInfo helps capture additional details in a matchSet +// exmaple match set in linux: +// ! azure-npm-123 src,src +// "!" this indicates a negative match of an IPset for src,src +// Included flag captures the negative or positive match +// MatchType captures match flags type SetInfo struct { IPSet *ipsets.IPSet Included bool From cf8e556c893d6f9361f859ce0a1fadb5df3410d4 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 1 Sep 2021 22:19:52 -0700 Subject: [PATCH 09/26] Fixing some golints --- npm/pkg/dataplane/dataplane.go | 4 ++-- npm/pkg/dataplane/dataplane_test.go | 1 - npm/pkg/dataplane/ipsets/ipset.go | 4 +++- npm/pkg/dataplane/ipsets/ipsetmanager.go | 4 ++-- npm/pkg/dataplane/ipsets/ipsetmanager_linux.go | 1 - npm/pkg/dataplane/policies/policy.go | 3 ++- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 276a1f9f60..25c1985884 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -82,9 +82,9 @@ func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { } // UpdatePod is to be called by pod_controller ONLY when a new pod is CREATED. -// this function has two responsibilties in windows +// this function has two responsibilities in windows // 1. Will call into dataplane and updates endpoint references of this pod. -// 2. Will check for exisitng applicable network policies and applies it on endpoint +// 2. Will check for existing applicable network policies and applies it on endpoint // In Linux, this function currently is a no-op func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { return nil diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 24483e4e99..f844dcc888 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -18,5 +18,4 @@ func TestNewDataPlane(t *testing.T) { if err != nil { t.Error("CreateIPSet() returned error") } - } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index f8c60785e7..df55159bea 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -128,6 +128,8 @@ func getSetKind(set *IPSet) SetKind { return ListSet case NestedLabelOfPod: return ListSet + case Unknown: // adding this to appease golint + return "unknown" default: return "unknown" } @@ -173,7 +175,7 @@ func (set *IPSet) CanBeDeleted() bool { return true } -// UsedByNetPol check if an IPSet is refered in network policies. +// UsedByNetPol check if an IPSet is referred in network policies. func (set *IPSet) UsedByNetPol() bool { if len(set.SelectorReference) == 0 && len(set.NetPolReference) == 0 { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 125850d928..51f860a380 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -64,7 +64,7 @@ func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { return nil } - // Call the dataplane specifc fucntion here to + // Call the dataplane specifc function here to // create the Set // append the cache if dataplane specific function @@ -233,7 +233,7 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err } // Nested IPSets are only supported for windows - //Check if we want to actually use that support + // Check if we want to actually use that support if getSetKind(set) != HashSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go index 9de813f516..93ec2896ca 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go @@ -18,4 +18,3 @@ func (iMgr *IPSetManager) applyIPSets(networkID string) error { } return nil } - diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index ddffe68516..6ac7fbb83e 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -32,7 +32,7 @@ type ACLPolicy struct { // Iptable rules } // SetInfo helps capture additional details in a matchSet -// exmaple match set in linux: +// example match set in linux: // ! azure-npm-123 src,src // "!" this indicates a negative match of an IPset for src,src // Included flag captures the negative or positive match @@ -49,6 +49,7 @@ type Ports struct { } type Verdict string + type Direction string const ( From bfff6a5413a9fee2825323e00f43694de43c0050 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 2 Sep 2021 11:34:06 -0700 Subject: [PATCH 10/26] addressing some comments --- npm/pkg/dataplane/dataplane.go | 6 +++-- npm/pkg/dataplane/ipsets/ipset.go | 29 ++++++++++-------------- npm/pkg/dataplane/ipsets/ipsetmanager.go | 4 +++- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 25c1985884..198f2d0281 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -15,8 +15,10 @@ type DataPlane struct { } type NPMEndpoint struct { - Name string - ID string + Name string + ID string + // Using a map to emulate set and value as struct{} for + // minimal memory consumption NetPolReference map[string]struct{} } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index df55159bea..a3bfa774e7 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -15,7 +15,8 @@ type IPSet struct { IPPodKey map[string]string // This is used for listMaps to store child IP Sets MemberIPSets map[string]*IPSet - // Using a map here to emulate set of netpol names + // using a map to emulate set and using struct{} for + // minimal memory consumption SelectorReference map[string]struct{} NetPolReference map[string]struct{} IpsetReferCount int @@ -92,21 +93,22 @@ func NewIPSet(name string, setType SetType) *IPSet { } func (set *IPSet) GetSetContents() ([]string, error) { - contents := make([]string, 0) setType := getSetKind(set) switch setType { case HashSet: + contents := make([]string, len(set.IPPodKey)) for podIP := range set.IPPodKey { contents = append(contents, podIP) } return contents, nil case ListSet: + contents := make([]string, len(set.MemberIPSets)) for _, memberSet := range set.MemberIPSets { contents = append(contents, memberSet.HashedName) } return contents, nil default: - return contents, fmt.Errorf("Unknown set type %s", setType) + return []string{}, fmt.Errorf("Unknown set type %s", setType) } } @@ -164,22 +166,15 @@ func (set *IPSet) DeleteNetPolReference(netPolName string) { } func (set *IPSet) CanBeDeleted() bool { - if len(set.SelectorReference) > 0 || - len(set.NetPolReference) > 0 || - set.IpsetReferCount > 0 || - len(set.MemberIPSets) > 0 || - len(set.IPPodKey) > 0 { - // return false if this IPSet has any references and memebrs. - return false - } - return true + return len(set.SelectorReference) == 0 && + len(set.NetPolReference) == 0 && + set.IpsetReferCount == 0 && + len(set.MemberIPSets) == 0 && + len(set.IPPodKey) == 0 } // UsedByNetPol check if an IPSet is referred in network policies. func (set *IPSet) UsedByNetPol() bool { - if len(set.SelectorReference) == 0 && - len(set.NetPolReference) == 0 { - return false - } - return true + return len(set.SelectorReference) >= 0 && + len(set.NetPolReference) >= 0 } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 51f860a380..e1ea4a59ae 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -11,7 +11,9 @@ import ( ) type IPSetManager struct { - setMap map[string]*IPSet + setMap map[string]*IPSet + // Using a map to emulate set and value as struct{} for + // minimal memory consumption dirtyCaches map[string]struct{} sync.Mutex } From 4fe29163926c2b3f6ab4b5a0db643414aa5cebb5 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 2 Sep 2021 12:49:26 -0700 Subject: [PATCH 11/26] adding some golint checks --- npm/pkg/dataplane/ipsets/ipset.go | 18 ++++++++++++------ npm/pkg/dataplane/policies/policymanager.go | 6 +++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index a3bfa774e7..09520c610f 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -15,7 +15,7 @@ type IPSet struct { IPPodKey map[string]string // This is used for listMaps to store child IP Sets MemberIPSets map[string]*IPSet - // using a map to emulate set and using struct{} for + // Using a map to emulate set and value as struct{} for // minimal memory consumption SelectorReference map[string]struct{} NetPolReference map[string]struct{} @@ -77,9 +77,11 @@ const ( func NewIPSet(name string, setType SetType) *IPSet { set := &IPSet{ - Name: name, - HashedName: util.GetHashedName(name), - Type: setType, + Name: name, + HashedName: util.GetHashedName(name), + Type: setType, + // Using a map to emulate set and value as struct{} for + // minimal memory consumption SelectorReference: make(map[string]struct{}), NetPolReference: make(map[string]struct{}), IpsetReferCount: 0, @@ -96,15 +98,19 @@ func (set *IPSet) GetSetContents() ([]string, error) { setType := getSetKind(set) switch setType { case HashSet: + i := 0 contents := make([]string, len(set.IPPodKey)) for podIP := range set.IPPodKey { - contents = append(contents, podIP) + contents[i] = podIP + i++ } return contents, nil case ListSet: + i := 0 contents := make([]string, len(set.MemberIPSets)) for _, memberSet := range set.MemberIPSets { - contents = append(contents, memberSet.HashedName) + contents[i] = memberSet.HashedName + i++ } return contents, nil default: diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 54a5241cee..3e454f863f 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -3,7 +3,7 @@ package policies import "sync" type PolicyMap struct { - sync.Mutex + sync.RWMutex cache map[string]*NPMNetworkPolicy } @@ -20,8 +20,8 @@ func NewPolicyManager() PolicyManager { } func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { - pMgr.policyMap.Lock() - defer pMgr.policyMap.Unlock() + pMgr.policyMap.RLock() + defer pMgr.policyMap.RUnlock() if policy, ok := pMgr.policyMap.cache[name]; ok { return policy, nil From 9441d082fe27becfa068f0d65b6d820e5bc827c8 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 7 Sep 2021 12:49:03 -0700 Subject: [PATCH 12/26] Adding a new field setProperty and also adding structure for policies --- npm/pkg/dataplane/ipsets/ipset.go | 28 +++++++--- npm/pkg/dataplane/ipsets/ipsetmanager.go | 39 +++----------- .../dataplane/ipsets/ipsetmanager_windows.go | 9 ++-- npm/pkg/dataplane/policies/policy_windows.go | 53 +++++++++++++++++++ .../dataplane/policies/policymanager_linux.go | 13 +++++ .../policies/policymanager_windows.go | 13 +++++ 6 files changed, 111 insertions(+), 44 deletions(-) create mode 100644 npm/pkg/dataplane/policies/policy_windows.go create mode 100644 npm/pkg/dataplane/policies/policymanager_linux.go create mode 100644 npm/pkg/dataplane/policies/policymanager_windows.go diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 09520c610f..b4b847dd47 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -9,7 +9,7 @@ import ( type IPSet struct { Name string HashedName string - Type SetType + Properties SetProperties // IpPodKey is used for setMaps to store Ips and ports as keys // and podKey as value IPPodKey map[string]string @@ -22,6 +22,13 @@ type IPSet struct { IpsetReferCount int } +type SetProperties struct { + // Stores type of ip grouping + Type SetType + // Stores kind of ipset in dataplane + Kind SetKind +} + type SetType int32 const ( @@ -79,14 +86,17 @@ func NewIPSet(name string, setType SetType) *IPSet { set := &IPSet{ Name: name, HashedName: util.GetHashedName(name), - Type: setType, + Properties: SetProperties{ + Type: setType, + Kind: getSetKind(setType), + }, // Using a map to emulate set and value as struct{} for // minimal memory consumption SelectorReference: make(map[string]struct{}), NetPolReference: make(map[string]struct{}), IpsetReferCount: 0, } - if getSetKind(set) == HashSet { + if set.Properties.Kind == HashSet { set.IPPodKey = make(map[string]string) } else { set.MemberIPSets = make(map[string]*IPSet) @@ -95,8 +105,7 @@ func NewIPSet(name string, setType SetType) *IPSet { } func (set *IPSet) GetSetContents() ([]string, error) { - setType := getSetKind(set) - switch setType { + switch set.Properties.Kind { case HashSet: i := 0 contents := make([]string, len(set.IPPodKey)) @@ -114,12 +123,12 @@ func (set *IPSet) GetSetContents() ([]string, error) { } return contents, nil default: - return []string{}, fmt.Errorf("Unknown set type %s", setType) + return []string{}, fmt.Errorf("Unknown set type %s", set.Properties.Type.String()) } } -func getSetKind(set *IPSet) SetKind { - switch set.Type { +func getSetKind(setType SetType) SetKind { + switch setType { case CIDRBlocks: return HashSet case NameSpace: @@ -152,6 +161,9 @@ func (set *IPSet) IncIpsetReferCount() { } func (set *IPSet) DecIpsetReferCount() { + if set.IpsetReferCount == 0 { + return + } set.IpsetReferCount-- } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index e1ea4a59ae..67e18fe16d 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -42,7 +42,7 @@ func (iMgr *IPSetManager) updateDirtyCache(setName string) { } iMgr.dirtyCaches[set.Name] = struct{}{} - if getSetKind(set) == ListSet { + if set.Properties.Kind == ListSet { // TODO check if we will need to add all the member ipsets // also to the dirty cache list for _, member := range set.MemberIPSets { @@ -66,9 +66,6 @@ func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { return nil } - // Call the dataplane specifc function here to - // create the Set - // append the cache if dataplane specific function // return nil as error iMgr.setMap[set.Name] = set @@ -87,14 +84,13 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error for _, updatedSet := range addToSets { set, exists := iMgr.setMap[updatedSet.Name] // check if the Set exists if !exists { - set = NewIPSet(updatedSet.Name, updatedSet.Type) err := iMgr.CreateIPSet(set) if err != nil { return err } } - if getSetKind(set) != HashSet { + if set.Properties.Kind != HashSet { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a hash set", set.Name)) } cachedPodKey, ok := set.IPPodKey[ip] @@ -108,10 +104,6 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error return nil } - // Now actually add the IP to the Set - // err := addToSet(setName, ip) - // some more error handling here - // update the IP ownership with podkey set.IPPodKey[ip] = podKey iMgr.updateDirtyCache(set.Name) @@ -133,7 +125,7 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } - if getSetKind(set) != HashSet { + if set.Properties.Kind != HashSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) } @@ -146,10 +138,6 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri return nil } - // Now actually delete the IP from the Set - // err := deleteFromSet(setName, ip) - // some more error handling here - // update the IP ownership with podkey delete(set.IPPodKey, ip) iMgr.updateDirtyCache(set.Name) @@ -167,7 +155,6 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { defer iMgr.Unlock() for _, setName := range setNames { - if listName == setName { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("list %s cannot be added to itself", listName)) } @@ -178,7 +165,7 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { // Nested IPSets are only supported for windows // Check if we want to actually use that support - if getSetKind(set) != HashSet { + if set.Properties.Kind != HashSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } @@ -187,7 +174,7 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } - if getSetKind(list) != ListSet { + if list.Properties.Kind != ListSet { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) } @@ -203,14 +190,9 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { return nil } - // Now actually add the Set to the List - // err := addToList(listName, setName) - // some more error handling here - // update the Ipset member list of list list.AddMemberIPSet(set) set.IncIpsetReferCount() - // Update metrics of the IpSet metrics.NumIPSetEntries.Inc() metrics.IncIPSetInventory(setName) @@ -230,13 +212,13 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } - if getSetKind(set) != HashSet { + if set.Properties.Kind != HashSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) } // Nested IPSets are only supported for windows // Check if we want to actually use that support - if getSetKind(set) != HashSet { + if set.Properties.Kind != HashSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } @@ -245,7 +227,7 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } - if getSetKind(list) != ListSet { + if list.Properties.Kind != ListSet { return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) } @@ -255,14 +237,9 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err return nil } - // Now actually delete the Set from the List - // err := deleteFromList(listName, setName) - // some more error handling here - // delete IPSet from the list delete(list.MemberIPSets, setName) set.DecIpsetReferCount() - // Update metrics of the IpSet metrics.NumIPSetEntries.Dec() metrics.DecIPSetInventory(setName) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index cd5b41be7b..daf913ef7e 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -97,7 +97,7 @@ func (iMgr *IPSetManager) calculateNewSetPolicies(existingSets []string) (map[st return nil, err } setsToUpdate[setName] = setPol - if getSetKind(set) == ListSet { + if set.Properties.Kind == ListSet { for _, memberSet := range set.MemberIPSets { // TODO check whats the name here, hashed or normal if _, ok := setsToUpdate[memberSet.Name]; ok { @@ -120,8 +120,8 @@ func isValidIPSet(set *IPSet) error { return fmt.Errorf("IPSet " + set.Name + " is missing Name") } - if set.Type == Unknown { - return fmt.Errorf("IPSet " + set.Type.String() + " is missing Type") + if set.Properties.Type == Unknown { + return fmt.Errorf("IPSet " + set.Properties.Type.String() + " is missing Type") } if set.HashedName == "" { @@ -132,8 +132,7 @@ func isValidIPSet(set *IPSet) error { } func getSetPolicyType(set *IPSet) SetPolicyType { - setKind := getSetKind(set) - switch setKind { + switch set.Properties.Kind { case ListSet: return SetPolicyTypeNestedIpSet case HashSet: diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go new file mode 100644 index 0000000000..644e15911a --- /dev/null +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -0,0 +1,53 @@ +package policies + +import ( + "fmt" + "strings" + + "github.com/Microsoft/hcsshim/hcn" +) + +func convertToAclSettings(acl ACLPolicy) (hcn.AclPolicySetting, error) { + policySettings := hcn.AclPolicySetting{} + for _, setInfo := range acl.SrcList { + if !setInfo.Included { + return policySettings, fmt.Errorf("Windows Dataplane does not support negative matches. ACL: %+v", acl) + } + } + + return policySettings, nil +} + +func getHCNDirection(direction Direction) hcn.DirectionType { + switch direction { + case Ingress: + return hcn.DirectionTypeIn + case Egress: + return hcn.DirectionTypeOut + case Both: + return "" + } + return "" +} + +func getHCNProtocol(protocol string) string { + // TODO need to check the protocol number of SCTP + switch strings.ToLower(protocol) { + case "tcp": + return "6" + case "udp": + return "17" + default: + return "" + } +} + +func getHCNAction(verdict Verdict) hcn.ActionType { + switch verdict { + case Allowed: + return hcn.ActionTypeAllow + case Dropped: + return hcn.ActionTypeBlock + } + return "" +} diff --git a/npm/pkg/dataplane/policies/policymanager_linux.go b/npm/pkg/dataplane/policies/policymanager_linux.go new file mode 100644 index 0000000000..5d6dddfc88 --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager_linux.go @@ -0,0 +1,13 @@ +package policies + +func (pMgr *PolicyManager) addPolicies(policy *NPMNetworkPolicy) error { + return nil +} + +func (pMgr *PolicyManager) removePolicies(name string) error { + return nil +} + +func (pMgr *PolicyManager) updatePolicies(policy *NPMNetworkPolicy) error { + return nil +} diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go new file mode 100644 index 0000000000..5d6dddfc88 --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -0,0 +1,13 @@ +package policies + +func (pMgr *PolicyManager) addPolicies(policy *NPMNetworkPolicy) error { + return nil +} + +func (pMgr *PolicyManager) removePolicies(name string) error { + return nil +} + +func (pMgr *PolicyManager) updatePolicies(policy *NPMNetworkPolicy) error { + return nil +} From 14ef65f78a29228d63cbd5692f66d83df7a11605 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 7 Sep 2021 14:10:48 -0700 Subject: [PATCH 13/26] applying some comments --- npm/util/util.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/npm/util/util.go b/npm/util/util.go index 001a073e72..c7cf3d0c9f 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -343,15 +343,5 @@ func CompareSlices(list1, list2 []string) bool { } func SliceToString(list []string) string { - returnStr := "" - - for _, s := range list { - if returnStr != "" { - returnStr = returnStr + SetPolicyDelimiter + s - } else { - returnStr = s - } - } - - return returnStr + return strings.Join(list[:], SetPolicyDelimiter) } From 9c1c55830f38f141c7e93269642669c9057c0d0a Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 7 Sep 2021 14:20:33 -0700 Subject: [PATCH 14/26] correcting a condition --- npm/pkg/dataplane/ipsets/ipset.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index b4b847dd47..6dda854c6c 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -193,6 +193,6 @@ func (set *IPSet) CanBeDeleted() bool { // UsedByNetPol check if an IPSet is referred in network policies. func (set *IPSet) UsedByNetPol() bool { - return len(set.SelectorReference) >= 0 && - len(set.NetPolReference) >= 0 + return len(set.SelectorReference) > 0 && + len(set.NetPolReference) > 0 } From 57d78affbf0a28483f41b80f3e8f2b02dfe28fbf Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 7 Sep 2021 14:56:02 -0700 Subject: [PATCH 15/26] Adding some comments --- npm/pkg/dataplane/ipsets/ipset.go | 48 +++++++++++++++------ npm/pkg/dataplane/policies/policy.go | 35 ++++++++++----- npm/pkg/dataplane/policies/policymanager.go | 18 +++++++- 3 files changed, 76 insertions(+), 25 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 6dda854c6c..c857a36c52 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -17,9 +17,15 @@ type IPSet struct { MemberIPSets map[string]*IPSet // Using a map to emulate set and value as struct{} for // minimal memory consumption + // SelectorReference holds networkpolicy names where this IPSet + // is being used in PodSelector and NameSpace SelectorReference map[string]struct{} - NetPolReference map[string]struct{} - IpsetReferCount int + // NetPolReference holds networkpolicy names where this IPSet + // is being referred as part of rules + NetPolReference map[string]struct{} + // IpsetReferCount keeps count of 2nd level Nested IPSets + // with member as this IPSet + IpsetReferCount int } type SetProperties struct { @@ -32,18 +38,30 @@ type SetProperties struct { type SetType int32 const ( - Unknown SetType = 0 - NameSpace SetType = 1 - KeyLabelOfNameSpace SetType = 2 + // Unknown SetType + Unknown SetType = 0 + // NameSpace IPSet is created to hold + // ips of pods in a given NameSapce + NameSpace SetType = 1 + // KeyLabelOfNameSpace IPSet is a list kind ipset + // with members as ipsets of namespace with this Label Key + KeyLabelOfNameSpace SetType = 2 + // KeyValueLabelOfNameSpace IPSet is a list kind ipset + // with members as ipsets of namespace with this Label KeyValueLabelOfNameSpace SetType = 3 - KeyLabelOfPod SetType = 4 - KeyValueLabelOfPod SetType = 5 - NamedPorts SetType = 6 - NestedLabelOfPod SetType = 7 - CIDRBlocks SetType = 8 + // KeyLabelOfPod IPSet contains IPs of Pods with this Label Key + KeyLabelOfPod SetType = 4 + // KeyValueLabelOfPod IPSet contains IPs of Pods with this Label + KeyValueLabelOfPod SetType = 5 + // NamedPorts IPSets contains a given namedport + NamedPorts SetType = 6 + // NestedLabelOfPod is derived for multivalue matchexpressions + NestedLabelOfPod SetType = 7 + // CIDRBlocks holds CIDR blocks + CIDRBlocks SetType = 8 ) -var SetTypeName = map[int32]string{ +var setTypeName = map[int32]string{ 0: "Unknown", 1: "NameSpace", 2: "KeyLabelOfNameSpace", @@ -55,7 +73,7 @@ var SetTypeName = map[int32]string{ 8: "CIDRBlocks", } -var SetTypeValue = map[string]int32{ +var setTypeValue = map[string]int32{ "Unknown": 0, "NameSpace": 1, "KeyLabelOfNameSpace": 2, @@ -68,17 +86,19 @@ var SetTypeValue = map[string]int32{ } func (x SetType) String() string { - return SetTypeName[int32(x)] + return setTypeName[int32(x)] } func GetSetType(x string) SetType { - return SetType(SetTypeValue[x]) + return SetType(setTypeValue[x]) } type SetKind string const ( + // ListSet is of kind list with members as other IPSets ListSet SetKind = "list" + // HashSet is of kind hashset with members as IPs and/or port HashSet SetKind = "set" ) diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 6ac7fbb83e..ed768ce84d 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -20,15 +20,25 @@ type NPMNetworkPolicy struct { } type ACLPolicy struct { // Iptable rules - PolicyID string - Comment string - SrcList []SetInfo - DstList []SetInfo - Target Verdict + // PolicyID is the rules name with a given network policy + PolicyID string + // Comment is the string attached to rule to identity its representation + Comment string + // SrcList source IPSets condition setinfos + SrcList []SetInfo + // DstList destination IPSets condition setinfos + DstList []SetInfo + // Target defines a target in iptables for linux. i,e, Mark, Accept, Drop + // in windows, this is either ALLOW or DENY + Target Verdict + // Direction defines the flow of traffic Direction Direction - SrcPorts []Ports - DstPorts []Ports - Protocol string + // SrcPorts holds the source port information + SrcPorts []Ports + // DstPorts holds the destination port information + DstPorts []Ports + // Protocol is the value of traffic protocol + Protocol string } // SetInfo helps capture additional details in a matchSet @@ -53,10 +63,15 @@ type Verdict string type Direction string const ( + // Ingress when packet is entering a container Ingress Direction = "IN" - Egress Direction = "OUT" - Both Direction = "BOTH" + // Egress when packet is leaving a container + Egress Direction = "OUT" + // Both applies to both directions + Both Direction = "BOTH" + // Allowed is accept in linux Allowed Verdict = "ALLOW" + // Dropped is denying a flow Dropped Verdict = "DROP" ) diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 3e454f863f..b5ea4c325d 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -34,8 +34,13 @@ func (pMgr *PolicyManager) AddPolicies(policy *NPMNetworkPolicy) error { pMgr.policyMap.Lock() defer pMgr.policyMap.Unlock() - pMgr.policyMap.cache[policy.Name] = policy + // Call actual dataplane function to apply changes + err := pMgr.addPolicies(policy) + if err != nil { + return err + } + pMgr.policyMap.cache[policy.Name] = policy return nil } @@ -43,6 +48,12 @@ func (pMgr *PolicyManager) RemovePolicies(name string) error { pMgr.policyMap.Lock() defer pMgr.policyMap.Unlock() + // Call actual dataplane function to apply changes + err := pMgr.removePolicies(name) + if err != nil { + return err + } + delete(pMgr.policyMap.cache, name) return nil @@ -53,6 +64,11 @@ func (pMgr *PolicyManager) UpdatePolicies(policy *NPMNetworkPolicy) error { defer pMgr.policyMap.Unlock() // check and update + // Call actual dataplane function to apply changes + err := pMgr.updatePolicies(policy) + if err != nil { + return err + } return nil } From f702d8875c6cbd49cfdc6cc5cfbf2f3c7bdd24a9 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 8 Sep 2021 10:21:17 -0700 Subject: [PATCH 16/26] Adding some test cases --- npm/pkg/dataplane/ipsets/ipsetmanager.go | 7 +- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 155 ++++++++++++++++++ npm/pkg/dataplane/policies/policy_windows.go | 7 +- .../dataplane/policies/policymanager_test.go | 39 +++++ 4 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 npm/pkg/dataplane/ipsets/ipsetmanager_test.go create mode 100644 npm/pkg/dataplane/policies/policymanager_test.go diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 67e18fe16d..c0afb4deb7 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -59,6 +59,10 @@ func (iMgr *IPSetManager) clearDirtyCache() { func (iMgr *IPSetManager) CreateIPSet(set *IPSet) error { iMgr.Lock() defer iMgr.Unlock() + return iMgr.createIPSet(set) +} + +func (iMgr *IPSetManager) createIPSet(set *IPSet) error { // Check if the Set already exists if iMgr.exists(set.Name) { // ipset already exists @@ -84,10 +88,11 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error for _, updatedSet := range addToSets { set, exists := iMgr.setMap[updatedSet.Name] // check if the Set exists if !exists { - err := iMgr.CreateIPSet(set) + err := iMgr.createIPSet(updatedSet) if err != nil { return err } + set = iMgr.setMap[updatedSet.Name] } if set.Properties.Kind != HashSet { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go new file mode 100644 index 0000000000..624554c261 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -0,0 +1,155 @@ +package ipsets + +import ( + "fmt" + "os" + "testing" + + "github.com/Azure/azure-container-networking/npm/metrics" +) + +func TestCreateIPSet(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("Test", NameSpace) + + err := iMgr.CreateIPSet(set) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } +} + +func TestAddToSet(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("Test", NameSpace) + + metrics.NumIPSetEntries.Set(0) + fmt.Println(set.Name) + err := iMgr.AddToSet([]*IPSet{set}, "10.0.0.0", "test") + if err != nil { + t.Errorf("AddToSet() returned error %s", err.Error()) + } +} + +func TestRemoveFromSet(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("Test", NameSpace) + + err := iMgr.AddToSet([]*IPSet{set}, "10.0.0.0", "test") + if err != nil { + t.Errorf("RemoveFromSet() returned error %s", err.Error()) + } + err = iMgr.RemoveFromSet([]string{"Test"}, "10.0.0.0", "test") + if err != nil { + t.Errorf("RemoveFromSet() returned error %s", err.Error()) + } +} + +func TestRemoveFromSetMissing(t *testing.T) { + iMgr := NewIPSetManager() + err := iMgr.RemoveFromSet([]string{"Test"}, "10.0.0.0", "test") + if err == nil { + t.Errorf("RemoveFromSet() did not return error") + } +} + +func TestAddToListMissing(t *testing.T) { + iMgr := NewIPSetManager() + err := iMgr.AddToList("test", []string{"newtest"}) + if err == nil { + t.Errorf("AddToList() did not return error") + } +} + +func TestAddToList(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("newtest", NameSpace) + err := iMgr.CreateIPSet(set) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + list := NewIPSet("test", KeyLabelOfNameSpace) + err = iMgr.CreateIPSet(list) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + err = iMgr.AddToList("test", []string{"newtest"}) + if err != nil { + t.Errorf("AddToList() returned error %s", err.Error()) + } +} + +func TestRemoveFromList(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("newtest", NameSpace) + err := iMgr.CreateIPSet(set) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + list := NewIPSet("test", KeyLabelOfNameSpace) + err = iMgr.CreateIPSet(list) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + err = iMgr.AddToList("test", []string{"newtest"}) + if err != nil { + t.Errorf("AddToList() returned error %s", err.Error()) + } + + err = iMgr.RemoveFromList("test", []string{"newtest"}) + if err != nil { + t.Errorf("RemoveFromList() returned error %s", err.Error()) + } + +} + +func TestRemoveFromListMissing(t *testing.T) { + iMgr := NewIPSetManager() + err := iMgr.RemoveFromList("test", []string{"newtest"}) + if err == nil { + t.Errorf("RemoveFromList() did not return error") + } + +} + +func TestDeleteList(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("Test", KeyValueLabelOfNameSpace) + + err := iMgr.CreateIPSet(set) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + err = iMgr.DeleteList(set.Name) + if err != nil { + t.Errorf("DeleteList() returned error %s", err.Error()) + } + +} + +func TestDeleteSet(t *testing.T) { + iMgr := NewIPSetManager() + set := NewIPSet("Test", NameSpace) + + err := iMgr.CreateIPSet(set) + if err != nil { + t.Errorf("CreateIPSet() returned error %s", err.Error()) + } + + err = iMgr.DeleteSet(set.Name) + if err != nil { + t.Errorf("DeleteSet() returned error %s", err.Error()) + } +} + +func TestMain(m *testing.M) { + metrics.InitializeAll() + + exitCode := m.Run() + + os.Exit(exitCode) +} diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 644e15911a..368d7734b5 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -37,8 +37,13 @@ func getHCNProtocol(protocol string) string { return "6" case "udp": return "17" + case "icmp": + return "1" + case "sctp": + return "132" default: - return "" + // HNS thinks 256 as ANY + return "256" } } diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go new file mode 100644 index 0000000000..a09a817b95 --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -0,0 +1,39 @@ +package policies + +import "testing" + +func TestAddPolicy(t *testing.T) { + pMgr := NewPolicyManager() + + netpol := NPMNetworkPolicy{} + + err := pMgr.AddPolicies(&netpol) + if err != nil { + t.Errorf("AddPolicies() returned error %s", err.Error()) + } +} + +func TestRemovePolicies(t *testing.T) { + pMgr := NewPolicyManager() + + err := pMgr.RemovePolicies("test") + if err != nil { + t.Errorf("RemovePolicies() returned error %s", err.Error()) + } +} + +func TestUpdatePolicies(t *testing.T) { + pMgr := NewPolicyManager() + + netpol := NPMNetworkPolicy{} + + err := pMgr.AddPolicies(&netpol) + if err != nil { + t.Errorf("UpdatePolicies() returned error %s", err.Error()) + } + + err = pMgr.UpdatePolicies(&netpol) + if err != nil { + t.Errorf("UpdatePolicies() returned error %s", err.Error()) + } +} From a9dfbc313fc8269c9a2c9ae1e741c509e4a068c1 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 8 Sep 2021 16:09:52 -0700 Subject: [PATCH 17/26] Addressing some comments --- npm/pkg/dataplane/dataplane.go | 8 +-- npm/pkg/dataplane/dataplane_linux.go | 11 +++- npm/pkg/dataplane/dataplane_windows.go | 16 ++++-- npm/pkg/dataplane/ipsets/ipset.go | 49 ++++++---------- npm/pkg/dataplane/ipsets/ipsetmanager.go | 56 +++++++++---------- .../dataplane/ipsets/ipsetmanager_windows.go | 10 ++-- npm/pkg/dataplane/policies/policy.go | 14 ++++- npm/pkg/dataplane/policies/policy_windows.go | 27 +++------ 8 files changed, 95 insertions(+), 96 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 198f2d0281..4d6f2e8915 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -31,8 +31,6 @@ func NewDataPlane() *DataPlane { } // InitializeDataPlane helps in setting up dataplane for NPM -// in linux this function should be adding required chains and rules -// in windows this function will help gather network and endpoint details func (dp *DataPlane) InitializeDataPlane() error { return dp.initializeDataPlane() } @@ -84,12 +82,8 @@ func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { } // UpdatePod is to be called by pod_controller ONLY when a new pod is CREATED. -// this function has two responsibilities in windows -// 1. Will call into dataplane and updates endpoint references of this pod. -// 2. Will check for existing applicable network policies and applies it on endpoint -// In Linux, this function currently is a no-op func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { - return nil + return dp.updatePod(pod) } // ApplyDataPlane all the IPSet operations just update cache and update a dirty ipset structure, diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go index 39711006e8..620e2b911b 100644 --- a/npm/pkg/dataplane/dataplane_linux.go +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -2,10 +2,19 @@ package dataplane import ( "fmt" + + "github.com/Azure/azure-container-networking/npm" + "k8s.io/klog" ) +// initializeDataPlane should be adding required chains and rules func (dp *DataPlane) initializeDataPlane() error { - fmt.Printf("Initializing dataplane for linux") + klog.Infof("Initializing dataplane for linux") + return nil +} + +// updatePod is no-op in Linux +func (dp *DataPlane) updatePod(pod *npm.NpmPod) error { return nil } diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 72aeb315e3..051a79421a 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -1,9 +1,9 @@ package dataplane import ( - "fmt" - + "github.com/Azure/azure-container-networking/npm" "github.com/Microsoft/hcsshim/hcn" + "k8s.io/klog" ) const ( @@ -11,8 +11,9 @@ const ( AzureNetworkName = "azure" ) +// initializeDataPlane will help gather network and endpoint details func (dp *DataPlane) initializeDataPlane() error { - fmt.Printf("Initializing dataplane for windows") + klog.Infof("Initializing dataplane for windows") // Get Network ID network, err := hcn.GetNetworkByName(AzureNetworkName) @@ -28,7 +29,7 @@ func (dp *DataPlane) initializeDataPlane() error { } for _, endpoint := range endpoints { - fmt.Println(endpoint.Policies) + klog.Infof("Endpoints info %+v", endpoint.Policies) ep := &NPMEndpoint{ Name: endpoint.Name, ID: endpoint.Id, @@ -41,6 +42,13 @@ func (dp *DataPlane) initializeDataPlane() error { return nil } +// updatePod has two responsibilities in windows +// 1. Will call into dataplane and updates endpoint references of this pod. +// 2. Will check for existing applicable network policies and applies it on endpoint +func (dp *DataPlane) updatePod(pod *npm.NpmPod) error { + return nil +} + func (dp *DataPlane) resetDataPlane() error { return nil } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index c857a36c52..cb262af75c 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -9,7 +9,8 @@ import ( type IPSet struct { Name string HashedName string - Properties SetProperties + // SetProperties embedding set properties + SetProperties // IpPodKey is used for setMaps to store Ips and ports as keys // and podKey as value IPPodKey map[string]string @@ -61,36 +62,20 @@ const ( CIDRBlocks SetType = 8 ) -var setTypeName = map[int32]string{ - 0: "Unknown", - 1: "NameSpace", - 2: "KeyLabelOfNameSpace", - 3: "KeyValueLabelOfNameSpace", - 4: "KeyLabelOfPod", - 5: "KeyValueLabelOfPod", - 6: "NamedPorts", - 7: "NestedLabelOfPod", - 8: "CIDRBlocks", -} - -var setTypeValue = map[string]int32{ - "Unknown": 0, - "NameSpace": 1, - "KeyLabelOfNameSpace": 2, - "KeyValueLabelOfNameSpace": 3, - "KeyLabelOfPod": 4, - "KeyValueLabelOfPod": 5, - "NamedPorts": 6, - "NestedLabelOfPod": 7, - "CIDRBlocks": 8, +var setTypeName = map[SetType]string{ + Unknown: "Unknown", + NameSpace: "NameSpace", + KeyLabelOfNameSpace: "KeyLabelOfNameSpace", + KeyValueLabelOfNameSpace: "KeyValueLabelOfNameSpace", + KeyLabelOfPod: "KeyLabelOfPod", + KeyValueLabelOfPod: "KeyValueLabelOfPod", + NamedPorts: "NamedPorts", + NestedLabelOfPod: "NestedLabelOfPod", + CIDRBlocks: "CIDRBlocks", } func (x SetType) String() string { - return setTypeName[int32(x)] -} - -func GetSetType(x string) SetType { - return SetType(setTypeValue[x]) + return setTypeName[x] } type SetKind string @@ -106,7 +91,7 @@ func NewIPSet(name string, setType SetType) *IPSet { set := &IPSet{ Name: name, HashedName: util.GetHashedName(name), - Properties: SetProperties{ + SetProperties: SetProperties{ Type: setType, Kind: getSetKind(setType), }, @@ -116,7 +101,7 @@ func NewIPSet(name string, setType SetType) *IPSet { NetPolReference: make(map[string]struct{}), IpsetReferCount: 0, } - if set.Properties.Kind == HashSet { + if set.Kind == HashSet { set.IPPodKey = make(map[string]string) } else { set.MemberIPSets = make(map[string]*IPSet) @@ -125,7 +110,7 @@ func NewIPSet(name string, setType SetType) *IPSet { } func (set *IPSet) GetSetContents() ([]string, error) { - switch set.Properties.Kind { + switch set.Kind { case HashSet: i := 0 contents := make([]string, len(set.IPPodKey)) @@ -143,7 +128,7 @@ func (set *IPSet) GetSetContents() ([]string, error) { } return contents, nil default: - return []string{}, fmt.Errorf("Unknown set type %s", set.Properties.Type.String()) + return []string{}, fmt.Errorf("Unknown set type %s", set.Type.String()) } } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index c0afb4deb7..9a8c15454f 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -7,7 +7,7 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/metrics" - "github.com/Azure/azure-container-networking/npm/util/errors" + npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" ) type IPSetManager struct { @@ -42,7 +42,7 @@ func (iMgr *IPSetManager) updateDirtyCache(setName string) { } iMgr.dirtyCaches[set.Name] = struct{}{} - if set.Properties.Kind == ListSet { + if set.Kind == ListSet { // TODO check if we will need to add all the member ipsets // also to the dirty cache list for _, member := range set.MemberIPSets { @@ -78,9 +78,9 @@ func (iMgr *IPSetManager) createIPSet(set *IPSet) error { } func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error { - // check if the IP is IPV$ family + // check if the IP is IPV4 family if net.ParseIP(ip).To4() == nil { - return errors.Errorf(errors.AppendIPSet, false, "IPV6 not supported") + return npmerrors.Errorf(npmerrors.AppendIPSet, false, "IPV6 not supported") } iMgr.Lock() defer iMgr.Unlock() @@ -95,8 +95,8 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error set = iMgr.setMap[updatedSet.Name] } - if set.Properties.Kind != HashSet { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a hash set", set.Name)) + if set.Kind != HashSet { + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a hash set", set.Name)) } cachedPodKey, ok := set.IPPodKey[ip] if ok { @@ -127,11 +127,11 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri for _, setName := range removeFromSets { set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } - if set.Properties.Kind != HashSet { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) + if set.Kind != HashSet { + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) } // in case the IP belongs to a new Pod, then ignore this Delete call as this might be stale @@ -161,26 +161,26 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { for _, setName := range setNames { if listName == setName { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("list %s cannot be added to itself", listName)) + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("list %s cannot be added to itself", listName)) } set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) } // Nested IPSets are only supported for windows // Check if we want to actually use that support - if set.Properties.Kind != HashSet { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) + if set.Kind != HashSet { + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } list, exists := iMgr.setMap[listName] // check if the Set exists if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } - if list.Properties.Kind != ListSet { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) + if list.Kind != ListSet { + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) } // check if Set is a member of List @@ -214,26 +214,26 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err for _, setName := range setNames { set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } - if set.Properties.Kind != HashSet { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) + if set.Kind != HashSet { + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a hash set", setName)) } // Nested IPSets are only supported for windows // Check if we want to actually use that support - if set.Properties.Kind != HashSet { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) + if set.Kind != HashSet { + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("member ipset %s is not a Set type and nestetd ipsets are not supported", setName)) } list, exists := iMgr.setMap[listName] // check if the Set exists if !exists { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s does not exist", listName)) } - if list.Properties.Kind != ListSet { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) + if list.Kind != ListSet { + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s is not a list set", listName)) } // check if Set is a member of List @@ -259,11 +259,11 @@ func (iMgr *IPSetManager) DeleteList(name string) error { defer iMgr.Unlock() set, exists := iMgr.setMap[name] // check if the Set exists if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) } if !set.CanBeDeleted() { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) } delete(iMgr.setMap, name) @@ -275,11 +275,11 @@ func (iMgr *IPSetManager) DeleteSet(name string) error { defer iMgr.Unlock() set, exists := iMgr.setMap[name] // check if the Set exists if !exists { - return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) + return npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", set.Name)) } if !set.CanBeDeleted() { - return errors.Errorf(errors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) + return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", set.Name)) } delete(iMgr.setMap, name) return nil diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index daf913ef7e..9e3354f461 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -47,6 +47,8 @@ func (iMgr *IPSetManager) applyIPSets(networkID string) error { } for _, policy := range network.Policies { + // TODO (vamsi) use NetPolicyType constant setpolicy for below check + // after updating HCSShim if policy.Type != "SetPolicy" { policyNetworkRequest.Policies = append(policyNetworkRequest.Policies, policy) } @@ -97,7 +99,7 @@ func (iMgr *IPSetManager) calculateNewSetPolicies(existingSets []string) (map[st return nil, err } setsToUpdate[setName] = setPol - if set.Properties.Kind == ListSet { + if set.Kind == ListSet { for _, memberSet := range set.MemberIPSets { // TODO check whats the name here, hashed or normal if _, ok := setsToUpdate[memberSet.Name]; ok { @@ -120,8 +122,8 @@ func isValidIPSet(set *IPSet) error { return fmt.Errorf("IPSet " + set.Name + " is missing Name") } - if set.Properties.Type == Unknown { - return fmt.Errorf("IPSet " + set.Properties.Type.String() + " is missing Type") + if set.Type == Unknown { + return fmt.Errorf("IPSet " + set.Type.String() + " is missing Type") } if set.HashedName == "" { @@ -132,7 +134,7 @@ func isValidIPSet(set *IPSet) error { } func getSetPolicyType(set *IPSet) SetPolicyType { - switch set.Properties.Kind { + switch set.Kind { case ListSet: return SetPolicyTypeNestedIpSet case HashSet: diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index ed768ce84d..55516aeba1 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -19,7 +19,9 @@ type NPMNetworkPolicy struct { RawNP *networkingv1.NetworkPolicy } -type ACLPolicy struct { // Iptable rules +// ACLPolicy equivalent to a single iptable rule in linux +// or a single HNS rule in windows +type ACLPolicy struct { // PolicyID is the rules name with a given network policy PolicyID string // Comment is the string attached to rule to identity its representation @@ -38,7 +40,7 @@ type ACLPolicy struct { // Iptable rules // DstPorts holds the destination port information DstPorts []Ports // Protocol is the value of traffic protocol - Protocol string + Protocol Protocol } // SetInfo helps capture additional details in a matchSet @@ -62,6 +64,8 @@ type Verdict string type Direction string +type Protocol string + const ( // Ingress when packet is entering a container Ingress Direction = "IN" @@ -74,4 +78,10 @@ const ( Allowed Verdict = "ALLOW" // Dropped is denying a flow Dropped Verdict = "DROP" + + TCP Protocol = "tcp" + UDP Protocol = "udp" + SCTP Protocol = "sctp" + ICMP Protocol = "icmp" + AnyProtocol Protocol = "any" ) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 368d7734b5..97fd05357b 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -2,11 +2,19 @@ package policies import ( "fmt" - "strings" "github.com/Microsoft/hcsshim/hcn" ) +var protocolNumMap = map[Protocol]string{ + TCP: "6", + UDP: "17", + ICMP: "1", + SCTP: "132", + // HNS thinks 256 as ANY protocol + AnyProtocol: "256", +} + func convertToAclSettings(acl ACLPolicy) (hcn.AclPolicySetting, error) { policySettings := hcn.AclPolicySetting{} for _, setInfo := range acl.SrcList { @@ -30,23 +38,6 @@ func getHCNDirection(direction Direction) hcn.DirectionType { return "" } -func getHCNProtocol(protocol string) string { - // TODO need to check the protocol number of SCTP - switch strings.ToLower(protocol) { - case "tcp": - return "6" - case "udp": - return "17" - case "icmp": - return "1" - case "sctp": - return "132" - default: - // HNS thinks 256 as ANY - return "256" - } -} - func getHCNAction(verdict Verdict) hcn.ActionType { switch verdict { case Allowed: From a5cbd0b3f2c274a41960a7d840f0488af3e63ffa Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 8 Sep 2021 16:27:24 -0700 Subject: [PATCH 18/26] Addressing more comments --- npm/pkg/dataplane/dataplane.go | 14 ++++++------- npm/pkg/dataplane/dataplane_linux.go | 2 -- .../dataplane/ipsets/ipsetmanager_linux.go | 3 ++- npm/pkg/dataplane/policies/policymanager.go | 12 +++++------ .../dataplane/policies/policymanager_linux.go | 6 +++--- .../dataplane/policies/policymanager_test.go | 20 +++++++++---------- .../policies/policymanager_windows.go | 6 +++--- 7 files changed, 31 insertions(+), 32 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 4d6f2e8915..34abaa51d7 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -95,18 +95,18 @@ func (dp *DataPlane) ApplyDataPlane() error { return dp.ipsetMgr.ApplyIPSets(dp.networkID) } -// AddPolicies takes in a translated NPMNetworkPolicy object and applies on dataplane -func (dp *DataPlane) AddPolicies(policies *policies.NPMNetworkPolicy) error { - return dp.policyMgr.AddPolicies(policies) +// AddPolicy takes in a translated NPMNetworkPolicy object and applies on dataplane +func (dp *DataPlane) AddPolicy(policies *policies.NPMNetworkPolicy) error { + return dp.policyMgr.AddPolicy(policies) } // RemovePolicies takes in network policy name and removes it from dataplane and cache -func (dp *DataPlane) RemovePolicies(policyName string) error { - return dp.policyMgr.RemovePolicies(policyName) +func (dp *DataPlane) RemovePolicy(policyName string) error { + return dp.policyMgr.RemovePolicy(policyName) } // UpdatePolicies takes in updated policy object, calculates the delta and applies changes // onto dataplane accordingly -func (dp *DataPlane) UpdatePolicies(policies *policies.NPMNetworkPolicy) error { - return dp.policyMgr.UpdatePolicies(policies) +func (dp *DataPlane) UpdatePolicy(policies *policies.NPMNetworkPolicy) error { + return dp.policyMgr.UpdatePolicy(policies) } diff --git a/npm/pkg/dataplane/dataplane_linux.go b/npm/pkg/dataplane/dataplane_linux.go index 620e2b911b..067028f09c 100644 --- a/npm/pkg/dataplane/dataplane_linux.go +++ b/npm/pkg/dataplane/dataplane_linux.go @@ -1,8 +1,6 @@ package dataplane import ( - "fmt" - "github.com/Azure/azure-container-networking/npm" "k8s.io/klog" ) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go index 93ec2896ca..f93d6d898a 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Azure/azure-container-networking/npm/util/errors" + "k8s.io/klog" ) func (iMgr *IPSetManager) applyIPSets(networkID string) error { @@ -13,7 +14,7 @@ func (iMgr *IPSetManager) applyIPSets(networkID string) error { return errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("member ipset %s does not exist", setName)) } - fmt.Printf(set.Name) + klog.Infof(set.Name) } return nil diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index b5ea4c325d..09046b419e 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -30,12 +30,12 @@ func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { return nil, nil } -func (pMgr *PolicyManager) AddPolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) AddPolicy(policy *NPMNetworkPolicy) error { pMgr.policyMap.Lock() defer pMgr.policyMap.Unlock() // Call actual dataplane function to apply changes - err := pMgr.addPolicies(policy) + err := pMgr.addPolicy(policy) if err != nil { return err } @@ -44,12 +44,12 @@ func (pMgr *PolicyManager) AddPolicies(policy *NPMNetworkPolicy) error { return nil } -func (pMgr *PolicyManager) RemovePolicies(name string) error { +func (pMgr *PolicyManager) RemovePolicy(name string) error { pMgr.policyMap.Lock() defer pMgr.policyMap.Unlock() // Call actual dataplane function to apply changes - err := pMgr.removePolicies(name) + err := pMgr.removePolicy(name) if err != nil { return err } @@ -59,13 +59,13 @@ func (pMgr *PolicyManager) RemovePolicies(name string) error { return nil } -func (pMgr *PolicyManager) UpdatePolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) UpdatePolicy(policy *NPMNetworkPolicy) error { pMgr.policyMap.Lock() defer pMgr.policyMap.Unlock() // check and update // Call actual dataplane function to apply changes - err := pMgr.updatePolicies(policy) + err := pMgr.updatePolicy(policy) if err != nil { return err } diff --git a/npm/pkg/dataplane/policies/policymanager_linux.go b/npm/pkg/dataplane/policies/policymanager_linux.go index 5d6dddfc88..ac1ef7b28d 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux.go +++ b/npm/pkg/dataplane/policies/policymanager_linux.go @@ -1,13 +1,13 @@ package policies -func (pMgr *PolicyManager) addPolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy) error { return nil } -func (pMgr *PolicyManager) removePolicies(name string) error { +func (pMgr *PolicyManager) removePolicy(name string) error { return nil } -func (pMgr *PolicyManager) updatePolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) updatePolicy(policy *NPMNetworkPolicy) error { return nil } diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index a09a817b95..77f7824225 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -7,33 +7,33 @@ func TestAddPolicy(t *testing.T) { netpol := NPMNetworkPolicy{} - err := pMgr.AddPolicies(&netpol) + err := pMgr.AddPolicy(&netpol) if err != nil { - t.Errorf("AddPolicies() returned error %s", err.Error()) + t.Errorf("AddPolicy() returned error %s", err.Error()) } } -func TestRemovePolicies(t *testing.T) { +func TestRemovePolicy(t *testing.T) { pMgr := NewPolicyManager() - err := pMgr.RemovePolicies("test") + err := pMgr.RemovePolicy("test") if err != nil { - t.Errorf("RemovePolicies() returned error %s", err.Error()) + t.Errorf("RemovePolicy() returned error %s", err.Error()) } } -func TestUpdatePolicies(t *testing.T) { +func TestUpdatePolicy(t *testing.T) { pMgr := NewPolicyManager() netpol := NPMNetworkPolicy{} - err := pMgr.AddPolicies(&netpol) + err := pMgr.AddPolicy(&netpol) if err != nil { - t.Errorf("UpdatePolicies() returned error %s", err.Error()) + t.Errorf("UpdatePolicy() returned error %s", err.Error()) } - err = pMgr.UpdatePolicies(&netpol) + err = pMgr.UpdatePolicy(&netpol) if err != nil { - t.Errorf("UpdatePolicies() returned error %s", err.Error()) + t.Errorf("UpdatePolicy() returned error %s", err.Error()) } } diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index 5d6dddfc88..ac1ef7b28d 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -1,13 +1,13 @@ package policies -func (pMgr *PolicyManager) addPolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy) error { return nil } -func (pMgr *PolicyManager) removePolicies(name string) error { +func (pMgr *PolicyManager) removePolicy(name string) error { return nil } -func (pMgr *PolicyManager) updatePolicies(policy *NPMNetworkPolicy) error { +func (pMgr *PolicyManager) updatePolicy(policy *NPMNetworkPolicy) error { return nil } From c921c1d7d1f353d768c4416b639af0cddb64dfc3 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 9 Sep 2021 14:57:08 -0700 Subject: [PATCH 19/26] resolving some comments --- npm/pkg/dataplane/ipsets/ipset.go | 2 +- npm/pkg/dataplane/policies/policy.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index cb262af75c..c1621ff686 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -36,7 +36,7 @@ type SetProperties struct { Kind SetKind } -type SetType int32 +type SetType int8 const ( // Unknown SetType diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 55516aeba1..f98ba3bdab 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -9,10 +9,10 @@ type NPMNetworkPolicy struct { Name string // PodSelectorIPSets holds all the IPSets generated from Pod Selector PodSelectorIPSets []*ipsets.IPSet - // OtherIPSets holds all IPSets generated from policy - // except for pod selector IPSets - OtherIPSets []*ipsets.IPSet - ACLs []*ACLPolicy + // RuleIPSets holds all IPSets generated from policy's rules + // and not from pod selector IPSets + RuleIPSets []*ipsets.IPSet + ACLs []*ACLPolicy // Making this a podKey instead should be // use NPMPod obj Pods []string From af6024724a68001de7f92082ce06be0f3d16e2e7 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 9 Sep 2021 14:57:52 -0700 Subject: [PATCH 20/26] resolving some comments --- npm/util/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/util/util.go b/npm/util/util.go index c7cf3d0c9f..4921187a5d 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -343,5 +343,5 @@ func CompareSlices(list1, list2 []string) bool { } func SliceToString(list []string) string { - return strings.Join(list[:], SetPolicyDelimiter) + return strings.Join(list, SetPolicyDelimiter) } From 987f82004d4afbe72dfe3e52abdc285c0da1484b Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 9 Sep 2021 15:03:14 -0700 Subject: [PATCH 21/26] fixing some comments --- npm/pkg/dataplane/dataplane.go | 4 ++-- npm/pkg/dataplane/ipsets/ipset.go | 10 ++++++---- npm/pkg/dataplane/ipsets/ipsetmanager.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 34abaa51d7..ec2b53d231 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -17,8 +17,8 @@ type DataPlane struct { type NPMEndpoint struct { Name string ID string - // Using a map to emulate set and value as struct{} for - // minimal memory consumption + // Map with Key as Network Policy name to to emulate set + // and value as struct{} for minimal memory consumption NetPolReference map[string]struct{} } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index c1621ff686..99e52eaa34 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -95,11 +95,13 @@ func NewIPSet(name string, setType SetType) *IPSet { Type: setType, Kind: getSetKind(setType), }, - // Using a map to emulate set and value as struct{} for - // minimal memory consumption + // Map with Key as Network Policy name to to emulate set + // and value as struct{} for minimal memory consumption SelectorReference: make(map[string]struct{}), - NetPolReference: make(map[string]struct{}), - IpsetReferCount: 0, + // Map with Key as Network Policy name to to emulate set + // and value as struct{} for minimal memory consumption + NetPolReference: make(map[string]struct{}), + IpsetReferCount: 0, } if set.Kind == HashSet { set.IPPodKey = make(map[string]string) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 9a8c15454f..93d73c4681 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -12,8 +12,8 @@ import ( type IPSetManager struct { setMap map[string]*IPSet - // Using a map to emulate set and value as struct{} for - // minimal memory consumption + // Map with Key as IPSet name to to emulate set + // and value as struct{} for minimal memory consumption dirtyCaches map[string]struct{} sync.Mutex } From ae527f4ec6e4d0f0c5886241e0dbb65a4ac0cb58 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 10 Sep 2021 13:15:30 -0700 Subject: [PATCH 22/26] removing lock on policymap --- npm/pkg/dataplane/policies/policymanager.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 09046b419e..c228af8a6d 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -1,9 +1,6 @@ package policies -import "sync" - type PolicyMap struct { - sync.RWMutex cache map[string]*NPMNetworkPolicy } @@ -20,9 +17,6 @@ func NewPolicyManager() PolicyManager { } func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { - pMgr.policyMap.RLock() - defer pMgr.policyMap.RUnlock() - if policy, ok := pMgr.policyMap.cache[name]; ok { return policy, nil } @@ -31,9 +25,6 @@ func (pMgr *PolicyManager) GetPolicy(name string) (*NPMNetworkPolicy, error) { } func (pMgr *PolicyManager) AddPolicy(policy *NPMNetworkPolicy) error { - pMgr.policyMap.Lock() - defer pMgr.policyMap.Unlock() - // Call actual dataplane function to apply changes err := pMgr.addPolicy(policy) if err != nil { @@ -45,9 +36,6 @@ func (pMgr *PolicyManager) AddPolicy(policy *NPMNetworkPolicy) error { } func (pMgr *PolicyManager) RemovePolicy(name string) error { - pMgr.policyMap.Lock() - defer pMgr.policyMap.Unlock() - // Call actual dataplane function to apply changes err := pMgr.removePolicy(name) if err != nil { @@ -60,9 +48,6 @@ func (pMgr *PolicyManager) RemovePolicy(name string) error { } func (pMgr *PolicyManager) UpdatePolicy(policy *NPMNetworkPolicy) error { - pMgr.policyMap.Lock() - defer pMgr.policyMap.Unlock() - // check and update // Call actual dataplane function to apply changes err := pMgr.updatePolicy(policy) From d0eb0c688fe6c40945901b8e24390490c6dc89c4 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 10 Sep 2021 13:34:07 -0700 Subject: [PATCH 23/26] fixing some golints --- npm/pkg/dataplane/dataplane.go | 4 ++-- npm/pkg/dataplane/ipsets/ipsetmanager.go | 1 - npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 3 --- npm/pkg/dataplane/policies/policy_windows.go | 2 -- 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index ec2b53d231..f5c2482bfb 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -100,12 +100,12 @@ func (dp *DataPlane) AddPolicy(policies *policies.NPMNetworkPolicy) error { return dp.policyMgr.AddPolicy(policies) } -// RemovePolicies takes in network policy name and removes it from dataplane and cache +// RemovePolicy takes in network policy name and removes it from dataplane and cache func (dp *DataPlane) RemovePolicy(policyName string) error { return dp.policyMgr.RemovePolicy(policyName) } -// UpdatePolicies takes in updated policy object, calculates the delta and applies changes +// UpdatePolicy takes in updated policy object, calculates the delta and applies changes // onto dataplane accordingly func (dp *DataPlane) UpdatePolicy(policies *policies.NPMNetworkPolicy) error { return dp.policyMgr.UpdatePolicy(policies) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 93d73c4681..5806f76777 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -49,7 +49,6 @@ func (iMgr *IPSetManager) updateDirtyCache(setName string) { iMgr.dirtyCaches[member.Name] = struct{}{} } } - return } func (iMgr *IPSetManager) clearDirtyCache() { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 624554c261..36e5e5917c 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -103,7 +103,6 @@ func TestRemoveFromList(t *testing.T) { if err != nil { t.Errorf("RemoveFromList() returned error %s", err.Error()) } - } func TestRemoveFromListMissing(t *testing.T) { @@ -112,7 +111,6 @@ func TestRemoveFromListMissing(t *testing.T) { if err == nil { t.Errorf("RemoveFromList() did not return error") } - } func TestDeleteList(t *testing.T) { @@ -128,7 +126,6 @@ func TestDeleteList(t *testing.T) { if err != nil { t.Errorf("DeleteList() returned error %s", err.Error()) } - } func TestDeleteSet(t *testing.T) { diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 97fd05357b..5b6d8f6d04 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -32,8 +32,6 @@ func getHCNDirection(direction Direction) hcn.DirectionType { return hcn.DirectionTypeIn case Egress: return hcn.DirectionTypeOut - case Both: - return "" } return "" } From 607ba4342503a7c4861a58cd1566f038805bf6bf Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 10 Sep 2021 16:05:59 -0700 Subject: [PATCH 24/26] merging with master --- npm/pkg/dataplane/ipsets/ipsetmanager.go | 14 +++++--------- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 1 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 5806f76777..6aa36ae163 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -72,7 +72,7 @@ func (iMgr *IPSetManager) createIPSet(set *IPSet) error { // append the cache if dataplane specific function // return nil as error iMgr.setMap[set.Name] = set - + metrics.IncNumIPSets() return nil } @@ -113,8 +113,7 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSet, ip, podKey string) error iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet - metrics.NumIPSetEntries.Inc() - metrics.IncIPSetInventory(set.Name) + metrics.AddEntryToIPSet(set.Name) } return nil @@ -147,8 +146,7 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri iMgr.updateDirtyCache(set.Name) // Update metrics of the IpSet - metrics.NumIPSetEntries.Dec() - metrics.DecIPSetInventory(setName) + metrics.RemoveEntryFromIPSet(setName) } return nil @@ -198,8 +196,7 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { list.AddMemberIPSet(set) set.IncIpsetReferCount() // Update metrics of the IpSet - metrics.NumIPSetEntries.Inc() - metrics.IncIPSetInventory(setName) + metrics.AddEntryToIPSet(listName) } iMgr.updateDirtyCache(listName) @@ -245,8 +242,7 @@ func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) err delete(list.MemberIPSets, setName) set.DecIpsetReferCount() // Update metrics of the IpSet - metrics.NumIPSetEntries.Dec() - metrics.DecIPSetInventory(setName) + metrics.RemoveEntryFromIPSet(listName) } iMgr.updateDirtyCache(listName) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 36e5e5917c..9749e526e3 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -22,7 +22,6 @@ func TestAddToSet(t *testing.T) { iMgr := NewIPSetManager() set := NewIPSet("Test", NameSpace) - metrics.NumIPSetEntries.Set(0) fmt.Println(set.Name) err := iMgr.AddToSet([]*IPSet{set}, "10.0.0.0", "test") if err != nil { From 942f894504653c821ea00abc23a5d7dc9f06066c Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 10 Sep 2021 16:26:09 -0700 Subject: [PATCH 25/26] fixingsome wrap checks --- npm/npm_test.go | 10 ---- npm/pkg/dataplane/dataplane.go | 74 ++++++++++++++++++++++++----- npm/pkg/dataplane/dataplane_test.go | 2 + 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/npm/npm_test.go b/npm/npm_test.go index 918b968aa2..e195421db7 100644 --- a/npm/npm_test.go +++ b/npm/npm_test.go @@ -9,7 +9,6 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "k8s.io/client-go/tools/cache" "k8s.io/utils/exec" - utilexec "k8s.io/utils/exec" ) // To indicate the object is needed to be DeletedFinalStateUnknown Object @@ -29,15 +28,6 @@ func getKey(obj interface{}, t *testing.T) string { return key } -func newNPMgr(t *testing.T, exec utilexec.Interface) *NetworkPolicyManager { - npMgr := &NetworkPolicyManager{ - ipsMgr: ipsm.NewIpsetManager(exec), - TelemetryEnabled: false, - } - - return npMgr -} - func TestMain(m *testing.M) { metrics.InitializeAll() exec := exec.New() diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index f5c2482bfb..ac7ea5ca9b 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -1,6 +1,8 @@ package dataplane import ( + "fmt" + "github.com/Azure/azure-container-networking/npm" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" @@ -43,47 +45,79 @@ func (dp *DataPlane) ResetDataPlane() error { // CreateIPSet takes in a set object and updates local cache with this set func (dp *DataPlane) CreateIPSet(set *ipsets.IPSet) error { - return dp.ipsetMgr.CreateIPSet(set) + err := dp.ipsetMgr.CreateIPSet(set) + if err != nil { + return fmt.Errorf("[DataPlane] error while creating set: %w", err) + } + return nil } // DeleteSet checks for members and references of the given "set" type ipset // if not used then will delete it from cache func (dp *DataPlane) DeleteSet(name string) error { - return dp.ipsetMgr.DeleteSet(name) + err := dp.ipsetMgr.DeleteSet(name) + if err != nil { + return fmt.Errorf("[DataPlane] error while deleting set: %w", err) + } + return nil } // DeleteList sanity checks and deletes a list ipset func (dp *DataPlane) DeleteList(name string) error { - return dp.ipsetMgr.DeleteList(name) + err := dp.ipsetMgr.DeleteList(name) + if err != nil { + return fmt.Errorf("[DataPlane] error while deleting list: %w", err) + } + return nil } // AddToSet takes in a list of IPset objects along with IP member // and then updates it local cache func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSet, ip, podKey string) error { - return dp.ipsetMgr.AddToSet(setNames, ip, podKey) + err := dp.ipsetMgr.AddToSet(setNames, ip, podKey) + if err != nil { + return fmt.Errorf("[DataPlane] error while adding to set: %w", err) + } + return nil } // RemoveFromSet takes in list of setnames from which a given IP member should be // removed and will update the local cache func (dp *DataPlane) RemoveFromSet(setNames []string, ip, podKey string) error { - return dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) + err := dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) + if err != nil { + return fmt.Errorf("[DataPlane] error while removing from set: %w", err) + } + return nil } // AddToList takes a list name and list of sets which are to be added as members // to given list func (dp *DataPlane) AddToList(listName string, setNames []string) error { - return dp.ipsetMgr.AddToList(listName, setNames) + err := dp.ipsetMgr.AddToList(listName, setNames) + if err != nil { + return fmt.Errorf("[DataPlane] error while adding to list: %w", err) + } + return nil } // RemoveFromList takes a list name and list of sets which are to be removed as members // to given list func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { - return dp.ipsetMgr.RemoveFromList(listName, setNames) + err := dp.ipsetMgr.RemoveFromList(listName, setNames) + if err != nil { + return fmt.Errorf("[DataPlane] error while removing from list: %w", err) + } + return nil } // UpdatePod is to be called by pod_controller ONLY when a new pod is CREATED. func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { - return dp.updatePod(pod) + err := dp.updatePod(pod) + if err != nil { + return fmt.Errorf("[DataPlane] error while updating pod: %w", err) + } + return nil } // ApplyDataPlane all the IPSet operations just update cache and update a dirty ipset structure, @@ -92,21 +126,37 @@ func (dp *DataPlane) UpdatePod(pod *npm.NpmPod) error { // and accordingly makes changes in dataplane. This function helps emulate a single call to // dataplane instead of multiple ipset operations calls ipset operations calls to dataplane func (dp *DataPlane) ApplyDataPlane() error { - return dp.ipsetMgr.ApplyIPSets(dp.networkID) + err := dp.ipsetMgr.ApplyIPSets(dp.networkID) + if err != nil { + return fmt.Errorf("[DataPlane] error while applying IPSets: %w", err) + } + return nil } // AddPolicy takes in a translated NPMNetworkPolicy object and applies on dataplane func (dp *DataPlane) AddPolicy(policies *policies.NPMNetworkPolicy) error { - return dp.policyMgr.AddPolicy(policies) + err := dp.policyMgr.AddPolicy(policies) + if err != nil { + return fmt.Errorf("[DataPlane] error while adding policy: %w", err) + } + return nil } // RemovePolicy takes in network policy name and removes it from dataplane and cache func (dp *DataPlane) RemovePolicy(policyName string) error { - return dp.policyMgr.RemovePolicy(policyName) + err := dp.policyMgr.RemovePolicy(policyName) + if err != nil { + return fmt.Errorf("[DataPlane] error while removing policy: %w", err) + } + return nil } // UpdatePolicy takes in updated policy object, calculates the delta and applies changes // onto dataplane accordingly func (dp *DataPlane) UpdatePolicy(policies *policies.NPMNetworkPolicy) error { - return dp.policyMgr.UpdatePolicy(policies) + err := dp.policyMgr.UpdatePolicy(policies) + if err != nil { + return fmt.Errorf("[DataPlane] error while updating policy: %w", err) + } + return nil } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index f844dcc888..5be84e2377 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -3,10 +3,12 @@ package dataplane import ( "testing" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" ) func TestNewDataPlane(t *testing.T) { + metrics.InitializeAll() dp := NewDataPlane() if dp == nil { From 50315e6105dc156a6a029d7d39515808124ed3e1 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 10 Sep 2021 16:56:38 -0700 Subject: [PATCH 26/26] fixing lints --- npm/pkg/dataplane/ipsets/ipset.go | 30 ++++++++++++++++------------ npm/pkg/dataplane/policies/policy.go | 13 ++++++++---- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 99e52eaa34..5b06ad340c 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -1,7 +1,7 @@ package ipsets import ( - "fmt" + "errors" "github.com/Azure/azure-container-networking/npm/util" ) @@ -62,17 +62,21 @@ const ( CIDRBlocks SetType = 8 ) -var setTypeName = map[SetType]string{ - Unknown: "Unknown", - NameSpace: "NameSpace", - KeyLabelOfNameSpace: "KeyLabelOfNameSpace", - KeyValueLabelOfNameSpace: "KeyValueLabelOfNameSpace", - KeyLabelOfPod: "KeyLabelOfPod", - KeyValueLabelOfPod: "KeyValueLabelOfPod", - NamedPorts: "NamedPorts", - NestedLabelOfPod: "NestedLabelOfPod", - CIDRBlocks: "CIDRBlocks", -} +var ( + setTypeName = map[SetType]string{ + Unknown: "Unknown", + NameSpace: "NameSpace", + KeyLabelOfNameSpace: "KeyLabelOfNameSpace", + KeyValueLabelOfNameSpace: "KeyValueLabelOfNameSpace", + KeyLabelOfPod: "KeyLabelOfPod", + KeyValueLabelOfPod: "KeyValueLabelOfPod", + NamedPorts: "NamedPorts", + NestedLabelOfPod: "NestedLabelOfPod", + CIDRBlocks: "CIDRBlocks", + } + // ErrIPSetInvalidKind is returned when IPSet kind is invalid + ErrIPSetInvalidKind = errors.New("Invalid IPSet Kind") +) func (x SetType) String() string { return setTypeName[x] @@ -130,7 +134,7 @@ func (set *IPSet) GetSetContents() ([]string, error) { } return contents, nil default: - return []string{}, fmt.Errorf("Unknown set type %s", set.Type.String()) + return []string{}, ErrIPSetInvalidKind } } diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index f98ba3bdab..064a1a0fee 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -79,9 +79,14 @@ const ( // Dropped is denying a flow Dropped Verdict = "DROP" - TCP Protocol = "tcp" - UDP Protocol = "udp" - SCTP Protocol = "sctp" - ICMP Protocol = "icmp" + // TCP Protocol + TCP Protocol = "tcp" + // UDP Protocol + UDP Protocol = "udp" + // SCTP Protocol + SCTP Protocol = "sctp" + // ICMP Protocol + ICMP Protocol = "icmp" + // AnyProtocol can be used for all other protocols AnyProtocol Protocol = "any" )