From fd8d378b735cbc3a6786fadece3dea84f49a62e2 Mon Sep 17 00:00:00 2001 From: IvanHunters Date: Thu, 20 Aug 2026 11:56:32 +0300 Subject: [PATCH] fix(kubernetes): rotate bootstrap-token Job on either token half; review nits Fold bootstrapTokenSecret into the Job content hash alongside bootstrapTokenId. talos-secrets.yaml regenerates the two token halves independently, so hashing only the id left a rotated secret no-oping a completed Job and the tenant stuck on the stale bootstrap-token. Also address review feedback: drop a review-round reference from the tracked test, correct a stale hash in a test comment, pin the alternate-image hash case exactly instead of by regex, and fix a comment indent in talos-reconcile-job.yaml. Signed-off-by: IvanHunters --- .../talos/bootstrap-token-tenant-job.yaml | 44 ++++++++++++------- .../templates/talos/talos-reconcile-job.yaml | 2 +- .../tests/talos_templates_test.yaml | 23 ++++------ 3 files changed, 37 insertions(+), 32 deletions(-) diff --git a/packages/apps/kubernetes/templates/talos/bootstrap-token-tenant-job.yaml b/packages/apps/kubernetes/templates/talos/bootstrap-token-tenant-job.yaml index a964ccd11..1e161a7c8 100644 --- a/packages/apps/kubernetes/templates/talos/bootstrap-token-tenant-job.yaml +++ b/packages/apps/kubernetes/templates/talos/bootstrap-token-tenant-job.yaml @@ -45,15 +45,18 @@ cluster.yaml): an identical render keeps the name and Helm no-ops it; any change that flows into the spec — notably .Values.images.kubectl — yields a new name so a fresh Job runs and Helm prunes the old one. The - hash also folds in the looked-up bootstrapTokenId, because the token - reaches the Job only through secretKeyRef and so never changes the - render on its own: without this, a rotated token (a recreated - talos-secrets Secret) would leave Helm no-oping a completed Job and the - tenant stuck on the stale bootstrap-token. Folding the id in rotates - the name on rotation so the new token is applied and the old Job is - pruned. (One idempotent re-run happens on the first reconcile after a - fresh install, when the looked-up id flips from empty to its value; the - kubectl apply is a no-op.) + hash also folds in the looked-up bootstrap token — both halves, + bootstrapTokenId and bootstrapTokenSecret — because the token reaches + the Job only through secretKeyRef and so never changes the render on + its own: without this, a rotated token (a recreated talos-secrets + Secret) would leave Helm no-oping a completed Job and the tenant stuck + on the stale bootstrap-token. talos-secrets.yaml regenerates the two + halves independently (each guarded by its own empty check), so a change + to either half must rotate the name; folding both in guarantees that. + Folding the token in rotates the name on rotation so the new token is + applied and the old Job is pruned. (One idempotent re-run happens on + the first reconcile after a fresh install, when the looked-up token + flips from empty to its value; the kubectl apply is a no-op.) */}} --- apiVersion: v1 @@ -213,18 +216,25 @@ spec: secretName: {{ .Release.Name }}-admin-kubeconfig optional: true {{- end }} -{{- /* Render the spec, then hash it (plus the looked-up bootstrapTokenId) into - a 6-char name suffix so the immutable Job is churn-free across upgrades - yet still rotates when its image or the bootstrap token changes. See the - header comment. lookup returns empty during `helm template`/unittest, so - the token contribution is stable there and the render stays testable. */}} +{{- /* Render the spec, then hash it (plus the looked-up bootstrap token, both + halves) into a 6-char name suffix so the immutable Job is churn-free + across upgrades yet still rotates when its image or the bootstrap token + changes. See the header comment. lookup returns empty during `helm + template`/unittest, so the token contribution is stable there and the + render stays testable. */}} {{- $jobSpec := include "kubernetes.bootstrap-token-tenant-job-spec" . }} {{- $btId := "" }} +{{- $btSecret := "" }} {{- $existing := lookup "v1" "Secret" .Release.Namespace (printf "%s-talos-secrets" .Release.Name) }} -{{- if and $existing $existing.data (hasKey $existing.data "bootstrapTokenId") }} -{{- $btId = index $existing.data "bootstrapTokenId" | b64dec }} +{{- if and $existing $existing.data }} +{{- if hasKey $existing.data "bootstrapTokenId" }} +{{- $btId = index $existing.data "bootstrapTokenId" | b64dec }} +{{- end }} +{{- if hasKey $existing.data "bootstrapTokenSecret" }} +{{- $btSecret = index $existing.data "bootstrapTokenSecret" | b64dec }} +{{- end }} {{- end }} -{{- $jobHash := printf "%s\n%s" $jobSpec $btId | sha256sum | trunc 6 }} +{{- $jobHash := printf "%s\n%s\n%s" $jobSpec $btId $btSecret | sha256sum | trunc 6 }} {{- /* Truncate the name prefix, never the hash, so the name stays within the 63-char DNS-label limit while the 6-char content hash is always the suffix. A no-op for normal-length release names. */}} diff --git a/packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml b/packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml index f6b19459d..ba1648c37 100644 --- a/packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml +++ b/packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml @@ -64,7 +64,7 @@ the wait loop tolerates its dependencies (and its own network policy) not yet being in place, so Helm's kind-ordered apply needs no explicit sequencing. The bootstrap-token Job is main-phase too, for the same - reasons (see bootstrap-token-tenant-job.yaml). + reasons (see bootstrap-token-tenant-job.yaml). Churn-free across upgrades. The Job is immutable (spec.template cannot be patched), so it carries a content-hash name suffix — mirrors the diff --git a/packages/apps/kubernetes/tests/talos_templates_test.yaml b/packages/apps/kubernetes/tests/talos_templates_test.yaml index 126368d59..e960333ff 100644 --- a/packages/apps/kubernetes/tests/talos_templates_test.yaml +++ b/packages/apps/kubernetes/tests/talos_templates_test.yaml @@ -366,9 +366,9 @@ tests: path: metadata.name pattern: ^test-k8s-bootstrap-token-tenant-[0-9a-f]{6}$ documentIndex: 1 - # Point 4 of the review: the admin-kubeconfig volume must be optional and - # gated by the shared wait-for-kubeconfig init container, so the Pod waits - # in init (uncounted by backoffLimit) instead of FailedMount at t0. + # The admin-kubeconfig volume must be optional and gated by the shared + # wait-for-kubeconfig init container, so the Pod waits in init (uncounted + # by backoffLimit) instead of FailedMount at t0. - equal: path: spec.template.spec.initContainers[0].name value: wait-for-kubeconfig @@ -406,7 +406,7 @@ tests: asserts: - equal: path: metadata.name - value: test-k8s-bootstrap-token-tenant-ddf388 + value: test-k8s-bootstrap-token-tenant-fc314f documentSelector: path: kind value: Job @@ -421,19 +421,14 @@ tests: images: kubectl: example.com/kubectl:v9.9.9 asserts: - # Still a valid hashed name ... - - matchRegex: - path: metadata.name - pattern: ^test-k8s-bootstrap-token-tenant-[0-9a-f]{6}$ - documentSelector: - path: kind - value: Job - # ... but a different one from the default-image render above (e8c6ea), + # A different exact hash from the default-image render above (fc314f), # proving the hash tracks the rendered spec (the kubectl image is in it), # so an image bump lands a fresh Job rather than an immutable-field patch. - - notMatchRegex: + # Pinned exactly (like the talos-reconcile sibling cases) rather than by + # regex, so a hash drift on either render is caught, not silently masked. + - equal: path: metadata.name - pattern: -ddf388$ + value: test-k8s-bootstrap-token-tenant-dbf53d documentSelector: path: kind value: Job