chore: [ACC-3193] always on HA mode for universal broker#1088
Open
gclapperton wants to merge 1 commit intomasterfrom
Open
chore: [ACC-3193] always on HA mode for universal broker#1088gclapperton wants to merge 1 commit intomasterfrom
gclapperton wants to merge 1 commit intomasterfrom
Conversation
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
gclapperton
force-pushed
the
feat/remove-ha-mode-flag
branch
from
2279f03 to
0740838
Compare
gclapperton
force-pushed
the
feat/remove-ha-mode-flag
branch
from
0740838 to
c430576
Compare
gclapperton
commented
Mar 19, 2026
| expect(httpChecks[0].enabled).toEqual(true); | ||
| }); | ||
|
|
||
| it('should return false for rest-api-status.enabled if high availability mode is off', async () => { |
Contributor
Author
There was a problem hiding this comment.
This test appears to have been broken as it asserts that enabled is true
gclapperton
force-pushed
the
feat/remove-ha-mode-flag
branch
3 times, most recently
from
dacadcc to
356ed26
Compare
gclapperton
force-pushed
the
feat/remove-ha-mode-flag
branch
6 times, most recently
from
010a521 to
e85b06c
Compare
…A_MODE_ENABLED Universal broker clients now always call getServerId() when establishing websocket connections. The BROKER_HA_MODE_ENABLED flag and its conditional gate in socket.ts are removed — getServerId() should always run for universal broker. Legacy clients are unaffected; createWebSocketConnectionPairs is only invoked when universalBrokerEnabled is true. Test infrastructure: introduce TestApiServer (test/setup/test-api-server.ts), a dedicated mock of api.snyk.io that handles dispatcher allocation and webhook fallback endpoints. Previously tests pointed API_BASE_URL at the broker server or web server, which broke once getServerId() became unconditional. All universal broker functional tests now spin up a TestApiServer alongside the broker server and web server, and webhook routes that belong to the API layer are moved out of TestWebServer.
gclapperton
force-pushed
the
feat/remove-ha-mode-flag
branch
from
571c337 to
9acbfa7
Compare
gclapperton
changed the title
gclapperton
changed the title
gclapperton
changed the title
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
1 participant
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.
What changed
getServerId()— which registers this client connection with the Snyk dispatcher and receives back aserver_idused to route websocket traffic in multi-pod deployments — is now called unconditionally when a universal broker client establishes a websocket connection. Previously it was gated behindBROKER_HA_MODE_ENABLED == 'true'. The flag and both conditional blocks insocket.tsare removed.BROKER_HA_MODE_ENABLEDis also removed from allconfig.universaltest*.jsonfiles and from the relevant unit test snapshots inconfig.test.ts.Legacy clients are not affected —
createWebSocketConnectionPairsis only called whenuniversalBrokerEnabledis true.Why
Universal broker works in both single-pod and multi-pod deployments, but we want to make scaling to multiple pods as seamless as possible. Since single-pod deployments can participate in server-side routing (HA mode) without any downside, we default to it — so that customers going from one pod to many only need to increase their replica count, with no config changes required.
Test infrastructure
With
getServerId()now unconditional, every test that starts a universal broker client makes a dispatcher request againstAPI_BASE_URL. Previously tests were pointing that variable at the broker server or theTestWebServer, neither of which speaks the dispatcher API, causing those calls to fail or hit unexpected routes.This PR introduces
TestApiServer(test/setup/test-api-server.ts): a lightweight Express mock ofapi.snyk.iothat handles the two dispatcher endpoints (/hidden/broker/…and/hidden/brokers/…) and the webhook fallback endpoints that the broker client posts to when the websocket is down. All universal broker functional tests now start aTestApiServeralongside the broker server and web server. The webhook routes that were previously onTestWebServerare removed from there and live only onTestApiServer.