fix!: adapt the Hub client to the FLAME Hub 0.13.0 API - #275
Open
tada5hi wants to merge 4 commits into
Open
Conversation
The FLAME Hub renamed its entity/HTTP-API vocabulary from snake_case to camelCase (PrivateAIM/hub#1806) and wrapped every entity record response in a data/meta envelope (PrivateAIM/hub#1801). Both ship in 0.13.0. Its OAuth2/OIDC parameters stay snake_case. Every one of these fails silently rather than loudly, which is why they are worth enumerating: - HttpHubClient sent filter[analysis_id] and filter[client_id]. rapiq prunes an unknown filter key instead of rejecting it, so against 0.13.0 the analysis-node lookup would have answered with EVERY analysis node the client can see rather than the analysis's, and the broker would have treated unrelated nodes as participants of the analysis. The node lookup would have degraded to NoMatchingNodeFoundException once more than one node existed. - Node and AnalysisNode carry @JsonIgnoreProperties(ignoreUnknown = true), so a stale @JsonProperty deserializes to null rather than throwing. public_key -> publicKey in particular would have turned every message into NoPublicKeyException, disabling end-to-end encryption for the whole node. Both HubClient calls target collection endpoints, whose data/meta envelope predates this release, so HubResponseContainer is unchanged. The system-test provisioning script creates its fixtures through the Hub and authup REST APIs and read the created id straight off the response body, which is now nested under data. Both request bodies and response reads move: the six setup templates to camelCase (including the authup client account, since authup renamed in lockstep), and `jq -r '.id'` to `jq -r '.data.id // empty'`. The `// empty` matters — without it jq emits the string "null" for a missing key, which the script's `[ -z ... ]` guards accept as success and then propagate as a literal "null" id into every later request. The OAuth2 token surface is untouched: HubOIDCAuthenticator's client_id form parameter and the system tests' equivalent stay snake_case, as does .access_token. BREAKING CHANGE: requires FLAME Hub 0.13.0 or newer.
📝 WalkthroughWalkthroughHub integrations and system-test resources now use camelCase property and filter names. Hub setup parsing reads resource IDs from wrapped ChangesHub contract alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The stack pinned ghcr.io/privateaim/hub:0.8.31 while pulling authup/authup
unpinned, so `latest` eventually moved to a release 0.8.31 could not talk to and
the Hub core container stopped becoming healthy — the last green run was
2026-06-20. Bump both Hub images to 0.13.0 and pin authup to 1.0.0-beta.57, the
version Hub 0.13.0 declares across its @authup/* dependencies.
Two further changes are needed before the stack comes up on 0.13.0:
- HARBOR_URL was passed as `${HARBOR_URL:-}`, i.e. an empty string. 0.13.0
validates harborURL as a URL when the key is present, so the empty value now
aborts startup with `Property harborURL is invalid`. Harbor is unused here, so
leave the variable unset rather than empty.
- Since 0.13.0 the authorization middleware genuinely enforces permissions,
where it previously ran in dry-run mode, and the Hub does not register its
permission names with authup itself. Without them every write fails with
`The project_create permission could not be resolved`. Mount a provisioning
module declaring all 30 PermissionName values, which authup's built-in `admin`
role resolves through its `globalPermissions: ['*']`; this mirrors
buildAuthupProvisioning() in @privateaim/server-test-kit.
Verified by bringing the stack up locally: core reaches `healthy` and
setup-hub-resources.sh completes end to end, creating the clients, project,
analysis and all three nodes, project nodes and analysis nodes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
system-tests/environment/hub/hub-docker-compose.yml (1)
89-92: 🩺 Stability & Availability | 🔵 TrivialVerify the external network’s IPAM before relying on the static address.
Because
hub-node-intercommis declaredexternal, this file does not create its subnet or reserve addresses. A clean environment will fail if the network is absent, does not cover172.99.20.11, or already assigns that address. Ensure provisioning creates the network with compatible IPAM and reserves.10–.12.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@system-tests/environment/hub/hub-docker-compose.yml` around lines 89 - 92, Update the provisioning for the external hub-node-intercomm network to create it when absent with IPAM covering 172.99.20.11, reserve addresses 172.99.20.10 through 172.99.20.12, and validate existing networks before Docker Compose relies on the static address in hub-node-intercomm.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@system-tests/environment/hub/hub-docker-compose.yml`:
- Around line 89-92: Update the provisioning for the external hub-node-intercomm
network to create it when absent with IPAM covering 172.99.20.11, reserve
addresses 172.99.20.10 through 172.99.20.12, and validate existing networks
before Docker Compose relies on the static address in hub-node-intercomm.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a92a8f73-2704-4fce-a248-7dc790a01814
📒 Files selected for processing (2)
system-tests/environment/hub/authup-provisioning.mjssystem-tests/environment/hub/hub-docker-compose.yml
…ethod
The node clients are created through authup's REST API and then authenticate
with the client-credentials grant to reach the Hub. Under authup 1.0.0-beta.57
the template no longer produces a usable client: `isConfidential` is gone from
the client read model and no longer implies secret-based authentication, so the
account was created with `authMethod: "none"` and `grantTypes: null`, and every
token request came back `invalid_client`. The broker could then never obtain a
Hub token, and answered `GET /analyses/{id}/participants/self` with 502.
Declare `authMethod: "secret"` and `grantTypes: "client_credentials"` instead,
which is what `isConfidential: true` used to stand for.
Verified against the local stack: after a clean provisioning run the node A
client obtains a token with no manual patching, and both queries the broker
makes succeed with it — `/analysis-nodes?filter[analysisId]` returns the
analysis's 3 nodes and `/nodes?filter[clientId]` returns exactly 1, carrying a
non-null publicKey. Note that the Hub's read endpoints are not permission-gated,
so the node clients need no role or permission binding; only writes are, and
those are made with the admin token.
The messenger container crash-looped on 0.13.0 and nothing relayed, which is why the send/receive test reported `number of sent messages (10) and received messages (0) differs`. Two independent gaps, both surfaced by running the stack: - `The database configuration could not be read from env variables.` The messenger gained a durable mailbox and now needs a datasource, where 0.8.31 was purely a stateless relay. Point it at the same MySQL as core with its own `messenger` database, and make it wait for MySQL to be healthy. - Once it started, every socket handshake was rejected with `Property token is invalid`. The socket authorization middleware introspects the incoming token through an authup client token creator built from `CLIENT_ID`, `CLIENT_SECRET` and `REALM` — which core is given and the messenger was not. Verified against the local stack with two node clients connected to the messenger over socket.io: A emits `send` addressed to B's client id and B receives it, `from` resolving to A's client identity. Both changes are required; with either missing the relay stays silent rather than erroring, since the sender's acknowledgement callback still reports success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FLAME Hub 0.13.0 landed two breaking changes together:
snake_case→camelCaseacross every entity, HTTP field and rapiq query key{ data, meta }The broker's Hub surface is small — two
GETs and two DTOs — but every one of these failures is silent. That is the reason for the PR rather than the size of the diff.The rapiq filter keys
HttpHubClientsentfilter[analysis_id]andfilter[client_id]. rapiq prunes an unknown filter key instead of rejecting it, so against 0.13.0:fetchAnalysisNodes→GET /analysis-nodes?filter[analysis_id]=…fetchPublicKey→GET /nodes?filter[client_id]=…resp.data.size() != 1→NoMatchingNodeFoundExceptionas soon as more than one node existsNo 4xx in either case. The first one looks like success.
The DTOs
NodeandAnalysisNodeare@JsonIgnoreProperties(ignoreUnknown = true), so a stale@JsonPropertydeserializes tonullrather than throwing:Node.public_key→publicKey. This is the one that matters: a null public key meansNoPublicKeyExceptionon every send, i.e. end-to-end encryption off for the whole node.Node.client_id→clientId— participant identity and thediscoverSelfcomparison.AnalysisNode.node_id→nodeId(the Java field is renamed alongside the annotation; it has no readers).HubResponseContaineris unchanged: bothHubClientcalls target collection endpoints, whose{ data, meta }envelope long predates this release. The Hub's record envelope does not reach this client.The integration tests build their stub bodies by serialising these same DTOs through Jackson, so only the four query-key assertions in
HttpHubClientITneeded touching.System tests
system-tests/environment/resources/hub/setup-hub-resources.shprovisions its fixtures through the Hub and authup REST APIs, so it is affected on both sides:*-setup-template.jsonfiles move to camelCase. That includesclient-account-setup-template.json, which posts to authup (secret_hashed→secretHashed,is_confidential→isConfidential): authup renamed in lockstep. Hub's validator drops an unknown body key silently, so a stalerealm_idwould have created the entity with no realm rather than erroring.jq -r '.id'→jq -r '.data.id // empty'at all 14 record-creating calls. The two collection reads (.data[0].id) already matched the envelope and just gained the same guard.The
// emptyis load-bearing. Without itjq -r '.data.id'emits the string"null"for a missing key, which the script'sif [ -z "${X}" ]guards accept as success and then propagate as a literalnullid into every subsequent request. Verified:Deliberately not renamed
The OAuth2 surface is spec-frozen and stays snake_case —
HubOIDCAuthenticator'sclient_idform parameter, the same insystem-tests/tests/message-broker/auth.go, andjq -r '.access_token'. Also untouched: the shell variable names insetup.sh, the keycloak realm export, and the socket.io message DTOs (HubMessageMetadata,HubMessageSender,HubMessageRecipient), which were already camelCase.The system-test environment had to move too (commits 2–5)
Run System Testswas already failing before this PR, and for a related reason. The stack pinnedghcr.io/privateaim/hub:0.8.31but pulledauthup/authupunpinned, solatestdrifted onto a release 0.8.31 cannot talk to and Hub core stopped becoming healthy. The last green CI run was 2026-06-20.Five things were needed to get the stack working on 0.13.0, each found by running it rather than by reading source:
0.13.0; authup pinned to1.0.0-beta.57, the version Hub 0.13.0 declares across its@authup/*dependencies.HARBOR_URL: ${HARBOR_URL:-}removed. 0.13.0 validatesharborURLas a URL when the key is present, and the empty string aborts startup withProperty harborURL is invalid. Harbor is unused here, so it is better left unset than empty.The project_create permission could not be resolved. A newauthup-provisioning.mjsdeclares all 30PermissionNamevalues and is mounted at/usr/src/app/writable/provisioning/hub.mjs; authup's built-inadminrole resolves them via itsglobalPermissions: ['*']. This mirrorsbuildAuthupProvisioning()in@privateaim/server-test-kit.isConfidentialis gone from the client read model and no longer implies secret-based authentication, so the template produced clients withauthMethod: "none"andgrantTypes: null. Every client-credentials request returnedinvalid_client, the broker could never obtain a Hub token, andGET /analyses/{id}/participants/selfanswered 502 — exactly what CI showed. The template now declaresauthMethod: "secret"andgrantTypes: "client_credentials".sent (10) and received (0) differs. The messenger container was crash-looping the whole time — nothing in the harness waits on it, only on core. Two separate gaps: 0.13.0's messenger gained a durable mailbox and now needs a datasource where 0.8.31 was a stateless relay (The database configuration could not be read from env variables.), and once it booted, every socket handshake was rejected withProperty token is invalidbecause its authorization middleware introspects incoming tokens through an authup client token creator built fromCLIENT_ID/CLIENT_SECRET/REALM, which core is given and the messenger was not.Two things worth noting for review:
{to:[{type,id}], data, metadata}payload and theclient-connection:${id}room key are identical between the two Hub versions, and the messenger DTOs were already camelCase. Every messenger problem here was environment configuration.Verification
I brought the stack up locally against Hub 0.13.0 and iterated on it until it worked. Core reaches
healthy, andsetup-hub-resources.shcompletes end to end — three clients, project, analysis, three nodes, three project nodes, three analysis nodes. After a clean provisioning run the node A client obtains a token with no manual patching, and both queries the broker makes succeed with it (/analysis-nodes→ the analysis's 3 nodes;/nodes?filter[clientId]→ exactly 1, carrying a non-nullpublicKey).Both of the broker's queries were also exercised old key versus new, using the admin token:
That first line is the confidentiality issue, demonstrated rather than argued: a deliberately non-existent analysis id still returned all three analysis nodes.
The
include=nodepayload also confirms the DTO annotations — the relation carriesclientId,publicKey,robotId,externalName,realmId,registryId,registryProjectId,createdAt,updatedAt.The messenger relay was verified separately, with two node clients connected to it over socket.io: A emits
sendaddressed to B's client id and B receives it,fromresolving to A's client identity. Both messenger changes are required — with either missing the relay stays silent rather than erroring, because the sender's acknowledgement callback still reports success, the same failure shape as a pruned rapiq filter.Not verified locally:
mvn verify, and the node half of the system tests. There is no JVM or Maven on the machine I worked from, and the node compose builds its image fromtarget/*.jar, so that half is only exercised by CI. The Java changes are annotation strings and query-parameter strings; no control flow moved.Ordering
Independent of the other 0.13.0 follow-ups (hub-python-client#129, node-ui#401) — this repo talks to the Hub directly. It must land with the 0.13.0 rollout, though: the current
developand 0.13.0 cannot interoperate in either direction.