From 321798cfb120b6169785d9bcad3a5c1197256423 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 18 Jan 2022 13:11:37 -0800 Subject: [PATCH 1/6] fix: [NPM] Windows Dataplane logic fix for src and dst ports --- npm/pkg/dataplane/dataplane.go | 2 +- npm/pkg/dataplane/dataplane_windows.go | 1 + npm/pkg/dataplane/policies/policy_windows.go | 20 ++++++++++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 19ad894c07..ff528cb350 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -256,7 +256,7 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { klog.Infof("[DataPlane] Update Policy called for %s", policy.PolicyKey) ok := dp.policyMgr.PolicyExists(policy.PolicyKey) if !ok { - klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policy.PolicyKey) + klog.Infof("[DataPlane] Policy %s is not found.", policy.PolicyKey) return dp.AddPolicy(policy) } diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 7ec803337d..e39d305902 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -112,6 +112,7 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { // Check if pod is already present in cache endpoint, ok := dp.endpointCache[pod.PodIP] if !ok { + // TODO ignore this err and delete the pod if the Pod endpoint is not found. return fmt.Errorf("[DataPlane] did not find endpoint with IPaddress %s", pod.PodIP) } diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 0dcec9d68e..c652a3c476 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -94,19 +94,23 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { // HNS has confusing Local and Remote address defintions // For Traffic Direction INGRESS - // LocalAddresses = Source IPs + // LocalAddresses = Source IPs // RemoteAddresses = Destination IPs - // For Traffic Direction EGRESS - // LocalAddresses = Destination IPs - // RemoteAddresses = Source IPs + // LocalPorts = Destination Ports + // RemotePorts = Source Ports policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr - policySettings.RemotePorts = dstPortStr - policySettings.LocalPorts = "" + policySettings.RemotePorts = "" + policySettings.LocalPorts = dstPortStr if policySettings.Direction == hcn.DirectionTypeOut { + // For Traffic Direction EGRESS + // LocalAddresses = Destination IPs + // RemoteAddresses = Source IPs + // LocalPorts = Source Ports + // RemotePorts = Destination Ports policySettings.LocalAddresses = dstListStr - policySettings.LocalPorts = dstPortStr - policySettings.RemotePorts = "" + policySettings.LocalPorts = "" + policySettings.RemotePorts = dstPortStr policySettings.RemoteAddresses = srcListStr } From c4f919a52e8357511199b5f0b07df31f109c70f8 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 18 Jan 2022 13:20:20 -0800 Subject: [PATCH 2/6] ignoring endpoint missing error for updatePod --- npm/pkg/dataplane/dataplane_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index e39d305902..a4c89e2607 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -113,7 +113,8 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { endpoint, ok := dp.endpointCache[pod.PodIP] if !ok { // TODO ignore this err and delete the pod if the Pod endpoint is not found. - return fmt.Errorf("[DataPlane] did not find endpoint with IPaddress %s", pod.PodIP) + klog.Errorf("[DataPlane] did not find endpoint with IPaddress %s", pod.PodIP) + return nil } if endpoint.IP != pod.PodIP { From af57aa926ebb69bdeacda123c5e023d3a42463c4 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 18 Jan 2022 14:19:48 -0800 Subject: [PATCH 3/6] ignoring endpoint missing error for updatePod --- npm/pkg/dataplane/dataplane_windows.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index a4c89e2607..f09d7e45b2 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -112,8 +112,9 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { // Check if pod is already present in cache endpoint, ok := dp.endpointCache[pod.PodIP] if !ok { - // TODO ignore this err and delete the pod if the Pod endpoint is not found. - klog.Errorf("[DataPlane] did not find endpoint with IPaddress %s", pod.PodIP) + // ignore this err and pod endpoint will be deleted in ApplyDP + // if the endpoint is not found, it means the pod is not part of this node or pod got deleted. + klog.Warningf("[DataPlane] did not find endpoint with IPaddress %s", pod.PodIP) return nil } From ecd69612f6168ecb445933bb4b1113fd3ad4fbed Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 18 Jan 2022 16:53:19 -0800 Subject: [PATCH 4/6] correcting the logic for egress --- npm/pkg/dataplane/policies/policy_windows.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index c652a3c476..503d396013 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -94,24 +94,25 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { // HNS has confusing Local and Remote address defintions // For Traffic Direction INGRESS - // LocalAddresses = Source IPs - // RemoteAddresses = Destination IPs + // LocalAddresses = Source Sets/IPs + // RemoteAddresses = Destination Sets/IPs // LocalPorts = Destination Ports // RemotePorts = Source Ports + + // For Traffic Direction EGRESS + // LocalAddresses = Source Sets/IPs + // RemoteAddresses = Destination Sets/IPs + // LocalPorts = Source Ports + // RemotePorts = Destination Ports policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr + + // Switch ports based on direction policySettings.RemotePorts = "" policySettings.LocalPorts = dstPortStr if policySettings.Direction == hcn.DirectionTypeOut { - // For Traffic Direction EGRESS - // LocalAddresses = Destination IPs - // RemoteAddresses = Source IPs - // LocalPorts = Source Ports - // RemotePorts = Destination Ports - policySettings.LocalAddresses = dstListStr policySettings.LocalPorts = "" policySettings.RemotePorts = dstPortStr - policySettings.RemoteAddresses = srcListStr } return policySettings, nil From 1ee972cc239063950b710f49be0b72857cd40758 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 19 Jan 2022 13:41:48 -0800 Subject: [PATCH 5/6] updating comment --- npm/pkg/dataplane/policies/policy_windows.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 503d396013..1080f16640 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -94,16 +94,25 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { // HNS has confusing Local and Remote address defintions // For Traffic Direction INGRESS - // LocalAddresses = Source Sets/IPs - // RemoteAddresses = Destination Sets/IPs + // LocalAddresses = Source Sets + // RemoteAddresses = Destination Sets // LocalPorts = Destination Ports // RemotePorts = Source Ports // For Traffic Direction EGRESS - // LocalAddresses = Source Sets/IPs - // RemoteAddresses = Destination Sets/IPs + // LocalAddresses = Source Sets + // RemoteAddresses = Destination Sets // LocalPorts = Source Ports // RemotePorts = Destination Ports + + // If we use IPs in ACLs, then INGRESS mapping is same, but EGRESS mapping will change to below + // For Traffic Direction INGRESS + // LocalAddresses = Source IPs + // RemoteAddresses = Destination IPs + // For Traffic Direction EGRESS + // LocalAddresses = Destination IPs + // RemoteAddresses = Source IPs + policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr From 25cf46ba656ee071230b98dc7b87a5d0b6a359cc Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Wed, 19 Jan 2022 13:42:38 -0800 Subject: [PATCH 6/6] updating comment --- npm/pkg/dataplane/policies/policy_windows.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 1080f16640..2705b733f4 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -94,14 +94,14 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { // HNS has confusing Local and Remote address defintions // For Traffic Direction INGRESS - // LocalAddresses = Source Sets - // RemoteAddresses = Destination Sets + // LocalAddresses = Source Sets + // RemoteAddresses = Destination Sets // LocalPorts = Destination Ports // RemotePorts = Source Ports // For Traffic Direction EGRESS - // LocalAddresses = Source Sets - // RemoteAddresses = Destination Sets + // LocalAddresses = Source Sets + // RemoteAddresses = Destination Sets // LocalPorts = Source Ports // RemotePorts = Destination Ports