Settle what a cloud apply would tell us, without performing one - #48
Merged
Conversation
Nobody could have joined the cluster.
retry_join's leader_tls_servername is the one name a follower verifies the
leader against, whichever node holds leadership, so every node's
certificate has to carry it. Both cloud templates asked for
vault.<cluster>.internal. The PKI role issued <host>.vault.internal with
SANs <host>,localhost. That name appeared in exactly two files in the
repository and nothing issued a certificate for it, so every join would
have failed TLS verification -- and the symptom is a cluster that never
forms while each node reports healthy on its own.
The PKI role now derives vault_pki_cluster_servername from the cluster
name and includes it in every certificate's SANs.
Finding it by reading was luck. tests/preflight-static is the part meant
to outlast that: it checks the agreements no single layer can see, and it
needs no credentials and creates nothing.
- every value a template reads is one Terraform passes, and every value
Terraform passes is one a template reads
- the rendered cloud-init parses, and is shellchecked -- CI's shellcheck
step covers scripts/ and the harnesses, so these two scripts, which
boot every node, were linted nowhere
- every auto_join key is one go-discover accepts, in a combination it
accepts, against key sets recorded from the provider sources; and the
tag AWS filters on is one compute.tf actually sets
- the name the templates verify the leader against is a name the PKI
role issues, and the CA path they read is where Ansible writes it
Both historical bugs are caught by it: reverting the SAN fix turns one
assertion red, and reintroducing Azure's tag/scale-set mix turns another
red. Neither was visible to terraform validate, terraform test against
mocked providers, or any other suite here.
This settles configuration, not behaviour. Neither profile has been
applied and this does not change that -- it means the apply, when it
happens, is spent on questions only a real API can answer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tive The new suite failed the lint job it was written to extend. A comment line beginning with the word shellcheck is parsed as a directive, not as prose, so "# shellcheck step covers scripts/..." became SC1072/SC1073: expected '=' after the directive key. Reworded so the word does not open a line, and the reason is recorded in place -- it is not obvious, and the next person writing a comment about linting will hit it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first version of this fix would have been worse than the bug.
It gave every certificate a SAN of vault.<cluster>.internal, but
bootstrap-pki.sh sets the role's allowed_domains to vault_pki_domain with
allow_subdomains, and vault.vault-reference.internal is not under
vault.internal. Vault would have refused every issuance -- so instead of
failing only the Raft joins, nothing would have had a certificate at all.
The shared name is now <cluster>.<pki_domain>, which is a subdomain of
the domain the role allows, and both cloud templates follow it.
Adds the assertion that would have caught this: the cluster servername
has to sit inside vault_pki_domain, because a name every certificate must
carry is worth nothing if the role refuses to issue it. Reintroducing the
broken form turns it red, along with the agreement check.
Also silences SC2016 over render(), where the single quotes are the point
-- sed has to receive ${x} literally rather than the empty expansion the
shell would hand it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sethbergman
force-pushed
the
fix/leader-tls-servername-and-preflight
branch
from
August 28, 2026 10:21
2ee76ce to
424f5de
Compare
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.
You asked me to be sure there was nothing left to figure out before
spending. There was. Nobody could have joined the cluster.
The bug
retry_join'sleader_tls_servernameis the one name a followerverifies the leader against — whichever node holds leadership — so every
node's certificate has to carry it.
vault.<cluster>.internal<host>.vault.internal, SANs<host>,localhostvault.<cluster>.internalappears in exactly two files in the repository —the two cloud-init templates — and nothing issues a certificate for it.
Every Raft join would have failed TLS verification. The symptom is the one
this repo keeps producing: every node healthy, alone, cluster never
forms.
Fixed by deriving
vault_pki_cluster_servernamefrom the cluster name andincluding it in every certificate's SANs.
The part that matters more
Finding that by reading was luck.
tests/preflight-staticis the partmeant to outlast it — 22 assertions, no credentials, nothing created,
seconds to run:
step covers
scripts/and the harnesses, so the two scripts that bootevery node were linted nowhere
auto_joinkey is one go-discover accepts, in a combination itaccepts, against key sets recorded from the provider sources; and the tag
AWS filters on is one
compute.tfactually setsissues, and the CA path they read is where Ansible writes it
Mutation-checked against both historical bugs: reverting the SAN fix
turns one assertion red; reintroducing Azure's
tag_name/resource_groupmix turns another red. Neither is visible to
terraform validate,terraform testagainst mocked providers, or any other suite here.Also checked, and clean
Not everything I suspected was real, and the negatives are worth recording:
auto_joinkeys verified against go-discover'saws_discover.go—region/tag_key/tag_valueare correct, and the unsetaddr_typedefaults to
private_v4, which matches private subnetsleader_ca_cert_fileagrees withvault_tls_dironce resolvedScope
This settles configuration, not behaviour. Neither profile has been
applied and this does not change that. What it changes is that the apply,
when you pay for it, is spent on questions only a real API can answer —
not on a TLS name that was never going to work.
🤖 Generated with Claude Code