Introduce access-review source snapshot and normalize naming
Decouple each campaign from the live access-review sources it was started with by introducing a per-campaign source snapshot table (access_review_campaign_sources). The snapshot captures the source name, category, and connector at start time, so a review remains coherent even after the underlying source is edited or deleted. Fetch tracking becomes an append-only log (access_review_campaign_source_fetch_attempts) that preserves every attempt with its own status and error rather than overwriting a single row. Rename the shared access-review tables and enums to use a consistent access_review_ prefix throughout: access_entries → access_review_entries access_sources → access_review_sources access_source_category → access_review_source_category access_entry_* → access_review_entry_* The same rename propagates to every coredata type, service, GraphQL schema, MCP specification, CLI command, frontend component, and e2e test. The accessreview package gains dedicated actions.go and policies.go files for its own IAM policy set, mirroring the agentrun package pattern. Signed-off-by: Bryan Frimin <bryan@probo.com>
This commit is contained in:
@@ -27,6 +27,11 @@ import (
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
)
|
||||
|
||||
// sourceFetchFailureMessage is the generic, user-facing message persisted on a
|
||||
// failed fetch attempt. The raw error is only ever written to the logs so that
|
||||
// internal connector details are never surfaced through the API or UI.
|
||||
const sourceFetchFailureMessage = "We couldn't fetch accounts from this source. Verify the source configuration and try again."
|
||||
|
||||
type sourceFetchHandler struct {
|
||||
svc *Service
|
||||
pg *pg.Client
|
||||
@@ -39,7 +44,7 @@ func NewSourceFetchWorker(
|
||||
pgClient *pg.Client,
|
||||
logger *log.Logger,
|
||||
opts ...worker.Option,
|
||||
) *worker.Worker[coredata.AccessReviewCampaignSourceFetch] {
|
||||
) *worker.Worker[coredata.AccessReviewCampaignSourceFetchAttempt] {
|
||||
h := &sourceFetchHandler{
|
||||
svc: svc,
|
||||
pg: pgClient,
|
||||
@@ -55,44 +60,43 @@ func NewSourceFetchWorker(
|
||||
)
|
||||
}
|
||||
|
||||
func (h *sourceFetchHandler) Claim(ctx context.Context) (coredata.AccessReviewCampaignSourceFetch, error) {
|
||||
var sourceFetch coredata.AccessReviewCampaignSourceFetch
|
||||
func (h *sourceFetchHandler) Claim(ctx context.Context) (coredata.AccessReviewCampaignSourceFetchAttempt, error) {
|
||||
var attempt coredata.AccessReviewCampaignSourceFetchAttempt
|
||||
|
||||
if err := h.pg.WithTx(
|
||||
ctx,
|
||||
func(ctx context.Context, tx pg.Tx) error {
|
||||
if err := sourceFetch.LoadNextQueuedForUpdateSkipLocked(ctx, tx); err != nil {
|
||||
if err := attempt.LoadNextQueuedForUpdateSkipLocked(ctx, tx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
now := time.Now()
|
||||
sourceFetch.Status = coredata.AccessReviewCampaignSourceFetchStatusFetching
|
||||
sourceFetch.AttemptCount++
|
||||
sourceFetch.LastError = nil
|
||||
sourceFetch.StartedAt = new(now)
|
||||
sourceFetch.CompletedAt = nil
|
||||
sourceFetch.UpdatedAt = now
|
||||
attempt.Status = coredata.AccessReviewCampaignSourceFetchStatusFetching
|
||||
attempt.Error = nil
|
||||
attempt.StartedAt = &now
|
||||
attempt.CompletedAt = nil
|
||||
attempt.UpdatedAt = now
|
||||
|
||||
scope := coredata.NewScope(sourceFetch.TenantID)
|
||||
if err := sourceFetch.Update(ctx, tx, scope); err != nil {
|
||||
return fmt.Errorf("cannot update source fetch status: %w", err)
|
||||
scope := coredata.NewScope(attempt.TenantID)
|
||||
if err := attempt.Update(ctx, tx, scope); err != nil {
|
||||
return fmt.Errorf("cannot update fetch attempt status: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
); err != nil {
|
||||
if errors.Is(err, coredata.ErrNoAccessReviewCampaignSourceFetchAvailable) {
|
||||
return coredata.AccessReviewCampaignSourceFetch{}, worker.ErrNoTask
|
||||
if errors.Is(err, coredata.ErrNoAccessReviewCampaignSourceFetchAttemptAvailable) {
|
||||
return coredata.AccessReviewCampaignSourceFetchAttempt{}, worker.ErrNoTask
|
||||
}
|
||||
|
||||
return coredata.AccessReviewCampaignSourceFetch{}, fmt.Errorf("cannot claim source fetch: %w", err)
|
||||
return coredata.AccessReviewCampaignSourceFetchAttempt{}, fmt.Errorf("cannot claim fetch attempt: %w", err)
|
||||
}
|
||||
|
||||
return sourceFetch, nil
|
||||
return attempt, nil
|
||||
}
|
||||
|
||||
func (h *sourceFetchHandler) Process(ctx context.Context, sourceFetch coredata.AccessReviewCampaignSourceFetch) error {
|
||||
return h.handle(ctx, &sourceFetch)
|
||||
func (h *sourceFetchHandler) Process(ctx context.Context, attempt coredata.AccessReviewCampaignSourceFetchAttempt) error {
|
||||
return h.handle(ctx, &attempt)
|
||||
}
|
||||
|
||||
func (h *sourceFetchHandler) RecoverStale(ctx context.Context) error {
|
||||
@@ -102,18 +106,18 @@ func (h *sourceFetchHandler) RecoverStale(ctx context.Context) error {
|
||||
return h.pg.WithTx(
|
||||
ctx,
|
||||
func(ctx context.Context, tx pg.Tx) error {
|
||||
var fetches coredata.AccessReviewCampaignSourceFetches
|
||||
var attempts coredata.AccessReviewCampaignSourceFetchAttempts
|
||||
|
||||
count, err := fetches.RecoverStale(ctx, tx, staleThreshold, now)
|
||||
count, err := attempts.RecoverStale(ctx, tx, staleThreshold, now)
|
||||
if err != nil {
|
||||
return fmt.Errorf("cannot recover stale source fetches: %w", err)
|
||||
return fmt.Errorf("cannot recover stale fetch attempts: %w", err)
|
||||
}
|
||||
|
||||
if count > 0 {
|
||||
h.logger.InfoCtx(
|
||||
ctx,
|
||||
"recovered stale source fetches",
|
||||
log.Int64("count", count),
|
||||
"recovered stale fetch attempts",
|
||||
log.Int("count", count),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -124,101 +128,123 @@ func (h *sourceFetchHandler) RecoverStale(ctx context.Context) error {
|
||||
|
||||
func (h *sourceFetchHandler) handle(
|
||||
ctx context.Context,
|
||||
sourceFetch *coredata.AccessReviewCampaignSourceFetch,
|
||||
attempt *coredata.AccessReviewCampaignSourceFetchAttempt,
|
||||
) error {
|
||||
scope := coredata.NewScope(sourceFetch.TenantID)
|
||||
scope := coredata.NewScope(attempt.TenantID)
|
||||
|
||||
campaign, err := h.svc.Campaigns(scope).Get(ctx, sourceFetch.AccessReviewCampaignID)
|
||||
if err != nil {
|
||||
commitErr := h.commitFailedSourceFetch(
|
||||
ctx,
|
||||
sourceFetch,
|
||||
fmt.Errorf("cannot load campaign: %w", err),
|
||||
)
|
||||
campaignSource := &coredata.AccessReviewCampaignSource{}
|
||||
if err := h.loadCampaignSource(ctx, scope, attempt.AccessReviewCampaignSourceID, campaignSource); err != nil {
|
||||
commitErr := h.commitFailedSourceFetch(ctx, attempt, fmt.Errorf("cannot load campaign source: %w", err))
|
||||
if commitErr != nil {
|
||||
return fmt.Errorf("cannot load campaign: %w, and cannot commit failed source fetch: %w", err, commitErr)
|
||||
return fmt.Errorf("cannot load campaign source: %w, and cannot commit failed fetch attempt: %w", err, commitErr)
|
||||
}
|
||||
|
||||
return fmt.Errorf("cannot load campaign source: %w", err)
|
||||
}
|
||||
|
||||
campaign, err := h.svc.GetCampaign(ctx, scope, campaignSource.AccessReviewCampaignID)
|
||||
if err != nil {
|
||||
commitErr := h.commitFailedSourceFetch(ctx, attempt, fmt.Errorf("cannot load campaign: %w", err))
|
||||
if commitErr != nil {
|
||||
return fmt.Errorf("cannot load campaign: %w, and cannot commit failed fetch attempt: %w", err, commitErr)
|
||||
}
|
||||
|
||||
return fmt.Errorf("cannot load campaign: %w", err)
|
||||
}
|
||||
|
||||
count, err := h.svc.Engine(scope).FetchSource(ctx, campaign, sourceFetch.AccessSourceID)
|
||||
count, err := h.svc.FetchSource(ctx, scope, campaign, campaignSource)
|
||||
if err != nil {
|
||||
commitErr := h.commitFailedSourceFetch(ctx, sourceFetch, err)
|
||||
if commitErr != nil {
|
||||
return fmt.Errorf("cannot fetch source: %w, and cannot commit failed source fetch: %w", err, commitErr)
|
||||
if commitErr := h.commitFailedSourceFetch(ctx, attempt, err); commitErr != nil {
|
||||
return fmt.Errorf("cannot fetch source: %w, and cannot commit failed fetch attempt: %w", err, commitErr)
|
||||
}
|
||||
|
||||
if finalizeErr := h.finalizeCampaignFetchLifecycle(ctx, sourceFetch.TenantID, sourceFetch.AccessReviewCampaignID); finalizeErr != nil {
|
||||
return fmt.Errorf("cannot finalize campaign after failed source fetch: %w", finalizeErr)
|
||||
if finalizeErr := h.finalizeCampaignFetchLifecycle(ctx, attempt.TenantID, campaignSource.AccessReviewCampaignID); finalizeErr != nil {
|
||||
return fmt.Errorf("cannot finalize campaign after failed fetch attempt: %w", finalizeErr)
|
||||
}
|
||||
|
||||
h.logger.WarnCtx(
|
||||
ctx,
|
||||
"source fetch failed but campaign can continue",
|
||||
log.String("campaign_id", sourceFetch.AccessReviewCampaignID.String()),
|
||||
log.String("access_source_id", sourceFetch.AccessSourceID.String()),
|
||||
log.Error(err),
|
||||
)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
if err := h.commitSuccessfulSourceFetch(ctx, sourceFetch, count); err != nil {
|
||||
return fmt.Errorf("cannot commit successful source fetch: %w", err)
|
||||
if err := h.commitSuccessfulSourceFetch(ctx, attempt, count); err != nil {
|
||||
return fmt.Errorf("cannot commit successful fetch attempt: %w", err)
|
||||
}
|
||||
|
||||
if err := h.finalizeCampaignFetchLifecycle(ctx, sourceFetch.TenantID, sourceFetch.AccessReviewCampaignID); err != nil {
|
||||
if err := h.finalizeCampaignFetchLifecycle(ctx, attempt.TenantID, campaignSource.AccessReviewCampaignID); err != nil {
|
||||
return fmt.Errorf("cannot finalize campaign fetch lifecycle: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (h *sourceFetchHandler) loadCampaignSource(
|
||||
ctx context.Context,
|
||||
scope coredata.Scoper,
|
||||
campaignSourceID gid.GID,
|
||||
campaignSource *coredata.AccessReviewCampaignSource,
|
||||
) error {
|
||||
return h.pg.WithConn(
|
||||
ctx,
|
||||
func(ctx context.Context, conn pg.Querier) error {
|
||||
return campaignSource.LoadByID(ctx, conn, scope, campaignSourceID)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// commitFailedSourceFetch marks the in-flight attempt as failed with a generic,
|
||||
// user-facing message and logs the raw error so the internal detail stays in the
|
||||
// logs only.
|
||||
func (h *sourceFetchHandler) commitFailedSourceFetch(
|
||||
ctx context.Context,
|
||||
sourceFetch *coredata.AccessReviewCampaignSourceFetch,
|
||||
attempt *coredata.AccessReviewCampaignSourceFetchAttempt,
|
||||
failureErr error,
|
||||
) error {
|
||||
var (
|
||||
now = time.Now()
|
||||
errMsg = failureErr.Error()
|
||||
scope = coredata.NewScopeFromObjectID(sourceFetch.AccessReviewCampaignID)
|
||||
h.logger.WarnCtx(
|
||||
ctx,
|
||||
"source fetch failed but campaign can continue",
|
||||
log.String("access_review_campaign_source_id", attempt.AccessReviewCampaignSourceID.String()),
|
||||
log.String("fetch_attempt_id", attempt.ID.String()),
|
||||
log.Error(failureErr),
|
||||
)
|
||||
|
||||
sourceFetch.Status = coredata.AccessReviewCampaignSourceFetchStatusFailed
|
||||
sourceFetch.LastError = &errMsg
|
||||
sourceFetch.CompletedAt = new(now)
|
||||
sourceFetch.UpdatedAt = now
|
||||
var (
|
||||
now = time.Now()
|
||||
errMsg = sourceFetchFailureMessage
|
||||
scope = coredata.NewScope(attempt.TenantID)
|
||||
)
|
||||
|
||||
attempt.Status = coredata.AccessReviewCampaignSourceFetchStatusFailed
|
||||
attempt.Error = &errMsg
|
||||
attempt.CompletedAt = &now
|
||||
attempt.UpdatedAt = now
|
||||
|
||||
return h.pg.WithTx(
|
||||
ctx,
|
||||
func(ctx context.Context, tx pg.Tx) error {
|
||||
return sourceFetch.Update(ctx, tx, scope)
|
||||
return attempt.Update(ctx, tx, scope)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
func (h *sourceFetchHandler) commitSuccessfulSourceFetch(
|
||||
ctx context.Context,
|
||||
sourceFetch *coredata.AccessReviewCampaignSourceFetch,
|
||||
attempt *coredata.AccessReviewCampaignSourceFetchAttempt,
|
||||
fetchedAccountsCount int,
|
||||
) error {
|
||||
var (
|
||||
now = time.Now()
|
||||
scope = coredata.NewScopeFromObjectID(sourceFetch.AccessReviewCampaignID)
|
||||
scope = coredata.NewScope(attempt.TenantID)
|
||||
)
|
||||
|
||||
sourceFetch.Status = coredata.AccessReviewCampaignSourceFetchStatusSuccess
|
||||
sourceFetch.FetchedAccountsCount = fetchedAccountsCount
|
||||
sourceFetch.LastError = nil
|
||||
sourceFetch.CompletedAt = new(now)
|
||||
sourceFetch.UpdatedAt = now
|
||||
attempt.Status = coredata.AccessReviewCampaignSourceFetchStatusSuccess
|
||||
attempt.FetchedAccountsCount = fetchedAccountsCount
|
||||
attempt.Error = nil
|
||||
attempt.CompletedAt = &now
|
||||
attempt.UpdatedAt = now
|
||||
|
||||
return h.pg.WithTx(
|
||||
ctx,
|
||||
func(ctx context.Context, tx pg.Tx) error {
|
||||
return sourceFetch.Update(ctx, tx, scope)
|
||||
return attempt.Update(ctx, tx, scope)
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -246,17 +272,17 @@ func (h *sourceFetchHandler) finalizeCampaignFetchLifecycle(
|
||||
return nil
|
||||
}
|
||||
|
||||
fetches := coredata.AccessReviewCampaignSourceFetches{}
|
||||
if err := fetches.LoadByCampaignID(ctx, tx, scope, campaignID); err != nil {
|
||||
return fmt.Errorf("cannot load source fetches: %w", err)
|
||||
latest := coredata.AccessReviewCampaignSourceFetchAttempts{}
|
||||
if err := latest.LoadLatestByCampaignID(ctx, tx, scope, campaignID); err != nil {
|
||||
return fmt.Errorf("cannot load latest fetch attempts: %w", err)
|
||||
}
|
||||
|
||||
if len(fetches) == 0 {
|
||||
if len(latest) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, fetch := range fetches {
|
||||
if !fetch.Status.IsTerminal() {
|
||||
for _, attempt := range latest {
|
||||
if !attempt.Status.IsTerminal() {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user