diff --git a/DESIGN.md b/DESIGN.md index 8f6ad15..90a9078 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -169,6 +169,8 @@ spec: matchers: "alertname=Watchdog" timeout: 15m severity: critical + credentials: + deletionPolicy: Retain # Retain (default) | Delete -- what deleting this CR does to the instance credential Secret; see §6 point 2 notify: ntfyURL: "http://ntfy.ntfy.svc.cluster.local" fallbackTopic: "" @@ -529,6 +531,18 @@ when nothing ever crosses into a tenant namespace in the first place. the same rigor §5's general idempotent-create rule already applies elsewhere: - `status.credentialsSecretRef` already set: done, nothing to do. + - Otherwise, look for the instance credential Secret itself + (`.-instance-credentials`, point 2 below): a + `TerdutServer` deleted and recreated under the same name leaves it + behind by default (`spec.credentials.deletionPolicy: Retain`), and the + server it logs in to has not changed, because deleting a + `TerdutServer` never touches its database. If it exists, ask the server + for the operator's own service account with that token + (`GET /api/service-accounts?name=terdut-operator`). Accepted: adopt it, + set `status.credentialsSecretRef`, and skip everything below. Rejected + with a `401`/`403`: it is stale (a database reset since), so ignore it + and carry on; the steps below replace it. Any other failure is a retry, + not a guess. - Otherwise, check for an intermediate `.-bootstrap-admin` Secret in the operator's own namespace first. If it exists, its key is a still- @@ -538,15 +552,21 @@ when nothing ever crosses into a tenant namespace in the first place. on `201`, immediately checkpoint its response's raw admin key (`{"user": ..., "api_key": {"key": "", ...}}`) into that Secret before doing anything else with it. A `403` with neither - `status.credentialsSecretRef` nor this checkpoint Secret present is - the one genuinely pathological case left (the checkpoint deleted out - from under a reconcile already past this point) — handled the same - way the design already handles unrecoverable server-issued material - elsewhere (§5's webhook-Secret-loss rule): fail closed, - `Ready: False, reason: BootstrapStateLost`, with the same recovery as - that case, delete and recreate the `TerdutServer` (its finalizer tears - down the Deployment/database-backing and server-side rows; a fresh - create starts clean) — not a workaround peculiar to this one path. + `status.credentialsSecretRef`, nor an instance credential the server + accepts, nor this checkpoint Secret present is the one genuinely + pathological case left: the server's database is already bootstrapped + and no credential for it survives here (the checkpoint deleted out from + under a reconcile already past this point, `deletionPolicy: Delete`, or + the Secret removed by hand). Fail closed, `Ready: False, reason: + BootstrapStateLost`, with a message that names the instance credential + Secret to restore. Deleting and recreating the `TerdutServer` does **not** + recover from it: the finalizer removes Secrets only, and the database + (the one thing that still says "already bootstrapped") is not the + operator's to reset. Restore the Secret from a copy, or reset the + server's database and then recreate the `TerdutServer`, which then + bootstraps like a first install. (This section used to say a fresh + create "starts clean"; that was only ever true when the database went + with it.) - With an admin key in hand (fresh or checkpointed): `POST /api/service-accounts {name: "terdut-operator", scope: "instance"}`. A `409` here means a prior attempt got this far before being @@ -563,10 +583,15 @@ when nothing ever crosses into a tenant namespace in the first place. under a fixed data key, `token`), referenced back from `TerdutServer.status.credentialsSecretRef: {name, key}` (§4.1). No `OwnerReference` (those can't cross namespaces, and this Secret doesn't - share a namespace with the `TerdutServer` that caused it); the `TerdutServer`'s - finalizer deletes this Secret directly as part of its own teardown, - the same way it already has to clean up the server-side resources it - created (§5's general finalizer rule extends naturally to this Secret). + share a namespace with the `TerdutServer` that caused it). What the + `TerdutServer`'s finalizer does with it is `spec.credentials.deletionPolicy`: + `Retain` (the default) leaves it for a recreated `TerdutServer` to adopt + (point 1), `Delete` removes it directly as part of the teardown. There are + no server-side resources to undo either way: the bootstrap user and + service account have no delete verb in terdut-server's API. The bootstrap + checkpoint Secret is always removed. A retained Secret of a `TerdutServer` + that is gone for good is an orphan to delete by hand, and it is inert: + adoption asks the server to accept the token first. 3. When a `TerdutTeam` first becomes `Ready` (its `serverRef` resolved, `allowedTeams` satisfied if cross-namespace), its controller uses the `TerdutServer`'s instance-scoped credential (read from the operator's own diff --git a/api/v1alpha1/terdutserver_types.go b/api/v1alpha1/terdutserver_types.go index 4e6543f..a522fb7 100644 --- a/api/v1alpha1/terdutserver_types.go +++ b/api/v1alpha1/terdutserver_types.go @@ -23,6 +23,37 @@ type SecretKeyRef struct { Key string `json:"key"` } +// CredentialsDeletionPolicy is what deleting a TerdutServer does to the +// instance credential Secret the operator generated for it. +// +kubebuilder:validation:Enum=Retain;Delete +type CredentialsDeletionPolicy string + +const ( + // CredentialsRetain keeps the Secret when the TerdutServer is deleted, so + // a TerdutServer recreated with the same name and namespace against the + // same database adopts it again instead of finding a server it cannot + // log in to. Deleting a TerdutServer never touches its database, so the + // operator's service account is still there to be reused. The default. + CredentialsRetain CredentialsDeletionPolicy = "Retain" + // CredentialsDelete removes the Secret with the TerdutServer. Choose it + // when the database goes too, or when the credential must not outlive the + // object. + CredentialsDelete CredentialsDeletionPolicy = "Delete" +) + +// CredentialsSpec configures the lifecycle of the generated instance +// credential. +type CredentialsSpec struct { + // deletionPolicy: whether the instance credential Secret is kept + // (Retain, the default) or removed (Delete) when this TerdutServer is + // deleted. A kept Secret is only ever adopted after the server accepts + // its token, so one left over from a database that has since been reset + // is ignored and replaced. + // +kubebuilder:default=Retain + // +optional + DeletionPolicy CredentialsDeletionPolicy `json:"deletionPolicy,omitempty"` +} + // ImageSpec is the terdut-server image to run. type ImageSpec struct { // +kubebuilder:validation:MinLength=1 @@ -324,6 +355,11 @@ type TerdutServerSpec struct { // +optional Deadman DeadmanSpec `json:"deadman,omitempty"` + // credentials: what happens to the instance credential this operator + // generates for the server. + // +optional + Credentials CredentialsSpec `json:"credentials,omitempty"` + // +optional Notify NotifySpec `json:"notify,omitempty"` @@ -382,9 +418,12 @@ const ( // ReasonBootstrapStateLost: a checkpointed admin credential // (DESIGN.md §6) was lost after being used but before the lasting // credential it was for could be persisted -- the one genuinely - // pathological case in the self-registration flow. Fail-closed, same - // recovery as DESIGN.md §5's webhook-Secret-loss rule: delete and - // recreate this TerdutServer. + // pathological case in the self-registration flow -- or the server's + // database is already bootstrapped and no credential for it survives + // (spec.credentials.deletionPolicy: Delete, or the Secret removed by + // hand). Fail-closed: the operator cannot mint a credential, and + // deleting and recreating the TerdutServer does not clear the database. + // Restore the Secret, or reset the server's database. ReasonBootstrapStateLost = "BootstrapStateLost" // ReasonAdopted: the happy path. A working credential is in hand, the // Deployment has a ready replica, and the database (if postgresClusterRef) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index bd6ed80..5b3dafe 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -47,6 +47,21 @@ func (in *AllowedTeamsNamespaces) DeepCopy() *AllowedTeamsNamespaces { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CredentialsSpec) DeepCopyInto(out *CredentialsSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CredentialsSpec. +func (in *CredentialsSpec) DeepCopy() *CredentialsSpec { + if in == nil { + return nil + } + out := new(CredentialsSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DatabaseSpec) DeepCopyInto(out *DatabaseSpec) { *out = *in @@ -764,6 +779,7 @@ func (in *TerdutServerSpec) DeepCopyInto(out *TerdutServerSpec) { in.Database.DeepCopyInto(&out.Database) out.Sweeper = in.Sweeper out.Deadman = in.Deadman + out.Credentials = in.Credentials in.Notify.DeepCopyInto(&out.Notify) in.OIDC.DeepCopyInto(&out.OIDC) in.AllowedTeams.DeepCopyInto(&out.AllowedTeams) diff --git a/charts/terdut-operator/templates/crd/terdutservers.terdut.ryuvia.com.yaml b/charts/terdut-operator/templates/crd/terdutservers.terdut.ryuvia.com.yaml index 8d9c37b..68b361d 100644 --- a/charts/terdut-operator/templates/crd/terdutservers.terdut.ryuvia.com.yaml +++ b/charts/terdut-operator/templates/crd/terdutservers.terdut.ryuvia.com.yaml @@ -128,6 +128,24 @@ spec: x-kubernetes-map-type: atomic type: object type: object + credentials: + description: |- + credentials: what happens to the instance credential this operator + generates for the server. + properties: + deletionPolicy: + default: Retain + description: |- + deletionPolicy: whether the instance credential Secret is kept + (Retain, the default) or removed (Delete) when this TerdutServer is + deleted. A kept Secret is only ever adopted after the server accepts + its token, so one left over from a database that has since been reset + is ignored and replaced. + enum: + - Retain + - Delete + type: string + type: object database: description: |- DatabaseSpec is the Postgres connection this TerdutServer uses. Exactly diff --git a/charts/terdut-operator/templates/terdutserver/terdutserver.yaml b/charts/terdut-operator/templates/terdutserver/terdutserver.yaml index 5129802..ff1b02e 100644 --- a/charts/terdut-operator/templates/terdutserver/terdutserver.yaml +++ b/charts/terdut-operator/templates/terdutserver/terdutserver.yaml @@ -38,6 +38,10 @@ spec: deadman: {{- toYaml . | nindent 4 }} {{- end }} + {{- with .Values.terdutServer.credentials }} + credentials: + {{- toYaml . | nindent 4 }} + {{- end }} {{- with .Values.terdutServer.notify }} notify: {{- toYaml . | nindent 4 }} diff --git a/charts/terdut-operator/values.yaml b/charts/terdut-operator/values.yaml index 1b9951c..cff9a3b 100644 --- a/charts/terdut-operator/values.yaml +++ b/charts/terdut-operator/values.yaml @@ -270,6 +270,13 @@ terdutServer: # matchers: "alertname=Watchdog" # timeout: 15m # severity: critical + # What happens to the instance credential the operator generates for this + # server when the TerdutServer is deleted. Retain (the default) keeps it, so + # a TerdutServer recreated with the same name against the same database + # adopts it again; Delete removes it with the TerdutServer. A kept Secret + # is only adopted if the server accepts its token. + # credentials: + # deletionPolicy: Retain # notify: {} # oidc: {} diff --git a/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml b/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml index 5f76fd2..9275bde 100644 --- a/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml +++ b/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml @@ -125,6 +125,24 @@ spec: x-kubernetes-map-type: atomic type: object type: object + credentials: + description: |- + credentials: what happens to the instance credential this operator + generates for the server. + properties: + deletionPolicy: + default: Retain + description: |- + deletionPolicy: whether the instance credential Secret is kept + (Retain, the default) or removed (Delete) when this TerdutServer is + deleted. A kept Secret is only ever adopted after the server accepts + its token, so one left over from a database that has since been reset + is ignored and replaced. + enum: + - Retain + - Delete + type: string + type: object database: description: |- DatabaseSpec is the Postgres connection this TerdutServer uses. Exactly diff --git a/internal/controller/terdutserver_bootstrap.go b/internal/controller/terdutserver_bootstrap.go index 77b2f90..80c1b8a 100644 --- a/internal/controller/terdutserver_bootstrap.go +++ b/internal/controller/terdutserver_bootstrap.go @@ -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) } diff --git a/internal/controller/terdutserver_controller.go b/internal/controller/terdutserver_controller.go index 4f22fc8..e455efb 100644 --- a/internal/controller/terdutserver_controller.go +++ b/internal/controller/terdutserver_controller.go @@ -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 diff --git a/internal/controller/terdutserver_controller_test.go b/internal/controller/terdutserver_controller_test.go index b94bb1e..c597a6b 100644 --- a/internal/controller/terdutserver_controller_test.go +++ b/internal/controller/terdutserver_controller_test.go @@ -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")) }) })