Skip to content

fix(scheduler/aws): tag rec rows with registered host-account UUID instead of nil when only ambient AWS account is configured #604

Description

@cristim

Symptom

Approving a pending purchase shows the Account column as (ambient) even though the deployment HAS a registered AWS account (CUDly host (909626172446)) that matches the Lambda's own STS identity.

Account     Provider  Service  Resource   Engine  Region     Count  Term     Payment      Upfront  Monthly savings  Eff. savings %
(ambient)   aws       ec2      t4g.nano   —       us-east-1  1      1 Year   no-upfront   $0       $1               37.9%

Confusing for the operator — looks like the purchase will hit some unnamed account, when in fact it WILL hit the registered CUDly host account (same AWS account ID).

Root cause

internal/scheduler/scheduler.go::collectAWSRecommendations (line 436+):

accounts := s.enabledAccounts(ctx, "aws")
if len(accounts) == 0 {
    recs, err := s.collectAWSAmbient(ctx, globalCfg)
    // ...
    return recs, []string{""}, nil  // CloudAccountID stays nil on each rec
}

Two paths land here with the symptom:

  1. Account registered but disabledenabledAccounts filters out disabled rows. Ambient fallback fires. Recs are saved with CloudAccountID = nil. The frontend's formatAccountLabel (approval-details.ts:241-249) falls through to '(ambient)'.
  2. Recs predate the account registration — collected on the ambient path before the operator added the host account, persisted with CloudAccountID = nil. The next collection cycle DOES tag new recs correctly, but the old pending purchase still carries the stale nil.

Fix

Collector-side (forward-fix)

In collectAWSAmbient (or in collectAWSRecommendations before delegating), call STS GetCallerIdentity once to get the host account ID. Look it up in the registered-accounts table by external_id. If a row exists (regardless of enabled status — registering the account is itself a strong signal it should be labeled), stamp that account's UUID onto every rec the ambient path returns.

hostAccountID, err := getSTSAccountID(ctx)
if err == nil {
    hostAcct, lookupErr := s.config.GetCloudAccountByExternalID(ctx, "aws", hostAccountID)
    if lookupErr == nil && hostAcct != nil {
        // Tag every rec with the registered account's UUID
        for i := range recs {
            id := hostAcct.ID  // copy for &
            recs[i].CloudAccountID = &id
        }
    }
}

If GetCloudAccountByExternalID doesn't exist yet, add it (cheap: indexed query on cloud_accounts(provider, external_id)).

Backfill (optional, one-shot)

For existing rec rows already in the DB with CloudAccountID = nil: a one-shot SQL UPDATE that joins on STS-derived account ID. Either:

  • (a) A migration that runs once: UPDATE recommendations SET cloud_account_id = ca.id FROM cloud_accounts ca WHERE recommendations.cloud_account_id IS NULL AND recommendations.provider = ca.provider AND ca.external_id = <host_sts_account_id_from_env>. The host account ID can't be hardcoded into a migration (varies per deployment); instead, the next collection cycle naturally heals the data as recs get re-upserted with the right ID.
  • (b) Skip backfill; let natural re-collection heal. Document in the PR that existing pending purchases will continue to show (ambient) until they're either approved/cancelled or the recommendation row gets re-upserted by the next collection cycle.

Pick (b) — backfill migrations are risky and the symptom is cosmetic on already-persisted rows. New recs will be correct immediately.

Regression test

Add a unit test in internal/scheduler/scheduler_test.go:

  • Mock STS returns account 909626172446.
  • Mock enabledAccounts(ctx, "aws") returns empty (forces ambient path).
  • Mock GetCloudAccountByExternalID(ctx, "aws", "909626172446") returns a registered CloudAccount{ID: "abc-uuid", ExternalID: "909626172446", Enabled: false} (disabled, but still registered).
  • Call collectAWSRecommendations.
  • Assert each returned rec has CloudAccountID = &"abc-uuid".

Also a negative-path test: when the STS account is NOT in the registered table, recs keep CloudAccountID = nil (preserves the truly-orphan case).

Severity

P2 — operator-confusing UX on the approve modal. Doesn't affect the actual purchase outcome (the purchase still hits the right AWS account because the Lambda's STS identity is correct) but undermines trust in the dashboard's labelling.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions