From 233d84f53efaafcc879839d23f75a0b720bad5d9 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Tue, 16 Jul 2024 20:08:52 +0200 Subject: [PATCH] fix(webhook): deny deletion of SubNamespace with child namespaces --- .../templates/generated/generated.yaml | 2 +- config/webhook/manifests.yaml | 2 +- hooks/subnamespace.go | 50 ++++++++++++++++--- hooks/subnamespace_test.go | 26 ++++++++++ 4 files changed, 70 insertions(+), 10 deletions(-) diff --git a/charts/accurate/templates/generated/generated.yaml b/charts/accurate/templates/generated/generated.yaml index 1a2c1b1..c85756b 100644 --- a/charts/accurate/templates/generated/generated.yaml +++ b/charts/accurate/templates/generated/generated.yaml @@ -297,7 +297,7 @@ webhooks: - v1 operations: - CREATE - - UPDATE + - DELETE resources: - subnamespaces sideEffects: None diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 2748159..b09dd16 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -69,7 +69,7 @@ webhooks: - v1 operations: - CREATE - - UPDATE + - DELETE resources: - subnamespaces sideEffects: None diff --git a/hooks/subnamespace.go b/hooks/subnamespace.go index 3ff5e9b..8c8e469 100644 --- a/hooks/subnamespace.go +++ b/hooks/subnamespace.go @@ -53,7 +53,7 @@ func (m *subNamespaceMutator) Handle(ctx context.Context, req admission.Request) return admission.PatchResponseFromRaw(req.Object.Raw, data) } -//+kubebuilder:webhook:path=/validate-accurate-cybozu-com-v1-subnamespace,mutating=false,failurePolicy=fail,sideEffects=None,groups=accurate.cybozu.com,resources=subnamespaces,verbs=create;update,versions=v1,matchPolicy=Equivalent,name=vsubnamespace.kb.io,admissionReviewVersions={v1} +//+kubebuilder:webhook:path=/validate-accurate-cybozu-com-v1-subnamespace,mutating=false,failurePolicy=fail,sideEffects=None,groups=accurate.cybozu.com,resources=subnamespaces,verbs=create;delete,versions=v1,matchPolicy=Equivalent,name=vsubnamespace.kb.io,admissionReviewVersions={v1} type subNamespaceValidator struct { client.Client @@ -64,17 +64,27 @@ type subNamespaceValidator struct { var _ admission.Handler = &subNamespaceValidator{} func (v *subNamespaceValidator) Handle(ctx context.Context, req admission.Request) admission.Response { - if req.Operation != admissionv1.Create { + switch req.Operation { + case admissionv1.Create: + sn := &accuratev1.SubNamespace{} + if err := v.dec.Decode(req, sn); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + return v.handleCreate(ctx, sn) + case admissionv1.Delete: + sn := &accuratev1.SubNamespace{} + if err := v.dec.DecodeRaw(req.OldObject, sn); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + return v.handleDelete(ctx, sn) + default: return admission.Allowed("") } +} - sn := &accuratev1.SubNamespace{} - if err := v.dec.Decode(req, sn); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - +func (v *subNamespaceValidator) handleCreate(ctx context.Context, sn *accuratev1.SubNamespace) admission.Response { ns := &corev1.Namespace{} - if err := v.Get(ctx, client.ObjectKey{Name: req.Namespace}, ns); err != nil { + if err := v.Get(ctx, client.ObjectKey{Name: sn.Namespace}, ns); err != nil { return admission.Errored(http.StatusInternalServerError, err) } @@ -103,6 +113,30 @@ func (v *subNamespaceValidator) Handle(ctx context.Context, req admission.Reques return admission.Allowed("") } +func (v *subNamespaceValidator) handleDelete(ctx context.Context, sn *accuratev1.SubNamespace) admission.Response { + ns := &corev1.Namespace{} + if err := v.Get(ctx, client.ObjectKey{Name: sn.Name}, ns); err != nil { + if apierrors.IsNotFound(err) { + return admission.Allowed("") + } + return admission.Errored(http.StatusInternalServerError, err) + } + + if ns.Labels[constants.LabelParent] != sn.Namespace { + return admission.Allowed("") + } + + children := &corev1.NamespaceList{} + if err := v.List(ctx, children, client.MatchingFields{constants.NamespaceParentKey: ns.Name}); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + if len(children.Items) > 0 { + return admission.Denied("child namespaces exist") + } + + return admission.Allowed("") +} + func (v *subNamespaceValidator) getRootNamespace(ctx context.Context, ns *corev1.Namespace) (*corev1.Namespace, error) { if ns.Labels[constants.LabelType] == constants.NSTypeRoot { return ns, nil diff --git a/hooks/subnamespace_test.go b/hooks/subnamespace_test.go index 5fc685b..d11f68a 100644 --- a/hooks/subnamespace_test.go +++ b/hooks/subnamespace_test.go @@ -74,6 +74,32 @@ var _ = Describe("SubNamespace webhook", func() { Expect(controllerutil.ContainsFinalizer(sn, constants.Finalizer)).To(BeTrue()) }) + It("should deny deletion of SubNamespace with child namespaces", func() { + nsR := &corev1.Namespace{} + nsR.GenerateName = "ns-" + nsR.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot} + Expect(k8sClient.Create(ctx, nsR)).To(Succeed()) + + snP := &accuratev2.SubNamespace{} + snP.Namespace = nsR.Name + snP.GenerateName = "ns-p-" + Expect(k8sClient.Create(ctx, snP)).To(Succeed()) + // Create sub-namespace since no controllers present in this test setup + nsP := &corev1.Namespace{} + nsP.Name = snP.Name + nsP.Labels = map[string]string{constants.LabelParent: nsR.Name} + Expect(k8sClient.Create(ctx, nsP)).To(Succeed()) + + ns := &corev1.Namespace{} + ns.GenerateName = "ns-c-" + ns.Labels = map[string]string{constants.LabelParent: nsP.Name} + Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + + err := k8sClient.Delete(ctx, snP) + Expect(err).To(HaveOccurred()) + Expect(errors.ReasonForError(err)).Should(Equal(metav1.StatusReasonForbidden)) + }) + Context("Naming Policy", func() { When("the root namespace name is matched some Root Naming Policies", func() { When("the SubNamespace name is matched to the Root's Match Naming Policy", func() {