address review comments

ironcore-dev · May 24, 2024 · 9b96879 · 9b96879
1 parent 796b8bb
commit 9b96879
Show file tree

Hide file tree

Showing 13 changed files with 38 additions and 189 deletions.
diff --git a/api/core/v1alpha1/networkpolicyrule_types.go b/api/core/v1alpha1/networkpolicyrule_types.go
@@ -7,7 +7,6 @@ import (
 	"github.com/ironcore-dev/ironcore-net/apimachinery/api/net"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -35,14 +34,7 @@ type TargetNetworkInterface struct {
 	// IP is the IP address of the target network interface.
 	IP net.IP `json:"ip"`
 	// TargetRef is the target providing the destination.
-	TargetRef *NetworkPolicyTargetRef `json:"targetRef,omitempty"`
-}
-
-type NetworkPolicyTargetRef struct {
-	// UID is the UID of the target.
-	UID types.UID `json:"uid"`
-	// Name is the name of the target.
-	Name string `json:"name"`
+	TargetRef *LocalUIDReference `json:"targetRef,omitempty"`
 }
 
 type Rule struct {

diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go
diff --git a/apinetlet/controllers/networkpolicy_controller.go b/apinetlet/controllers/networkpolicy_controller.go
@@ -90,7 +90,7 @@ func (r *NetworkPolicyReconciler) deleteGone(ctx context.Context, log logr.Logge
 		return ctrl.Result{}, fmt.Errorf("error deleting apinet network policies: %w", err)
 	}
 
-	log.V(1).Info("Issued delete for any leftover APINet network policy")
+	log.V(1).Info("Deleted any leftover APINet network policy")
 	return ctrl.Result{}, nil
 }
 
@@ -120,17 +120,16 @@ func (r *NetworkPolicyReconciler) delete(ctx context.Context, log logr.Logger, n
 		if !apierrors.IsNotFound(err) {
 			return ctrl.Result{}, fmt.Errorf("error deleting apinet network policy: %w", err)
 		}
+	}
 
-		log.V(1).Info("APINet network policy is gone, removing finalizer")
-		if err := clientutils.PatchRemoveFinalizer(ctx, r.Client, networkPolicy, networkPolicyFinalizer); err != nil {
-			return ctrl.Result{}, fmt.Errorf("error removing finalizer: %w", err)
-		}
-		log.V(1).Info("Deleted")
-		return ctrl.Result{}, nil
+	log.V(1).Info("APINet network policy is gone, removing finalizer")
+	if err := clientutils.PatchRemoveFinalizer(ctx, r.Client, networkPolicy, networkPolicyFinalizer); err != nil {
+		return ctrl.Result{}, fmt.Errorf("error removing finalizer: %w", err)
 	}
 
-	log.V(1).Info("APINet network policy is not yet gone, requeueing")
-	return ctrl.Result{Requeue: true}, nil
+	log.V(1).Info("Deleted")
+
+	return ctrl.Result{}, nil
 }
 
 func (r *NetworkPolicyReconciler) reconcile(ctx context.Context, log logr.Logger, networkPolicy *networkingv1alpha1.NetworkPolicy) (ctrl.Result, error) {
@@ -220,7 +219,7 @@ func (r *NetworkPolicyReconciler) findTargets(ctx context.Context, apiNetNetwork
 		for _, ip := range apiNetNic.Spec.IPs {
 			targets = append(targets, apinetv1alpha1.TargetNetworkInterface{
 				IP: ip,
-				TargetRef: &apinetv1alpha1.NetworkPolicyTargetRef{
+				TargetRef: &apinetv1alpha1.LocalUIDReference{
 					UID:  apiNetNic.UID,
 					Name: apiNetNic.Name,
 				},
@@ -330,9 +329,14 @@ func (r *NetworkPolicyReconciler) fetchIPsFromNetworkInterfaces(ctx context.Cont
 		}
 
 		for _, ip := range nic.Spec.IPs {
+			ipFamily := corev1.IPv4Protocol
+			if ip.Addr.Is6() {
+				ipFamily = corev1.IPv6Protocol
+			}
+			ip.Addr.Is4()
 			ips = append(ips, apinetv1alpha1.ObjectIP{
-				Prefix:   net.IPPrefix{Prefix: netip.PrefixFrom(ip.Addr, 32)},
-				IPFamily: corev1.IPv4Protocol, // TODO: later support for IPv6
+				Prefix:   net.IPPrefix{Prefix: netip.PrefixFrom(ip.Addr, ip.Addr.BitLen())},
+				IPFamily: ipFamily,
 			})
 		}
 	}
@@ -362,8 +366,8 @@ func (r *NetworkPolicyReconciler) fetchIPsFromLoadBalancers(ctx context.Context,
 		for _, ip := range lb.Spec.IPs {
 			// TODO: handle LoadBalancerIP when only IPFamily is specified to allocate a random IP.
 			ips = append(ips, apinetv1alpha1.ObjectIP{
-				Prefix:   net.IPPrefix{Prefix: netip.PrefixFrom(ip.IP.Addr, 32)},
-				IPFamily: corev1.IPv4Protocol, // TODO: later support for IPv6
+				Prefix:   net.IPPrefix{Prefix: netip.PrefixFrom(ip.IP.Addr, ip.IP.Addr.BitLen())},
+				IPFamily: ip.IPFamily,
 			})
 		}
 	}
@@ -401,7 +405,11 @@ func (r *NetworkPolicyReconciler) applyNetworkPolicyRule(
 		IngressRules: ingressRules,
 		EgressRules:  egressRules,
 	}
-	_ = ctrl.SetControllerReference(apiNetNetworkPolicy, networkPolicyRule, r.Scheme())
+	err := ctrl.SetControllerReference(apiNetNetworkPolicy, networkPolicyRule, r.Scheme())
+	if err != nil {
+		return fmt.Errorf("error setting controller reference: %w", err)
+	}
+
 	if err := r.Patch(ctx, networkPolicyRule, client.Apply, networkPolicyFieldOwner, client.ForceOwnership); err != nil {
 		return fmt.Errorf("error applying network policy rule: %w", err)
 	}

diff --git a/client-go/applyconfigurations/core/v1alpha1/targetnetworkinterface.go b/client-go/applyconfigurations/core/v1alpha1/targetnetworkinterface.go
diff --git a/client-go/applyconfigurations/internal/internal.go b/client-go/applyconfigurations/internal/internal.go
diff --git a/client-go/applyconfigurations/utils.go b/client-go/applyconfigurations/utils.go
diff --git a/client-go/openapi/zz_generated.openapi.go b/client-go/openapi/zz_generated.openapi.go
diff --git a/gen/swagger.json b/gen/swagger.json
@@ -62135,23 +62135,6 @@
 				}
 			}
 		},
-		"com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkPolicyTargetRef": {
-			"type": "object",
-			"required": [
-				"uid",
-				"name"
-			],
-			"properties": {
-				"name": {
-					"description": "Name is the name of the target.",
-					"type": "string"
-				},
-				"uid": {
-					"description": "UID is the UID of the target.",
-					"type": "string"
-				}
-			}
-		},
 		"com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkSpec": {
 			"type": "object",
 			"properties": {
@@ -62404,7 +62387,7 @@
 				},
 				"targetRef": {
 					"description": "TargetRef is the target providing the destination.",
-					"$ref": "#/definitions/com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkPolicyTargetRef"
+					"$ref": "#/definitions/com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.LocalUIDReference"
 				}
 			}
 		},

diff --git a/gen/v3/apis__core.apinet.ironcore.dev__v1alpha1_openapi.json b/gen/v3/apis__core.apinet.ironcore.dev__v1alpha1_openapi.json
@@ -23361,25 +23361,6 @@
 					}
 				}
 			},
-			"com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkPolicyTargetRef": {
-				"type": "object",
-				"required": [
-					"uid",
-					"name"
-				],
-				"properties": {
-					"name": {
-						"description": "Name is the name of the target.",
-						"type": "string",
-						"default": ""
-					},
-					"uid": {
-						"description": "UID is the UID of the target.",
-						"type": "string",
-						"default": ""
-					}
-				}
-			},
 			"com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkSpec": {
 				"type": "object",
 				"properties": {
@@ -23707,7 +23688,7 @@
 						"description": "TargetRef is the target providing the destination.",
 						"allOf": [
 							{
-								"$ref": "#/components/schemas/com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.NetworkPolicyTargetRef"
+								"$ref": "#/components/schemas/com.github.ironcore-dev.ironcore-net.api.core.v1alpha1.LocalUIDReference"
 							}
 						]
 					}

diff --git a/internal/apis/core/networkpolicyrule_types.go b/internal/apis/core/networkpolicyrule_types.go
@@ -7,7 +7,6 @@ import (
 	"github.com/ironcore-dev/ironcore-net/apimachinery/api/net"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -35,14 +34,7 @@ type TargetNetworkInterface struct {
 	// IP is the IP address of the target network interface.
 	IP net.IP
 	// TargetRef is the target providing the destination.
-	TargetRef *NetworkPolicyTargetRef
-}
-
-type NetworkPolicyTargetRef struct {
-	// UID is the UID of the target.
-	UID types.UID
-	// Name is the name of the target.
-	Name string
+	TargetRef *LocalUIDReference
 }
 
 type Rule struct {