fix: move managed resource VAP out of CRDinstaller (#1202)

Author: nwnt

.codespellignore

@@ -4,3 +4,4 @@ CROs
 NotIn
 fo
 allReady
+AtLeast

cmd/crdinstaller/main.go

@@ -71,13 +71,6 @@ func main() {
 	}
 
 	klog.Infof("Successfully installed %s CRDs", *mode)
-
-	if err := utils.InstallManagedResourceVAP(ctx, client, *mode); err != nil {
-		klog.Warningf("Failed to install managed resource ValidatingAdmissionPolicy: %v", err)
-		return
-	}
-
-	klog.Infof("Successfully installed %s managed resource ValidatingAdmissionPolicy", *mode)
 }
 
 // installCRDs installs the CRDs from the specified directory based on the mode.

cmd/crdinstaller/utils/util.go

@@ -15,15 +15,12 @@ import (
 	"strings"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-
-	"go.goms.io/fleet/pkg/webhook/managedresource"
 )
 
 const (
@@ -174,19 +171,3 @@ func GetCRDFromPath(crdPath string, scheme *runtime.Scheme) (*apiextensionsv1.Cu
 
 	return crd, nil
 }
-
-func InstallManagedResourceVAP(ctx context.Context, c client.Client, mode string) error {
-	vap := managedresource.GetValidatingAdmissionPolicy(mode == "hub")
-	vapBinding := managedresource.GetValidatingAdmissionPolicyBinding()
-	for _, ob := range []client.Object{vap, vapBinding} {
-		if err := install(ctx, c, ob, nil); err != nil {
-			if meta.IsNoMatchError(err) {
-				klog.Infof("Cluster does not support %s resource, skipping installation", ob.GetObjectKind().GroupVersionKind().Kind)
-				return nil
-			}
-			return err
-		}
-	}
-	klog.Infof("Successfully installed managed resource ValidatingAdmissionPolicy")
-	return nil
-}

cmd/crdinstaller/utils/util_test.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	admv1 "k8s.io/api/admissionregistration/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -301,38 +300,3 @@ func TestInstall(t *testing.T) {
 		})
 	}
 }
-
-func TestInstallManagedResourceVAP(t *testing.T) {
-	tests := []struct {
-		name string
-		mode string
-	}{
-		{
-			name: "hub mode",
-			mode: "hub",
-		},
-		{
-			name: "member mode",
-			mode: "member",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			scheme := runtime.NewScheme()
-			if err := admv1.AddToScheme(scheme); err != nil {
-				t.Fatalf("Failed to add admissionregistration scheme: %v", err)
-			}
-
-			fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
-			err := InstallManagedResourceVAP(context.Background(), fakeClient, tt.mode)
-
-			// The function should complete without errors
-			// The actual installation behavior depends on the RESTMapper implementation
-			// which is difficult to test reliably with the fake client
-			if err != nil {
-				t.Errorf("InstallManagedResourceVAP() unexpected error: %v", err)
-			}
-		})
-	}
-}

cmd/hubagent/main.go

@@ -52,6 +52,7 @@ import (
 	mcv1beta1 "go.goms.io/fleet/pkg/controllers/membercluster/v1beta1"
 	fleetmetrics "go.goms.io/fleet/pkg/metrics"
 	"go.goms.io/fleet/pkg/webhook"
+	"go.goms.io/fleet/pkg/webhook/managedresource"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -210,6 +211,18 @@ func main() {
 		klog.InfoS("The controller manager has exited")
 	}()
 
+	// Wait for the cache to sync before creating managed resources
+	if !mgr.GetCache().WaitForCacheSync(ctx) {
+		klog.Error("failed to wait for cache sync")
+		exitWithErrorFunc()
+	}
+
+	if err := managedresource.EnsureVAP(ctx, mgr.GetClient(), true); err != nil {
+		klog.Errorf("unable to create managed resource validating admission policy: %s", err)
+		exitWithErrorFunc()
+	}
+	klog.Info("managed resource validating admission policy is successfully setup")
+
 	// Wait for the controller manager and the scheduler to exit.
 	wg.Wait()
 }

pkg/controllers/internalmembercluster/v1beta1/member_controller.go

@@ -44,6 +44,7 @@ import (
 	"go.goms.io/fleet/pkg/propertyprovider"
 	"go.goms.io/fleet/pkg/utils/condition"
 	"go.goms.io/fleet/pkg/utils/controller"
+	"go.goms.io/fleet/pkg/webhook/managedresource"
 )
 
 // propertyProviderConfig is a group of settings for configuring the the property provider.
@@ -213,6 +214,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	switch imc.Spec.State {
 	case clusterv1beta1.ClusterStateJoin:
+		if err := managedresource.EnsureVAP(ctx, r.memberClient, false); err != nil {
+			return ctrl.Result{}, err
+		}
+		klog.V(2).InfoS("Successfully installed managed resource validating admission policy")
 		if err := r.startAgents(ctx, &imc); err != nil {
 			return ctrl.Result{}, err
 		}
@@ -242,6 +247,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{RequeueAfter: time.Millisecond *
 			(time.Duration(hbinterval) + time.Duration(utilrand.Int63nRange(0, jitterRange)-jitterRange/2))}, nil
 	case clusterv1beta1.ClusterStateLeave:
+		if err := managedresource.EnsureNoVAP(ctx, r.memberClient, false); err != nil {
+			return ctrl.Result{}, err
+		}
+		klog.V(2).InfoS("Successfully uninstalled managed resource validating admission policy")
 		if err := r.stopAgents(ctx, &imc); err != nil {
 			return ctrl.Result{}, err
 		}

pkg/webhook/managedresource/createordelete.go

@@ -0,0 +1,86 @@
+/*
+Copyright (c) Microsoft Corporation.
+Licensed under the MIT license.
+*/
+
+package managedresource
+
+import (
+	"context"
+
+	v1 "k8s.io/api/admissionregistration/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+)
+
+func EnsureNoVAP(ctx context.Context, c client.Client, isHub bool) error {
+	objs := []client.Object{getValidatingAdmissionPolicy(isHub), getValidatingAdmissionPolicyBinding()}
+	for _, obj := range objs {
+		err := c.Delete(ctx, obj)
+		switch {
+		case err == nil, apierrors.IsNotFound(err):
+			// continue
+		case meta.IsNoMatchError(err):
+			klog.Infof("object type %T is not supported in this cluster, continuing", obj)
+			// continue
+		default:
+			klog.Errorf("Delete object type %T failed: %s", obj, err)
+			return err
+		}
+	}
+	return nil
+}
+
+func EnsureVAP(ctx context.Context, c client.Client, isHub bool) error {
+	type vapObjectAndMutator struct {
+		obj    client.Object
+		mutate func() error
+	}
+	// TODO: this can be simplified by dealing with the specific type rather than using client.Object
+	vap, mutateVAP := getVAPWithMutator(isHub)
+	vapb, mutateVAPB := getVAPBindingWithMutator()
+	objsAndMutators := []vapObjectAndMutator{
+		{
+			obj:    vap,
+			mutate: mutateVAP,
+		},
+		{
+			obj:    vapb,
+			mutate: mutateVAPB,
+		},
+	}
+
+	for _, objectMutator := range objsAndMutators {
+		opResult, err := controllerutil.CreateOrUpdate(ctx, c, objectMutator.obj, objectMutator.mutate)
+		switch {
+		case err == nil:
+			// continue
+		case meta.IsNoMatchError(err):
+			klog.Infof("object type %T is not supported in this cluster, continuing", objectMutator.obj)
+			// continue
+		default:
+			klog.Errorf("CreateOrUpdate (operation: %s) for object type %T failed: %s", opResult, objectMutator.obj, err)
+			return err
+		}
+	}
+	return nil
+}
+
+func getVAPWithMutator(isHub bool) (*v1.ValidatingAdmissionPolicy, func() error) {
+	vap := getValidatingAdmissionPolicy(isHub)
+	return vap, func() error {
+		mutateValidatingAdmissionPolicy(vap, isHub)
+		return nil
+	}
+}
+
+func getVAPBindingWithMutator() (*v1.ValidatingAdmissionPolicyBinding, func() error) {
+	vapb := getValidatingAdmissionPolicyBinding()
+	return vapb, func() error {
+		mutateValidatingAdmissionPolicyBinding(vapb)
+		return nil
+	}
+}