fix(noderesource): wire SEI_KEYRING_BACKEND; add gentx-key fallback path#296
Merged
Conversation
The existing .secret operator-keyring path emitted only SEI_KEYRING_PASSPHRASE,
leaving SEI_KEYRING_BACKEND unset — the Cosmos SDK then fell back to its
compile-time default ("os") which has no distroless analogue, so the sidecar
could not open the projected file-backend keyring. Set SEI_KEYRING_BACKEND=file
explicitly on the .secret path.
Add an empty-block opt-in to OperatorKeyringSource: when an operator sets
operatorKeyring: {} without .secret, the controller wires the sidecar with
SEI_KEYRING_BACKEND=test and SEI_KEYRING_DIR=$SEI_HOME/keyring-test — where
the seictl gentx task writes the validator key during the genesis ceremony.
Reusing the gentx key avoids requiring a per-deployment operator Secret on
ephemeral / bench chains where the data PVC is already the trust boundary.
Production chains continue to set .secret. The empty-block path on a prod
chain means sign-tx tasks fail at execution with "key not found" rather
than silently falling back to an unintended identity.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
2 tasks
…ix SEI_KEYRING_DIR path
Validators inherently want signing capability — the empty-block opt-in
(operatorKeyring: {}) was carrying zero information. Drop it: any SeiNode
with a Validator spec now gets keyring env vars by default, and .secret
becomes the override for the passphrase-locked / projected-Secret path
(symmetric with signingKey and nodeKey). Non-validators continue to get
no keyring env vars.
Fix the SEI_KEYRING_DIR value on both branches. The Cosmos SDK appends
"keyring-<backend>" to whatever rootDir we pass — so the prior
SEI_KEYRING_DIR=\$SEI_HOME/keyring-test made the SDK resolve
\$SEI_HOME/keyring-test/keyring-test/, where the gentx task never writes.
Pass \$SEI_HOME on both branches and let the SDK append the suffix; this
also makes the resolution behavior symmetric between file and test
backends.
Update OperatorKeyringSource docstring to describe the override semantic
(default test-backend, .secret overrides to file-backend). Drop project-
specific framing — this is general-purpose infrastructure. Tests
renamed and rescoped: validator-without-secret asserts the default
test-backend wiring; non-validator asserts no env vars.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
Closed
3 tasks
Reframe OperatorKeyringSource docstring around current behavior:
"with this field unset, the controller wires X" rather than
"validators sign by default; set .secret to override." Same semantics,
less change-framing.
Fix the off-by-one in operatorKeyringEnvVars's doc ("Two branches"
preceded a three-bullet list) and tighten the surrounding prose.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
1 similar comment
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
The default test-backend signing path rides on the sidecar both writing the operator key to and reading it from the data PVC. Make that invariant load-bearing in tests (assert the data mount exists, is at dataDir, and is not ReadOnly) and surface it on the mount declaration itself so a future "harden the sidecar" change doesn't silently flip the mount to read-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bdchatham
force-pushed
the
fix/operator-keyring-backend-env-vars
branch
from
3ed09c6 to
0db0740
Compare
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
…words Drop scaffolding and redundancy from the const docs, operatorKeyringEnvVars doc, OperatorKeyringSource docstring, test-func comments, and assertion messages. Same load-bearing content; fewer words. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
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.
Summary
SEI_KEYRING_BACKEND=test+SEI_KEYRING_DIR=$SEI_HOME— the SDK appendskeyring-test/, which is the same path the seictl gentx task writes the operator key to during the genesis ceremony. This unlocks gov-vote and gov-software-upgrade tasks on any validator without requiring a per-deployment Secret. Non-validators get no env vars..secretis now an override, not an opt-in. Operators who want a passphrase-locked, projected-Secret-backed keyring set.secretonvalidator.operatorKeyring. Symmetric with howsigningKeyandnodeKeyalready work (Secret-supplied or controller-generated).SEI_KEYRING_DIR=$SEI_HOME/keyring-testwas wrong. The Cosmos SDK appendskeyring-<backend>to whatever rootDir we pass — the prior value made the SDK resolve$SEI_HOME/keyring-test/keyring-test/, where the gentx task never writes. Pass$SEI_HOMEon both branches and let the SDK append.SEI_KEYRING_BACKENDon.secretpath. The existing path emitted onlySEI_KEYRING_PASSPHRASE, leaving the SDK on its compile-time default (os) which has no distroless analogue. Both branches now emitSEI_KEYRING_BACKENDandSEI_KEYRING_DIRexplicitly.(has(self.secret) ? 1 : 0) == 1CEL rule onOperatorKeyringSource— the struct is now a container for an optional override, with semantics fully captured in Go branch logic and the docstring.Companion PR
validator→node_adminso it matches the controller's CRD default (SecretOperatorKeyringSource.KeyName). Without chore: bump controller image to 374f615 (fork-genesis revert) #186, the test-backend default works only if scenario authors passKEY_NAME=validatorper task; with chore: bump controller image to 374f615 (fork-genesis revert) #186, both controller paths (test/file) produce the same identity name and scenarios just usenode_admineverywhere.Behavior change for existing validators
Validators on main today fall into three buckets:
.secretset: sidecar gainsSEI_KEYRING_BACKEND=file+SEI_KEYRING_DIR=$SEI_HOMEenv vars. PodSpec hash changes → StatefulSet rolls the pod. This is the latent bug-fix path; the sidecar's signing flow has been broken since feat: SeiNode validator.operatorKeyring CRD surface (closes #219) #220 because the SDK couldn't resolve a backend. Worth a coordinated rollout (one cluster at a time)..secret, validator withsigningKey: sidecar gainsSEI_KEYRING_BACKEND=test+SEI_KEYRING_DIR=$SEI_HOMEenv vars. The keyring will be empty until/unless the gentx task runs. Sign-tx tasks would fail withkey not found, same as today's behavior (nothing was signing today either).Test plan
TestOperatorKeyring_SidecarContainerHasMountAndEnvupdated to assertSEI_KEYRING_BACKEND=fileandSEI_KEYRING_DIR=$SEI_HOMEon the.secretpath.TestOperatorKeyring_ValidatorWithoutSecret_TestBackend(new) asserts the default test-backend wiring for validators without.secret.TestOperatorKeyring_NonValidator_NoEnv(new) guards the non-validator branch.TestOperatorKeyring_Unset_NoVolumeOrMount(snapshot node, no validator) andTestOperatorKeyring_SeidMainContainerHasNoMountOrEnv(trust-boundary guard) preserved.make manifests generateclean; CRD YAML diff matches the docstring update.go test ./...passes.🤖 Generated with Claude Code