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 eeacfac10..db5071cd2 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)