From 61561d0a27cec528f9a83fe9786157220c33dfb9 Mon Sep 17 00:00:00 2001 From: BryanFRD Date: Sun, 6 Sep 2026 07:54:25 +0000 Subject: [PATCH] fix(controller): un 429 sur la sonde ne marque plus la connexion en echec --- .../ferrvaultconnection_controller.go | 36 ++++++++-- .../ferrvaultconnection_probe_test.go | 70 +++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 internal/controller/ferrvaultconnection_probe_test.go diff --git a/internal/controller/ferrvaultconnection_controller.go b/internal/controller/ferrvaultconnection_controller.go index fa06db7..2f0026e 100644 --- a/internal/controller/ferrvaultconnection_controller.go +++ b/internal/controller/ferrvaultconnection_controller.go @@ -65,7 +65,28 @@ func (r *FerrVaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl. return r.handleDelete(ctx, &conn) } - status, reason, message := r.probe(ctx, &conn) + status, reason, message, probeErr := r.probe(ctx, &conn) + + // Un 429 n'est pas une panne : la sonde était bien formée et autorisée, le + // serveur a seulement demandé qu'on le rappelle plus tard. Le traiter comme + // un échec a figé 25 connexions sur 33, entraînant avec elles tous les + // `FerrVaultSecret` qui en dépendent, alors que les instances FerrVault + // répondaient normalement. + // + // La cause de ce pic est corrigée côté serveur — la sonde de santé est + // sortie du limiteur — mais le raisonnement restait faux ici : toute autre + // saturation reproduirait le symptôme. C'est le pendant, pour les + // connexions, de ce que #249 a fait pour les secrets. + // + // La condition n'est PAS réécrite : une connexion prête le reste, une + // connexion en échec garde la cause de son échec. Seul l'horodatage du + // dernier contrôle serait trompeur, et il n'est pas touché non plus. + if ferrvault.IsRateLimited(probeErr) { + logger.Info("probe rate limited, condition left as-is", + "requeueAfter", rateLimitRequeue) + return ctrl.Result{RequeueAfter: rateLimitRequeue}, nil + } + logger.Info("probe finished", "ready", status, "reason", reason) now := metav1.Now() @@ -139,28 +160,31 @@ func (r *FerrVaultConnectionReconciler) handleDelete( return ctrl.Result{RequeueAfter: connectionInUseRequeue}, nil } +// probe rend l'erreur en plus de son message : l'appelant doit pouvoir la +// TYPER, et une chaîne ne se distingue pas d'une autre. C'est ce qui permet de +// traiter un 429 pour ce qu'il est, un report, plutôt que comme une panne. func (r *FerrVaultConnectionReconciler) probe( ctx context.Context, conn *fvv1alpha1.FerrVaultConnection, -) (metav1.ConditionStatus, string, string) { +) (metav1.ConditionStatus, string, string, error) { broker := r.Broker if broker == nil { broker = NewTokenBroker(r.Client) } token, err := broker.TokenFor(ctx, conn) if err != nil { - return metav1.ConditionFalse, "TokenUnreadable", err.Error() + return metav1.ConditionFalse, "TokenUnreadable", err.Error(), err } ffc, err := ferrvault.New(conn.Spec.URL, token) if err != nil { - return metav1.ConditionFalse, "InvalidConnection", err.Error() + return metav1.ConditionFalse, "InvalidConnection", err.Error(), err } probeCtx, cancel := context.WithTimeout(ctx, 15*time.Second) defer cancel() if err := ffc.Probe(probeCtx); err != nil { - return metav1.ConditionFalse, "Unreachable", err.Error() + return metav1.ConditionFalse, "Unreachable", err.Error(), err } - return metav1.ConditionTrue, "Reachable", fmt.Sprintf("%s responded to /healthz", conn.Spec.URL) + return metav1.ConditionTrue, "Reachable", fmt.Sprintf("%s responded to /healthz", conn.Spec.URL), nil } func (r *FerrVaultConnectionReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/ferrvaultconnection_probe_test.go b/internal/controller/ferrvaultconnection_probe_test.go new file mode 100644 index 0000000..a80b740 --- /dev/null +++ b/internal/controller/ferrvaultconnection_probe_test.go @@ -0,0 +1,70 @@ +package controller + +import ( + "errors" + "net/http" + "testing" + + "github.com/FerrLabs/FerrVault/internal/ferrvault" +) + +// Le tri que fait `Reconcile` sur le résultat de `probe`. +// +// Un 429 doit sortir tôt, sans réécrire la condition : la sonde était bien +// formée et autorisée, le serveur a seulement demandé qu'on le rappelle. Le +// traiter comme une panne a figé 25 connexions sur 33, entraînant avec elles +// tous les `FerrVaultSecret` qui en dépendent, alors que les instances +// FerrVault répondaient normalement. +// +// Le test porte sur le prédicat appliqué aux erreurs que `probe` remonte +// réellement, et non sur trois valeurs construites pour l'occasion : c'est la +// distinction qui décide si la condition est écrasée ou laissée en place. +func TestOnlyRateLimitingSkipsTheConditionUpdate(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + { + // Le seul cas qui doit court-circuiter. + name: "429 : report, condition inchangee", + err: &ferrvault.APIError{Status: http.StatusTooManyRequests, Message: "Too Many Requests"}, + want: true, + }, + { + // Succès : `probe` rend `nil`, et le prédicat doit le supporter + // sans paniquer — c'est le chemin nominal, de loin le plus fréquent. + name: "succes : aucune erreur", + err: nil, + want: false, + }, + { + // Les vraies pannes doivent continuer d'écrire la condition, sinon + // une connexion cassée resterait verte indéfiniment : l'inverse + // exact du défaut corrigé, et bien pire. + name: "500 : vraie panne", + err: &ferrvault.APIError{Status: http.StatusInternalServerError, Message: "boom"}, + want: false, + }, + { + name: "401 : jeton refuse", + err: &ferrvault.AuthError{Kind: ferrvault.AuthUnauthorized, Message: "nope"}, + want: false, + }, + { + // `TokenUnreadable` : le Secret n'existe pas. Ne se répare jamais + // seul, doit rester visible. + name: "Secret de jeton absent", + err: errors.New(`load token Secret ns/name: Secret "name" not found`), + want: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := ferrvault.IsRateLimited(c.err); got != c.want { + t.Fatalf("IsRateLimited(%v) = %v, attendu %v", c.err, got, c.want) + } + }) + } +}