Skip to content

Commit f79188b

Browse files
rayaisaiahrayaisaiah
committed
added more service uts for nodeport and organized scenarios
1 parent c329191 commit f79188b

File tree

2 files changed

+128
-84
lines changed

2 files changed

+128
-84
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,18 +282,22 @@ func checkServiceTargetPortMatchPolicyPorts(servicePorts *[]corev1.ServicePort,
282282
}
283283

284284
// Check if all the services target ports are in the policies ingress ports
285-
serviceTargetPortPolicyPort := false
285+
matchedserviceTargetPortToPolicyPort := false
286286
for _, policyPort := range *policyPorts {
287287
// Check if the policys port exists
288288
if policyPort.Port == nil {
289289
return false
290290
}
291+
// If the port is a string then it is a named port and service is at risk
292+
if policyPort.Port.Type == intstr.String {
293+
return false
294+
}
291295
if servicePort.TargetPort.IntValue() == int(policyPort.Port.IntVal) && string(servicePort.Protocol) == string(*policyPort.Protocol) {
292-
serviceTargetPortPolicyPort = true
296+
matchedserviceTargetPortToPolicyPort = true
293297
break
294298
}
295299
}
296-
if !serviceTargetPortPolicyPort {
300+
if !matchedserviceTargetPortToPolicyPort {
297301
return false
298302
}
299303
}

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

Lines changed: 121 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
803803
expectedUnsafeServicesAtRisk []string
804804
expectedUnsafeNoSelectorServices []string
805805
}{
806+
// Scenarios where there are no LoadBalancer or NodePort services
806807
{
807808
name: "No namespaces",
808809
namespaces: &corev1.NamespaceList{
@@ -829,8 +830,9 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
829830
expectedUnsafeServicesAtRisk: []string{},
830831
expectedUnsafeNoSelectorServices: []string{},
831832
},
833+
// Scenarios where there are LoadBalancer or NodePort services but externalTrafficPolicy is not Cluster
832834
{
833-
name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and no policies",
835+
name: "LoadBalancer service with externalTrafficPolicy=Local with no selector and a deny all ingress policy with no selector",
834836
namespaces: &corev1.NamespaceList{
835837
Items: []corev1.Namespace{
836838
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
@@ -842,19 +844,27 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
842844
ObjectMeta: metav1.ObjectMeta{Name: "service-with-no-selector"},
843845
Spec: corev1.ServiceSpec{
844846
Type: corev1.ServiceTypeLoadBalancer,
845-
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
847+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
846848
},
847849
},
848850
},
849851
},
850852
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
851-
"namespace1": {},
853+
"namespace1": {
854+
{
855+
ObjectMeta: metav1.ObjectMeta{Name: "deny-all-ingress-policy-with-no-selector"},
856+
Spec: networkingv1.NetworkPolicySpec{
857+
PodSelector: metav1.LabelSelector{},
858+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
859+
},
860+
},
861+
},
852862
},
853863
expectedUnsafeServicesAtRisk: []string{},
854864
expectedUnsafeNoSelectorServices: []string{},
855865
},
856866
{
857-
name: "LoadBalancer service with externalTrafficPolicy=Local with no selector and a deny all ingress policy with no selector",
867+
name: "NodePort service with externalTrafficPolicy=Local with no selector and a deny all ingress policy with no selector",
858868
namespaces: &corev1.NamespaceList{
859869
Items: []corev1.Namespace{
860870
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
@@ -865,7 +875,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
865875
{
866876
ObjectMeta: metav1.ObjectMeta{Name: "service-with-no-selector"},
867877
Spec: corev1.ServiceSpec{
868-
Type: corev1.ServiceTypeLoadBalancer,
878+
Type: corev1.ServiceTypeNodePort,
869879
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
870880
},
871881
},
@@ -885,6 +895,56 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
885895
expectedUnsafeServicesAtRisk: []string{},
886896
expectedUnsafeNoSelectorServices: []string{},
887897
},
898+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster but no policies
899+
{
900+
name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and no policies",
901+
namespaces: &corev1.NamespaceList{
902+
Items: []corev1.Namespace{
903+
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
904+
},
905+
},
906+
servicesByNamespace: map[string][]*corev1.Service{
907+
"namespace1": {
908+
{
909+
ObjectMeta: metav1.ObjectMeta{Name: "service-with-no-selector"},
910+
Spec: corev1.ServiceSpec{
911+
Type: corev1.ServiceTypeLoadBalancer,
912+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
913+
},
914+
},
915+
},
916+
},
917+
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
918+
"namespace1": {},
919+
},
920+
expectedUnsafeServicesAtRisk: []string{},
921+
expectedUnsafeNoSelectorServices: []string{},
922+
},
923+
{
924+
name: "NodePort service with externalTrafficPolicy=Cluster with no selector and no policies",
925+
namespaces: &corev1.NamespaceList{
926+
Items: []corev1.Namespace{
927+
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
928+
},
929+
},
930+
servicesByNamespace: map[string][]*corev1.Service{
931+
"namespace1": {
932+
{
933+
ObjectMeta: metav1.ObjectMeta{Name: "service-with-no-selector"},
934+
Spec: corev1.ServiceSpec{
935+
Type: corev1.ServiceTypeNodePort,
936+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
937+
},
938+
},
939+
},
940+
},
941+
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
942+
"namespace1": {},
943+
},
944+
expectedUnsafeServicesAtRisk: []string{},
945+
expectedUnsafeNoSelectorServices: []string{},
946+
},
947+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and policies allow traffic
888948
{
889949
name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and an allow all ingress policy with no selector",
890950
namespaces: &corev1.NamespaceList{
@@ -1141,6 +1201,7 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
11411201
expectedUnsafeServicesAtRisk: []string{},
11421202
expectedUnsafeNoSelectorServices: []string{},
11431203
},
1204+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and policies deny traffic
11441205
{
11451206
name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and a deny all ingress policy with no selector",
11461207
namespaces: &corev1.NamespaceList{
@@ -1254,6 +1315,44 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
12541315
expectedUnsafeServicesAtRisk: []string{"namespace1/external-traffic-policy-cluster-service"},
12551316
expectedUnsafeNoSelectorServices: []string{},
12561317
},
1318+
{
1319+
name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a selector that doesnt match",
1320+
namespaces: &corev1.NamespaceList{
1321+
Items: []corev1.Namespace{
1322+
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
1323+
},
1324+
},
1325+
servicesByNamespace: map[string][]*corev1.Service{
1326+
"namespace1": {
1327+
{
1328+
ObjectMeta: metav1.ObjectMeta{Name: "service-with-selector"},
1329+
Spec: corev1.ServiceSpec{
1330+
Type: corev1.ServiceTypeLoadBalancer,
1331+
Selector: map[string]string{"app": "test", "app3": "test3", "app4": "test4"},
1332+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1333+
},
1334+
},
1335+
},
1336+
},
1337+
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
1338+
"namespace1": {
1339+
{
1340+
ObjectMeta: metav1.ObjectMeta{Name: "allow-all-ingress-policy-with-selector"},
1341+
Spec: networkingv1.NetworkPolicySpec{
1342+
PodSelector: metav1.LabelSelector{
1343+
MatchLabels: map[string]string{"app": "test", "app2": "test2"},
1344+
},
1345+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
1346+
Ingress: []networkingv1.NetworkPolicyIngressRule{
1347+
{},
1348+
},
1349+
},
1350+
},
1351+
},
1352+
},
1353+
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector"},
1354+
expectedUnsafeNoSelectorServices: []string{},
1355+
},
12571356
{
12581357
name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and an allow all ingress policy with a matching selector but ports dont match",
12591358
namespaces: &corev1.NamespaceList{
@@ -1274,6 +1373,11 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
12741373
Protocol: corev1.ProtocolTCP,
12751374
TargetPort: intstr.FromInt(80),
12761375
},
1376+
{
1377+
Port: 100,
1378+
Protocol: corev1.ProtocolTCP,
1379+
TargetPort: intstr.FromInt(100),
1380+
},
12771381
},
12781382
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
12791383
},
@@ -1292,6 +1396,13 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
12921396
Ingress: []networkingv1.NetworkPolicyIngressRule{
12931397
{
12941398
Ports: []networkingv1.NetworkPolicyPort{
1399+
{
1400+
Port: intstrPtr(intstr.FromInt(80)),
1401+
Protocol: func() *corev1.Protocol {
1402+
protocol := corev1.ProtocolTCP
1403+
return &protocol
1404+
}(),
1405+
},
12951406
{
12961407
Port: intstrPtr(intstr.FromInt(90)),
12971408
Protocol: func() *corev1.Protocol {
@@ -1364,82 +1475,11 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
13641475
expectedUnsafeServicesAtRisk: []string{"namespace1/service-with-selector-and-named-ports"},
13651476
expectedUnsafeNoSelectorServices: []string{},
13661477
},
1367-
// {
1368-
// name: "Multiple namespaces with various policies and services",
1369-
// namespaces: &corev1.NamespaceList{
1370-
// Items: []corev1.Namespace{
1371-
// {ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
1372-
// {ObjectMeta: metav1.ObjectMeta{Name: "namespace2"}},
1373-
// },
1374-
// },
1375-
// servicesByNamespace: map[string][]*corev1.Service{
1376-
// "namespace1": {
1377-
// {
1378-
// ObjectMeta: metav1.ObjectMeta{Name: "service1"},
1379-
// Spec: corev1.ServiceSpec{
1380-
// Type: corev1.ServiceTypeLoadBalancer,
1381-
// ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1382-
// Selector: map[string]string{"app": "test1"},
1383-
// },
1384-
// },
1385-
// },
1386-
// "namespace2": {
1387-
// {
1388-
// ObjectMeta: metav1.ObjectMeta{Name: "service2"},
1389-
// Spec: corev1.ServiceSpec{
1390-
// Type: corev1.ServiceTypeNodePort,
1391-
// ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1392-
// Selector: map[string]string{"app": "test2"},
1393-
// },
1394-
// },
1395-
// {
1396-
// ObjectMeta: metav1.ObjectMeta{Name: "service3"},
1397-
// Spec: corev1.ServiceSpec{
1398-
// Type: corev1.ServiceTypeLoadBalancer,
1399-
// ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1400-
// },
1401-
// },
1402-
// },
1403-
// },
1404-
// policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
1405-
// "namespace1": {
1406-
// {
1407-
// ObjectMeta: metav1.ObjectMeta{Name: "policy1"},
1408-
// Spec: networkingv1.NetworkPolicySpec{
1409-
// PodSelector: metav1.LabelSelector{
1410-
// MatchLabels: map[string]string{"app": "test1"},
1411-
// },
1412-
// Ingress: []networkingv1.NetworkPolicyIngressRule{
1413-
// {
1414-
// From: []networkingv1.NetworkPolicyPeer{
1415-
// {PodSelector: &metav1.LabelSelector{}},
1416-
// },
1417-
// },
1418-
// },
1419-
// },
1420-
// },
1421-
// },
1422-
// "namespace2": {
1423-
// {
1424-
// ObjectMeta: metav1.ObjectMeta{Name: "policy2"},
1425-
// Spec: networkingv1.NetworkPolicySpec{
1426-
// PodSelector: metav1.LabelSelector{
1427-
// MatchLabels: map[string]string{"app": "test2"},
1428-
// },
1429-
// Ingress: []networkingv1.NetworkPolicyIngressRule{
1430-
// {
1431-
// From: []networkingv1.NetworkPolicyPeer{
1432-
// {PodSelector: &metav1.LabelSelector{}},
1433-
// },
1434-
// },
1435-
// },
1436-
// },
1437-
// },
1438-
// },
1439-
// },
1440-
// expectedUnsafeServicesAtRisk: []string{"namespace2/service3"},
1441-
// expectedUnsafeNoSelectorServices: []string{"namespace2/service3"},
1442-
// },
1478+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple policies
1479+
1480+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple services
1481+
1482+
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple namespaces
14431483
}
14441484

14451485
for _, tt := range tests {

0 commit comments

Comments
 (0)