diff --git a/cmd/hubagent/main.go b/cmd/hubagent/main.go index f896ab71a..42409c0ea 100644 --- a/cmd/hubagent/main.go +++ b/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 } diff --git a/pkg/webhook/add_handler.go b/pkg/webhook/add_handler.go index 08380e6b8..24f3a6eb8 100644 --- a/pkg/webhook/add_handler.go +++ b/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) diff --git a/pkg/webhook/membercluster/membercluster_validating_webhook.go b/pkg/webhook/membercluster/membercluster_validating_webhook.go index 24df38cdd..895797bd2 100644 --- a/pkg/webhook/membercluster/membercluster_validating_webhook.go +++ b/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) diff --git a/pkg/webhook/membercluster/membercluster_validating_webhook_test.go b/pkg/webhook/membercluster/membercluster_validating_webhook_test.go new file mode 100644 index 000000000..a0497b5da --- /dev/null +++ b/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", + }, + }, + } +} diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 781833e3e..4f4c89b56 100644 --- a/pkg/webhook/webhook.go +++ b/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) }