wip

Author: ndk

controllers/serviceaccount_controller.go

@@ -163,7 +163,7 @@ func (r *GrafanaServiceAccountReconciler) Reconcile(ctx context.Context, req ctr
 	removeNoMatchingInstance(&cr.Status.Conditions)
 
 	// 5. For active resources - reconcile the actual state with the desired state (creates, updates, removes as needed)
-	err = r.reconcile(ctx, gClient, cr)
+	err = r.reconcileWithInstance(ctx, gClient, cr)
 
 	// 6. Update the resource status with current state and conditions
 	applyErrors := map[string]string{}
@@ -215,6 +215,8 @@ func (r *GrafanaServiceAccountReconciler) finalize(ctx context.Context, cr *v1be
 	return nil
 }
 
+// lookupGrafana retrieves the Grafana instance referenced by the GrafanaServiceAccount
+// and validates that it's in a ready state for accepting API requests.
 func (r *GrafanaServiceAccountReconciler) lookupGrafana(
 	ctx context.Context,
 	cr *v1beta1.GrafanaServiceAccount,
@@ -237,30 +239,75 @@ func (r *GrafanaServiceAccountReconciler) lookupGrafana(
 	return &grafana, nil
 }
 
-// reconcile performs the core reconciliation logic for active resources.
+// reconcileWithInstance performs the core reconciliation logic for active resources.
 //
 // It orchestrates the complete synchronization process:
 // 1. Ensures the service account exists in Grafana (creates if missing)
 // 2. Updates service account properties to match the spec
 // 3. Manages the lifecycle of authentication tokens and their secrets
-func (r *GrafanaServiceAccountReconciler) reconcile(
+func (r *GrafanaServiceAccountReconciler) reconcileWithInstance(
 	ctx context.Context,
 	gClient *genapi.GrafanaHTTPAPI,
 	cr *v1beta1.GrafanaServiceAccount,
 ) error {
-	err := r.ensureAccount(ctx, gClient, cr)
+	err := r.upsertAccount(ctx, gClient, cr)
 	if err != nil {
-		return fmt.Errorf("ensuring account exists: %w", err)
+		return fmt.Errorf("upserting service account: %w", err)
 	}
 
-	err = r.reconcileAccount(ctx, gClient, cr)
+	updateForm := buildUpdateFormIfNeeded(&cr.Spec, cr.Status.Account)
+	if updateForm != nil {
+		update, err := gClient.ServiceAccounts.UpdateServiceAccount(
+			service_accounts.
+				NewUpdateServiceAccountParamsWithContext(ctx).
+				WithServiceAccountID(cr.Status.Account.ID).
+				WithBody(updateForm),
+		)
+		if err != nil {
+			return fmt.Errorf("updating service account: %w", err)
+		}
+
+		cr.Status.Account.IsDisabled = update.Payload.Serviceaccount.IsDisabled
+		cr.Status.Account.Role = update.Payload.Serviceaccount.Role
+		cr.Status.Account.Name = update.Payload.Serviceaccount.Name
+	}
+
+	// Ensure tokens are always sorted for stable ordering
+	defer func() {
+		sort.Slice(cr.Status.Account.Tokens, func(i, j int) bool {
+			return cr.Status.Account.Tokens[i].Name < cr.Status.Account.Tokens[j].Name
+		})
+	}()
+
+	// Phase 1: Prune orphaned secrets and index valid ones
+	secretsByTokenName, err := r.pruneAndIndexSecrets(ctx, cr)
+	if err != nil {
+		return err
+	}
+
+	// Phase 2: Remove outdated tokens (will be recreated with correct configuration)
+	err = r.removeOutdatedTokens(ctx, gClient, cr)
+	if err != nil {
+		return err
+	}
+
+	// Phase 3: Validate existing tokens and restore their secret references
+	err = r.validateAndRestoreTokenSecrets(ctx, gClient, cr, secretsByTokenName)
 	if err != nil {
-		return fmt.Errorf("updating account properties: %w", err)
+		return err
 	}
 
-	err = r.reconcileTokens(ctx, gClient, cr)
+	// Phase 4: Provision missing tokens
+	tokensToCreate := r.determineMissingTokens(cr)
+
+	err = r.provisionTokens(ctx, gClient, cr, tokensToCreate, secretsByTokenName)
 	if err != nil {
-		return fmt.Errorf("managing tokens: %w", err)
+		return err
+	}
+
+	if len(cr.Status.Account.Tokens) != 0 {
+		// Grafana's create token API doesn't return expiration, requiring a separate fetch
+		return r.populateTokenExpirations(ctx, gClient, cr)
 	}
 
 	return nil
@@ -307,206 +354,6 @@ func buildUpdateFormIfNeeded(spec *v1beta1.GrafanaServiceAccountSpec, status *v1
 	return form
 }
 
-// ensureAccount guarantees that a service account exists in Grafana with the desired name.
-// If found, it syncs the account status including tokens. If not found, it creates a new account.
-func (r *GrafanaServiceAccountReconciler) ensureAccount(
-	ctx context.Context,
-	gClient *genapi.GrafanaHTTPAPI,
-	cr *v1beta1.GrafanaServiceAccount,
-) error {
-	// If we have an ID in status, try to fetch the account directly
-	if cr.Status.Account != nil && cr.Status.Account.ID > 0 {
-		retrieve, err := gClient.ServiceAccounts.RetrieveServiceAccountWithParams(
-			service_accounts.
-				NewRetrieveServiceAccountParamsWithContext(ctx).
-				WithServiceAccountID(cr.Status.Account.ID),
-		)
-		if err == nil {
-			return r.populateStatusFromGrafana(ctx, gClient, cr, retrieve.Payload)
-		}
-
-		// ATM, service_accounts.RetrieveServiceAccountNotFound doesn't have Is, Unwrap, Unwrap.
-		// So, we cannot rely only on errors.Is().
-		_, notFound := err.(*service_accounts.RetrieveServiceAccountNotFound) // nolint:errorlint
-		if !notFound && !errors.Is(err, service_accounts.NewRetrieveServiceAccountNotFound()) {
-			return fmt.Errorf("retrieving service account by ID %d: %w", cr.Status.Account.ID, err)
-		}
-
-		// Service account not found, clear the status and create a new one
-		cr.Status.Account = nil
-	}
-
-	// TODO: Handle edge case where service account was created but status update failed
-	//
-	// Problem: If createAccount succeeds but saving the ID to status fails (e.g., network issue,
-	// controller crash), the next reconciliation will try to create a duplicate and fail.
-	//
-	// Solution: Search for existing service account by login before creating.
-	// Grafana generates login as: sa-<orgId>-<normalized-name>
-	// where normalized-name is: name in lowercase, spaces replaced with dashes, double dashes with single.
-	// See: https://github.com/grafana/grafana/blob/v11.3.0/pkg/services/serviceaccounts/database/store.go#L52-L57
-	//
-	// Implementation would require searching the service account using a reproduced Grafana login.
-
-	// No ID in status or account not found, create a new one
-	if err := r.createAccount(ctx, gClient, cr); err != nil {
-		return fmt.Errorf("creating service account: %w", err)
-	}
-
-	return nil
-}
-
-// populateStatusFromGrafana updates the CR status to reflect the current state in Grafana.
-// This includes the account properties and tokens as they exist in Grafana, regardless of
-// how they got there (operator-managed or manual changes).
-//
-// Note: This does not populate secret references as they cannot be reliably matched
-// with manually created tokens or external modifications.
-func (r *GrafanaServiceAccountReconciler) populateStatusFromGrafana(
-	ctx context.Context,
-	gClient *genapi.GrafanaHTTPAPI,
-	cr *v1beta1.GrafanaServiceAccount,
-	sa *models.ServiceAccountDTO,
-) error {
-	// Preserve existing ID if it was already set (IDs should be immutable)
-	if cr.Status.Account != nil && cr.Status.Account.ID != 0 && cr.Status.Account.ID != sa.ID {
-		return fmt.Errorf("service account ID mismatch detected, preserving existing ID: %d", cr.Status.Account.ID)
-	}
-
-	cr.Status.Account = &v1beta1.GrafanaServiceAccountInfo{
-		ID:         sa.ID,
-		Role:       sa.Role,
-		IsDisabled: sa.IsDisabled,
-		Name:       sa.Name,
-		Login:      sa.Login,
-	}
-
-	// Load existing tokens from Grafana
-	tokenList, err := gClient.ServiceAccounts.ListTokensWithParams(
-		service_accounts.
-			NewListTokensParamsWithContext(ctx).
-			WithServiceAccountID(sa.ID),
-	)
-	if err != nil {
-		return fmt.Errorf("listing tokens: %w", err)
-	}
-
-	cr.Status.Account.Tokens = make([]v1beta1.GrafanaServiceAccountTokenStatus, 0, len(tokenList.Payload))
-	for _, token := range tokenList.Payload {
-		if token != nil {
-			cr.Status.Account.Tokens = append(cr.Status.Account.Tokens, v1beta1.GrafanaServiceAccountTokenStatus{
-				ID:      token.ID,
-				Name:    token.Name,
-				Expires: convertGrafanaExpiration(token.Expiration),
-			})
-		}
-	}
-
-	return nil
-}
-
-// reconcileAccount ensures the service account properties in Grafana match the desired spec.
-// It compares the current state (status) with desired state (spec) and updates if needed.
-func (r *GrafanaServiceAccountReconciler) reconcileAccount(
-	ctx context.Context,
-	gClient *genapi.GrafanaHTTPAPI,
-	cr *v1beta1.GrafanaServiceAccount,
-) error {
-	if cr.Status.Account == nil {
-		return fmt.Errorf("service account status is not initialized")
-	}
-
-	updateForm := buildUpdateFormIfNeeded(&cr.Spec, cr.Status.Account)
-	if updateForm == nil {
-		// No changes needed
-		return nil
-	}
-
-	update, err := gClient.ServiceAccounts.UpdateServiceAccount(
-		service_accounts.
-			NewUpdateServiceAccountParamsWithContext(ctx).
-			WithServiceAccountID(cr.Status.Account.ID).
-			WithBody(updateForm),
-	)
-	if err != nil {
-		return fmt.Errorf("updating service account: %w", err)
-	}
-
-	cr.Status.Account.IsDisabled = update.Payload.Serviceaccount.IsDisabled
-	cr.Status.Account.Role = update.Payload.Serviceaccount.Role
-	cr.Status.Account.Name = update.Payload.Serviceaccount.Name
-
-	return nil
-}
-
-// reconcileTokens ensures that service account tokens in Grafana match the desired spec.
-// It orchestrates a multi-phase process to maintain consistency between Grafana tokens and Kubernetes secrets.
-//
-// The reconciliation process consists of 5 phases:
-//  1. Prune orphaned secrets and index valid ones by token name
-//  2. Remove outdated tokens (not in spec or with wrong expiration)
-//  3. Validate and restore secret references for existing tokens
-//  4. Provision missing tokens with their secrets
-//  5. Populate expiration times (workaround for Grafana API limitation)
-//
-// Tokens in Grafana are immutable (cannot be updated), so any mismatch requires recreation.
-// The function handles cases where either Grafana or Kubernetes resources may have been modified externally.
-func (r *GrafanaServiceAccountReconciler) reconcileTokens(
-	ctx context.Context,
-	gClient *genapi.GrafanaHTTPAPI,
-	cr *v1beta1.GrafanaServiceAccount,
-) error {
-	if cr.Status.Account == nil {
-		return fmt.Errorf("service account status is not initialized")
-	}
-
-	// Ensure tokens are always sorted for stable ordering
-	defer func() {
-		sort.Slice(cr.Status.Account.Tokens, func(i, j int) bool {
-			return cr.Status.Account.Tokens[i].Name < cr.Status.Account.Tokens[j].Name
-		})
-	}()
-
-	// Phase 1: Prune orphaned secrets and index valid ones
-	secretsByTokenName, err := r.pruneAndIndexSecrets(ctx, cr)
-	if err != nil {
-		return err
-	}
-
-	// Phase 2: Remove outdated tokens (will be recreated with correct configuration)
-	err = r.removeOutdatedTokens(ctx, gClient, cr)
-	if err != nil {
-		return err
-	}
-
-	// Phase 3: Validate existing tokens and restore their secret references
-	err = r.validateAndRestoreTokenSecrets(ctx, gClient, cr, secretsByTokenName)
-	if err != nil {
-		return err
-	}
-
-	// Phase 4: Provision missing tokens
-	tokensToCreate := r.determineMissingTokens(cr)
-
-	err = r.provisionTokens(ctx, gClient, cr, tokensToCreate, secretsByTokenName)
-	if err != nil {
-		return err
-	}
-
-	if len(cr.Status.Account.Tokens) == 0 {
-		return nil
-	}
-
-	// Phase 5: Fetch and populate expiration times
-	// Grafana's create token API doesn't return expiration, requiring a separate fetch
-	err = r.populateTokenExpirations(ctx, gClient, cr)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // removeOutdatedTokens removes tokens that are not in the desired spec or have mismatched expiration times.
 // These tokens will be recreated later with the correct configuration.
 func (r *GrafanaServiceAccountReconciler) removeOutdatedTokens(
@@ -764,13 +611,62 @@ func (r *GrafanaServiceAccountReconciler) pruneAndIndexSecrets(
 	return filtered, nil
 }
 
-// createAccount creates a new service account in Grafana and all related resources such as tokens and secrets.
-// This operation isn't atomic, so can succeed partially.
-func (r *GrafanaServiceAccountReconciler) createAccount(
+// upsertAccount ensures a service account exists in Grafana.
+func (r *GrafanaServiceAccountReconciler) upsertAccount(
 	ctx context.Context,
 	gClient *genapi.GrafanaHTTPAPI,
 	cr *v1beta1.GrafanaServiceAccount,
 ) error {
+	if cr.Status.Account != nil {
+		retrieve, err := gClient.ServiceAccounts.RetrieveServiceAccountWithParams(
+			service_accounts.
+				NewRetrieveServiceAccountParamsWithContext(ctx).
+				WithServiceAccountID(cr.Status.Account.ID),
+		)
+		if err == nil {
+			cr.Status.Account = &v1beta1.GrafanaServiceAccountInfo{
+				ID:         retrieve.Payload.ID,
+				Role:       retrieve.Payload.Role,
+				IsDisabled: retrieve.Payload.IsDisabled,
+				Name:       retrieve.Payload.Name,
+				Login:      retrieve.Payload.Login,
+			}
+
+			// Load existing tokens from Grafana
+			tokenList, err := gClient.ServiceAccounts.ListTokensWithParams(
+				service_accounts.
+					NewListTokensParamsWithContext(ctx).
+					WithServiceAccountID(cr.Status.Account.ID),
+			)
+			if err != nil {
+				return fmt.Errorf("listing tokens: %w", err)
+			}
+
+			cr.Status.Account.Tokens = make([]v1beta1.GrafanaServiceAccountTokenStatus, 0, len(tokenList.Payload))
+			for _, token := range tokenList.Payload {
+				if token != nil {
+					cr.Status.Account.Tokens = append(cr.Status.Account.Tokens, v1beta1.GrafanaServiceAccountTokenStatus{
+						ID:      token.ID,
+						Name:    token.Name,
+						Expires: convertGrafanaExpiration(token.Expiration),
+					})
+				}
+			}
+
+			return nil
+		}
+
+		// ATM, service_accounts.RetrieveServiceAccountNotFound doesn't have Is, Unwrap, Unwrap.
+		// So, we cannot rely only on errors.Is().
+		_, notFound := err.(*service_accounts.RetrieveServiceAccountNotFound) // nolint:errorlint
+		if !notFound && !errors.Is(err, service_accounts.NewRetrieveServiceAccountNotFound()) {
+			return fmt.Errorf("retrieving service account by ID %d: %w", cr.Status.Account.ID, err)
+		}
+
+		// Service account not found, clear the status and create a new one
+		cr.Status.Account = nil
+	}
+
 	create, err := gClient.ServiceAccounts.CreateServiceAccount(
 		service_accounts.
 			NewCreateServiceAccountParamsWithContext(ctx).
@@ -784,7 +680,15 @@ func (r *GrafanaServiceAccountReconciler) createAccount(
 		return fmt.Errorf("creating service account: %w", err)
 	}
 
-	return r.populateStatusFromGrafana(ctx, gClient, cr, create.Payload)
+	cr.Status.Account = &v1beta1.GrafanaServiceAccountInfo{
+		ID:         create.Payload.ID,
+		Role:       create.Payload.Role,
+		IsDisabled: create.Payload.IsDisabled,
+		Name:       create.Payload.Name,
+		Login:      create.Payload.Login,
+	}
+
+	return nil
 }
 
 func (r *GrafanaServiceAccountReconciler) removeAccountSecrets(