Merge a30dcea311 into 6fbf962b1c

charts/postgres-operator/crds/operatorconfigurations.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: operatorconfigurations.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all operatorconfiguration custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

charts/postgres-operator/crds/postgresqls.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: postgresqls.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all postgresql custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

charts/postgres-operator/crds/postgresteams.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: postgresteams.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all postgresteam custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

docs/administrator.md

@@ -22,6 +22,34 @@ By default, the operator will register the CRDs in the `all` category so
 that resources are listed on `kubectl get all` commands. The `crd_categories`
 config option allows for customization of categories.
 
+### Protecting CRDs from accidental deletion
+
+Deleting a CRD in Kubernetes is destructive: the API server also deletes
+every custom resource of that kind across the cluster. A stray
+`kubectl delete crd` therefore wipes out all `postgresql`,
+`operatorconfiguration`, `postgresteam` and `fabriceventstream` objects in
+one go.
+
+To prevent this, the operator ships its CRDs with a `acid.zalan.do/crd-protection`
+finalizer on the `metadata` of each CRD. While the finalizer is present, the
+CRD is stuck in `Terminating` and the custom resources are not removed. On
+startup, the operator also re-applies the finalizer to the `postgresql` and
+`operatorconfiguration` CRDs if they are missing it, so older deployments of
+those two CRDs pick up the safety net after upgrading.
+
+To intentionally delete a CRD, remove the finalizer first:
+
+```bash
+kubectl patch crd postgresqls.acid.zalan.do -p '{"metadata":{"finalizers":[]}}' --type=merge
+kubectl delete crd postgresqls.acid.zalan.do
+```
+
+As noted in the
+[CRD deletion checklist](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#delete-a-customresourcedefinition),
+delete the CRs first when possible, wait for their finalizers to clear, and
+only then drop the CRDs - this avoids `Terminating` CRDs and orphaned
+external state.
+
 ## Upgrading the operator
 
 The Postgres Operator is upgraded by changing the docker image within the

e2e/tests/k8s_api.py

@@ -28,6 +28,7 @@ class K8sApi:
         self.custom_objects_api = client.CustomObjectsApi()
         self.policy_v1 = client.PolicyV1Api()
         self.storage_v1_api = client.StorageV1Api()
+        self.apiextensions_v1 = client.ApiextensionsV1Api()
 
 
 class K8s:

e2e/tests/test_e2e.py

@@ -2469,6 +2469,151 @@ class EndToEndTestCase(unittest.TestCase):
         self.eventuallyEqual(lambda: k8s.count_pods_with_label(cluster_labels), 2, "Postgresql StatefulSet are scale to 2")
         self.eventuallyEqual(lambda: k8s.count_running_pods(), 2, "All pods are running")
 
+    @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
+    def test_zy_crd_protection_finalizer(self):
+        '''
+           CRDs ship with a 'acid.zalan.do/crd-protection' finalizer so that an
+           accidental 'kubectl delete crd' does not cascade-delete every
+           custom resource of that kind. Exercise the protection against the
+           most critical CRD - the postgresqls one - by attempting to delete
+           it while a live 'postgresql' CR (the e2e cluster) is in place,
+           then verify the cluster survives both the blocked attempt and the
+           cascade once the finalizer is cleared. The test re-creates the
+           CRD and the cluster in a finally block so a failed assertion
+           cannot leave the suite in a broken state.
+        '''
+        k8s = self.k8s
+        crd_api = k8s.api.apiextensions_v1
+        custom_api = k8s.api.custom_objects_api
+
+        target_crd = "postgresqls.acid.zalan.do"
+        cluster_name = "acid-minimal-cluster"
+        cluster_namespace = "default"
+        cluster_group = "acid.zalan.do"
+        cluster_version = "v1"
+        cluster_plural = "postgresqls"
+        cluster_label = "application=spilo,cluster-name=" + cluster_name
+
+        # Step 1: every operator CRD must carry the protection finalizer on
+        # its metadata. This is the assertion that fails before the change
+        # is in place.
+        for name in (
+            "postgresqls.acid.zalan.do",
+            "operatorconfigurations.acid.zalan.do",
+            "postgresteams.acid.zalan.do",
+            "fabriceventstreams.zalando.org",
+        ):
+            crd = crd_api.read_custom_resource_definition(name)
+            finalizers = crd.metadata.finalizers or []
+            self.assertIn(
+                "acid.zalan.do/crd-protection",
+                finalizers,
+                f"CRD {name} is missing the protection finalizer: {finalizers}",
+            )
+
+        # Step 2: snapshot the live 'postgresql' CR so the finally block can
+        # restore the cluster after the test. The cluster is real workload
+        # data - the whole point of the protection - and losing it would
+        # defeat the proof and break the rest of the suite.
+        cluster_snapshot = custom_api.get_namespaced_custom_object(
+            cluster_group, cluster_version, cluster_namespace,
+            cluster_plural, cluster_name)
+
+        try:
+            # Step 3: attempt to delete the postgresqls CRD. With the
+            # protection finalizer in place the CRD must stay around (stuck
+            # in 'Terminating') and the cluster CR must survive. Before the
+            # change, both would be wiped in one shot.
+            crd_api.delete_custom_resource_definition(target_crd)
+
+            def crd_still_protected():
+                crd = crd_api.read_custom_resource_definition(target_crd)
+                return "acid.zalan.do/crd-protection" in (crd.metadata.finalizers or [])
+
+            self.eventuallyTrue(
+                crd_still_protected,
+                f"CRD {target_crd} should still exist with its protection finalizer set",
+            )
+
+            def cluster_still_present():
+                return self._custom_object_exists(
+                    custom_api, cluster_group, cluster_version, cluster_namespace,
+                    cluster_plural, cluster_name)
+
+            self.eventuallyTrue(
+                cluster_still_present,
+                "postgresql CR was cascade-deleted while the CRD was stuck "
+                "in Terminating; the protection finalizer did not hold.",
+            )
+
+            # Step 4: removing the finalizer (the documented manual
+            # intervention) is what actually unblocks the deletion. The CRD
+            # and the cluster CR are then fully gone.
+            clear_finalizers_patch = [
+                {"op": "replace", "path": "/metadata/finalizers", "value": []}
+            ]
+            crd_api.patch_custom_resource_definition(target_crd, clear_finalizers_patch)
+
+            self.eventuallyEqual(
+                lambda: self._crd_exists(crd_api, target_crd),
+                False,
+                f"CRD {target_crd} should be fully removed after finalizer cleared",
+            )
+            self.eventuallyEqual(
+                cluster_still_present,
+                False,
+                "postgresql CR should be cascade-deleted once the CRD is gone",
+            )
+        finally:
+            # Always restore the CRD and the cluster so a failed assertion
+            # does not leave the postgresql API missing for any future test
+            # run on the same kind cluster. The postgresqls CRD is the most
+            # important one in the operator, so getting it back is a hard
+            # prerequisite for the rest of the suite.
+            if not self._crd_exists(crd_api, target_crd):
+                result = k8s.create_with_kubectl("manifests/postgresql.crd.yaml")
+                self.assertEqual(
+                    result.returncode, 0,
+                    f"failed to re-install postgresql.crd.yaml: {result.stderr.decode()}",
+                )
+
+            if cluster_snapshot is not None and not self._custom_object_exists(
+                    custom_api, cluster_group, cluster_version, cluster_namespace,
+                    cluster_plural, cluster_name):
+                # strip server-managed fields that block a plain re-apply
+                cluster_snapshot["metadata"].pop("resourceVersion", None)
+                cluster_snapshot["metadata"].pop("uid", None)
+                cluster_snapshot["metadata"].pop("managedFields", None)
+                cluster_snapshot["metadata"].pop("creationTimestamp", None)
+                cluster_snapshot["status"] = {}
+                custom_api.create_namespaced_custom_object(
+                    cluster_group, cluster_version, cluster_namespace,
+                    cluster_plural, cluster_snapshot)
+                k8s.wait_for_pod_start("spilo-role=master," + cluster_label)
+                k8s.wait_for_pod_start("spilo-role=replica," + cluster_label)
+                self.eventuallyEqual(
+                    lambda: k8s.count_running_pods(), 2, "All pods are running")
+
+    @staticmethod
+    def _crd_exists(crd_api, name):
+        try:
+            crd_api.read_custom_resource_definition(name)
+            return True
+        except ApiException as e:
+            if e.status == 404:
+                return False
+            raise
+
+    @staticmethod
+    def _custom_object_exists(api, group, version, namespace, plural, name):
+        try:
+            api.get_namespaced_custom_object(group, version, namespace, plural, name)
+            return True
+        except ApiException as e:
+            if e.status == 404:
+                return False
+            raise
+
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_zz_cluster_deletion(self):
         '''

manifests/fes.crd.yaml

@@ -2,6 +2,10 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   name: fabriceventstreams.zalando.org
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all fabriceventstream custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: zalando.org
   names:

manifests/operatorconfiguration.crd.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: operatorconfigurations.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all operatorconfiguration custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

manifests/postgresql.crd.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: postgresqls.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all postgresql custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

manifests/postgresteam.crd.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: postgresteams.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all postgresteam custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

pkg/apis/acid.zalan.do/v1/crds.go

@@ -11,6 +11,12 @@ import (
 const (
 	PostgresCRDResourceKind       = "postgresql"
 	OperatorConfigCRDResourceKind = "OperatorConfiguration"
+	// CRDProtectionFinalizer blocks accidental deletion of the operator's
+	// CRDs. Deleting a CRD cascades to all of its custom resources, so
+	// removing it must be a deliberate, manual step. controller-gen does
+	// not emit finalizers, so the value is set here on the Go side rather
+	// than in the generated YAML.
+	CRDProtectionFinalizer = "acid.zalan.do/crd-protection"
 )
 
 //go:embed postgresql.crd.yaml
@@ -25,6 +31,7 @@ func PostgresCRD(crdCategories []string) (*apiextv1.CustomResourceDefinition, er
 	}
 
 	crd.Spec.Names.Categories = crdCategories
+	crd.Finalizers = []string{CRDProtectionFinalizer}
 
 	return &crd, nil
 }
@@ -41,6 +48,7 @@ func OperatorConfigurationCRD(crdCategories []string) (*apiextv1.CustomResourceD
 	}
 
 	crd.Spec.Names.Categories = crdCategories
+	crd.Finalizers = []string{CRDProtectionFinalizer}
 
 	return &crd, nil
 }

pkg/apis/acid.zalan.do/v1/operatorconfiguration.crd.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: operatorconfigurations.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all operatorconfiguration custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml

@@ -7,6 +7,10 @@ metadata:
   labels:
     app.kubernetes.io/name: postgres-operator
   name: postgresqls.acid.zalan.do
+  # finalizer to prevent accidental deletion of the CRD which would erase
+  # all postgresql custom resources. Remove explicitly to delete the CRD.
+  finalizers:
+  - acid.zalan.do/crd-protection
 spec:
   group: acid.zalan.do
   names:

pkg/controller/util.go

@@ -51,6 +51,26 @@ func (c *Controller) clusterWorkerID(clusterName spec.NamespacedName) uint32 {
 	return c.clusterWorkers[clusterName]
 }
 
+// ensureCRDProtectionFinalizer adds the protection finalizer to an existing
+// CRD if it is missing. Existing CRs that do not yet carry the finalizer
+// (e.g. clusters upgraded from a previous operator version) are back-filled
+// here on operator startup. A missing finalizer on a CRD is logged but does
+// not fail the startup.
+func (c *Controller) ensureCRDProtectionFinalizer(name string) error {
+	crd, err := c.KubeClient.CustomResourceDefinitions().Get(context.TODO(), name, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+	for _, f := range crd.Finalizers {
+		if f == acidv1.CRDProtectionFinalizer {
+			return nil
+		}
+	}
+	crd.Finalizers = append(crd.Finalizers, acidv1.CRDProtectionFinalizer)
+	_, err = c.KubeClient.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
+	return err
+}
+
 func (c *Controller) createOperatorCRD(desiredCrd *apiextv1.CustomResourceDefinition) error {
 	crd, err := c.KubeClient.CustomResourceDefinitions().Get(context.TODO(), desiredCrd.Name, metav1.GetOptions{})
 	if k8sutil.ResourceNotFound(err) {
@@ -68,6 +88,9 @@ func (c *Controller) createOperatorCRD(desiredCrd *apiextv1.CustomResourceDefini
 		if err != nil {
 			return fmt.Errorf("could not update customResourceDefinition %q: %v", crd.Name, err)
 		}
+		if err := c.ensureCRDProtectionFinalizer(desiredCrd.Name); err != nil {
+			c.logger.Warnf("could not ensure protection finalizer on customResourceDefinition %q: %v", desiredCrd.Name, err)
+		}
 	}
 	c.logger.Infof("customResourceDefinition %q is registered", crd.Name)