Keep the instance credential across a TerdutServer delete, and adopt it on recreate
Deleting a TerdutServer removed the credential Secrets but never touched the database, so a recreated one found a server that was already bootstrapped and no key for it: /api/bootstrap answered 403 and the operator stopped at BootstrapStateLost, whose message and DESIGN.md both said "delete and recreate". That is how the terdut-demo install on the cluster got stuck on 2026-10-03: Helm's cleanupOnFail deleted its TerdutServer after a failed upgrade, the recreate found the bootstrapped database, and it sat at Ready: False for five days until the database was reset by hand. Recreating cannot fix it, because the finalizer clears Secrets and the database is not its to reset, so "a fresh create starts clean" was only ever true when the database went with it. spec.credentials.deletionPolicy is Retain by default: the finalizer keeps the instance credential Secret (Delete removes it, as before). The bootstrap checkpoint is always removed. Before calling /api/bootstrap, reconcile now looks for the retained Secret and asks the server for the operator's own service account with its token. Accepted: adopt it and skip bootstrap. Rejected with 401/403: the Secret outlived a database reset, so ignore it and bootstrap like a first install, which replaces it. Any other error retries. terdut-server's own tests already call that endpoint with an instance-scoped key, so the permission is not new. BootstrapStateLost is still the answer when the server is bootstrapped and no credential it accepts survives, but its message now names the Secret to restore and says that recreating does not clear the database. DESIGN.md §6 says the same, and the chart passes the setting through as terdutServer.credentials.deletionPolicy. A retained Secret of a TerdutServer that is gone for good is an orphan to delete by hand. It is inert: nothing adopts it unless the server accepts the token. Checked on the kind demo with a locally built image against the real terdut-server v0.43.0: deleting the TerdutServer kept the Secret, recreating it reached Ready with the same credential (identical hash) and both TerdutTeams came back Ready with their original ids. The controller specs cover adoption, a rejected token after a reset, the bootstrapped-and-rejected failure, and both deletion policies. Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -16,18 +16,24 @@ import (
|
||||
)
|
||||
|
||||
// bootstrapStateLostError is DESIGN.md §6's one genuinely pathological
|
||||
// case: a checkpointed admin credential was used and then lost before the
|
||||
// lasting credential it was for could be persisted. Distinct from a plain
|
||||
// error so Reconcile can route it to a Ready: False condition (the
|
||||
// documented recovery is delete-and-recreate, not an automatic retry) rather
|
||||
// than treating it as a transient reconcile failure.
|
||||
type bootstrapStateLostError struct{ detail string }
|
||||
// case: the server's database is already bootstrapped, and no credential for
|
||||
// it survives here -- a checkpointed admin key was used and then lost before
|
||||
// the lasting credential could be persisted, or the instance credential
|
||||
// Secret was deleted (spec.credentials.deletionPolicy: Delete, or by hand).
|
||||
// Distinct from a plain error so Reconcile can route it to a Ready: False
|
||||
// condition rather than treating it as a transient reconcile failure: no
|
||||
// retry can fix it.
|
||||
type bootstrapStateLostError struct {
|
||||
detail string
|
||||
secretName string // the instance credential Secret that would have fixed it
|
||||
}
|
||||
|
||||
func (e *bootstrapStateLostError) Error() string {
|
||||
return fmt.Sprintf(
|
||||
"server reports already bootstrapped, but neither status.credentialsSecretRef nor a "+
|
||||
"checkpointed admin credential exist here: %s. This TerdutServer cannot recover a "+
|
||||
"credential on its own; delete and recreate it", e.detail)
|
||||
"server reports already bootstrapped, but there is no credential for it here: %s. "+
|
||||
"The operator cannot mint one on its own, and deleting and recreating this TerdutServer does "+
|
||||
"not clear the database. Restore Secret %q in the operator's namespace if you have a copy, "+
|
||||
"or reset the server's database and recreate this TerdutServer", e.detail, e.secretName)
|
||||
}
|
||||
|
||||
// reconcileBootstrap implements DESIGN.md §6 point 1's self-registration
|
||||
@@ -36,6 +42,17 @@ func (e *bootstrapStateLostError) Error() string {
|
||||
// srv.Status.CredentialsSecretRef is nil and the Deployment has a ready
|
||||
// replica.
|
||||
func (r *TerdutServerReconciler) reconcileBootstrap(ctx context.Context, srv *terdutv1alpha1.TerdutServer) error {
|
||||
// A TerdutServer recreated against a database that is already
|
||||
// bootstrapped: its instance credential may still be here (the default
|
||||
// deletion policy keeps it), and then there is nothing to bootstrap.
|
||||
adopted, err := r.adoptRetainedCredentials(ctx, srv)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if adopted {
|
||||
return nil
|
||||
}
|
||||
|
||||
adminKey, err := r.getOrCreateCheckpointedAdminKey(ctx, srv)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -64,6 +81,51 @@ func (r *TerdutServerReconciler) reconcileBootstrap(ctx context.Context, srv *te
|
||||
return nil
|
||||
}
|
||||
|
||||
// adoptRetainedCredentials looks for the instance credential Secret an
|
||||
// earlier TerdutServer of the same name and namespace left behind
|
||||
// (spec.credentials.deletionPolicy: Retain), and adopts it if the server
|
||||
// still accepts its token. Reports whether it did.
|
||||
//
|
||||
// The token is tried before it is trusted: a Secret that outlived a database
|
||||
// reset holds a key the server has never heard of, and adopting that would
|
||||
// make every later call 401. A rejected token (401/403) is not an error here;
|
||||
// it just means there is nothing to adopt, and bootstrap proceeds as it would
|
||||
// for a first install -- which succeeds against a freshly reset database and
|
||||
// replaces the Secret. Anything else (the server unreachable, a 5xx) is
|
||||
// returned for a retry rather than guessed at.
|
||||
func (r *TerdutServerReconciler) adoptRetainedCredentials(ctx context.Context, srv *terdutv1alpha1.TerdutServer) (bool, error) {
|
||||
name := credentialsSecretName(srv)
|
||||
var sec corev1.Secret
|
||||
if err := r.Get(ctx, client.ObjectKey{Namespace: r.OperatorNamespace, Name: name}, &sec); err != nil {
|
||||
if apierrors.IsNotFound(err) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
token := string(sec.Data[credentialsSecretDataKey])
|
||||
if token == "" {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// The operator's own service account is the one thing this credential
|
||||
// is for, so asking the server for it by name checks the token and that
|
||||
// it belongs to that account in the same call.
|
||||
sa, err := r.NewClient(serviceURL(srv)).WithToken(token).GetServiceAccountByName(ctx, serviceAccountName)
|
||||
if err != nil {
|
||||
if statusErr, ok := errors.AsType[*tdclient.StatusError](err); ok &&
|
||||
(statusErr.Code == http.StatusUnauthorized || statusErr.Code == http.StatusForbidden) {
|
||||
return false, nil
|
||||
}
|
||||
return false, fmt.Errorf("checking the retained credential in Secret %q: %w", name, err)
|
||||
}
|
||||
if sa == nil {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
srv.Status.CredentialsSecretRef = &terdutv1alpha1.SecretKeyRef{Name: name, Key: credentialsSecretDataKey}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// getOrCreateCheckpointedAdminKey returns a usable admin key: from the
|
||||
// checkpoint Secret if an earlier, interrupted attempt already got one, or
|
||||
// freshly from /api/bootstrap, immediately checkpointed before it's used
|
||||
@@ -89,7 +151,10 @@ func (r *TerdutServerReconciler) getOrCreateCheckpointedAdminKey(ctx context.Con
|
||||
// status.credentialsSecretRef) means a prior reconcile already
|
||||
// won this exact race and its checkpoint was lost afterward --
|
||||
// the one case §6 doesn't try to paper over.
|
||||
return "", &bootstrapStateLostError{detail: "/api/bootstrap returned 403"}
|
||||
return "", &bootstrapStateLostError{
|
||||
detail: "/api/bootstrap returned 403",
|
||||
secretName: credentialsSecretName(srv),
|
||||
}
|
||||
}
|
||||
return "", fmt.Errorf("POST /api/bootstrap: %w", err)
|
||||
}
|
||||
|
||||
@@ -39,10 +39,10 @@ const resyncInterval = 5 * time.Minute
|
||||
// than "someone edited something out of band."
|
||||
const waitInterval = 15 * time.Second
|
||||
|
||||
// finalizerName cleans up the credentials Secret(s) this controller
|
||||
// generates in the operator's own namespace on delete — the Deployment and
|
||||
// Service are owned (OwnerReference, DESIGN.md §7) and need no finalizer of
|
||||
// their own.
|
||||
// finalizerName cleans up the Secret(s) this controller generates in the
|
||||
// operator's own namespace on delete (which of them, spec.credentials.
|
||||
// deletionPolicy decides) — the Deployment and Service are owned
|
||||
// (OwnerReference, DESIGN.md §7) and need no finalizer of their own.
|
||||
const finalizerName = "terdut.ryuvia.com/terdutserver"
|
||||
|
||||
// serviceAccountName is the name the operator registers itself under
|
||||
@@ -215,18 +215,31 @@ func (r *TerdutServerReconciler) setNotReady(
|
||||
return ctrl.Result{RequeueAfter: d}, nil
|
||||
}
|
||||
|
||||
// reconcileDelete cleans up the credentials Secret(s) this controller
|
||||
// generated in the operator's own namespace. The Deployment and Service are
|
||||
// owned (OwnerReference, DESIGN.md §7) and need no attention here — normal
|
||||
// GC handles them. There is no server-side "delete this install" call to
|
||||
// reconcileDelete cleans up the Secrets this controller generated in the
|
||||
// operator's own namespace. The Deployment and Service are owned
|
||||
// (OwnerReference, DESIGN.md §7) and need no attention here — normal GC
|
||||
// handles them. There is no server-side "delete this install" call to
|
||||
// make: bootstrap created a user and a service account, and terdut-server's
|
||||
// API has no way to delete either (only to revoke individual keys), so
|
||||
// there is nothing meaningful to undo there either.
|
||||
// there is nothing meaningful to undo there either, and the database is
|
||||
// never touched.
|
||||
//
|
||||
// That is why the instance credential is kept by default
|
||||
// (spec.credentials.deletionPolicy: Retain). Everything it logs in to
|
||||
// outlives the TerdutServer, so a recreated one finds a bootstrapped server
|
||||
// it has no key for -- unless the key is still here to be adopted (see
|
||||
// adoptRetainedCredentials). The bootstrap checkpoint is always removed: it
|
||||
// is a short-lived admin key, and the instance credential is all that is
|
||||
// needed afterwards.
|
||||
func (r *TerdutServerReconciler) reconcileDelete(ctx context.Context, srv *terdutv1alpha1.TerdutServer) (ctrl.Result, error) {
|
||||
if !controllerutil.ContainsFinalizer(srv, finalizerName) {
|
||||
return ctrl.Result{}, nil
|
||||
}
|
||||
for _, name := range []string{checkpointSecretName(srv), credentialsSecretName(srv)} {
|
||||
names := []string{checkpointSecretName(srv)}
|
||||
if srv.Spec.Credentials.DeletionPolicy == terdutv1alpha1.CredentialsDelete {
|
||||
names = append(names, credentialsSecretName(srv))
|
||||
}
|
||||
for _, name := range names {
|
||||
sec := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.OperatorNamespace}}
|
||||
if err := r.Delete(ctx, sec); err != nil && !apierrors.IsNotFound(err) {
|
||||
return ctrl.Result{}, err
|
||||
|
||||
@@ -54,9 +54,13 @@ type fakeTerdutServer struct {
|
||||
mu sync.Mutex
|
||||
bootstrapped bool
|
||||
bootstrap403 bool // force every /api/bootstrap call to 403, even the first
|
||||
nextID int64
|
||||
accounts map[string]int64 // name -> id
|
||||
keyMints map[int64]int // id -> number of keys minted so far
|
||||
// rejectedTokens: bearer tokens the fake answers 401 on GET
|
||||
// /api/service-accounts, the way a server that never issued the key
|
||||
// (a database reset since) would.
|
||||
rejectedTokens map[string]bool
|
||||
nextID int64
|
||||
accounts map[string]int64 // name -> id
|
||||
keyMints map[int64]int // id -> number of keys minted so far
|
||||
|
||||
nextTeamID int64
|
||||
teams map[string]int64 // name -> id
|
||||
@@ -170,6 +174,10 @@ func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
|
||||
case r.URL.Path == "/api/service-accounts" && r.Method == http.MethodGet:
|
||||
if f.rejectedTokens[strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")] {
|
||||
writeJSON(w, http.StatusUnauthorized, map[string]string{errJSONKey: "invalid or expired API key"})
|
||||
return
|
||||
}
|
||||
name := r.URL.Query().Get("name")
|
||||
id, exists := f.accounts[name]
|
||||
if !exists {
|
||||
@@ -875,13 +883,14 @@ var _ = Describe("TerdutServer Controller", func() {
|
||||
})
|
||||
|
||||
Describe("deletion", func() {
|
||||
It("removes the credentials and checkpoint Secrets and the finalizer", func(ctx SpecContext) {
|
||||
fake, fakeSrv := newFakeTerdutServer()
|
||||
_ = fake
|
||||
// runToReady brings a server to Ready against a fresh fake and
|
||||
// returns the name of the instance credential Secret it minted.
|
||||
runToReady := func(ctx SpecContext, spec terdutv1alpha1.TerdutServerSpec) string {
|
||||
_, fakeSrv := newFakeTerdutServer()
|
||||
DeferCleanup(fakeSrv.Close)
|
||||
reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) }
|
||||
|
||||
createServer(ctx, dsnSpec())
|
||||
createServer(ctx, spec)
|
||||
reconcileOnce(ctx)
|
||||
reconcileOnce(ctx)
|
||||
markDeploymentReady(ctx)
|
||||
@@ -889,17 +898,114 @@ var _ = Describe("TerdutServer Controller", func() {
|
||||
|
||||
srv := &terdutv1alpha1.TerdutServer{}
|
||||
Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed())
|
||||
credsName := srv.Status.CredentialsSecretRef.Name
|
||||
|
||||
return srv.Status.CredentialsSecretRef.Name
|
||||
}
|
||||
deleteAndFinalize := func(ctx SpecContext) {
|
||||
srv := &terdutv1alpha1.TerdutServer{}
|
||||
Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed())
|
||||
Expect(k8sClient.Delete(ctx, srv)).To(Succeed())
|
||||
reconcileOnce(ctx) // runs the finalizer
|
||||
Expect(k8sClient.Get(ctx, objKey, srv)).NotTo(Succeed(),
|
||||
"the TerdutServer itself should be gone once the finalizer clears")
|
||||
}
|
||||
secretExists := func(ctx SpecContext, secretName string) bool {
|
||||
var s corev1.Secret
|
||||
err := k8sClient.Get(ctx, types.NamespacedName{Name: secretName, Namespace: operatorNamespace}, &s)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
err := k8sClient.Get(ctx, objKey, srv)
|
||||
Expect(err).To(HaveOccurred(), "the TerdutServer itself should be gone once the finalizer clears")
|
||||
It("keeps the instance credential by default and removes the checkpoint", func(ctx SpecContext) {
|
||||
credsName := runToReady(ctx, dsnSpec())
|
||||
// A checkpoint left behind, as if the best-effort delete had failed.
|
||||
Expect(k8sClient.Create(ctx, &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: checkpointSecretNameFor(name), Namespace: operatorNamespace},
|
||||
Data: map[string][]byte{credentialsSecretDataKey: []byte("admin-key-raw")},
|
||||
})).To(Succeed())
|
||||
|
||||
var leftover corev1.Secret
|
||||
err = k8sClient.Get(ctx, types.NamespacedName{Name: credsName, Namespace: operatorNamespace}, &leftover)
|
||||
Expect(err).To(HaveOccurred(), "the credentials Secret should have been cleaned up by the finalizer")
|
||||
deleteAndFinalize(ctx)
|
||||
|
||||
Expect(secretExists(ctx, credsName)).To(BeTrue(),
|
||||
"the credential is kept so a recreated TerdutServer can adopt it")
|
||||
Expect(secretExists(ctx, checkpointSecretNameFor(name))).To(BeFalse(),
|
||||
"the checkpoint is a short-lived admin key and is always removed")
|
||||
})
|
||||
|
||||
It("removes the credentials and checkpoint Secrets and the finalizer when the policy is Delete", func(ctx SpecContext) {
|
||||
spec := dsnSpec()
|
||||
spec.Credentials.DeletionPolicy = terdutv1alpha1.CredentialsDelete
|
||||
credsName := runToReady(ctx, spec)
|
||||
|
||||
deleteAndFinalize(ctx)
|
||||
|
||||
Expect(secretExists(ctx, credsName)).To(BeFalse(),
|
||||
"the credentials Secret should have been cleaned up by the finalizer")
|
||||
})
|
||||
})
|
||||
|
||||
Describe("recreating a TerdutServer against an already-bootstrapped server", func() {
|
||||
// retainedSecret stands in for what the previous TerdutServer left.
|
||||
retainedSecret := func(ctx SpecContext, token string) {
|
||||
Expect(k8sClient.Create(ctx, &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: credentialsSecretNameFor(name), Namespace: operatorNamespace},
|
||||
Data: map[string][]byte{credentialsSecretDataKey: []byte(token)},
|
||||
})).To(Succeed())
|
||||
}
|
||||
bringUp := func(ctx SpecContext, fakeSrv *httptest.Server) {
|
||||
DeferCleanup(fakeSrv.Close)
|
||||
reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) }
|
||||
createServer(ctx, dsnSpec())
|
||||
reconcileOnce(ctx) // finalizer
|
||||
reconcileOnce(ctx) // Deployment/Service
|
||||
markDeploymentReady(ctx)
|
||||
reconcileOnce(ctx)
|
||||
}
|
||||
credsToken := func(ctx SpecContext) string {
|
||||
var s corev1.Secret
|
||||
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: credentialsSecretNameFor(name), Namespace: operatorNamespace}, &s)).To(Succeed())
|
||||
return string(s.Data[credentialsSecretDataKey])
|
||||
}
|
||||
|
||||
It("adopts the retained credential when the server still accepts it, without bootstrapping", func(ctx SpecContext) {
|
||||
fake, fakeSrv := newFakeTerdutServer()
|
||||
fake.bootstrapped = true // every /api/bootstrap call 403s
|
||||
fake.accounts[serviceAccountName] = 7
|
||||
retainedSecret(ctx, "retained-key")
|
||||
|
||||
bringUp(ctx, fakeSrv)
|
||||
|
||||
Expect(readyCondition(ctx).Status).To(Equal(metav1.ConditionTrue))
|
||||
srv := &terdutv1alpha1.TerdutServer{}
|
||||
Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed())
|
||||
Expect(srv.Status.CredentialsSecretRef).NotTo(BeNil())
|
||||
Expect(srv.Status.CredentialsSecretRef.Name).To(Equal(credentialsSecretNameFor(name)))
|
||||
Expect(credsToken(ctx)).To(Equal("retained-key"), "the retained key is reused, not replaced")
|
||||
})
|
||||
|
||||
It("ignores a retained credential the server rejects and bootstraps afresh after a database reset", func(ctx SpecContext) {
|
||||
fake, fakeSrv := newFakeTerdutServer() // a fresh database: bootstrap succeeds
|
||||
fake.rejectedTokens = map[string]bool{"stale-key": true}
|
||||
retainedSecret(ctx, "stale-key")
|
||||
|
||||
bringUp(ctx, fakeSrv)
|
||||
|
||||
Expect(readyCondition(ctx).Status).To(Equal(metav1.ConditionTrue))
|
||||
Expect(credsToken(ctx)).NotTo(Equal("stale-key"), "the stale credential is replaced by a freshly minted one")
|
||||
})
|
||||
|
||||
It("fails closed, naming the Secret to restore, when the server is bootstrapped and the retained credential is rejected", func(ctx SpecContext) {
|
||||
fake, fakeSrv := newFakeTerdutServer()
|
||||
fake.bootstrapped = true
|
||||
fake.rejectedTokens = map[string]bool{"stale-key": true}
|
||||
retainedSecret(ctx, "stale-key")
|
||||
|
||||
bringUp(ctx, fakeSrv)
|
||||
|
||||
cond := readyCondition(ctx)
|
||||
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
|
||||
Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonBootstrapStateLost))
|
||||
Expect(cond.Message).To(ContainSubstring(credentialsSecretNameFor(name)),
|
||||
"the message names the Secret that would restore it")
|
||||
Expect(cond.Message).To(ContainSubstring("does not clear the database"))
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user