Fix watch setup in the NetworkPolicy reconciler

In order to support proper cross-cluster watches, we need to use the `apiNetCache` in our watch setup of the reconciler.
ironcore-dev · Aug 2, 2024 · dbd998b · dbd998b
1 parent 9fe167f
commit dbd998b
Show file tree

Hide file tree

Showing 10 changed files with 26 additions and 47 deletions.
diff --git a/api/core/v1alpha1/network_types.go b/api/core/v1alpha1/network_types.go
@@ -26,7 +26,7 @@ type NetworkPeering struct {
 	Prefixes []PeeringPrefix `json:"prefixes,omitempty"`
 }
 
-// PeeringPrefixes defines prefixes to be exposed to the peered network
+// PeeringPrefix defines prefixes to be exposed to the peered network
 type PeeringPrefix struct {
 	// Name is the semantical name of the peering prefixes
 	Name string `json:"name"`

diff --git a/apinetlet/controllers/natgateway_controller.go b/apinetlet/controllers/natgateway_controller.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/ironcore-dev/controller-utils/clientutils"
-	"github.com/ironcore-dev/ironcore-net/api/core/v1alpha1"
 	apinetv1alpha1 "github.com/ironcore-dev/ironcore-net/api/core/v1alpha1"
 	apinetletclient "github.com/ironcore-dev/ironcore-net/apinetlet/client"
 	"github.com/ironcore-dev/ironcore-net/apinetlet/handler"
@@ -67,7 +66,7 @@ func (r *NATGatewayReconciler) deleteGone(ctx context.Context, log logr.Logger,
 	log.V(1).Info("Delete gone")
 
 	log.V(1).Info("Deleting any APINet NAT gateway by key")
-	if err := r.APINetClient.DeleteAllOf(ctx, &v1alpha1.NATGateway{},
+	if err := r.APINetClient.DeleteAllOf(ctx, &apinetv1alpha1.NATGateway{},
 		client.InNamespace(r.APINetNamespace),
 		apinetletclient.MatchingSourceKeyLabels(r.Scheme(), r.RESTMapper(), key, &networkingv1alpha1.NATGateway{}),
 	); err != nil {
@@ -95,7 +94,7 @@ func (r *NATGatewayReconciler) delete(ctx context.Context, log logr.Logger, natG
 	log.V(1).Info("Finalizer present, running cleanup")
 
 	log.V(1).Info("Deleting APINet NAT gateway")
-	apiNetNATGateway := &v1alpha1.NATGateway{
+	apiNetNATGateway := &apinetv1alpha1.NATGateway{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: r.APINetNamespace,
 			Name:      string(natGateway.UID),
@@ -160,7 +159,7 @@ func (r *NATGatewayReconciler) reconcile(ctx context.Context, log logr.Logger, n
 		return ctrl.Result{}, fmt.Errorf("error applying apinet nat gateway: %w", err)
 	}
 
-	apiNetNATGatewayAutoscaler := &v1alpha1.NATGatewayAutoscaler{
+	apiNetNATGatewayAutoscaler := &apinetv1alpha1.NATGatewayAutoscaler{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: apinetv1alpha1.SchemeGroupVersion.String(),
 			Kind:       "NATGatewayAutoscaler",
@@ -170,7 +169,7 @@ func (r *NATGatewayReconciler) reconcile(ctx context.Context, log logr.Logger, n
 			Name:      string(natGateway.UID),
 			Labels:    apinetletclient.SourceLabels(r.Scheme(), r.RESTMapper(), natGateway),
 		},
-		Spec: v1alpha1.NATGatewayAutoscalerSpec{
+		Spec: apinetv1alpha1.NATGatewayAutoscalerSpec{
 			NATGatewayRef: corev1.LocalObjectReference{Name: apiNetNATGateway.Name},
 			MinPublicIPs:  generic.Pointer[int32](1),  // TODO: Make this configurable via ironcore NAT gateway
 			MaxPublicIPs:  generic.Pointer[int32](10), // TODO: Configure depending on ironcore NAT gateway
@@ -181,7 +180,7 @@ func (r *NATGatewayReconciler) reconcile(ctx context.Context, log logr.Logger, n
 		return ctrl.Result{}, fmt.Errorf("error applying apinet NAT gateway autoscaler: %w", err)
 	}
 
-	natGatewayIPs := apiNetIPsToIPs(v1alpha1.GetNATGatewayIPs(apiNetNATGateway))
+	natGatewayIPs := apiNetIPsToIPs(apinetv1alpha1.GetNATGatewayIPs(apiNetNATGateway))
 	if !slices.Equal(natGateway.Status.IPs, natGatewayIPs) {
 		if err := r.updateNATGatewayStatus(ctx, natGateway, natGatewayIPs); err != nil {
 			return ctrl.Result{}, fmt.Errorf("error updating NAT gateway status IPs: %w", err)
@@ -215,11 +214,11 @@ func (r *NATGatewayReconciler) SetupWithManager(mgr ctrl.Manager, apiNetCache ca
 			),
 		).
 		WatchesRawSource(
-			source.Kind(apiNetCache, &v1alpha1.NATGateway{}),
+			source.Kind(apiNetCache, &apinetv1alpha1.NATGateway{}),
 			handler.EnqueueRequestForSource(mgr.GetScheme(), mgr.GetRESTMapper(), &networkingv1alpha1.NATGateway{}),
 		).
 		WatchesRawSource(
-			source.Kind(apiNetCache, &v1alpha1.NATGatewayAutoscaler{}),
+			source.Kind(apiNetCache, &apinetv1alpha1.NATGatewayAutoscaler{}),
 			handler.EnqueueRequestForSource(mgr.GetScheme(), mgr.GetRESTMapper(), &networkingv1alpha1.NATGateway{}),
 		).
 		Complete(r)

diff --git a/apinetlet/controllers/networkpolicy_controller.go b/apinetlet/controllers/networkpolicy_controller.go
@@ -8,6 +8,10 @@ import (
 	"fmt"
 	"net/netip"
 
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+
 	"github.com/go-logr/logr"
 	"github.com/ironcore-dev/controller-utils/clientutils"
 	apinetv1alpha1 "github.com/ironcore-dev/ironcore-net/api/core/v1alpha1"
@@ -23,16 +27,13 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
@@ -59,11 +60,10 @@ type NetworkPolicyReconciler struct {
 //+kubebuilder:rbac:groups=networking.ironcore.dev,resources=networkpolicies,verbs=get;list;watch;update;patch
 //+kubebuilder:rbac:groups=networking.ironcore.dev,resources=networkpolicies/finalizers,verbs=update;patch
 
-//+kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networkinterfaces,verbs=get;list;watch
-//+kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networks,verbs=get;list;watch
-
-//+cluster=apinet:kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networkpolicies,verbs=get;list;watch;create;update;patch;delete;deletecollection
+//+cluster=apinet:kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networkpolicies,verbs=get;list;watch;update;patch;delete;deletecollection
 //+cluster=apinet:kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networkpolicyrules,verbs=get;list;watch;create;update;patch;delete;deletecollection
+//+cluster=apinet:kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networks,verbs=get;list;watch
+//+cluster=apinet:kubebuilder:rbac:groups=core.apinet.ironcore.dev,resources=networkinterfaces,verbs=get;list;watch
 
 func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
@@ -589,12 +589,12 @@ func (r *NetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager, apiNetCache
 			source.Kind(apiNetCache, &apinetv1alpha1.NetworkPolicy{}),
 			apinetlethandler.EnqueueRequestForSource(r.Scheme(), r.RESTMapper(), &networkingv1alpha1.NetworkPolicy{}),
 		).
-		Watches(
-			&apinetv1alpha1.Network{},
+		WatchesRawSource(
+			source.Kind(apiNetCache, &apinetv1alpha1.Network{}),
 			r.enqueueByNetwork(),
 		).
-		Watches(
-			&apinetv1alpha1.NetworkInterface{},
+		WatchesRawSource(
+			source.Kind(apiNetCache, &apinetv1alpha1.NetworkInterface{}),
 			r.enqueueByNetworkInterface(),
 			builder.WithPredicates(r.networkInterfaceReadyPredicate()),
 		).

diff --git a/apinetlet/controllers/networkpolicy_controller_test.go b/apinetlet/controllers/networkpolicy_controller_test.go
@@ -31,7 +31,6 @@ var _ = Describe("NetworkPolicyController", func() {
 	network, apiNetNetwork := SetupNetwork(ns, apiNetNs)
 
 	It("should manage and reconcile the APINet network policy and its rules without target apinet nic", func(ctx SpecContext) {
-
 		By("creating an apinet nic for ingress")
 		ingressApiNetNic := &apinetv1alpha1.NetworkInterface{
 			ObjectMeta: metav1.ObjectMeta{
@@ -305,7 +304,6 @@ var _ = Describe("NetworkPolicyController", func() {
 	})
 
 	It("should manage and reconcile the APINet network policy and its rules with available target apinet nic ", func(ctx SpecContext) {
-
 		By("creating a target apinet nic")
 		targetApiNetNic1 := &apinetv1alpha1.NetworkInterface{
 			ObjectMeta: metav1.ObjectMeta{

diff --git a/client-go/openapi/zz_generated.openapi.go b/client-go/openapi/zz_generated.openapi.go
diff --git a/cmd/apinetlet/main.go b/cmd/apinetlet/main.go
@@ -10,6 +10,7 @@ import (
 	"os"
 
 	ironcorenetv1alpha1 "github.com/ironcore-dev/ironcore-net/api/core/v1alpha1"
+	apinetletclient "github.com/ironcore-dev/ironcore-net/apinetlet/client"
 	apinetletconfig "github.com/ironcore-dev/ironcore-net/apinetlet/client/config"
 	"github.com/ironcore-dev/ironcore-net/apinetlet/controllers"
 	"github.com/ironcore-dev/ironcore-net/client-go/ironcorenet"
@@ -232,6 +233,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	if err := apinetletclient.SetupNetworkPolicyNetworkNameFieldIndexer(ctx, apiNetCluster.GetFieldIndexer()); err != nil {
+		setupLog.Error(err, "unable to setup field indexer", "field", apinetletclient.NetworkPolicyNetworkNameField)
+		os.Exit(1)
+	}
+
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)

diff --git a/cmd/controller-manager/main.go b/cmd/controller-manager/main.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/ironcore-dev/controller-utils/configutils"
 	ironcorenetv1alpha1 "github.com/ironcore-dev/ironcore-net/api/core/v1alpha1"
-	apinetletclient "github.com/ironcore-dev/ironcore-net/apinetlet/client"
 	apinetclient "github.com/ironcore-dev/ironcore-net/internal/client"
 
 	"github.com/ironcore-dev/ironcore-net/internal/controllers"
@@ -181,11 +180,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err := apinetletclient.SetupNetworkPolicyNetworkNameFieldIndexer(ctx, mgr.GetFieldIndexer()); err != nil {
-		setupLog.Error(err, "unable to setup field indexer", "field", apinetletclient.NetworkPolicyNetworkNameField)
-		os.Exit(1)
-	}
-
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)

diff --git a/config/apinetlet/apinet-rbac/role.yaml b/config/apinetlet/apinet-rbac/role.yaml
@@ -111,7 +111,6 @@ rules:
   resources:
   - networkpolicies
   verbs:
-  - create
   - delete
   - deletecollection
   - get

diff --git a/config/apinetlet/rbac/role.yaml b/config/apinetlet/rbac/role.yaml
@@ -22,22 +22,6 @@ rules:
   - patch
   - update
   - watch
-- apiGroups:
-  - core.apinet.ironcore.dev
-  resources:
-  - networkinterfaces
-  verbs:
-  - get
-  - list
-  - watch
-- apiGroups:
-  - core.apinet.ironcore.dev
-  resources:
-  - networks
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - ipam.ironcore.dev
   resources:

diff --git a/config/apiserver/rbac/apinetlet_role.yaml b/config/apiserver/rbac/apinetlet_role.yaml
@@ -111,7 +111,6 @@ rules:
   resources:
   - networkpolicies
   verbs:
-  - create
   - delete
   - deletecollection
   - get
-Original file line number
+Diff line change
@@ Expand Up / @@ -111,7 +111,6 @@ rules: @@
       resources:
       - networkpolicies
       verbs:
-      - create
       - delete
       - deletecollection
       - get
@@ Expand Down @@