From f4f423649566ff8a3203d70d74f50ffddb61e9f8 Mon Sep 17 00:00:00 2001 From: Thomas Sapelza Date: Sun, 25 Oct 2026 15:45:13 +0100 Subject: [PATCH] keep the current owner of a Database or Schema without an owner in the spec Before, the reconcile compared the current owner with null and ran "alter ... owner to """, which PostgreSQL rejects. Assisted-by: Claude:claude-opus-5-5 --- docs/database.md | 2 +- docs/schema.md | 2 +- .../crd/database/DatabaseReconciler.java | 3 +- .../postgresql/crd/database/DatabaseSpec.java | 1 + .../crd/schema/SchemaReconciler.java | 3 +- .../postgresql/crd/schema/SchemaSpec.java | 1 + .../crd/database/DatabaseReconcilerTest.java | 38 +++++++++++--- .../crd/schema/SchemaReconcilerTest.java | 51 ++++++++++++++++--- 8 files changed, 83 insertions(+), 18 deletions(-) diff --git a/docs/database.md b/docs/database.md index 1bc7be4..bc1302d 100644 --- a/docs/database.md +++ b/docs/database.md @@ -8,7 +8,7 @@ The `Database` Custom Resource Definition (CRD) is responsible for managing Post |-----------------|---------------|------------------------------------------------------------------------------------------------------|----------|---------| | `clusterRef` | `ResourceRef` | Reference to the `ClusterConnection` to use. | Yes | Yes | | `name` | `string` | The name of the database to create. | Yes | No | -| `owner` | `string` | The owner of the database. | No | Yes | +| `owner` | `string` | The owner of the database. If not set, the database keeps its current owner. | No | Yes | | `reclaimPolicy` | `string` | The policy for reclaiming the database when the CR is deleted. Values: `Retain` (Default), `Delete`. | No | Yes | ### ResourceRef (`clusterRef`) diff --git a/docs/schema.md b/docs/schema.md index 9a042a0..aaebebf 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -9,7 +9,7 @@ The `Schema` Custom Resource Definition (CRD) is responsible for managing Postgr | `clusterRef` | `ResourceRef` | Reference to the `ClusterConnection` to use. | Yes | Yes | | `database` | `string` | The name of the database in which the schema is created. | Yes | No | | `name` | `string` | The name of the schema to create. | Yes | No | -| `owner` | `string` | The owner of the schema. | No | Yes | +| `owner` | `string` | The owner of the schema. If not set, the schema keeps its current owner. | No | Yes | | `reclaimPolicy` | `string` | The policy for reclaiming the schema when the CR is deleted. Values: `Retain` (Default), `Delete`. | No | Yes | ### ResourceRef (`clusterRef`) diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseReconciler.java b/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseReconciler.java index 72d4f8a..2ef47c2 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseReconciler.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseReconciler.java @@ -210,7 +210,8 @@ private UpdateControl reconcileWithoutTransaction( var currentOwner = databaseService.fetchDatabaseOwner(dsl, spec); var expectedOwner = spec.getOwner(); - if (Objects.equals(currentOwner, expectedOwner)) { + // Without an owner in the spec, the database keeps its current owner + if (expectedOwner == null || Objects.equals(currentOwner, expectedOwner)) { log.info( "Database up-to-date [resource={}/{}]", namespace, diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseSpec.java b/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseSpec.java index a5c80c5..416fcef 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseSpec.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/database/DatabaseSpec.java @@ -33,6 +33,7 @@ public class DatabaseSpec { /// The owner of the database. /// If not specified, the database will be owned by the logged-in admin user specified in the clusterRef -> ClusterConnection#adminSecretRef CR instance. + /// If removed later, the database keeps its current owner. @io.fabric8.generator.annotation.Nullable private @Nullable String owner = null; } diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaReconciler.java b/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaReconciler.java index 968a973..09356c6 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaReconciler.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaReconciler.java @@ -214,7 +214,8 @@ private UpdateControl reconcileInTransaction( var currentOwner = schemaService.fetchSchemaOwner(tx, spec); var expectedOwner = spec.getOwner(); - if (Objects.equals(currentOwner, expectedOwner)) { + // Without an owner in the spec, the schema keeps its current owner + if (expectedOwner == null || Objects.equals(currentOwner, expectedOwner)) { log.info( "Schema up-to-date [resource={}/{}]", namespace, diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaSpec.java b/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaSpec.java index 4818ffe..63fcac6 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaSpec.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/schema/SchemaSpec.java @@ -44,6 +44,7 @@ public class SchemaSpec { /// The owner of the schema. /// If not specified, the schema will be owned by the logged-in admin user specified in the clusterRef -> ClusterConnection#adminSecretRef CR instance. + /// If removed later, the schema keeps its current owner. @io.fabric8.generator.annotation.Nullable private @Nullable String owner = null; } diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/database/DatabaseReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/database/DatabaseReconcilerTest.java index 0d564d9..7e59152 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/database/DatabaseReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/database/DatabaseReconcilerTest.java @@ -78,7 +78,7 @@ void createDatabase_andStatusReady() { "When the ClusterConnection of an up-to-date Database is back, the Database should be reconciled to READY" ) void upToDateDatabase_afterPendingClusterConnection_setsReady() { - // given: a reconciled Database with an explicit owner, so that the next reconciles find it up to date + // given: a reconciled Database var clusterConnection = given.one() .clusterConnection() .withName("test-connection-database-pending-to-ready") @@ -90,12 +90,7 @@ void upToDateDatabase_afterPendingClusterConnection_setsReady() { .withReclaimPolicy(DELETE) .returnFirst(); - var spec = database.getSpec(); - var clusterRef = spec.getClusterRef(); - - var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); - - spec.setOwner(databaseService.fetchDatabaseOwner(dsl, spec)); + var clusterRef = database.getSpec().getClusterRef(); // and: the ClusterConnection is not available for a while clusterRef.setName("non-existing-cc"); @@ -114,6 +109,35 @@ void upToDateDatabase_afterPendingClusterConnection_setsReady() { assertThat(updatedDatabase.getStatus().getMessage()).isNull(); } + @Test + @DisplayName("When a Database without owner is reconciled again, it should keep its owner and stay READY") + void databaseWithoutOwner_reconciledAgain_keepsOwner() { + // given: a reconciled Database without owner + var clusterConnection = given.one() + .clusterConnection() + .withName("test-connection-database-without-owner") + .returnFirst(); + + var database = given.one() + .database() + .withClusterConnectionName(clusterConnection.getMetadata().getName()) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); + + var initialOwner = databaseService.fetchDatabaseOwner(dsl, database.getSpec()); + + // when: a spec change that needs no SQL statement triggers the next reconcile + database.getSpec().setReclaimPolicy(DELETE); + + var updatedDatabase = applyDatabase(database); + + // then + assertThat(updatedDatabase.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updatedDatabase.getStatus().getMessage()).isNull(); + assertThat(databaseService.fetchDatabaseOwner(dsl, database.getSpec())).isEqualTo(initialOwner); + } + private Database applyDatabase(Database database) { var namespace = kubernetesClient.getNamespace(); diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/schema/SchemaReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/schema/SchemaReconcilerTest.java index 4d8beae..a2f598c 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/schema/SchemaReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/schema/SchemaReconcilerTest.java @@ -89,7 +89,7 @@ void createSchema_andStatusReady() { @Test @DisplayName("When the ClusterConnection of an up-to-date Schema is back, the Schema should be reconciled to READY") void upToDateSchema_afterPendingClusterConnection_setsReady() { - // given: a reconciled Schema with an explicit owner, so that the next reconciles find it up to date + // given: a reconciled Schema var clusterConnectionMain = given.one() .clusterConnection() .withName("test-connection-schema-pending-to-ready") @@ -114,12 +114,7 @@ void upToDateSchema_afterPendingClusterConnection_setsReady() { .withReclaimPolicy(DELETE) .returnFirst(); - var spec = schema.getSpec(); - var clusterRef = spec.getClusterRef(); - - var dsl = postgreSQLContextFactory.getDSLContext(clusterConnectionDb); - - spec.setOwner(schemaService.fetchSchemaOwner(dsl, spec)); + var clusterRef = schema.getSpec().getClusterRef(); // and: the ClusterConnection is not available for a while clusterRef.setName("non-existing-cc"); @@ -138,6 +133,48 @@ void upToDateSchema_afterPendingClusterConnection_setsReady() { assertThat(updatedSchema.getStatus().getMessage()).isNull(); } + @Test + @DisplayName("When a Schema without owner is reconciled again, it should keep its owner and stay READY") + void schemaWithoutOwner_reconciledAgain_keepsOwner() { + // given: a reconciled Schema without owner + var clusterConnectionMain = given.one() + .clusterConnection() + .withName("test-connection-schema-without-owner") + .returnFirst(); + + var database = given.one() + .database() + .withClusterConnectionName(clusterConnectionMain.getMetadata().getName()) + .returnFirst(); + + // Creating a second ClusterConnection CR is only required for the tests + var clusterConnectionDb = given.one() + .clusterConnection() + .withName("test-connection-schema-without-owner-db") + .withDatabase(database.getSpec().getName()) + .returnFirst(); + + var schema = given.one() + .schema() + .withClusterConnectionName(clusterConnectionDb.getMetadata().getName()) + .withDatabase(database.getSpec().getName()) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnectionDb); + + var initialOwner = schemaService.fetchSchemaOwner(dsl, schema.getSpec()); + + // when: a spec change that needs no SQL statement triggers the next reconcile + schema.getSpec().setReclaimPolicy(DELETE); + + var updatedSchema = applySchema(schema); + + // then + assertThat(updatedSchema.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updatedSchema.getStatus().getMessage()).isNull(); + assertThat(schemaService.fetchSchemaOwner(dsl, schema.getSpec())).isEqualTo(initialOwner); + } + private Schema applySchema(Schema schema) { var namespace = kubernetesClient.getNamespace();