🤖 feat: sweep the cleanup records of CoderAIGatewayKey objects that went without a delete (#353 part 2d)

docs/how-to/manage-ai-gateway-keys.md

@@ -151,10 +151,19 @@ Before each key create, the operator writes a cleanup record: a ConfigMap named
 
 When the control plane later reaches another Coder deployment (for example after its database Secret changed), the record still locates the keys in the old one. The operator then stalls the object with `CoderDeploymentChanged` at its next key create, keeps the record and creates no key in the new deployment. A delete of the object then removes the finalizer without a delete in Coder. Delete the keys in the old deployment, then delete the ConfigMap.
 
-The operator removes the record only after a list in the record's deployment shows no key of the object: at the end of a normal delete, or after the one bounded pass when the last create was more than 5 minutes before it. In every other release without a delete, the record stays and names where the keys are. Find the records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record=CoderAIGatewayKey`. The operator gets write access to ConfigMaps only in that namespace. Do not give other users write access to ConfigMaps there.
+The operator removes the record only after a list in the record's deployment shows no key of the object: at the end of a normal delete, or after the one bounded pass when the last create was more than 5 minutes before it. In every other release without a delete, for example in a namespace deletion, the record stays, and the operator sweeps it:
+
+1. It waits until the object with that UID is gone and 5 minutes have passed since the last create, so that a create still in flight commits first.
+2. It looks for a ready `CoderControlPlane` in the object's namespace whose Coder answers the record's deployment ID now. It never matches by URL or name: a new Coder database behind the same URL has another deployment ID and is never swept. A control plane in another namespace never sweeps the record.
+3. Through that control plane, whose operator user must be a site owner, it lists the keys and deletes every key whose name is the record's prefix `ck8s-<UID without dashes>-` followed by a serial number (the names that the operator gives, such as `ck8s-<UID without dashes>-3`). It never deletes any other key.
+4. It removes the record when a list shows no such key. A failed list or delete, or a `404` that is not Coder's own, keeps the record, and the operator retries every minute.
+
+So after a namespace deletion with an external database, the leftover keys go once you create the namespace again with a control plane that uses the same database. The metric `coder_k8s_cleanup_records_swept_total` (label `kind`) counts the swept records.
+
+When no ready control plane reaches the record's deployment, the record stays. The gauge `coder_k8s_cleanup_records_unmatched` counts these records, and the operator writes a `NoMatchingDeployment` Warning Event on the ConfigMap every 10 minutes. When that Coder deployment never comes back, delete the keys with the record's prefix in Coder by hand, then delete the ConfigMap. Find the records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record=CoderAIGatewayKey`. The operator gets write access to ConfigMaps only in that namespace. Do not give other users write access to ConfigMaps there: a record names the keys that a sweep deletes.
 
 !!! warning "Delete the keys before the namespace"
-    When you delete the namespace, Kubernetes deletes the control plane and the `CoderAIGatewayKey` objects at the same time. The control plane is usually gone before the operator handles the objects, so the finalizers go without a delete, and the keys stay valid in Coder. The namespace deletion finishes, and the operator logs the key IDs with reason `ControlPlaneGone`.
+    When you delete the namespace, Kubernetes deletes the control plane and the `CoderAIGatewayKey` objects at the same time. The control plane is usually gone before the operator handles the objects, so the finalizers go without a delete, and the keys stay valid in Coder. The namespace deletion finishes, and the operator logs the key IDs with reason `ControlPlaneGone`. A [sweep](#cleanup-records) deletes the keys only when the namespace comes back with a control plane on the same Coder database. Until then they stay valid.
 
 Delete the `CoderAIGatewayKey` objects first and wait until they are gone, then delete the control plane or the namespace:
 
@@ -191,7 +200,7 @@ A deletion that keeps failing never ends by itself. Leave it in one of these way
 
 ## Known limits
 
-- **Leftover keys.** A key that commits more than 5 minutes after a lost create answer, or after the object is gone, stays in Coder. So do the keys of objects that go together with their control plane, for example in a namespace deletion. Find keys of deleted objects with `coder ai-gateway keys list`: their names start with `ck8s-` and a UID that no `CoderAIGatewayKey` has.
+- **Leftover keys.** A key that commits more than 5 minutes after a lost create answer, or after the object is gone, stays in Coder. So do the keys of objects that go together with their control plane, for example in a namespace deletion, until a [sweep](#cleanup-records) deletes them. Find keys of deleted objects with `coder ai-gateway keys list`: their names start with `ck8s-` and a UID that no `CoderAIGatewayKey` has.
 - **A short key list.** The operator reads all keys with one list call. When a list answer names the current key but leaves out a replaced key, the operator treats the replaced key as deleted. If the key appears again, the operator deletes it as an orphan, without the rest of its grace period. Coder lists all keys in one answer, so only a proxy between the operator and Coder that changes the answer can cause this.
 
 ## Grant access

internal/controller/cleanup_sweep.go

@@ -63,8 +63,8 @@ func init() {
 // CleanupRecordReconciler sweeps the cleanup records of objects that went
 // without a confirmed delete in Coder (#353). It acts only through a ready
 // control plane whose current Coder deployment ID equals the record's, and
-// it deletes only the recorded provider ID or the provider with the
-// recorded nonce.
+// it deletes only the recorded provider ID, the provider with the recorded
+// nonce, or the AI Gateway keys whose names start with the recorded prefix.
 type CleanupRecordReconciler struct {
 	client.Client
 	// APIReader reads the object of a record live, so a stale cache never
@@ -78,20 +78,23 @@ type CleanupRecordReconciler struct {
 }
 
 // SetupWithManager watches the records in the operator namespace. It wakes a
-// record when its CoderAIProvider is deleted, and every record when a control
-// plane gets operator access, since that control plane can match them.
+// record when its CoderAIProvider or CoderAIGatewayKey is deleted, and every
+// record when a control plane gets operator access, since that control plane
+// can match them.
 func (r *CleanupRecordReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	if mgr == nil || r.Client == nil || r.APIReader == nil || r.OperatorNamespace == "" || r.Clock == nil || r.Recorder == nil {
 		return errors.New("assertion failed: the cleanup record reconciler needs a manager, a client, an API reader, the operator namespace, a clock and an event recorder")
 	}
 	isRecord := predicate.NewPredicateFuncs(func(obj client.Object) bool {
 		return obj.GetNamespace() == r.OperatorNamespace && obj.GetLabels()[cleanupRecordLabel] != ""
 	})
+	recordOf := handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request {
+		return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: r.OperatorNamespace, Name: cleanupRecordObjectName(obj.GetUID())}}}
+	})
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.ConfigMap{}, builder.WithPredicates(isRecord)).
-		Watches(&coderv1alpha1.CoderAIProvider{}, handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request {
-			return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: r.OperatorNamespace, Name: cleanupRecordObjectName(obj.GetUID())}}}
-		}), builder.WithPredicates(dependentRemoved)).
+		Watches(&coderv1alpha1.CoderAIProvider{}, recordOf, builder.WithPredicates(dependentRemoved)).
+		Watches(&coderv1alpha1.CoderAIGatewayKey{}, recordOf, builder.WithPredicates(dependentRemoved)).
 		Watches(&coderv1alpha1.CoderControlPlane{}, handler.EnqueueRequestsFromMapFunc(r.allRecords), builder.WithPredicates(operatorAccessGained)).
 		Named("cleanuprecord").
 		Complete(r)
@@ -149,13 +152,14 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		log.FromContext(ctx).Error(err, "Ignoring a ConfigMap that is not a valid cleanup record")
 		return ctrl.Result{}, nil
 	}
-	if record.Kind != cleanupRecordKindAIProvider {
-		return ctrl.Result{}, nil // Only provider records are swept so far.
+	kind, ok := cleanupRecordKinds[record.Kind]
+	if !ok {
+		return ctrl.Result{}, nil // A kind that this operator does not sweep.
 	}
 
 	// Act only when the object with that UID no longer exists (invariant 6).
-	provider := &coderv1alpha1.CoderAIProvider{}
-	if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: record.Namespace, Name: record.Name}, provider); err == nil && provider.UID == record.UID {
+	object := kind.object()
+	if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: record.Namespace, Name: record.Name}, object); err == nil && object.GetUID() == record.UID {
 		r.setUnmatched(record.UID, record.Kind, false)
 		return ctrl.Result{RequeueAfter: cleanupObjectRecheck}, nil
 	} else if err != nil && !apierrors.IsNotFound(err) {
@@ -181,23 +185,23 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	if len(matches) == 0 {
 		r.setUnmatched(record.UID, record.Kind, true)
 		r.Recorder.Eventf(configMap, nil, corev1.EventTypeWarning, "NoMatchingDeployment", "Sweep",
-			"No ready CoderControlPlane in namespace %s reaches Coder deployment %s, which holds the provider of deleted %s %s/%s. Delete the provider in Coder by hand, then delete this ConfigMap.",
-			record.Namespace, record.DeploymentID, record.Kind, record.Namespace, record.Name)
+			"No ready CoderControlPlane in namespace %s reaches Coder deployment %s, which holds %s of deleted %s %s/%s. Delete %s in Coder by hand, then delete this ConfigMap.",
+			record.Namespace, record.DeploymentID, kind.what, record.Kind, record.Namespace, record.Name, kind.what)
 		return ctrl.Result{RequeueAfter: cleanupUnmatchedRecheck}, nil
 	}
 	r.setUnmatched(record.UID, record.Kind, false)
-	logger := log.FromContext(ctx).WithValues("record", configMap.Name, "deploymentID", record.DeploymentID, "provider", record.Namespace+"/"+record.Name)
+	logger := log.FromContext(ctx).WithValues("record", configMap.Name, "deploymentID", record.DeploymentID, "object", record.Kind+" "+record.Namespace+"/"+record.Name)
 	// Each matching control plane has its own operator token, so a failure
 	// through one tries the next.
 	gone, why, through := false, "", logger
 	for _, m := range matches {
 		through = logger.WithValues("controlPlane", types.NamespacedName{Namespace: m.controlPlane.Namespace, Name: m.controlPlane.Name})
-		if gone, why, err = sweepAIProvider(ctx, m.sdk, record); err != nil {
+		if gone, why, err = kind.sweep(ctx, m.sdk, record); err != nil {
 			return ctrl.Result{}, err
 		} else if gone {
 			break
 		}
-		through.Info("The cleanup of a deleted provider in Coder failed", "why", why)
+		through.Info("The cleanup of a deleted object in Coder failed", "why", why)
 	}
 	if !gone {
 		return ctrl.Result{RequeueAfter: cleanupRetry}, nil
@@ -207,10 +211,26 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, err
 	}
 	cleanupRecordsSwept.WithLabelValues(record.Kind).Inc()
-	through.Info("Swept the provider of a deleted CoderAIProvider in Coder", "result", why, "providerID", record.CoderID)
+	through.Info("Swept the Coder resources of a deleted object", "result", why, "coderID", record.CoderID)
 	return ctrl.Result{}, nil
 }
 
+// cleanupSweepKind is how a sweep handles the records of one object kind.
+type cleanupSweepKind struct {
+	object func() client.Object
+	what   string // What the object holds in Coder, for the Event.
+	sweep  func(context.Context, *codersdk.Client, *cleanupRecord) (gone bool, why string, err error)
+}
+
+var cleanupRecordKinds = map[string]cleanupSweepKind{
+	cleanupRecordKindAIProvider: {
+		object: func() client.Object { return &coderv1alpha1.CoderAIProvider{} }, what: "the provider", sweep: sweepAIProvider,
+	},
+	cleanupRecordKindAIGatewayKey: {
+		object: func() client.Object { return &coderv1alpha1.CoderAIGatewayKey{} }, what: "the AI Gateway keys", sweep: sweepAIGatewayKeys,
+	},
+}
+
 // controlPlaneMatch is a control plane whose Coder deployment is a record's.
 type controlPlaneMatch struct {
 	sdk          *codersdk.Client
@@ -309,6 +329,44 @@ func sweepAIProvider(ctx context.Context, sdk *codersdk.Client, record *cleanupR
 	return true, "deleted provider " + id, nil
 }
 
+// sweepAIGatewayKeys deletes the keys of record in Coder: every key whose
+// name is the record's prefix ck8s-<uid>- and a serial (invariant 5). The
+// object is gone, so no serial bound applies: the settle window has passed,
+// and no create of the object can still commit. It returns gone only when a
+// LIST, read by a site owner, shows no such key.
+func sweepAIGatewayKeys(ctx context.Context, sdk *codersdk.Client, record *cleanupRecord) (gone bool, why string, err error) {
+	prefix, err := aiGatewayKeyNamePrefix(record.UID)
+	if err != nil {
+		return false, "", err
+	}
+	if record.Nonce != prefix {
+		return false, "", fmt.Errorf("assertion failed: cleanup record of %s has key prefix %q, not the prefix of its UID", record.UID, record.Nonce)
+	}
+	if step := requireSiteOwner(ctx, sdk); step != nil {
+		// Only a site owner's LIST shows every key.
+		return false, step.message, nil
+	}
+	deleted := 0
+	for range 2 {
+		listed, err := sdk.ListAIGatewayKeys(ctx)
+		if err != nil {
+			return false, "list AI Gateway keys: " + coderErrorSummary(err), nil
+		}
+		own := ownAIGatewayKeys(prefix, listed)
+		if len(own) == 0 {
+			return true, fmt.Sprintf("deleted %d keys, and a list shows none left", deleted), nil
+		}
+		for _, k := range own {
+			// Only Coder's own 404 proves that the key is gone (#325).
+			if err := sdk.DeleteAIGatewayKey(ctx, k.ID); err != nil && !isCoderResourceNotFound(err) {
+				return false, "delete key " + k.Name + ": " + coderErrorSummary(err), nil
+			}
+			deleted++
+		}
+	}
+	return false, "Coder still lists keys of the object after their delete", nil
+}
+
 // setUnmatched keeps the unmatched gauge equal to the records that no
 // deployment reaches.
 func (r *CleanupRecordReconciler) setUnmatched(uid types.UID, kind string, unmatched bool) {
@@ -317,7 +375,7 @@ func (r *CleanupRecordReconciler) setUnmatched(uid types.UID, kind string, unmat
 	} else {
 		r.unmatched.Delete(uid)
 	}
-	counts := map[string]float64{cleanupRecordKindAIProvider: 0}
+	counts := map[string]float64{cleanupRecordKindAIProvider: 0, cleanupRecordKindAIGatewayKey: 0}
 	r.unmatched.Range(func(_, v any) bool {
 		counts[v.(string)]++
 		return true