fix: fix the validation webhook

cmd/hubagent/main.go

@@ -156,7 +156,8 @@ func main() {
 
 	if opts.EnableWebhook {
 		whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",")
-		if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers, opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload); err != nil {
+		if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers,
+			opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled); err != nil {
 			klog.ErrorS(err, "unable to set up webhook")
 			exitWithErrorFunc()
 		}
@@ -188,7 +189,8 @@ func main() {
 }
 
 // SetupWebhook generates the webhook cert and then set up the webhook configurator.
-func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string, whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool) error {
+func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string,
+	whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool) error {
 	// Generate self-signed key and crt files in FleetWebhookCertDir for the webhook server to start.
 	w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir, enableGuardRail, denyModifyMemberClusterLabels, enableWorkload)
 	if err != nil {
@@ -199,7 +201,7 @@ func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.Webho
 		klog.ErrorS(err, "unable to add WebhookConfig")
 		return err
 	}
-	if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels); err != nil {
+	if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels, networkingAgentsEnabled); err != nil {
 		klog.ErrorS(err, "unable to register webhooks to the manager")
 		return err
 	}

pkg/webhook/add_handler.go

@@ -16,13 +16,13 @@ import (
 func init() {
 	// AddToManagerFleetResourceValidator is a function to register fleet guard rail resource validator to the webhook server
 	AddToManagerFleetResourceValidator = fleetresourcehandler.Add
+	AddToManagerMemberclusterValidator = membercluster.Add
 	// AddToManagerFuncs is a list of functions to register webhook validators and mutators to the webhook server
 	AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.AddMutating)
 	AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, resourceplacement.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, pod.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add)
-	AddToManagerFuncs = append(AddToManagerFuncs, membercluster.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceoverride.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, resourceoverride.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacementeviction.Add)

pkg/webhook/membercluster/membercluster_validating_webhook.go

@@ -26,15 +26,19 @@ var (
 )
 
 type memberClusterValidator struct {
-	client  client.Client
-	decoder webhook.AdmissionDecoder
+	client                  client.Client
+	decoder                 webhook.AdmissionDecoder
+	networkingAgentsEnabled bool
 }
 
 // Add registers the webhook for K8s bulit-in object types.
-func Add(mgr manager.Manager) error {
+func Add(mgr manager.Manager, networkingAgentsEnabled bool) {
 	hookServer := mgr.GetWebhookServer()
-	hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{client: mgr.GetClient(), decoder: admission.NewDecoder(mgr.GetScheme())}})
-	return nil
+	hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{
+		client:                  mgr.GetClient(),
+		decoder:                 admission.NewDecoder(mgr.GetScheme()),
+		networkingAgentsEnabled: networkingAgentsEnabled,
+	}})
 }
 
 // Handle memberClusterValidator checks to see if member cluster has valid fields.
@@ -50,8 +54,12 @@ func (v *memberClusterValidator) Handle(ctx context.Context, req admission.Reque
 		}
 
 		if mc.Spec.DeleteOptions != nil && mc.Spec.DeleteOptions.ValidationMode == clusterv1beta1.DeleteValidationModeSkip {
-			klog.V(2).InfoS("Skipping validation for member cluster DELETE", "memberCluster", mcObjectName)
-			return admission.Allowed("Skipping validation for member cluster DELETE")
+			klog.V(2).InfoS("Skipping validation for member cluster DELETE when the validation mode is set to skip", "memberCluster", mcObjectName)
+			return admission.Allowed("Skipping validation for member cluster DELETE when the validation mode is set to skip")
+		}
+		if !v.networkingAgentsEnabled {
+			klog.V(2).InfoS("Networking agents disabled; skipping ServiceExport validation", "memberCluster", mcObjectName)
+			return admission.Allowed("Networking agents disabled; skipping ServiceExport validation")
 		}
 
 		klog.V(2).InfoS("Validating webhook member cluster DELETE", "memberCluster", mcObjectName)

pkg/webhook/membercluster/membercluster_validating_webhook_test.go

@@ -0,0 +1,141 @@
+package membercluster
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"strings"
+	"testing"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+
+	clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
+	"github.com/kubefleet-dev/kubefleet/pkg/utils"
+
+	fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+func TestHandleDeleteServiceExportValidation(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		networkingEnabled     bool
+		expectAllowed         bool
+		expectedMessageSubstr string
+		validationMode        clusterv1beta1.DeleteValidationMode
+	}{
+		"networking-disabled-allows-delete": {
+			networkingEnabled: false,
+			expectAllowed:     true,
+			validationMode:    clusterv1beta1.DeleteValidationModeStrict,
+		},
+		"networking-enabled-denies-delete": {
+			networkingEnabled:     true,
+			expectAllowed:         false,
+			validationMode:        clusterv1beta1.DeleteValidationModeStrict,
+			expectedMessageSubstr: "Please delete serviceExport",
+		},
+		"delete-options-skip-bypasses-validation": {
+			networkingEnabled: true,
+			expectAllowed:     true,
+			validationMode:    clusterv1beta1.DeleteValidationModeSkip,
+		},
+	}
+
+	for name, tc := range testCases {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			mcName := fmt.Sprintf("member-%s", name)
+			namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, mcName)
+			svcExport := newInternalServiceExport(mcName, namespaceName)
+
+			validator := newMemberClusterValidatorForTest(t, tc.networkingEnabled, svcExport)
+			mc := &clusterv1beta1.MemberCluster{ObjectMeta: metav1.ObjectMeta{Name: mcName}}
+			mc.Spec.DeleteOptions = &clusterv1beta1.DeleteOptions{ValidationMode: tc.validationMode}
+			req := buildDeleteRequestFromObject(t, mc)
+
+			resp := validator.Handle(context.Background(), req)
+			if resp.Allowed != tc.expectAllowed {
+				t.Fatalf("expected allowed=%t but got response: %+v", tc.expectAllowed, resp)
+			}
+			if tc.expectedMessageSubstr != "" {
+				if resp.Result == nil || !strings.Contains(resp.Result.Message, tc.expectedMessageSubstr) {
+					t.Fatalf("expected message to contain %q but got: %v", tc.expectedMessageSubstr, resp.Result)
+				}
+			}
+		})
+	}
+}
+
+func newMemberClusterValidatorForTest(t *testing.T, networkingEnabled bool, objs ...client.Object) *memberClusterValidator {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	if err := clusterv1beta1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add member cluster scheme: %v", err)
+	}
+	if err := fleetnetworkingv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add fleet networking scheme: %v", err)
+	}
+	scheme.AddKnownTypes(fleetnetworkingv1alpha1.GroupVersion,
+		&fleetnetworkingv1alpha1.InternalServiceExport{},
+		&fleetnetworkingv1alpha1.InternalServiceExportList{},
+	)
+	metav1.AddToGroupVersion(scheme, fleetnetworkingv1alpha1.GroupVersion)
+
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build()
+	decoder := admission.NewDecoder(scheme)
+
+	return &memberClusterValidator{
+		client:                  fakeClient,
+		decoder:                 decoder,
+		networkingAgentsEnabled: networkingEnabled,
+	}
+}
+
+func buildDeleteRequestFromObject(t *testing.T, mc *clusterv1beta1.MemberCluster) admission.Request {
+	t.Helper()
+
+	raw, err := json.Marshal(mc)
+	if err != nil {
+		t.Fatalf("failed to marshal member cluster: %v", err)
+	}
+
+	return admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Operation: admissionv1.Delete,
+			Name:      mc.Name,
+			OldObject: runtime.RawExtension{Raw: raw},
+		},
+	}
+}
+
+func newInternalServiceExport(clusterID, namespace string) *fleetnetworkingv1alpha1.InternalServiceExport {
+	return &fleetnetworkingv1alpha1.InternalServiceExport{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "sample-service",
+			Namespace: namespace,
+		},
+		Spec: fleetnetworkingv1alpha1.InternalServiceExportSpec{
+			ServiceReference: fleetnetworkingv1alpha1.ExportedObjectReference{
+				ClusterID:       clusterID,
+				Kind:            "Service",
+				Namespace:       "work",
+				Name:            "sample-service",
+				ResourceVersion: "1",
+				Generation:      1,
+				UID:             types.UID("svc-uid"),
+				NamespacedName:  "work/sample-service",
+			},
+		},
+	}
+}

pkg/webhook/webhook.go

@@ -131,14 +131,16 @@ var (
 
 var AddToManagerFuncs []func(manager.Manager) error
 var AddToManagerFleetResourceValidator func(manager.Manager, []string, bool) error
+var AddToManagerMemberclusterValidator func(manager.Manager, bool)
 
 // AddToManager adds all Controllers to the Manager
-func AddToManager(m manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool) error {
+func AddToManager(m manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool, networkingAgentsEnabled bool) error {
 	for _, f := range AddToManagerFuncs {
 		if err := f(m); err != nil {
 			return err
 		}
 	}
+	AddToManagerMemberclusterValidator(m, networkingAgentsEnabled)
 	return AddToManagerFleetResourceValidator(m, whiteListedUsers, denyModifyMemberClusterLabels)
 }