wip

Author: ndk

controllers/serviceaccount_controller.go

@@ -201,15 +201,42 @@ func (r *GrafanaServiceAccountReconciler) finalize(ctx context.Context, cr *v1be
 		return err
 	}
 
-	// Try to create client and remove account if Grafana instance exists
 	gClient, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, grafana)
 	if err != nil {
 		return fmt.Errorf("creating Grafana client: %w", err)
 	}
 
-	err = r.removeAccount(ctx, gClient, cr)
+	_, err = gClient.ServiceAccounts.DeleteServiceAccountWithParams( // nolint:errcheck
+		service_accounts.
+			NewDeleteServiceAccountParamsWithContext(ctx).
+			WithServiceAccountID(cr.Status.Account.ID),
+	)
 	if err != nil {
-		return fmt.Errorf("removing service account: %w", err)
+		// ATM, service_accounts.DeleteServiceAccountNotFound doesn't have Is, Unwrap, Unwrap.
+		// So, we cannot rely only on errors.Is().
+		_, ok := err.(*service_accounts.DeleteServiceAccountNotFound) // nolint:errorlint
+		if ok || errors.Is(err, service_accounts.NewDeleteServiceAccountNotFound()) {
+			logf.FromContext(ctx).Info("service account not found, skipping removal",
+				"serviceAccountID", cr.Status.Account.ID,
+				"serviceAccountName", cr.Spec.Name,
+			)
+
+			return nil
+		}
+
+		// TODO: Grafana Operator currently deploys Grafana 11.3.0 (see controllers/config/operator_constants.go#L6).
+		// Until Grafana 12.0.2 there was no reliable way to detect a 404 when deleting a service account.
+		// The API still returns 500 (see https://github.com/grafana/grafana/issues/106618).
+		//
+		// Once we upgrade to Grafana > 12.0.2 and bump github.com/grafana/grafana-openapi-client-go,
+		// we can handle the real 404 explicitly.
+		//
+		// In the meantime we treat any non-nil error from the delete call as "already removed" and just log it for visibility.
+		logf.FromContext(ctx).Error(err, "failed to delete service account (may already be deleted)",
+			"serviceAccountID", cr.Status.Account.ID,
+			"serviceAccountName", cr.Spec.Name,
+		)
+		// return fmt.Errorf("deleting service account %q: %w", status.SpecID, err)
 	}
 
 	return nil
@@ -691,70 +718,6 @@ func (r *GrafanaServiceAccountReconciler) upsertAccount(
 	return nil
 }
 
-func (r *GrafanaServiceAccountReconciler) removeAccountSecrets(
-	ctx context.Context,
-	cr *v1beta1.GrafanaServiceAccount,
-) error {
-	for i := range cr.Status.Account.Tokens {
-		err := r.removeTokenSecret(ctx, &cr.Status.Account.Tokens[i])
-		if err != nil {
-			return fmt.Errorf("removing token secret %q: %w", cr.Status.Account.Tokens[i].Name, err)
-		}
-	}
-
-	return nil
-}
-
-func (r *GrafanaServiceAccountReconciler) removeAccount(
-	ctx context.Context,
-	gClient *genapi.GrafanaHTTPAPI,
-	cr *v1beta1.GrafanaServiceAccount,
-) error {
-	// We don't need to remove tokens, because they will be removed automatically.
-	// The only thing we need to do is to remove the secrets.
-	err := r.removeAccountSecrets(ctx, cr)
-	if err != nil {
-		return fmt.Errorf("removing secrets: %w", err)
-	}
-
-	cr.Status.Account.Tokens = nil
-
-	_, err = gClient.ServiceAccounts.DeleteServiceAccountWithParams( // nolint:errcheck
-		service_accounts.
-			NewDeleteServiceAccountParamsWithContext(ctx).
-			WithServiceAccountID(cr.Status.Account.ID),
-	)
-	if err != nil {
-		// ATM, service_accounts.DeleteServiceAccountNotFound doesn't have Is, Unwrap, Unwrap.
-		// So, we cannot rely only on errors.Is().
-		_, ok := err.(*service_accounts.DeleteServiceAccountNotFound) // nolint:errorlint
-		if ok || errors.Is(err, service_accounts.NewDeleteServiceAccountNotFound()) {
-			logf.FromContext(ctx).Info("service account not found, skipping removal",
-				"serviceAccountID", cr.Status.Account.ID,
-				"serviceAccountName", cr.Spec.Name,
-			)
-
-			return nil
-		}
-
-		// TODO: Grafana Operator currently deploys Grafana 11.3.0 (see controllers/config/operator_constants.go#L6).
-		// Until Grafana 12.0.2 there was no reliable way to detect a 404 when deleting a service account.
-		// The API still returns 500 (see https://github.com/grafana/grafana/issues/106618).
-		//
-		// Once we upgrade to Grafana > 12.0.2 and bump github.com/grafana/grafana-openapi-client-go,
-		// we can handle the real 404 explicitly.
-		//
-		// In the meantime we treat any non-nil error from the delete call as "already removed" and just log it for visibility.
-		logf.FromContext(ctx).Error(err, "failed to delete service account (may already be deleted)",
-			"serviceAccountID", cr.Status.Account.ID,
-			"serviceAccountName", cr.Spec.Name,
-		)
-		// return fmt.Errorf("deleting service account %q: %w", status.SpecID, err)
-	}
-
-	return nil
-}
-
 func (r *GrafanaServiceAccountReconciler) removeTokenSecret(
 	ctx context.Context,
 	tokenStatus *v1beta1.GrafanaServiceAccountTokenStatus,