From fa0d6f65a5f4ffb523e9ae865fd78bb7486e2016 Mon Sep 17 00:00:00 2001
From: ymmt <ymmt2005@gmail.com>
Date: Mon, 25 Oct 2021 22:57:32 +0000
Subject: [PATCH] Do not delete non-propagated resources in template/root
 namespaces

There is a bug that would delete non-propagated resources in
namespaces that are neither a sub-namespace nor a template instance
when resources are annotated with `accurate.cybozu.com/propagate=update`.
---
 controllers/namespace_controller.go      | 10 ++++++++
 controllers/namespace_controller_test.go | 29 ++++++++++++++++++++++++
 docs/reconcile.md                        |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/controllers/namespace_controller.go b/controllers/namespace_controller.go
index e11b8a8..dd446d8 100644
--- a/controllers/namespace_controller.go
+++ b/controllers/namespace_controller.go
@@ -59,6 +59,9 @@ func (r *NamespaceReconciler) reconcile(ctx context.Context, ns *corev1.Namespac
 		}
 		// a template instance may also be a root or a template namespace, so don't return here.
 	} else {
+		// Here, ns is neither a sub-namespace nor a template instance.
+		// Since there is no parent|template namespace for this namespace,
+		// propagated resources in this namespace, if any, should be deleted.
 		if err := r.deletePropagatedResources(ctx, ns); err != nil {
 			return err
 		}
@@ -244,6 +247,13 @@ func (r *NamespaceReconciler) deleteResource(ctx context.Context, res *unstructu
 	}
 	for i := range l.Items {
 		obj := &l.Items[i]
+
+		from := obj.GetAnnotations()[constants.AnnFrom]
+		if from == "" {
+			// don't delete origins
+			continue
+		}
+
 		if err := r.Delete(ctx, obj); err != nil {
 			return fmt.Errorf("failed to delete %s/%s of %s: %w", ns, obj.GetName(), gvkStr, err)
 		}
diff --git a/controllers/namespace_controller_test.go b/controllers/namespace_controller_test.go
index 1c7057a..7f181db 100644
--- a/controllers/namespace_controller_test.go
+++ b/controllers/namespace_controller_test.go
@@ -331,6 +331,35 @@ var _ = Describe("Namespace controller", func() {
 		Expect(tmpl2.Labels["team"]).Should(Equal("neco"))
 	})
 
+	It("should not delete resources in an independent namespace", func() {
+		secret := &corev1.Secret{}
+		secret.Namespace = "default"
+		secret.Name = "independent"
+		secret.Annotations = map[string]string{constants.AnnPropagate: constants.PropagateUpdate}
+		secret.Data = map[string][]byte{"foo": []byte("bar")}
+
+		err := k8sClient.Create(ctx, secret)
+		Expect(err).NotTo(HaveOccurred())
+
+		time.Sleep(100 * time.Millisecond)
+
+		ns := &corev1.Namespace{}
+		err = k8sClient.Get(ctx, client.ObjectKey{Name: "default"}, ns)
+		Expect(err).NotTo(HaveOccurred())
+
+		if ns.Labels == nil {
+			ns.Labels = make(map[string]string)
+		}
+		ns.Labels["accurate-test"] = "test"
+		err = k8sClient.Update(ctx, ns)
+		Expect(err).NotTo(HaveOccurred())
+
+		Consistently(func() error {
+			s := &corev1.Secret{}
+			return k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "independent"}, s)
+		}, 1, 0.1).Should(Succeed())
+	})
+
 	It("should implement a sub namespace correctly", func() {
 		root := &corev1.Namespace{}
 		root.Name = "root"
diff --git a/docs/reconcile.md b/docs/reconcile.md
index 2302d50..f13383e 100644
--- a/docs/reconcile.md
+++ b/docs/reconcile.md
@@ -21,7 +21,7 @@ These namespaces reference a template namespace and propagate the labels, annota
     - the value of `accurate.cybozu.com/from` annotation is not the template namespace name, or
     - there is not a resource of the same kind and the same name in the template namespace.
 
-### Namespaces w/o `accurate.cybozu.com/from` and `accurate.cybozu.com/template` labels
+### Namespaces w/o `accurate.cybozu.com/type` and `accurate.cybozu.com/template` labels
 
 If these labels are removed from the Namespace, Accurate should delete propagated resources with mode == `update`.