fix bug where access policy would stop adding ports if app doesn't ex… (

#526)

* fix bug where access policy would stop adding ports if app doesn't exist in namespace

* Review fix and let access policies partly fail

internal/controllers/common/reconciler.go

@@ -12,6 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -186,12 +187,15 @@ func (r *ReconcilerBase) updateStatus(ctx context.Context, skipObj v1alpha1.SKIP
 
 func (r *ReconcilerBase) getTargetApplicationPorts(ctx context.Context, appName string, namespace string) ([]networkingv1.NetworkPolicyPort, error) {
 	service := &corev1.Service{}
+	var servicePorts []networkingv1.NetworkPolicyPort
+
 	if err := r.GetClient().Get(ctx, types.NamespacedName{Name: appName, Namespace: namespace}, service); err != nil {
-		return nil, fmt.Errorf("error when trying to get target application: %w", err)
+		if errors.IsNotFound(err) {
+			return servicePorts, nil
+		}
+		return nil, fmt.Errorf("error when trying to get target application: %s", err.Error())
 	}
 
-	var servicePorts []networkingv1.NetworkPolicyPort
-
 	for _, port := range service.Spec.Ports {
 		servicePorts = append(servicePorts, networkingv1.NetworkPolicyPort{
 			Port: util.PointTo(intstr.FromInt32(port.Port)),
@@ -206,13 +210,16 @@ func (r *ReconcilerBase) UpdateAccessPolicy(ctx context.Context, obj v1alpha1.SK
 	}
 
 	if obj.GetCommonSpec().AccessPolicy.Outbound != nil {
-		if err := r.setPortsForRules(ctx, obj.GetCommonSpec().AccessPolicy.Outbound.Rules, obj.GetNamespace()); err != nil {
-			r.EmitWarningEvent(obj, "InvalidAccessPolicy", fmt.Sprintf("failed to set ports for outbound rules: %s", err.Error()))
+		if errs := r.setPortsForRules(ctx, obj.GetCommonSpec().AccessPolicy.Outbound.Rules, obj.GetNamespace()); len(errs) != 0 {
+			for _, err := range errs {
+				r.EmitWarningEvent(obj, "InvalidAccessPolicy", fmt.Sprintf("failed to set ports for outbound rules: %s", err.Error()))
+			}
 		}
 	}
 }
 
-func (r *ReconcilerBase) setPortsForRules(ctx context.Context, rules []podtypes.InternalRule, skipObjNamespace string) error {
+func (r *ReconcilerBase) setPortsForRules(ctx context.Context, rules []podtypes.InternalRule, skipObjNamespace string) []error {
+	var ruleErrors []error
 	for i := range rules {
 		rule := &rules[i]
 		if len(rule.Ports) != 0 {
@@ -226,11 +233,11 @@ func (r *ReconcilerBase) setPortsForRules(ctx context.Context, rules []podtypes.
 			selector := metav1.LabelSelector{MatchLabels: rule.NamespacesByLabel}
 			selectorString, err := metav1.LabelSelectorAsSelector(&selector)
 			if err != nil {
-			    return err
+				ruleErrors = append(ruleErrors, fmt.Errorf("failed to create label selector: %w", err))
 			}
 			namespaces := &corev1.NamespaceList{}
-			if err := r.GetClient().List(ctx, namespaces, &client.ListOptions{LabelSelector: selectorString}); err != nil {
-				return err
+			if err = r.GetClient().List(ctx, namespaces, &client.ListOptions{LabelSelector: selectorString}); err != nil {
+				ruleErrors = append(ruleErrors, fmt.Errorf("failed to list namespaces: %w", err))
 			}
 			for _, ns := range namespaces.Items {
 				namespaceList = append(namespaceList, ns.Name)
@@ -240,16 +247,20 @@ func (r *ReconcilerBase) setPortsForRules(ctx context.Context, rules []podtypes.
 		}
 
 		if len(namespaceList) == 0 {
-			return fmt.Errorf("expected namespace, but found none for rule %s", rule.Application)
+			ruleErrors = append(ruleErrors, fmt.Errorf("expected namespace, but found none for application %s", rule.Application))
 		}
 
 		for _, ns := range namespaceList {
 			targetAppPorts, err := r.getTargetApplicationPorts(ctx, rule.Application, ns)
 			if err != nil {
-				return err
+				ruleErrors = append(ruleErrors, err)
+			}
+			if len(targetAppPorts) == 0 {
+				ruleErrors = append(ruleErrors, fmt.Errorf("no ports found for application %s in namespace %s", rule.Application, ns))
+				continue
 			}
 			rule.Ports = append(rule.Ports, targetAppPorts...)
 		}
 	}
-	return nil
+	return ruleErrors
 }

tests/application/access-policy/bad-policy-assert.yaml

@@ -80,3 +80,6 @@ involvedObject:
   apiVersion: skiperator.kartverket.no/v1alpha1
   kind: Application
   name: no-namespace-with-labels
+message: >-
+  failed to set ports for outbound rules: expected namespace, but found none for
+  application access-policy-other

tests/application/access-policy/multiple-ns-same-label-assert.yaml

@@ -29,8 +29,9 @@ spec:
             matchLabels:
               app: app
       ports:
-        - port: 8080
-          protocol: TCP
         - port: 8082
           protocol: TCP
+        - port: 8080
+          protocol: TCP
+
 

tests/application/access-policy/multiple-ns-same-label.yaml

@@ -60,8 +60,16 @@ spec:
             team: someteam
     outbound:
       rules:
+        - application: idontexist
+        - application: idontexist
+          namespacesByLabel:
+            nonexisting: label
+        - application: idontexist
+          namespacesByLabel:
+              team: ateam
         - application: app
           namespacesByLabel:
             team: ateam
 
 
+