[isaactwong]

Adding spec.ingress.sql.host to a running secure cluster regenerates the node certificate to include the new host, but the pods keep serving the old certificate. verify-full clients connecting through the ingress fail hostname verification until something unrelated restarts the pods.

Root cause

The certificate actor made two decisions from two different inputs. generateNodeCert regenerated an existing certificate when status.sqlHost differed from the spec. Act requested a rolling restart when the CrdbSQLIngressExposed condition was true. The ingress actor sets that condition, and it runs after the certificate actor, so on the first exposure the condition was still false: the certificate was reissued, no restart was requested, and pods, which copy certificates into an emptyDir once at init, never saw it. Changing an already-exposed host worked only because the condition was true by then.

Fix

generateNodeCert now reports whether it replaced a certificate that a ready secret already held, and Act requests the rolling restart from that fact. One decision, one input. First generation has no pods to restart and requests none. Any future reason to regenerate a live certificate gets the restart without a second decision to keep in sync.

Commits

1. Test whether regenerating the node certificate requests a restart. Drives the actor against a fake API server with real certificate generation and records today's behavior: certificate regenerated, no restart. Passes on master. Adds the //hack/bin:cockroach data dependency and a TestMain to the actor tests, the same pattern pkg/security uses.
2. Request a restart whenever a live node certificate is regenerated. The fix. Flips the test and adds: a no-change run with the ingress condition true requests nothing; a later host change still restarts; a cluster created with the ingress host already in its spec requests no restart on first generation. Reinstating the old condition check in OR form fails the no-change assertion, which was verified by trying it.

Behavior changes

• A rolling restart is requested on the first SQL ingress exposure and on any regeneration of a live node certificate, including after the ingress is removed and re-added.
• A run that regenerates nothing no longer requests a restart. The old check re-set the annotation on every certificate-actor run with the ingress exposed. This was mostly latent, since the director dispatches the actor only on first generation or a host change.
• The status.sqlHost write is now guarded on the object whose host was baked into the certificate. The old guard checked the freshly fetched object and could dereference a nil ingress on the original if the ingress was added mid-run.

Known limitation

The proxy for "pods may hold this certificate" is a ready node secret. A node secret deleted by hand and regenerated during a host change reports no replacement, so pods keep the old certificate until something else restarts them. The director does not route to the certificate actor for a deleted secret alone, so this needs a deletion and a host change in the same window.

For reviewers

The annotation is honored by the restart actor only when the ClusterRestart and CrdbVersionValidator feature gates are on (both default on) and the version has been checked. With a gate off, the annotation sits unconsumed, exactly as it did for the old condition-based path. Separately, the restart actor logs but does not propagate a failure to clear the annotation after a restart, which can cause a second roll; pre-existing, left for a follow-up.

Testing

bazel test //pkg/actor:go_default_test passes on both commits. The new tests shell out to the real cockroach cert command, provided as bazel test data. No e2e covers ingress.

Fixes #1168

Epic: none

🤖 Generated with Claude Code

---

Part of a sweep of fixes for one bug shape: two code paths that should build the same thing from the same inputs were written as separate copies and drifted. Each PR in the set follows one arc: the first commit adds a test that exercises both paths side by side and records today's divergence (it passes on master); the second commit makes the paths share one definition and flips the test to equality.

Siblings: cockroach-operator #1167, cockroach-operator #1172, helm-charts #689, helm-charts #691.

[cockroachlabs-cla-agent]

All committers have signed the CLA.

[NishanthNalluri]

Could the changelog entry also note that adding ingress.sql.host to a running cluster now triggers a rolling restart, and a note saying that clusters already affected by #1168(this issue) aren't self-healed and they need a manual restart or a remove/re-add of the host?

[isaactwong]

Added both: the entry now says that adding ingress.sql.host to a running cluster triggers a rolling restart, and a separate Note says clusters already affected by #1168 are not repaired by the upgrade, because the certificate is not regenerated again, and need a rolling restart or a remove/re-add of the host. Rebased onto master for CI.

[NishanthNalluri]

Looks good!