From 3179b1f0e43a8cb9145d837397182b4a9b17dadf Mon Sep 17 00:00:00 2001 From: Riya Date: Sun, 2 Nov 2025 23:33:36 +0000 Subject: [PATCH 01/10] added logic to bypass ipsets for /32 cidrs with npm lite --- .../translation/translatePolicy.go | 72 ++++++-- .../translation/translatePolicy_test.go | 163 ------------------ npm/pkg/dataplane/policies/policy.go | 8 +- npm/pkg/dataplane/policies/policy_windows.go | 13 +- npm/util/util.go | 4 + 5 files changed, 80 insertions(+), 180 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 9b029b4616..276ae945b9 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -3,6 +3,7 @@ package translation import ( "errors" "fmt" + "strings" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" @@ -362,6 +363,53 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return nil } +func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error { + ip := strings.TrimSuffix(cidr, "/32") + if len(ports) == 0 { + acl := policies.NewACLPolicy(policies.Allowed, direction) + // bypasses ipset creation for /32 cidrs and directly creates an acl with the cidr + if direction == policies.Ingress { + acl.SrcDirectIPs = []string{ip} + } else { + acl.DstDirectIPs = []string{ip} + } + npmNetPol.ACLs = append(npmNetPol.ACLs, acl) + return nil + } else { + // handle each port separately + for i := range ports { + portKind, err := portType(ports[i]) + if err != nil { + return err + } + + err = checkForNamedPortType(portKind, npmLiteToggle) + if err != nil { + return err + } + + acl := policies.NewACLPolicy(policies.Allowed, direction) + + // Set direct IP based on direction + if direction == policies.Ingress { + acl.SrcDirectIPs = []string{ip} + } else { + acl.DstDirectIPs = []string{ip} + } + + // Handle ports + if portKind == numericPortType { + portInfo, protocol := numericPortRule(&ports[i]) + acl.DstPorts = portInfo + acl.Protocol = policies.Protocol(protocol) + } + npmNetPol.ACLs = append(npmNetPol.ACLs, acl) + + } + } + return nil +} + // translateRule translates ingress or egress rules and update npmNetPol object. func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, @@ -405,6 +453,16 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, // #2.1 Handle IPBlock and port if exist if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { + // add logic that if the peer is only IPBlock and npm lite is enabled and is a /32 cidr block + // then skip creating IpBlockIPSet + if npmLiteToggle && util.IsCIDR32(peer.IPBlock.CIDR) { + err = directPeerAndPortRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) + if err != nil { + return err + } + continue + } + ipBlockIPSet, ipBlockSetInfo, err := ipBlockRule(netPolName, npmNetPol.Namespace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) if err != nil { return err @@ -417,12 +475,6 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, } } - // if npm lite is configured, check network policy only consists of CIDR blocks - err := npmLiteValidPolicy(peer, npmLiteToggle) - if err != nil { - return err - } - // Do not need to run below code to translate PodSelector and NamespaceSelector // since IPBlock field is exclusive in NetworkPolicyPeer (i.e., peer in this code). @@ -642,14 +694,6 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy, npmLiteToggle bool) (*po return npmNetPol, nil } -// validates only CIDR based peer is present + no combination of CIDR with pod/namespace selectors are present -func npmLiteValidPolicy(peer networkingv1.NetworkPolicyPeer, npmLiteEnabled bool) error { - if npmLiteEnabled && (peer.PodSelector != nil || peer.NamespaceSelector != nil) { - return ErrUnsupportedNonCIDR - } - return nil -} - func checkForNamedPortType(portKind netpolPortType, npmLiteToggle bool) error { if npmLiteToggle && portKind == namedPortType { return ErrUnsupportedNonCIDR diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index dc49c7bec3..d308bba77f 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -2921,169 +2921,6 @@ func TestEgressPolicy(t *testing.T) { } } -func TestNpmLiteCidrPolicy(t *testing.T) { - // Test 1) Npm lite enabled, CIDR + Namespace label Peers, returns error - // Test 2) NPM lite disabled, CIDR + Namespace label Peers, returns no error - // Test 3) Npm Lite enabled, CIDR Peers , returns no error - // Test 4) NPM Lite enabled, Combination of CIDR + Label in same peer, returns an error - // test 5) NPM Lite enabled, no peer, returns no error - // test 6) NPM Lite enabled, no cidr, no peer, only ports + protocol - - port8000 := intstr.FromInt(8000) - tcp := v1.ProtocolTCP - tests := []struct { - name string - targetSelector *metav1.LabelSelector - ports []networkingv1.NetworkPolicyPort - peersFrom []networkingv1.NetworkPolicyPeer - peersTo []networkingv1.NetworkPolicyPeer - npmLiteEnabled bool - wantErr bool - }{ - { - name: "CIDR + port + namespace", - targetSelector: nil, - ports: []networkingv1.NetworkPolicyPort{ - { - Protocol: &tcp, - Port: &port8000, - }, - }, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: true, - }, - { - name: "cidr + namespace label + disabledLite ", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: false, - wantErr: false, - }, - { - name: "CIDR Only", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - { - name: "CIDR + namespace labels", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/17", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: true, - }, - { - name: "no peers", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{}, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - { - name: "port only", - targetSelector: nil, - ports: []networkingv1.NetworkPolicyPort{ - { - Protocol: &tcp, - Port: &port8000, - }, - }, - peersFrom: []networkingv1.NetworkPolicyPeer{}, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - // run the function passing in peers and a flag indicating whether npm lite is enabled - var err error - for _, peer := range tt.peersFrom { - err = npmLiteValidPolicy(peer, tt.npmLiteEnabled) - if err != nil { - break - } - } - if tt.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - }) - } -} - func TestCheckForNamedPortType(t *testing.T) { port8000 := intstr.FromInt(8000) namedPort := intstr.FromString("namedPort") diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 646a03633a..d36b3530c5 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -114,6 +114,10 @@ type ACLPolicy struct { SrcList []SetInfo // DstList destination IPSets condition setinfos DstList []SetInfo + // SrcDirectIPs holds direct IPs for source matching (used for /32 CIDRs on Windows with npm lite enabled) + SrcDirectIPs []string + // DstDirectIPs holds direct IPs for destination matching (used for /32 CIDRs on Windows with npm lite enabled) + DstDirectIPs []string // Target defines a target in iptables for linux. i,e, Mark, Accept, Drop // in windows, this is either ALLOW or DENY Target Verdict @@ -282,7 +286,9 @@ func translatedIPSetsToString(items []*ipsets.TranslatedIPSet) string { // Included is false when match set have "!". // MatchType captures match direction flags. // For example match set in linux: -// ! azure-npm-123 src +// +// ! azure-npm-123 src +// // "!" this indicates a negative match (Included is false) of an azure-npm-123 // MatchType is "src" type SetInfo struct { diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 5f4fbd16ff..0f9ed9447c 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -3,6 +3,7 @@ package policies import ( "errors" "fmt" + "strings" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" @@ -101,8 +102,16 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // policySettings.RuleType = hcn.RuleTypeSwitch // ACLPolicy settings uses ID field of SetPolicy in LocalAddresses or RemoteAddresses - srcListStr := getAddrListFromSetInfo(acl.SrcList) - dstListStr := getAddrListFromSetInfo(acl.DstList) + var srcListStr, dstListStr string + // Check if we have direct IPs (NPM Lite /32 bypass) + if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 { + srcListStr = strings.Join(acl.SrcDirectIPs, ",") + dstListStr = strings.Join(acl.DstDirectIPs, ",") + } else { + // Original IPSet-based approach + srcListStr = getAddrListFromSetInfo(acl.SrcList) + dstListStr = getAddrListFromSetInfo(acl.DstList) + } dstPortStr := getPortStrFromPorts(acl.DstPorts) // HNS has confusing Local and Remote address defintions diff --git a/npm/util/util.go b/npm/util/util.go index 8d3f73e88c..4181e3c550 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -359,3 +359,7 @@ func NodeIP() (string, error) { return nodeIP, nil } + +func IsCIDR32(cidr string) bool { + return strings.HasSuffix(cidr, "/32") +} From c32653beae4ca5f2073a6fa62b103d903eaffdd3 Mon Sep 17 00:00:00 2001 From: Riya Date: Tue, 4 Nov 2025 23:27:47 +0000 Subject: [PATCH 02/10] removed logic to only look at /32 pod cidrs and allow all pod cidr --- npm/pkg/controlplane/translation/translatePolicy.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 276ae945b9..8f102d6acb 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -3,7 +3,6 @@ package translation import ( "errors" "fmt" - "strings" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" @@ -364,14 +363,13 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di } func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error { - ip := strings.TrimSuffix(cidr, "/32") if len(ports) == 0 { acl := policies.NewACLPolicy(policies.Allowed, direction) // bypasses ipset creation for /32 cidrs and directly creates an acl with the cidr if direction == policies.Ingress { - acl.SrcDirectIPs = []string{ip} + acl.SrcDirectIPs = []string{cidr} } else { - acl.DstDirectIPs = []string{ip} + acl.DstDirectIPs = []string{cidr} } npmNetPol.ACLs = append(npmNetPol.ACLs, acl) return nil @@ -392,9 +390,9 @@ func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction polic // Set direct IP based on direction if direction == policies.Ingress { - acl.SrcDirectIPs = []string{ip} + acl.SrcDirectIPs = []string{cidr} } else { - acl.DstDirectIPs = []string{ip} + acl.DstDirectIPs = []string{cidr} } // Handle ports @@ -455,7 +453,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, if len(peer.IPBlock.CIDR) > 0 { // add logic that if the peer is only IPBlock and npm lite is enabled and is a /32 cidr block // then skip creating IpBlockIPSet - if npmLiteToggle && util.IsCIDR32(peer.IPBlock.CIDR) { + if npmLiteToggle { err = directPeerAndPortRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) if err != nil { return err From 0f96f04b2e076a21af1edcddb8aad0b64ff86eeb Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 22:01:05 +0000 Subject: [PATCH 03/10] updated code specific to direct ip logic --- npm/pkg/dataplane/policies/policy_windows.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 0f9ed9447c..53fa97e899 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -135,6 +135,18 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // LocalAddresses = Destination IPs // RemoteAddresses = Source IPs + // if direct IPs are used, we leave local addresses to be an empty string + if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 { + policySettings.LocalAddresses = "" + if policySettings.Direction == hcn.DirectionTypeOut { + // EGRESS: Remote = Destination IPs from policy + policySettings.RemoteAddresses = dstListStr + } else { + // INGRESS: Remote = Source IPs from policy + policySettings.RemoteAddresses = srcListStr + } + } + policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr From af8d266cd186b7fb78cd531e16fa9762930f3569 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 22:01:35 +0000 Subject: [PATCH 04/10] fixed if else logic --- npm/pkg/dataplane/policies/policy_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 53fa97e899..7fc4a6b6e0 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -145,11 +145,11 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // INGRESS: Remote = Source IPs from policy policySettings.RemoteAddresses = srcListStr } + } else { + policySettings.LocalAddresses = srcListStr + policySettings.RemoteAddresses = dstListStr } - policySettings.LocalAddresses = srcListStr - policySettings.RemoteAddresses = dstListStr - // Switch ports based on direction policySettings.RemotePorts = "" policySettings.LocalPorts = dstPortStr From 48168b191d9608f5d1d85d06530557c5e87c0d41 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 22:08:07 +0000 Subject: [PATCH 05/10] added error for named port --- npm/pkg/controlplane/translation/translatePolicy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 8f102d6acb..5b7d8112e8 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -396,6 +396,9 @@ func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction polic } // Handle ports + if portKind == namedPortType { + return ErrUnsupportedNamedPort + } if portKind == numericPortType { portInfo, protocol := numericPortRule(&ports[i]) acl.DstPorts = portInfo From ce8cb5ab5b67c031ca888f8d5b165c3ccc81c291 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 22:08:38 +0000 Subject: [PATCH 06/10] get rid of unneeded comments --- npm/pkg/controlplane/translation/translatePolicy.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 5b7d8112e8..d7a71e3a84 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -454,8 +454,6 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, // #2.1 Handle IPBlock and port if exist if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { - // add logic that if the peer is only IPBlock and npm lite is enabled and is a /32 cidr block - // then skip creating IpBlockIPSet if npmLiteToggle { err = directPeerAndPortRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) if err != nil { From 351d8fb9db9cc162bb681b1ded9520ec3ae0e66f Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 22:11:18 +0000 Subject: [PATCH 07/10] got rid of function in utils that was not neede --- npm/util/util.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/npm/util/util.go b/npm/util/util.go index 4181e3c550..8d3f73e88c 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -359,7 +359,3 @@ func NodeIP() (string, error) { return nodeIP, nil } - -func IsCIDR32(cidr string) bool { - return strings.HasSuffix(cidr, "/32") -} From 55ec65eaf05683dab60aae05b7a2cb2a6a384c61 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 5 Nov 2025 23:46:45 +0000 Subject: [PATCH 08/10] added unit test for translate policy --- .../translation/translatePolicy_test.go | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index d308bba77f..581b8a39d1 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -1458,6 +1458,195 @@ func TestPeerAndPortRule(t *testing.T) { } } +func TestDirectPeerAndPortRule(t *testing.T) { + namedPort := intstr.FromString(namedPortStr) + port8000 := intstr.FromInt(8000) + var endPort int32 = 8100 + tcp := v1.ProtocolTCP + + tests := []struct { + name string + direction policies.Direction + ports []networkingv1.NetworkPolicyPort + cidr string + npmNetPol *policies.NPMNetworkPolicy + skipWindows bool + }{ + { + name: "egress tcp port 8000-8100 with /28 subnet", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &port8000, + EndPort: &endPort, + }, + }, + cidr: "10.0.1.0/28", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"10.0.1.0/28"}, + DstPorts: policies.Ports{ + Port: 8000, + EndPort: 8100, + }, + Protocol: "TCP", + }, + }, + }, + }, + { + name: "ingress no ports - single IP (/32)", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "10.226.0.49/32", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"10.226.0.49/32"}, + }, + }, + }, + }, + { + name: "egress no ports - subnet (/24)", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "192.168.1.0/24", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"192.168.1.0/24"}, + }, + }, + }, + }, + { + name: "ingress no ports - large subnet (/16)", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "172.16.0.0/16", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"172.16.0.0/16"}, + }, + }, + }, + }, + { + name: "egress tcp port 8000-8100 with /28 subnet", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &port8000, + EndPort: &endPort, + }, + }, + cidr: "10.0.1.0/28", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"10.0.1.0/28"}, + DstPorts: policies.Ports{ + Port: 8000, + EndPort: 8100, + }, + Protocol: "TCP", + }, + }, + }, + }, + { + name: "ingress udp port 53 with /32", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &[]v1.Protocol{v1.ProtocolUDP}[0], + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 53}, + }, + }, + cidr: "8.8.8.8/32", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"8.8.8.8/32"}, + DstPorts: policies.Ports{ + Port: 53, + EndPort: 0, + }, + Protocol: "UDP", + }, + }, + }, + }, + { + name: "named port should fail in NPM Lite", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &namedPort, + }, + }, + cidr: "10.226.0.49/32", + skipWindows: true, // Should fail on both platforms + }, + } + + for _, tt := range tests { + tt := tt + npmLiteToggle := true + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + npmNetPol := &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + } + err := directPeerAndPortRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle) + if tt.skipWindows { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.npmNetPol, npmNetPol) + } + }) + } +} + func TestIngressPolicy(t *testing.T) { tcp := v1.ProtocolTCP targetPodMatchType := policies.EitherMatch From 19fbf1e90bb2258fb577d7422966acceb671ee75 Mon Sep 17 00:00:00 2001 From: Riya Date: Mon, 10 Nov 2025 22:53:47 +0000 Subject: [PATCH 09/10] resolved pr comments --- .../translation/translatePolicy.go | 7 ++++--- .../translation/translatePolicy_test.go | 4 ++-- npm/pkg/dataplane/policies/policy_windows.go | 21 +++++++------------ 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index d7a71e3a84..3e997f23fa 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -362,7 +362,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return nil } -func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error { +func directPeerAndPortAllowRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error { if len(ports) == 0 { acl := policies.NewACLPolicy(policies.Allowed, direction) // bypasses ipset creation for /32 cidrs and directly creates an acl with the cidr @@ -397,7 +397,8 @@ func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction polic // Handle ports if portKind == namedPortType { - return ErrUnsupportedNamedPort + return fmt.Errorf("named port not supported in policy %s (namespace: %s, direction: %s, cidr: %s, port: %v): %w", + npmNetPol.PolicyKey, npmNetPol.Namespace, direction, cidr, ports[i].Port, ErrUnsupportedNamedPort) } if portKind == numericPortType { portInfo, protocol := numericPortRule(&ports[i]) @@ -455,7 +456,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { if npmLiteToggle { - err = directPeerAndPortRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) + err = directPeerAndPortAllowRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) if err != nil { return err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index 581b8a39d1..b74aadcceb 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -1458,7 +1458,7 @@ func TestPeerAndPortRule(t *testing.T) { } } -func TestDirectPeerAndPortRule(t *testing.T) { +func TestDirectPeerAndPortAllowRule(t *testing.T) { namedPort := intstr.FromString(namedPortStr) port8000 := intstr.FromInt(8000) var endPort int32 = 8100 @@ -1636,7 +1636,7 @@ func TestDirectPeerAndPortRule(t *testing.T) { PolicyKey: namedPortPolicyKey, ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), } - err := directPeerAndPortRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle) + err := directPeerAndPortAllowRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle) if tt.skipWindows { require.Error(t, err) } else { diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 7fc4a6b6e0..c74bcdf226 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -101,19 +101,6 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // Ignore adding ruletype for now as there is a bug // policySettings.RuleType = hcn.RuleTypeSwitch - // ACLPolicy settings uses ID field of SetPolicy in LocalAddresses or RemoteAddresses - var srcListStr, dstListStr string - // Check if we have direct IPs (NPM Lite /32 bypass) - if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 { - srcListStr = strings.Join(acl.SrcDirectIPs, ",") - dstListStr = strings.Join(acl.DstDirectIPs, ",") - } else { - // Original IPSet-based approach - srcListStr = getAddrListFromSetInfo(acl.SrcList) - dstListStr = getAddrListFromSetInfo(acl.DstList) - } - dstPortStr := getPortStrFromPorts(acl.DstPorts) - // HNS has confusing Local and Remote address defintions // For Traffic Direction INGRESS // LocalAddresses = Source Sets @@ -135,8 +122,11 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // LocalAddresses = Destination IPs // RemoteAddresses = Source IPs + var srcListStr, dstListStr string // if direct IPs are used, we leave local addresses to be an empty string if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 { + srcListStr = strings.Join(acl.SrcDirectIPs, ",") + dstListStr = strings.Join(acl.DstDirectIPs, ",") policySettings.LocalAddresses = "" if policySettings.Direction == hcn.DirectionTypeOut { // EGRESS: Remote = Destination IPs from policy @@ -146,10 +136,15 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er policySettings.RemoteAddresses = srcListStr } } else { + // Original IPSet-based approach + srcListStr = getAddrListFromSetInfo(acl.SrcList) + dstListStr = getAddrListFromSetInfo(acl.DstList) policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr } + dstPortStr := getPortStrFromPorts(acl.DstPorts) + // Switch ports based on direction policySettings.RemotePorts = "" policySettings.LocalPorts = dstPortStr From eb7319d061472127a3ef8c59f6b1d8e420cae10e Mon Sep 17 00:00:00 2001 From: Riya Date: Tue, 11 Nov 2025 00:36:43 +0000 Subject: [PATCH 10/10] resolved copilot comments --- .../translation/translatePolicy.go | 15 +++++------- .../translation/translatePolicy_test.go | 24 +++++++++++++++++-- npm/pkg/dataplane/policies/policy_windows.go | 2 +- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 3e997f23fa..69931e28bc 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -349,7 +349,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return err } - err = checkForNamedPortType(portKind, npmLiteToggle) + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], "") if err != nil { return err } @@ -381,7 +381,7 @@ func directPeerAndPortAllowRule(npmNetPol *policies.NPMNetworkPolicy, direction return err } - err = checkForNamedPortType(portKind, npmLiteToggle) + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], cidr) if err != nil { return err } @@ -396,10 +396,6 @@ func directPeerAndPortAllowRule(npmNetPol *policies.NPMNetworkPolicy, direction } // Handle ports - if portKind == namedPortType { - return fmt.Errorf("named port not supported in policy %s (namespace: %s, direction: %s, cidr: %s, port: %v): %w", - npmNetPol.PolicyKey, npmNetPol.Namespace, direction, cidr, ports[i].Port, ErrUnsupportedNamedPort) - } if portKind == numericPortType { portInfo, protocol := numericPortRule(&ports[i]) acl.DstPorts = portInfo @@ -694,9 +690,10 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy, npmLiteToggle bool) (*po return npmNetPol, nil } -func checkForNamedPortType(portKind netpolPortType, npmLiteToggle bool) error { +func checkForNamedPortType(npmNetPol *policies.NPMNetworkPolicy, portKind netpolPortType, npmLiteToggle bool, direction policies.Direction, port *networkingv1.NetworkPolicyPort, cidr string) error { if npmLiteToggle && portKind == namedPortType { - return ErrUnsupportedNonCIDR + return fmt.Errorf("named port not supported in policy %s (namespace: %s, direction: %s, cidr: %s, port: %v, protocol: %v): %w", + npmNetPol.PolicyKey, npmNetPol.Namespace, direction, cidr, port.Port, port.Protocol, ErrUnsupportedNamedPort) } return nil } @@ -717,7 +714,7 @@ func checkOnlyPortRuleExists( if err != nil { return err } - err = checkForNamedPortType(portKind, npmLiteToggle) + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], "") if err != nil { return err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index b74aadcceb..634191a2a8 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -3153,8 +3153,28 @@ func TestCheckForNamedPortType(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - // run the function passing in peers and a flag indicating whether npm lite is enabled - err := checkForNamedPortType(tt.portKind, tt.npmLiteEnabled) + // Create a mock NPM network policy for testing + npmNetPol := &policies.NPMNetworkPolicy{ + PolicyKey: "test-policy/test", + Namespace: "test-namespace", + } + + // Use the first port from test data, or create a default one if ports are empty + var testPort *networkingv1.NetworkPolicyPort + if len(tt.ports) > 0 { + testPort = &tt.ports[0] + } else { + // Create a default port for tests without specific port data + port := intstr.FromInt(8080) + protocol := v1.ProtocolTCP + testPort = &networkingv1.NetworkPolicyPort{ + Protocol: &protocol, + Port: &port, + } + } + + // run the function passing in all required parameters + err := checkForNamedPortType(npmNetPol, tt.portKind, tt.npmLiteEnabled, policies.Ingress, testPort, "10.0.0.0/24") if tt.wantErr { require.Error(t, err) } else { diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index c74bcdf226..9e23dcb92d 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -101,7 +101,7 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // Ignore adding ruletype for now as there is a bug // policySettings.RuleType = hcn.RuleTypeSwitch - // HNS has confusing Local and Remote address defintions + // HNS has confusing Local and Remote address definitions // For Traffic Direction INGRESS // LocalAddresses = Source Sets // RemoteAddresses = Destination Sets