Skip to content

fix: BCE-11514 - tell an empty keystore table apart from an empty cluster filter - #40

Merged
kenrick-g merged 2 commits into
mainfrom
fix/BCE-11515-empty-table-vs-empty-filter
Sep 7, 2026
Merged

kenrick-g merged 2 commits into
mainfrom
fix/BCE-11515-empty-table-vs-empty-filter

Conversation

@kenrick-g

@kenrick-g kenrick-g commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #38. Releases as 0.4.3.

0.4.2 refuses a zero-row read unconditionally, with --allow-no-keys as the opt-out. That flag has to track a signer's lifecycle: left on it disables the guard permanently, removed at the wrong moment the pod won't start.

Instead count_all_keys() counts the table ignoring any cluster predicate, which separates the two cases:

  • empty table — start with an empty keystore, as 0.4.1 did
  • rows present, none for this cluster — fail, since that means a wrong cluster id or keystore URL

--allow-no-keys is removed. A table with no client_cluster_id column behaves exactly as 0.4.1, since filtered and unfiltered are then the same read.

Verified: the new unit tests and two new real-PostgreSQL tests fail against 0.4.2 and pass here; 79 tests pass against a live PostgreSQL 16; mypy, ruff check and ruff format clean.

Chart users: skip 0.4.2. It stops a signer that legitimately serves no validators from starting.

The zero-row guard in 0.4.2 was unconditional, with --allow-no-keys as the way
out. That put the burden on an operator to declare which kind of empty a signer
is, and the flag has to be toggled in lockstep with the signer's lifecycle:
left on it permanently disables the guard for that namespace, and removed at the
wrong moment the pod will not start. Either way the protection is unreliable,
which makes the flag close to useless as a safety mechanism.

The two cases are distinguishable from the data instead. count_all_keys()
counts the table ignoring any cluster predicate:

  table empty                   -> nothing provisioned yet. Start with an empty
                                   keystore, which is what web3signer already
                                   does and what 0.4.1 did.
  rows present, none for my      -> this database holds keys, just not for the
  cluster                          cluster asked for. Almost always a wrong
                                   cluster id or a keystore URL pointing at
                                   another namespace, so refuse.

The keystore directory cannot answer this, because it is a tmpfs emptyDir and
is therefore empty on every pod start either way.

Consequences: QA's euw1/hoodi-2 needs no flag at all, and neither does a fresh
KOS database before keys are copied into it. The dangerous case now fails
whether or not anyone remembered a flag. --allow-no-keys stays only as a narrow
escape hatch for a signer that genuinely serves none of a populated table's
clusters, such as one of two co-located clusters being empty.

Verified: the new unit tests and the two new real-PostgreSQL tests all fail
against 0.4.2 and pass here; 79 tests pass with a live server; and run against
the real QA euw1/hoodi-2 database with no flags at all it prints "starting with
an empty keystore" and exits 0, writing nothing.
@kenrick-g
kenrick-g requested a review from a team as a code owner September 7, 2026 13:07
Under one KOS database per namespace the filtered read returns the same set an
unfiltered read would, so the refusal below it is unreachable for a correctly
configured signer. It can only fire when the cluster id or the keystore URL is
wrong, and those are not conditions to wave through -- the flag could only ever
mask a real misconfiguration while silently disabling the guard for that
namespace from then on.

Empty tables are already tolerated without it, which is the case that actually
occurs: QA's euw1/hoodi-2 has zero rows with web3signer running 3/3 ready, and a
fresh KOS database before the key copy is the same shape.

Verified: 79 tests pass against live PostgreSQL 16; the refusal is now proven
unoverridable by a test asserting the option is rejected; and re-run against the
real euw1/hoodi-2 database with no flags it prints "starting with an empty
keystore" and exits 0, writing nothing.
@kenrick-g
kenrick-g merged commit 1d054ee into main Sep 7, 2026
3 checks passed
@kenrick-g
kenrick-g deleted the fix/BCE-11515-empty-table-vs-empty-filter branch September 7, 2026 14:17
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.

2 participants