Skip to content

Commit 894da23

Browse files
committed
removed unused parameter and added edge case scenarios to UTs
1 parent 840feab commit 894da23

File tree

2 files changed

+92
-2
lines changed

2 files changed

+92
-2
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func getExternalTrafficPolicyClusterServices(
191191
noSelectorServices = append(noSelectorServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
192192
}
193193
// Check if are there services with selector that match the network policy
194-
if checkServiceRisk(service, &namespace.Name, policyListAtNamespace) {
194+
if checkServiceRisk(service, policyListAtNamespace) {
195195
safeServices = append(safeServices, fmt.Sprintf("%s/%s", namespace.Name, service.Name))
196196
}
197197
}
@@ -218,7 +218,7 @@ func hasIngressPolicies(policies []*networkingv1.NetworkPolicy) bool {
218218
return false
219219
}
220220

221-
func checkServiceRisk(service *corev1.Service, namespace *string, policiesListAtNamespace []*networkingv1.NetworkPolicy) bool {
221+
func checkServiceRisk(service *corev1.Service, policiesListAtNamespace []*networkingv1.NetworkPolicy) bool {
222222
for _, policy := range policiesListAtNamespace {
223223
// Skips deny all policies as they do not have any ingress rules
224224
for _, ingress := range policy.Spec.Ingress {

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,96 @@ func TestGetExternalTrafficPolicyClusterServices(t *testing.T) {
14751475
expectedUnsafeRiskServices: []string{"namespace1/service-with-selector-and-named-ports"},
14761476
expectedUnsafeNoSelectorServices: []string{},
14771477
},
1478+
// Scenarios covering edge cases
1479+
{
1480+
name: "LoadBalancer service with externalTrafficPolicy=Cluster with no selector and a allow all and deny all ingress policy with no selector",
1481+
namespaces: &corev1.NamespaceList{
1482+
Items: []corev1.Namespace{
1483+
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
1484+
},
1485+
},
1486+
servicesByNamespace: map[string][]*corev1.Service{
1487+
"namespace1": {
1488+
{
1489+
ObjectMeta: metav1.ObjectMeta{Name: "service-with-no-selector"},
1490+
Spec: corev1.ServiceSpec{
1491+
Type: corev1.ServiceTypeLoadBalancer,
1492+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1493+
},
1494+
},
1495+
},
1496+
},
1497+
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
1498+
"namespace1": {
1499+
{
1500+
ObjectMeta: metav1.ObjectMeta{Name: "deny-all-ingress-policy-with-no-selector"},
1501+
Spec: networkingv1.NetworkPolicySpec{
1502+
PodSelector: metav1.LabelSelector{},
1503+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
1504+
},
1505+
},
1506+
{
1507+
ObjectMeta: metav1.ObjectMeta{Name: "allow-all-ingress-policy-with-no-selector"},
1508+
Spec: networkingv1.NetworkPolicySpec{
1509+
PodSelector: metav1.LabelSelector{},
1510+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
1511+
Ingress: []networkingv1.NetworkPolicyIngressRule{
1512+
{},
1513+
},
1514+
},
1515+
},
1516+
},
1517+
},
1518+
expectedUnsafeRiskServices: []string{},
1519+
expectedUnsafeNoSelectorServices: []string{},
1520+
},
1521+
{
1522+
name: "LoadBalancer service with externalTrafficPolicy=Cluster with a selector and a allow all and deny all ingress policy with a matching selector",
1523+
namespaces: &corev1.NamespaceList{
1524+
Items: []corev1.Namespace{
1525+
{ObjectMeta: metav1.ObjectMeta{Name: "namespace1"}},
1526+
},
1527+
},
1528+
servicesByNamespace: map[string][]*corev1.Service{
1529+
"namespace1": {
1530+
{
1531+
ObjectMeta: metav1.ObjectMeta{Name: "service-with-selector"},
1532+
Spec: corev1.ServiceSpec{
1533+
Type: corev1.ServiceTypeLoadBalancer,
1534+
Selector: map[string]string{"app": "test"},
1535+
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
1536+
},
1537+
},
1538+
},
1539+
},
1540+
policiesByNamespace: map[string][]*networkingv1.NetworkPolicy{
1541+
"namespace1": {
1542+
{
1543+
ObjectMeta: metav1.ObjectMeta{Name: "deny-all-ingress-policy-with-selector"},
1544+
Spec: networkingv1.NetworkPolicySpec{
1545+
PodSelector: metav1.LabelSelector{
1546+
MatchLabels: map[string]string{"app": "test"},
1547+
},
1548+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
1549+
},
1550+
},
1551+
{
1552+
ObjectMeta: metav1.ObjectMeta{Name: "allow-all-ingress-policy-with-no-selector"},
1553+
Spec: networkingv1.NetworkPolicySpec{
1554+
PodSelector: metav1.LabelSelector{
1555+
MatchLabels: map[string]string{"app": "test"},
1556+
},
1557+
PolicyTypes: []networkingv1.PolicyType{"Ingress"},
1558+
Ingress: []networkingv1.NetworkPolicyIngressRule{
1559+
{},
1560+
},
1561+
},
1562+
},
1563+
},
1564+
},
1565+
expectedUnsafeRiskServices: []string{},
1566+
expectedUnsafeNoSelectorServices: []string{},
1567+
},
14781568
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple policies
14791569

14801570
// Scenarios where there are LoadBalancer or NodePort services with externalTrafficPolicy=Cluster and there are multiple services

0 commit comments

Comments
 (0)