From a30dcea311fb9dc1a47e7716055a205b26d10315 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Thu, 13 Aug 2026 12:32:52 +0100 Subject: [PATCH] Protect operator CRDs from accidental deletion A 'kubectl delete crd' is destructive: the API server cascades the delete to every custom resource of that kind in the cluster, so a single mistyped command can wipe out all postgresql, operator configuration, postgresteam and fabriceventstream objects at once. This is especially dangerous for an operator whose whole value proposition is the data behind those CRs. The fix is a 'acid.zalan.do/crd-protection' finalizer on each operator CRD. The apiserver blocks the deletion while the finalizer is set, so the destructive action is gated behind a manual step (patch the finalizer away) that anyone running the delete has to take deliberately. Removal procedure is documented in docs/administrator.md under 'Protecting CRDs from accidental deletion'. Assisted-by: OpenCode + MiniMax M3 --- .../crds/operatorconfigurations.yaml | 4 + .../postgres-operator/crds/postgresqls.yaml | 4 + .../postgres-operator/crds/postgresteams.yaml | 4 + docs/administrator.md | 28 ++++ e2e/tests/k8s_api.py | 1 + e2e/tests/test_e2e.py | 145 ++++++++++++++++++ manifests/fes.crd.yaml | 4 + manifests/operatorconfiguration.crd.yaml | 4 + manifests/postgresql.crd.yaml | 4 + manifests/postgresteam.crd.yaml | 4 + pkg/apis/acid.zalan.do/v1/crds.go | 8 + .../v1/operatorconfiguration.crd.yaml | 4 + pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml | 4 + pkg/controller/util.go | 23 +++ 14 files changed, 241 insertions(+) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 0f21f7cf4..74c6dc356 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/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: diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index f3dcbab59..995fa2265 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/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: diff --git a/charts/postgres-operator/crds/postgresteams.yaml b/charts/postgres-operator/crds/postgresteams.yaml index 3bc7fcd1d..0a666cc13 100644 --- a/charts/postgres-operator/crds/postgresteams.yaml +++ b/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: diff --git a/docs/administrator.md b/docs/administrator.md index 289bef8fb..cc73db79a 100644 --- a/docs/administrator.md +++ b/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 diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 0ef3d6315..f07926152 100644 --- a/e2e/tests/k8s_api.py +++ b/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: diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index ca13966ed..ce5f60f26 100644 --- a/e2e/tests/test_e2e.py +++ b/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): ''' diff --git a/manifests/fes.crd.yaml b/manifests/fes.crd.yaml index 70a8c9555..8c5fffef5 100644 --- a/manifests/fes.crd.yaml +++ b/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: diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 0f21f7cf4..74c6dc356 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/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: diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index f3dcbab59..995fa2265 100644 --- a/manifests/postgresql.crd.yaml +++ b/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: diff --git a/manifests/postgresteam.crd.yaml b/manifests/postgresteam.crd.yaml index 3bc7fcd1d..0a666cc13 100644 --- a/manifests/postgresteam.crd.yaml +++ b/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: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 54aea7c1b..9a4d82a0a 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/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 } diff --git a/pkg/apis/acid.zalan.do/v1/operatorconfiguration.crd.yaml b/pkg/apis/acid.zalan.do/v1/operatorconfiguration.crd.yaml index 0f21f7cf4..74c6dc356 100644 --- a/pkg/apis/acid.zalan.do/v1/operatorconfiguration.crd.yaml +++ b/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: diff --git a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml b/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml index f3dcbab59..995fa2265 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml +++ b/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: diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 6296e5341..e06b66b2a 100644 --- a/pkg/controller/util.go +++ b/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)