helm: make the Postgres role and bootstrap database configurable for SV charts - #7069
Open
SLoeuillet wants to merge 1 commit into
Open
helm: make the Postgres role and bootstrap database configurable for SV charts#7069SLoeuillet wants to merge 1 commit into
SLoeuillet wants to merge 1 commit into
Conversation
…SV charts Follow-up to canton-network#6803, which applied the same change to the validator and participant charts. This covers what was deliberately left out of it: splice-sv-node, splice-scan, splice-global-domain (sequencer and mediator), splice-domain, and splice-splitwell-app. As in canton-network#6803, persistence.user and persistence.bootstrapDatabaseName replace the hardcoded cnadmin role and cantonnet bootstrap database, defaulting to those same values so rendered output is byte-identical unless set. `rg -- '--dbname=cantonnet|--username=cnadmin' cluster/helm/` now returns nothing across the whole tree. The domain, mediator and sequencer needed the same second half as the participant did: canton-base's storage.conf reads `user = ${?CANTON_DOMAIN_POSTGRES_USER}` and the charts never set it, so the node kept connecting as cnadmin even when persistence.user was configured. The sequencer's cantonbft-driver init container also connected to a hardcoded bootstrap database while already deriving host, port and user from values; it now derives the database the same way. splitwell is included here rather than in canton-network#6803 because it is neither a validator nor an SV component, so it belongs with the batch that waits for the new test coverage rather than with the one merged on the strength of the validator testing. The release-notes entry from canton-network#6803 is extended rather than duplicated, since both ship in the same release. Signed-off-by: Stephane Loeuillet <stephane.loeuillet@kaiko.com>
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.
Follow-up to #6803, as agreed there. That PR was cut down to validator + participant so the validator use case could land early; this carries the parts that were held back.
No rush on merging this one. You said you would not merge the SV charts without cluster test coverage for SV deployments, and that such coverage is planned as part of a security-hardening story. This is here so the work is not lost and is ready when that lands — review it whenever it suits you.
What it covers
splice-sv-node,splice-scan,splice-global-domain(sequencer + mediator),splice-domain, andsplice-splitwell-app.Same shape as #6803:
persistence.userandpersistence.bootstrapDatabaseNamereplace the hardcodedcnadminrole andcantonnetbootstrap database, defaulting to those values so rendered output is byte-identical unless you set them.With this,
rg -- '--dbname=cantonnet|--username=cnadmin' cluster/helm/returns nothing across the whole tree.The node-level half
The domain, mediator and sequencer needed the same second fix the participant did:
canton-base'sstorage.confreadsuser = ${?CANTON_DOMAIN_POSTGRES_USER}and the charts never set it, so the node kept connecting ascnadmineven withpersistence.userconfigured — the half-applied configuration we discussed on #6803.The sequencer's cantonbft-driver init container was a second instance of the original problem: it already derived host, port and user from values but connected to a hardcoded bootstrap database. It now derives that the same way, via a
$dbBootstraptemplate variable exposed asDB_BOOTSTRAP, matching the file's own idiom rather than inlining an expression into the shell script.Why splitwell is here
It is neither a validator nor an SV component, so it seemed to belong with the batch waiting for new coverage rather than with the one you merged on the strength of the validator testing. Happy to move it if you would rather have had it in #6803.
Tests
Two tests on the global-domain mediator assert
persistence.userreaches the node's env and that nothing is emitted when unset; both were checked to fail without the change. Every touched chart's suite gives the same results asmain.The release-notes entry from #6803 is extended rather than duplicated, since both ship in the same release.
Testing this on a cluster
For whenever the SV coverage exists, the setup that made the #6803 validation meaningful was initialising Postgres with
POSTGRES_USER=appuser/POSTGRES_DB=bootstrapdbso thatcnadminandcantonnetdo not exist — otherwise the test passes whether or not the values are honoured. The unpatched charts then fail loudly withpassword authentication failed for user "cnadmin", which is what makes it a real check.