Skip to content

Commit 1689071

Browse files
committed
nit changes
1 parent 5805952 commit 1689071

File tree

2 files changed

+9
-6
lines changed

2 files changed

+9
-6
lines changed

tools/azure-npm-to-cilium-validator/azure-npm-to-cilium-validator.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func getExternalTrafficPolicyClusterServices(
198198
noSelectorServices = append(noSelectorServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
199199
}
200200
// Check if are there services with selector that match the network policy
201-
if checkServiceRisk(service, policyListAtNamespace) {
201+
if checkNoServiceRisk(service, policyListAtNamespace) {
202202
safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
203203
}
204204
}
@@ -225,7 +225,7 @@ func hasIngressPolicies(policies []*networkingv1.NetworkPolicy) bool {
225225
return false
226226
}
227227

228-
func checkServiceRisk(service *corev1.Service, policiesListAtNamespace []*networkingv1.NetworkPolicy) bool {
228+
func checkNoServiceRisk(service *corev1.Service, policiesListAtNamespace []*networkingv1.NetworkPolicy) bool {
229229
for _, policy := range policiesListAtNamespace {
230230
// Skips deny all policies as they do not have any ingress rules
231231
for _, ingress := range policy.Spec.Ingress {
@@ -261,7 +261,7 @@ func checkPolicyMatchServiceLabels(serviceLabels, policyLabels map[string]string
261261
}
262262

263263
// Check for each policy label that that label is present in the service labels
264-
// Note does not check matchExpressions
264+
// Note: does not check matchExpressions
265265
for policyKey, policyValue := range policyLabels {
266266
matchedPolicyLabelToServiceLabel := false
267267
for serviceKey, serviceValue := range serviceLabels {
@@ -302,7 +302,8 @@ func checkServiceTargetPortMatchPolicyPorts(servicePorts []corev1.ServicePort, p
302302
return false
303303
}
304304
// If the policy only has a protocol check the protocol against the service
305-
// Note if a network policy on NPM just targets a protocol it will allow all traffic with containing that protocol (ignoring the port)
305+
// Note: if a network policy on NPM just targets a protocol it will allow all traffic with containing that protocol (ignoring the port)
306+
// Note: an empty protocols default to "TCP" for both policies and services
306307
if policyPort.Port == nil && policyPort.Protocol != nil {
307308
if string(servicePort.Protocol) == string(*policyPort.Protocol) {
308309
matchedserviceTargetPortToPolicyPort = true
@@ -319,7 +320,8 @@ func checkServiceTargetPortMatchPolicyPorts(servicePorts []corev1.ServicePort, p
319320
return false
320321
}
321322
// Check if the service target port and protocol matches the policy port and protocol
322-
// Note that the service target port will never been undefined as it defaults to port which is a required field when Ports is defined
323+
// Note: that the service target port will never been undefined as it defaults to port which is a required field when Ports is defined
324+
// Note: an empty protocols default to "TCP" for both policies and services
323325
if servicePort.TargetPort.IntValue() == int(policyPort.Port.IntVal) && string(servicePort.Protocol) == string(*policyPort.Protocol) {
324326
matchedserviceTargetPortToPolicyPort = true
325327
break

tools/azure-npm-to-cilium-validator/azure-npm-to-cilium-validator_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1566,8 +1566,9 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
15661566
expectedUnsafeNoSelectorServices: []string{},
15671567
},
15681568
// add ut where target port is nil
1569-
// add a ut where target port matches to portocol
1569+
// add a ut where target port matches to protocol
15701570
// add a ut where the port is 0
1571+
// add a ut testing for match expressions
15711572

15721573
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple policies
15731574

0 commit comments

Comments
 (0)