Skip to content

Commit cdb91b2

Browse files
committed
fixed logic on services with allow all ingress polcies
1 parent c7a6c04 commit cdb91b2

File tree

1 file changed

+26
-24
lines changed

1 file changed

+26
-24
lines changed

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

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,12 @@ func checkForEgressPolicies(policiesByNamespace map[string][]networkingv1.Networ
233233
func checkExternalTrafficPolicyServices(namespaces *corev1.NamespaceList, servicesByNamespace map[string][]corev1.Service, policiesByNamespace map[string][]networkingv1.NetworkPolicy) bool {
234234
var servicesAtRisk, noSelectorServices, safeServices []string
235235

236-
for _, ns := range namespaces.Items {
236+
for _, namespace := range namespaces.Items {
237237
// Check if are there ingress policies in the namespace if not skip
238-
if !hasIngressPolicies(policiesByNamespace[ns.Name]) {
238+
if !hasIngressPolicies(policiesByNamespace[namespace.Name]) {
239239
continue
240240
}
241-
serviceListAtNamespace := servicesByNamespace[ns.Name]
241+
serviceListAtNamespace := servicesByNamespace[namespace.Name]
242242

243243
// Check if are there services with externalTrafficPolicy=Cluster (applicable if Type=NodePort or Type=LoadBalancer)
244244
for _, service := range serviceListAtNamespace {
@@ -252,13 +252,13 @@ func checkExternalTrafficPolicyServices(namespaces *corev1.NamespaceList, servic
252252
// If the service has externalTrafficPolicy is set to "Cluster" add it to the servicesAtRisk list (ExternalTrafficPolicy: "" defaults to Cluster)
253253
if externalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeLocal {
254254
// Any service with externalTrafficPolicy=Cluster is at risk so need to elimate any services that are incorrectly flagged
255-
servicesAtRisk = append(servicesAtRisk, fmt.Sprintf("%s/%s", ns.Name, service.Name))
255+
servicesAtRisk = append(servicesAtRisk, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
256256
// If the service has no selector add it to the noSelectorServices list
257257
if service.Spec.Selector == nil {
258-
noSelectorServices = append(noSelectorServices, fmt.Sprintf("%s/%s", ns.Name, service.Name))
258+
noSelectorServices = append(noSelectorServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
259259
} else {
260260
// Check if are there services with selector that match the network policy
261-
checkServiceRisk(service, ns.Name, servicePorts, policiesByNamespace[ns.Name], safeServices)
261+
safeServices = checkServiceRisk(service, namespace.Name, servicePorts, policiesByNamespace[namespace.Name], safeServices)
262262
}
263263
}
264264
}
@@ -314,33 +314,35 @@ func hasIngressPolicies(policies []networkingv1.NetworkPolicy) bool {
314314
return false
315315
}
316316

317-
func checkServiceRisk(service v1.Service, namespace string, servicePorts []string, policiesListAtNamespace []networkingv1.NetworkPolicy, safeServices []string) {
317+
func checkServiceRisk(service v1.Service, namespace string, servicePorts []string, policiesListAtNamespace []networkingv1.NetworkPolicy, safeServices []string) []string {
318318
for _, policy := range policiesListAtNamespace {
319319
for _, ingress := range policy.Spec.Ingress {
320-
// If there are no ingress from and ports in the policy check; check if the service is safe
320+
// Check if there is an allow all policy that matches labels the service is safe
321321
if len(ingress.From) == 0 && len(ingress.Ports) == 0 {
322322
if matchAllServiceSelector(&metav1.LabelSelector{MatchLabels: service.Spec.Selector}, &policy.Spec.PodSelector) {
323323
safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace, service.Name))
324-
return
325-
}
326-
}
327-
// If there are no ingress from but there are ports in the policy; check if the service is safe
328-
if len(ingress.From) == 0 && len(ingress.Ports) > 0 {
329-
if matchAllServiceSelector(&metav1.LabelSelector{MatchLabels: service.Spec.Selector}, &policy.Spec.PodSelector) {
330-
matchingPorts := []string{}
331-
for _, port := range ingress.Ports {
332-
matchingPorts = append(matchingPorts, fmt.Sprintf("%d/%s", port.Port.IntVal, string(*port.Protocol)))
333-
}
334-
for _, sevicePort := range servicePorts {
335-
if contains(matchingPorts, sevicePort) {
336-
safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace, service.Name))
337-
return
338-
}
339-
}
324+
return safeServices
340325
}
341326
}
327+
// Check if all the labels in
328+
// // If there are no ingress from but there are ports in the policy; check if the service is safe
329+
// if len(ingress.From) == 0 && len(ingress.Ports) > 0 {
330+
// if matchAllServiceSelector(&metav1.LabelSelector{MatchLabels: service.Spec.Selector}, &policy.Spec.PodSelector) {
331+
// matchingPorts := []string{}
332+
// for _, port := range ingress.Ports {
333+
// matchingPorts = append(matchingPorts, fmt.Sprintf("%d/%s", port.Port.IntVal, string(*port.Protocol)))
334+
// }
335+
// for _, sevicePort := range servicePorts {
336+
// if contains(matchingPorts, sevicePort) {
337+
// safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace, service.Name))
338+
// return
339+
// }
340+
// }
341+
// }
342+
// }
342343
}
343344
}
345+
return safeServices
344346
}
345347

346348
func matchAllServiceSelector(serviceSelector *metav1.LabelSelector, policyPodSelector *metav1.LabelSelector) bool {

0 commit comments

Comments
 (0)