Skip to content

Commit 840feab

Browse files
rayaisaiahrayaisaiah
committed
updated getExternalTrafficPolicyClusterServices to be less than 200 characters and updated servicesAtRisk to riskSerivces
1 parent 593f29e commit 840feab

File tree

2 files changed

+39
-38
lines changed

2 files changed

+39
-38
lines changed

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,12 @@ func getEgressPolicies(policiesByNamespace map[string][]*networkingv1.NetworkPol
162162
return egressPolicies
163163
}
164164

165-
func getExternalTrafficPolicyClusterServices(namespaces *corev1.NamespaceList, servicesByNamespace map[string][]*corev1.Service, policiesByNamespace map[string][]*networkingv1.NetworkPolicy) (unsafeServicesAtRisk, unsafeNoSelectorServices []string) {
166-
var servicesAtRisk, noSelectorServices, safeServices []string
165+
func getExternalTrafficPolicyClusterServices(
166+
namespaces *corev1.NamespaceList,
167+
servicesByNamespace map[string][]*corev1.Service,
168+
policiesByNamespace map[string][]*networkingv1.NetworkPolicy,
169+
) (unsafeRiskServices, unsafeNoSelectorServices []string) {
170+
var riskServices, noSelectorServices, safeServices []string
167171

168172
for i := range namespaces.Items {
169173
namespace := &namespaces.Items[i]
@@ -178,10 +182,10 @@ func getExternalTrafficPolicyClusterServices(namespaces *corev1.NamespaceList, s
178182
for _, service := range serviceListAtNamespace {
179183
if service.Spec.Type == corev1.ServiceTypeLoadBalancer || service.Spec.Type == corev1.ServiceTypeNodePort {
180184
externalTrafficPolicy := service.Spec.ExternalTrafficPolicy
181-
// If the service has externalTrafficPolicy is set to "Cluster" add it to the servicesAtRisk list (ExternalTrafficPolicy: "" defaults to Cluster)
185+
// If the service has externalTrafficPolicy is set to "Cluster" add it to the riskServices list (ExternalTrafficPolicy: "" defaults to Cluster)
182186
if externalTrafficPolicy != corev1.ServiceExternalTrafficPolicyTypeLocal {
183187
// Any service with externalTrafficPolicy=Cluster is at risk so need to elimate any services that are incorrectly flagged
184-
servicesAtRisk = append(servicesAtRisk, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
188+
riskServices = append(riskServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
185189
// If the service has no selector add it to the noSelectorServices list
186190
if service.Spec.Selector == nil {
187191
noSelectorServices = append(noSelectorServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
@@ -196,10 +200,10 @@ func getExternalTrafficPolicyClusterServices(namespaces *corev1.NamespaceList, s
196200
}
197201

198202
// Remove all the safe services from the services at risk
199-
unsafeServicesAtRisk = difference(&servicesAtRisk, &safeServices)
203+
unsafeRiskServices = difference(&riskServices, &safeServices)
200204
// Remove all the safe services from the no selector services
201205
unsafeNoSelectorServices = difference(&noSelectorServices, &safeServices)
202-
return unsafeServicesAtRisk, unsafeNoSelectorServices
206+
return unsafeRiskServices, unsafeNoSelectorServices
203207
}
204208

205209
func hasIngressPolicies(policies []*networkingv1.NetworkPolicy) bool {
@@ -222,13 +226,11 @@ func checkServiceRisk(service *corev1.Service, namespace *string, policiesListAt
222226
if len(ingress.From) == 0 && len(ingress.Ports) == 0 {
223227
// Check if there is an allow all ingress policy with empty selectors return true as the policy allows all services in the namespace
224228
if checkPolicySelectorsAreEmpty(&policy.Spec.PodSelector) {
225-
fmt.Printf("found an allow all ingress policy: %s with empty selectors so service %s in the namespace %s is safe\n", policy.Name, service.Name, *namespace)
226229
return true
227230
}
228231
// Check if there is an allow all ingress policy that matches the service labels
229232
if checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector.MatchLabels) {
230233
// TODO add this to above logic and check in one if statement after i am done printing the logs
231-
fmt.Printf("found an allow all ingress policy: %s with matching selectors so service %s in the namespace %s is safe\n", policy.Name, service.Name, *namespace)
232234
return true
233235
}
234236
}
@@ -237,7 +239,6 @@ func checkServiceRisk(service *corev1.Service, namespace *string, policiesListAt
237239
// 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
238240
if checkPolicySelectorsAreEmpty(&policy.Spec.PodSelector) || checkPolicyMatchServiceLabels(service.Spec.Selector, policy.Spec.PodSelector.MatchLabels) {
239241
if checkServiceTargetPortMatchPolicyPorts(&service.Spec.Ports, &ingress.Ports) {
240-
fmt.Printf("found an ingress port policy: %s with matching selectors and target ports so service %s in the namespace %s is safe\n", policy.Name, service.Name, *namespace)
241242
return true
242243
}
243244
}
@@ -349,15 +350,15 @@ func printMigrationSummary(namespaces *corev1.NamespaceList, policiesByNamespace
349350
fmt.Println("+------------------------------+-------------------------------+")
350351

351352
// Get services that have externalTrafficPolicy!=Local
352-
unsafeServicesAtRisk, unsafeNoSelectorServices := getExternalTrafficPolicyClusterServices(namespaces, servicesByNamespace, policiesByNamespace)
353+
unsafeRiskServices, unsafeNoSelectorServices := getExternalTrafficPolicyClusterServices(namespaces, servicesByNamespace, policiesByNamespace)
353354

354355
// Print the services that are at risk
355-
printUnsafeServices(&unsafeServicesAtRisk, &unsafeNoSelectorServices)
356+
printUnsafeServices(&unsafeRiskServices, &unsafeNoSelectorServices)
356357

357358
fmt.Println("+------------------------------+-------------------------------+")
358359
if len(ingressEndportNetworkPolicy) > 0 || len(egressEndportNetworkPolicy) > 0 ||
359360
len(ingressPoliciesWithCIDR) > 0 || len(egressPoliciesWithCIDR) > 0 ||
360-
len(egressPolicies) > 0 || len(unsafeServicesAtRisk) > 0 {
361+
len(egressPolicies) > 0 || len(unsafeRiskServices) > 0 {
361362
fmt.Println("\033[31m✘ Review above issues before migration.\033[0m")
362363
fmt.Println("Please see \033[32maka.ms/azurenpmtocilium\033[0m for instructions on how to evaluate/assess the above warnings marked by ❌.")
363364
fmt.Println("NOTE: rerun this script if any modifications (create/update/delete) are made to services or policies.")
@@ -420,15 +421,15 @@ func printEgressPolicies(egressPolicies *[]string) {
420421
}
421422
}
422423

423-
func printUnsafeServices(unsafeServicesAtRisk, unsafeNoSelectorServices *[]string) {
424+
func printUnsafeServices(unsafeRiskServices, unsafeNoSelectorServices *[]string) {
424425
// If there is no unsafe services and services with no selectors then migration is safe for services with extranalTrafficPolicy=Cluster
425-
if len(*unsafeServicesAtRisk) == 0 {
426+
if len(*unsafeRiskServices) == 0 {
426427
fmt.Printf("%-30s | %-30s \n", "Disruption for some", "✅")
427428
fmt.Printf("%-30s | %-30s \n", "Services with", "")
428429
fmt.Printf("%-30s | %-30s \n", "externalTrafficPolicy=Cluster", "")
429430
} else {
430431
// Remove all no selector services from unsafe services to prevent repeating the same flagged service
431-
*unsafeServicesAtRisk = difference(unsafeServicesAtRisk, unsafeNoSelectorServices)
432+
*unsafeRiskServices = difference(unsafeRiskServices, unsafeNoSelectorServices)
432433
fmt.Printf("%-30s | %-30s \n", "Disruption for some", "❌")
433434
fmt.Printf("%-30s | %-30s \n", "Services with", "")
434435
fmt.Printf("%-30s | %-30s \n", "externalTrafficPolicy=Cluster", "")
@@ -441,8 +442,8 @@ func printUnsafeServices(unsafeServicesAtRisk, unsafeNoSelectorServices *[]strin
441442
fmt.Printf("❌ Found Service: \033[31m%s\033[0m without selectors in namespace: \033[31m%s\033[0m\n", serviceName, serviceNamespace)
442443
}
443444
}
444-
if len(*unsafeServicesAtRisk) > 0 {
445-
for _, service := range *unsafeServicesAtRisk {
445+
if len(*unsafeRiskServices) > 0 {
446+
for _, service := range *unsafeRiskServices {
446447
serviceName := strings.Split(service, "/")[1]
447448
serviceNamespace := strings.Split(service, "/")[0]
448449
fmt.Printf("❌ Found Service: \033[31m%s\033[0m with selectors in namespace: \033[31m%s\033[0m\n", serviceName, serviceNamespace)

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
800800
namespaces *corev1.NamespaceList
801801
servicesByNamespace map[string][]*corev1.Service
802802
policiesByNamespace map[string][]*networkingv1.NetworkPolicy
803-
expectedUnsafeServicesAtRisk []string
803+
expectedUnsafeRiskServices []string
804804
expectedUnsafeNoSelectorServices []string
805805
}{
806806
// Scenarios where there are no LoadBalancer or NodePort services
@@ -811,7 +811,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
811811
},
812812
servicesByNamespace: map[string][]*corev1.Service{},
813813
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{},
814-
expectedUnsafeServicesAtRisk: []string{},
814+
expectedUnsafeRiskServices: []string{},
815815
expectedUnsafeNoSelectorServices: []string{},
816816
},
817817
{
@@ -827,7 +827,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
827827
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
828828
"namespace1": {},
829829
},
830-
expectedUnsafeServicesAtRisk: []string{},
830+
expectedUnsafeRiskServices: []string{},
831831
expectedUnsafeNoSelectorServices: []string{},
832832
},
833833
// Scenarios where there are LoadBalancer or NodePort services but externalTrafficPolicy is not Cluster
@@ -860,7 +860,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
860860
},
861861
},
862862
},
863-
expectedUnsafeServicesAtRisk: []string{},
863+
expectedUnsafeRiskServices: []string{},
864864
expectedUnsafeNoSelectorServices: []string{},
865865
},
866866
{
@@ -892,7 +892,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
892892
},
893893
},
894894
},
895-
expectedUnsafeServicesAtRisk: []string{},
895+
expectedUnsafeRiskServices: []string{},
896896
expectedUnsafeNoSelectorServices: []string{},
897897
},
898898
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster but no policies
@@ -917,7 +917,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
917917
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
918918
"namespace1": {},
919919
},
920-
expectedUnsafeServicesAtRisk: []string{},
920+
expectedUnsafeRiskServices: []string{},
921921
expectedUnsafeNoSelectorServices: []string{},
922922
},
923923
{
@@ -941,7 +941,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
941941
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
942942
"namespace1": {},
943943
},
944-
expectedUnsafeServicesAtRisk: []string{},
944+
expectedUnsafeRiskServices: []string{},
945945
expectedUnsafeNoSelectorServices: []string{},
946946
},
947947
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and policies allow traffic
@@ -977,7 +977,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
977977
},
978978
},
979979
},
980-
expectedUnsafeServicesAtRisk: []string{},
980+
expectedUnsafeRiskServices: []string{},
981981
expectedUnsafeNoSelectorServices: []string{},
982982
},
983983
{
@@ -1015,7 +1015,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
10151015
},
10161016
},
10171017
},
1018-
expectedUnsafeServicesAtRisk: []string{},
1018+
expectedUnsafeRiskServices: []string{},
10191019
expectedUnsafeNoSelectorServices: []string{},
10201020
},
10211021
{
@@ -1070,7 +1070,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
10701070
},
10711071
},
10721072
},
1073-
expectedUnsafeServicesAtRisk: []string{},
1073+
expectedUnsafeRiskServices: []string{},
10741074
expectedUnsafeNoSelectorServices: []string{},
10751075
},
10761076
{
@@ -1105,7 +1105,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
11051105
},
11061106
},
11071107
},
1108-
expectedUnsafeServicesAtRisk: []string{},
1108+
expectedUnsafeRiskServices: []string{},
11091109
expectedUnsafeNoSelectorServices: []string{},
11101110
},
11111111
{
@@ -1143,7 +1143,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
11431143
},
11441144
},
11451145
},
1146-
expectedUnsafeServicesAtRisk: []string{},
1146+
expectedUnsafeRiskServices: []string{},
11471147
expectedUnsafeNoSelectorServices: []string{},
11481148
},
11491149
{
@@ -1198,7 +1198,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
11981198
},
11991199
},
12001200
},
1201-
expectedUnsafeServicesAtRisk: []string{},
1201+
expectedUnsafeRiskServices: []string{},
12021202
expectedUnsafeNoSelectorServices: []string{},
12031203
},
12041204
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and policies deny traffic
@@ -1231,7 +1231,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
12311231
},
12321232
},
12331233
},
1234-
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-no-selector"},
1234+
expectedUnsafeRiskServices: []string{"namespace1/service-with-no-selector"},
12351235
expectedUnsafeNoSelectorServices: []string{"namespace1/service-with-no-selector"},
12361236
},
12371237
{
@@ -1266,7 +1266,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
12661266
},
12671267
},
12681268
},
1269-
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector"},
1269+
expectedUnsafeRiskServices: []string{"namespace1/service-with-selector"},
12701270
expectedUnsafeNoSelectorServices: []string{},
12711271
},
12721272
{
@@ -1312,7 +1312,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
13121312
},
13131313
},
13141314
},
1315-
expectedUnsafeServicesAtRisk: []string{"namespace1/external-traffic-policy-cluster-service"},
1315+
expectedUnsafeRiskServices: []string{"namespace1/external-traffic-policy-cluster-service"},
13161316
expectedUnsafeNoSelectorServices: []string{},
13171317
},
13181318
{
@@ -1350,7 +1350,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
13501350
},
13511351
},
13521352
},
1353-
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector"},
1353+
expectedUnsafeRiskServices: []string{"namespace1/service-with-selector"},
13541354
expectedUnsafeNoSelectorServices: []string{},
13551355
},
13561356
{
@@ -1417,7 +1417,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
14171417
},
14181418
},
14191419
},
1420-
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector-and-named-ports"},
1420+
expectedUnsafeRiskServices: []string{"namespace1/service-with-selector-and-named-ports"},
14211421
expectedUnsafeNoSelectorServices: []string{},
14221422
},
14231423
{
@@ -1472,7 +1472,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
14721472
},
14731473
},
14741474
},
1475-
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector-and-named-ports"},
1475+
expectedUnsafeRiskServices: []string{"namespace1/service-with-selector-and-named-ports"},
14761476
expectedUnsafeNoSelectorServices: []string{},
14771477
},
14781478
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple policies
@@ -1485,8 +1485,8 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
14851485
for _, tt := range tests {
14861486
t.Run(tt.name, func(t *testing.T) {
14871487
unsafeServices, noSelectorServices := getExternalTrafficPolicyClusterServices(tt.namespaces, tt.servicesByNamespace, tt.policiesByNamespace)
1488-
if !equal(unsafeServices, tt.expectedUnsafeServicesAtRisk) {
1489-
t.Errorf("expected unsafe services %v, got %v", tt.expectedUnsafeServicesAtRisk, unsafeServices)
1488+
if !equal(unsafeServices, tt.expectedUnsafeRiskServices) {
1489+
t.Errorf("expected unsafe services %v, got %v", tt.expectedUnsafeRiskServices, unsafeServices)
14901490
}
14911491
if !equal(noSelectorServices, tt.expectedUnsafeNoSelectorServices) {
14921492
t.Errorf("expected no selector services %v, got %v", tt.expectedUnsafeNoSelectorServices, noSelectorServices)

0 commit comments

Comments
 (0)