From c25e3bba3803334e3a516ff7412cb204d852b31c Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Thu, 1 Oct 2026 16:45:46 +0200 Subject: [PATCH 1/6] add the GarageCluster and S3Connection resources GarageCluster assigns and applies the layout of a single-node Garage, S3Connection probes the Admin API that buckets and keys are later created through. Assisted-by: Claude:claude-opus-5-5 --- AGENTS.md | 2 + README.md | 4 +- docs/garage-cluster.md | 77 ++++++ docs/s3-connection.md | 60 +++++ .../src/main/docker/compose-devservices.yml | 17 ++ operator/src/main/docker/garage.toml | 29 ++ .../aboutbits/garage/core/BaseReconciler.java | 138 ++++++++++ .../garage/core/KubernetesService.java | 138 ++++++++++ .../core/adminapi/BearerTokenFilter.java | 22 ++ .../core/adminapi/ConnectionProbeResult.java | 18 ++ .../garage/core/adminapi/GarageAdminApi.java | 44 +++ .../adminapi/GarageAdminClientFactory.java | 134 +++++++++ .../garage/core/adminapi/GarageService.java | 39 +++ .../dto/ApplyClusterLayoutRequest.java | 10 + .../dto/ApplyClusterLayoutResponse.java | 14 + .../dto/GetClusterHealthResponse.java | 19 ++ .../dto/GetClusterLayoutResponse.java | 15 ++ .../dto/GetClusterStatusResponse.java | 14 + .../core/adminapi/dto/LayoutNodeRole.java | 20 ++ .../core/adminapi/dto/NodeAssignedRole.java | 19 ++ .../garage/core/adminapi/dto/NodeResp.java | 20 ++ .../adminapi/dto/NodeRoleChangeRequest.java | 41 +++ .../dto/UpdateClusterLayoutRequest.java | 13 + .../crd/garagecluster/GarageCluster.java | 59 ++++ .../GarageClusterLayoutSpec.java | 46 ++++ .../GarageClusterReconciler.java | 149 ++++++++++ .../garagecluster/GarageClusterService.java | 129 +++++++++ .../crd/garagecluster/GarageClusterSpec.java | 29 ++ .../garagecluster/GarageClusterStatus.java | 26 ++ .../garagecluster/LayoutReconcileResult.java | 78 ++++++ .../garage/crd/s3connection/S3Connection.java | 50 ++++ .../s3connection/S3ConnectionReconciler.java | 96 +++++++ .../crd/s3connection/S3ConnectionSpec.java | 25 ++ .../src/main/resources/application-dev.yml | 7 + .../src/main/resources/application-test.yml | 8 + .../testdata/base/TestDataCreator.java | 70 +++++ .../_support/testdata/base/TestUtil.java | 36 +++ .../_support/testdata/persisted/Given.java | 96 +++++++ .../creator/GarageClusterCreate.java | 180 +++++++++++++ .../persisted/creator/S3ConnectionCreate.java | 135 ++++++++++ .../persisted/creator/SecretKeyRefCreate.java | 143 ++++++++++ .../core/adminapi/GarageServiceTest.java | 66 +++++ .../GarageClusterReconcilerTest.java | 254 ++++++++++++++++++ .../GarageClusterServiceTest.java | 136 ++++++++++ .../S3ConnectionReconcilerTest.java | 177 ++++++++++++ .../it/aboutbits/garage/helm/HelmTest.java | 10 +- 46 files changed, 2910 insertions(+), 2 deletions(-) create mode 100644 docs/garage-cluster.md create mode 100644 docs/s3-connection.md create mode 100644 operator/src/main/docker/compose-devservices.yml create mode 100644 operator/src/main/docker/garage.toml create mode 100644 operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/ConnectionProbeResult.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminApi.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageService.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutRequest.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutResponse.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterHealthResponse.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterLayoutResponse.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterStatusResponse.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/LayoutNodeRole.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeAssignedRole.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeResp.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/UpdateClusterLayoutRequest.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterStatus.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconciler.java create mode 100644 operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestDataCreator.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java create mode 100644 operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java create mode 100644 operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java create mode 100644 operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java create mode 100644 operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java create mode 100644 operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java diff --git a/AGENTS.md b/AGENTS.md index c4ddcae..20bcaad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,8 @@ When in doubt about a pattern, look there first. - **Stack**: Java 25, Quarkus, the Quarkus Operator SDK and the fabric8 Kubernetes client. - **Generated artifacts**: the CRDs and the Helm chart are generated from the code at build time, into `operator/build/kubernetes` and `operator/build/helm`. Change the Java classes or `application.yml`, never the output. +- **Backend**: Garage only, driven through its Admin API v2. `GarageService` in `core/adminapi` is what the + reconcilers call; `GarageAdminApi` is the HTTP client underneath. - **Tests**: `@QuarkusTest` integration tests against a K3s cluster and a real Garage node, both provided by Quarkus Dev Services, so Docker is required. Prefer a test against the real Garage over a mock: what matters is how the Admin API actually behaves. diff --git a/README.md b/README.md index fd59d87..d97e9c0 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ git config core.hooksPath .githooks ### Development -You can run the operator in dev mode, against a throwaway K3s cluster, using: +You can run the operator in dev mode, against a throwaway K3s cluster and a real Garage node, using: ```bash make run @@ -82,6 +82,8 @@ make test ./gradlew :operator:test ``` +The tests are `@QuarkusTest` integration tests against real infrastructure provided by Quarkus Dev Services: a K3s cluster with the CRDs applied, and a Garage node started from `operator/src/main/docker/compose-devservices.yml`. The Garage image there is pinned to the version the Helm chart deploys, so the Admin API contract is exercised against the real thing. + ## Information About Bits is a company based in South Tyrol, Italy. You can find more information about us on [our website](https://aboutbits.it). diff --git a/docs/garage-cluster.md b/docs/garage-cluster.md new file mode 100644 index 0000000..5322c33 --- /dev/null +++ b/docs/garage-cluster.md @@ -0,0 +1,77 @@ +# GarageCluster + +A `GarageCluster` bootstraps and converges the **cluster layout** of a Garage installation. + +This is the resource everything else depends on. A freshly installed Garage node knows about itself but has no role in any layout, and in that state it **serves no S3 traffic at all** — every request fails until a layout is assigned and applied. The +[AboutBits Garage Helm chart](https://github.com/aboutbits/helm-garage) deliberately stops at day-0 concerns (StatefulSet, volumes, `garage.toml`, the admin token), so assigning that layout is the operator's job. + +The operator drives Garage through its +[Admin API v2](https://garagehq.deuxfleurs.fr/documentation/reference-manual/admin-api/), not through the S3 API — Garage implements neither IAM nor bucket policies, so buckets, keys and permissions are all Admin API concerns too. + +Once the layout is applied, an [`S3Connection`](s3-connection.md) pointed at the same cluster becomes +`READY`, which is what unblocks buckets and access keys. + +## Scope + +This version manages **single-node clusters**, which is what the Helm chart deploys (`replication_factor = 1`, no clustering or node discovery). If the cluster reports more than one node, the operator refuses to touch the layout and reports `ERROR` rather than guessing a zone and capacity per node. + +Deleting a `GarageCluster` removes the operator's record of the cluster. It does **not** tear the storage cluster down: removing a node's role would trigger a rebalance and, on a single-node cluster, discard the data. There is therefore no reclaim policy on this resource. + +## Example + +```yaml +apiVersion: garage.aboutbits.it/v1 +kind: GarageCluster +metadata: + name: acme + namespace: acme +spec: + adminEndpoint: http://acme-garage.acme.svc:3903 + adminSecretRef: + name: acme-garage + key: admin_token + layout: + zone: default + capacity: 20Gi + tags: + - production +``` + +## Spec + +| Field | Required | Description | +|----------------------------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `adminEndpoint` | yes | Base URL of the Garage Admin API. The Helm chart exposes it on port 3903 as a ClusterIP-only Service, reachable from inside the cluster only. | +| `adminSecretRef.name` | yes | Name of the `Opaque` Secret holding the Admin API bearer token. The chart creates one named after the release. | +| `adminSecretRef.namespace` | no | Defaults to the namespace of the `GarageCluster`. | +| `adminSecretRef.key` | yes | Key within the Secret. The chart stores the token under `admin_token`. | +| `layout.zone` | yes | Zone the node is placed in. With `replication_factor = 1` this is effectively a label. | +| `layout.capacity` | yes | Storage capacity to advertise, as a Kubernetes quantity such as `20Gi`. Should match the size of the chart's data volume. Garage uses it to weight partition distribution; it is not enforced as a limit. | +| `layout.tags` | no | Tags to attach to the node. Informational; order is not significant. | + +Note that the CRD schema carries no defaults — every required field has to be spelled out in the manifest. + +## Status + +| Field | Description | +|-----------------------------------|--------------------------------------------------------------------------------| +| `phase` | `PENDING`, `READY`, `ERROR` | +| `layoutVersion` | Version number of the applied cluster layout. `0` means none has been applied. | +| `nodeId` | Full-length identifier of the Garage node. | +| `health` | Cluster health as reported by Garage: `healthy`, `degraded` or `unavailable`. | +| `storageNodes` / `storageNodesUp` | Storage nodes in the layout, and how many are connected. | +| `message` | Human-readable detail about the current state. | + +`PENDING` is the normal state right after a `helm install`, while Garage is still starting and reports no nodes or a disconnected node; it is retried every 15 seconds. + +## How reconciliation works + +1. Read `GetClusterStatus`. No nodes, or the node not connected, means Garage is still coming up → + `PENDING`, retry. +2. Compare the node's assigned role against the spec. Tags are compared as a set, and capacity is compared in bytes, so a reordered tag list or `20Gi` versus `20Gi` is not a change. +3. If they match, stop — nothing is written to Garage. +4. Otherwise read `GetClusterLayout` for the authoritative version number, stage the role with + `UpdateClusterLayout`, then commit it with `ApplyClusterLayout` at version + 1. +5. Read `GetClusterHealth` to fill in the status, and report `READY`. + +Because step 2 short-circuits, reconciliation is idempotent: a `GarageCluster` whose layout already matches never creates a new layout version, no matter how often it is reconciled. diff --git a/docs/s3-connection.md b/docs/s3-connection.md new file mode 100644 index 0000000..6ccc66d --- /dev/null +++ b/docs/s3-connection.md @@ -0,0 +1,60 @@ +# S3Connection + +An `S3Connection` names a Garage Admin API and the token to administer it. Buckets, access keys and permissions reference a connection, which tells the operator where to create them. + +The operator's work is almost entirely **admin-plane**, and the admin plane is not part of the S3 API: Garage has no IAM and no bucket policies, and manages buckets, keys and their per-key, per-bucket grants through its Admin API only. + +## Relationship to GarageCluster + +For Garage the two are siblings, not alternatives: + +| Resource | Answers | +|--------------------------------------|-------------------------------------------------------------| +| [`GarageCluster`](garage-cluster.md) | Is the cluster itself set up? (layout assigned and applied) | +| `S3Connection` | Can I administer object storage on this backend right now? | + +A connection only reports `READY` when the backend can actually serve requests. A Garage node that answers its Admin API but has no layout reports health `unavailable`, and the connection stays +`PENDING` with a message pointing at the `GarageCluster`. Because `Bucket` and key resources gate on their connection being `READY`, that one check keeps them from acting against a cluster that is not finished bootstrapping. + +Both resources reference the same admin endpoint and Secret. That duplication is deliberate — each is self-contained, and a backend the operator only *uses* does not need a `GarageCluster` at all. + +## Example + +```yaml +apiVersion: garage.aboutbits.it/v1 +kind: S3Connection +metadata: + name: acme + namespace: acme +spec: + adminEndpoint: http://acme-garage.acme.svc:3903 + adminSecretRef: + name: acme-garage + key: admin_token +``` + +## Spec + +| Field | Required | Description | +|----------------------------|----------|-----------------------------------------------------------------------------------------------------------| +| `adminEndpoint` | yes | Base URL of the Garage **Admin** API, port 3903. This is not the S3 endpoint clients use. | +| `adminSecretRef.name` | yes | Name of the Secret holding the admin credentials. | +| `adminSecretRef.namespace` | no | Defaults to the namespace of the `S3Connection`. | +| `adminSecretRef.key` | yes | Key within the Secret. The AboutBits Garage chart uses `admin_token`. | + +## Status + +| Phase | Meaning | +|-----------|---------------------------------------------------------------------------------------------------------------------------------------------------------| +| `READY` | The backend is reachable, the credentials are accepted, and it can serve requests. | +| `PENDING` | Reachable and authenticated, but not ready — for Garage, health is `unavailable`, usually because no layout has been applied. Retried every 30 seconds. | +| `ERROR` | Unreachable, or the credentials were rejected, or the Secret is missing. | + +`message` carries the backend detail, for example +`Garage cluster healthy [storageNodes=1, storageNodesUp=1, partitionsAllOk=256/256]`. + +Note that Garage's `degraded` status still counts as `READY`: a quorum of write nodes is available, so reads and writes succeed. Only `unavailable` blocks. + +## Health endpoint + +The operator's readiness endpoint does **not** probe backends. One unreachable Garage would otherwise make the operator itself unready and block its rollout, although the operator is fine and every other backend is still being served. The state of each backend is reported on its `S3Connection` instead. diff --git a/operator/src/main/docker/compose-devservices.yml b/operator/src/main/docker/compose-devservices.yml new file mode 100644 index 0000000..a6b2235 --- /dev/null +++ b/operator/src/main/docker/compose-devservices.yml @@ -0,0 +1,17 @@ +services: + garage: + # Keep this in sync with the image the AboutBits Garage Helm chart deploys. + # https://github.com/aboutbits/helm-garage + image: dxflrs/garage:v2.3.0 + volumes: + - ./garage.toml:/etc/garage.toml:ro + tmpfs: + - /var/lib/garage:rw,async,noatime + ports: + # S3 API + - "3900" + # Admin API + - "3903" + labels: + io.quarkus.devservices.compose.config_map.port.3900: garage.s3.port + io.quarkus.devservices.compose.config_map.port.3903: garage.admin.port diff --git a/operator/src/main/docker/garage.toml b/operator/src/main/docker/garage.toml new file mode 100644 index 0000000..318c5a5 --- /dev/null +++ b/operator/src/main/docker/garage.toml @@ -0,0 +1,29 @@ +# Garage configuration used by the Compose Dev Service that backs the test suite. +# +# It mirrors what the AboutBits Garage Helm chart deploys — a single node with a replication +# factor of 1 and the Admin API enabled — so the operator is exercised against the same shape of +# cluster it manages in production. +# +# Note that no cluster layout is configured here. A freshly started Garage node has no layout and +# serves no S3 traffic until one is assigned, which is exactly the bootstrap situation the +# GarageCluster reconciler is responsible for. + +metadata_dir = "/var/lib/garage/meta" +data_dir = "/var/lib/garage/data" +db_engine = "lmdb" + +replication_factor = 1 + +rpc_bind_addr = "[::]:3901" +rpc_public_addr = "127.0.0.1:3901" +# Test-only credentials. Never reuse these anywhere else. +rpc_secret = "3e0d0b0a3d4b2a5c6f7e8d9c0b1a2f3e4d5c6b7a8f9e0d1c2b3a4f5e6d7c8b9a" + +[s3_api] +s3_region = "garage" +api_bind_addr = "[::]:3900" +root_domain = ".s3.garage" + +[admin] +api_bind_addr = "[::]:3903" +admin_token = "test-admin-token" diff --git a/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java new file mode 100644 index 0000000..a573908 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java @@ -0,0 +1,138 @@ +package it.aboutbits.garage.core; + +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.time.OffsetDateTime; +import java.time.ZoneOffset; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +@Slf4j +@NullMarked +public abstract class BaseReconciler & Named, S extends CRStatus> { + /// Bounds how long backend drift, which Kubernetes sees no events for, goes unnoticed. + public static final long RESYNC_INTERVAL_MINUTES = 10; + + protected abstract S newStatus(); + + public S initializeStatus(CR resource) { + S status = resource.getStatus(); + + //noinspection ConstantConditions + if (status == null) { + status = newStatus(); + resource.setStatus(status); + } + + status.setName(resource.getName()); + status.setLastProbeTime(OffsetDateTime.now(ZoneOffset.UTC)); + status.setObservedGeneration(resource.getMetadata().getGeneration()); + + return status; + } + + public String getResourceNamespaceOrOwn( + CR resource, + @Nullable String resourceNamespace + ) { + if (resourceNamespace != null) { + return resourceNamespace; + } + + return resource.getMetadata().getNamespace(); + } + + /// An empty result means "not yet", not "broken": callers report [CRPhase#PENDING] and retry. + public Optional getReferencedS3Connection( + KubernetesClient kubernetesClient, + CR resource, + ResourceRef connectionRef + ) { + var connectionName = connectionRef.getName(); + var connectionNamespace = getResourceNamespaceOrOwn(resource, connectionRef.getNamespace()); + + var s3Connection = kubernetesClient.resources(S3Connection.class) + .inNamespace(connectionNamespace) + .withName(connectionName) + .get(); + + //noinspection ConstantConditions + if (s3Connection == null) { + log.error( + "The specified S3Connection does not exist [resource={}/{}]", + connectionNamespace, + connectionName + ); + + return Optional.empty(); + } + + var status = s3Connection.getStatus(); + + // No status yet means the connection has not been probed yet. + //noinspection ConstantConditions + if (status == null) { + log.warn( + "The specified S3Connection has not been probed yet [resource={}/{}]", + connectionNamespace, + connectionName + ); + + return Optional.empty(); + } + + var currentPhase = status.getPhase(); + var expectedPhase = CRPhase.READY; + + if (!Objects.equals(currentPhase, expectedPhase)) { + log.warn( + "The specified S3Connection is not ready yet [resource={}/{}, status.phase={}]", + connectionNamespace, + connectionName, + currentPhase + ); + + return Optional.empty(); + } + + // READY of an older generation says nothing about the current spec. + if (status.getObservedGeneration() != s3Connection.getMetadata().getGeneration()) { + log.warn( + "The specified S3Connection changed and has not been probed again yet [resource={}/{}, status.observedGeneration={}, metadata.generation={}]", + connectionNamespace, + connectionName, + status.getObservedGeneration(), + s3Connection.getMetadata().getGeneration() + ); + + return Optional.empty(); + } + + return Optional.of(s3Connection); + } + + public UpdateControl handleError( + CR resource, + S status, + E exception + ) { + log.error( + "Failed to reconcile resource [resource={}]", + resource.getMetadata().getName(), + exception + ); + + status.updatePhase(CRPhase.ERROR) + .setMessage(exception.getMessage()); + + return UpdateControl.patchStatus(resource) + .rescheduleAfter(60, TimeUnit.SECONDS); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java b/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java new file mode 100644 index 0000000..ef35103 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java @@ -0,0 +1,138 @@ +package it.aboutbits.garage.core; + +import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.client.KubernetesClient; +import jakarta.inject.Singleton; +import org.jspecify.annotations.NullMarked; + +import java.nio.charset.Charset; +import java.util.Base64; +import java.util.Map; + +@Singleton +@NullMarked +public final class KubernetesService { + public static final String SECRET_TYPE_OPAQUE = "Opaque"; + + /// The key the AboutBits Garage Helm chart stores the Admin API token under (https://github.com/aboutbits/helm-garage). + public static final String SECRET_DATA_ADMIN_TOKEN_KEY = "admin_token"; + + public String getSecretRefToken( + KubernetesClient kubernetesClient, + SecretKeyRef secretRef, + String defaultNamespace + ) { + var secretNamespace = getSecretNamespaceOrDefault(secretRef, defaultNamespace); + var secretName = secretRef.getName(); + var secretKey = secretRef.getKey(); + + var secret = getSecret( + kubernetesClient, + secretNamespace, + secretName + ); + + requireSecretType( + secret, + SECRET_TYPE_OPAQUE, + secretNamespace, + secretName + ); + + var data = requireSecretData( + secret, + secretNamespace, + secretName + ); + + var tokenBase64 = data.get(secretKey); + if (tokenBase64 == null) { + throw new IllegalStateException("The Secret reference is missing the referenced key [secret.namespace=%s, secret.name=%s, secret.data.key=%s, available.secret.data.keys=%s]".formatted( + secretNamespace, + secretName, + secretKey, + data.keySet() + )); + } + + var token = decode(tokenBase64); + if (token.isBlank()) { + throw new IllegalStateException("The Secret reference has a blank token [secret.namespace=%s, secret.name=%s, secret.data.key=%s]".formatted( + secretNamespace, + secretName, + secretKey + )); + } + + return token; + } + + private String getSecretNamespaceOrDefault( + ResourceRef secretRef, + String defaultNamespace + ) { + return secretRef.getNamespace() != null + ? secretRef.getNamespace() + : defaultNamespace; + } + + private Secret getSecret( + KubernetesClient kubernetesClient, + String secretNamespace, + String secretName + ) { + var secret = kubernetesClient.secrets() + .inNamespace(secretNamespace) + .withName(secretName) + .get(); + + if (secret == null) { + throw new IllegalStateException("Secret reference not found [secret.namespace=%s, secret.name=%s]".formatted( + secretNamespace, + secretName + )); + } + + return secret; + } + + private void requireSecretType( + Secret secret, + String expectedType, + String secretNamespace, + String secretName + ) { + if (!expectedType.equals(secret.getType())) { + throw new IllegalArgumentException("The Secret reference is of the wrong type [secret.namespace=%s, secret.name=%s, expected.secret.type=%s, actual.secret.type=%s]".formatted( + secretNamespace, + secretName, + expectedType, + secret.getType() + )); + } + } + + private Map requireSecretData( + Secret secret, + String secretNamespace, + String secretName + ) { + var data = secret.getData(); + + if (data == null || data.isEmpty()) { + throw new IllegalStateException("The Secret reference has no data set [secret.namespace=%s, secret.name=%s]".formatted( + secretNamespace, + secretName + )); + } + + return data; + } + + private String decode(String base64Value) { + return new String( + Base64.getDecoder().decode(base64Value), + Charset.defaultCharset() + ); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java new file mode 100644 index 0000000..12aaf7e --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java @@ -0,0 +1,22 @@ +package it.aboutbits.garage.core.adminapi; + +import jakarta.ws.rs.client.ClientRequestContext; +import jakarta.ws.rs.client.ClientRequestFilter; +import jakarta.ws.rs.core.HttpHeaders; +import lombok.RequiredArgsConstructor; +import org.jspecify.annotations.NullMarked; + +/// Adds the `Authorization: Bearer ` header expected by the Garage Admin API. +@RequiredArgsConstructor +@NullMarked +public class BearerTokenFilter implements ClientRequestFilter { + private final String token; + + @Override + public void filter(ClientRequestContext requestContext) { + requestContext.getHeaders().putSingle( + HttpHeaders.AUTHORIZATION, + "Bearer %s".formatted(token) + ); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/ConnectionProbeResult.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/ConnectionProbeResult.java new file mode 100644 index 0000000..bf3bea2 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/ConnectionProbeResult.java @@ -0,0 +1,18 @@ +package it.aboutbits.garage.core.adminapi; + +import org.jspecify.annotations.NullMarked; + +/// A reachable Garage that is not ready is not an error; an unreachable one throws instead. +@NullMarked +public record ConnectionProbeResult( + boolean available, + String detail +) { + public static ConnectionProbeResult available(String detail) { + return new ConnectionProbeResult(true, detail); + } + + public static ConnectionProbeResult unavailable(String detail) { + return new ConnectionProbeResult(false, detail); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminApi.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminApi.java new file mode 100644 index 0000000..938b043 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminApi.java @@ -0,0 +1,44 @@ +package it.aboutbits.garage.core.adminapi; + +import it.aboutbits.garage.core.adminapi.dto.ApplyClusterLayoutRequest; +import it.aboutbits.garage.core.adminapi.dto.ApplyClusterLayoutResponse; +import it.aboutbits.garage.core.adminapi.dto.GetClusterHealthResponse; +import it.aboutbits.garage.core.adminapi.dto.GetClusterLayoutResponse; +import it.aboutbits.garage.core.adminapi.dto.GetClusterStatusResponse; +import it.aboutbits.garage.core.adminapi.dto.UpdateClusterLayoutRequest; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; +import org.jspecify.annotations.NullMarked; + +/// The subset of the [Garage Admin API v2](https://garagehq.deuxfleurs.fr/documentation/reference-manual/admin-api/) +/// the operator needs. Clients are built per endpoint and token by [GarageAdminClientFactory]. +@Path("/v2") +@Produces(MediaType.APPLICATION_JSON) +@Consumes(MediaType.APPLICATION_JSON) +@NullMarked +public interface GarageAdminApi { + @GET + @Path("/GetClusterStatus") + GetClusterStatusResponse getClusterStatus(); + + @GET + @Path("/GetClusterHealth") + GetClusterHealthResponse getClusterHealth(); + + @GET + @Path("/GetClusterLayout") + GetClusterLayoutResponse getClusterLayout(); + + /// Staged changes take effect only on [#applyClusterLayout]. + @POST + @Path("/UpdateClusterLayout") + GetClusterLayoutResponse updateClusterLayout(UpdateClusterLayoutRequest request); + + @POST + @Path("/ApplyClusterLayout") + ApplyClusterLayoutResponse applyClusterLayout(ApplyClusterLayoutRequest request); +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java new file mode 100644 index 0000000..eef3200 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java @@ -0,0 +1,134 @@ +package it.aboutbits.garage.core.adminapi; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.quarkus.rest.client.reactive.QuarkusRestClientBuilder; +import it.aboutbits.garage.core.KubernetesService; +import it.aboutbits.garage.core.SecretKeyRef; +import it.aboutbits.garage.crd.garagecluster.GarageCluster; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import jakarta.annotation.PreDestroy; +import jakarta.enterprise.context.ApplicationScoped; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NullMarked; + +import java.io.Closeable; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; + +/// Clients are kept per admin endpoint and token, because each holds a connection pool. Keying by +/// token too keeps two resources with different tokens for one endpoint from closing each other's +/// client mid-request. The token is read from its Secret on every call, so rotation applies at once. +@ApplicationScoped +@RequiredArgsConstructor +@Slf4j +@NullMarked +public class GarageAdminClientFactory { + private static final long CONNECT_TIMEOUT_SECONDS = 5; + private static final long READ_TIMEOUT_SECONDS = 30; + + private final KubernetesService kubernetesService; + private final KubernetesClient kubernetesClient; + + private final Map clients = new ConcurrentHashMap<>(); + + public GarageAdminApi create(GarageCluster garageCluster) { + var spec = garageCluster.getSpec(); + + return create( + spec.getAdminEndpoint(), + spec.getAdminSecretRef(), + garageCluster.getMetadata().getNamespace() + ); + } + + public GarageAdminApi create(S3Connection s3Connection) { + var spec = s3Connection.getSpec(); + + return create( + spec.getAdminEndpoint(), + spec.getAdminSecretRef(), + s3Connection.getMetadata().getNamespace() + ); + } + + public GarageAdminApi create( + String adminEndpoint, + SecretKeyRef adminSecretRef, + String defaultNamespace + ) { + var token = kubernetesService.getSecretRefToken( + kubernetesClient, + adminSecretRef, + defaultNamespace + ); + + var endpoint = parseEndpoint(adminEndpoint, adminSecretRef); + + return clients.computeIfAbsent( + new ClientKey(endpoint, token), + key -> create(key.endpoint(), key.token()) + ); + } + + @PreDestroy + void closeAll() { + clients.values().forEach(this::close); + clients.clear(); + } + + /// Uncached; the caller owns the client and has to close it. + public GarageAdminApi create( + URI adminEndpoint, + String token + ) { + return QuarkusRestClientBuilder.newBuilder() + .baseUri(adminEndpoint) + .connectTimeout(CONNECT_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .readTimeout(READ_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .register(new BearerTokenFilter(token)) + .build(GarageAdminApi.class); + } + + private void close(GarageAdminApi garageAdminApi) { + if (garageAdminApi instanceof Closeable closeable) { + try { + closeable.close(); + } catch (IOException | RuntimeException e) { + log.warn("Failed to close a Garage Admin API client", e); + } + } + } + + private URI parseEndpoint( + String adminEndpoint, + SecretKeyRef adminSecretRef + ) { + try { + return new URI(adminEndpoint); + } catch (URISyntaxException e) { + throw new IllegalArgumentException( + "The admin endpoint is not a valid URI [adminEndpoint=%s, adminSecretRef.name=%s]".formatted( + adminEndpoint, + adminSecretRef.getName() + ), + e + ); + } + } + + private record ClientKey( + URI endpoint, + String token + ) { + // Hides the token from logs and exception messages. + @Override + public String toString() { + return "ClientKey[endpoint=%s]".formatted(endpoint); + } + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageService.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageService.java new file mode 100644 index 0000000..a73c342 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageService.java @@ -0,0 +1,39 @@ +package it.aboutbits.garage.core.adminapi; + +import it.aboutbits.garage.crd.s3connection.S3Connection; +import jakarta.enterprise.context.ApplicationScoped; +import lombok.RequiredArgsConstructor; +import org.jspecify.annotations.NullMarked; + +/// Drives Garage through its Admin API v2. +@ApplicationScoped +@RequiredArgsConstructor +@NullMarked +public class GarageService { + /// Includes a node that has no cluster layout applied yet. + static final String HEALTH_STATUS_UNAVAILABLE = "unavailable"; + + private final GarageAdminClientFactory garageAdminClientFactory; + + public ConnectionProbeResult probe(S3Connection s3Connection) { + var health = garageAdminClientFactory.create(s3Connection) + .getClusterHealth(); + + var detail = "Garage cluster %s [storageNodes=%d, storageNodesUp=%d, partitionsAllOk=%d/%d]".formatted( + health.status(), + health.storageNodes(), + health.storageNodesUp(), + health.partitionsAllOk(), + health.partitions() + ); + + // `degraded` still serves reads and writes, so only `unavailable` counts as not ready. + if (HEALTH_STATUS_UNAVAILABLE.equals(health.status())) { + return ConnectionProbeResult.unavailable( + "%s. The cluster cannot serve requests — check that a GarageCluster has applied a layout.".formatted(detail) + ); + } + + return ConnectionProbeResult.available(detail); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutRequest.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutRequest.java new file mode 100644 index 0000000..dd7646c --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutRequest.java @@ -0,0 +1,10 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import org.jspecify.annotations.NullMarked; + +/// [#version] is the *new* version: the current one plus one. +@NullMarked +public record ApplyClusterLayoutRequest( + long version +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutResponse.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutResponse.java new file mode 100644 index 0000000..fd195fa --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ApplyClusterLayoutResponse.java @@ -0,0 +1,14 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; + +import java.util.List; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record ApplyClusterLayoutResponse( + List message, + GetClusterLayoutResponse layout +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterHealthResponse.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterHealthResponse.java new file mode 100644 index 0000000..09710e6 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterHealthResponse.java @@ -0,0 +1,19 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record GetClusterHealthResponse( + // One of `healthy`, `degraded` or `unavailable`. + String status, + int knownNodes, + int connectedNodes, + int storageNodes, + int storageNodesUp, + int partitions, + int partitionsQuorum, + int partitionsAllOk +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterLayoutResponse.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterLayoutResponse.java new file mode 100644 index 0000000..fa31f9e --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterLayoutResponse.java @@ -0,0 +1,15 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; + +import java.util.List; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record GetClusterLayoutResponse( + long version, + List roles, + List stagedRoleChanges +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterStatusResponse.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterStatusResponse.java new file mode 100644 index 0000000..ed0dae8 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/GetClusterStatusResponse.java @@ -0,0 +1,14 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; + +import java.util.List; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record GetClusterStatusResponse( + long layoutVersion, + List nodes +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/LayoutNodeRole.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/LayoutNodeRole.java new file mode 100644 index 0000000..247f11e --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/LayoutNodeRole.java @@ -0,0 +1,20 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.List; + +/// A node that currently has a role in the cluster layout. +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record LayoutNodeRole( + String id, + String zone, + @Nullable Long capacity, + List tags, + @Nullable Long storedPartitions, + @Nullable Long usableCapacity +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeAssignedRole.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeAssignedRole.java new file mode 100644 index 0000000..dab76b3 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeAssignedRole.java @@ -0,0 +1,19 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.List; + +/// A node's layout role. A `null` [#capacity] marks a gateway node, which stores no data. +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +@NullMarked +public record NodeAssignedRole( + String zone, + @Nullable Long capacity, + List tags +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeResp.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeResp.java new file mode 100644 index 0000000..0067095 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeResp.java @@ -0,0 +1,20 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record NodeResp( + String id, + @JsonProperty("isUp") boolean isUp, + // Part of an older layout version and still draining data. + boolean draining, + @Nullable String hostname, + @Nullable String garageVersion, + // `null` until the node is assigned a role. + @Nullable NodeAssignedRole role +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java new file mode 100644 index 0000000..32bc63f --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java @@ -0,0 +1,41 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.List; + +/// A staged role change: a union of removal and role assignment. `NON_NULL` keeps the unused half out. +@JsonInclude(JsonInclude.Include.NON_NULL) +@NullMarked +public record NodeRoleChangeRequest( + String id, + @Nullable String zone, + @Nullable Long capacity, + @Nullable List tags, + @Nullable Boolean remove +) { + public static NodeRoleChangeRequest assign( + String id, + NodeAssignedRole role + ) { + return new NodeRoleChangeRequest( + id, + role.zone(), + role.capacity(), + role.tags(), + null + ); + } + + public static NodeRoleChangeRequest remove(String id) { + return new NodeRoleChangeRequest( + id, + null, + null, + null, + true + ); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/UpdateClusterLayoutRequest.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/UpdateClusterLayoutRequest.java new file mode 100644 index 0000000..921dc6d --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/UpdateClusterLayoutRequest.java @@ -0,0 +1,13 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; +import org.jspecify.annotations.NullMarked; + +import java.util.List; + +@JsonInclude(JsonInclude.Include.NON_NULL) +@NullMarked +public record UpdateClusterLayoutRequest( + List roles +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java new file mode 100644 index 0000000..94c5432 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java @@ -0,0 +1,59 @@ +package it.aboutbits.garage.crd.garagecluster; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import io.fabric8.crd.generator.annotation.AdditionalPrinterColumn; +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; +import it.aboutbits.garage.core.Named; +import org.jspecify.annotations.NullMarked; + +/// A Garage cluster whose layout the operator manages; everything else is owned by the Helm chart. +@Version("v1") +@Group("garage.aboutbits.it") +@AdditionalPrinterColumn( + name = "Name", + jsonPath = ".status.name", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Phase", + jsonPath = ".status.phase", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Layout", + jsonPath = ".status.layoutVersion", + type = AdditionalPrinterColumn.Type.INTEGER +) +@AdditionalPrinterColumn( + name = "Health", + jsonPath = ".status.health", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Message", + jsonPath = ".status.message", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Since", + jsonPath = ".status.lastPhaseTransitionTime", + type = AdditionalPrinterColumn.Type.DATE +) +@AdditionalPrinterColumn( + name = "Age", + jsonPath = ".metadata.creationTimestamp", + type = AdditionalPrinterColumn.Type.DATE +) +@NullMarked +public class GarageCluster + extends CustomResource + implements Namespaced, Named { + @Override + @JsonIgnore + public String getName() { + return getSpec().getAdminEndpoint(); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java new file mode 100644 index 0000000..ba9c5a3 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java @@ -0,0 +1,46 @@ +package it.aboutbits.garage.crd.garagecluster; + +import io.fabric8.generator.annotation.Required; +import io.fabric8.generator.annotation.ValidationRule; +import lombok.Getter; +import lombok.Setter; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; + +/// A fresh Garage node serves no S3 traffic until it is assigned a role in a layout. +@Getter +@Setter +@NullMarked +public class GarageClusterLayoutSpec { + /// The CRD schema carries no defaults, so it has to be spelled out (e.g. `default`). + @Required + @ValidationRule( + value = "self.trim().size() > 0", + message = "The zone must not be empty." + ) + private String zone = "default"; + + /// Should match the data volume size. Garage uses it as a weight, not as a limit. + @Required + @ValidationRule( + value = "self.trim().size() > 0", + message = "The capacity must not be empty." + ) + @ValidationRule( + value = "self.matches('^[0-9]+(\\\\.[0-9]+)?(E|P|T|G|M|k|Ei|Pi|Ti|Gi|Mi|Ki)?$')", + message = "The capacity must be a Kubernetes quantity, for example '20Gi'." + ) + private String capacity = ""; + + @io.fabric8.generator.annotation.Nullable + private @Nullable List tags = new ArrayList<>(); + + public List getTags() { + return tags != null + ? tags + : List.of(); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java new file mode 100644 index 0000000..babeeef --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java @@ -0,0 +1,149 @@ +package it.aboutbits.garage.crd.garagecluster; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRules; +import io.quarkiverse.operatorsdk.annotations.RBACRule; +import it.aboutbits.garage.core.BaseReconciler; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.adminapi.GarageAdminApi; +import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NullMarked; + +import java.util.concurrent.TimeUnit; + +/// Deliberately no [io.javaoperatorsdk.operator.api.reconciler.Cleaner]: tearing down the layout would +/// trigger a rebalance and, on a single node, discard the data. +@Slf4j +@AdditionalRBACRules({ + @RBACRule( + apiGroups = {""}, + resources = {"secrets"}, + verbs = {"get", "list", "watch"} + ) +}) +@ControllerConfiguration( + maxReconciliationInterval = @MaxReconciliationInterval( + interval = BaseReconciler.RESYNC_INTERVAL_MINUTES, + timeUnit = TimeUnit.MINUTES + ) +) +@RequiredArgsConstructor +@NullMarked +public class GarageClusterReconciler + extends BaseReconciler + implements Reconciler { + private static final long BOOTSTRAP_RETRY_SECONDS = 15; + + private final GarageClusterService garageClusterService; + private final GarageAdminClientFactory garageAdminClientFactory; + + @Override + public UpdateControl reconcile( + GarageCluster resource, + Context context + ) { + var status = initializeStatus(resource); + + var namespace = resource.getMetadata().getNamespace(); + var name = resource.getMetadata().getName(); + + log.info( + "Reconciling GarageCluster [resource={}/{}, status.phase={}]", + namespace, + name, + status.getPhase() + ); + + try { + var garageAdminApi = garageAdminClientFactory.create(resource); + + var result = garageClusterService.reconcileLayout( + garageAdminApi, + resource.getSpec().getLayout() + ); + + status.setNodeId(result.nodeId()) + .setLayoutVersion(result.layoutVersion()); + + return switch (result.outcome()) { + case NO_NODES, NODE_DOWN -> awaitingCluster(resource, status, result); + case UNSUPPORTED_TOPOLOGY -> unsupportedTopology(resource, status, result); + case UP_TO_DATE, APPLIED -> ready(resource, status, result, garageAdminApi); + }; + } catch (Exception e) { + return handleError( + resource, + status, + e + ); + } + } + + @Override + protected GarageClusterStatus newStatus() { + return new GarageClusterStatus(); + } + + /// Normal right after a `helm install`, so retried quickly rather than treated as an error. + private UpdateControl awaitingCluster( + GarageCluster resource, + GarageClusterStatus status, + LayoutReconcileResult result + ) { + log.info( + "Waiting for the Garage cluster to become available [resource={}/{}, outcome={}]", + resource.getMetadata().getNamespace(), + resource.getMetadata().getName(), + result.outcome() + ); + + status.updatePhase(CRPhase.PENDING) + .setMessage(result.message()); + + return UpdateControl.patchStatus(resource) + .rescheduleAfter(BOOTSTRAP_RETRY_SECONDS, TimeUnit.SECONDS); + } + + /// An ERROR, since retrying will not fix it. + private UpdateControl unsupportedTopology( + GarageCluster resource, + GarageClusterStatus status, + LayoutReconcileResult result + ) { + log.error( + "Refusing to manage the cluster layout [resource={}/{}, message={}]", + resource.getMetadata().getNamespace(), + resource.getMetadata().getName(), + result.message() + ); + + status.updatePhase(CRPhase.ERROR) + .setMessage(result.message()); + + return UpdateControl.patchStatus(resource); + } + + private UpdateControl ready( + GarageCluster resource, + GarageClusterStatus status, + LayoutReconcileResult result, + GarageAdminApi garageAdminApi + ) { + var health = garageAdminApi.getClusterHealth(); + + status.setHealth(health.status()) + .setStorageNodes(health.storageNodes()) + .setStorageNodesUp(health.storageNodesUp()); + + status.updatePhase(CRPhase.READY) + .setMessage(result.message()); + + return UpdateControl.patchStatus(resource); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java new file mode 100644 index 0000000..838d087 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java @@ -0,0 +1,129 @@ +package it.aboutbits.garage.crd.garagecluster; + +import it.aboutbits.garage.core.Quantities; +import it.aboutbits.garage.core.adminapi.GarageAdminApi; +import it.aboutbits.garage.core.adminapi.dto.ApplyClusterLayoutRequest; +import it.aboutbits.garage.core.adminapi.dto.NodeAssignedRole; +import it.aboutbits.garage.core.adminapi.dto.NodeRoleChangeRequest; +import it.aboutbits.garage.core.adminapi.dto.UpdateClusterLayoutRequest; +import jakarta.inject.Singleton; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.HashSet; +import java.util.List; +import java.util.Objects; + +@Singleton +@Slf4j +@NullMarked +public class GarageClusterService { + /// Idempotent: when the role already matches, no write request is sent. + public LayoutReconcileResult reconcileLayout( + GarageAdminApi garageAdminApi, + GarageClusterLayoutSpec layoutSpec + ) { + var clusterStatus = garageAdminApi.getClusterStatus(); + var nodes = clusterStatus.nodes(); + + if (nodes.isEmpty()) { + return LayoutReconcileResult.noNodes(); + } + + // Only single-node clusters are assigned a layout: the Helm chart deploys replication_factor=1. + if (nodes.size() > 1) { + return LayoutReconcileResult.unsupportedTopology(nodes.size()); + } + + var node = nodes.getFirst(); + + if (!node.isUp()) { + return LayoutReconcileResult.nodeDown( + node.id(), + clusterStatus.layoutVersion() + ); + } + + var desiredRole = desiredRole(layoutSpec); + + if (rolesMatch(node.role(), desiredRole)) { + return LayoutReconcileResult.upToDate( + node.id(), + clusterStatus.layoutVersion() + ); + } + + return applyRole( + garageAdminApi, + node.id(), + desiredRole + ); + } + + NodeAssignedRole desiredRole(GarageClusterLayoutSpec layoutSpec) { + return new NodeAssignedRole( + layoutSpec.getZone(), + Quantities.toBytes(layoutSpec.getCapacity(), "layout capacity"), + List.copyOf(layoutSpec.getTags()) + ); + } + + /// Tags are compared as a set, since their order carries no meaning. + boolean rolesMatch( + @Nullable NodeAssignedRole currentRole, + NodeAssignedRole desiredRole + ) { + if (currentRole == null) { + return false; + } + + return Objects.equals(currentRole.zone(), desiredRole.zone()) + && Objects.equals(currentRole.capacity(), desiredRole.capacity()) + && new HashSet<>(currentRole.tags()).equals(new HashSet<>(desiredRole.tags())); + } + + private LayoutReconcileResult applyRole( + GarageAdminApi garageAdminApi, + String nodeId, + NodeAssignedRole desiredRole + ) { + // GetClusterLayout, not the cluster status, is authoritative for the version to apply. + var currentLayout = garageAdminApi.getClusterLayout(); + + log.info( + "Staging Garage cluster layout role [node.id={}, layout.version={}, zone={}, capacity={}]", + nodeId, + currentLayout.version(), + desiredRole.zone(), + desiredRole.capacity() + ); + + garageAdminApi.updateClusterLayout( + new UpdateClusterLayoutRequest( + List.of( + NodeRoleChangeRequest.assign(nodeId, desiredRole) + ) + ) + ); + + // Garage requires the new version to be spelled out as a safety measure. + var newVersion = currentLayout.version() + 1; + + var applied = garageAdminApi.applyClusterLayout( + new ApplyClusterLayoutRequest(newVersion) + ); + + log.info( + "Applied Garage cluster layout [node.id={}, layout.version={}]", + nodeId, + applied.layout().version() + ); + + return LayoutReconcileResult.applied( + nodeId, + applied.layout().version(), + "Applied cluster layout version %d".formatted(applied.layout().version()) + ); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java new file mode 100644 index 0000000..5af182c --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java @@ -0,0 +1,29 @@ +package it.aboutbits.garage.crd.garagecluster; + +import io.fabric8.generator.annotation.Required; +import io.fabric8.generator.annotation.ValidationRule; +import it.aboutbits.garage.core.SecretKeyRef; +import lombok.Getter; +import lombok.Setter; +import org.jspecify.annotations.NullMarked; + +@Getter +@Setter +@NullMarked +public class GarageClusterSpec { + /// The Garage Admin API (port 3903), not the S3 endpoint, e.g. `http://my-release-garage.storage.svc:3903`. + @Required + @ValidationRule( + value = "self.startsWith('http://') || self.startsWith('https://')", + message = "The admin endpoint must be an http:// or https:// URL." + ) + private String adminEndpoint = ""; + + /// The chart's `Opaque` Secret, i.e. `{name: -garage, key: admin_token}`. + /// The key has to be spelled out: the CRD schema carries no defaults. + @Required + private SecretKeyRef adminSecretRef = new SecretKeyRef(); + + @Required + private GarageClusterLayoutSpec layout = new GarageClusterLayoutSpec(); +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterStatus.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterStatus.java new file mode 100644 index 0000000..f99d484 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterStatus.java @@ -0,0 +1,26 @@ +package it.aboutbits.garage.crd.garagecluster; + +import it.aboutbits.garage.core.CRStatus; +import lombok.Getter; +import lombok.Setter; +import lombok.experimental.Accessors; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@Getter +@Setter +@Accessors(chain = true) +@NullMarked +public class GarageClusterStatus extends CRStatus { + /// `0` means no layout has been applied yet. + private long layoutVersion = 0; + + private @Nullable String nodeId = null; + + /// `healthy`, `degraded` or `unavailable`. + private @Nullable String health = null; + + private int storageNodes = 0; + + private int storageNodesUp = 0; +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java new file mode 100644 index 0000000..439d515 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java @@ -0,0 +1,78 @@ +package it.aboutbits.garage.crd.garagecluster; + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/// Mapping the outcome onto a phase and a reschedule is left to [GarageClusterReconciler]. +@NullMarked +public record LayoutReconcileResult( + Outcome outcome, + @Nullable String nodeId, + long layoutVersion, + String message +) { + public enum Outcome { + NO_NODES, + NODE_DOWN, + UNSUPPORTED_TOPOLOGY, + UP_TO_DATE, + APPLIED + } + + public static LayoutReconcileResult noNodes() { + return new LayoutReconcileResult( + Outcome.NO_NODES, + null, + 0, + "The Garage cluster reports no nodes yet" + ); + } + + public static LayoutReconcileResult nodeDown( + String nodeId, + long layoutVersion + ) { + return new LayoutReconcileResult( + Outcome.NODE_DOWN, + nodeId, + layoutVersion, + "The Garage node is not connected [node.id=%s]".formatted(nodeId) + ); + } + + public static LayoutReconcileResult unsupportedTopology(int nodeCount) { + return new LayoutReconcileResult( + Outcome.UNSUPPORTED_TOPOLOGY, + null, + 0, + "The Garage cluster reports %d nodes, but this operator only assigns layout roles for single-node clusters. Assign the layout manually, or open an issue if multi-node support is needed.".formatted( + nodeCount + ) + ); + } + + public static LayoutReconcileResult upToDate( + String nodeId, + long layoutVersion + ) { + return new LayoutReconcileResult( + Outcome.UP_TO_DATE, + nodeId, + layoutVersion, + "The cluster layout is up-to-date" + ); + } + + public static LayoutReconcileResult applied( + String nodeId, + long layoutVersion, + String message + ) { + return new LayoutReconcileResult( + Outcome.APPLIED, + nodeId, + layoutVersion, + message + ); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java new file mode 100644 index 0000000..1a9827b --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java @@ -0,0 +1,50 @@ +package it.aboutbits.garage.crd.s3connection; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import io.fabric8.crd.generator.annotation.AdditionalPrinterColumn; +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; +import it.aboutbits.garage.core.CRStatus; +import it.aboutbits.garage.core.Named; +import org.jspecify.annotations.NullMarked; + +/// The Garage Admin API that buckets, access keys and permissions are created through. +@Version("v1") +@Group("garage.aboutbits.it") +@AdditionalPrinterColumn( + name = "Name", + jsonPath = ".status.name", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Phase", + jsonPath = ".status.phase", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Message", + jsonPath = ".status.message", + type = AdditionalPrinterColumn.Type.STRING +) +@AdditionalPrinterColumn( + name = "Since", + jsonPath = ".status.lastPhaseTransitionTime", + type = AdditionalPrinterColumn.Type.DATE +) +@AdditionalPrinterColumn( + name = "Age", + jsonPath = ".metadata.creationTimestamp", + type = AdditionalPrinterColumn.Type.DATE +) +@NullMarked +public class S3Connection + extends CustomResource + implements Namespaced, Named { + @Override + @JsonIgnore + public String getName() { + return getSpec().getAdminEndpoint(); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconciler.java b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconciler.java new file mode 100644 index 0000000..695c171 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconciler.java @@ -0,0 +1,96 @@ +package it.aboutbits.garage.crd.s3connection; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRules; +import io.quarkiverse.operatorsdk.annotations.RBACRule; +import it.aboutbits.garage.core.BaseReconciler; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.CRStatus; +import it.aboutbits.garage.core.adminapi.GarageService; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NullMarked; + +import java.util.concurrent.TimeUnit; + +/// Only probes, nothing is created here. Dependent resources gate on the connection being `READY`. +@Slf4j +@AdditionalRBACRules({ + @RBACRule( + apiGroups = {""}, + resources = {"secrets"}, + verbs = {"get", "list", "watch"} + ) +}) +@ControllerConfiguration( + maxReconciliationInterval = @MaxReconciliationInterval( + interval = BaseReconciler.RESYNC_INTERVAL_MINUTES, + timeUnit = TimeUnit.MINUTES + ) +) +@RequiredArgsConstructor +@NullMarked +public class S3ConnectionReconciler + extends BaseReconciler + implements Reconciler { + private static final long UNAVAILABLE_RETRY_SECONDS = 30; + + private final GarageService garageService; + + @Override + public UpdateControl reconcile( + S3Connection resource, + Context context + ) { + var status = initializeStatus(resource); + + var namespace = resource.getMetadata().getNamespace(); + var name = resource.getMetadata().getName(); + + log.info( + "Reconciling S3Connection [resource={}/{}, status.phase={}]", + namespace, + name, + status.getPhase() + ); + + try { + var probe = garageService.probe(resource); + + if (!probe.available()) { + log.warn( + "The backend is reachable but not ready yet [resource={}/{}, detail={}]", + namespace, + name, + probe.detail() + ); + + status.updatePhase(CRPhase.PENDING) + .setMessage(probe.detail()); + + return UpdateControl.patchStatus(resource) + .rescheduleAfter(UNAVAILABLE_RETRY_SECONDS, TimeUnit.SECONDS); + } + + status.updatePhase(CRPhase.READY) + .setMessage(probe.detail()); + + return UpdateControl.patchStatus(resource); + } catch (Exception e) { + return handleError( + resource, + status, + e + ); + } + } + + @Override + protected CRStatus newStatus() { + return new CRStatus(); + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java new file mode 100644 index 0000000..d220b1f --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java @@ -0,0 +1,25 @@ +package it.aboutbits.garage.crd.s3connection; + +import io.fabric8.generator.annotation.Required; +import io.fabric8.generator.annotation.ValidationRule; +import it.aboutbits.garage.core.SecretKeyRef; +import lombok.Getter; +import lombok.Setter; +import org.jspecify.annotations.NullMarked; + +@Getter +@Setter +@NullMarked +public class S3ConnectionSpec { + /// The Garage Admin API on port 3903, not the S3 endpoint, e.g. `http://acme-garage.acme.svc:3903`. + @Required + @ValidationRule( + value = "self.startsWith('http://') || self.startsWith('https://')", + message = "The admin endpoint must be an http:// or https:// URL." + ) + private String adminEndpoint = ""; + + /// The Garage Helm chart's `Opaque` Secret `-garage`, with the key `admin_token`. + @Required + private SecretKeyRef adminSecretRef = new SecretKeyRef(); +} diff --git a/operator/src/main/resources/application-dev.yml b/operator/src/main/resources/application-dev.yml index 510348e..ba60dd5 100644 --- a/operator/src/main/resources/application-dev.yml +++ b/operator/src/main/resources/application-dev.yml @@ -2,3 +2,10 @@ quarkus: operator-sdk: activate-leader-election-for-profiles: - dev + compose: + devservices: + files: src/main/docker/compose-devservices.yml + +garage: + admin: + token: test-admin-token diff --git a/operator/src/main/resources/application-test.yml b/operator/src/main/resources/application-test.yml index c16d3b8..fd55390 100644 --- a/operator/src/main/resources/application-test.yml +++ b/operator/src/main/resources/application-test.yml @@ -2,3 +2,11 @@ quarkus: operator-sdk: activate-leader-election-for-profiles: - test + compose: + devservices: + files: src/main/docker/compose-devservices.yml + +# The admin token the Compose Dev Service's garage.toml is configured with. +garage: + admin: + token: test-admin-token diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestDataCreator.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestDataCreator.java new file mode 100644 index 0000000..088dc53 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestDataCreator.java @@ -0,0 +1,70 @@ +package it.aboutbits.garage._support.testdata.base; + +import net.datafaker.Faker; +import org.jspecify.annotations.NullMarked; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +@NullMarked +public abstract class TestDataCreator { + protected static final Faker FAKER = new Faker(); + + protected final int numberOfItems; + + protected TestDataCreator(int numberOfItems) { + this.numberOfItems = numberOfItems; + } + + public void apply() { + create(); + } + + public T returnFirst() { + return create().getFirst(); + } + + public List returnAll() { + return create(); + } + + public Set returnSet() { + return new HashSet<>(create()); + } + + protected List create() { + var result = new ArrayList(); + + for (var index = 0; index < numberOfItems; index++) { + result.add( + create(index) + ); + } + + return result; + } + + protected abstract T create(int index); + + public static String randomKubernetesNameSuffix(String name) { + var maxLength = 63; // Kubernetes hard enforces RFC-1123 + + if (name.length() > 60) { + throw new IllegalArgumentException( + "The name is too long (must be <= 60 to allow '-' + at least 2 random chars, max %d total) [name=%s, length=%d]".formatted( + maxLength, + name, + name.length() + ) + ); + } + + var separator = "-"; + var availableSpace = maxLength - name.length() - separator.length(); + var suffixLength = Math.min(7, availableSpace); + + return name + separator + FAKER.regexify("[a-z0-9]{%d}".formatted(suffixLength)); + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java new file mode 100644 index 0000000..0839710 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java @@ -0,0 +1,36 @@ +package it.aboutbits.garage._support.testdata.base; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage.crd.garagecluster.GarageCluster; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import org.jspecify.annotations.NullMarked; + +import java.util.concurrent.TimeUnit; + +import static org.awaitility.Awaitility.await; + +@NullMarked +public final class TestUtil { + public static void resetEnvironment(KubernetesClient kubernetesClient) { + // Reverse Dependency Deletion + deleteResource(kubernetesClient, S3Connection.class); + deleteResource(kubernetesClient, GarageCluster.class); + } + + public static void deleteResource( + KubernetesClient kubernetesClient, + Class resourceClass + ) { + // Async call to the Kubernetes API Server that triggers the Operator Controllers + kubernetesClient.resources(resourceClass).delete(); + + // Wait for the Operator Controller to finish the CR cleanup + await().atMost(5, TimeUnit.SECONDS) + .pollInterval(100, TimeUnit.MILLISECONDS) + .until(() -> kubernetesClient.resources(resourceClass).list().getItems().isEmpty()); + } + + private TestUtil() { + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java new file mode 100644 index 0000000..be65961 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java @@ -0,0 +1,96 @@ +package it.aboutbits.garage._support.testdata.persisted; + +import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage._support.testdata.persisted.creator.GarageClusterCreate; +import it.aboutbits.garage._support.testdata.persisted.creator.S3ConnectionCreate; +import it.aboutbits.garage._support.testdata.persisted.creator.SecretKeyRefCreate; +import jakarta.enterprise.context.ApplicationScoped; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jspecify.annotations.NullMarked; + +@ApplicationScoped +@RequiredArgsConstructor +@NullMarked +public class Given { + private final KubernetesClient kubernetesClient; + + /// Host port the Compose Dev Service published the Garage Admin API on. + @SuppressWarnings("NullAway.Init") + @ConfigProperty(name = "garage.admin.port") + Integer garageAdminPort; + + @SuppressWarnings("NullAway.Init") + @ConfigProperty(name = "garage.admin.token") + String garageAdminToken; + + public GarageConnectionDetails garageConnectionDetails() { + return new GarageConnectionDetails( + "http://localhost:%d".formatted(garageAdminPort), + garageAdminToken + ); + } + + public One one() { + return new One(this); + } + + public Many many(int numberOfItems) { + return new Many(numberOfItems, this); + } + + public class One extends Item { + One(Given given) { + super(1, given); + } + } + + public class Many extends Item { + Many(int numberOfItems, Given given) { + super(numberOfItems, given); + } + } + + @RequiredArgsConstructor(access = AccessLevel.PACKAGE) + public abstract class Item { + private final int numberOfItems; + private final Given given; + + @SuppressWarnings("unused") + public Item describedAs(String description) { + return this; + } + + public SecretKeyRefCreate secretKeyRef() { + return new SecretKeyRefCreate( + numberOfItems, + kubernetesClient + ); + } + + public GarageClusterCreate garageCluster() { + return new GarageClusterCreate( + numberOfItems, + given, + kubernetesClient, + garageConnectionDetails() + ); + } + + public S3ConnectionCreate s3Connection() { + return new S3ConnectionCreate( + numberOfItems, + given, + kubernetesClient, + garageConnectionDetails() + ); + } + } + + public record GarageConnectionDetails( + String adminEndpoint, + String adminToken + ) { + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java new file mode 100644 index 0000000..fb7f913 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java @@ -0,0 +1,180 @@ +package it.aboutbits.garage._support.testdata.persisted.creator; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage._support.testdata.base.TestDataCreator; +import it.aboutbits.garage._support.testdata.persisted.Given; +import it.aboutbits.garage.core.SecretKeyRef; +import it.aboutbits.garage.crd.garagecluster.GarageCluster; +import it.aboutbits.garage.crd.garagecluster.GarageClusterLayoutSpec; +import it.aboutbits.garage.crd.garagecluster.GarageClusterSpec; +import lombok.AccessLevel; +import lombok.Setter; +import lombok.experimental.Accessors; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.List; +import java.util.concurrent.TimeUnit; + +@Setter +@Accessors(fluent = true, chain = true) +@NullMarked +public class GarageClusterCreate extends TestDataCreator { + private final Given given; + + private final KubernetesClient kubernetesClient; + private final Given.GarageConnectionDetails garageConnectionDetails; + + private @Nullable String withNamespace; + + @Setter(AccessLevel.NONE) + private boolean withoutNamespace = false; + + private @Nullable String withName; + + private @Nullable String withAdminEndpoint; + + private @Nullable SecretKeyRef withAdminSecretRef; + + private @Nullable String withZone; + + private @Nullable String withCapacity; + + private @Nullable List withTags; + + public GarageClusterCreate( + int numberOfItems, + Given given, + KubernetesClient kubernetesClient, + Given.GarageConnectionDetails garageConnectionDetails + ) { + super(numberOfItems); + this.given = given; + this.kubernetesClient = kubernetesClient; + this.garageConnectionDetails = garageConnectionDetails; + } + + public GarageClusterCreate withoutNamespace() { + this.withoutNamespace = true; + return this; + } + + @Override + protected GarageCluster create(int index) { + var namespace = getNamespace(); + var name = getName(); + + var item = new GarageCluster(); + + item.setMetadata(new ObjectMetaBuilder() + .withName(name) + .withNamespace(namespace) + .build() + ); + + var layout = new GarageClusterLayoutSpec(); + + layout.setZone(getZone()); + layout.setCapacity(getCapacity()); + layout.setTags(getTags()); + + var spec = new GarageClusterSpec(); + + spec.setAdminEndpoint(getAdminEndpoint()); + spec.setAdminSecretRef(getAdminSecretRef()); + spec.setLayout(layout); + + item.setSpec(spec); + + kubernetesClient.resources(GarageCluster.class) + .inNamespace(namespace) + .resource(item) + .serverSideApply(); + + //noinspection ConstantConditions + return kubernetesClient.resources(GarageCluster.class) + .inNamespace(namespace) + .withName(name) + .waitUntilCondition( + garageCluster -> garageCluster != null && garageCluster.getStatus() != null, + 30, + TimeUnit.SECONDS + ); + } + + private @Nullable String getNamespace() { + if (withoutNamespace) { + return null; + } + + if (withNamespace != null) { + return withNamespace; + } + + withNamespace = kubernetesClient.getNamespace(); + + return withNamespace; + } + + private String getName() { + if (withName != null) { + return withName; + } + + withName = randomKubernetesNameSuffix("test-garage-cluster"); + + return withName; + } + + private String getAdminEndpoint() { + if (withAdminEndpoint != null) { + return withAdminEndpoint; + } + + withAdminEndpoint = garageConnectionDetails.adminEndpoint(); + + return withAdminEndpoint; + } + + private SecretKeyRef getAdminSecretRef() { + if (withAdminSecretRef != null) { + return withAdminSecretRef; + } + + return given.one() + .secretKeyRef() + .withToken(garageConnectionDetails.adminToken()) + .returnFirst(); + } + + private String getZone() { + if (withZone != null) { + return withZone; + } + + withZone = "default"; + + return withZone; + } + + private String getCapacity() { + if (withCapacity != null) { + return withCapacity; + } + + withCapacity = "20Gi"; + + return withCapacity; + } + + private List getTags() { + if (withTags != null) { + return withTags; + } + + withTags = List.of(); + + return withTags; + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java new file mode 100644 index 0000000..8e9813a --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java @@ -0,0 +1,135 @@ +package it.aboutbits.garage._support.testdata.persisted.creator; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage._support.testdata.base.TestDataCreator; +import it.aboutbits.garage._support.testdata.persisted.Given; +import it.aboutbits.garage.core.SecretKeyRef; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import it.aboutbits.garage.crd.s3connection.S3ConnectionSpec; +import lombok.AccessLevel; +import lombok.Setter; +import lombok.experimental.Accessors; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.util.concurrent.TimeUnit; + +@Setter +@Accessors(fluent = true, chain = true) +@NullMarked +public class S3ConnectionCreate extends TestDataCreator { + private final Given given; + + private final KubernetesClient kubernetesClient; + private final Given.GarageConnectionDetails garageConnectionDetails; + + private @Nullable String withNamespace; + + @Setter(AccessLevel.NONE) + private boolean withoutNamespace = false; + + private @Nullable String withName; + + private @Nullable String withAdminEndpoint; + + private @Nullable SecretKeyRef withAdminSecretRef; + + public S3ConnectionCreate( + int numberOfItems, + Given given, + KubernetesClient kubernetesClient, + Given.GarageConnectionDetails garageConnectionDetails + ) { + super(numberOfItems); + this.given = given; + this.kubernetesClient = kubernetesClient; + this.garageConnectionDetails = garageConnectionDetails; + } + + public S3ConnectionCreate withoutNamespace() { + this.withoutNamespace = true; + return this; + } + + @Override + protected S3Connection create(int index) { + var namespace = getNamespace(); + var name = getName(); + + var item = new S3Connection(); + + item.setMetadata(new ObjectMetaBuilder() + .withName(name) + .withNamespace(namespace) + .build() + ); + + var spec = new S3ConnectionSpec(); + + spec.setAdminEndpoint(getAdminEndpoint()); + spec.setAdminSecretRef(getAdminSecretRef()); + + item.setSpec(spec); + + kubernetesClient.resources(S3Connection.class) + .inNamespace(namespace) + .resource(item) + .serverSideApply(); + + //noinspection ConstantConditions + return kubernetesClient.resources(S3Connection.class) + .inNamespace(namespace) + .withName(name) + .waitUntilCondition( + s3Connection -> s3Connection != null && s3Connection.getStatus() != null, + 30, + TimeUnit.SECONDS + ); + } + + private @Nullable String getNamespace() { + if (withoutNamespace) { + return null; + } + + if (withNamespace != null) { + return withNamespace; + } + + withNamespace = kubernetesClient.getNamespace(); + + return withNamespace; + } + + private String getName() { + if (withName != null) { + return withName; + } + + withName = randomKubernetesNameSuffix("test-s3-connection"); + + return withName; + } + + private String getAdminEndpoint() { + if (withAdminEndpoint != null) { + return withAdminEndpoint; + } + + withAdminEndpoint = garageConnectionDetails.adminEndpoint(); + + return withAdminEndpoint; + } + + private SecretKeyRef getAdminSecretRef() { + if (withAdminSecretRef != null) { + return withAdminSecretRef; + } + + return given.one() + .secretKeyRef() + .withToken(garageConnectionDetails.adminToken()) + .returnFirst(); + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java new file mode 100644 index 0000000..c802655 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java @@ -0,0 +1,143 @@ +package it.aboutbits.garage._support.testdata.persisted.creator; + +import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage._support.testdata.base.TestDataCreator; +import it.aboutbits.garage.core.SecretKeyRef; +import lombok.AccessLevel; +import lombok.Setter; +import lombok.experimental.Accessors; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import static it.aboutbits.garage.core.KubernetesService.SECRET_DATA_ADMIN_TOKEN_KEY; +import static it.aboutbits.garage.core.KubernetesService.SECRET_TYPE_OPAQUE; + +/// Creates an `Opaque` Secret holding a token, plus the [SecretKeyRef] pointing at it. +@Setter +@Accessors(fluent = true, chain = true) +@NullMarked +public class SecretKeyRefCreate extends TestDataCreator { + private final KubernetesClient kubernetesClient; + + private @Nullable String withNamespace; + + @Setter(AccessLevel.NONE) + private boolean withoutNamespace = false; + + private @Nullable String withName; + + private @Nullable String withKey; + + private @Nullable String withToken; + + @Setter(AccessLevel.NONE) + private boolean withoutToken = false; + + @Setter(AccessLevel.NONE) + private boolean withoutSecret = false; + + public SecretKeyRefCreate( + int numberOfItems, + KubernetesClient kubernetesClient + ) { + super(numberOfItems); + this.kubernetesClient = kubernetesClient; + } + + public SecretKeyRefCreate withoutNamespace() { + this.withoutNamespace = true; + return this; + } + + public SecretKeyRefCreate withoutToken() { + this.withoutToken = true; + return this; + } + + /// Return a reference that points at a Secret which is never created. + public SecretKeyRefCreate withoutSecret() { + this.withoutSecret = true; + return this; + } + + @Override + protected SecretKeyRef create(int index) { + var namespace = getNamespace(); + var name = getName(); + var key = getKey(); + + if (!withoutSecret) { + var secretBuilder = new SecretBuilder() + .withNewMetadata() + .withNamespace(namespace) + .withName(name) + .endMetadata() + .withType(SECRET_TYPE_OPAQUE); + + if (!withoutToken) { + secretBuilder.addToStringData(key, getToken()); + } else { + // A Secret with data, but not the referenced key. + secretBuilder.addToStringData("unrelated", "value"); + } + + kubernetesClient.secrets() + .inNamespace(namespace) + .resource(secretBuilder.build()) + .serverSideApply(); + } + + var secretKeyRef = new SecretKeyRef(); + + secretKeyRef.setName(name); + secretKeyRef.setNamespace(namespace); + secretKeyRef.setKey(key); + + return secretKeyRef; + } + + private @Nullable String getNamespace() { + if (withoutNamespace) { + return null; + } + + if (withNamespace != null) { + return withNamespace; + } + + withNamespace = kubernetesClient.getNamespace(); + + return withNamespace; + } + + private String getName() { + if (withName != null) { + return withName; + } + + withName = randomKubernetesNameSuffix("test-admin-secret"); + + return withName; + } + + private String getKey() { + if (withKey != null) { + return withKey; + } + + withKey = SECRET_DATA_ADMIN_TOKEN_KEY; + + return withKey; + } + + private String getToken() { + if (withToken != null) { + return withToken; + } + + withToken = FAKER.regexify("[a-z0-9]{32}"); + + return withToken; + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java b/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java new file mode 100644 index 0000000..da035f8 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java @@ -0,0 +1,66 @@ +package it.aboutbits.garage.core.adminapi; + +import it.aboutbits.garage.core.adminapi.dto.GetClusterHealthResponse; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import org.jspecify.annotations.NullMarked; +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 org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/// The cluster states a shared Dev Service cannot be driven into reliably — an unconfigured +/// cluster, in particular, since other tests apply a layout to it — are pinned down here instead. +@NullMarked +class GarageServiceTest { + private final GarageAdminApi garageAdminApi = mock(GarageAdminApi.class); + private final GarageAdminClientFactory garageAdminClientFactory = mock(GarageAdminClientFactory.class); + + private final GarageService garageService = new GarageService(garageAdminClientFactory); + + private void givenClusterHealth(String status) { + when(garageAdminApi.getClusterHealth()).thenReturn(new GetClusterHealthResponse( + status, + 1, + 1, + 1, + 1, + 256, + 256, + 256 + )); + + when(garageAdminClientFactory.create(any(S3Connection.class))).thenReturn(garageAdminApi); + } + + @Nested + class Probe { + @ParameterizedTest + @ValueSource(strings = {"healthy", "degraded"}) + @DisplayName("A cluster that can serve requests should be available") + void servingCluster_isAvailable(String status) { + givenClusterHealth(status); + + var result = garageService.probe(new S3Connection()); + + assertThat(result.available()).isTrue(); + assertThat(result.detail()).contains(status); + } + + @Test + @DisplayName("An unavailable cluster should not be available, and should point at the layout") + void unavailableCluster_isNotAvailable() { + givenClusterHealth("unavailable"); + + var result = garageService.probe(new S3Connection()); + + assertThat(result.available()).isFalse(); + assertThat(result.detail()).contains("GarageCluster"); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java new file mode 100644 index 0000000..7aa9466 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java @@ -0,0 +1,254 @@ +package it.aboutbits.garage.crd.garagecluster; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.quarkus.test.junit.QuarkusTest; +import it.aboutbits.garage._support.testdata.base.TestUtil; +import it.aboutbits.garage._support.testdata.persisted.Given; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.adminapi.GarageAdminApi; +import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; +import jakarta.inject.Inject; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jspecify.annotations.NullMarked; +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.net.URI; +import java.util.Objects; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/// Exercises the reconciler against a real, unconfigured Garage node provided by the Compose Dev +/// Service. +/// +/// The Garage container is shared by every test in this class and, unlike the Kubernetes +/// resources, its layout cannot be rolled back between tests. Assertions are therefore written +/// against *relative* changes — did the layout version advance, does the node's role now match the +/// spec — rather than against absolute version numbers. +@QuarkusTest +@NullMarked +class GarageClusterReconcilerTest { + private static final long AWAIT_SECONDS = 60; + + @Inject + @SuppressWarnings("NullAway.Init") + KubernetesClient kubernetesClient; + + @Inject + @SuppressWarnings("NullAway.Init") + Given given; + + @Inject + @SuppressWarnings("NullAway.Init") + GarageAdminClientFactory garageAdminClientFactory; + + @ConfigProperty(name = "garage.admin.port") + @SuppressWarnings("NullAway.Init") + Integer garageAdminPort; + + @ConfigProperty(name = "garage.admin.token") + @SuppressWarnings("NullAway.Init") + String garageAdminToken; + + private GarageAdminApi garageAdminApi() { + return garageAdminClientFactory.create( + URI.create("http://localhost:%d".formatted(garageAdminPort)), + garageAdminToken + ); + } + + @BeforeEach + void setUp() { + // The Compose Dev Service starts the container, but Garage needs a moment before the + // Admin API answers. + await().atMost(AWAIT_SECONDS, TimeUnit.SECONDS) + .pollInterval(500, TimeUnit.MILLISECONDS) + .ignoreExceptions() + .until(() -> garageAdminApi().getClusterStatus().nodes().size() == 1); + + TestUtil.resetEnvironment(kubernetesClient); + } + + private GarageCluster awaitPhase( + GarageCluster garageCluster, + CRPhase expectedPhase + ) { + //noinspection ConstantConditions + return kubernetesClient.resources(GarageCluster.class) + .inNamespace(garageCluster.getMetadata().getNamespace()) + .withName(garageCluster.getMetadata().getName()) + .waitUntilCondition( + item -> item != null + && item.getStatus() != null + && item.getStatus().getPhase() == expectedPhase, + AWAIT_SECONDS, + TimeUnit.SECONDS + ); + } + + @Nested + class Bootstrap { + @Test + @DisplayName("A GarageCluster should assign and apply a layout for the node") + void garageCluster_appliesLayout() { + var garageCluster = given.one() + .garageCluster() + .withZone("default") + .withCapacity("20Gi") + .returnFirst(); + + var reconciled = awaitPhase(garageCluster, CRPhase.READY); + + var status = reconciled.getStatus(); + + assertThat(status.getLayoutVersion()).isPositive(); + assertThat(status.getNodeId()).isNotBlank(); + assertThat(status.getHealth()).isNotBlank(); + assertThat(status.getStorageNodes()).isEqualTo(1); + + // Verify against Garage itself rather than trusting the status we just wrote. + var node = garageAdminApi().getClusterStatus().nodes().getFirst(); + + assertThat(node.id()).isEqualTo(status.getNodeId()); + assertThat(node.role()).isNotNull(); + + var role = Objects.requireNonNull(node.role()); + + assertThat(role.zone()).isEqualTo("default"); + assertThat(role.capacity()).isEqualTo(21474836480L); + } + + @Test + @DisplayName("A GarageCluster whose layout already matches should not create a new layout version") + void matchingLayout_isNotReapplied() { + var first = given.one() + .garageCluster() + .withZone("default") + .withCapacity("20Gi") + .returnFirst(); + + awaitPhase(first, CRPhase.READY); + + var layoutVersion = garageAdminApi().getClusterLayout().version(); + + // A second resource describing the same desired state must be a no-op against Garage. + var second = given.one() + .garageCluster() + .withZone("default") + .withCapacity("20Gi") + .returnFirst(); + + var reconciled = awaitPhase(second, CRPhase.READY); + + assertThat(reconciled.getStatus().getLayoutVersion()).isEqualTo(layoutVersion); + assertThat(garageAdminApi().getClusterLayout().version()).isEqualTo(layoutVersion); + } + + @Test + @DisplayName("Changing the capacity should apply a new layout version") + void changedCapacity_appliesNewLayoutVersion() { + var garageCluster = given.one() + .garageCluster() + .withZone("default") + .withCapacity("20Gi") + .returnFirst(); + + awaitPhase(garageCluster, CRPhase.READY); + + var layoutVersion = garageAdminApi().getClusterLayout().version(); + + var changed = given.one() + .garageCluster() + .withZone("default") + .withCapacity("30Gi") + .returnFirst(); + + var reconciled = awaitPhase(changed, CRPhase.READY); + + assertThat(reconciled.getStatus().getLayoutVersion()).isGreaterThan(layoutVersion); + + var node = garageAdminApi().getClusterStatus().nodes().getFirst(); + + assertThat(node.role()).isNotNull(); + + var role = Objects.requireNonNull(node.role()); + + assertThat(role.capacity()).isEqualTo(32212254720L); + } + } + + @Nested + class Failures { + @Test + @DisplayName("An invalid admin token should surface as an error") + void invalidToken_setsErrorPhase() { + var adminSecretRef = given.one() + .secretKeyRef() + .withToken("definitely-not-the-admin-token") + .returnFirst(); + + var garageCluster = given.one() + .garageCluster() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + } + + @Test + @DisplayName("A missing admin Secret should surface as an error") + void missingSecret_setsErrorPhase() { + var adminSecretRef = given.one() + .secretKeyRef() + .withoutSecret() + .returnFirst(); + + var garageCluster = given.one() + .garageCluster() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); + } + + @Test + @DisplayName("An admin Secret without the referenced key should surface as an error") + void missingSecretKey_setsErrorPhase() { + var adminSecretRef = given.one() + .secretKeyRef() + .withoutToken() + .returnFirst(); + + var garageCluster = given.one() + .garageCluster() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).contains("missing the referenced key"); + } + + @Test + @DisplayName("An unreachable admin endpoint should surface as an error") + void unreachableEndpoint_setsErrorPhase() { + var garageCluster = given.one() + .garageCluster() + // Reserved port that nothing listens on. + .withAdminEndpoint("http://localhost:1") + .returnFirst(); + + var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java new file mode 100644 index 0000000..d6983af --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java @@ -0,0 +1,136 @@ +package it.aboutbits.garage.crd.garagecluster; + +import it.aboutbits.garage.core.adminapi.dto.NodeAssignedRole; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class GarageClusterServiceTest { + private final GarageClusterService garageClusterService = new GarageClusterService(); + + @Nested + class DesiredRole { + @Test + @DisplayName("The spec should be translated into the role Garage records") + void spec_isTranslatedToRole() { + var layoutSpec = new GarageClusterLayoutSpec(); + + layoutSpec.setZone("dc1"); + layoutSpec.setCapacity("20Gi"); + layoutSpec.setTags(List.of("ssd")); + + var role = garageClusterService.desiredRole(layoutSpec); + + assertThat(role.zone()).isEqualTo("dc1"); + assertThat(role.capacity()).isEqualTo(21474836480L); + assertThat(role.tags()).containsExactly("ssd"); + } + + @Test + @DisplayName("Tags set to an explicit null should mean no tags") + void nullTags_meanNoTags() { + var layoutSpec = new GarageClusterLayoutSpec(); + + layoutSpec.setZone("dc1"); + layoutSpec.setCapacity("20Gi"); + layoutSpec.setTags(null); + + var role = garageClusterService.desiredRole(layoutSpec); + + assertThat(role.tags()).isEmpty(); + } + } + + @Nested + class RolesMatch { + private final NodeAssignedRole desiredRole = new NodeAssignedRole( + "default", + 21474836480L, + List.of("ssd", "fast") + ); + + @Test + @DisplayName("A node without a role should never match") + void noRole_doesNotMatch() { + assertThat(garageClusterService.rolesMatch(null, desiredRole)).isFalse(); + } + + @Test + @DisplayName("An identical role should match") + void identicalRole_matches() { + var currentRole = new NodeAssignedRole( + "default", + 21474836480L, + List.of("ssd", "fast") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isTrue(); + } + + @Test + @DisplayName("Tag order should not count as a difference") + void reorderedTags_match() { + var currentRole = new NodeAssignedRole( + "default", + 21474836480L, + List.of("fast", "ssd") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isTrue(); + } + + @Test + @DisplayName("A different zone should count as a difference") + void differentZone_doesNotMatch() { + var currentRole = new NodeAssignedRole( + "dc2", + 21474836480L, + List.of("ssd", "fast") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + } + + @Test + @DisplayName("A different capacity should count as a difference") + void differentCapacity_doesNotMatch() { + var currentRole = new NodeAssignedRole( + "default", + 1073741824L, + List.of("ssd", "fast") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + } + + @Test + @DisplayName("A gateway node should not match a node with a capacity") + void gatewayNode_doesNotMatch() { + var currentRole = new NodeAssignedRole( + "default", + null, + List.of("ssd", "fast") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + } + + @Test + @DisplayName("Different tags should count as a difference") + void differentTags_doNotMatch() { + var currentRole = new NodeAssignedRole( + "default", + 21474836480L, + List.of("ssd") + ); + + assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java new file mode 100644 index 0000000..0c45c55 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java @@ -0,0 +1,177 @@ +package it.aboutbits.garage.crd.s3connection; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.quarkus.test.junit.QuarkusTest; +import it.aboutbits.garage._support.testdata.base.TestUtil; +import it.aboutbits.garage._support.testdata.persisted.Given; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.adminapi.GarageAdminApi; +import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; +import it.aboutbits.garage.crd.garagecluster.GarageCluster; +import jakarta.inject.Inject; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jspecify.annotations.NullMarked; +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.net.URI; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/// Exercises the connection probe against the real Garage node provided by the Compose Dev +/// Service. +/// +/// Every test that expects a `READY` connection first brings the cluster up through a +/// `GarageCluster`, so it does not depend on a layout another test happened to apply. +@QuarkusTest +@NullMarked +class S3ConnectionReconcilerTest { + private static final long AWAIT_SECONDS = 60; + + @Inject + @SuppressWarnings("NullAway.Init") + KubernetesClient kubernetesClient; + + @Inject + @SuppressWarnings("NullAway.Init") + Given given; + + @Inject + @SuppressWarnings("NullAway.Init") + GarageAdminClientFactory garageAdminClientFactory; + + @ConfigProperty(name = "garage.admin.port") + @SuppressWarnings("NullAway.Init") + Integer garageAdminPort; + + @ConfigProperty(name = "garage.admin.token") + @SuppressWarnings("NullAway.Init") + String garageAdminToken; + + private GarageAdminApi garageAdminApi() { + return garageAdminClientFactory.create( + URI.create("http://localhost:%d".formatted(garageAdminPort)), + garageAdminToken + ); + } + + @BeforeEach + void setUp() { + await().atMost(AWAIT_SECONDS, TimeUnit.SECONDS) + .pollInterval(500, TimeUnit.MILLISECONDS) + .ignoreExceptions() + .until(() -> garageAdminApi().getClusterStatus().nodes().size() == 1); + + TestUtil.resetEnvironment(kubernetesClient); + } + + private S3Connection awaitPhase( + S3Connection s3Connection, + CRPhase expectedPhase + ) { + //noinspection ConstantConditions + return kubernetesClient.resources(S3Connection.class) + .inNamespace(s3Connection.getMetadata().getNamespace()) + .withName(s3Connection.getMetadata().getName()) + .waitUntilCondition( + item -> item != null + && item.getStatus() != null + && item.getStatus().getPhase() == expectedPhase, + AWAIT_SECONDS, + TimeUnit.SECONDS + ); + } + + /// Make sure the cluster has a layout, so that it can actually serve requests. + private void givenAppliedLayout() { + var garageCluster = given.one() + .garageCluster() + .returnFirst(); + + //noinspection ConstantConditions + kubernetesClient.resources(GarageCluster.class) + .inNamespace(garageCluster.getMetadata().getNamespace()) + .withName(garageCluster.getMetadata().getName()) + .waitUntilCondition( + item -> item != null + && item.getStatus() != null + && item.getStatus().getPhase() == CRPhase.READY, + AWAIT_SECONDS, + TimeUnit.SECONDS + ); + } + + @Nested + class Probe { + @Test + @DisplayName("A connection to a serving cluster should become ready") + void servingCluster_becomesReady() { + givenAppliedLayout(); + + var s3Connection = given.one() + .s3Connection() + .returnFirst(); + + var reconciled = awaitPhase(s3Connection, CRPhase.READY); + + assertThat(reconciled.getStatus().getMessage()).contains("Garage cluster"); + } + } + + @Nested + class Failures { + @Test + @DisplayName("An invalid admin token should surface as an error") + void invalidToken_setsErrorPhase() { + var adminSecretRef = given.one() + .secretKeyRef() + .withToken("definitely-not-the-admin-token") + .returnFirst(); + + var s3Connection = given.one() + .s3Connection() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + } + + @Test + @DisplayName("A missing admin Secret should surface as an error") + void missingSecret_setsErrorPhase() { + var adminSecretRef = given.one() + .secretKeyRef() + .withoutSecret() + .returnFirst(); + + var s3Connection = given.one() + .s3Connection() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); + } + + @Test + @DisplayName("An unreachable admin endpoint should surface as an error") + void unreachableEndpoint_setsErrorPhase() { + var s3Connection = given.one() + .s3Connection() + // Reserved port that nothing listens on. + .withAdminEndpoint("http://localhost:1") + .returnFirst(); + + var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java b/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java index 5ee747d..12d5fa2 100644 --- a/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java +++ b/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java @@ -50,7 +50,10 @@ class HelmTest { /// Singular name (used for the reconciler's role binding) to plural name (used for the CRD /// file). Spelled out rather than derived, because `bucketaccess` → `bucketaccesses` is not a /// plain `+ "s"`. - private static final Map CRD_NAMES = Map.of(); + private static final Map CRD_NAMES = Map.of( + "garagecluster", "garageclusters", + "s3connection", "s3connections" + ); private final String chartName; /// Dekorate uses this value for the Deployment name and for the container name. @@ -160,12 +163,17 @@ void helmInstall_createsDeployment() throws IOException { } // ./templates/ + assertThat(chartPath.resolve("templates/clusterrole.yaml")).exists(); + assertThat(chartPath.resolve("templates/clusterrolebinding.yaml")).exists(); assertThat(chartPath.resolve("templates/deployment.yaml")).exists(); assertThat(chartPath.resolve("templates/rolebinding.yaml")).exists(); assertThat(chartPath.resolve("templates/service.yaml")).exists(); assertThat(chartPath.resolve("templates/serviceaccount.yaml")).exists(); // `quarkus.operator-sdk.crd.validate: false`: its ClusterRole has a fixed name that another QOSDK chart also uses. assertThat(chartPath.resolve("templates/validating-clusterrolebinding.yaml")).doesNotExist(); + assertThat(chartPath.resolve("templates/clusterrole.yaml")) + .content() + .doesNotContain("josdk-crd-validating-cluster-role"); // The indent of each expression has to match the depth of its field in the Deployment. // A wrong `nindent` produces invalid YAML as soon as a user sets the value. From 66ff039afdd16b12116748287dfb465a7d5d003c Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Wed, 14 Oct 2026 08:33:28 +0200 Subject: [PATCH 2/6] address the review of the GarageCluster and S3Connection resources The CEL quantity library has no `sign()` overload on Kubernetes 1.34, so the capacity rule compares against `quantity('0')` instead. Assisted-by: Claude:claude-opus-5-5 --- docs/garage-cluster.md | 42 +++- docs/s3-connection.md | 24 +- .../src/main/docker/compose-devservices.yml | 2 +- .../it/aboutbits/garage/core/Constants.java | 12 + .../garage/core/KubernetesService.java | 26 +- .../core/adminapi/BearerTokenFilter.java | 2 +- .../adminapi/GarageAdminClientFactory.java | 1 + .../core/adminapi/GarageErrorMapper.java | 54 +++++ .../core/adminapi/dto/ErrorResponse.java | 13 + .../crd/garagecluster/GarageCluster.java | 5 +- .../GarageClusterLayoutSpec.java | 8 +- .../GarageClusterReconciler.java | 10 +- .../garagecluster/GarageClusterService.java | 50 +++- .../crd/garagecluster/GarageClusterSpec.java | 4 +- .../garagecluster/LayoutReconcileResult.java | 13 +- .../garage/crd/s3connection/S3Connection.java | 5 +- .../crd/s3connection/S3ConnectionSpec.java | 9 +- operator/src/main/resources/application.yml | 4 + .../_support/testdata/persisted/Given.java | 6 +- .../persisted/creator/SecretKeyRefCreate.java | 4 +- .../garage/core/BaseReconcilerTest.java | 166 +++++++++++++ .../core/adminapi/GarageServiceTest.java | 6 + .../GarageClusterReconcilerTest.java | 198 ++++++++++++--- .../GarageClusterServiceTest.java | 229 +++++++++++++++++- .../S3ConnectionReconcilerTest.java | 138 +++++++++-- .../it/aboutbits/garage/helm/HelmTest.java | 4 +- 26 files changed, 899 insertions(+), 136 deletions(-) create mode 100644 operator/src/main/java/it/aboutbits/garage/core/Constants.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageErrorMapper.java create mode 100644 operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ErrorResponse.java create mode 100644 operator/src/test/java/it/aboutbits/garage/core/BaseReconcilerTest.java diff --git a/docs/garage-cluster.md b/docs/garage-cluster.md index 5322c33..52bbf3c 100644 --- a/docs/garage-cluster.md +++ b/docs/garage-cluster.md @@ -15,6 +15,8 @@ Once the layout is applied, an [`S3Connection`](s3-connection.md) pointed at the This version manages **single-node clusters**, which is what the Helm chart deploys (`replication_factor = 1`, no clustering or node discovery). If the cluster reports more than one node, the operator refuses to touch the layout and reports `ERROR` rather than guessing a zone and capacity per node. +Only one `GarageCluster` manages a Garage node. A second `GarageCluster` for the same node, in any namespace, reports `ERROR` and leaves the layout alone. Otherwise two resources with different layouts would apply their roles in turn, a new layout version at every reconcile. The node is recognized by its id, so two spellings of the same `adminEndpoint` count as the same Garage. When the first `GarageCluster` is deleted, the second one takes over at its next retry. + Deleting a `GarageCluster` removes the operator's record of the cluster. It does **not** tear the storage cluster down: removing a node's role would trigger a rebalance and, on a single-node cluster, discard the data. There is therefore no reclaim policy on this resource. ## Example @@ -23,8 +25,8 @@ Deleting a `GarageCluster` removes the operator's record of the cluster. It does apiVersion: garage.aboutbits.it/v1 kind: GarageCluster metadata: - name: acme namespace: acme + name: acme spec: adminEndpoint: http://acme-garage.acme.svc:3903 adminSecretRef: @@ -39,18 +41,34 @@ spec: ## Spec -| Field | Required | Description | -|----------------------------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `adminEndpoint` | yes | Base URL of the Garage Admin API. The Helm chart exposes it on port 3903 as a ClusterIP-only Service, reachable from inside the cluster only. | -| `adminSecretRef.name` | yes | Name of the `Opaque` Secret holding the Admin API bearer token. The chart creates one named after the release. | -| `adminSecretRef.namespace` | no | Defaults to the namespace of the `GarageCluster`. | -| `adminSecretRef.key` | yes | Key within the Secret. The chart stores the token under `admin_token`. | -| `layout.zone` | yes | Zone the node is placed in. With `replication_factor = 1` this is effectively a label. | -| `layout.capacity` | yes | Storage capacity to advertise, as a Kubernetes quantity such as `20Gi`. Should match the size of the chart's data volume. Garage uses it to weight partition distribution; it is not enforced as a limit. | -| `layout.tags` | no | Tags to attach to the node. Informational; order is not significant. | +| Field | Type | Description | Required | Mutable | +|------------------|--------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------|----------|---------| +| `adminEndpoint` | `string` | Base URL of the Garage Admin API. The Helm chart exposes it on port 3903 as a ClusterIP-only Service, reachable from inside the cluster only. | Yes | Yes | +| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the `Opaque` Secret holding the Admin API bearer token. The chart creates one named after the release. | Yes | Yes | +| `layout` | [`GarageClusterLayoutSpec`](#garageclusterlayoutspec-layout) | The role of the node in the cluster layout. | Yes | Yes | + +### SecretKeyRef (`adminSecretRef`) + +| Field | Type | Description | Required | +|-------------|----------|---------------------------------------------------------------------------------------|----------| +| `namespace` | `string` | Namespace of the Secret. If not specified, uses the namespace of the `GarageCluster`. | No | +| `name` | `string` | Name of the Secret. | Yes | +| `key` | `string` | Key within the Secret. The chart stores the token under `admin_token`. | Yes | + +### GarageClusterLayoutSpec (`layout`) + +| Field | Type | Description | Required | +|------------|-----------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------| +| `zone` | `string` | Zone the node is placed in. With `replication_factor = 1` this is effectively a label. | Yes | +| `capacity` | `string` | Storage capacity to advertise, as a Kubernetes quantity such as `20Gi`. Should match the size of the chart's data volume. Garage uses it to weight partition distribution; it is not enforced as a limit. | Yes | +| `tags` | `array[string]` | Tags to attach to the node. Informational; order is not significant. | No | + +A change to a `Mutable` field is applied at the next reconcile. A change to `layout` applies a new layout version. On a single node no data moves. A role that you change by hand with the Garage CLI is reset at the next reconcile, at most 10 minutes later. Note that the CRD schema carries no defaults — every required field has to be spelled out in the manifest. +The token needs access to these Admin API endpoints: `GetClusterStatus`, `GetClusterHealth`, `GetClusterLayout`, `UpdateClusterLayout` and `ApplyClusterLayout`. The token of the Helm chart has access to all endpoints. A Garage admin token with a smaller scope must list the endpoints that it allows. + ## Status | Field | Description | @@ -67,8 +85,8 @@ Note that the CRD schema carries no defaults — every required field has to be ## How reconciliation works 1. Read `GetClusterStatus`. No nodes, or the node not connected, means Garage is still coming up → - `PENDING`, retry. -2. Compare the node's assigned role against the spec. Tags are compared as a set, and capacity is compared in bytes, so a reordered tag list or `20Gi` versus `20Gi` is not a change. + `PENDING`, retry. A node that another `GarageCluster` already manages → `ERROR`. +2. Compare the node's assigned role against the spec. Tags are compared as a set, and capacity is compared in bytes, so a reordered tag list or `20Gi` versus `21474836480` is not a change. 3. If they match, stop — nothing is written to Garage. 4. Otherwise read `GetClusterLayout` for the authoritative version number, stage the role with `UpdateClusterLayout`, then commit it with `ApplyClusterLayout` at version + 1. diff --git a/docs/s3-connection.md b/docs/s3-connection.md index 6ccc66d..d212d26 100644 --- a/docs/s3-connection.md +++ b/docs/s3-connection.md @@ -24,8 +24,8 @@ Both resources reference the same admin endpoint and Secret. That duplication is apiVersion: garage.aboutbits.it/v1 kind: S3Connection metadata: - name: acme namespace: acme + name: acme spec: adminEndpoint: http://acme-garage.acme.svc:3903 adminSecretRef: @@ -35,12 +35,22 @@ spec: ## Spec -| Field | Required | Description | -|----------------------------|----------|-----------------------------------------------------------------------------------------------------------| -| `adminEndpoint` | yes | Base URL of the Garage **Admin** API, port 3903. This is not the S3 endpoint clients use. | -| `adminSecretRef.name` | yes | Name of the Secret holding the admin credentials. | -| `adminSecretRef.namespace` | no | Defaults to the namespace of the `S3Connection`. | -| `adminSecretRef.key` | yes | Key within the Secret. The AboutBits Garage chart uses `admin_token`. | +| Field | Type | Description | Required | Mutable | +|------------------|------------------------------------------------|-------------------------------------------------------------------------------------------|----------|---------| +| `adminEndpoint` | `string` | Base URL of the Garage **Admin** API, port 3903. This is not the S3 endpoint clients use. | Yes | No | +| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the Secret holding the admin credentials. | Yes | Yes | + +### SecretKeyRef (`adminSecretRef`) + +| Field | Type | Description | Required | +|-------------|----------|--------------------------------------------------------------------------------------|----------| +| `namespace` | `string` | Namespace of the Secret. If not specified, uses the namespace of the `S3Connection`. | No | +| `name` | `string` | Name of the Secret. | Yes | +| `key` | `string` | Key within the Secret. The AboutBits Garage chart uses `admin_token`. | Yes | + +`adminEndpoint` is immutable, because buckets and access keys record the ids that they get from this Garage. A changed endpoint would create second ones on another Garage. Delete and recreate the `S3Connection` to point it at a different Garage. `adminSecretRef` can change, for example to rotate the token. + +The token needs access to the Admin API endpoint `GetClusterHealth`. The token of the Helm chart has access to all endpoints. A Garage admin token with a smaller scope must list the endpoints that it allows. ## Status diff --git a/operator/src/main/docker/compose-devservices.yml b/operator/src/main/docker/compose-devservices.yml index a6b2235..c03dbbe 100644 --- a/operator/src/main/docker/compose-devservices.yml +++ b/operator/src/main/docker/compose-devservices.yml @@ -2,7 +2,7 @@ services: garage: # Keep this in sync with the image the AboutBits Garage Helm chart deploys. # https://github.com/aboutbits/helm-garage - image: dxflrs/garage:v2.3.0 + image: dxflrs/garage:v2.4.1 volumes: - ./garage.toml:/etc/garage.toml:ro tmpfs: diff --git a/operator/src/main/java/it/aboutbits/garage/core/Constants.java b/operator/src/main/java/it/aboutbits/garage/core/Constants.java new file mode 100644 index 0000000..d5272aa --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/Constants.java @@ -0,0 +1,12 @@ +package it.aboutbits.garage.core; + +import org.jspecify.annotations.NullMarked; + +@NullMarked +public final class Constants { + public static final String CRD_GROUP = "garage.aboutbits.it"; + public static final String CRD_VERSION = "v1"; + + private Constants() { + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java b/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java index ef35103..c9a7076 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java +++ b/operator/src/main/java/it/aboutbits/garage/core/KubernetesService.java @@ -14,9 +14,6 @@ public final class KubernetesService { public static final String SECRET_TYPE_OPAQUE = "Opaque"; - /// The key the AboutBits Garage Helm chart stores the Admin API token under (https://github.com/aboutbits/helm-garage). - public static final String SECRET_DATA_ADMIN_TOKEN_KEY = "admin_token"; - public String getSecretRefToken( KubernetesClient kubernetesClient, SecretKeyRef secretRef, @@ -47,17 +44,20 @@ public String getSecretRefToken( var tokenBase64 = data.get(secretKey); if (tokenBase64 == null) { - throw new IllegalStateException("The Secret reference is missing the referenced key [secret.namespace=%s, secret.name=%s, secret.data.key=%s, available.secret.data.keys=%s]".formatted( + var availableKeys = data.keySet(); + + throw new IllegalStateException("The Secret reference is missing the referenced key [secret=%s/%s, secret.key=%s, availableKeys=%s]".formatted( secretNamespace, secretName, secretKey, - data.keySet() + availableKeys )); } - var token = decode(tokenBase64); + // A token written with `echo "$TOKEN" > file` ends with a newline, which is invalid in an HTTP header. + var token = decode(tokenBase64).strip(); if (token.isBlank()) { - throw new IllegalStateException("The Secret reference has a blank token [secret.namespace=%s, secret.name=%s, secret.data.key=%s]".formatted( + throw new IllegalStateException("The Secret reference has a blank token [secret=%s/%s, secret.key=%s]".formatted( secretNamespace, secretName, secretKey @@ -87,7 +87,7 @@ private Secret getSecret( .get(); if (secret == null) { - throw new IllegalStateException("Secret reference not found [secret.namespace=%s, secret.name=%s]".formatted( + throw new IllegalStateException("Secret reference not found [secret=%s/%s]".formatted( secretNamespace, secretName )); @@ -102,12 +102,14 @@ private void requireSecretType( String secretNamespace, String secretName ) { - if (!expectedType.equals(secret.getType())) { - throw new IllegalArgumentException("The Secret reference is of the wrong type [secret.namespace=%s, secret.name=%s, expected.secret.type=%s, actual.secret.type=%s]".formatted( + var actualType = secret.getType(); + + if (!expectedType.equals(actualType)) { + throw new IllegalArgumentException("The Secret reference is of the wrong type [secret=%s/%s, expectedType=%s, actualType=%s]".formatted( secretNamespace, secretName, expectedType, - secret.getType() + actualType )); } } @@ -120,7 +122,7 @@ private Map requireSecretData( var data = secret.getData(); if (data == null || data.isEmpty()) { - throw new IllegalStateException("The Secret reference has no data set [secret.namespace=%s, secret.name=%s]".formatted( + throw new IllegalStateException("The Secret reference has no data set [secret=%s/%s]".formatted( secretNamespace, secretName )); diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java index 12aaf7e..5f5cfc7 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/BearerTokenFilter.java @@ -9,7 +9,7 @@ /// Adds the `Authorization: Bearer ` header expected by the Garage Admin API. @RequiredArgsConstructor @NullMarked -public class BearerTokenFilter implements ClientRequestFilter { +class BearerTokenFilter implements ClientRequestFilter { private final String token; @Override diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java index eef3200..4520d8e 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java @@ -91,6 +91,7 @@ public GarageAdminApi create( .connectTimeout(CONNECT_TIMEOUT_SECONDS, TimeUnit.SECONDS) .readTimeout(READ_TIMEOUT_SECONDS, TimeUnit.SECONDS) .register(new BearerTokenFilter(token)) + .register(new GarageErrorMapper()) .build(GarageAdminApi.class); } diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageErrorMapper.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageErrorMapper.java new file mode 100644 index 0000000..d01d129 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageErrorMapper.java @@ -0,0 +1,54 @@ +package it.aboutbits.garage.core.adminapi; + +import it.aboutbits.garage.core.adminapi.dto.ErrorResponse; +import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.core.Response; +import org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/// The default mapper keeps only the reason phrase, but Garage explains the error in a JSON body with `code` and +/// `message`. Still a [WebApplicationException], so that callers can handle a status such as `409`. +@NullMarked +class GarageErrorMapper implements ResponseExceptionMapper { + @Override + public WebApplicationException toThrowable(Response response) { + // Mappers run on the event loop, and reading an entity that is not buffered blocks. + response.bufferEntity(); + + var status = response.getStatus(); + var error = readError(response); + + // Not a Garage error body, for example from a proxy in front of Garage. + if (error == null) { + return new WebApplicationException( + "The Garage Admin API request failed [status=%d, reason=%s]".formatted( + status, + response.getStatusInfo().getReasonPhrase() + ), + response + ); + } + + return new WebApplicationException( + "The Garage Admin API request failed [status=%d, code=%s, message=%s]".formatted( + status, + error.code(), + error.message() + ), + response + ); + } + + private @Nullable ErrorResponse readError(Response response) { + if (!response.hasEntity()) { + return null; + } + + try { + return response.readEntity(ErrorResponse.class); + } catch (RuntimeException e) { + return null; + } + } +} diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ErrorResponse.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ErrorResponse.java new file mode 100644 index 0000000..7876d6c --- /dev/null +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/ErrorResponse.java @@ -0,0 +1,13 @@ +package it.aboutbits.garage.core.adminapi.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@JsonIgnoreProperties(ignoreUnknown = true) +@NullMarked +public record ErrorResponse( + @Nullable String code, + @Nullable String message +) { +} diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java index 94c5432..d1df711 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageCluster.java @@ -6,12 +6,13 @@ import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; +import it.aboutbits.garage.core.Constants; import it.aboutbits.garage.core.Named; import org.jspecify.annotations.NullMarked; /// A Garage cluster whose layout the operator manages; everything else is owned by the Helm chart. -@Version("v1") -@Group("garage.aboutbits.it") +@Version(Constants.CRD_VERSION) +@Group(Constants.CRD_GROUP) @AdditionalPrinterColumn( name = "Name", jsonPath = ".status.name", diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java index ba9c5a3..6fa96b0 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterLayoutSpec.java @@ -26,12 +26,8 @@ public class GarageClusterLayoutSpec { /// Should match the data volume size. Garage uses it as a weight, not as a limit. @Required @ValidationRule( - value = "self.trim().size() > 0", - message = "The capacity must not be empty." - ) - @ValidationRule( - value = "self.matches('^[0-9]+(\\\\.[0-9]+)?(E|P|T|G|M|k|Ei|Pi|Ti|Gi|Mi|Ki)?$')", - message = "The capacity must be a Kubernetes quantity, for example '20Gi'." + value = "isQuantity(self) && quantity(self).isInteger() && quantity(self).isGreaterThan(quantity('0'))", + message = "The capacity must be a Kubernetes quantity of whole bytes, greater than zero, for example '20Gi'." ) private String capacity = ""; diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java index babeeef..2cf35d0 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java @@ -65,11 +65,15 @@ public UpdateControl reconcile( var result = garageClusterService.reconcileLayout( garageAdminApi, - resource.getSpec().getLayout() + resource ); - status.setNodeId(result.nodeId()) - .setLayoutVersion(result.layoutVersion()); + // Keep the recorded node while Garage reports none, so that no second GarageCluster can claim it meanwhile. + if (result.nodeId() != null) { + status.setNodeId(result.nodeId()); + } + + status.setLayoutVersion(result.layoutVersion()); return switch (result.outcome()) { case NO_NODES, NODE_DOWN -> awaitingCluster(resource, status, result); diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java index 838d087..4d217ec 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java @@ -1,5 +1,6 @@ package it.aboutbits.garage.crd.garagecluster; +import io.fabric8.kubernetes.client.KubernetesClient; import it.aboutbits.garage.core.Quantities; import it.aboutbits.garage.core.adminapi.GarageAdminApi; import it.aboutbits.garage.core.adminapi.dto.ApplyClusterLayoutRequest; @@ -7,6 +8,7 @@ import it.aboutbits.garage.core.adminapi.dto.NodeRoleChangeRequest; import it.aboutbits.garage.core.adminapi.dto.UpdateClusterLayoutRequest; import jakarta.inject.Singleton; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -16,28 +18,36 @@ import java.util.Objects; @Singleton +@RequiredArgsConstructor @Slf4j @NullMarked public class GarageClusterService { + private final KubernetesClient kubernetesClient; + /// Idempotent: when the role already matches, no write request is sent. public LayoutReconcileResult reconcileLayout( GarageAdminApi garageAdminApi, - GarageClusterLayoutSpec layoutSpec + GarageCluster garageCluster ) { var clusterStatus = garageAdminApi.getClusterStatus(); var nodes = clusterStatus.nodes(); if (nodes.isEmpty()) { - return LayoutReconcileResult.noNodes(); + return LayoutReconcileResult.noNodes(clusterStatus.layoutVersion()); } // Only single-node clusters are assigned a layout: the Helm chart deploys replication_factor=1. if (nodes.size() > 1) { - return LayoutReconcileResult.unsupportedTopology(nodes.size()); + return LayoutReconcileResult.unsupportedTopology( + nodes.size(), + clusterStatus.layoutVersion() + ); } var node = nodes.getFirst(); + requireNotManagedElsewhere(garageCluster, node.id()); + if (!node.isUp()) { return LayoutReconcileResult.nodeDown( node.id(), @@ -45,7 +55,7 @@ public LayoutReconcileResult reconcileLayout( ); } - var desiredRole = desiredRole(layoutSpec); + var desiredRole = desiredRole(garageCluster.getSpec().getLayout()); if (rolesMatch(node.role(), desiredRole)) { return LayoutReconcileResult.upToDate( @@ -83,6 +93,34 @@ boolean rolesMatch( && new HashSet<>(currentRole.tags()).equals(new HashSet<>(desiredRole.tags())); } + /// Two resources with different layouts would apply their roles in turn, a new layout version at every resync. + /// The node id finds the same Garage even when the two `adminEndpoint` values are spelled differently. + private void requireNotManagedElsewhere( + GarageCluster garageCluster, + String nodeId + ) { + var managedElsewhere = kubernetesClient.resources(GarageCluster.class) + .inAnyNamespace() + .list() + .getItems() + .stream() + .filter(other -> !Objects.equals(other.getMetadata().getUid(), garageCluster.getMetadata().getUid())) + .filter(other -> other.getStatus() != null && nodeId.equals(other.getStatus().getNodeId())) + .findFirst(); + + if (managedElsewhere.isPresent()) { + var other = managedElsewhere.get(); + + throw new IllegalStateException( + "The Garage node is already managed by GarageCluster %s/%s [nodeId=%s]".formatted( + other.getMetadata().getNamespace(), + other.getMetadata().getName(), + nodeId + ) + ); + } + } + private LayoutReconcileResult applyRole( GarageAdminApi garageAdminApi, String nodeId, @@ -92,7 +130,7 @@ private LayoutReconcileResult applyRole( var currentLayout = garageAdminApi.getClusterLayout(); log.info( - "Staging Garage cluster layout role [node.id={}, layout.version={}, zone={}, capacity={}]", + "Staging Garage cluster layout role [nodeId={}, version={}, zone={}, capacity={}]", nodeId, currentLayout.version(), desiredRole.zone(), @@ -115,7 +153,7 @@ private LayoutReconcileResult applyRole( ); log.info( - "Applied Garage cluster layout [node.id={}, layout.version={}]", + "Applied Garage cluster layout [nodeId={}, version={}]", nodeId, applied.layout().version() ); diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java index 5af182c..d2cd773 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterSpec.java @@ -14,8 +14,8 @@ public class GarageClusterSpec { /// The Garage Admin API (port 3903), not the S3 endpoint, e.g. `http://my-release-garage.storage.svc:3903`. @Required @ValidationRule( - value = "self.startsWith('http://') || self.startsWith('https://')", - message = "The admin endpoint must be an http:// or https:// URL." + value = "isURL(self) && url(self).getScheme() in ['http', 'https'] && url(self).getHostname() != ''", + message = "The admin endpoint must be an http:// or https:// URL with a host." ) private String adminEndpoint = ""; diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java index 439d515..346b3c9 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/LayoutReconcileResult.java @@ -19,11 +19,11 @@ public enum Outcome { APPLIED } - public static LayoutReconcileResult noNodes() { + public static LayoutReconcileResult noNodes(long layoutVersion) { return new LayoutReconcileResult( Outcome.NO_NODES, null, - 0, + layoutVersion, "The Garage cluster reports no nodes yet" ); } @@ -36,15 +36,18 @@ public static LayoutReconcileResult nodeDown( Outcome.NODE_DOWN, nodeId, layoutVersion, - "The Garage node is not connected [node.id=%s]".formatted(nodeId) + "The Garage node is not connected [nodeId=%s]".formatted(nodeId) ); } - public static LayoutReconcileResult unsupportedTopology(int nodeCount) { + public static LayoutReconcileResult unsupportedTopology( + int nodeCount, + long layoutVersion + ) { return new LayoutReconcileResult( Outcome.UNSUPPORTED_TOPOLOGY, null, - 0, + layoutVersion, "The Garage cluster reports %d nodes, but this operator only assigns layout roles for single-node clusters. Assign the layout manually, or open an issue if multi-node support is needed.".formatted( nodeCount ) diff --git a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java index 1a9827b..1590968 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3Connection.java @@ -7,12 +7,13 @@ import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; import it.aboutbits.garage.core.CRStatus; +import it.aboutbits.garage.core.Constants; import it.aboutbits.garage.core.Named; import org.jspecify.annotations.NullMarked; /// The Garage Admin API that buckets, access keys and permissions are created through. -@Version("v1") -@Group("garage.aboutbits.it") +@Version(Constants.CRD_VERSION) +@Group(Constants.CRD_GROUP) @AdditionalPrinterColumn( name = "Name", jsonPath = ".status.name", diff --git a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java index d220b1f..23b726a 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/s3connection/S3ConnectionSpec.java @@ -14,8 +14,13 @@ public class S3ConnectionSpec { /// The Garage Admin API on port 3903, not the S3 endpoint, e.g. `http://acme-garage.acme.svc:3903`. @Required @ValidationRule( - value = "self.startsWith('http://') || self.startsWith('https://')", - message = "The admin endpoint must be an http:// or https:// URL." + value = "isURL(self) && url(self).getScheme() in ['http', 'https'] && url(self).getHostname() != ''", + message = "The admin endpoint must be an http:// or https:// URL with a host." + ) + // Buckets and access keys record the ids this Garage gave them, so another Garage would get second ones. + @ValidationRule( + value = "self == oldSelf", + message = "The adminEndpoint is immutable. Delete and recreate the S3Connection to point it at a different Garage." ) private String adminEndpoint = ""; diff --git a/operator/src/main/resources/application.yml b/operator/src/main/resources/application.yml index f5d1ab6..6de88f3 100644 --- a/operator/src/main/resources/application.yml +++ b/operator/src/main/resources/application.yml @@ -34,6 +34,10 @@ quarkus: # Whether controllers should only process events if the associated resource generation # has increased since the last reconciliation, otherwise will process all events. generation-aware: true + rest-client: + # Names the operator in the logs of a proxy in front of Garage. Only the product and the version: + # `adminEndpoint` can point at any server, so no Pod, node or cluster details. + user-agent: garage-operator/${quarkus.application.version} test: hang-detection-timeout: PT1M diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java index be65961..7a02f9b 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java @@ -17,13 +17,11 @@ public class Given { private final KubernetesClient kubernetesClient; /// Host port the Compose Dev Service published the Garage Admin API on. - @SuppressWarnings("NullAway.Init") @ConfigProperty(name = "garage.admin.port") - Integer garageAdminPort; + private final int garageAdminPort; - @SuppressWarnings("NullAway.Init") @ConfigProperty(name = "garage.admin.token") - String garageAdminToken; + private final String garageAdminToken; public GarageConnectionDetails garageConnectionDetails() { return new GarageConnectionDetails( diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java index c802655..3274812 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java @@ -10,7 +10,6 @@ import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; -import static it.aboutbits.garage.core.KubernetesService.SECRET_DATA_ADMIN_TOKEN_KEY; import static it.aboutbits.garage.core.KubernetesService.SECRET_TYPE_OPAQUE; /// Creates an `Opaque` Secret holding a token, plus the [SecretKeyRef] pointing at it. @@ -18,6 +17,9 @@ @Accessors(fluent = true, chain = true) @NullMarked public class SecretKeyRefCreate extends TestDataCreator { + /// The key the AboutBits Garage Helm chart stores the Admin API token under (https://github.com/aboutbits/helm-garage). + private static final String SECRET_DATA_ADMIN_TOKEN_KEY = "admin_token"; + private final KubernetesClient kubernetesClient; private @Nullable String withNamespace; diff --git a/operator/src/test/java/it/aboutbits/garage/core/BaseReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/core/BaseReconcilerTest.java new file mode 100644 index 0000000..5f1d59a --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/core/BaseReconcilerTest.java @@ -0,0 +1,166 @@ +package it.aboutbits.garage.core; + +import io.fabric8.kubernetes.api.model.KubernetesResourceList; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import it.aboutbits.garage.core.adminapi.GarageService; +import it.aboutbits.garage.crd.s3connection.S3Connection; +import it.aboutbits.garage.crd.s3connection.S3ConnectionReconciler; +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 static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@NullMarked +class BaseReconcilerTest { + private static final String NAMESPACE = "acme"; + private static final String CONNECTION_NAME = "acme-connection"; + + private final KubernetesClient kubernetesClient = mock(KubernetesClient.class); + + // Any concrete reconciler, since the method under test lives in the base class. + private final S3ConnectionReconciler reconciler = new S3ConnectionReconciler(mock(GarageService.class)); + + private static S3Connection s3Connection( + String name, + long generation + ) { + var s3Connection = new S3Connection(); + + s3Connection.setMetadata(new ObjectMetaBuilder() + .withNamespace(NAMESPACE) + .withName(name) + .withGeneration(generation) + .build() + ); + + return s3Connection; + } + + private static CRStatus status( + CRPhase phase, + long observedGeneration + ) { + var status = new CRStatus(); + + status.updatePhase(phase) + .setObservedGeneration(observedGeneration); + + return status; + } + + @Nested + class GetReferencedS3Connection { + /// The connectionRef has no namespace, so the connection is looked up in the namespace of the resource. + private final S3Connection resource = s3Connection("acme-resource", 1); + + private final ResourceRef connectionRef = connectionRef(); + + private static ResourceRef connectionRef() { + var connectionRef = new ResourceRef(); + + connectionRef.setName(CONNECTION_NAME); + + return connectionRef; + } + + @SuppressWarnings("unchecked") + private void givenConnection(@Nullable S3Connection s3Connection) { + MixedOperation, Resource> operation = + mock(MixedOperation.class); + NonNamespaceOperation, Resource> namespaceOperation = + mock(NonNamespaceOperation.class); + Resource connectionResource = mock(Resource.class); + + when(kubernetesClient.resources(S3Connection.class)).thenReturn(operation); + when(operation.inNamespace(NAMESPACE)).thenReturn(namespaceOperation); + when(namespaceOperation.withName(CONNECTION_NAME)).thenReturn(connectionResource); + when(connectionResource.get()).thenReturn(s3Connection); + } + + @Test + @DisplayName("A missing connection should not be returned") + void missingConnection_isEmpty() { + // given + givenConnection(null); + + // when + var result = reconciler.getReferencedS3Connection(kubernetesClient, resource, connectionRef); + + // then + assertThat(result).isEmpty(); + } + + @Test + @DisplayName("A connection that has not been probed yet should not be returned") + void connectionWithoutStatus_isEmpty() { + // given + givenConnection(s3Connection(CONNECTION_NAME, 1)); + + // when + var result = reconciler.getReferencedS3Connection(kubernetesClient, resource, connectionRef); + + // then + assertThat(result).isEmpty(); + } + + @Test + @DisplayName("A connection that is not ready should not be returned") + void pendingConnection_isEmpty() { + // given + var s3Connection = s3Connection(CONNECTION_NAME, 1); + + s3Connection.setStatus(status(CRPhase.PENDING, 1)); + + givenConnection(s3Connection); + + // when + var result = reconciler.getReferencedS3Connection(kubernetesClient, resource, connectionRef); + + // then + assertThat(result).isEmpty(); + } + + @Test + @DisplayName("A connection that is ready for an older generation should not be returned") + void readyForOlderGeneration_isEmpty() { + // given + var s3Connection = s3Connection(CONNECTION_NAME, 2); + + s3Connection.setStatus(status(CRPhase.READY, 1)); + + givenConnection(s3Connection); + + // when + var result = reconciler.getReferencedS3Connection(kubernetesClient, resource, connectionRef); + + // then + assertThat(result).isEmpty(); + } + + @Test + @DisplayName("A connection that is ready for the current generation should be returned") + void readyForCurrentGeneration_isReturned() { + // given + var s3Connection = s3Connection(CONNECTION_NAME, 2); + + s3Connection.setStatus(status(CRPhase.READY, 2)); + + givenConnection(s3Connection); + + // when + var result = reconciler.getReferencedS3Connection(kubernetesClient, resource, connectionRef); + + // then + assertThat(result).containsSame(s3Connection); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java b/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java index da035f8..19d1f0f 100644 --- a/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java +++ b/operator/src/test/java/it/aboutbits/garage/core/adminapi/GarageServiceTest.java @@ -44,10 +44,13 @@ class Probe { @ValueSource(strings = {"healthy", "degraded"}) @DisplayName("A cluster that can serve requests should be available") void servingCluster_isAvailable(String status) { + // given givenClusterHealth(status); + // when var result = garageService.probe(new S3Connection()); + // then assertThat(result.available()).isTrue(); assertThat(result.detail()).contains(status); } @@ -55,10 +58,13 @@ void servingCluster_isAvailable(String status) { @Test @DisplayName("An unavailable cluster should not be available, and should point at the layout") void unavailableCluster_isNotAvailable() { + // given givenClusterHealth("unavailable"); + // when var result = garageService.probe(new S3Connection()); + // then assertThat(result.available()).isFalse(); assertThat(result.detail()).contains("GarageCluster"); } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java index 7aa9466..1bf4500 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java @@ -1,25 +1,36 @@ package it.aboutbits.garage.crd.garagecluster; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; import io.quarkus.test.junit.QuarkusTest; import it.aboutbits.garage._support.testdata.base.TestUtil; import it.aboutbits.garage._support.testdata.persisted.Given; import it.aboutbits.garage.core.CRPhase; import it.aboutbits.garage.core.adminapi.GarageAdminApi; import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; -import jakarta.inject.Inject; +import lombok.RequiredArgsConstructor; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.AfterAll; 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.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import java.io.Closeable; +import java.io.IOException; import java.net.URI; +import java.util.List; import java.util.Objects; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowableOfType; import static org.awaitility.Awaitility.await; /// Exercises the reconciler against a real, unconfigured Garage node provided by the Compose Dev @@ -30,35 +41,42 @@ /// against *relative* changes — did the layout version advance, does the node's role now match the /// spec — rather than against absolute version numbers. @QuarkusTest +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@RequiredArgsConstructor @NullMarked class GarageClusterReconcilerTest { private static final long AWAIT_SECONDS = 60; - @Inject - @SuppressWarnings("NullAway.Init") - KubernetesClient kubernetesClient; + private final KubernetesClient kubernetesClient; - @Inject - @SuppressWarnings("NullAway.Init") - Given given; + private final Given given; - @Inject - @SuppressWarnings("NullAway.Init") - GarageAdminClientFactory garageAdminClientFactory; + private final GarageAdminClientFactory garageAdminClientFactory; @ConfigProperty(name = "garage.admin.port") - @SuppressWarnings("NullAway.Init") - Integer garageAdminPort; + private final int garageAdminPort; @ConfigProperty(name = "garage.admin.token") - @SuppressWarnings("NullAway.Init") - String garageAdminToken; + private final String garageAdminToken; + + private @Nullable GarageAdminApi garageAdminApi; private GarageAdminApi garageAdminApi() { - return garageAdminClientFactory.create( - URI.create("http://localhost:%d".formatted(garageAdminPort)), - garageAdminToken - ); + if (garageAdminApi == null) { + garageAdminApi = garageAdminClientFactory.create( + URI.create("http://localhost:%d".formatted(garageAdminPort)), + garageAdminToken + ); + } + + return garageAdminApi; + } + + @AfterAll + void closeGarageAdminApi() throws IOException { + if (garageAdminApi instanceof Closeable closeable) { + closeable.close(); + } } @BeforeEach @@ -73,6 +91,7 @@ void setUp() { TestUtil.resetEnvironment(kubernetesClient); } + /// Waits for the phase at the current generation, so that a phase from before an edit does not count. private GarageCluster awaitPhase( GarageCluster garageCluster, CRPhase expectedPhase @@ -84,25 +103,42 @@ private GarageCluster awaitPhase( .waitUntilCondition( item -> item != null && item.getStatus() != null + && item.getStatus().getObservedGeneration() == item.getMetadata().getGeneration() && item.getStatus().getPhase() == expectedPhase, AWAIT_SECONDS, TimeUnit.SECONDS ); } + private GarageCluster editLayout( + GarageCluster garageCluster, + Consumer edit + ) { + return kubernetesClient.resources(GarageCluster.class) + .inNamespace(garageCluster.getMetadata().getNamespace()) + .withName(garageCluster.getMetadata().getName()) + .edit(item -> { + edit.accept(item.getSpec().getLayout()); + return item; + }); + } + @Nested class Bootstrap { @Test @DisplayName("A GarageCluster should assign and apply a layout for the node") void garageCluster_appliesLayout() { + // given var garageCluster = given.one() .garageCluster() .withZone("default") .withCapacity("20Gi") .returnFirst(); + // when var reconciled = awaitPhase(garageCluster, CRPhase.READY); + // then var status = reconciled.getStatus(); assertThat(status.getLayoutVersion()).isPositive(); @@ -123,26 +159,23 @@ void garageCluster_appliesLayout() { } @Test - @DisplayName("A GarageCluster whose layout already matches should not create a new layout version") - void matchingLayout_isNotReapplied() { - var first = given.one() + @DisplayName("A reordered tag list should not create a new layout version") + void reorderedTags_isNotReapplied() { + // given + var garageCluster = given.one() .garageCluster() - .withZone("default") - .withCapacity("20Gi") + .withTags(List.of("ssd", "fast")) .returnFirst(); - awaitPhase(first, CRPhase.READY); + awaitPhase(garageCluster, CRPhase.READY); var layoutVersion = garageAdminApi().getClusterLayout().version(); - // A second resource describing the same desired state must be a no-op against Garage. - var second = given.one() - .garageCluster() - .withZone("default") - .withCapacity("20Gi") - .returnFirst(); + // when + var edited = editLayout(garageCluster, layout -> layout.setTags(List.of("fast", "ssd"))); - var reconciled = awaitPhase(second, CRPhase.READY); + // then + var reconciled = awaitPhase(edited, CRPhase.READY); assertThat(reconciled.getStatus().getLayoutVersion()).isEqualTo(layoutVersion); assertThat(garageAdminApi().getClusterLayout().version()).isEqualTo(layoutVersion); @@ -151,9 +184,9 @@ void matchingLayout_isNotReapplied() { @Test @DisplayName("Changing the capacity should apply a new layout version") void changedCapacity_appliesNewLayoutVersion() { + // given var garageCluster = given.one() .garageCluster() - .withZone("default") .withCapacity("20Gi") .returnFirst(); @@ -161,13 +194,11 @@ void changedCapacity_appliesNewLayoutVersion() { var layoutVersion = garageAdminApi().getClusterLayout().version(); - var changed = given.one() - .garageCluster() - .withZone("default") - .withCapacity("30Gi") - .returnFirst(); + // when + var edited = editLayout(garageCluster, layout -> layout.setCapacity("30Gi")); - var reconciled = awaitPhase(changed, CRPhase.READY); + // then + var reconciled = awaitPhase(edited, CRPhase.READY); assertThat(reconciled.getStatus().getLayoutVersion()).isGreaterThan(layoutVersion); @@ -184,36 +215,79 @@ void changedCapacity_appliesNewLayoutVersion() { @Nested class Failures { @Test - @DisplayName("An invalid admin token should surface as an error") + @DisplayName("A second GarageCluster for the same Garage node should surface as an error, and leave the layout alone") + void secondGarageCluster_setsErrorPhase() { + // given + var first = given.one() + .garageCluster() + .withCapacity("20Gi") + .returnFirst(); + + awaitPhase(first, CRPhase.READY); + + var layoutVersion = garageAdminApi().getClusterLayout().version(); + + // when + var second = given.one() + .garageCluster() + .withCapacity("30Gi") + .returnFirst(); + + // then + var reconciled = awaitPhase(second, CRPhase.ERROR); + + assertThat(reconciled.getStatus().getMessage()).contains( + "already managed by GarageCluster %s/%s".formatted( + first.getMetadata().getNamespace(), + first.getMetadata().getName() + ) + ); + assertThat(reconciled.getStatus().getNodeId()).isNull(); + assertThat(garageAdminApi().getClusterLayout().version()).isEqualTo(layoutVersion); + + assertThat(awaitPhase(first, CRPhase.READY).getStatus().getNodeId()).isNotBlank(); + } + + @Test + @DisplayName("An invalid admin token should surface as an error with the message of Garage") void invalidToken_setsErrorPhase() { + // given var adminSecretRef = given.one() .secretKeyRef() .withToken("definitely-not-the-admin-token") .returnFirst(); + // when var garageCluster = given.one() .garageCluster() .withAdminSecretRef(adminSecretRef) .returnFirst(); + // then var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); - assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + assertThat(reconciled.getStatus().getMessage()) + .startsWith("The Garage Admin API request failed [status=403, code=") + .doesNotContain("code=null") + .doesNotContain("message=null"); } @Test @DisplayName("A missing admin Secret should surface as an error") void missingSecret_setsErrorPhase() { + // given var adminSecretRef = given.one() .secretKeyRef() .withoutSecret() .returnFirst(); + // when var garageCluster = given.one() .garageCluster() .withAdminSecretRef(adminSecretRef) .returnFirst(); + // then var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); @@ -222,16 +296,19 @@ void missingSecret_setsErrorPhase() { @Test @DisplayName("An admin Secret without the referenced key should surface as an error") void missingSecretKey_setsErrorPhase() { + // given var adminSecretRef = given.one() .secretKeyRef() .withoutToken() .returnFirst(); + // when var garageCluster = given.one() .garageCluster() .withAdminSecretRef(adminSecretRef) .returnFirst(); + // then var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("missing the referenced key"); @@ -240,15 +317,56 @@ void missingSecretKey_setsErrorPhase() { @Test @DisplayName("An unreachable admin endpoint should surface as an error") void unreachableEndpoint_setsErrorPhase() { + // when var garageCluster = given.one() .garageCluster() // Reserved port that nothing listens on. .withAdminEndpoint("http://localhost:1") .returnFirst(); + // then var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).isNotBlank(); } } + + @Nested + class Validation { + @ParameterizedTest + @ValueSource(strings = {"http://", "http://:3903", "http:///v2", "ftp://garage:3903", "garage:3903"}) + @DisplayName("An admin endpoint that is not an http:// or https:// URL with a host should be rejected") + void invalidAdminEndpoint_isRejected(String adminEndpoint) { + // when + var thrown = catchThrowableOfType( + KubernetesClientException.class, + () -> given.one() + .garageCluster() + .withAdminEndpoint(adminEndpoint) + .returnFirst() + ); + + // then + assertThat(thrown.getCode()).isEqualTo(422); + assertThat(thrown.getMessage()).contains("The admin endpoint must be an http:// or https:// URL with a host."); + } + + @ParameterizedTest + @ValueSource(strings = {"", "0", "-1Gi", "0.5", "1m", "20 Gi", "twenty"}) + @DisplayName("A capacity that is not a whole number of bytes greater than zero should be rejected") + void invalidCapacity_isRejected(String capacity) { + // when + var thrown = catchThrowableOfType( + KubernetesClientException.class, + () -> given.one() + .garageCluster() + .withCapacity(capacity) + .returnFirst() + ); + + // then + assertThat(thrown.getCode()).isEqualTo(422); + assertThat(thrown.getMessage()).contains("The capacity must be a Kubernetes quantity of whole bytes"); + } + } } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java index d6983af..3f282ef 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java @@ -1,7 +1,17 @@ package it.aboutbits.garage.crd.garagecluster; +import io.fabric8.kubernetes.api.model.KubernetesResourceList; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.AnyNamespaceOperation; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import it.aboutbits.garage.core.adminapi.GarageAdminApi; +import it.aboutbits.garage.core.adminapi.dto.GetClusterStatusResponse; import it.aboutbits.garage.core.adminapi.dto.NodeAssignedRole; +import it.aboutbits.garage.core.adminapi.dto.NodeResp; import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -9,24 +19,190 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @NullMarked class GarageClusterServiceTest { - private final GarageClusterService garageClusterService = new GarageClusterService(); + private static final long LAYOUT_VERSION = 5; + + private final KubernetesClient kubernetesClient = mock(KubernetesClient.class); + private final GarageAdminApi garageAdminApi = mock(GarageAdminApi.class); + + private final GarageClusterService garageClusterService = new GarageClusterService(kubernetesClient); + + private static GarageCluster garageCluster( + String uid, + String name + ) { + var garageCluster = new GarageCluster(); + + garageCluster.setMetadata(new ObjectMetaBuilder() + .withNamespace("acme") + .withName(name) + .withUid(uid) + .build() + ); + + var layout = new GarageClusterLayoutSpec(); + + layout.setZone("default"); + layout.setCapacity("20Gi"); + + var spec = new GarageClusterSpec(); + + spec.setLayout(layout); + + garageCluster.setSpec(spec); + + return garageCluster; + } + + private static NodeResp node( + String id, + boolean isUp + ) { + return new NodeResp( + id, + isUp, + false, + null, + null, + null + ); + } + + @Nested + class ReconcileLayout { + private final GarageCluster garageCluster = garageCluster("uid-1", "acme"); + + @BeforeEach + void givenOnlyThisGarageCluster() { + givenGarageClusters(garageCluster); + } + + @SuppressWarnings("unchecked") + private void givenGarageClusters(GarageCluster... garageClusters) { + MixedOperation, Resource> operation = + mock(MixedOperation.class); + AnyNamespaceOperation, Resource> anyNamespaceOperation = + mock(AnyNamespaceOperation.class); + KubernetesResourceList list = mock(KubernetesResourceList.class); + + when(kubernetesClient.resources(GarageCluster.class)).thenReturn(operation); + when(operation.inAnyNamespace()).thenReturn(anyNamespaceOperation); + when(anyNamespaceOperation.list()).thenReturn(list); + when(list.getItems()).thenReturn(List.of(garageClusters)); + } + + private void givenNodes(NodeResp... nodes) { + when(garageAdminApi.getClusterStatus()) + .thenReturn(new GetClusterStatusResponse(LAYOUT_VERSION, List.of(nodes))); + } + + private void thenNoLayoutIsWritten() { + verify(garageAdminApi, never()).updateClusterLayout(any()); + verify(garageAdminApi, never()).applyClusterLayout(any()); + } + + @Test + @DisplayName("A cluster without nodes should wait, and keep the layout version") + void noNodes_awaitsCluster() { + // given + givenNodes(); + + // when + var result = garageClusterService.reconcileLayout(garageAdminApi, garageCluster); + + // then + assertThat(result.outcome()).isEqualTo(LayoutReconcileResult.Outcome.NO_NODES); + assertThat(result.nodeId()).isNull(); + assertThat(result.layoutVersion()).isEqualTo(LAYOUT_VERSION); + + thenNoLayoutIsWritten(); + } + + @Test + @DisplayName("A cluster with more than one node should be refused, and keep the layout version") + void twoNodes_isUnsupported() { + // given + givenNodes( + node("node-1", true), + node("node-2", true) + ); + + // when + var result = garageClusterService.reconcileLayout(garageAdminApi, garageCluster); + + // then + assertThat(result.outcome()).isEqualTo(LayoutReconcileResult.Outcome.UNSUPPORTED_TOPOLOGY); + assertThat(result.layoutVersion()).isEqualTo(LAYOUT_VERSION); + assertThat(result.message()).contains("2 nodes"); + + thenNoLayoutIsWritten(); + } + + @Test + @DisplayName("A node that is not connected should wait") + void nodeDown_awaitsCluster() { + // given + givenNodes(node("node-1", false)); + + // when + var result = garageClusterService.reconcileLayout(garageAdminApi, garageCluster); + + // then + assertThat(result.outcome()).isEqualTo(LayoutReconcileResult.Outcome.NODE_DOWN); + assertThat(result.nodeId()).isEqualTo("node-1"); + assertThat(result.layoutVersion()).isEqualTo(LAYOUT_VERSION); + + thenNoLayoutIsWritten(); + } + + @Test + @DisplayName("A node that another GarageCluster manages should be refused") + void managedElsewhere_isRefused() { + // given + givenNodes(node("node-1", true)); + + var other = garageCluster("uid-2", "other"); + + other.setStatus(new GarageClusterStatus().setNodeId("node-1")); + + givenGarageClusters(garageCluster, other); + + // when + var thrown = catchThrowable(() -> garageClusterService.reconcileLayout(garageAdminApi, garageCluster)); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("already managed by GarageCluster acme/other [nodeId=node-1]"); + + thenNoLayoutIsWritten(); + } + } @Nested class DesiredRole { @Test @DisplayName("The spec should be translated into the role Garage records") void spec_isTranslatedToRole() { + // given var layoutSpec = new GarageClusterLayoutSpec(); layoutSpec.setZone("dc1"); layoutSpec.setCapacity("20Gi"); layoutSpec.setTags(List.of("ssd")); + // when var role = garageClusterService.desiredRole(layoutSpec); + // then assertThat(role.zone()).isEqualTo("dc1"); assertThat(role.capacity()).isEqualTo(21474836480L); assertThat(role.tags()).containsExactly("ssd"); @@ -35,14 +211,17 @@ void spec_isTranslatedToRole() { @Test @DisplayName("Tags set to an explicit null should mean no tags") void nullTags_meanNoTags() { + // given var layoutSpec = new GarageClusterLayoutSpec(); layoutSpec.setZone("dc1"); layoutSpec.setCapacity("20Gi"); layoutSpec.setTags(null); + // when var role = garageClusterService.desiredRole(layoutSpec); + // then assertThat(role.tags()).isEmpty(); } } @@ -58,79 +237,113 @@ class RolesMatch { @Test @DisplayName("A node without a role should never match") void noRole_doesNotMatch() { - assertThat(garageClusterService.rolesMatch(null, desiredRole)).isFalse(); + // when + var matches = garageClusterService.rolesMatch(null, desiredRole); + + // then + assertThat(matches).isFalse(); } @Test @DisplayName("An identical role should match") void identicalRole_matches() { + // given var currentRole = new NodeAssignedRole( "default", 21474836480L, List.of("ssd", "fast") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isTrue(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isTrue(); } @Test @DisplayName("Tag order should not count as a difference") void reorderedTags_match() { + // given var currentRole = new NodeAssignedRole( "default", 21474836480L, List.of("fast", "ssd") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isTrue(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isTrue(); } @Test @DisplayName("A different zone should count as a difference") void differentZone_doesNotMatch() { + // given var currentRole = new NodeAssignedRole( "dc2", 21474836480L, List.of("ssd", "fast") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isFalse(); } @Test @DisplayName("A different capacity should count as a difference") void differentCapacity_doesNotMatch() { + // given var currentRole = new NodeAssignedRole( "default", 1073741824L, List.of("ssd", "fast") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isFalse(); } @Test @DisplayName("A gateway node should not match a node with a capacity") void gatewayNode_doesNotMatch() { + // given var currentRole = new NodeAssignedRole( "default", null, List.of("ssd", "fast") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isFalse(); } @Test @DisplayName("Different tags should count as a difference") void differentTags_doNotMatch() { + // given var currentRole = new NodeAssignedRole( "default", 21474836480L, List.of("ssd") ); - assertThat(garageClusterService.rolesMatch(currentRole, desiredRole)).isFalse(); + // when + var matches = garageClusterService.rolesMatch(currentRole, desiredRole); + + // then + assertThat(matches).isFalse(); } } } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java index 0c45c55..daecd80 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java @@ -1,6 +1,7 @@ package it.aboutbits.garage.crd.s3connection; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; import io.quarkus.test.junit.QuarkusTest; import it.aboutbits.garage._support.testdata.base.TestUtil; import it.aboutbits.garage._support.testdata.persisted.Given; @@ -8,18 +9,26 @@ import it.aboutbits.garage.core.adminapi.GarageAdminApi; import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; import it.aboutbits.garage.crd.garagecluster.GarageCluster; -import jakarta.inject.Inject; +import lombok.RequiredArgsConstructor; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.AfterAll; 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.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import java.io.Closeable; +import java.io.IOException; import java.net.URI; import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowableOfType; import static org.awaitility.Awaitility.await; /// Exercises the connection probe against the real Garage node provided by the Compose Dev @@ -28,35 +37,42 @@ /// Every test that expects a `READY` connection first brings the cluster up through a /// `GarageCluster`, so it does not depend on a layout another test happened to apply. @QuarkusTest +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@RequiredArgsConstructor @NullMarked class S3ConnectionReconcilerTest { private static final long AWAIT_SECONDS = 60; - @Inject - @SuppressWarnings("NullAway.Init") - KubernetesClient kubernetesClient; + private final KubernetesClient kubernetesClient; - @Inject - @SuppressWarnings("NullAway.Init") - Given given; + private final Given given; - @Inject - @SuppressWarnings("NullAway.Init") - GarageAdminClientFactory garageAdminClientFactory; + private final GarageAdminClientFactory garageAdminClientFactory; @ConfigProperty(name = "garage.admin.port") - @SuppressWarnings("NullAway.Init") - Integer garageAdminPort; + private final int garageAdminPort; @ConfigProperty(name = "garage.admin.token") - @SuppressWarnings("NullAway.Init") - String garageAdminToken; + private final String garageAdminToken; + + private @Nullable GarageAdminApi garageAdminApi; private GarageAdminApi garageAdminApi() { - return garageAdminClientFactory.create( - URI.create("http://localhost:%d".formatted(garageAdminPort)), - garageAdminToken - ); + if (garageAdminApi == null) { + garageAdminApi = garageAdminClientFactory.create( + URI.create("http://localhost:%d".formatted(garageAdminPort)), + garageAdminToken + ); + } + + return garageAdminApi; + } + + @AfterAll + void closeGarageAdminApi() throws IOException { + if (garageAdminApi instanceof Closeable closeable) { + closeable.close(); + } } @BeforeEach @@ -110,51 +126,85 @@ class Probe { @Test @DisplayName("A connection to a serving cluster should become ready") void servingCluster_becomesReady() { + // given givenAppliedLayout(); + // when var s3Connection = given.one() .s3Connection() .returnFirst(); + // then var reconciled = awaitPhase(s3Connection, CRPhase.READY); assertThat(reconciled.getStatus().getMessage()).contains("Garage cluster"); } + + @Test + @DisplayName("An admin token that ends with a newline should be accepted") + void tokenWithTrailingNewline_becomesReady() { + // given + givenAppliedLayout(); + + // As written by `echo "$TOKEN" > token.txt` and `kubectl create secret generic --from-file`. + var adminSecretRef = given.one() + .secretKeyRef() + .withToken(garageAdminToken + "\n") + .returnFirst(); + + // when + var s3Connection = given.one() + .s3Connection() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + // then + awaitPhase(s3Connection, CRPhase.READY); + } } @Nested class Failures { @Test - @DisplayName("An invalid admin token should surface as an error") + @DisplayName("An invalid admin token should surface as an error with the message of Garage") void invalidToken_setsErrorPhase() { + // given var adminSecretRef = given.one() .secretKeyRef() .withToken("definitely-not-the-admin-token") .returnFirst(); + // when var s3Connection = given.one() .s3Connection() .withAdminSecretRef(adminSecretRef) .returnFirst(); + // then var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); - assertThat(reconciled.getStatus().getMessage()).isNotBlank(); + assertThat(reconciled.getStatus().getMessage()) + .startsWith("The Garage Admin API request failed [status=403, code=") + .doesNotContain("code=null") + .doesNotContain("message=null"); } @Test @DisplayName("A missing admin Secret should surface as an error") void missingSecret_setsErrorPhase() { + // given var adminSecretRef = given.one() .secretKeyRef() .withoutSecret() .returnFirst(); + // when var s3Connection = given.one() .s3Connection() .withAdminSecretRef(adminSecretRef) .returnFirst(); + // then var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); @@ -163,15 +213,63 @@ void missingSecret_setsErrorPhase() { @Test @DisplayName("An unreachable admin endpoint should surface as an error") void unreachableEndpoint_setsErrorPhase() { + // when var s3Connection = given.one() .s3Connection() // Reserved port that nothing listens on. .withAdminEndpoint("http://localhost:1") .returnFirst(); + // then var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).isNotBlank(); } } + + @Nested + class Validation { + @ParameterizedTest + @ValueSource(strings = {"http://", "http://:3903", "http:///v2", "ftp://garage:3903", "garage:3903"}) + @DisplayName("An admin endpoint that is not an http:// or https:// URL with a host should be rejected") + void invalidAdminEndpoint_isRejected(String adminEndpoint) { + // when + var thrown = catchThrowableOfType( + KubernetesClientException.class, + () -> given.one() + .s3Connection() + .withAdminEndpoint(adminEndpoint) + .returnFirst() + ); + + // then + assertThat(thrown.getCode()).isEqualTo(422); + assertThat(thrown.getMessage()).contains("The admin endpoint must be an http:// or https:// URL with a host."); + } + + @Test + @DisplayName("Changing the admin endpoint should be rejected") + void changedAdminEndpoint_isRejected() { + // given + var s3Connection = given.one() + .s3Connection() + .returnFirst(); + + // when + var thrown = catchThrowableOfType( + KubernetesClientException.class, + () -> kubernetesClient.resources(S3Connection.class) + .inNamespace(s3Connection.getMetadata().getNamespace()) + .withName(s3Connection.getMetadata().getName()) + .edit(item -> { + item.getSpec().setAdminEndpoint("http://other-garage.acme.svc:3903"); + return item; + }) + ); + + // then + assertThat(thrown.getCode()).isEqualTo(422); + assertThat(thrown.getMessage()).contains("The adminEndpoint is immutable."); + } + } } diff --git a/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java b/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java index 12d5fa2..6a175fa 100644 --- a/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java +++ b/operator/src/test/java/it/aboutbits/garage/helm/HelmTest.java @@ -11,6 +11,7 @@ import io.fabric8.kubernetes.client.utils.Serialization; import io.quarkus.test.junit.QuarkusTest; import io.smallrye.common.process.ProcessBuilder; +import it.aboutbits.garage.core.Constants; import lombok.extern.slf4j.Slf4j; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jspecify.annotations.NullMarked; @@ -46,7 +47,6 @@ class HelmTest { "volumeMounts" ); - private static final String CRD_GROUP = "garage.aboutbits.it"; /// Singular name (used for the reconciler's role binding) to plural name (used for the CRD /// file). Spelled out rather than derived, because `bucketaccess` → `bucketaccesses` is not a /// plain `+ "s"`. @@ -158,7 +158,7 @@ void helmInstall_createsDeployment() throws IOException { for (var crdPlural : CRD_NAMES.values()) { assertThat(chartPath.resolve("crds/%s.%s-v1.yml".formatted( crdPlural, - CRD_GROUP + Constants.CRD_GROUP ))).exists(); } From 7e3c7a89434b7040e0e9cef41aaed84bff582f20 Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Wed, 14 Oct 2026 10:06:19 +0200 Subject: [PATCH 3/6] address the second review of the GarageCluster and S3Connection resources The operator owns the layout, so a GarageCluster needs Garage started without `--single-node` (chart value `garage.singleNode: false`): in single-node mode Garage refuses to start once the layout version is higher than 1. Assisted-by: Claude:claude-opus-5-5 --- AGENTS.md | 3 +- README.md | 2 + docs/garage-cluster.md | 28 +++++++++--- docs/s3-connection.md | 2 +- .../src/main/docker/compose-devservices.yml | 2 + operator/src/main/docker/garage.toml | 6 +-- .../aboutbits/garage/core/BaseReconciler.java | 3 +- .../adminapi/GarageAdminClientFactory.java | 6 +-- .../adminapi/dto/NodeRoleChangeRequest.java | 10 ----- .../GarageClusterReconciler.java | 5 +++ .../garagecluster/GarageClusterService.java | 2 +- .../creator/GarageClusterCreate.java | 2 +- .../persisted/creator/S3ConnectionCreate.java | 2 +- .../persisted/creator/SecretKeyRefCreate.java | 2 +- .../GarageClusterReconcilerTest.java | 43 +++++++++++++++++++ 15 files changed, 88 insertions(+), 30 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 20bcaad..55549bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,8 @@ When in doubt about a pattern, look there first. - **Generated artifacts**: the CRDs and the Helm chart are generated from the code at build time, into `operator/build/kubernetes` and `operator/build/helm`. Change the Java classes or `application.yml`, never the output. - **Backend**: Garage only, driven through its Admin API v2. `GarageService` in `core/adminapi` is what the - reconcilers call; `GarageAdminApi` is the HTTP client underneath. + reconcilers call; `GarageAdminApi` is the HTTP client underneath. The exception is `GarageClusterReconciler`: it + builds the client from its own `adminEndpoint` and passes it to `GarageClusterService`. - **Tests**: `@QuarkusTest` integration tests against a K3s cluster and a real Garage node, both provided by Quarkus Dev Services, so Docker is required. Prefer a test against the real Garage over a mock: what matters is how the Admin API actually behaves. diff --git a/README.md b/README.md index d97e9c0..7704de8 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,8 @@ It is the companion of the [AboutBits Garage Helm chart](https://github.com/abou | **Kubernetes** | 1.34+ | > **Note:** Kubernetes 1.34+ is required. The tests run against K3s 1.34, and older Kubernetes versions are end-of-life. The Helm chart refuses to install on an older cluster. +> +> **Note:** A [`GarageCluster`](docs/garage-cluster.md) needs Garage installed with the chart value `garage.singleNode: false`. With the default `--single-node`, Garage creates its layout itself and does not start again once the operator has applied a newer one. Without a `GarageCluster`, keep the default. ## Contribute diff --git a/docs/garage-cluster.md b/docs/garage-cluster.md index 52bbf3c..33b0ad8 100644 --- a/docs/garage-cluster.md +++ b/docs/garage-cluster.md @@ -2,8 +2,15 @@ A `GarageCluster` bootstraps and converges the **cluster layout** of a Garage installation. -This is the resource everything else depends on. A freshly installed Garage node knows about itself but has no role in any layout, and in that state it **serves no S3 traffic at all** — every request fails until a layout is assigned and applied. The -[AboutBits Garage Helm chart](https://github.com/aboutbits/helm-garage) deliberately stops at day-0 concerns (StatefulSet, volumes, `garage.toml`, the admin token), so assigning that layout is the operator's job. +A Garage node started without `--single-node` knows about itself but has no role in any layout, and in that state it **serves no S3 traffic at all** — every request fails until a layout is assigned and applied. The `GarageCluster` assigns that layout. + +## Requirement: `garage.singleNode: false` + +By default the [AboutBits Garage Helm chart](https://github.com/aboutbits/helm-garage) starts Garage with `--single-node`. Garage then creates the layout itself, and refuses to start once the layout version is higher than 1. A `GarageCluster` applies a new layout version as soon as its spec differs from the role of the node, so with `--single-node` the next restart of the Garage Pod fails for good. + +Install the chart with `garage.singleNode: false` to manage the layout with a `GarageCluster`. Without a `GarageCluster`, keep the default: Garage creates its own layout, and [`S3Connection`](s3-connection.md), `Bucket` and `AccessKey` work the same. The chart's `bootstrap` option needs single-node mode, so with `garage.singleNode: false` create the bucket and the key with a `Bucket` and an `AccessKey` instead. + +The operator cannot see how Garage was started, so it cannot refuse a `GarageCluster` for a Garage in single-node mode. The operator drives Garage through its [Admin API v2](https://garagehq.deuxfleurs.fr/documentation/reference-manual/admin-api/), not through the S3 API — Garage implements neither IAM nor bucket policies, so buckets, keys and permissions are all Admin API concerns too. @@ -41,11 +48,11 @@ spec: ## Spec -| Field | Type | Description | Required | Mutable | -|------------------|--------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------|----------|---------| -| `adminEndpoint` | `string` | Base URL of the Garage Admin API. The Helm chart exposes it on port 3903 as a ClusterIP-only Service, reachable from inside the cluster only. | Yes | Yes | -| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the `Opaque` Secret holding the Admin API bearer token. The chart creates one named after the release. | Yes | Yes | -| `layout` | [`GarageClusterLayoutSpec`](#garageclusterlayoutspec-layout) | The role of the node in the cluster layout. | Yes | Yes | +| Field | Type | Description | Required | Mutable | +|------------------|--------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|---------| +| `adminEndpoint` | `string` | Base URL of the Garage Admin API, port 3903. The chart puts it on the same Service as the S3 API. Keep `service.type: ClusterIP`, because another type also makes the Admin API reachable from outside the cluster. | Yes | Yes | +| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the `Opaque` Secret holding the Admin API bearer token. The chart creates one named after the release. | Yes | Yes | +| `layout` | [`GarageClusterLayoutSpec`](#garageclusterlayoutspec-layout) | The role of the node in the cluster layout. | Yes | Yes | ### SecretKeyRef (`adminSecretRef`) @@ -82,6 +89,13 @@ The token needs access to these Admin API endpoints: `GetClusterStatus`, `GetClu `PENDING` is the normal state right after a `helm install`, while Garage is still starting and reports no nodes or a disconnected node; it is retried every 15 seconds. +`ERROR` has two causes: + +- The Secret, the token or the endpoint is wrong, or Garage cannot be reached. The operator tries again every 60 seconds. +- Garage reports more than one node. The operator does not try again before the next resync after 10 minutes, or a change of the spec. + +`health`, `storageNodes` and `storageNodesUp` are only set while the phase is `READY`, so another phase does not show an old value. + ## How reconciliation works 1. Read `GetClusterStatus`. No nodes, or the node not connected, means Garage is still coming up → diff --git a/docs/s3-connection.md b/docs/s3-connection.md index d212d26..d3bbc63 100644 --- a/docs/s3-connection.md +++ b/docs/s3-connection.md @@ -58,7 +58,7 @@ The token needs access to the Admin API endpoint `GetClusterHealth`. The token o |-----------|---------------------------------------------------------------------------------------------------------------------------------------------------------| | `READY` | The backend is reachable, the credentials are accepted, and it can serve requests. | | `PENDING` | Reachable and authenticated, but not ready — for Garage, health is `unavailable`, usually because no layout has been applied. Retried every 30 seconds. | -| `ERROR` | Unreachable, or the credentials were rejected, or the Secret is missing. | +| `ERROR` | Unreachable, or the credentials were rejected, or the Secret is missing. Retried every 60 seconds. | `message` carries the backend detail, for example `Garage cluster healthy [storageNodes=1, storageNodesUp=1, partitionsAllOk=256/256]`. diff --git a/operator/src/main/docker/compose-devservices.yml b/operator/src/main/docker/compose-devservices.yml index c03dbbe..9991654 100644 --- a/operator/src/main/docker/compose-devservices.yml +++ b/operator/src/main/docker/compose-devservices.yml @@ -3,6 +3,8 @@ services: # Keep this in sync with the image the AboutBits Garage Helm chart deploys. # https://github.com/aboutbits/helm-garage image: dxflrs/garage:v2.4.1 + # The chart's command with `garage.singleNode: false`, the mode a GarageCluster requires. + command: ["/garage", "server"] volumes: - ./garage.toml:/etc/garage.toml:ro tmpfs: diff --git a/operator/src/main/docker/garage.toml b/operator/src/main/docker/garage.toml index 318c5a5..c9540ce 100644 --- a/operator/src/main/docker/garage.toml +++ b/operator/src/main/docker/garage.toml @@ -4,9 +4,9 @@ # factor of 1 and the Admin API enabled — so the operator is exercised against the same shape of # cluster it manages in production. # -# Note that no cluster layout is configured here. A freshly started Garage node has no layout and -# serves no S3 traffic until one is assigned, which is exactly the bootstrap situation the -# GarageCluster reconciler is responsible for. +# Garage runs without `--single-node`, as the chart does with `garage.singleNode: false`. The node +# then starts without a layout and serves no S3 traffic until one is assigned, which is exactly the +# bootstrap situation the GarageCluster reconciler is responsible for. metadata_dir = "/var/lib/garage/meta" data_dir = "/var/lib/garage/data" diff --git a/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java index a573908..786ae0c 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java +++ b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java @@ -124,7 +124,8 @@ public UpdateControl handleError( E exception ) { log.error( - "Failed to reconcile resource [resource={}]", + "Failed to reconcile resource [resource={}/{}]", + resource.getMetadata().getNamespace(), resource.getMetadata().getName(), exception ); diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java index 4520d8e..590430d 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java @@ -20,9 +20,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; -/// Clients are kept per admin endpoint and token, because each holds a connection pool. Keying by -/// token too keeps two resources with different tokens for one endpoint from closing each other's -/// client mid-request. The token is read from its Secret on every call, so rotation applies at once. +/// Clients are kept per admin endpoint and token, because each holds a connection pool. The token is part of the key, +/// because each client sends one fixed token through its [BearerTokenFilter]. The token is read from its Secret at +/// every reconcile, so a new token gets a new client at the next reconcile. @ApplicationScoped @RequiredArgsConstructor @Slf4j diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java index 32bc63f..a5c2f0f 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/dto/NodeRoleChangeRequest.java @@ -28,14 +28,4 @@ public static NodeRoleChangeRequest assign( null ); } - - public static NodeRoleChangeRequest remove(String id) { - return new NodeRoleChangeRequest( - id, - null, - null, - null, - true - ); - } } diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java index 2cf35d0..f44cd74 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconciler.java @@ -50,6 +50,11 @@ public UpdateControl reconcile( ) { var status = initializeStatus(resource); + // Only `ready()` reads the health, so that other outcomes do not show an old value. + status.setHealth(null) + .setStorageNodes(0) + .setStorageNodesUp(0); + var namespace = resource.getMetadata().getNamespace(); var name = resource.getMetadata().getName(); diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java index 4d217ec..43caa9f 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java @@ -74,7 +74,7 @@ public LayoutReconcileResult reconcileLayout( NodeAssignedRole desiredRole(GarageClusterLayoutSpec layoutSpec) { return new NodeAssignedRole( layoutSpec.getZone(), - Quantities.toBytes(layoutSpec.getCapacity(), "layout capacity"), + Quantities.toBytes(layoutSpec.getCapacity(), "spec.layout.capacity"), List.copyOf(layoutSpec.getTags()) ); } diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java index fb7f913..419f35e 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java @@ -68,8 +68,8 @@ protected GarageCluster create(int index) { var item = new GarageCluster(); item.setMetadata(new ObjectMetaBuilder() - .withName(name) .withNamespace(namespace) + .withName(name) .build() ); diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java index 8e9813a..036d140 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java @@ -60,8 +60,8 @@ protected S3Connection create(int index) { var item = new S3Connection(); item.setMetadata(new ObjectMetaBuilder() - .withName(name) .withNamespace(namespace) + .withName(name) .build() ); diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java index 3274812..2f97f13 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java @@ -92,8 +92,8 @@ protected SecretKeyRef create(int index) { var secretKeyRef = new SecretKeyRef(); - secretKeyRef.setName(name); secretKeyRef.setNamespace(namespace); + secretKeyRef.setName(name); secretKeyRef.setKey(key); return secretKeyRef; diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java index 1bf4500..e74111c 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java @@ -24,7 +24,10 @@ import java.io.Closeable; import java.io.IOException; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; @@ -248,6 +251,46 @@ void secondGarageCluster_setsErrorPhase() { assertThat(awaitPhase(first, CRPhase.READY).getStatus().getNodeId()).isNotBlank(); } + @Test + @DisplayName("An error after READY should not keep the health of the last READY") + void errorAfterReady_clearsHealth() { + // given + var adminSecretRef = given.one() + .secretKeyRef() + .withToken(garageAdminToken) + .returnFirst(); + + var garageCluster = given.one() + .garageCluster() + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + // precondition: the health is set while READY + assertThat(awaitPhase(garageCluster, CRPhase.READY).getStatus().getHealth()).isNotNull(); + + kubernetesClient.secrets() + .inNamespace(adminSecretRef.getNamespace()) + .withName(adminSecretRef.getName()) + .edit(secret -> { + secret.setData(Map.of( + adminSecretRef.getKey(), + Base64.getEncoder().encodeToString("definitely-not-the-admin-token".getBytes(StandardCharsets.UTF_8)) + )); + return secret; + }); + + // when + // A change of the spec triggers the reconcile; a Secret change alone waits for the resync. + var edited = editLayout(garageCluster, layout -> layout.setTags(List.of("edited"))); + + // then + var status = awaitPhase(edited, CRPhase.ERROR).getStatus(); + + assertThat(status.getHealth()).isNull(); + assertThat(status.getStorageNodes()).isZero(); + assertThat(status.getStorageNodesUp()).isZero(); + } + @Test @DisplayName("An invalid admin token should surface as an error with the message of Garage") void invalidToken_setsErrorPhase() { From 5d5a1c64e816ec360cc43505973ed680d36c9a76 Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Wed, 14 Oct 2026 10:25:43 +0200 Subject: [PATCH 4/6] keep the developer kubeconfig out of the tests On token refresh, the fabric8 client re-reads the kubeconfig. An exec plugin there (e.g. `aws eks get-token`) then ran before many requests against the Dev Services cluster, slow enough to time out the cleanup between tests. Assisted-by: Claude:claude-opus-5-5 --- operator/build.gradle.kts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/operator/build.gradle.kts b/operator/build.gradle.kts index c5ffefd..ff2a590 100644 --- a/operator/build.gradle.kts +++ b/operator/build.gradle.kts @@ -71,6 +71,11 @@ tasks.withType().configureEach { dependsOn(tasks.quarkusAppPartsBuild) jvmArgumentProviders.add(MockitoArgumentProvider(mockitoAgentProvider)) + + // The fabric8 client re-reads the kubeconfig on token refresh. A developer kubeconfig with an exec plugin + // (e.g. `aws eks get-token`) then runs that plugin before many requests, which slows every test against the + // Dev Services cluster. The tests use only the Dev Services cluster, so they must not see that file at all. + environment("KUBECONFIG", layout.buildDirectory.file("no-kubeconfig").get().asFile.absolutePath) } class MockitoArgumentProvider( From 6a3e9161a21cc7ded53334e8e3b2d1fdc39f8d37 Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Wed, 14 Oct 2026 11:01:57 +0200 Subject: [PATCH 5/6] address the third review of the GarageCluster and S3Connection resources The shared test helpers and `RETRY_SECONDS` move here from the Bucket and AccessKey PR, so that every test waits for the current generation. Assisted-by: Claude:claude-opus-5-5 --- docs/garage-cluster.md | 3 +- .../aboutbits/garage/core/BaseReconciler.java | 11 +-- .../garagecluster/GarageClusterService.java | 2 +- .../_support/testdata/base/TestUtil.java | 26 +++++++ .../_support/testdata/persisted/Given.java | 37 ++++++++++ .../creator/GarageClusterCreate.java | 15 +--- .../persisted/creator/S3ConnectionCreate.java | 15 +--- .../persisted/creator/SecretKeyRefCreate.java | 14 +--- .../GarageClusterReconcilerTest.java | 69 +++++++++---------- .../GarageClusterServiceTest.java | 2 +- .../S3ConnectionReconcilerTest.java | 55 +++------------ 11 files changed, 119 insertions(+), 130 deletions(-) diff --git a/docs/garage-cluster.md b/docs/garage-cluster.md index 33b0ad8..14187de 100644 --- a/docs/garage-cluster.md +++ b/docs/garage-cluster.md @@ -89,9 +89,10 @@ The token needs access to these Admin API endpoints: `GetClusterStatus`, `GetClu `PENDING` is the normal state right after a `helm install`, while Garage is still starting and reports no nodes or a disconnected node; it is retried every 15 seconds. -`ERROR` has two causes: +`ERROR` has three causes: - The Secret, the token or the endpoint is wrong, or Garage cannot be reached. The operator tries again every 60 seconds. +- Another `GarageCluster` already manages the node. The operator tries again every 60 seconds, so this one takes over when the other one is deleted. - Garage reports more than one node. The operator does not try again before the next resync after 10 minutes, or a change of the spec. `health`, `storageNodes` and `storageNodesUp` are only set while the phase is `READY`, so another phase does not show an old value. diff --git a/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java index 786ae0c..2cbbd16 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java +++ b/operator/src/main/java/it/aboutbits/garage/core/BaseReconciler.java @@ -19,6 +19,7 @@ public abstract class BaseReconciler & Named, S extends CRStatus> { /// Bounds how long backend drift, which Kubernetes sees no events for, goes unnoticed. public static final long RESYNC_INTERVAL_MINUTES = 10; + public static final long RETRY_SECONDS = 60; protected abstract S newStatus(); @@ -66,7 +67,7 @@ public Optional getReferencedS3Connection( //noinspection ConstantConditions if (s3Connection == null) { log.error( - "The specified S3Connection does not exist [resource={}/{}]", + "The specified S3Connection does not exist [spec.connectionRef={}/{}]", connectionNamespace, connectionName ); @@ -80,7 +81,7 @@ public Optional getReferencedS3Connection( //noinspection ConstantConditions if (status == null) { log.warn( - "The specified S3Connection has not been probed yet [resource={}/{}]", + "The specified S3Connection has not been probed yet [spec.connectionRef={}/{}]", connectionNamespace, connectionName ); @@ -93,7 +94,7 @@ public Optional getReferencedS3Connection( if (!Objects.equals(currentPhase, expectedPhase)) { log.warn( - "The specified S3Connection is not ready yet [resource={}/{}, status.phase={}]", + "The specified S3Connection is not ready yet [spec.connectionRef={}/{}, status.phase={}]", connectionNamespace, connectionName, currentPhase @@ -105,7 +106,7 @@ public Optional getReferencedS3Connection( // READY of an older generation says nothing about the current spec. if (status.getObservedGeneration() != s3Connection.getMetadata().getGeneration()) { log.warn( - "The specified S3Connection changed and has not been probed again yet [resource={}/{}, status.observedGeneration={}, metadata.generation={}]", + "The specified S3Connection changed and has not been probed again yet [spec.connectionRef={}/{}, status.observedGeneration={}, metadata.generation={}]", connectionNamespace, connectionName, status.getObservedGeneration(), @@ -134,6 +135,6 @@ public UpdateControl handleError( .setMessage(exception.getMessage()); return UpdateControl.patchStatus(resource) - .rescheduleAfter(60, TimeUnit.SECONDS); + .rescheduleAfter(RETRY_SECONDS, TimeUnit.SECONDS); } } diff --git a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java index 43caa9f..3260bac 100644 --- a/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java +++ b/operator/src/main/java/it/aboutbits/garage/crd/garagecluster/GarageClusterService.java @@ -112,7 +112,7 @@ private void requireNotManagedElsewhere( var other = managedElsewhere.get(); throw new IllegalStateException( - "The Garage node is already managed by GarageCluster %s/%s [nodeId=%s]".formatted( + "The Garage node is already managed by GarageCluster %s/%s. Delete one of the two GarageClusters [nodeId=%s]".formatted( other.getMetadata().getNamespace(), other.getMetadata().getName(), nodeId diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java index 0839710..b66f55a 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/base/TestUtil.java @@ -1,7 +1,10 @@ package it.aboutbits.garage._support.testdata.base; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.KubernetesClient; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.CRStatus; import it.aboutbits.garage.crd.garagecluster.GarageCluster; import it.aboutbits.garage.crd.s3connection.S3Connection; import org.jspecify.annotations.NullMarked; @@ -12,6 +15,8 @@ @NullMarked public final class TestUtil { + public static final long AWAIT_SECONDS = 60; + public static void resetEnvironment(KubernetesClient kubernetesClient) { // Reverse Dependency Deletion deleteResource(kubernetesClient, S3Connection.class); @@ -31,6 +36,27 @@ public static void deleteResource( .until(() -> kubernetesClient.resources(resourceClass).list().getItems().isEmpty()); } + /// Waits for the phase at the current generation, so that a phase from before an edit does not count. + @SuppressWarnings("unchecked") + public static > R awaitPhase( + KubernetesClient kubernetesClient, + R resource, + CRPhase expectedPhase + ) { + //noinspection ConstantConditions + return kubernetesClient.resources((Class) resource.getClass()) + .inNamespace(resource.getMetadata().getNamespace()) + .withName(resource.getMetadata().getName()) + .waitUntilCondition( + item -> item != null + && item.getStatus() != null + && item.getStatus().getObservedGeneration() == item.getMetadata().getGeneration() + && item.getStatus().getPhase() == expectedPhase, + AWAIT_SECONDS, + TimeUnit.SECONDS + ); + } + private TestUtil() { } } diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java index 7a02f9b..a8c06cb 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/Given.java @@ -4,12 +4,16 @@ import it.aboutbits.garage._support.testdata.persisted.creator.GarageClusterCreate; import it.aboutbits.garage._support.testdata.persisted.creator.S3ConnectionCreate; import it.aboutbits.garage._support.testdata.persisted.creator.SecretKeyRefCreate; +import it.aboutbits.garage.core.CRPhase; +import it.aboutbits.garage.core.ResourceRef; import jakarta.enterprise.context.ApplicationScoped; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jspecify.annotations.NullMarked; +import static it.aboutbits.garage._support.testdata.base.TestUtil.awaitPhase; + @ApplicationScoped @RequiredArgsConstructor @NullMarked @@ -30,6 +34,39 @@ public GarageConnectionDetails garageConnectionDetails() { ); } + /// Apply a cluster layout, without which Garage serves no bucket or key calls at all. + public void appliedLayout() { + var garageCluster = one() + .garageCluster() + .returnFirst(); + + awaitPhase(kubernetesClient, garageCluster, CRPhase.READY); + } + + /// Bring the cluster up and return a ready connection in the default namespace. + public ResourceRef readyConnection() { + return readyConnection(kubernetesClient.getNamespace()); + } + + /// Bring the cluster up and return a ready connection in the given namespace. + public ResourceRef readyConnection(String namespace) { + appliedLayout(); + + var s3Connection = one() + .s3Connection() + .withNamespace(namespace) + .returnFirst(); + + awaitPhase(kubernetesClient, s3Connection, CRPhase.READY); + + var connectionRef = new ResourceRef(); + + connectionRef.setNamespace(s3Connection.getMetadata().getNamespace()); + connectionRef.setName(s3Connection.getMetadata().getName()); + + return connectionRef; + } + public One one() { return new One(this); } diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java index 419f35e..5791cdb 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/GarageClusterCreate.java @@ -8,7 +8,6 @@ import it.aboutbits.garage.crd.garagecluster.GarageCluster; import it.aboutbits.garage.crd.garagecluster.GarageClusterLayoutSpec; import it.aboutbits.garage.crd.garagecluster.GarageClusterSpec; -import lombok.AccessLevel; import lombok.Setter; import lombok.experimental.Accessors; import org.jspecify.annotations.NullMarked; @@ -28,9 +27,6 @@ public class GarageClusterCreate extends TestDataCreator { private @Nullable String withNamespace; - @Setter(AccessLevel.NONE) - private boolean withoutNamespace = false; - private @Nullable String withName; private @Nullable String withAdminEndpoint; @@ -55,11 +51,6 @@ public GarageClusterCreate( this.garageConnectionDetails = garageConnectionDetails; } - public GarageClusterCreate withoutNamespace() { - this.withoutNamespace = true; - return this; - } - @Override protected GarageCluster create(int index) { var namespace = getNamespace(); @@ -103,11 +94,7 @@ protected GarageCluster create(int index) { ); } - private @Nullable String getNamespace() { - if (withoutNamespace) { - return null; - } - + private String getNamespace() { if (withNamespace != null) { return withNamespace; } diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java index 036d140..0354b97 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/S3ConnectionCreate.java @@ -7,7 +7,6 @@ import it.aboutbits.garage.core.SecretKeyRef; import it.aboutbits.garage.crd.s3connection.S3Connection; import it.aboutbits.garage.crd.s3connection.S3ConnectionSpec; -import lombok.AccessLevel; import lombok.Setter; import lombok.experimental.Accessors; import org.jspecify.annotations.NullMarked; @@ -26,9 +25,6 @@ public class S3ConnectionCreate extends TestDataCreator { private @Nullable String withNamespace; - @Setter(AccessLevel.NONE) - private boolean withoutNamespace = false; - private @Nullable String withName; private @Nullable String withAdminEndpoint; @@ -47,11 +43,6 @@ public S3ConnectionCreate( this.garageConnectionDetails = garageConnectionDetails; } - public S3ConnectionCreate withoutNamespace() { - this.withoutNamespace = true; - return this; - } - @Override protected S3Connection create(int index) { var namespace = getNamespace(); @@ -88,11 +79,7 @@ protected S3Connection create(int index) { ); } - private @Nullable String getNamespace() { - if (withoutNamespace) { - return null; - } - + private String getNamespace() { if (withNamespace != null) { return withNamespace; } diff --git a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java index 2f97f13..3719ab8 100644 --- a/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java +++ b/operator/src/test/java/it/aboutbits/garage/_support/testdata/persisted/creator/SecretKeyRefCreate.java @@ -24,9 +24,6 @@ public class SecretKeyRefCreate extends TestDataCreator { private @Nullable String withNamespace; - @Setter(AccessLevel.NONE) - private boolean withoutNamespace = false; - private @Nullable String withName; private @Nullable String withKey; @@ -47,11 +44,6 @@ public SecretKeyRefCreate( this.kubernetesClient = kubernetesClient; } - public SecretKeyRefCreate withoutNamespace() { - this.withoutNamespace = true; - return this; - } - public SecretKeyRefCreate withoutToken() { this.withoutToken = true; return this; @@ -99,11 +91,7 @@ protected SecretKeyRef create(int index) { return secretKeyRef; } - private @Nullable String getNamespace() { - if (withoutNamespace) { - return null; - } - + private String getNamespace() { if (withNamespace != null) { return withNamespace; } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java index e74111c..11ebff3 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterReconcilerTest.java @@ -32,6 +32,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import static it.aboutbits.garage._support.testdata.base.TestUtil.AWAIT_SECONDS; +import static it.aboutbits.garage._support.testdata.base.TestUtil.awaitPhase; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowableOfType; import static org.awaitility.Awaitility.await; @@ -48,8 +50,6 @@ @RequiredArgsConstructor @NullMarked class GarageClusterReconcilerTest { - private static final long AWAIT_SECONDS = 60; - private final KubernetesClient kubernetesClient; private final Given given; @@ -94,25 +94,6 @@ void setUp() { TestUtil.resetEnvironment(kubernetesClient); } - /// Waits for the phase at the current generation, so that a phase from before an edit does not count. - private GarageCluster awaitPhase( - GarageCluster garageCluster, - CRPhase expectedPhase - ) { - //noinspection ConstantConditions - return kubernetesClient.resources(GarageCluster.class) - .inNamespace(garageCluster.getMetadata().getNamespace()) - .withName(garageCluster.getMetadata().getName()) - .waitUntilCondition( - item -> item != null - && item.getStatus() != null - && item.getStatus().getObservedGeneration() == item.getMetadata().getGeneration() - && item.getStatus().getPhase() == expectedPhase, - AWAIT_SECONDS, - TimeUnit.SECONDS - ); - } - private GarageCluster editLayout( GarageCluster garageCluster, Consumer edit @@ -139,7 +120,7 @@ void garageCluster_appliesLayout() { .returnFirst(); // when - var reconciled = awaitPhase(garageCluster, CRPhase.READY); + var reconciled = awaitPhase(kubernetesClient, garageCluster, CRPhase.READY); // then var status = reconciled.getStatus(); @@ -170,7 +151,7 @@ void reorderedTags_isNotReapplied() { .withTags(List.of("ssd", "fast")) .returnFirst(); - awaitPhase(garageCluster, CRPhase.READY); + awaitPhase(kubernetesClient, garageCluster, CRPhase.READY); var layoutVersion = garageAdminApi().getClusterLayout().version(); @@ -178,7 +159,7 @@ void reorderedTags_isNotReapplied() { var edited = editLayout(garageCluster, layout -> layout.setTags(List.of("fast", "ssd"))); // then - var reconciled = awaitPhase(edited, CRPhase.READY); + var reconciled = awaitPhase(kubernetesClient, edited, CRPhase.READY); assertThat(reconciled.getStatus().getLayoutVersion()).isEqualTo(layoutVersion); assertThat(garageAdminApi().getClusterLayout().version()).isEqualTo(layoutVersion); @@ -193,7 +174,7 @@ void changedCapacity_appliesNewLayoutVersion() { .withCapacity("20Gi") .returnFirst(); - awaitPhase(garageCluster, CRPhase.READY); + awaitPhase(kubernetesClient, garageCluster, CRPhase.READY); var layoutVersion = garageAdminApi().getClusterLayout().version(); @@ -201,7 +182,7 @@ void changedCapacity_appliesNewLayoutVersion() { var edited = editLayout(garageCluster, layout -> layout.setCapacity("30Gi")); // then - var reconciled = awaitPhase(edited, CRPhase.READY); + var reconciled = awaitPhase(kubernetesClient, edited, CRPhase.READY); assertThat(reconciled.getStatus().getLayoutVersion()).isGreaterThan(layoutVersion); @@ -226,7 +207,7 @@ void secondGarageCluster_setsErrorPhase() { .withCapacity("20Gi") .returnFirst(); - awaitPhase(first, CRPhase.READY); + awaitPhase(kubernetesClient, first, CRPhase.READY); var layoutVersion = garageAdminApi().getClusterLayout().version(); @@ -237,7 +218,7 @@ void secondGarageCluster_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(second, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, second, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains( "already managed by GarageCluster %s/%s".formatted( @@ -248,7 +229,7 @@ void secondGarageCluster_setsErrorPhase() { assertThat(reconciled.getStatus().getNodeId()).isNull(); assertThat(garageAdminApi().getClusterLayout().version()).isEqualTo(layoutVersion); - assertThat(awaitPhase(first, CRPhase.READY).getStatus().getNodeId()).isNotBlank(); + assertThat(awaitPhase(kubernetesClient, first, CRPhase.READY).getStatus().getNodeId()).isNotBlank(); } @Test @@ -266,7 +247,7 @@ void errorAfterReady_clearsHealth() { .returnFirst(); // precondition: the health is set while READY - assertThat(awaitPhase(garageCluster, CRPhase.READY).getStatus().getHealth()).isNotNull(); + assertThat(awaitPhase(kubernetesClient, garageCluster, CRPhase.READY).getStatus().getHealth()).isNotNull(); kubernetesClient.secrets() .inNamespace(adminSecretRef.getNamespace()) @@ -284,7 +265,7 @@ void errorAfterReady_clearsHealth() { var edited = editLayout(garageCluster, layout -> layout.setTags(List.of("edited"))); // then - var status = awaitPhase(edited, CRPhase.ERROR).getStatus(); + var status = awaitPhase(kubernetesClient, edited, CRPhase.ERROR).getStatus(); assertThat(status.getHealth()).isNull(); assertThat(status.getStorageNodes()).isZero(); @@ -307,7 +288,7 @@ void invalidToken_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()) .startsWith("The Garage Admin API request failed [status=403, code=") @@ -331,7 +312,7 @@ void missingSecret_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); } @@ -352,7 +333,7 @@ void missingSecretKey_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("missing the referenced key"); } @@ -368,7 +349,7 @@ void unreachableEndpoint_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(garageCluster, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, garageCluster, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).isNotBlank(); } @@ -411,5 +392,23 @@ void invalidCapacity_isRejected(String capacity) { assertThat(thrown.getCode()).isEqualTo(422); assertThat(thrown.getMessage()).contains("The capacity must be a Kubernetes quantity of whole bytes"); } + + @ParameterizedTest + @ValueSource(strings = {"", " "}) + @DisplayName("A blank zone should be rejected") + void blankZone_isRejected(String zone) { + // when + var thrown = catchThrowableOfType( + KubernetesClientException.class, + () -> given.one() + .garageCluster() + .withZone(zone) + .returnFirst() + ); + + // then + assertThat(thrown.getCode()).isEqualTo(422); + assertThat(thrown.getMessage()).contains("The zone must not be empty."); + } } } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java index 3f282ef..6e3c65d 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/garagecluster/GarageClusterServiceTest.java @@ -181,7 +181,7 @@ void managedElsewhere_isRefused() { // then assertThat(thrown) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("already managed by GarageCluster acme/other [nodeId=node-1]"); + .hasMessageContaining("already managed by GarageCluster acme/other. Delete one of the two GarageClusters [nodeId=node-1]"); thenNoLayoutIsWritten(); } diff --git a/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java index daecd80..7fd0d03 100644 --- a/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/garage/crd/s3connection/S3ConnectionReconcilerTest.java @@ -8,7 +8,6 @@ import it.aboutbits.garage.core.CRPhase; import it.aboutbits.garage.core.adminapi.GarageAdminApi; import it.aboutbits.garage.core.adminapi.GarageAdminClientFactory; -import it.aboutbits.garage.crd.garagecluster.GarageCluster; import lombok.RequiredArgsConstructor; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jspecify.annotations.NullMarked; @@ -27,6 +26,8 @@ import java.net.URI; import java.util.concurrent.TimeUnit; +import static it.aboutbits.garage._support.testdata.base.TestUtil.AWAIT_SECONDS; +import static it.aboutbits.garage._support.testdata.base.TestUtil.awaitPhase; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowableOfType; import static org.awaitility.Awaitility.await; @@ -41,8 +42,6 @@ @RequiredArgsConstructor @NullMarked class S3ConnectionReconcilerTest { - private static final long AWAIT_SECONDS = 60; - private final KubernetesClient kubernetesClient; private final Given given; @@ -85,49 +84,13 @@ void setUp() { TestUtil.resetEnvironment(kubernetesClient); } - private S3Connection awaitPhase( - S3Connection s3Connection, - CRPhase expectedPhase - ) { - //noinspection ConstantConditions - return kubernetesClient.resources(S3Connection.class) - .inNamespace(s3Connection.getMetadata().getNamespace()) - .withName(s3Connection.getMetadata().getName()) - .waitUntilCondition( - item -> item != null - && item.getStatus() != null - && item.getStatus().getPhase() == expectedPhase, - AWAIT_SECONDS, - TimeUnit.SECONDS - ); - } - - /// Make sure the cluster has a layout, so that it can actually serve requests. - private void givenAppliedLayout() { - var garageCluster = given.one() - .garageCluster() - .returnFirst(); - - //noinspection ConstantConditions - kubernetesClient.resources(GarageCluster.class) - .inNamespace(garageCluster.getMetadata().getNamespace()) - .withName(garageCluster.getMetadata().getName()) - .waitUntilCondition( - item -> item != null - && item.getStatus() != null - && item.getStatus().getPhase() == CRPhase.READY, - AWAIT_SECONDS, - TimeUnit.SECONDS - ); - } - @Nested class Probe { @Test @DisplayName("A connection to a serving cluster should become ready") void servingCluster_becomesReady() { // given - givenAppliedLayout(); + given.appliedLayout(); // when var s3Connection = given.one() @@ -135,7 +98,7 @@ void servingCluster_becomesReady() { .returnFirst(); // then - var reconciled = awaitPhase(s3Connection, CRPhase.READY); + var reconciled = awaitPhase(kubernetesClient, s3Connection, CRPhase.READY); assertThat(reconciled.getStatus().getMessage()).contains("Garage cluster"); } @@ -144,7 +107,7 @@ void servingCluster_becomesReady() { @DisplayName("An admin token that ends with a newline should be accepted") void tokenWithTrailingNewline_becomesReady() { // given - givenAppliedLayout(); + given.appliedLayout(); // As written by `echo "$TOKEN" > token.txt` and `kubectl create secret generic --from-file`. var adminSecretRef = given.one() @@ -159,7 +122,7 @@ void tokenWithTrailingNewline_becomesReady() { .returnFirst(); // then - awaitPhase(s3Connection, CRPhase.READY); + awaitPhase(kubernetesClient, s3Connection, CRPhase.READY); } } @@ -181,7 +144,7 @@ void invalidToken_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, s3Connection, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()) .startsWith("The Garage Admin API request failed [status=403, code=") @@ -205,7 +168,7 @@ void missingSecret_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, s3Connection, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).contains("Secret reference not found"); } @@ -221,7 +184,7 @@ void unreachableEndpoint_setsErrorPhase() { .returnFirst(); // then - var reconciled = awaitPhase(s3Connection, CRPhase.ERROR); + var reconciled = awaitPhase(kubernetesClient, s3Connection, CRPhase.ERROR); assertThat(reconciled.getStatus().getMessage()).isNotBlank(); } From 26a5b4f3ffc945a346603e81bf51992792f73a89 Mon Sep 17 00:00:00 2001 From: Simon Planinschek Date: Wed, 14 Oct 2026 11:23:45 +0200 Subject: [PATCH 6/6] address the last review of the GarageCluster and S3Connection resources Assisted-by: Claude:claude-opus-5-5 --- docs/s3-connection.md | 2 +- operator/build.gradle.kts | 1 + .../adminapi/GarageAdminClientFactory.java | 10 +- .../garage/core/KubernetesServiceTest.java | 215 ++++++++++++++++++ 4 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 operator/src/test/java/it/aboutbits/garage/core/KubernetesServiceTest.java diff --git a/docs/s3-connection.md b/docs/s3-connection.md index d3bbc63..0c486fa 100644 --- a/docs/s3-connection.md +++ b/docs/s3-connection.md @@ -38,7 +38,7 @@ spec: | Field | Type | Description | Required | Mutable | |------------------|------------------------------------------------|-------------------------------------------------------------------------------------------|----------|---------| | `adminEndpoint` | `string` | Base URL of the Garage **Admin** API, port 3903. This is not the S3 endpoint clients use. | Yes | No | -| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the Secret holding the admin credentials. | Yes | Yes | +| `adminSecretRef` | [`SecretKeyRef`](#secretkeyref-adminsecretref) | Reference to the `Opaque` Secret holding the Admin API bearer token. | Yes | Yes | ### SecretKeyRef (`adminSecretRef`) diff --git a/operator/build.gradle.kts b/operator/build.gradle.kts index ff2a590..41571ed 100644 --- a/operator/build.gradle.kts +++ b/operator/build.gradle.kts @@ -51,6 +51,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.awsSdk.s3) diff --git a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java index 590430d..d80ebaf 100644 --- a/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java +++ b/operator/src/main/java/it/aboutbits/garage/core/adminapi/GarageAdminClientFactory.java @@ -67,7 +67,7 @@ public GarageAdminApi create( defaultNamespace ); - var endpoint = parseEndpoint(adminEndpoint, adminSecretRef); + var endpoint = parseEndpoint(adminEndpoint, adminSecretRef, defaultNamespace); return clients.computeIfAbsent( new ClientKey(endpoint, token), @@ -107,14 +107,18 @@ private void close(GarageAdminApi garageAdminApi) { private URI parseEndpoint( String adminEndpoint, - SecretKeyRef adminSecretRef + SecretKeyRef adminSecretRef, + String defaultNamespace ) { try { return new URI(adminEndpoint); } catch (URISyntaxException e) { throw new IllegalArgumentException( - "The admin endpoint is not a valid URI [adminEndpoint=%s, adminSecretRef.name=%s]".formatted( + "The admin endpoint is not a valid URI [spec.adminEndpoint=%s, spec.adminSecretRef=%s/%s]".formatted( adminEndpoint, + adminSecretRef.getNamespace() != null + ? adminSecretRef.getNamespace() + : defaultNamespace, adminSecretRef.getName() ), e diff --git a/operator/src/test/java/it/aboutbits/garage/core/KubernetesServiceTest.java b/operator/src/test/java/it/aboutbits/garage/core/KubernetesServiceTest.java new file mode 100644 index 0000000..2986eae --- /dev/null +++ b/operator/src/test/java/it/aboutbits/garage/core/KubernetesServiceTest.java @@ -0,0 +1,215 @@ +package it.aboutbits.garage.core; + +import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; +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.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +@NullMarked +@EnableKubernetesMockClient(crud = true) +class KubernetesServiceTest { + private static final String NAMESPACE = "acme"; + private static final String DEFAULT_NAMESPACE = "default-ns"; + private static final String SECRET_NAME = "acme-garage"; + private static final String SECRET_KEY = "admin_token"; + + @SuppressWarnings("NullAway.Init") + static KubernetesClient client; + + private final KubernetesService kubernetesService = new KubernetesService(); + + @BeforeEach + void clearSecrets() { + client.secrets().inAnyNamespace().delete(); + } + + private static String base64(String value) { + return Base64.getEncoder().encodeToString(value.getBytes(StandardCharsets.UTF_8)); + } + + private static SecretKeyRef secretKeyRef(@Nullable String namespace) { + var secretKeyRef = new SecretKeyRef(); + + secretKeyRef.setNamespace(namespace); + secretKeyRef.setName(SECRET_NAME); + secretKeyRef.setKey(SECRET_KEY); + + return secretKeyRef; + } + + private static void givenSecret( + String namespace, + String type, + @Nullable Map data + ) { + var secret = new SecretBuilder() + .withNewMetadata() + .withNamespace(namespace) + .withName(SECRET_NAME) + .endMetadata() + .withType(type) + .withData(data) + .build(); + + client.secrets() + .inNamespace(namespace) + .resource(secret) + .create(); + } + + @Nested + class GetSecretRefToken { + @Test + @DisplayName("The token of an Opaque Secret should be returned") + void opaqueSecret_returnsToken() { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of(SECRET_KEY, base64("s3cret"))); + + // when + var token = kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE); + + // then + assertThat(token).isEqualTo("s3cret"); + } + + @Test + @DisplayName("A newline at the end of the token should be stripped") + void trailingNewline_isStripped() { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of(SECRET_KEY, base64("s3cret\n"))); + + // when + var token = kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE); + + // then + assertThat(token).isEqualTo("s3cret"); + } + + @Test + @DisplayName("A reference without a namespace should look in the default namespace") + void noNamespace_usesDefaultNamespace() { + // given + givenSecret(DEFAULT_NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of(SECRET_KEY, base64("s3cret"))); + + // when + var token = kubernetesService.getSecretRefToken(client, secretKeyRef(null), DEFAULT_NAMESPACE); + + // then + assertThat(token).isEqualTo("s3cret"); + } + + @Test + @DisplayName("A missing Secret should be refused") + void missingSecret_isRefused() { + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Secret reference not found [secret=acme/acme-garage]"); + } + + @Test + @DisplayName("A Secret of another type should be refused") + void wrongType_isRefused() { + // given + givenSecret(NAMESPACE, "kubernetes.io/basic-auth", Map.of(SECRET_KEY, base64("s3cret"))); + + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The Secret reference is of the wrong type [secret=acme/acme-garage, expectedType=Opaque, actualType=kubernetes.io/basic-auth]"); + } + + @Test + @DisplayName("A Secret without data should be refused") + void noData_isRefused() { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, null); + + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessage("The Secret reference has no data set [secret=acme/acme-garage]"); + } + + @Test + @DisplayName("A Secret with an empty data map should be refused") + void emptyData_isRefused() { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of()); + + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessage("The Secret reference has no data set [secret=acme/acme-garage]"); + } + + @Test + @DisplayName("A Secret without the referenced key should be refused, and name the keys it has") + void missingKey_isRefused() { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of("other", base64("value"))); + + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessage("The Secret reference is missing the referenced key [secret=acme/acme-garage, secret.key=admin_token, availableKeys=[other]]"); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\n"}) + @DisplayName("A blank token should be refused") + void blankToken_isRefused(String token) { + // given + givenSecret(NAMESPACE, KubernetesService.SECRET_TYPE_OPAQUE, Map.of(SECRET_KEY, base64(token))); + + // when + var thrown = catchThrowable( + () -> kubernetesService.getSecretRefToken(client, secretKeyRef(NAMESPACE), DEFAULT_NAMESPACE) + ); + + // then + assertThat(thrown) + .isInstanceOf(IllegalStateException.class) + .hasMessage("The Secret reference has a blank token [secret=acme/acme-garage, secret.key=admin_token]"); + } + } +}