Skip to content

fix(noderesource): wire SEI_KEYRING_BACKEND; add gentx-key fallback path#296

Merged
bdchatham merged 5 commits into
mainfrom
fix/operator-keyring-backend-env-vars
May 20, 2026
Merged

fix(noderesource): wire SEI_KEYRING_BACKEND; add gentx-key fallback path#296
bdchatham merged 5 commits into
mainfrom
fix/operator-keyring-backend-env-vars

Conversation

@bdchatham
Copy link
Copy Markdown
Collaborator

@bdchatham bdchatham commented May 20, 2026

Summary

  • Default validators to a test-backend keyring on the data PVC. Any SeiNode with a Validator spec now gets SEI_KEYRING_BACKEND=test + SEI_KEYRING_DIR=$SEI_HOME — the SDK appends keyring-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.
  • .secret is now an override, not an opt-in. Operators who want a passphrase-locked, projected-Secret-backed keyring set .secret on validator.operatorKeyring. Symmetric with how signingKey and nodeKey already work (Secret-supplied or controller-generated).
  • Bug fix: SEI_KEYRING_DIR=$SEI_HOME/keyring-test was wrong. The Cosmos SDK appends keyring-<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_HOME on both branches and let the SDK append.
  • Bug fix: latent missing SEI_KEYRING_BACKEND on .secret path. The existing path emitted only SEI_KEYRING_PASSPHRASE, leaving the SDK on its compile-time default (os) which has no distroless analogue. Both branches now emit SEI_KEYRING_BACKEND and SEI_KEYRING_DIR explicitly.
  • Drop the (has(self.secret) ? 1 : 0) == 1 CEL rule on OperatorKeyringSource — the struct is now a container for an optional override, with semantics fully captured in Go branch logic and the docstring.

Companion PR

Behavior change for existing validators

Validators on main today fall into three buckets:

  1. .secret set: sidecar gains SEI_KEYRING_BACKEND=file + SEI_KEYRING_DIR=$SEI_HOME env 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).
  2. No .secret, validator with signingKey: sidecar gains SEI_KEYRING_BACKEND=test + SEI_KEYRING_DIR=$SEI_HOME env vars. The keyring will be empty until/unless the gentx task runs. Sign-tx tasks would fail with key not found, same as today's behavior (nothing was signing today either).
  3. Non-validator: unchanged.

Test plan

  • TestOperatorKeyring_SidecarContainerHasMountAndEnv updated to assert SEI_KEYRING_BACKEND=file and SEI_KEYRING_DIR=$SEI_HOME on the .secret path.
  • 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) and TestOperatorKeyring_SeidMainContainerHasNoMountOrEnv (trust-boundary guard) preserved.
  • make manifests generate clean; CRD YAML diff matches the docstring update.
  • Full go test ./... passes.
  • End-to-end: nightly major-upgrade workflow on harbor cluster signs gov-vote + gov-software-upgrade after both this PR and seictl#186 land.

🤖 Generated with Claude Code

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>
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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.

…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>
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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.

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>
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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 bdchatham force-pushed the fix/operator-keyring-backend-env-vars branch from 3ed09c6 to 0db0740 Compare May 20, 2026 02:27
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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>
@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

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.

@bdchatham bdchatham merged commit b41f6d5 into main May 20, 2026
2 checks passed
bdchatham added a commit that referenced this pull request May 20, 2026
Picks up:
- #296 fix(noderesource): wire SEI_KEYRING_BACKEND; add gentx-key fallback path
- #297 feat(nodetask): derive keyName from target SeiNode on gov tasks

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant