added service selectors to the appended list instead to simplify logic

Author: rayaisaiah

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

@@ -194,11 +194,16 @@ func getUnsafeExternalTrafficPolicyClusterServices(
 				externalTrafficPolicy := service.Spec.ExternalTrafficPolicy
 				// If the service has externalTrafficPolicy is set to "Cluster" add it to the riskServices list (ExternalTrafficPolicy: "" defaults to Cluster)
 				if externalTrafficPolicy != corev1.ServiceExternalTrafficPolicyTypeLocal {
+					// Check if the service has a selector
+					serviceSelector := "selectors"
+					if service.Spec.Selector == nil {
+						serviceSelector = "no selectors"
+					}
+					riskServices = append(riskServices, fmt.Sprintf("%s/%s/%s", namespace.Name, service.Name, serviceSelector))
 					// Any service with externalTrafficPolicy=Cluster is at risk so need to elimate any services that are incorrectly flagged
-					riskServices = append(riskServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
-					// Check if are there services with selector that are allowed by a network policy that can be safely migrated
 					if checkNoServiceRisk(service, policyListAtNamespace) {
-						safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
+						// Check if are there services with selector that are allowed by a network policy that can be safely migrated
+						safeServices = append(safeServices, fmt.Sprintf("%s/%s/%s", namespace.Name, service.Name, serviceSelector))
 					}
 				}
 			}
@@ -471,11 +476,12 @@ func addUnsafeServicesToTable(table *tablewriter.Table, unsafeServices []string)
 		table.Append([]string{"Disruption for some Services with externalTrafficPolicy=Cluster", "✅", ""})
 	} else {
 		table.Append([]string{"Disruption for some Services with externalTrafficPolicy=Cluster", "❌", "Services affected:"})
-		// If there are any unsafe services then print them as they could be impacted by migration
+		// Print unsafe services as they could be impacted by migration
 		for _, service := range unsafeServices {
 			serviceName := strings.Split(service, "/")[1]
 			serviceNamespace := strings.Split(service, "/")[0]
-			table.Append([]string{"", "❌", fmt.Sprintf("Found Service: \033[31m%s\033[0m in namespace: \033[31m%s\033[0m\n", serviceName, serviceNamespace)})
+			serviceSelector := strings.Split(service, "/")[2]
+			table.Append([]string{"", "❌", fmt.Sprintf("Found Service: \033[31m%s\033[0m with \033[31m%s\033[0m in namespace: \033[31m%s\033[0m\n", serviceName, serviceSelector, serviceNamespace)})
 		}
 		table.Append([]string{"", "", "Manual investigation is required to evaluate if ingress is allowed to the service's backend Pods."})
 		table.Append([]string{"", "", "Please evaluate if these services would be impacted by migration."})

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

@@ -1266,7 +1266,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-no-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-no-selector/no selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and an allow all ingress policy with a selector",
@@ -1302,7 +1302,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-no-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-no-selector/no selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an deny all ingress policy with a matching selector",
@@ -1336,7 +1336,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster and matching policy that is not an allow all",
@@ -1381,7 +1381,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/external-traffic-policy-cluster-service"},
+			expectedUnsafeServices: []string{"namespace1/external-traffic-policy-cluster-service/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a selector that doesnt match",
@@ -1418,7 +1418,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a selector that has more labels",
@@ -1455,7 +1455,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a matching selector but ports dont match",
@@ -1521,7 +1521,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-named-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-named-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a matching selector but uses named ports",
@@ -1575,7 +1575,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-named-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-named-ports/selectors"},
 		},
 		// Scenarios covering edge cases
 		{
@@ -1710,7 +1710,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and an allow all ingress policy with a matchExpressions selector",
@@ -1760,7 +1760,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a matchExpressions selector",
@@ -1802,7 +1802,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-no-selector"},
+			expectedUnsafeServices: []string{"namespace1/service-with-no-selector/no selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a matching selector and protocol with no ports",
@@ -1970,7 +1970,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and targetport=0 and an allow all ingress policy with a matching selector and ports=0",
@@ -2024,7 +2024,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow 168.63.129.18/32 ingress policy with a matching selector and no ports",
@@ -2072,7 +2072,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow 168.63.129.16/32 ingress policy with a matching selector and ports",
@@ -2129,7 +2129,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-and-ports/selectors"},
 		},
 		// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple namespaces
 		{
@@ -2375,7 +2375,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector", "namespace2/service-with-selector-and-ports", "namespace3/service-with-selector-and-ports"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector/selectors", "namespace2/service-with-selector-and-ports/selectors", "namespace3/service-with-selector-and-ports/selectors"},
 		},
 		{
 			name: "LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and allow all ingress policies with some matching label and ports in multiple namespaces",
@@ -2538,7 +2538,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
 					},
 				},
 			},
-			expectedUnsafeServices: []string{"namespace1/service-with-selector-no-match", "namespace2/service-with-selector-and-ports-no-match", "namespace3/service-with-selector-and-ports-no-match"},
+			expectedUnsafeServices: []string{"namespace1/service-with-selector-no-match/selectors", "namespace2/service-with-selector-and-ports-no-match/selectors", "namespace3/service-with-selector-and-ports-no-match/selectors"},
 		},
 	}