Skip to content

Commit 19fbf1e

Browse files
committed
resolved pr comments
1 parent 55ec65e commit 19fbf1e

File tree

3 files changed

+14
-18
lines changed

3 files changed

+14
-18
lines changed

npm/pkg/controlplane/translation/translatePolicy.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di
362362
return nil
363363
}
364364

365-
func directPeerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error {
365+
func directPeerAndPortAllowRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error {
366366
if len(ports) == 0 {
367367
acl := policies.NewACLPolicy(policies.Allowed, direction)
368368
// 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
397397

398398
// Handle ports
399399
if portKind == namedPortType {
400-
return ErrUnsupportedNamedPort
400+
return fmt.Errorf("named port not supported in policy %s (namespace: %s, direction: %s, cidr: %s, port: %v): %w",
401+
npmNetPol.PolicyKey, npmNetPol.Namespace, direction, cidr, ports[i].Port, ErrUnsupportedNamedPort)
401402
}
402403
if portKind == numericPortType {
403404
portInfo, protocol := numericPortRule(&ports[i])
@@ -455,7 +456,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy,
455456
if peer.IPBlock != nil {
456457
if len(peer.IPBlock.CIDR) > 0 {
457458
if npmLiteToggle {
458-
err = directPeerAndPortRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle)
459+
err = directPeerAndPortAllowRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle)
459460
if err != nil {
460461
return err
461462
}

npm/pkg/controlplane/translation/translatePolicy_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,7 @@ func TestPeerAndPortRule(t *testing.T) {
14581458
}
14591459
}
14601460

1461-
func TestDirectPeerAndPortRule(t *testing.T) {
1461+
func TestDirectPeerAndPortAllowRule(t *testing.T) {
14621462
namedPort := intstr.FromString(namedPortStr)
14631463
port8000 := intstr.FromInt(8000)
14641464
var endPort int32 = 8100
@@ -1636,7 +1636,7 @@ func TestDirectPeerAndPortRule(t *testing.T) {
16361636
PolicyKey: namedPortPolicyKey,
16371637
ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey),
16381638
}
1639-
err := directPeerAndPortRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle)
1639+
err := directPeerAndPortAllowRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle)
16401640
if tt.skipWindows {
16411641
require.Error(t, err)
16421642
} else {

npm/pkg/dataplane/policies/policy_windows.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,19 +101,6 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er
101101
// Ignore adding ruletype for now as there is a bug
102102
// policySettings.RuleType = hcn.RuleTypeSwitch
103103

104-
// ACLPolicy settings uses ID field of SetPolicy in LocalAddresses or RemoteAddresses
105-
var srcListStr, dstListStr string
106-
// Check if we have direct IPs (NPM Lite /32 bypass)
107-
if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 {
108-
srcListStr = strings.Join(acl.SrcDirectIPs, ",")
109-
dstListStr = strings.Join(acl.DstDirectIPs, ",")
110-
} else {
111-
// Original IPSet-based approach
112-
srcListStr = getAddrListFromSetInfo(acl.SrcList)
113-
dstListStr = getAddrListFromSetInfo(acl.DstList)
114-
}
115-
dstPortStr := getPortStrFromPorts(acl.DstPorts)
116-
117104
// HNS has confusing Local and Remote address defintions
118105
// For Traffic Direction INGRESS
119106
// LocalAddresses = Source Sets
@@ -135,8 +122,11 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er
135122
// LocalAddresses = Destination IPs
136123
// RemoteAddresses = Source IPs
137124

125+
var srcListStr, dstListStr string
138126
// if direct IPs are used, we leave local addresses to be an empty string
139127
if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 {
128+
srcListStr = strings.Join(acl.SrcDirectIPs, ",")
129+
dstListStr = strings.Join(acl.DstDirectIPs, ",")
140130
policySettings.LocalAddresses = ""
141131
if policySettings.Direction == hcn.DirectionTypeOut {
142132
// EGRESS: Remote = Destination IPs from policy
@@ -146,10 +136,15 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er
146136
policySettings.RemoteAddresses = srcListStr
147137
}
148138
} else {
139+
// Original IPSet-based approach
140+
srcListStr = getAddrListFromSetInfo(acl.SrcList)
141+
dstListStr = getAddrListFromSetInfo(acl.DstList)
149142
policySettings.LocalAddresses = srcListStr
150143
policySettings.RemoteAddresses = dstListStr
151144
}
152145

146+
dstPortStr := getPortStrFromPorts(acl.DstPorts)
147+
153148
// Switch ports based on direction
154149
policySettings.RemotePorts = ""
155150
policySettings.LocalPorts = dstPortStr

0 commit comments

Comments
 (0)