Merge pull request #145 from erikgb/fix-sn-delete-webhook

fix(webhook): deny deletion of SubNamespace with child namespaces

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

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