Skip to content

docs(seinode): LLD for sidecar TLS via externally-provisioned Secret#256

Merged
bdchatham merged 1 commit into
mainfrom
docs/seinode-sidecar-tls-externalized-cert-lld
May 16, 2026
Merged

docs(seinode): LLD for sidecar TLS via externally-provisioned Secret#256
bdchatham merged 1 commit into
mainfrom
docs/seinode-sidecar-tls-externalized-cert-lld

Conversation

@bdchatham
Copy link
Copy Markdown
Collaborator

Summary

  • Lands the LLD for the revised sidecar TLS design as a doc-only PR, so implementation work in Sidecar TLS via externally-provisioned Secret #255 references a merged design rather than a moving target.
  • Replaces the in-place-toggle approach explored in closed PR feat(planner): TLS toggle on Running SeiNodes via NodeUpdate plan #254. Cross-review surfaced architectural concerns (brittle plan-task-list classification, cert lifecycle coupled to plan lifecycle, attacker-controllable Issuer trust-anchor) that the revised design eliminates by externalizing cert ownership and making spec.sidecar.tls immutable.

Design at a glance (LLD §0.1)

Three coupled decisions:

  • Externalize cert ownership. Operator/platform tooling provisions the kubernetes.io/tls Secret; controller references it by name and gates on presence + cert validity. Mirrors signingKey / nodeKey / operatorKeyring / imported-PVC patterns already in the controller.
  • Immutable post-creation. Toggle via delete + recreate with PVC retention. At ~10 nodes manual cycling is fine; each transition gets a clean Pending → Running boundary.
  • Machine-readable contract. Controller publishes status.sidecarTLS.{secretName, requiredDNSNames} whenever TLS is enabled. Platform tooling reads this directly — no naming-convention docs the operator must find.

What collapses (vs. PR #254 design)

  • The drift detector (sidecarTLSDrift)
  • The status mirror (status.currentSidecarTLS)
  • Condition reason expansion (TLSToggleStarted, UpdateAndTLSToggleStarted)
  • classifyPlan discrimination
  • ApplySidecarCert task and GenerateSidecarCertificate generator
  • ObserveSidecarTLS task
  • Issuer trust-anchor security surface (IssuerName/IssuerKind)

CRD shrinks to spec.sidecar.tls.secretName: string + immutability CEL.

What stays

  • noderesource pod/service generation with the existing if SidecarTLSEnabled(node) { ... } branching
  • ApplyRBACProxyConfig task (controller-owned, namespace/name-dependent)

Tracking

Test plan

This is a doc-only PR; verification of the design happens in #255's implementation tests (LLD §8 lists them). Reviewing here is reviewing the design itself — section by section.

🤖 Generated with Claude Code

Captures the revised design that replaces the in-place-toggle approach
explored in closed PR #254. Lands as a doc-only PR so subsequent
implementation work in #255 can reference a merged design.

Key design decisions (§0.1):
- Externalize cert ownership: operator/platform tooling provisions the
  kubernetes.io/tls Secret; controller references it by name and gates
  on presence + cert validity. Mirrors signingKey / nodeKey /
  operatorKeyring / imported-PVC patterns already in the controller.
- spec.sidecar.tls is immutable post-creation; toggle via delete +
  recreate with PVC retention.
- Cert→SeiNode contract is machine-readable via
  status.sidecarTLS.{secretName, requiredDNSNames} — no naming-
  convention docs the operator must find.

Collapses the drift detector, observer task, status mirror, condition
reason expansion, classifyPlan discrimination, cert-manager Ready logic,
and Issuer trust-anchor security surface. CRD shrinks to
spec.sidecar.tls.secretName: string + immutability CEL.

Tracking: #255.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cursor
Copy link
Copy Markdown

cursor Bot commented May 16, 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 5c606d2 into main May 16, 2026
2 checks passed
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.

Sidecar TLS disable path on Running SeiNodes

1 participant