Skip to content

Commit 809fc90

Browse files
committed
moved checkPolicyMatchServiceLabels check to the top since every block uses it and removed excplicit check for ports
1 parent dabc25e commit 809fc90

File tree

1 file changed

+16
-21
lines changed

1 file changed

+16
-21
lines changed

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

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -226,24 +226,25 @@ func checkNoServiceRisk(service *corev1.Service, policiesListAtNamespace []*netw
226226
for _, policy := range policiesListAtNamespace {
227227
// Skips deny all policies as they do not have any ingress rules
228228
for _, ingress := range policy.Spec.Ingress {
229-
// Check if there is an allow all ingress policy that matches labels the service is safe
230-
if len(ingress.From) == 0 && len(ingress.Ports) == 0 {
231-
// Check if there is an allow all ingress policy with empty selectors or matching service labels as the policy allows all services in the namespace
232-
if checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector) {
229+
// Check for each policy label that that label is present in the service labels meaning the service is being targeted by the policy
230+
if checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector) {
231+
// Check if there is an allow all ingress policy as the policy allows all services in the namespace
232+
if len(ingress.From) == 0 && len(ingress.Ports) == 0 {
233233
return true
234234
}
235-
}
236-
// Check if service is a loadbalancer and policy allows 168.63.129.16 and has no ports
237-
if service.Spec.Type == corev1.ServiceTypeLoadBalancer && len(ingress.Ports) == 0 {
238-
if checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector) && checkAllowsLoadBalancerIP(ingress.From) {
239-
return true
235+
// Check if service is a loadbalancer and policy allows 168.63.129.16 and has no ports
236+
if service.Spec.Type == corev1.ServiceTypeLoadBalancer && len(ingress.Ports) == 0 {
237+
if checkAllowsLoadBalancerIP(ingress.From) {
238+
return true
239+
}
240240
}
241-
}
242-
// If there are no ingress from but there are ports in the policy; check if the service is safe
243-
if len(ingress.From) == 0 {
244-
// If the policy targets all pods (allow all) or only pods that are in the service selector, check if traffic is allowed to all the service's target ports
245-
if checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector) && checkServiceTargetPortMatchPolicyPorts(service.Spec.Ports, ingress.Ports) {
246-
return true
241+
// If there are no ingress from but there are ports in the policy; check if the service is safe
242+
if len(ingress.From) == 0 {
243+
// If the policy targets all pods (allow all) or only pods that are in the service selector, check if traffic is allowed to all the service's target ports
244+
// Note: ingress.Ports.protocol will never be nil if len(ingress.Ports) is greater than 0. It defaults to "TCP" if not set
245+
if checkServiceTargetPortMatchPolicyPorts(service.Spec.Ports, ingress.Ports) {
246+
return true
247+
}
247248
}
248249
}
249250
}
@@ -291,12 +292,6 @@ func checkServiceTargetPortMatchPolicyPorts(servicePorts []corev1.ServicePort, p
291292
return false
292293
}
293294

294-
// If the policy is allowing no traffic from ports then the service is at risk
295-
// Note: ingress.Ports.protocol will never be nil if len(ingress.Ports) is greater than 0. It defaults to "TCP" if not set
296-
if len(policyPorts) == 0 {
297-
return false
298-
}
299-
300295
for _, servicePort := range servicePorts {
301296
// If the target port is a string then it is a named port and service is at risk
302297
if servicePort.TargetPort.Type == intstr.String {

0 commit comments

Comments
 (0)