SV app: deprecate /v0/dso in favor of authenticated /v1/dso - #6957
SV app: deprecate /v0/dso in favor of authenticated /v1/dso#6957martinflorian-da wants to merge 23 commits into
/v0/dso in favor of authenticated /v1/dso#6957Conversation
/v0/dso remains available until 0.8.0 (deprecation announced in 0.5.5, DACH-NY/canton-network-internal#2106). Joining SVs now fetch DSO info from a configurable (sponsor) scan; without the new config they fall back to the sponsor SV app's /v0/dso, so existing configs keep working. Preflight tests that had no SV-app credentials read DSO info via the SV's scan instead. Verified: apps-sv + apps-app/Test compile, helm unittest (39/39), sv-frontend vitest (the two remaining full-suite failures reproduce on a clean tree under sandbox load), and SvOnboardingAddlIntegrationTest against local Canton with all joining SVs configured to fetch DSO info via sv1's scan (3/3 passed). Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…ack [ci] The sponsor-scan config (Helm: joinWithKeyOnboarding.sponsorScanUrl, enforced via 'required' + values schema) must now be set by any SV whose config still carries a join-with-key onboarding section; noted as breaking in the release notes. Also removes the now-unused /v0/dso Scala client command and SvConnection.getDsoInfo. Verified: apps-sv + apps-app/Test compile, helm unittest (39/39 incl. a missing-sponsorScanUrl failure test), SvOnboardingAddlIntegrationTest against local Canton with the scan-client now coming from the shared test config include (3/3 passed). Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…operator client DsoInfo and decodeDsoInfo now live in HttpSvOperatorAppClient, next to the only remaining command that returns them; HttpSvPublicAppClient no longer references DSO info at all. Verified via apps-sv + apps-app/Test compile. Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…n helper DsoInfo and its decoder now live in HttpScanAppClient (scan is the canonical server of the shared GetDsoInfoResponse schema); the scan console command decodes via the console environment's template decoder, so the preflight tests' bespoke ResourceTemplateDecoder helper is gone and scan clients are used directly. The raw GetDsoInfo command is kept for ScanConnection (BFT response comparison and the validator scan-proxy need the undecoded response). Verified: apps-scan/apps-sv/apps-app-Test compile and ScanIntegrationTest (incl. the retyped scan-vs-SV dso-info comparison) against local Canton, 7/7 passed. Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
[backport] ReminderPlease consider backporting to the following branches:
And your PR is currently against base branch: main. Note: Any PR comment containing [backport] will be considered for auto-backporting upon merge, |
/vo/dso in favor of authenticated /v1/dso/v0/dso in favor of authenticated /v1/dso
…nfo, old-version reads in AppUpgrade [ci] All four CI failures were direct consequences of this PR: - SvOnboardingConfigIntegrationTest / SvStateManagementIntegrationTest onboarded sv2 without any scan running; sv1's scan is now required onboarding infra. The sponsor-down test also stops the scan before sv2's restart, verifying restarts don't need it. - ValidatorIntegrationTest compared the scan-proxy's raw GetDsoInfoResponse with scan's now-typed DsoInfo; the scan-proxy console is now typed the same way (only the console used that command). - AppUpgradeIntegrationTest called getDsoInfo (now /v1, authenticated) against sv1 while it still runs the previous release; the party comes from sv1's wallet instead, and the post-upgrade amuletRules read uses sv1Backend. Verified locally against Canton: SvOnboardingConfigIntegrationTest (2/2), the fixed SvStateManagement and ValidatorIntegrationTest cases (1/1 each); AppUpgradeIntegrationTest needs CI's prepped release bundles. Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
|
/upgrade_test |
|
Deploy upgrade pipeline triggered for Commit 17e2a934a5b91191bcd52b84b7f3df8602170c43 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/79712 |
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
|
Failed because of So let's retry on latest main:
|
|
Deploy cluster test triggered for Commit af5354579d9cfcbd6c8358f3baa85df316a1a805 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/79798 |
The DsoPreflightIntegrationTest 403s came from the ActAsKnownParty check on /v1/dso: the SPLICE_OAUTH_DEV_CLIENT_ID_SV* client-credentials users are not provisioned as ledger users with the SV party as primary party (only the SV app's own user and the onboarded UI users are), so their tokens verify but fail authorization. PreflightAuthUtil had no prior users, so this was never exercised before. Since no externally available principal can read the SV apps' /v1/dso, the preflight now probes the SV apps' public readyz endpoint for liveness and reads SV parties from each SV's scan (added sv3Scan/svda1Scan remote client configs). The scan cannot vouch for the SV app's sv_user (it reports its own ledger user), so the SV-UI check now asserts the party only. Verified: apps-app/Test compile; behavior only exercisable against a deployed cluster. Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
|
/cluster_test |
|
Deploy cluster test triggered for Commit 9af44593e63cdb1bc38bdd558ab273c9e70ce180 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/79830 |
|
/upgrade_test |
|
Deploy upgrade pipeline triggered for Commit 9af44593e63cdb1bc38bdd558ab273c9e70ce180 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/79838 |
nicu-da
left a comment
There was a problem hiding this comment.
Thanks!
In an ideal world we wouldn't need this and just use a scan connection in the UI but as this is the SV app it's low return on improving it.
| bobValidatorWalletClient, | ||
| sv1WalletClient, | ||
| sv1Client.getDsoInfo().svParty, | ||
| PartyId.tryFromProtoPrimitive(sv1WalletClient.userStatus().party), |
There was a problem hiding this comment.
why can't we use the scan connection getDsoInfo?
There was a problem hiding this comment.
It is actually quite annoying to thread this through in this test; will try to clean it up with the next PR that can assume the new endpoint is already there...
| } | ||
|
|
||
| def getDsoInfo(): HttpSvPublicAppClient.DsoInfo = | ||
| def getDsoInfo(): HttpScanAppClient.DsoInfo = |
There was a problem hiding this comment.
Nit: This doesn't seem right, referencing the scan api from the sv app client,
There was a problem hiding this comment.
Fixed by larger refactoring of DsoInfo stuff
There was a problem hiding this comment.
FYI @nicu-da this ended up being a bigger refactoring but I'm sufficiently confident that the end-result is just way nicer than what we had to not bother you with another round of 👀; so if this blows up we can't blame the reviewer!
| @@ -1,5 +1,6 @@ | |||
| joinWithKeyOnboarding: | |||
| sponsorApiUrl: "https://sv.sv-2.TARGET_HOSTNAME" | |||
| sponsorScanUrl: "https://scan.sv-2.TARGET_HOSTNAME" | |||
There was a problem hiding this comment.
Lets make sure we check the docs as well to ensure they still make sense and cover this addition
Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
[ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
Falling through to the ScanConnection default meant consensus ran over the full DsoInfo, whose svUser/svParty differ per scan and never agree. Verified: new unit test reproduces the CI ConsensusNotReached failure without the fix and passes with it; full BftScanConnectionTest suite green. Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
…2106-dso-info-v2 [ci] Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
...and use the scan version of that endpoint for SV onboarding.
Both upgrade and basic cluster tests worked in the end.
Main part of https://github.com/DACH-NY/canton-network-internal/issues/2106
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./upgrade_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./lsu_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines