From 01612444da5dd37b4f76c54799de319395535ca2 Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Wed, 2 Sep 2026 12:47:40 +0200 Subject: [PATCH 1/6] Add adminSecretFileRef as alternative to adminSecretRef for file-based credentials in ClusterConnection --- .gitignore | 3 + docs/cluster-connection.md | 44 +- docs/docker-environment.md | 70 ++- docs/terraform.md | 2 +- gradle.properties | 3 + .../postgresql/core/KubernetesService.java | 55 +- .../postgresql/core/ResourceFileRef.java | 34 ++ .../ClusterConnectionSpec.java | 12 +- .../core/KubernetesServiceTest.java | 525 ++++++++++++++++++ 9 files changed, 731 insertions(+), 17 deletions(-) create mode 100644 operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java create mode 100644 operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java diff --git a/.gitignore b/.gitignore index f917d07..5007d0a 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,6 @@ gradle-app.setting # Quinoa .quinoa/ + +generated/out/ +operator/out/ diff --git a/docs/cluster-connection.md b/docs/cluster-connection.md index 139fcfb..6851961 100644 --- a/docs/cluster-connection.md +++ b/docs/cluster-connection.md @@ -7,13 +7,16 @@ Other Custom Resources (like `Database`, `Role`, `Schema`, `Grant`, `DefaultPriv ## Spec -| Field | Type | Description | Required | Mutable | -|------------------|---------------------|-----------------------------------------------------------------------|----------|---------| -| `host` | `string` | The hostname of the PostgreSQL instance. | Yes | Yes | -| `port` | `integer` | The port of the PostgreSQL instance (1-65535). | Yes | Yes | -| `database` | `string` | The database to connect to (usually `postgres` for admin operations). | Yes | Yes | -| `adminSecretRef` | `ResourceRef` | Reference to the Kubernetes Secret containing the admin credentials. | Yes | Yes | -| `parameters` | `map[string]string` | Additional connection parameters. | No | Yes | +| Field | Type | Description | Required | Mutable | +|---------------------|---------------------|-----------------------------------------------------------------------|----------|---------| +| `host` | `string` | The hostname of the PostgreSQL instance. | Yes | Yes | +| `port` | `integer` | The port of the PostgreSQL instance (1-65535). | Yes | Yes | +| `database` | `string` | The database to connect to (usually `postgres` for admin operations). | Yes | Yes | +| `adminSecretRef` | `ResourceRef` | Reference to the Kubernetes Secret containing the admin credentials. | No | Yes | +| `adminSecretFileRef`| `ResourceFileRef` | Reference to a file containing the admin credentials. | No | Yes | +| `parameters` | `map[string]string` | Additional connection parameters. | No | Yes | + +> **Note:** Exactly one of `adminSecretRef` or `adminSecretFileRef` must be provided. ### ResourceRef (`adminSecretRef`) @@ -24,7 +27,17 @@ Other Custom Resources (like `Database`, `Role`, `Schema`, `Grant`, `DefaultPriv The referenced secret must be of type `kubernetes.io/basic-auth` and contain the keys `username` and `password`. -### Example +### ResourceFileRef (`adminSecretFileRef`) + +| Field | Type | Description | Required | +|--------|----------|----------------------------------------------------------------|----------| +| `path` | `string` | The path to the file containing the admin credentials. | Yes | + +Use this option when credentials are mounted as a file (e.g. via AWS Secrets Manager) instead of a Kubernetes Secret. + +### Examples + +#### Using a Kubernetes Secret (`adminSecretRef`) ```yaml apiVersion: v1 @@ -54,3 +67,18 @@ spec: #sslmode: "require" # Enforce SSL encryption #connectTimeout: "10" # Timeout in seconds for connection attempts ``` + +#### Using a file reference (`adminSecretFileRef`) + +```yaml +apiVersion: postgresql.aboutbits.it/v1 +kind: ClusterConnection +metadata: + name: my-postgres-connection +spec: + adminSecretFileRef: + path: "/mnt/db-password" + host: localhost + port: 5432 + database: postgres +``` diff --git a/docs/docker-environment.md b/docs/docker-environment.md index 37cf7cb..4c8be9a 100644 --- a/docs/docker-environment.md +++ b/docs/docker-environment.md @@ -39,7 +39,16 @@ users: ## 2. Create PostgreSQL Connection and Secret -For the `postgresql` Dev Service, you can generate the necessary Custom Resources to test the Operator: +For the `postgresql` Dev Service, you can generate the necessary Custom Resources to test the Operator. + +A `ClusterConnection` requires admin credentials, which can be provided in one of two ways: + +- **`adminSecretRef`** — references a Kubernetes `basic-auth` Secret (username + password). +- **`adminSecretFileRef`** — references a JSON file mounted into the operator pod (e.g. from AWS Secrets Manager). + +Exactly one of these must be specified. + +### Using a Kubernetes Secret (`adminSecretRef`) 1. From the Dev UI, get the `postgresql` Dev Service properties (username, password, host, port). 2. Convert the `postgresql` Dev Service properties to a **Basic Auth Secret** and a **ClusterConnection** CR instance. @@ -79,6 +88,65 @@ spec: database: postgres ``` +### Using a file reference (`adminSecretFileRef`) + +Instead of a Kubernetes Secret, you can mount a JSON credentials file into the operator pod and reference its path. This is useful when credentials are managed externally (e.g. AWS Secrets Manager). + +#### File format + +The file must contain JSON with the following fields: + +```json +{ + "username": "root", + "password": "password" +} +``` + +- `password` — **required** +- `username` — optional (can be omitted) + +#### Mount the credentials file + +The file must be accessible inside the operator pod at the path specified in `adminSecretFileRef.path`. Mount it using a Volume and VolumeMount on the operator Deployment: + +```yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: postgresql-operator +spec: + template: + spec: + containers: + - name: operator + volumeMounts: + - name: db-credentials + mountPath: /mnt/secrets + readOnly: true + volumes: + - name: db-credentials + secret: + secretName: db-credentials-secret +``` + +> **Note:** The volume source can be any type that provides a file (e.g. a Kubernetes Secret, a CSI volume from AWS Secrets Manager, or a ConfigMap for testing). + +#### Example ClusterConnection + +```yaml +apiVersion: postgresql.aboutbits.it/v1 +kind: ClusterConnection +metadata: + name: quarkus-postgres-connection +spec: + adminSecretFileRef: + path: "/mnt/secrets/db-credentials.json" + host: localhost + port: 5432 + database: postgres +``` + ![Established Cluster Connection](images/established-cluster-connection.png) ## 3. Create a Role diff --git a/docs/terraform.md b/docs/terraform.md index 6bc4817..5253c74 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -95,7 +95,7 @@ Every optional field of every Custom Resource is affected, in particular: | Custom Resource | Optional fields | |---------------------|------------------------------------------------------------------------------------------------| -| `ClusterConnection` | `parameters`, `adminSecretRef.namespace` | +| `ClusterConnection` | `parameters`, `adminSecretRef`, `adminSecretRef.namespace`, `adminSecretFileRef` | | `Database` | `owner`, `reclaimPolicy`, `clusterRef.namespace` | | `Schema` | `owner`, `reclaimPolicy`, `clusterRef.namespace` | | `Role` | `comment`, `passwordSecretRef`, `flags` (including `flags.validUntil`), `clusterRef.namespace` | diff --git a/gradle.properties b/gradle.properties index d3cfe0c..427e524 100644 --- a/gradle.properties +++ b/gradle.properties @@ -13,3 +13,6 @@ quarkusPlatformGroupId=io.quarkus.platform quarkusPlatformArtifactId=quarkus-bom quarkusPlatformVersion=3.35.3 systemProp.quarkus.analytics.disabled=true + +# Workaround for Windows: avoid forked process where -D args with {{ }} get mangled by cmd.exe +systemProp.gradle.quarkus.gradle-worker.no-process=true diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java index a5763ba..ac91c84 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java @@ -1,11 +1,16 @@ package it.aboutbits.postgresql.core; +import com.fasterxml.jackson.databind.ObjectMapper; import io.fabric8.kubernetes.client.KubernetesClient; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; +import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnectionSpec; import jakarta.inject.Singleton; import org.jspecify.annotations.NullMarked; +import java.io.IOException; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Base64; @Singleton @@ -19,11 +24,51 @@ public Credentials getSecretRefCredentials( KubernetesClient kubernetesClient, ClusterConnection clusterConnection ) { - return getSecretRefCredentials( - kubernetesClient, - clusterConnection.getSpec().getAdminSecretRef(), - clusterConnection.getMetadata().getNamespace() - ); + ClusterConnectionSpec spec = clusterConnection.getSpec(); + if (spec.getAdminSecretRef() != null) { + return getSecretRefCredentials( + kubernetesClient, + clusterConnection.getSpec().getAdminSecretRef(), + clusterConnection.getMetadata().getNamespace() + ); + + } else if (spec.getAdminSecretFileRef() != null) { + return getSecretFileRefCredentials(spec.getAdminSecretFileRef()); + } + + throw new IllegalStateException("Exactly one of 'adminSecretRef' or 'adminSecretFileRef' must be provided"); + + } + + public Credentials getSecretFileRefCredentials(ResourceFileRef fileRef) { + var path = Path.of(fileRef.getPath()); + + if (!Files.exists(path)) { + throw new IllegalStateException("AWS Secrets Manager file not found [path=%s]".formatted(path)); + } + + try { + var content = Files.readString(path); + var objectMapper = new ObjectMapper(); + var json = objectMapper.readTree(content); + + var usernameNode = json.get(SECRET_DATA_BASIC_AUTH_USERNAME_KEY); + var username = usernameNode != null && !usernameNode.isNull() + ? usernameNode.asText() + : null; + + var passwordNode = json.get(SECRET_DATA_BASIC_AUTH_PASSWORD_KEY); + if (passwordNode == null || passwordNode.isNull()) { + throw new IllegalStateException("AWS Secrets Manager file is missing required field '%s' [path=%s]".formatted( + SECRET_DATA_BASIC_AUTH_PASSWORD_KEY, + path + )); + } + + return new Credentials(username, passwordNode.asText()); + } catch (IOException e) { + throw new IllegalStateException("Failed to read AWS Secrets Manager file [path=%s]".formatted(path), e); + } } public Credentials getSecretRefCredentials( diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java b/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java new file mode 100644 index 0000000..10a91e2 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java @@ -0,0 +1,34 @@ +package it.aboutbits.postgresql.core; + +import io.fabric8.generator.annotation.Required; +import io.fabric8.generator.annotation.ValidationRule; +import lombok.Getter; +import lombok.Setter; +import org.jspecify.annotations.NullMarked; + +/// A reference to a file inside an AWS Secrets Manager secret. +/// +/// This class is used wherever a CRD spec needs to point to a specific file +/// within an AWS secret. The [#path] field identifies the file location +/// inside the secret. +/// +/// ### Example usage in a CR manifest +/// +/// ```yaml +/// spec: +/// adminSecretFileRef: +/// path: "/mnt/db-password" +/// ``` +@Getter +@Setter +@NullMarked +public class ResourceFileRef { + /// The path to the file inside the AWS Secrets Manager secret. + /// Must not be blank. + @Required + @ValidationRule( + value = "self.trim().size() > 0", + message = "The path must not be empty." + ) + private String path = ""; +} diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java b/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java index 41d7914..e494dd7 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java @@ -5,6 +5,7 @@ import io.fabric8.generator.annotation.Min; import io.fabric8.generator.annotation.Required; import io.fabric8.generator.annotation.ValidationRule; +import it.aboutbits.postgresql.core.ResourceFileRef; import it.aboutbits.postgresql.core.ResourceRef; import it.aboutbits.postgresql.core.schema_customizer.HostCustomizer; import lombok.Getter; @@ -18,6 +19,10 @@ @Setter @SchemaCustomizer(value = HostCustomizer.class, input = "host") @NullMarked +@ValidationRule( + value = "(has(self.adminSecretRef) ? 1 : 0) + (has(self.adminSecretFileRef) ? 1 : 0) == 1", + message = "Exactly one of 'adminSecretRef' or 'adminSecretFileRef' must be provided" +) public class ClusterConnectionSpec { @Required @ValidationRule( @@ -38,8 +43,11 @@ public class ClusterConnectionSpec { ) private String database = "postgres"; - @Required - private ResourceRef adminSecretRef = new ResourceRef(); + @io.fabric8.generator.annotation.Nullable + private ResourceRef adminSecretRef; + + @io.fabric8.generator.annotation.Nullable + private ResourceFileRef adminSecretFileRef; @io.fabric8.generator.annotation.Nullable private Map parameters = new HashMap<>(); diff --git a/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java new file mode 100644 index 0000000..b9635c3 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java @@ -0,0 +1,525 @@ +package it.aboutbits.postgresql.core; + +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.NamespaceableResource; +import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; +import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnectionSpec; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Base64; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@NullMarked +class KubernetesServiceTest { + private final KubernetesService service = new KubernetesService(); + + @TempDir + Path tempDir; + + @Nested + class GetSecretFileRefCredentials { + @Test + @DisplayName("when both username and password present, should return credentials") + void whenBothUsernameAndPassword_shouldReturnCredentials() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"username": "admin", "password": "s3cret"} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when + var result = service.getSecretFileRefCredentials(fileRef); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when username key missing, should return null username") + void whenUsernameKeyMissing_shouldReturnNullUsername() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"password": "s3cret"} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when + var result = service.getSecretFileRefCredentials(fileRef); + + // then + assertThat(result.username()).isNull(); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when username is null, should return null username") + void whenUsernameIsNull_shouldReturnNullUsername() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"username": null, "password": "s3cret"} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when + var result = service.getSecretFileRefCredentials(fileRef); + + // then + assertThat(result.username()).isNull(); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when password key missing, should throw") + void whenPasswordKeyMissing_shouldThrow() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"username": "admin"} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when / then + assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required field 'password'"); + } + + @Test + @DisplayName("when password is null, should throw") + void whenPasswordIsNull_shouldThrow() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"username": "admin", "password": null} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when / then + assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required field 'password'"); + } + + @Test + @DisplayName("when file not found, should throw") + void whenFileNotFound_shouldThrow() { + // given + var fileRef = new ResourceFileRef(); + fileRef.setPath(tempDir.resolve("nonexistent.json").toString()); + + // when / then + assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("file not found"); + } + + @Test + @DisplayName("when invalid JSON, should throw") + void whenInvalidJson_shouldThrow() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, "not valid json {{{"); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + // when / then + assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Failed to read"); + } + } + + @Nested + class GetSecretRefCredentials { + @Test + @DisplayName("when secret exists, should return decoded credentials") + void whenSecretExists_shouldReturnDecodedCredentials() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of( + "username", base64("admin"), + "password", base64("s3cret") + )); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when secret not found, should throw") + void whenSecretNotFound_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var client = mockKubernetesClient("my-ns", "my-secret", null); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Secret reference not found"); + } + + @Test + @DisplayName("when secret wrong type, should throw") + void whenSecretWrongType_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType("Opaque"); + secret.setData(Map.of("password", base64("s3cret"))); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("wrong type"); + } + + @Test + @DisplayName("when secret has no data, should throw") + void whenSecretHasNoData_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(null); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("has no data set"); + } + + @Test + @DisplayName("when secret missing password, should throw") + void whenSecretMissingPassword_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of("username", base64("admin"))); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required data password"); + } + + @Test + @DisplayName("when namespace null, should use default namespace") + void whenNamespaceNull_shouldUseDefaultNamespace() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace(null); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of( + "username", base64("admin"), + "password", base64("s3cret") + )); + + var client = mockKubernetesClient("default-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when secret has empty data map, should throw") + void whenSecretHasEmptyDataMap_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of()); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("has no data set"); + } + + @Test + @DisplayName("when secret has username only (no password), should throw") + void whenSecretHasUsernameOnly_shouldThrow() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of("username", base64("admin"))); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required data password"); + } + + @Test + @DisplayName("when secret has password only (no username), should return null username") + void whenSecretHasPasswordOnly_shouldReturnNullUsername() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of("password", base64("s3cret"))); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + + // then + assertThat(result.username()).isNull(); + assertThat(result.password()).isEqualTo("s3cret"); + } + } + + @Nested + class GetCredentialsDispatcher { + @Test + @DisplayName("when only adminSecretRef set, should delegate to secret ref") + void whenOnlyAdminSecretRefSet_shouldDelegateToSecretRef() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("my-ns"); + secretRef.setName("my-secret"); + + var spec = new ClusterConnectionSpec(); + spec.setAdminSecretRef(secretRef); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of( + "username", base64("admin"), + "password", base64("s3cret") + )); + + var client = mockKubernetesClient("my-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, clusterConnection); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when only adminSecretFileRef set, should delegate to file ref") + void whenOnlyAdminSecretFileRefSet_shouldDelegateToFileRef() throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, """ + {"username": "file-admin", "password": "file-s3cret"} + """); + + var fileRef = new ResourceFileRef(); + fileRef.setPath(file.toString()); + + var spec = new ClusterConnectionSpec(); + spec.setAdminSecretFileRef(fileRef); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + var client = mock(KubernetesClient.class); + + // when + var result = service.getSecretRefCredentials(client, clusterConnection); + + // then + assertThat(result.username()).isEqualTo("file-admin"); + assertThat(result.password()).isEqualTo("file-s3cret"); + } + + @Test + @DisplayName("when neither ref set, should throw") + void whenNeitherRefSet_shouldThrow() { + // given + var spec = new ClusterConnectionSpec(); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + var client = mock(KubernetesClient.class); + + // when / then + assertThatThrownBy(() -> service.getSecretRefCredentials(client, clusterConnection)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Exactly one of"); + } + + @Test + @DisplayName("when adminSecretRef set, should use its namespace over CR namespace") + void whenAdminSecretRefHasNamespace_shouldUseSecretRefNamespace() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace("explicit-ns"); + secretRef.setName("my-secret"); + + var spec = new ClusterConnectionSpec(); + spec.setAdminSecretRef(secretRef); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of( + "username", base64("admin"), + "password", base64("s3cret") + )); + + var client = mockKubernetesClient("explicit-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, clusterConnection); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + + @Test + @DisplayName("when adminSecretRef has null namespace, should fall back to CR namespace") + void whenAdminSecretRefHasNullNamespace_shouldUseCrNamespace() { + // given + var secretRef = new ResourceRef(); + secretRef.setNamespace(null); + secretRef.setName("my-secret"); + + var spec = new ClusterConnectionSpec(); + spec.setAdminSecretRef(secretRef); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + var secret = new Secret(); + secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); + secret.setData(Map.of( + "username", base64("admin"), + "password", base64("s3cret") + )); + + var client = mockKubernetesClient("cr-ns", "my-secret", secret); + + // when + var result = service.getSecretRefCredentials(client, clusterConnection); + + // then + assertThat(result.username()).isEqualTo("admin"); + assertThat(result.password()).isEqualTo("s3cret"); + } + } + + private static ClusterConnection buildClusterConnection(ClusterConnectionSpec spec, String namespace) { + var meta = new ObjectMeta(); + meta.setNamespace(namespace); + + var clusterConnection = new ClusterConnection(); + clusterConnection.setSpec(spec); + clusterConnection.setMetadata(meta); + + return clusterConnection; + } + + @SuppressWarnings("unchecked") + private static KubernetesClient mockKubernetesClient( + String namespace, + String name, + @Nullable Secret secret + ) { + var client = mock(KubernetesClient.class); + var secrets = mock(MixedOperation.class); + var nsOp = mock(NonNamespaceOperation.class); + var resource = mock(NamespaceableResource.class); + + when(client.secrets()).thenReturn(secrets); + when(secrets.inNamespace(namespace)).thenReturn(nsOp); + when(nsOp.withName(name)).thenReturn(resource); + when(resource.get()).thenReturn(secret); + + return client; + } + + private static String base64(String value) { + return Base64.getEncoder().encodeToString(value.getBytes(Charset.defaultCharset())); + } +} From 263b294c5fcca19c4839728af1c4b81c7b5a0aae Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Thu, 3 Sep 2026 13:40:40 +0200 Subject: [PATCH 2/6] revert and ammend as per PR request --- .gitignore | 3 +- gradle.properties | 3 -- .../it/aboutbits/postgresql/core/FileRef.java | 34 +++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java diff --git a/.gitignore b/.gitignore index 5007d0a..3319c66 100644 --- a/.gitignore +++ b/.gitignore @@ -61,5 +61,4 @@ gradle-app.setting # Quinoa .quinoa/ -generated/out/ -operator/out/ +**/out/ diff --git a/gradle.properties b/gradle.properties index 427e524..d3cfe0c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -13,6 +13,3 @@ quarkusPlatformGroupId=io.quarkus.platform quarkusPlatformArtifactId=quarkus-bom quarkusPlatformVersion=3.35.3 systemProp.quarkus.analytics.disabled=true - -# Workaround for Windows: avoid forked process where -D args with {{ }} get mangled by cmd.exe -systemProp.gradle.quarkus.gradle-worker.no-process=true diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java new file mode 100644 index 0000000..51ccf7e --- /dev/null +++ b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java @@ -0,0 +1,34 @@ +package it.aboutbits.postgresql.core; + +import io.fabric8.generator.annotation.Required; +import io.fabric8.generator.annotation.ValidationRule; +import lombok.Getter; +import lombok.Setter; +import org.jspecify.annotations.NullMarked; + +/// A reference to a file inside +/// +/// This class is used wherever a CRD spec needs to point to a specific file +/// The [#path] field identifies the file location +/// inside the secret. +/// +/// ### Example usage in a CR manifest +/// +/// ```yaml +/// spec: +/// adminSecretFileRef: +/// path: "/mnt/db-password" +/// ``` +@Getter +@Setter +@NullMarked +public class FileRef { + /// The path to the file. + /// Must not be blank. + @Required + @ValidationRule( + value = "self.trim().size() > 0", + message = "The path must not be empty." + ) + private String path = ""; +} From 3f50c5371d3b442d836c5ec608b407aa3517d9ec Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Thu, 3 Sep 2026 15:00:34 +0200 Subject: [PATCH 3/6] Addressed PR review comments: require username in adminSecretFileRef, rename to FileRef, use fabric8 mock server, remove AWS references, updated unit tests and docs --- docs/cluster-connection.md | 54 ++- docs/docker-environment.md | 63 +-- operator/build.gradle.kts | 1 + .../it/aboutbits/postgresql/core/FileRef.java | 4 +- .../postgresql/core/KubernetesService.java | 27 +- .../core/PostgreSQLContextFactory.java | 2 +- .../postgresql/core/ResourceFileRef.java | 34 -- .../ClusterConnectionSpec.java | 7 +- .../creator/ClusterConnectionCreate.java | 27 +- .../core/KubernetesServiceTest.java | 365 ++++++------------ .../ClusterConnectionReconcilerTest.java | 33 ++ 11 files changed, 243 insertions(+), 374 deletions(-) delete mode 100644 operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java diff --git a/docs/cluster-connection.md b/docs/cluster-connection.md index 6851961..9281333 100644 --- a/docs/cluster-connection.md +++ b/docs/cluster-connection.md @@ -33,7 +33,8 @@ The referenced secret must be of type `kubernetes.io/basic-auth` and contain the |--------|----------|----------------------------------------------------------------|----------| | `path` | `string` | The path to the file containing the admin credentials. | Yes | -Use this option when credentials are mounted as a file (e.g. via AWS Secrets Manager) instead of a Kubernetes Secret. +Use this option when the credentials are mounted as a file instead of a Kubernetes Secret. The file must be a JSON object with the keys `username` and `password`, for example `{"username": "postgres", "password": "password"}`. + ### Examples @@ -68,17 +69,62 @@ spec: #connectTimeout: "10" # Timeout in seconds for connection attempts ``` -#### Using a file reference (`adminSecretFileRef`) +### Using a file reference (`adminSecretFileRef`) + +Instead of a Kubernetes Secret, you can mount a JSON credentials file into the operator pod and reference its path. This is useful when credentials are managed externally. + +#### File format + +The file must contain JSON with the following fields: + +```json +{ + "username": "root", + "password": "password" +} +``` + +- `password` **required** +- `username` **required** + +#### Mount the credentials file + +The file must be accessible inside the operator pod at the path specified in `adminSecretFileRef.path`. Mount it using a Volume and VolumeMount on the operator Deployment: + +```yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: postgresql-operator +spec: + template: + spec: + containers: + - name: operator + volumeMounts: + - name: db-credentials + mountPath: /mnt/secrets + readOnly: true + volumes: + - name: db-credentials + secret: + secretName: db-credentials-secret +``` + +> **Note:** The volume source can be any type that provides a file. + +> **Note:** The Helm chart does not support extra volumes yet. ```yaml apiVersion: postgresql.aboutbits.it/v1 kind: ClusterConnection metadata: - name: my-postgres-connection + name: quarkus-postgres-connection spec: adminSecretFileRef: - path: "/mnt/db-password" + path: "/mnt/secrets/db-credentials.json" host: localhost port: 5432 database: postgres ``` + diff --git a/docs/docker-environment.md b/docs/docker-environment.md index 4c8be9a..134ae3d 100644 --- a/docs/docker-environment.md +++ b/docs/docker-environment.md @@ -43,8 +43,8 @@ For the `postgresql` Dev Service, you can generate the necessary Custom Resource A `ClusterConnection` requires admin credentials, which can be provided in one of two ways: -- **`adminSecretRef`** — references a Kubernetes `basic-auth` Secret (username + password). -- **`adminSecretFileRef`** — references a JSON file mounted into the operator pod (e.g. from AWS Secrets Manager). +- **`adminSecretRef`** references a Kubernetes `basic-auth` Secret (username + password). +- **`adminSecretFileRef`** references a JSON file mounted into the operator pod. Exactly one of these must be specified. @@ -88,65 +88,6 @@ spec: database: postgres ``` -### Using a file reference (`adminSecretFileRef`) - -Instead of a Kubernetes Secret, you can mount a JSON credentials file into the operator pod and reference its path. This is useful when credentials are managed externally (e.g. AWS Secrets Manager). - -#### File format - -The file must contain JSON with the following fields: - -```json -{ - "username": "root", - "password": "password" -} -``` - -- `password` — **required** -- `username` — optional (can be omitted) - -#### Mount the credentials file - -The file must be accessible inside the operator pod at the path specified in `adminSecretFileRef.path`. Mount it using a Volume and VolumeMount on the operator Deployment: - -```yaml -apiVersion: apps/v1 -kind: Deployment -metadata: - name: postgresql-operator -spec: - template: - spec: - containers: - - name: operator - volumeMounts: - - name: db-credentials - mountPath: /mnt/secrets - readOnly: true - volumes: - - name: db-credentials - secret: - secretName: db-credentials-secret -``` - -> **Note:** The volume source can be any type that provides a file (e.g. a Kubernetes Secret, a CSI volume from AWS Secrets Manager, or a ConfigMap for testing). - -#### Example ClusterConnection - -```yaml -apiVersion: postgresql.aboutbits.it/v1 -kind: ClusterConnection -metadata: - name: quarkus-postgres-connection -spec: - adminSecretFileRef: - path: "/mnt/secrets/db-credentials.json" - host: localhost - port: 5432 - database: postgres -``` - ![Established Cluster Connection](images/established-cluster-connection.png) ## 3. Create a Role diff --git a/operator/build.gradle.kts b/operator/build.gradle.kts index 0e8edf0..ca06d13 100644 --- a/operator/build.gradle.kts +++ b/operator/build.gradle.kts @@ -62,6 +62,7 @@ dependencies { */ testImplementation("io.quarkus:quarkus-junit") testImplementation("io.quarkus:quarkus-junit-mockito") + testImplementation("io.fabric8:kubernetes-server-mock") testImplementation("org.awaitility:awaitility") testImplementation(libs.assertj) testImplementation(libs.datafaker) diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java index 51ccf7e..436c2b5 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java @@ -10,14 +10,14 @@ /// /// This class is used wherever a CRD spec needs to point to a specific file /// The [#path] field identifies the file location -/// inside the secret. +/// within the container. /// /// ### Example usage in a CR manifest /// /// ```yaml /// spec: /// adminSecretFileRef: -/// path: "/mnt/db-password" +/// path: "/mnt/secrets/db-credentials.json" /// ``` @Getter @Setter diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java index ac91c84..9744a85 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java @@ -16,50 +16,55 @@ @Singleton @NullMarked public final class KubernetesService { + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + public static final String SECRET_TYPE_BASIC_AUTH = "kubernetes.io/basic-auth"; public static final String SECRET_DATA_BASIC_AUTH_USERNAME_KEY = "username"; public static final String SECRET_DATA_BASIC_AUTH_PASSWORD_KEY = "password"; - public Credentials getSecretRefCredentials( + public Credentials getAdminCredentials( KubernetesClient kubernetesClient, ClusterConnection clusterConnection ) { - ClusterConnectionSpec spec = clusterConnection.getSpec(); + var spec = clusterConnection.getSpec(); if (spec.getAdminSecretRef() != null) { return getSecretRefCredentials( kubernetesClient, - clusterConnection.getSpec().getAdminSecretRef(), + spec.getAdminSecretRef(), clusterConnection.getMetadata().getNamespace() ); - } else if (spec.getAdminSecretFileRef() != null) { return getSecretFileRefCredentials(spec.getAdminSecretFileRef()); } throw new IllegalStateException("Exactly one of 'adminSecretRef' or 'adminSecretFileRef' must be provided"); - } - public Credentials getSecretFileRefCredentials(ResourceFileRef fileRef) { + public Credentials getSecretFileRefCredentials(FileRef fileRef) { var path = Path.of(fileRef.getPath()); if (!Files.exists(path)) { - throw new IllegalStateException("AWS Secrets Manager file not found [path=%s]".formatted(path)); + throw new IllegalStateException("Credential file not found [path=%s]".formatted(path)); } try { var content = Files.readString(path); - var objectMapper = new ObjectMapper(); - var json = objectMapper.readTree(content); + var json = OBJECT_MAPPER.readTree(content); var usernameNode = json.get(SECRET_DATA_BASIC_AUTH_USERNAME_KEY); var username = usernameNode != null && !usernameNode.isNull() ? usernameNode.asText() : null; + if (username == null) { + throw new IllegalStateException("Credential file is missing required field '%s' [path=%s]".formatted( + SECRET_DATA_BASIC_AUTH_USERNAME_KEY, + path + )); + } var passwordNode = json.get(SECRET_DATA_BASIC_AUTH_PASSWORD_KEY); if (passwordNode == null || passwordNode.isNull()) { - throw new IllegalStateException("AWS Secrets Manager file is missing required field '%s' [path=%s]".formatted( + throw new IllegalStateException("Credential file is missing required field '%s' [path=%s]".formatted( SECRET_DATA_BASIC_AUTH_PASSWORD_KEY, path )); @@ -67,7 +72,7 @@ public Credentials getSecretFileRefCredentials(ResourceFileRef fileRef) { return new Credentials(username, passwordNode.asText()); } catch (IOException e) { - throw new IllegalStateException("Failed to read AWS Secrets Manager file [path=%s]".formatted(path), e); + throw new IllegalStateException("Failed to read Credential file [path=%s]".formatted(path), e); } } diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLContextFactory.java b/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLContextFactory.java index c2a2faa..711519e 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLContextFactory.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLContextFactory.java @@ -34,7 +34,7 @@ public CloseableDSLContext getDSLContext( ClusterConnection clusterConnection, String database ) throws DataAccessException { - var credentials = kubernetesService.getSecretRefCredentials( + var credentials = kubernetesService.getAdminCredentials( kubernetesClient, clusterConnection ); diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java b/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java deleted file mode 100644 index 10a91e2..0000000 --- a/operator/src/main/java/it/aboutbits/postgresql/core/ResourceFileRef.java +++ /dev/null @@ -1,34 +0,0 @@ -package it.aboutbits.postgresql.core; - -import io.fabric8.generator.annotation.Required; -import io.fabric8.generator.annotation.ValidationRule; -import lombok.Getter; -import lombok.Setter; -import org.jspecify.annotations.NullMarked; - -/// A reference to a file inside an AWS Secrets Manager secret. -/// -/// This class is used wherever a CRD spec needs to point to a specific file -/// within an AWS secret. The [#path] field identifies the file location -/// inside the secret. -/// -/// ### Example usage in a CR manifest -/// -/// ```yaml -/// spec: -/// adminSecretFileRef: -/// path: "/mnt/db-password" -/// ``` -@Getter -@Setter -@NullMarked -public class ResourceFileRef { - /// The path to the file inside the AWS Secrets Manager secret. - /// Must not be blank. - @Required - @ValidationRule( - value = "self.trim().size() > 0", - message = "The path must not be empty." - ) - private String path = ""; -} diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java b/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java index e494dd7..ebde156 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionSpec.java @@ -5,12 +5,13 @@ import io.fabric8.generator.annotation.Min; import io.fabric8.generator.annotation.Required; import io.fabric8.generator.annotation.ValidationRule; -import it.aboutbits.postgresql.core.ResourceFileRef; +import it.aboutbits.postgresql.core.FileRef; import it.aboutbits.postgresql.core.ResourceRef; import it.aboutbits.postgresql.core.schema_customizer.HostCustomizer; import lombok.Getter; import lombok.Setter; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.HashMap; import java.util.Map; @@ -44,10 +45,10 @@ public class ClusterConnectionSpec { private String database = "postgres"; @io.fabric8.generator.annotation.Nullable - private ResourceRef adminSecretRef; + private @Nullable ResourceRef adminSecretRef; @io.fabric8.generator.annotation.Nullable - private ResourceFileRef adminSecretFileRef; + private @Nullable FileRef adminSecretFileRef; @io.fabric8.generator.annotation.Nullable private Map parameters = new HashMap<>(); diff --git a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java index fe2d70b..81c3415 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java +++ b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import it.aboutbits.postgresql._support.testdata.base.TestDataCreator; import it.aboutbits.postgresql._support.testdata.persisted.Given; +import it.aboutbits.postgresql.core.FileRef; import it.aboutbits.postgresql.core.ResourceRef; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnectionSpec; @@ -41,6 +42,11 @@ public class ClusterConnectionCreate extends TestDataCreator private @Nullable ResourceRef withAdminSecretRef; + private @Nullable FileRef withAdminSecretFileRef; + + @Setter(AccessLevel.NONE) + private boolean withoutAdminSecret = false; + private @Nullable String withApplicationName; public ClusterConnectionCreate( @@ -61,6 +67,16 @@ public ClusterConnectionCreate withoutNamespace() { return this; } + public ClusterConnectionCreate withAdminSecretFileRef(FileRef fileRef) { + this.withAdminSecretFileRef = fileRef; + return this; + } + + public ClusterConnectionCreate withoutAdminSecret() { + this.withoutAdminSecret = true; + return this; + } + @Override protected ClusterConnection create(int index) { // given @@ -81,6 +97,9 @@ protected ClusterConnection create(int index) { spec.setPort(getPort()); spec.setDatabase(getDatabase()); spec.setAdminSecretRef(getAdminSecretRef()); + if (withAdminSecretFileRef != null) { + spec.setAdminSecretFileRef(withAdminSecretFileRef); + } spec.setParameters(getParameters()); item.setSpec(spec); @@ -116,7 +135,7 @@ protected ClusterConnection create(int index) { } private String getName() { - if (withName != null) { + if (withName != null) { return withName; } @@ -154,7 +173,11 @@ private String getDatabase() { return withDatabase; } - private ResourceRef getAdminSecretRef() { + private @Nullable ResourceRef getAdminSecretRef() { + if (withoutAdminSecret) { + return null; + } + if (withAdminSecretRef != null) { return withAdminSecretRef; } diff --git a/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java index b9635c3..b191cc6 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java @@ -2,18 +2,20 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.api.model.SecretBuilder; import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.dsl.MixedOperation; -import io.fabric8.kubernetes.client.dsl.NamespaceableResource; -import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnectionSpec; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.IOException; import java.nio.charset.Charset; @@ -24,16 +26,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; @NullMarked +@EnableKubernetesMockClient(crud = true) class KubernetesServiceTest { private final KubernetesService service = new KubernetesService(); + @SuppressWarnings("NullAway.Init") + static KubernetesClient client; + @TempDir Path tempDir; + @BeforeEach + void clearSecrets() { + client.secrets().inAnyNamespace().delete(); + } + @Nested class GetSecretFileRefCredentials { @Test @@ -45,7 +54,7 @@ void whenBothUsernameAndPassword_shouldReturnCredentials() throws IOException { {"username": "admin", "password": "s3cret"} """); - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(file.toString()); // when @@ -56,74 +65,36 @@ void whenBothUsernameAndPassword_shouldReturnCredentials() throws IOException { assertThat(result.password()).isEqualTo("s3cret"); } - @Test - @DisplayName("when username key missing, should return null username") - void whenUsernameKeyMissing_shouldReturnNullUsername() throws IOException { - // given - var file = tempDir.resolve("secret.json"); - Files.writeString(file, """ - {"password": "s3cret"} - """); - - var fileRef = new ResourceFileRef(); - fileRef.setPath(file.toString()); - - // when - var result = service.getSecretFileRefCredentials(fileRef); - - // then - assertThat(result.username()).isNull(); - assertThat(result.password()).isEqualTo("s3cret"); - } - - @Test - @DisplayName("when username is null, should return null username") - void whenUsernameIsNull_shouldReturnNullUsername() throws IOException { - // given - var file = tempDir.resolve("secret.json"); - Files.writeString(file, """ - {"username": null, "password": "s3cret"} - """); - - var fileRef = new ResourceFileRef(); - fileRef.setPath(file.toString()); - - // when - var result = service.getSecretFileRefCredentials(fileRef); - - // then - assertThat(result.username()).isNull(); - assertThat(result.password()).isEqualTo("s3cret"); - } - - @Test - @DisplayName("when password key missing, should throw") - void whenPasswordKeyMissing_shouldThrow() throws IOException { + @ParameterizedTest(name = "when username {0}, should throw") + @ValueSource(strings = { + "{\"password\": \"s3cret\"}", + "{\"username\": null, \"password\": \"s3cret\"}" + }) + void whenUsernameMissingOrNull_shouldThrow(String json) throws IOException { // given var file = tempDir.resolve("secret.json"); - Files.writeString(file, """ - {"username": "admin"} - """); + Files.writeString(file, json); - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(file.toString()); // when / then assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("missing required field 'password'"); + .hasMessageContaining("missing required field 'username'"); } - @Test - @DisplayName("when password is null, should throw") - void whenPasswordIsNull_shouldThrow() throws IOException { + @ParameterizedTest(name = "when password {0}, should throw") + @ValueSource(strings = { + "{\"username\": \"admin\"}", + "{\"username\": \"admin\", \"password\": null}" + }) + void whenPasswordMissingOrNull_shouldThrow(String json) throws IOException { // given var file = tempDir.resolve("secret.json"); - Files.writeString(file, """ - {"username": "admin", "password": null} - """); + Files.writeString(file, json); - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(file.toString()); // when / then @@ -136,13 +107,13 @@ void whenPasswordIsNull_shouldThrow() throws IOException { @DisplayName("when file not found, should throw") void whenFileNotFound_shouldThrow() { // given - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(tempDir.resolve("nonexistent.json").toString()); // when / then assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("file not found"); + .hasMessageContaining("Credential file not found"); } @Test @@ -152,7 +123,7 @@ void whenInvalidJson_shouldThrow() throws IOException { var file = tempDir.resolve("secret.json"); Files.writeString(file, "not valid json {{{"); - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(file.toString()); // when / then @@ -168,21 +139,11 @@ class GetSecretRefCredentials { @DisplayName("when secret exists, should return decoded credentials") void whenSecretExists_shouldReturnDecodedCredentials() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of( - "username", base64("admin"), - "password", base64("s3cret") - )); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = basicAuthSecret("my-ns", "my-secret", "admin", "s3cret"); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when - var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + var result = service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns"); // then assertThat(result.username()).isEqualTo("admin"); @@ -192,15 +153,10 @@ void whenSecretExists_shouldReturnDecodedCredentials() { @Test @DisplayName("when secret not found, should throw") void whenSecretNotFound_shouldThrow() { - // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var client = mockKubernetesClient("my-ns", "my-secret", null); + // given — no secret created // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("Secret reference not found"); } @@ -209,18 +165,15 @@ void whenSecretNotFound_shouldThrow() { @DisplayName("when secret wrong type, should throw") void whenSecretWrongType_shouldThrow() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType("Opaque"); - secret.setData(Map.of("password", base64("s3cret"))); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType("Opaque") + .addToData("password", base64("s3cret")) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("wrong type"); } @@ -229,18 +182,14 @@ void whenSecretWrongType_shouldThrow() { @DisplayName("when secret has no data, should throw") void whenSecretHasNoData_shouldThrow() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(null); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("has no data set"); } @@ -249,18 +198,15 @@ void whenSecretHasNoData_shouldThrow() { @DisplayName("when secret missing password, should throw") void whenSecretMissingPassword_shouldThrow() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of("username", base64("admin"))); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .addToData("username", base64("admin")) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("missing required data password"); } @@ -269,21 +215,11 @@ void whenSecretMissingPassword_shouldThrow() { @DisplayName("when namespace null, should use default namespace") void whenNamespaceNull_shouldUseDefaultNamespace() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace(null); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of( - "username", base64("admin"), - "password", base64("s3cret") - )); - - var client = mockKubernetesClient("default-ns", "my-secret", secret); + var secret = basicAuthSecret("default-ns", "my-secret", "admin", "s3cret"); + client.secrets().inNamespace("default-ns").resource(secret).create(); // when - var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + var result = service.getSecretRefCredentials(client, secretRef(null, "my-secret"), "default-ns"); // then assertThat(result.username()).isEqualTo("admin"); @@ -294,18 +230,15 @@ void whenNamespaceNull_shouldUseDefaultNamespace() { @DisplayName("when secret has empty data map, should throw") void whenSecretHasEmptyDataMap_shouldThrow() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of()); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .withData(Map.of()) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("has no data set"); } @@ -314,18 +247,15 @@ void whenSecretHasEmptyDataMap_shouldThrow() { @DisplayName("when secret has username only (no password), should throw") void whenSecretHasUsernameOnly_shouldThrow() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of("username", base64("admin"))); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .addToData("username", base64("admin")) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef, "default-ns")) + assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("missing required data password"); } @@ -334,18 +264,15 @@ void whenSecretHasUsernameOnly_shouldThrow() { @DisplayName("when secret has password only (no username), should return null username") void whenSecretHasPasswordOnly_shouldReturnNullUsername() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of("password", base64("s3cret"))); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .addToData("password", base64("s3cret")) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); // when - var result = service.getSecretRefCredentials(client, secretRef, "default-ns"); + var result = service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns"); // then assertThat(result.username()).isNull(); @@ -354,31 +281,21 @@ void whenSecretHasPasswordOnly_shouldReturnNullUsername() { } @Nested - class GetCredentialsDispatcher { + class GetAdminCredentials { @Test @DisplayName("when only adminSecretRef set, should delegate to secret ref") void whenOnlyAdminSecretRefSet_shouldDelegateToSecretRef() { // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("my-ns"); - secretRef.setName("my-secret"); + var secret = basicAuthSecret("my-ns", "my-secret", "admin", "s3cret"); + client.secrets().inNamespace("my-ns").resource(secret).create(); var spec = new ClusterConnectionSpec(); - spec.setAdminSecretRef(secretRef); + spec.setAdminSecretRef(secretRef("my-ns", "my-secret")); var clusterConnection = buildClusterConnection(spec, "cr-ns"); - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of( - "username", base64("admin"), - "password", base64("s3cret") - )); - - var client = mockKubernetesClient("my-ns", "my-secret", secret); - // when - var result = service.getSecretRefCredentials(client, clusterConnection); + var result = service.getAdminCredentials(client, clusterConnection); // then assertThat(result.username()).isEqualTo("admin"); @@ -394,7 +311,7 @@ void whenOnlyAdminSecretFileRefSet_shouldDelegateToFileRef() throws IOException {"username": "file-admin", "password": "file-s3cret"} """); - var fileRef = new ResourceFileRef(); + var fileRef = new FileRef(); fileRef.setPath(file.toString()); var spec = new ClusterConnectionSpec(); @@ -402,10 +319,8 @@ void whenOnlyAdminSecretFileRefSet_shouldDelegateToFileRef() throws IOException var clusterConnection = buildClusterConnection(spec, "cr-ns"); - var client = mock(KubernetesClient.class); - // when - var result = service.getSecretRefCredentials(client, clusterConnection); + var result = service.getAdminCredentials(client, clusterConnection); // then assertThat(result.username()).isEqualTo("file-admin"); @@ -420,73 +335,11 @@ void whenNeitherRefSet_shouldThrow() { var clusterConnection = buildClusterConnection(spec, "cr-ns"); - var client = mock(KubernetesClient.class); - // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, clusterConnection)) + assertThatThrownBy(() -> service.getAdminCredentials(client, clusterConnection)) .isInstanceOf(IllegalStateException.class) .hasMessageContaining("Exactly one of"); } - - @Test - @DisplayName("when adminSecretRef set, should use its namespace over CR namespace") - void whenAdminSecretRefHasNamespace_shouldUseSecretRefNamespace() { - // given - var secretRef = new ResourceRef(); - secretRef.setNamespace("explicit-ns"); - secretRef.setName("my-secret"); - - var spec = new ClusterConnectionSpec(); - spec.setAdminSecretRef(secretRef); - - var clusterConnection = buildClusterConnection(spec, "cr-ns"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of( - "username", base64("admin"), - "password", base64("s3cret") - )); - - var client = mockKubernetesClient("explicit-ns", "my-secret", secret); - - // when - var result = service.getSecretRefCredentials(client, clusterConnection); - - // then - assertThat(result.username()).isEqualTo("admin"); - assertThat(result.password()).isEqualTo("s3cret"); - } - - @Test - @DisplayName("when adminSecretRef has null namespace, should fall back to CR namespace") - void whenAdminSecretRefHasNullNamespace_shouldUseCrNamespace() { - // given - var secretRef = new ResourceRef(); - secretRef.setNamespace(null); - secretRef.setName("my-secret"); - - var spec = new ClusterConnectionSpec(); - spec.setAdminSecretRef(secretRef); - - var clusterConnection = buildClusterConnection(spec, "cr-ns"); - - var secret = new Secret(); - secret.setType(KubernetesService.SECRET_TYPE_BASIC_AUTH); - secret.setData(Map.of( - "username", base64("admin"), - "password", base64("s3cret") - )); - - var client = mockKubernetesClient("cr-ns", "my-secret", secret); - - // when - var result = service.getSecretRefCredentials(client, clusterConnection); - - // then - assertThat(result.username()).isEqualTo("admin"); - assertThat(result.password()).isEqualTo("s3cret"); - } } private static ClusterConnection buildClusterConnection(ClusterConnectionSpec spec, String namespace) { @@ -500,23 +353,23 @@ private static ClusterConnection buildClusterConnection(ClusterConnectionSpec sp return clusterConnection; } - @SuppressWarnings("unchecked") - private static KubernetesClient mockKubernetesClient( - String namespace, - String name, - @Nullable Secret secret - ) { - var client = mock(KubernetesClient.class); - var secrets = mock(MixedOperation.class); - var nsOp = mock(NonNamespaceOperation.class); - var resource = mock(NamespaceableResource.class); - - when(client.secrets()).thenReturn(secrets); - when(secrets.inNamespace(namespace)).thenReturn(nsOp); - when(nsOp.withName(name)).thenReturn(resource); - when(resource.get()).thenReturn(secret); - - return client; + private static ResourceRef secretRef(@Nullable String namespace, String name) { + var ref = new ResourceRef(); + ref.setNamespace(namespace); + ref.setName(name); + return ref; + } + + private static Secret basicAuthSecret(String namespace, String name, + @Nullable String username, String password) { + var builder = new SecretBuilder() + .withNewMetadata().withNamespace(namespace).withName(name).endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .addToData("password", base64(password)); + if (username != null) { + builder.addToData("username", base64(username)); + } + return builder.build(); } private static String base64(String value) { diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java index 22054be..11f454e 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java @@ -1,11 +1,13 @@ package it.aboutbits.postgresql.crd.clusterconnection; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; import io.quarkus.test.junit.QuarkusTest; import it.aboutbits.postgresql._support.testdata.base.TestUtil; import it.aboutbits.postgresql._support.testdata.persisted.Given; import it.aboutbits.postgresql.core.CRPhase; import it.aboutbits.postgresql.core.CRStatus; +import it.aboutbits.postgresql.core.FileRef; import it.aboutbits.postgresql.core.PostgreSQLContextFactory; import lombok.RequiredArgsConstructor; import org.jooq.DSLContext; @@ -13,6 +15,7 @@ import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import java.time.OffsetDateTime; @@ -23,6 +26,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.within; @QuarkusTest @@ -69,6 +73,35 @@ void createsCustomResource_andReconcilerStatusIsReady() { ); } + @Nested + @DisplayName("CRD Validation: AdminSecret Exclusivity") + class AdminSecretExclusivity { + @Test + @DisplayName("when both adminSecretRef and adminSecretFileRef set, should reject") + void whenBothSet_shouldReject() { + var fileRef = new FileRef(); + fileRef.setPath("/mnt/secrets/db-credentials.json"); + + assertThatThrownBy(() -> given.one() + .clusterConnection() + .withAdminSecretFileRef(fileRef) + .returnFirst() + ).isInstanceOf(KubernetesClientException.class) + .hasMessageContaining("Exactly one of"); + } + + @Test + @DisplayName("when neither adminSecretRef nor adminSecretFileRef set, should reject") + void whenNeitherSet_shouldReject() { + assertThatThrownBy(() -> given.one() + .clusterConnection() + .withoutAdminSecret() + .returnFirst() + ).isInstanceOf(KubernetesClientException.class) + .hasMessageContaining("Exactly one of"); + } + } + private static void assertThatClusterConnectionHasExpectedStatus( ClusterConnection clusterConnection, CRStatus expectedStatus, From 611095890c2ebf343ce244a122ad503c0f028e7d Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Thu, 3 Sep 2026 15:03:24 +0200 Subject: [PATCH 4/6] fix indentation --- .../testdata/persisted/creator/ClusterConnectionCreate.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java index 81c3415..72e41b4 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java +++ b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java @@ -135,7 +135,7 @@ protected ClusterConnection create(int index) { } private String getName() { - if (withName != null) { + if (withName != null) { return withName; } From b335a78ca595381cb5f4fcc68c30c9f78a6576ee Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Fri, 4 Sep 2026 10:27:37 +0200 Subject: [PATCH 5/6] Addressed PR review: inject ObjectMapper, fix username validation for Secret branch, improve tests and docs --- .gitignore | 3 +- docs/cluster-connection.md | 99 +++++++++---------- .../it/aboutbits/postgresql/core/FileRef.java | 5 +- .../postgresql/core/KubernetesService.java | 79 ++++++++------- .../core/KubernetesServiceTest.java | 66 +++++++++---- .../ClusterConnectionReconcilerTest.java | 70 +++++++++++++ 6 files changed, 210 insertions(+), 112 deletions(-) diff --git a/.gitignore b/.gitignore index 3319c66..279947e 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ config/ !.idea/misc.xml !.idea/sqldialects.xml !.idea/vcs.xml +**/out/ *.iml *.ipr @@ -60,5 +61,3 @@ gradle-app.setting # Quinoa .quinoa/ - -**/out/ diff --git a/docs/cluster-connection.md b/docs/cluster-connection.md index 9281333..86905b9 100644 --- a/docs/cluster-connection.md +++ b/docs/cluster-connection.md @@ -7,14 +7,14 @@ Other Custom Resources (like `Database`, `Role`, `Schema`, `Grant`, `DefaultPriv ## Spec -| Field | Type | Description | Required | Mutable | -|---------------------|---------------------|-----------------------------------------------------------------------|----------|---------| -| `host` | `string` | The hostname of the PostgreSQL instance. | Yes | Yes | -| `port` | `integer` | The port of the PostgreSQL instance (1-65535). | Yes | Yes | -| `database` | `string` | The database to connect to (usually `postgres` for admin operations). | Yes | Yes | -| `adminSecretRef` | `ResourceRef` | Reference to the Kubernetes Secret containing the admin credentials. | No | Yes | -| `adminSecretFileRef`| `ResourceFileRef` | Reference to a file containing the admin credentials. | No | Yes | -| `parameters` | `map[string]string` | Additional connection parameters. | No | Yes | +| Field | Type | Description | Required | Mutable | +|----------------------|----------------------|-----------------------------------------------------------------------|----------|---------| +| `host` | `string` | The hostname of the PostgreSQL instance. | Yes | Yes | +| `port` | `integer` | The port of the PostgreSQL instance (1-65535). | Yes | Yes | +| `database` | `string` | The database to connect to (usually `postgres` for admin operations). | Yes | Yes | +| `adminSecretRef` | `ResourceRef` | Reference to the Kubernetes Secret containing the admin credentials. | No | Yes | +| `adminSecretFileRef` | `FileRef` | Reference to a file containing the admin credentials. | No | Yes | +| `parameters` | `map[string]string` | Additional connection parameters. | No | Yes | > **Note:** Exactly one of `adminSecretRef` or `adminSecretFileRef` must be provided. @@ -27,53 +27,15 @@ Other Custom Resources (like `Database`, `Role`, `Schema`, `Grant`, `DefaultPriv The referenced secret must be of type `kubernetes.io/basic-auth` and contain the keys `username` and `password`. -### ResourceFileRef (`adminSecretFileRef`) +### FileRef (`adminSecretFileRef`) | Field | Type | Description | Required | |--------|----------|----------------------------------------------------------------|----------| | `path` | `string` | The path to the file containing the admin credentials. | Yes | -Use this option when the credentials are mounted as a file instead of a Kubernetes Secret. The file must be a JSON object with the keys `username` and `password`, for example `{"username": "postgres", "password": "password"}`. +Use this option when the credentials are mounted as a file instead of a Kubernetes Secret. - -### Examples - -#### Using a Kubernetes Secret (`adminSecretRef`) - -```yaml -apiVersion: v1 -kind: Secret -metadata: - name: my-db-secret -type: kubernetes.io/basic-auth -stringData: - username: postgres - password: password -``` - -```yaml -apiVersion: postgresql.aboutbits.it/v1 -kind: ClusterConnection -metadata: - name: my-postgres-connection -spec: - adminSecretRef: - name: my-db-secret - host: localhost - port: 5432 - database: postgres - # Example parameters - parameters: - ApplicationName: "k8s-operator" # Helps identify this connection in Postgres logs - #sslmode: "require" # Enforce SSL encryption - #connectTimeout: "10" # Timeout in seconds for connection attempts -``` - -### Using a file reference (`adminSecretFileRef`) - -Instead of a Kubernetes Secret, you can mount a JSON credentials file into the operator pod and reference its path. This is useful when credentials are managed externally. - -#### File format +### File format The file must contain JSON with the following fields: @@ -100,7 +62,7 @@ spec: template: spec: containers: - - name: operator + - name: postgresql-operator volumeMounts: - name: db-credentials mountPath: /mnt/secrets @@ -113,7 +75,42 @@ spec: > **Note:** The volume source can be any type that provides a file. -> **Note:** The Helm chart does not support extra volumes yet. +> **Note:** The Helm chart does not support extra volumes yet. + +### Examples + +#### Using a Kubernetes Secret (`adminSecretRef`) + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: my-db-secret +type: kubernetes.io/basic-auth +stringData: + username: postgres + password: password +``` + +```yaml +apiVersion: postgresql.aboutbits.it/v1 +kind: ClusterConnection +metadata: + name: my-postgres-connection +spec: + adminSecretRef: + name: my-db-secret + host: localhost + port: 5432 + database: postgres + # Example parameters + parameters: + ApplicationName: "k8s-operator" # Helps identify this connection in Postgres logs + #sslmode: "require" # Enforce SSL encryption + #connectTimeout: "10" # Timeout in seconds for connection attempts +``` + +#### Using a file reference (`adminSecretFileRef`) ```yaml apiVersion: postgresql.aboutbits.it/v1 diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java index 436c2b5..ab73258 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/FileRef.java @@ -6,11 +6,10 @@ import lombok.Setter; import org.jspecify.annotations.NullMarked; -/// A reference to a file inside +/// A reference to a file inside the operator container. /// /// This class is used wherever a CRD spec needs to point to a specific file -/// The [#path] field identifies the file location -/// within the container. +/// The [#path] field identifies the file location within the container. /// /// ### Example usage in a CR manifest /// diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java index 9744a85..33a4d78 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/KubernetesService.java @@ -3,20 +3,29 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.fabric8.kubernetes.client.KubernetesClient; import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; -import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnectionSpec; import jakarta.inject.Singleton; +import lombok.RequiredArgsConstructor; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.io.IOException; import java.nio.charset.Charset; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Base64; @Singleton +@RequiredArgsConstructor @NullMarked public final class KubernetesService { - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private final ObjectMapper objectMapper; + + private record FileCredentials( + @Nullable String username, + @Nullable String password + ) { + } public static final String SECRET_TYPE_BASIC_AUTH = "kubernetes.io/basic-auth"; public static final String SECRET_DATA_BASIC_AUTH_USERNAME_KEY = "username"; @@ -28,11 +37,16 @@ public Credentials getAdminCredentials( ) { var spec = clusterConnection.getSpec(); if (spec.getAdminSecretRef() != null) { - return getSecretRefCredentials( - kubernetesClient, - spec.getAdminSecretRef(), - clusterConnection.getMetadata().getNamespace() - ); + var secretRef = spec.getAdminSecretRef(); + var defaultNamespace = clusterConnection.getMetadata().getNamespace(); + var credentials = getSecretRefCredentials(kubernetesClient, secretRef, defaultNamespace); + if (credentials.username() == null) { + var secretNamespace = getSecretNamespace(secretRef, defaultNamespace); + throw new IllegalStateException( + "The Secret reference is missing required data username [secret.namespace=%s, secret.name=%s]".formatted( + secretNamespace, secretRef.getName())); + } + return credentials; } else if (spec.getAdminSecretFileRef() != null) { return getSecretFileRefCredentials(spec.getAdminSecretFileRef()); } @@ -43,36 +57,23 @@ public Credentials getAdminCredentials( public Credentials getSecretFileRefCredentials(FileRef fileRef) { var path = Path.of(fileRef.getPath()); - if (!Files.exists(path)) { - throw new IllegalStateException("Credential file not found [path=%s]".formatted(path)); - } - - try { - var content = Files.readString(path); - var json = OBJECT_MAPPER.readTree(content); - - var usernameNode = json.get(SECRET_DATA_BASIC_AUTH_USERNAME_KEY); - var username = usernameNode != null && !usernameNode.isNull() - ? usernameNode.asText() - : null; - if (username == null) { - throw new IllegalStateException("Credential file is missing required field '%s' [path=%s]".formatted( - SECRET_DATA_BASIC_AUTH_USERNAME_KEY, - path - )); + try (var in = Files.newInputStream(path)) { + var file = objectMapper.readValue(in, FileCredentials.class); + if (file.username() == null) { + throw new IllegalStateException( + "Credentials file is missing required field 'username' [path=%s]".formatted(path)); } - - var passwordNode = json.get(SECRET_DATA_BASIC_AUTH_PASSWORD_KEY); - if (passwordNode == null || passwordNode.isNull()) { - throw new IllegalStateException("Credential file is missing required field '%s' [path=%s]".formatted( - SECRET_DATA_BASIC_AUTH_PASSWORD_KEY, - path - )); + if (file.password() == null) { + throw new IllegalStateException( + "Credentials file is missing required field 'password' [path=%s]".formatted(path)); } - - return new Credentials(username, passwordNode.asText()); + return new Credentials(file.username(), file.password()); + } catch (NoSuchFileException e) { + throw new IllegalStateException( + "Credentials file not found [path=%s]".formatted(path), e); } catch (IOException e) { - throw new IllegalStateException("Failed to read Credential file [path=%s]".formatted(path), e); + throw new IllegalStateException( + "Failed to read the credentials file [path=%s]".formatted(path), e); } } @@ -81,9 +82,7 @@ public Credentials getSecretRefCredentials( ResourceRef secretRef, String defaultNamespace ) { - var secretNamespace = secretRef.getNamespace() != null - ? secretRef.getNamespace() - : defaultNamespace; + var secretNamespace = getSecretNamespace(secretRef, defaultNamespace); var secretName = secretRef.getName(); @@ -141,4 +140,10 @@ public Credentials getSecretRefCredentials( password ); } + + private String getSecretNamespace(ResourceRef secretRef, String defaultNamespace) { + return secretRef.getNamespace() != null + ? secretRef.getNamespace() + : defaultNamespace; + } } diff --git a/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java index b191cc6..481133d 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/core/KubernetesServiceTest.java @@ -17,6 +17,8 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.io.IOException; import java.nio.charset.Charset; import java.nio.file.Files; @@ -30,7 +32,7 @@ @NullMarked @EnableKubernetesMockClient(crud = true) class KubernetesServiceTest { - private final KubernetesService service = new KubernetesService(); + private final KubernetesService service = new KubernetesService(new ObjectMapper()); @SuppressWarnings("NullAway.Init") static KubernetesClient client; @@ -103,6 +105,27 @@ void whenPasswordMissingOrNull_shouldThrow(String json) throws IOException { .hasMessageContaining("missing required field 'password'"); } + @ParameterizedTest(name = "when field has wrong type {0}, should throw") + @ValueSource(strings = { + "{\"username\": {}, \"password\": \"s3cret\"}", + "{\"username\": [], \"password\": \"s3cret\"}", + "{\"username\": \"admin\", \"password\": {}}", + "{\"username\": \"admin\", \"password\": []}" + }) + void whenFieldHasWrongType_shouldThrow(String json) throws IOException { + // given + var file = tempDir.resolve("secret.json"); + Files.writeString(file, json); + + var fileRef = new FileRef(); + fileRef.setPath(file.toString()); + + // when / then + assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Failed to read"); + } + @Test @DisplayName("when file not found, should throw") void whenFileNotFound_shouldThrow() { @@ -113,7 +136,7 @@ void whenFileNotFound_shouldThrow() { // when / then assertThatThrownBy(() -> service.getSecretFileRefCredentials(fileRef)) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Credential file not found"); + .hasMessageContaining("Credentials file not found"); } @Test @@ -194,23 +217,6 @@ void whenSecretHasNoData_shouldThrow() { .hasMessageContaining("has no data set"); } - @Test - @DisplayName("when secret missing password, should throw") - void whenSecretMissingPassword_shouldThrow() { - // given - var secret = new SecretBuilder() - .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() - .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) - .addToData("username", base64("admin")) - .build(); - client.secrets().inNamespace("my-ns").resource(secret).create(); - - // when / then - assertThatThrownBy(() -> service.getSecretRefCredentials(client, secretRef("my-ns", "my-secret"), "default-ns")) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("missing required data password"); - } - @Test @DisplayName("when namespace null, should use default namespace") void whenNamespaceNull_shouldUseDefaultNamespace() { @@ -327,6 +333,28 @@ void whenOnlyAdminSecretFileRefSet_shouldDelegateToFileRef() throws IOException assertThat(result.password()).isEqualTo("file-s3cret"); } + @Test + @DisplayName("when adminSecretRef missing username, should throw with message not NPE") + void whenAdminSecretRefMissingUsername_shouldThrowWithMessage() { + // given + var secret = new SecretBuilder() + .withNewMetadata().withNamespace("my-ns").withName("my-secret").endMetadata() + .withType(KubernetesService.SECRET_TYPE_BASIC_AUTH) + .addToData("password", base64("s3cret")) + .build(); + client.secrets().inNamespace("my-ns").resource(secret).create(); + + var spec = new ClusterConnectionSpec(); + spec.setAdminSecretRef(secretRef("my-ns", "my-secret")); + + var clusterConnection = buildClusterConnection(spec, "cr-ns"); + + // when / then + assertThatThrownBy(() -> service.getAdminCredentials(client, clusterConnection)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required data username"); + } + @Test @DisplayName("when neither ref set, should throw") void whenNeitherRefSet_shouldThrow() { diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java index 11f454e..9ac4f33 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java @@ -10,6 +10,7 @@ import it.aboutbits.postgresql.core.FileRef; import it.aboutbits.postgresql.core.PostgreSQLContextFactory; import lombok.RequiredArgsConstructor; +import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jooq.DSLContext; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -18,6 +19,8 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.io.IOException; +import java.nio.file.Files; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; @@ -39,6 +42,14 @@ class ClusterConnectionReconcilerTest { private final KubernetesClient kubernetesClient; + @SuppressWarnings("NullAway.Init") + @ConfigProperty(name = "quarkus.datasource.devservices.username") + String dbUsername; + + @SuppressWarnings("NullAway.Init") + @ConfigProperty(name = "quarkus.datasource.devservices.password") + String dbPassword; + @BeforeEach void resetEnvironment() { TestUtil.resetEnvironment(kubernetesClient); @@ -73,6 +84,50 @@ void createsCustomResource_andReconcilerStatusIsReady() { ); } + @Test + @DisplayName("When a ClusterConnection with adminSecretFileRef is created, the status should be ready") + void createsCustomResourceWithFileRef_andReconcilerStatusIsReady() throws IOException { + // given + var credentialsFile = Files.createTempFile("db-credentials", ".json"); + try { + Files.writeString(credentialsFile, """ + {"username": "%s", "password": "%s"} + """.formatted(dbUsername, dbPassword)); + + var fileRef = new FileRef(); + fileRef.setPath(credentialsFile.toAbsolutePath().toString()); + + // when + var customResource = given.one() + .clusterConnection() + .withName("test-connection-file-ref") + .withoutAdminSecret() + .withAdminSecretFileRef(fileRef) + .returnFirst(); + + // then + AtomicReference<@Nullable DSLContext> dslAtomic = new AtomicReference<>(); + assertThatNoException().isThrownBy( + () -> dslAtomic.set(postgreSQLContextFactory.getDSLContext(customResource)) + ); + + var dsl = Objects.requireNonNull(dslAtomic.get()); + + var version = dsl.fetchSingle("select version()").into(String.class); + + var expectedStatus = getInitialClusterConnectionStatus(customResource); + expectedStatus.setMessage(version); + + assertThatClusterConnectionHasExpectedStatus( + customResource, + expectedStatus, + OffsetDateTime.now(ZoneOffset.UTC) + ); + } finally { + Files.deleteIfExists(credentialsFile); + } + } + @Nested @DisplayName("CRD Validation: AdminSecret Exclusivity") class AdminSecretExclusivity { @@ -100,6 +155,21 @@ void whenNeitherSet_shouldReject() { ).isInstanceOf(KubernetesClientException.class) .hasMessageContaining("Exactly one of"); } + + @Test + @DisplayName("when adminSecretFileRef has blank path, should reject") + void whenFileRefBlankPath_shouldReject() { + var fileRef = new FileRef(); + fileRef.setPath(" "); + + assertThatThrownBy(() -> given.one() + .clusterConnection() + .withoutAdminSecret() + .withAdminSecretFileRef(fileRef) + .returnFirst() + ).isInstanceOf(KubernetesClientException.class) + .hasMessageContaining("must not be empty"); + } } private static void assertThatClusterConnectionHasExpectedStatus( From 544360c1913fe0b51e7c945078ef2c765d1d5743 Mon Sep 17 00:00:00 2001 From: Fred Campos Date: Fri, 4 Sep 2026 10:50:55 +0200 Subject: [PATCH 6/6] Moved AdminSecretExclusivity to CRDValidation class --- .../ClusterConnectionReconcilerTest.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java index 9ac4f33..e5b29dd 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/clusterconnection/ClusterConnectionReconcilerTest.java @@ -5,6 +5,7 @@ import io.quarkus.test.junit.QuarkusTest; import it.aboutbits.postgresql._support.testdata.base.TestUtil; import it.aboutbits.postgresql._support.testdata.persisted.Given; +import it.aboutbits.postgresql._support.valuesource.BlankSource; import it.aboutbits.postgresql.core.CRPhase; import it.aboutbits.postgresql.core.CRStatus; import it.aboutbits.postgresql.core.FileRef; @@ -18,6 +19,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; import java.io.IOException; import java.nio.file.Files; @@ -129,8 +131,7 @@ void createsCustomResourceWithFileRef_andReconcilerStatusIsReady() throws IOExce } @Nested - @DisplayName("CRD Validation: AdminSecret Exclusivity") - class AdminSecretExclusivity { + class CRDValidation { @Test @DisplayName("when both adminSecretRef and adminSecretFileRef set, should reject") void whenBothSet_shouldReject() { @@ -156,11 +157,12 @@ void whenNeitherSet_shouldReject() { .hasMessageContaining("Exactly one of"); } - @Test + @ParameterizedTest + @BlankSource @DisplayName("when adminSecretFileRef has blank path, should reject") - void whenFileRefBlankPath_shouldReject() { + void whenFileRefBlankPath_shouldReject(String blankOrEmptyString) { var fileRef = new FileRef(); - fileRef.setPath(" "); + fileRef.setPath(blankOrEmptyString); assertThatThrownBy(() -> given.one() .clusterConnection()