fix setup PAT rollback behavior

This commit is contained in:
jnfrati
2026-04-30 15:14:29 +02:00
parent 2cb5cf75e6
commit 663d028cec
4 changed files with 54 additions and 15 deletions

View File

@@ -83,7 +83,8 @@ func normalizeSetupOptions(opts SetupOptions, setupPATEnabled bool) (SetupOption
// SetupOwner creates the initial owner user and, when requested and enabled by
// SetupPATEnabledEnvKey, provisions the account and a setup Personal Access
// Token. If account or PAT provisioning fails, created resources are rolled
// back so setup can be retried.
// back so setup can be retried. If account rollback fails, user rollback is
// skipped to avoid leaving an account without its owner user.
func (m *SetupService) SetupOwner(ctx context.Context, email, password, name string, opts SetupOptions) (*SetupResult, error) {
opts, err := normalizeSetupOptions(opts, m.setupPATEnabled)
if err != nil {
@@ -113,14 +114,18 @@ func (m *SetupService) SetupOwner(ctx context.Context, email, password, name str
accountID, err := m.accountManager.GetAccountIDByUserID(ctx, userAuth)
if err != nil {
err = fmt.Errorf("create account for setup user: %w", err)
m.rollbackSetup(ctx, userData.ID, "account provisioning failed", err, "")
if rollbackErr := m.rollbackSetup(ctx, userData.ID, "account provisioning failed", err, ""); rollbackErr != nil {
return nil, fmt.Errorf("%w; failed to roll back setup resources: %v", err, rollbackErr)
}
return nil, err
}
pat, err := m.accountManager.CreatePAT(ctx, accountID, userData.ID, userData.ID, setupPATTokenName, *opts.PATExpireInDays)
if err != nil {
err = fmt.Errorf("create setup PAT: %w", err)
m.rollbackSetup(ctx, userData.ID, "setup PAT provisioning failed", err, accountID)
if rollbackErr := m.rollbackSetup(ctx, userData.ID, "setup PAT provisioning failed", err, accountID); rollbackErr != nil {
return nil, fmt.Errorf("%w; failed to roll back setup resources: %v", err, rollbackErr)
}
return nil, err
}
@@ -128,28 +133,33 @@ func (m *SetupService) SetupOwner(ctx context.Context, email, password, name str
return result, nil
}
func (m *SetupService) rollbackSetup(ctx context.Context, userID, reason string, origErr error, accountID string) {
func (m *SetupService) rollbackSetup(ctx context.Context, userID, reason string, origErr error, accountID string) error {
if accountID == "" {
resolvedAccountID, err := m.lookupSetupAccountIDForRollback(ctx, userID)
if err != nil {
log.WithContext(ctx).Errorf("failed to resolve setup account for user %s after %s: original error: %v, rollback error: %v", userID, reason, origErr, err)
rollbackErr := fmt.Errorf("resolve setup account for rollback: %w", err)
log.WithContext(ctx).Errorf("failed to resolve setup account for user %s after %s: original error: %v, rollback error: %v", userID, reason, origErr, rollbackErr)
return rollbackErr
}
accountID = resolvedAccountID
}
if accountID != "" {
if err := m.rollbackSetupAccount(ctx, accountID); err != nil {
log.WithContext(ctx).Errorf("failed to roll back setup account %s for user %s after %s: original error: %v, rollback error: %v", accountID, userID, reason, origErr, err)
} else {
log.WithContext(ctx).Warnf("rolled back setup account %s for user %s after %s: %v", accountID, userID, reason, origErr)
rollbackErr := fmt.Errorf("roll back setup account %s: %w", accountID, err)
log.WithContext(ctx).Errorf("failed to roll back setup account %s for user %s after %s: original error: %v, rollback error: %v", accountID, userID, reason, origErr, rollbackErr)
return rollbackErr
}
log.WithContext(ctx).Warnf("rolled back setup account %s for user %s after %s: %v", accountID, userID, reason, origErr)
}
if err := m.instanceManager.RollbackSetup(ctx, userID); err != nil {
log.WithContext(ctx).Errorf("failed to roll back setup user %s after %s: original error: %v, rollback error: %v", userID, reason, origErr, err)
return
rollbackErr := fmt.Errorf("roll back setup user %s: %w", userID, err)
log.WithContext(ctx).Errorf("failed to roll back setup user %s after %s: original error: %v, rollback error: %v", userID, reason, origErr, rollbackErr)
return rollbackErr
}
log.WithContext(ctx).Warnf("rolled back setup user %s after %s: %v", userID, reason, origErr)
return nil
}
func (m *SetupService) lookupSetupAccountIDForRollback(ctx context.Context, userID string) (string, error) {

View File

@@ -153,9 +153,11 @@ func TestSetupOwner_AccountProvisioningFails_RollsBackSideEffectAccountAndUser(t
accountStore.EXPECT().DeleteAccount(gomock.Any(), account).Return(nil)
rolledBackFor := ""
rollbackCalls := 0
setupManager := NewSetupService(
&setupInstanceManagerMock{
rollbackSetupFn: func(_ context.Context, userID string) error {
rollbackCalls++
rolledBackFor = userID
return nil
},
@@ -180,6 +182,7 @@ func TestSetupOwner_AccountProvisioningFails_RollsBackSideEffectAccountAndUser(t
assert.Nil(t, result)
assert.Contains(t, err.Error(), "create account for setup user")
assert.Equal(t, "owner-id", rolledBackFor)
assert.Equal(t, 1, rollbackCalls)
}
func TestSetupOwner_CreatePATFails_RollsBackSetupAccountAndUser(t *testing.T) {
@@ -238,9 +241,11 @@ func TestSetupOwner_CreatePATFails_AccountAlreadyGoneStillRollsBackUser(t *testi
accountStore.EXPECT().GetAccount(gomock.Any(), "acc-1").Return(nil, status.NewAccountNotFoundError("acc-1"))
rolledBackFor := ""
rollbackCalls := 0
setupManager := NewSetupService(
&setupInstanceManagerMock{
rollbackSetupFn: func(_ context.Context, userID string) error {
rollbackCalls++
rolledBackFor = userID
return nil
},
@@ -267,9 +272,10 @@ func TestSetupOwner_CreatePATFails_AccountAlreadyGoneStillRollsBackUser(t *testi
assert.Nil(t, result)
assert.Contains(t, err.Error(), "create setup PAT")
assert.Equal(t, "owner-id", rolledBackFor)
assert.Equal(t, 1, rollbackCalls)
}
func TestSetupOwner_CreatePATFails_AccountRollbackFailureStillRollsBackUser(t *testing.T) {
func TestSetupOwner_CreatePATFails_AccountRollbackFailureStopsBeforeUserRollback(t *testing.T) {
t.Setenv(SetupPATEnabledEnvKey, "true")
ctrl := gomock.NewController(t)
@@ -278,11 +284,11 @@ func TestSetupOwner_CreatePATFails_AccountRollbackFailureStillRollsBackUser(t *t
accountStore.EXPECT().GetAccount(gomock.Any(), "acc-1").Return(account, nil)
accountStore.EXPECT().DeleteAccount(gomock.Any(), account).Return(errors.New("delete failed"))
rolledBackFor := ""
rollbackCalls := 0
setupManager := NewSetupService(
&setupInstanceManagerMock{
rollbackSetupFn: func(_ context.Context, userID string) error {
rolledBackFor = userID
rollbackCalls++
return nil
},
},
@@ -307,5 +313,6 @@ func TestSetupOwner_CreatePATFails_AccountRollbackFailureStillRollsBackUser(t *t
require.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "create setup PAT")
assert.Equal(t, "owner-id", rolledBackFor)
assert.Contains(t, err.Error(), "failed to roll back setup resources")
assert.Equal(t, 0, rollbackCalls)
}