diff --git a/charts/spiderpool/README.md b/charts/spiderpool/README.md index be01dacb2..f0eae4307 100644 --- a/charts/spiderpool/README.md +++ b/charts/spiderpool/README.md @@ -343,6 +343,7 @@ helm install spiderpool spiderpool/spiderpool --wait --namespace kube-system \ | `spiderpoolController.healthChecking.readinessProbe.failureThreshold` | the failure threshold of startup probe for spiderpoolController health checking | `3` | | `spiderpoolController.healthChecking.readinessProbe.periodSeconds` | the period seconds of startup probe for spiderpoolController health checking | `10` | | `spiderpoolController.webhookPort` | the http port for spiderpoolController webhook | `5722` | +| `spiderpoolController.enableValidatingResourcesDeletedWebhook` | enable validating resources deleted webhook for spiderpoolController | `false` | | `spiderpoolController.podResourceInject.enabled` | enable pod resource inject | `false` | | `spiderpoolController.podResourceInject.namespacesExclude` | exclude the namespaces of the pod resource inject | `["kube-system","spiderpool","metallb-system","istio-system"]` | | `spiderpoolController.podResourceInject.namespacesInclude` | include the namespaces of the pod resource inject, empty means all namespaces but exclude the namespaces in namespacesExclude, not empty means only include the namespaces in namespacesInclude | `[]` | diff --git a/charts/spiderpool/templates/configmap.yaml b/charts/spiderpool/templates/configmap.yaml index 57a215840..d519e30fd 100644 --- a/charts/spiderpool/templates/configmap.yaml +++ b/charts/spiderpool/templates/configmap.yaml @@ -24,6 +24,7 @@ data: enableAutoPoolForApplication: {{ .Values.ipam.spiderSubnet.autoPool.enable }} enableIPConflictDetection: {{ .Values.ipam.enableIPConflictDetection }} enableGatewayDetection: {{ .Values.ipam.enableGatewayDetection }} + enableValidatingResourcesDeletedWebhook: {{ .Values.spiderpoolController.enableValidatingResourcesDeletedWebhook }} {{- if and .Values.ipam.spiderSubnet.enable .Values.ipam.spiderSubnet.autoPool.enable }} clusterSubnetDefaultFlexibleIPNumber: {{ .Values.ipam.spiderSubnet.autoPool.defaultRedundantIPNumber }} {{- else}} diff --git a/charts/spiderpool/values.yaml b/charts/spiderpool/values.yaml index 1ed1e461a..4f673517e 100644 --- a/charts/spiderpool/values.yaml +++ b/charts/spiderpool/values.yaml @@ -670,6 +670,9 @@ spiderpoolController: ## @param spiderpoolController.webhookPort the http port for spiderpoolController webhook webhookPort: 5722 + ## @param spiderpoolController.enableValidatingResourcesDeletedWebhook enable validating resources deleted webhook for spiderpoolController + enableValidatingResourcesDeletedWebhook: false + podResourceInject: ## @param spiderpoolController.podResourceInject.enabled enable pod resource inject enabled: false diff --git a/cmd/spiderpool-controller/cmd/daemon.go b/cmd/spiderpool-controller/cmd/daemon.go index 20db104d6..df0d754c1 100644 --- a/cmd/spiderpool-controller/cmd/daemon.go +++ b/cmd/spiderpool-controller/cmd/daemon.go @@ -338,11 +338,12 @@ func initControllerServiceManagers(ctx context.Context) { logger.Debug("Begin to set up IPPool webhook") if err := (&ippoolmanager.IPPoolWebhook{ - Client: controllerContext.CRDManager.GetClient(), - APIReader: controllerContext.CRDManager.GetAPIReader(), - EnableIPv4: controllerContext.Cfg.EnableIPv4, - EnableIPv6: controllerContext.Cfg.EnableIPv6, - EnableSpiderSubnet: controllerContext.Cfg.EnableSpiderSubnet, + Client: controllerContext.CRDManager.GetClient(), + APIReader: controllerContext.CRDManager.GetAPIReader(), + EnableIPv4: controllerContext.Cfg.EnableIPv4, + EnableIPv6: controllerContext.Cfg.EnableIPv6, + EnableSpiderSubnet: controllerContext.Cfg.EnableSpiderSubnet, + EnableValidatingResourcesDeletedWebhook: controllerContext.Cfg.EnableValidatingResourcesDeletedWebhook, }).SetupWebhookWithManager(controllerContext.CRDManager); err != nil { logger.Fatal(err.Error()) } @@ -368,10 +369,11 @@ func initControllerServiceManagers(ctx context.Context) { logger.Debug("Begin to set up Subnet webhook") if err := (&subnetmanager.SubnetWebhook{ - Client: controllerContext.CRDManager.GetClient(), - APIReader: controllerContext.CRDManager.GetAPIReader(), - EnableIPv4: controllerContext.Cfg.EnableIPv4, - EnableIPv6: controllerContext.Cfg.EnableIPv6, + Client: controllerContext.CRDManager.GetClient(), + APIReader: controllerContext.CRDManager.GetAPIReader(), + EnableIPv4: controllerContext.Cfg.EnableIPv4, + EnableIPv6: controllerContext.Cfg.EnableIPv6, + EnableValidatingResourcesDeletedWebhook: controllerContext.Cfg.EnableValidatingResourcesDeletedWebhook, }).SetupWebhookWithManager(controllerContext.CRDManager); err != nil { logger.Fatal(err.Error()) } diff --git a/pkg/ippoolmanager/ippool_webhook.go b/pkg/ippoolmanager/ippool_webhook.go index 1506e0e3a..9452cbe8b 100644 --- a/pkg/ippoolmanager/ippool_webhook.go +++ b/pkg/ippoolmanager/ippool_webhook.go @@ -6,6 +6,7 @@ package ippoolmanager import ( "context" "errors" + "fmt" "strings" "go.uber.org/zap" @@ -30,9 +31,10 @@ type IPPoolWebhook struct { Client client.Client APIReader client.Reader - EnableIPv4 bool - EnableIPv6 bool - EnableSpiderSubnet bool + EnableIPv4 bool + EnableIPv6 bool + EnableSpiderSubnet bool + EnableValidatingResourcesDeletedWebhook bool } func (iw *IPPoolWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -136,6 +138,10 @@ func (iw *IPPoolWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runt // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. func (iw *IPPoolWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + if !iw.EnableValidatingResourcesDeletedWebhook { + return nil, nil + } + ipPool := obj.(*spiderpoolv2beta1.SpiderIPPool) logger := WebhookLogger.Named("Validating").With( @@ -149,7 +155,7 @@ func (iw *IPPoolWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) return nil, apierrors.NewForbidden( schema.GroupResource{Group: constant.SpiderpoolAPIGroup, Resource: "spiderippools"}, ipPool.Name, - errors.New("cannot delete an IPPool with allocated IPs"), + fmt.Errorf("cannot delete an IPPool with allocated IPs(%v)", *ipPool.Status.AllocatedIPCount), ) } return nil, nil diff --git a/pkg/ippoolmanager/ippool_webhook_test.go b/pkg/ippoolmanager/ippool_webhook_test.go index 07766f5ed..1684c1da6 100644 --- a/pkg/ippoolmanager/ippool_webhook_test.go +++ b/pkg/ippoolmanager/ippool_webhook_test.go @@ -2152,6 +2152,32 @@ var _ = Describe("IPPoolWebhook", Label("ippool_webhook_test"), func() { Expect(err).NotTo(HaveOccurred()) Expect(warns).To(BeNil()) }) + + It("should allow deletion when EnableValidatingResourcesDeletedWebhook is false", func() { + ipPoolWebhook.EnableValidatingResourcesDeletedWebhook = false + warns, err := ipPoolWebhook.ValidateDelete(ctx, ipPoolT) + Expect(err).NotTo(HaveOccurred()) + Expect(warns).To(BeNil()) + }) + + It("should prevent deletion when IPPool has allocated IPs", func() { + ipPoolWebhook.EnableValidatingResourcesDeletedWebhook = true + allocatedIPs := int64(5) + ipPoolT.Status.AllocatedIPCount = &allocatedIPs + warns, err := ipPoolWebhook.ValidateDelete(ctx, ipPoolT) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsForbidden(err)).To(BeTrue()) + Expect(warns).To(BeNil()) + }) + + It("should allow deletion when IPPool has no allocated IPs", func() { + ipPoolWebhook.EnableValidatingResourcesDeletedWebhook = true + allocatedIPs := int64(0) + ipPoolT.Status.AllocatedIPCount = &allocatedIPs + warns, err := ipPoolWebhook.ValidateDelete(ctx, ipPoolT) + Expect(err).NotTo(HaveOccurred()) + Expect(warns).To(BeNil()) + }) }) }) }) diff --git a/pkg/podmanager/pod_manager_suite_test.go b/pkg/podmanager/pod_manager_suite_test.go index bc2b70098..a7c57828a 100644 --- a/pkg/podmanager/pod_manager_suite_test.go +++ b/pkg/podmanager/pod_manager_suite_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/spidernet-io/spiderpool/pkg/podmanager" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -15,8 +16,6 @@ import ( k8stesting "k8s.io/client-go/testing" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/spidernet-io/spiderpool/pkg/podmanager" ) var scheme *runtime.Scheme @@ -58,4 +57,5 @@ var _ = BeforeSuite(func() { fakeAPIReader, ) Expect(err).NotTo(HaveOccurred()) + }) diff --git a/pkg/podmanager/pod_webhook.go b/pkg/podmanager/pod_webhook.go index 7afa56096..93e63e818 100644 --- a/pkg/podmanager/pod_webhook.go +++ b/pkg/podmanager/pod_webhook.go @@ -30,7 +30,7 @@ type PodWebhook interface { admission.CustomValidator } -type podWebhook struct { +type PWebhook struct { spiderClient crdclientset.Interface } @@ -46,7 +46,7 @@ func InitPodWebhook(mgr ctrl.Manager) error { return err } - pw := &podWebhook{ + pw := &PWebhook{ spiderClient: spiderClient, } @@ -67,7 +67,7 @@ func InitPodWebhook(mgr ctrl.Manager) error { // - obj: The runtime object (expected to be a Pod) // // Returns an error if defaulting fails. -func (pw *podWebhook) Default(ctx context.Context, obj runtime.Object) error { +func (pw *PWebhook) Default(ctx context.Context, obj runtime.Object) error { logger := logutils.FromContext(ctx) pod := obj.(*corev1.Pod) mutateLogger := logger.Named("PodMutating").With( @@ -97,18 +97,18 @@ func (pw *podWebhook) Default(ctx context.Context, obj runtime.Object) error { // ValidateCreate implements the validation webhook for pod creation. // Currently, it performs no validation and always returns nil. -func (pw *podWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (pw *PWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return nil, nil } // ValidateUpdate implements the validation webhook for pod updates. // Currently, it performs no validation and always returns nil. -func (pw *podWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (pw *PWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { return nil, nil } // ValidateDelete implements the validation webhook for pod deletion. // Currently, it performs no validation and always returns nil. -func (pw *podWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (pw *PWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return nil, nil } diff --git a/pkg/podmanager/pod_webhook_test.go b/pkg/podmanager/pod_webhook_test.go new file mode 100644 index 000000000..d788e23a9 --- /dev/null +++ b/pkg/podmanager/pod_webhook_test.go @@ -0,0 +1,72 @@ +// Copyright 2025 Authors of spidernet-io +// SPDX-License-Identifier: Apache-2.0 +package podmanager_test + +// import ( +// "context" +// "testing" + +// . "github.com/onsi/ginkgo/v2" +// . "github.com/onsi/gomega" +// "github.com/spidernet-io/spiderpool/pkg/podmanager" +// corev1 "k8s.io/api/core/v1" +// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +// "k8s.io/client-go/rest" +// "sigs.k8s.io/controller-runtime/pkg/client" +// "sigs.k8s.io/controller-runtime/pkg/manager" +// ) + +// func TestPodWebhook(t *testing.T) { +// RegisterFailHandler(Fail) +// RunSpecs(t, "Pod Webhook Suite") +// } + +// var _ = Describe("Pod Webhook", Label("pod_webhook_test"), func() { +// var ( +// err error +// mgr manager.Manager +// ctx context.Context +// pod *corev1.Pod +// ) + +// BeforeEach(func() { +// // Create a new manager + +// mgr, err = manager.New(cfg, manager.Options{ +// NewClient: func(config *rest.Config, options client.Options) (client.Client, error) { +// return nil, nil +// }, +// }) +// Expect(err).NotTo(HaveOccurred()) + +// pod = &corev1.Pod{ +// ObjectMeta: metav1.ObjectMeta{ +// Namespace: "default", +// GenerateName: "test-pod", +// Annotations: map[string]string{}, +// }, +// } +// }) + +// Context("InitPodWebhook", func() { +// It("should initialize without error", func() { +// err := podmanager.InitPodWebhook(mgr) +// Expect(err).NotTo(HaveOccurred()) +// }) +// }) + +// Context("Default", func() { +// It("should not inject resources if no annotations are present", func() { +// pw := &podmanager.PWebhook{} +// err := pw.Default(ctx, pod) +// Expect(err).NotTo(HaveOccurred()) +// }) + +// It("should inject resources if annotations are present", func() { +// pod.Annotations["spiderpool.io/pod-resource-inject"] = "true" +// pw := podmanager.PWebhook{} +// err := pw.Default(ctx, pod) +// Expect(err).NotTo(HaveOccurred()) +// }) +// }) +// }) diff --git a/pkg/subnetmanager/subnet_webhook.go b/pkg/subnetmanager/subnet_webhook.go index eac7bcc19..54eee30c7 100644 --- a/pkg/subnetmanager/subnet_webhook.go +++ b/pkg/subnetmanager/subnet_webhook.go @@ -6,6 +6,7 @@ package subnetmanager import ( "context" "errors" + "fmt" "strings" "go.uber.org/zap" @@ -30,8 +31,9 @@ type SubnetWebhook struct { Client client.Client APIReader client.Reader - EnableIPv4 bool - EnableIPv6 bool + EnableIPv4 bool + EnableIPv6 bool + EnableValidatingResourcesDeletedWebhook bool } func (sw *SubnetWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -135,6 +137,10 @@ func (sw *SubnetWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runt // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. func (sw *SubnetWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + if !sw.EnableValidatingResourcesDeletedWebhook { + return nil, nil + } + subnet := obj.(*spiderpoolv2beta1.SpiderSubnet) logger := WebhookLogger.Named("Validating").With( @@ -148,7 +154,7 @@ func (sw *SubnetWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) return nil, apierrors.NewForbidden( schema.GroupResource{Group: constant.SpiderpoolAPIGroup, Resource: "spidersubnets"}, subnet.Name, - errors.New("cannot delete an Subnet with allocated IPs"), + fmt.Errorf("cannot delete an Subnet with allocated IPs(%v)", *subnet.Status.AllocatedIPCount), ) } return nil, nil diff --git a/pkg/subnetmanager/subnet_webhook_test.go b/pkg/subnetmanager/subnet_webhook_test.go index abb9ba197..3ee037f06 100644 --- a/pkg/subnetmanager/subnet_webhook_test.go +++ b/pkg/subnetmanager/subnet_webhook_test.go @@ -1488,6 +1488,32 @@ var _ = Describe("SubnetWebhook", Label("subnet_webhook_test"), func() { Expect(err).NotTo(HaveOccurred()) Expect(warns).To(BeNil()) }) + + It("should allow deletion when EnableValidatingResourcesDeletedWebhook is false", func() { + subnetWebhook.EnableValidatingResourcesDeletedWebhook = false + warns, err := subnetWebhook.ValidateDelete(ctx, subnetT) + Expect(err).NotTo(HaveOccurred()) + Expect(warns).To(BeNil()) + }) + + It("should prevent deletion when Subnet has allocated IPs", func() { + subnetWebhook.EnableValidatingResourcesDeletedWebhook = true + allocatedIPs := int64(5) + subnetT.Status.AllocatedIPCount = &allocatedIPs + warns, err := subnetWebhook.ValidateDelete(ctx, subnetT) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsForbidden(err)).To(BeTrue()) + Expect(warns).To(BeNil()) + }) + + It("should allow deletion when Subnet has no allocated IPs", func() { + subnetWebhook.EnableValidatingResourcesDeletedWebhook = true + allocatedIPs := int64(0) + subnetT.Status.AllocatedIPCount = &allocatedIPs + warns, err := subnetWebhook.ValidateDelete(ctx, subnetT) + Expect(err).NotTo(HaveOccurred()) + Expect(warns).To(BeNil()) + }) }) }) }) diff --git a/pkg/types/k8s.go b/pkg/types/k8s.go index 67b57856c..c33072820 100644 --- a/pkg/types/k8s.go +++ b/pkg/types/k8s.go @@ -121,6 +121,7 @@ type SpiderpoolConfigmapConfig struct { EnableGatewayDetection bool `yaml:"enableGatewayDetection"` ClusterSubnetAutoPoolDefaultRedundantIPNumber int `yaml:"clusterSubnetAutoPoolDefaultRedundantIPNumber"` PodResourceInjectConfig PodResourceInjectConfig `yaml:"podResourceInject"` + EnableValidatingResourcesDeletedWebhook bool `yaml:"enableValidatingResourcesDeletedWebhook"` } type PodResourceInjectConfig struct { diff --git a/test/e2e/common/resourceclaim.go b/test/e2e/common/resourceclaim.go index bed7206c3..18e2021bf 100644 --- a/test/e2e/common/resourceclaim.go +++ b/test/e2e/common/resourceclaim.go @@ -2,59 +2,59 @@ // SPDX-License-Identifier: Apache-2.0 package common -import ( - "errors" - "fmt" - - frame "github.com/spidernet-io/e2eframework/framework" - - resourcev1alpha2 "k8s.io/api/resource/v1alpha2" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - apitypes "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func ListResourceClaim(f *frame.Framework, opts ...client.ListOption) (*resourcev1alpha2.ResourceClaimList, error) { - list := resourcev1alpha2.ResourceClaimList{} - if err := f.ListResource(&list, opts...); err != nil { - return nil, err - } - - return &list, nil -} - -func GetResourceClaim(f *frame.Framework, name, ns string) (*resourcev1alpha2.ResourceClaim, error) { - if name == "" || f == nil { - return nil, errors.New("wrong input") - } - - v := apitypes.NamespacedName{Name: name, Namespace: ns} - existing := &resourcev1alpha2.ResourceClaim{} - e := f.GetResource(v, existing) - if e != nil { - return nil, e - } - return existing, nil -} - -func CreateResourceClaimTemplate(f *frame.Framework, rct *resourcev1alpha2.ResourceClaimTemplate, opts ...client.CreateOption) error { - if f == nil || rct == nil { - return fmt.Errorf("invalid parameters") - } - - return f.CreateResource(rct, opts...) -} - -func DeleteResourceClaimTemplate(f *frame.Framework, name, ns string, opts ...client.DeleteOption) error { - if name == "" || ns == "" || f == nil { - return errors.New("wrong input") - } - - rct := &resourcev1alpha2.ResourceClaimTemplate{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - } - return f.DeleteResource(rct, opts...) -} +// import ( +// "errors" +// "fmt" + +// frame "github.com/spidernet-io/e2eframework/framework" + +// resourcev1alpha2 "k8s.io/api/resource/v1alpha2" +// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +// apitypes "k8s.io/apimachinery/pkg/types" +// "sigs.k8s.io/controller-runtime/pkg/client" +// ) + +// func ListResourceClaim(f *frame.Framework, opts ...client.ListOption) (*resourcev1alpha2.ResourceClaimList, error) { +// list := resourcev1alpha2.ResourceClaimList{} +// if err := f.ListResource(&list, opts...); err != nil { +// return nil, err +// } + +// return &list, nil +// } + +// func GetResourceClaim(f *frame.Framework, name, ns string) (*resourcev1alpha2.ResourceClaim, error) { +// if name == "" || f == nil { +// return nil, errors.New("wrong input") +// } + +// v := apitypes.NamespacedName{Name: name, Namespace: ns} +// existing := &resourcev1alpha2.ResourceClaim{} +// e := f.GetResource(v, existing) +// if e != nil { +// return nil, e +// } +// return existing, nil +// } + +// func CreateResourceClaimTemplate(f *frame.Framework, rct *resourcev1alpha2.ResourceClaimTemplate, opts ...client.CreateOption) error { +// if f == nil || rct == nil { +// return fmt.Errorf("invalid parameters") +// } + +// return f.CreateResource(rct, opts...) +// } + +// func DeleteResourceClaimTemplate(f *frame.Framework, name, ns string, opts ...client.DeleteOption) error { +// if name == "" || ns == "" || f == nil { +// return errors.New("wrong input") +// } + +// rct := &resourcev1alpha2.ResourceClaimTemplate{ +// ObjectMeta: metav1.ObjectMeta{ +// Name: name, +// Namespace: ns, +// }, +// } +// return f.DeleteResource(rct, opts...) +// }