From 3e87a0750cf88f8b212da7224292497471dae32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serdar=20Dalg=C4=B1=C3=A7?= Date: Mon, 3 Aug 2026 22:43:34 +0200 Subject: [PATCH 1/3] Skip owner references on user secrets when secret deletion is disabled Kubernetes garbage-collects owner-referenced secrets as soon as the owning Postgresql resource is deleted, regardless of the operator's own EnableSecretsDeletion check in Delete() (which only guards the operator's explicit deleteSecrets() call, not GC). This made enable_secrets_deletion=false ineffective whenever enable_owner_references was also enabled, since GC removed the credential secrets anyway. Now the generated secrets are not removed when enable_owner_references: true, enable_secrets_deletion: false. --- pkg/cluster/k8sres.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 724986dbc..c54d93001 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1944,7 +1944,8 @@ func (c *Cluster) generateSingleUserSecret(pgUser spec.PgUser) *v1.Secret { // if secret lives in another namespace we cannot set ownerReferences var ownerReferences []metav1.OwnerReference - if c.Config.OpConfig.EnableCrossNamespaceSecret && c.Postgresql.ObjectMeta.Namespace != pgUser.Namespace { + secretsDeletionDisabled := c.OpConfig.EnableSecretsDeletion != nil && !*c.OpConfig.EnableSecretsDeletion + if secretsDeletionDisabled || (c.Config.OpConfig.EnableCrossNamespaceSecret && c.Postgresql.ObjectMeta.Namespace != pgUser.Namespace) { ownerReferences = nil } else { ownerReferences = c.ownerReferences() From 45fd8c28ed32ea6774162f865c36018ae98f360b Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Mon, 10 Aug 2026 14:57:37 +0530 Subject: [PATCH 2/3] SCF-857: Document skip-owner-refs on user secrets when deletion disabled - refresh inline comment in generateSingleUserSecret - extend enable_owner_references / enable_secrets_deletion docs in operator_parameters.md to describe the interaction - add third exception in administrator.md "Owner References and Finalizers" --- docs/administrator.md | 3 +- docs/reference/operator_parameters.md | 17 +++- pkg/cluster/k8sres.go | 7 +- pkg/cluster/k8sres_test.go | 128 ++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 5 deletions(-) diff --git a/docs/administrator.md b/docs/administrator.md index 60db0f3f1..628fec863 100644 --- a/docs/administrator.md +++ b/docs/administrator.md @@ -281,11 +281,12 @@ will differ and trigger a rolling update of the pods. ## Owner References and Finalizers The Postgres Operator can set [owner references](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/) to most of a cluster's child resources to improve -monitoring with GitOps tools and enable cascading deletes. There are two +monitoring with GitOps tools and enable cascading deletes. There are three exceptions: * Persistent Volume Claims, because they are handled by the [PV Reclaim Policy]https://kubernetes.io/docs/tasks/administer-cluster/change-pv-reclaim-policy/ of the Stateful Set * Cross-namespace secrets, because owner references are not allowed across namespaces by design +* User-credential secrets when [`enable_secrets_deletion`](reference/operator_parameters.md#enable_secrets_deletion) is `false`, so Kubernetes garbage collection does not cascade-delete them after the Postgresql resource is removed (the `enable_secrets_deletion` flag alone only suppresses the operator's own delete path, not K8s GC) The operator would clean these resources up with its regular delete loop unless they got synced correctly. If for some reason the initial cluster sync diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index a12153c03..ed0612f43 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -298,8 +298,12 @@ configuration they are grouped under the `kubernetes` key. * **enable_owner_references** The operator can set owner references on its child resources (except PVCs, Patroni config service/endpoint, cross-namespace secrets) to improve cluster - monitoring and enable cascading deletion. The default is `false`. Warning, - enabling this option disables configured delete protection checks (see below). + monitoring and enable cascading deletion. User-credential secrets are also + excluded from controller owner references whenever + [enable_secrets_deletion](#enable_secrets_deletion) is `false`, so that + Kubernetes garbage collection does not cascade-delete them when the + Postgresql resource is removed. The default is `false`. Warning, enabling + this option disables configured delete protection checks (see below). * **delete_annotation_date_key** key name for annotation that compares manifest value with current date in the @@ -386,7 +390,14 @@ configuration they are grouped under the `kubernetes` key. * **enable_secrets_deletion** By default, the operator deletes secrets when removing the Postgres cluster - manifest. To keep secrets, set this option to `false`. The default is `true`. + manifest. To keep secrets, set this option to `false`. Note that this only + guards the operator's own deletion logic; Kubernetes garbage collection can + still remove user-credential secrets when + [enable_owner_references](#enable_owner_references) is `true` because the + Postgresql resource acts as a controller owner. To prevent that, the + operator skips the controller owner reference on user-credential secrets + whenever `enable_secrets_deletion` is `false`, so the two settings work + together. The default is `true`. * **enable_persistent_volume_claim_deletion** By default, the operator deletes persistent volume claims when removing the diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index c54d93001..434d32609 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1942,7 +1942,12 @@ func (c *Cluster) generateSingleUserSecret(pgUser spec.PgUser) *v1.Secret { lbls = c.connectionPoolerLabels("", false).MatchLabels } - // if secret lives in another namespace we cannot set ownerReferences + // Skip a controller ownerReference on user-credential secrets when the + // operator is configured to keep them (enable_secrets_deletion=false); + // otherwise Kubernetes garbage collection would still cascade-delete them + // once the owning Postgresql CR is removed, defeating that setting. + // Cross-namespace secrets also have no ownerReference because K8s forbids + // cross-namespace ownerRefs by design. var ownerReferences []metav1.OwnerReference secretsDeletionDisabled := c.OpConfig.EnableSecretsDeletion != nil && !*c.OpConfig.EnableSecretsDeletion if secretsDeletionDisabled || (c.Config.OpConfig.EnableCrossNamespaceSecret && c.Postgresql.ObjectMeta.Namespace != pgUser.Namespace) { diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 04f6476a6..420bb2a3d 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -2819,6 +2819,134 @@ func TestGeneratePodDisruptionBudget(t *testing.T) { } } +func TestGenerateSingleUserSecret_OwnerReferences(t *testing.T) { + testName := "Test generateSingleUserSecret owner references" + + newCluster := func(ownerRefs, secretsDeletion *bool, crossNamespaceSecret bool) *Cluster { + cfg := Config{ + OpConfig: config.Config{ + Resources: config.Resources{ + ClusterNameLabel: "cluster-name", + PodRoleLabel: "spilo-role", + EnableOwnerReferences: ownerRefs, + }, + EnableSecretsDeletion: secretsDeletion, + EnableCrossNamespaceSecret: crossNamespaceSecret, + }, + } + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapp-database", + Namespace: "myapp", + UID: types.UID("myapp-database-uid"), + }, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 1}, + } + return New(cfg, k8sutil.KubernetesClient{}, pg, logger, eventRecorder) + } + + newPgUser := func(namespace string) spec.PgUser { + return spec.PgUser{ + Name: "app_user", + Namespace: namespace, + Password: "secret", + } + } + + hasControllerOwnerRef := func(cluster *Cluster) func(*v1.Secret) error { + return func(secret *v1.Secret) error { + for _, ref := range secret.OwnerReferences { + if ref.UID == cluster.Postgresql.ObjectMeta.UID && + ref.Name == cluster.Postgresql.ObjectMeta.Name && + ref.Controller != nil && *ref.Controller { + return nil + } + } + return fmt.Errorf("expected a controller owner reference pointing at the Postgresql CR, got %#v", + secret.OwnerReferences) + } + } + + hasNoControllerOwnerRef := func(cluster *Cluster) func(*v1.Secret) error { + return func(secret *v1.Secret) error { + for _, ref := range secret.OwnerReferences { + if ref.UID == cluster.Postgresql.ObjectMeta.UID && ref.Controller != nil && *ref.Controller { + return fmt.Errorf("expected no controller owner reference, got %#v", secret.OwnerReferences) + } + } + return nil + } + } + + tests := []struct { + scenario string + cluster *Cluster + pgUser spec.PgUser + expectControllerOwner bool + }{ + { + scenario: "owner refs + secrets deletion enabled (default)", + cluster: newCluster(util.True(), util.True(), false), + pgUser: newPgUser("myapp"), + expectControllerOwner: true, + }, + { + scenario: "owner refs enabled, secrets deletion disabled (skip owner ref)", + cluster: newCluster(util.True(), util.False(), false), + pgUser: newPgUser("myapp"), + expectControllerOwner: false, + }, + { + scenario: "owner refs enabled, secrets deletion unset (default true)", + cluster: newCluster(util.True(), nil, false), + pgUser: newPgUser("myapp"), + expectControllerOwner: true, + }, + { + scenario: "owner refs disabled, secrets deletion enabled", + cluster: newCluster(util.False(), util.True(), false), + pgUser: newPgUser("myapp"), + expectControllerOwner: false, + }, + { + scenario: "owner refs disabled, secrets deletion disabled", + cluster: newCluster(util.False(), util.False(), false), + pgUser: newPgUser("myapp"), + expectControllerOwner: false, + }, + { + scenario: "cross-namespace secret, owner refs + secrets deletion enabled", + cluster: newCluster(util.True(), util.True(), true), + pgUser: newPgUser("other-ns"), + expectControllerOwner: false, + }, + { + scenario: "cross-namespace secret, owner refs enabled, secrets deletion disabled", + cluster: newCluster(util.True(), util.False(), true), + pgUser: newPgUser("other-ns"), + expectControllerOwner: false, + }, + } + + for _, tt := range tests { + secret := tt.cluster.generateSingleUserSecret(tt.pgUser) + if secret == nil { + t.Errorf("%s [%s]: expected a non-nil secret", testName, tt.scenario) + continue + } + + var check func(*v1.Secret) error + if tt.expectControllerOwner { + check = hasControllerOwnerRef(tt.cluster) + } else { + check = hasNoControllerOwnerRef(tt.cluster) + } + if err := check(secret); err != nil { + t.Errorf("%s [%s]: %+v", testName, tt.scenario, err) + } + } +} + func TestGenerateService(t *testing.T) { var spec acidv1.PostgresSpec var cluster *Cluster From 673785f1feec3dfeaa570633c4951b16c2b83469 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Mon, 10 Aug 2026 17:42:11 +0530 Subject: [PATCH 3/3] SCF-857: Updated doc --- docs/reference/operator_parameters.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index ed0612f43..8de1f244e 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -397,7 +397,8 @@ configuration they are grouped under the `kubernetes` key. Postgresql resource acts as a controller owner. To prevent that, the operator skips the controller owner reference on user-credential secrets whenever `enable_secrets_deletion` is `false`, so the two settings work - together. The default is `true`. + together. This protection takes effect on the cluster's next sync after + the setting is applied. The default is `true`. * **enable_persistent_volume_claim_deletion** By default, the operator deletes persistent volume claims when removing the