Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a knob to initiate webhook deletion checks for ip resources #4675

Merged
merged 1 commit into from
Feb 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions charts/spiderpool/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | `[]` |
Expand Down
1 change: 1 addition & 0 deletions charts/spiderpool/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
3 changes: 3 additions & 0 deletions charts/spiderpool/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions cmd/spiderpool-controller/cmd/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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())
}
Expand Down
14 changes: 10 additions & 4 deletions pkg/ippoolmanager/ippool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package ippoolmanager
import (
"context"
"errors"
"fmt"
"strings"

"go.uber.org/zap"
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions pkg/ippoolmanager/ippool_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
})
})
4 changes: 2 additions & 2 deletions pkg/podmanager/pod_manager_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ 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"
k8sscheme "k8s.io/client-go/kubernetes/scheme"
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
Expand Down Expand Up @@ -58,4 +57,5 @@ var _ = BeforeSuite(func() {
fakeAPIReader,
)
Expect(err).NotTo(HaveOccurred())

})
12 changes: 6 additions & 6 deletions pkg/podmanager/pod_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
admission.CustomValidator
}

type podWebhook struct {
type PWebhook struct {
spiderClient crdclientset.Interface
}

Expand All @@ -46,7 +46,7 @@
return err
}

pw := &podWebhook{
pw := &PWebhook{

Check warning on line 49 in pkg/podmanager/pod_webhook.go

View check run for this annotation

Codecov / codecov/patch

pkg/podmanager/pod_webhook.go#L49

Added line #L49 was not covered by tests
spiderClient: spiderClient,
}

Expand All @@ -67,7 +67,7 @@
// - 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 {

Check warning on line 70 in pkg/podmanager/pod_webhook.go

View check run for this annotation

Codecov / codecov/patch

pkg/podmanager/pod_webhook.go#L70

Added line #L70 was not covered by tests
logger := logutils.FromContext(ctx)
pod := obj.(*corev1.Pod)
mutateLogger := logger.Named("PodMutating").With(
Expand Down Expand Up @@ -97,18 +97,18 @@

// 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) {

Check warning on line 100 in pkg/podmanager/pod_webhook.go

View check run for this annotation

Codecov / codecov/patch

pkg/podmanager/pod_webhook.go#L100

Added line #L100 was not covered by tests
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) {

Check warning on line 106 in pkg/podmanager/pod_webhook.go

View check run for this annotation

Codecov / codecov/patch

pkg/podmanager/pod_webhook.go#L106

Added line #L106 was not covered by tests
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) {

Check warning on line 112 in pkg/podmanager/pod_webhook.go

View check run for this annotation

Codecov / codecov/patch

pkg/podmanager/pod_webhook.go#L112

Added line #L112 was not covered by tests
return nil, nil
}
72 changes: 72 additions & 0 deletions pkg/podmanager/pod_webhook_test.go
Original file line number Diff line number Diff line change
@@ -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())
// })
// })
// })
12 changes: 9 additions & 3 deletions pkg/subnetmanager/subnet_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package subnetmanager
import (
"context"
"errors"
"fmt"
"strings"

"go.uber.org/zap"
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions pkg/subnetmanager/subnet_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
})
})
1 change: 1 addition & 0 deletions pkg/types/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading