Skip to content

Commit 0141ddd

Browse files
ostermanclaude
andcommitted
fix: Defer warnings until after spinner completes to prevent concurrent output
- Add ProvisionResult type to collect warnings from backend provisioning - Update BackendCreateFunc to return (*ProvisionResult, error) - Update applyS3BucketDefaults to return warnings instead of printing directly - Display warnings AFTER spinner completes to avoid text corruption - Add tests verifying warning collection for existing buckets This fixes the concurrent output issue where ui.Warning() was called inside the spinner callback, causing text interleaving and display corruption (e.g., bucket name + component name merged together). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent da2bbe2 commit 0141ddd

File tree

8 files changed

+141
-89
lines changed

8 files changed

+141
-89
lines changed

pkg/provisioner/backend/backend.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@ import (
1010
"github.com/cloudposse/atmos/pkg/schema"
1111
)
1212

13+
// ProvisionResult holds the result of a backend provisioning operation.
14+
type ProvisionResult struct {
15+
Warnings []string // Warning messages to display after spinner completes.
16+
}
17+
1318
// BackendCreateFunc is a function that creates a Terraform backend.
1419
type BackendCreateFunc func(
1520
ctx context.Context,
1621
atmosConfig *schema.AtmosConfiguration,
1722
backendConfig map[string]any,
1823
authContext *schema.AuthContext,
19-
) error
24+
) (*ProvisionResult, error)
2025

2126
// BackendDeleteFunc is a function that deletes a Terraform backend.
2227
type BackendDeleteFunc func(
@@ -190,35 +195,35 @@ func BackendName(backendType string, backendConfig map[string]any) string {
190195
}
191196

192197
// ProvisionBackend provisions a backend if provisioning is enabled.
193-
// Returns an error if provisioning fails or no provisioner is registered.
198+
// Returns ProvisionResult with any warnings, and an error if provisioning fails or no provisioner is registered.
194199
func ProvisionBackend(
195200
ctx context.Context,
196201
atmosConfig *schema.AtmosConfiguration,
197202
componentConfig map[string]any,
198203
authContext *schema.AuthContext,
199-
) error {
204+
) (*ProvisionResult, error) {
200205
defer perf.Track(atmosConfig, "backend.ProvisionBackend")()
201206

202207
// Check if provisioning is enabled.
203208
provision, ok := componentConfig["provision"].(map[string]any)
204209
if !ok {
205-
return errUtils.Build(errUtils.ErrProvisioningNotConfigured).
210+
return nil, errUtils.Build(errUtils.ErrProvisioningNotConfigured).
206211
WithExplanation("No 'provision' configuration found for this component").
207212
WithHint("Add 'provision.backend.enabled: true' to the component's stack configuration").
208213
Err()
209214
}
210215

211216
backend, ok := provision["backend"].(map[string]any)
212217
if !ok {
213-
return errUtils.Build(errUtils.ErrProvisioningNotConfigured).
218+
return nil, errUtils.Build(errUtils.ErrProvisioningNotConfigured).
214219
WithExplanation("No 'provision.backend' configuration found for this component").
215220
WithHint("Add 'provision.backend.enabled: true' to the component's stack configuration").
216221
Err()
217222
}
218223

219224
enabled, ok := backend["enabled"].(bool)
220225
if !ok || !enabled {
221-
return errUtils.Build(errUtils.ErrProvisioningNotConfigured).
226+
return nil, errUtils.Build(errUtils.ErrProvisioningNotConfigured).
222227
WithExplanation("Backend provisioning is not enabled for this component").
223228
WithHint("Set 'provision.backend.enabled: true' in the component's stack configuration").
224229
Err()
@@ -227,18 +232,18 @@ func ProvisionBackend(
227232
// Get backend configuration.
228233
backendConfig, ok := componentConfig["backend"].(map[string]any)
229234
if !ok {
230-
return fmt.Errorf("%w: backend configuration not found", errUtils.ErrBackendNotFound)
235+
return nil, fmt.Errorf("%w: backend configuration not found", errUtils.ErrBackendNotFound)
231236
}
232237

233238
backendType, ok := componentConfig["backend_type"].(string)
234239
if !ok {
235-
return fmt.Errorf("%w: backend_type not specified", errUtils.ErrBackendTypeRequired)
240+
return nil, fmt.Errorf("%w: backend_type not specified", errUtils.ErrBackendTypeRequired)
236241
}
237242

238243
// Get create function for backend type.
239244
createFunc := GetBackendCreate(backendType)
240245
if createFunc == nil {
241-
return fmt.Errorf("%w: %s", errUtils.ErrCreateNotImplemented, backendType)
246+
return nil, fmt.Errorf("%w: %s", errUtils.ErrCreateNotImplemented, backendType)
242247
}
243248

244249
// Execute create function.

pkg/provisioner/backend/backend_test.go

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ func TestRegisterBackendCreate(t *testing.T) {
2424
// Reset registry before test.
2525
resetBackendRegistry()
2626

27-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
28-
return nil
27+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
28+
return &ProvisionResult{}, nil
2929
}
3030

3131
RegisterBackendCreate("s3", mockProvisioner)
@@ -46,12 +46,12 @@ func TestGetBackendCreate_MultipleTypes(t *testing.T) {
4646
// Reset registry before test.
4747
resetBackendRegistry()
4848

49-
s3Provisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
50-
return nil
49+
s3Provisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
50+
return &ProvisionResult{}, nil
5151
}
5252

53-
gcsProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
54-
return nil
53+
gcsProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
54+
return &ProvisionResult{}, nil
5555
}
5656

5757
RegisterBackendCreate("s3", s3Provisioner)
@@ -109,8 +109,8 @@ func TestGetBackendDelete_MultipleTypes(t *testing.T) {
109109

110110
func TestResetRegistryForTesting(t *testing.T) {
111111
// Register some functions first.
112-
mockCreator := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
113-
return nil
112+
mockCreator := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
113+
return &ProvisionResult{}, nil
114114
}
115115
mockDeleter := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext, force bool) error {
116116
return nil
@@ -135,8 +135,8 @@ func TestResetRegistryForTesting_ClearsAllEntries(t *testing.T) {
135135
// Reset at start.
136136
ResetRegistryForTesting()
137137

138-
mockCreator := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
139-
return nil
138+
mockCreator := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
139+
return &ProvisionResult{}, nil
140140
}
141141
mockDeleter := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext, force bool) error {
142142
return nil
@@ -180,7 +180,7 @@ func TestProvisionBackend_NoProvisioningConfiguration(t *testing.T) {
180180
},
181181
}
182182

183-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
183+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
184184
require.Error(t, err, "Should return error when no provisioning configuration exists")
185185
assert.ErrorIs(t, err, errUtils.ErrProvisioningNotConfigured)
186186
}
@@ -203,7 +203,7 @@ func TestProvisionBackend_NoBackendProvisioningConfiguration(t *testing.T) {
203203
},
204204
}
205205

206-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
206+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
207207
require.Error(t, err, "Should return error when no backend provisioning configuration exists")
208208
assert.ErrorIs(t, err, errUtils.ErrProvisioningNotConfigured)
209209
}
@@ -226,7 +226,7 @@ func TestProvisionBackend_ProvisioningDisabled(t *testing.T) {
226226
},
227227
}
228228

229-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
229+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
230230
require.Error(t, err, "Should return error when provisioning is explicitly disabled")
231231
assert.ErrorIs(t, err, errUtils.ErrProvisioningNotConfigured)
232232
}
@@ -247,7 +247,7 @@ func TestProvisionBackend_ProvisioningEnabledMissingField(t *testing.T) {
247247
},
248248
}
249249

250-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
250+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
251251
require.Error(t, err, "Should return error when enabled field is missing")
252252
assert.ErrorIs(t, err, errUtils.ErrProvisioningNotConfigured)
253253
}
@@ -266,7 +266,7 @@ func TestProvisionBackend_MissingBackendConfiguration(t *testing.T) {
266266
},
267267
}
268268

269-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
269+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
270270
require.Error(t, err)
271271
assert.ErrorIs(t, err, errUtils.ErrBackendNotFound)
272272
assert.Contains(t, err.Error(), "backend configuration not found")
@@ -289,7 +289,7 @@ func TestProvisionBackend_MissingBackendType(t *testing.T) {
289289
},
290290
}
291291

292-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
292+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
293293
require.Error(t, err)
294294
assert.ErrorIs(t, err, errUtils.ErrBackendTypeRequired)
295295
assert.Contains(t, err.Error(), "backend_type not specified")
@@ -315,7 +315,7 @@ func TestProvisionBackend_UnsupportedBackendType(t *testing.T) {
315315
},
316316
}
317317

318-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
318+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
319319
require.Error(t, err)
320320
assert.ErrorIs(t, err, errUtils.ErrCreateNotImplemented)
321321
assert.Contains(t, err.Error(), "unsupported")
@@ -332,11 +332,11 @@ func TestProvisionBackend_Success(t *testing.T) {
332332
var capturedBackendConfig map[string]any
333333
var capturedAuthContext *schema.AuthContext
334334

335-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
335+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
336336
provisionerCalled = true
337337
capturedBackendConfig = backendConfig
338338
capturedAuthContext = authContext
339-
return nil
339+
return &ProvisionResult{}, nil
340340
}
341341

342342
RegisterBackendCreate("s3", mockProvisioner)
@@ -354,7 +354,7 @@ func TestProvisionBackend_Success(t *testing.T) {
354354
},
355355
}
356356

357-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
357+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
358358
require.NoError(t, err)
359359
assert.True(t, provisionerCalled, "Provisioner should have been called")
360360
assert.NotNil(t, capturedBackendConfig)
@@ -372,9 +372,9 @@ func TestProvisionBackend_WithAuthContext(t *testing.T) {
372372

373373
var capturedAuthContext *schema.AuthContext
374374

375-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
375+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
376376
capturedAuthContext = authContext
377-
return nil
377+
return &ProvisionResult{}, nil
378378
}
379379

380380
RegisterBackendCreate("s3", mockProvisioner)
@@ -399,7 +399,7 @@ func TestProvisionBackend_WithAuthContext(t *testing.T) {
399399
},
400400
}
401401

402-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, authContext)
402+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, authContext)
403403
require.NoError(t, err)
404404
require.NotNil(t, capturedAuthContext)
405405
require.NotNil(t, capturedAuthContext.AWS)
@@ -414,8 +414,8 @@ func TestProvisionBackend_ProvisionerFailure(t *testing.T) {
414414
ctx := context.Background()
415415
atmosConfig := &schema.AtmosConfiguration{}
416416

417-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
418-
return errors.New("bucket creation failed: permission denied")
417+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
418+
return nil, errors.New("bucket creation failed: permission denied")
419419
}
420420

421421
RegisterBackendCreate("s3", mockProvisioner)
@@ -433,7 +433,7 @@ func TestProvisionBackend_ProvisionerFailure(t *testing.T) {
433433
},
434434
}
435435

436-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
436+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
437437
require.Error(t, err)
438438
assert.Contains(t, err.Error(), "bucket creation failed")
439439
assert.Contains(t, err.Error(), "permission denied")
@@ -449,14 +449,14 @@ func TestProvisionBackend_MultipleBackendTypes(t *testing.T) {
449449
s3Called := false
450450
gcsCalled := false
451451

452-
mockS3Provisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
452+
mockS3Provisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
453453
s3Called = true
454-
return nil
454+
return &ProvisionResult{}, nil
455455
}
456456

457-
mockGCSProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
457+
mockGCSProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
458458
gcsCalled = true
459-
return nil
459+
return &ProvisionResult{}, nil
460460
}
461461

462462
RegisterBackendCreate("s3", mockS3Provisioner)
@@ -476,7 +476,7 @@ func TestProvisionBackend_MultipleBackendTypes(t *testing.T) {
476476
},
477477
}
478478

479-
err := ProvisionBackend(ctx, atmosConfig, componentConfigS3, nil)
479+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfigS3, nil)
480480
require.NoError(t, err)
481481
assert.True(t, s3Called, "S3 provisioner should have been called")
482482
assert.False(t, gcsCalled, "GCS provisioner should not have been called")
@@ -499,7 +499,7 @@ func TestProvisionBackend_MultipleBackendTypes(t *testing.T) {
499499
},
500500
}
501501

502-
err = ProvisionBackend(ctx, atmosConfig, componentConfigGCS, nil)
502+
_, err = ProvisionBackend(ctx, atmosConfig, componentConfigGCS, nil)
503503
require.NoError(t, err)
504504
assert.False(t, s3Called, "S3 provisioner should not have been called")
505505
assert.True(t, gcsCalled, "GCS provisioner should have been called")
@@ -515,11 +515,11 @@ func TestConcurrentBackendProvisioning(t *testing.T) {
515515
var callCount int
516516
var mu sync.Mutex
517517

518-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
518+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
519519
mu.Lock()
520520
callCount++
521521
mu.Unlock()
522-
return nil
522+
return &ProvisionResult{}, nil
523523
}
524524

525525
RegisterBackendCreate("s3", mockProvisioner)
@@ -550,7 +550,7 @@ func TestConcurrentBackendProvisioning(t *testing.T) {
550550
"backend": baseComponentConfig["backend"],
551551
"provision": baseComponentConfig["provision"],
552552
}
553-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
553+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
554554
assert.NoError(t, err)
555555
}()
556556
}
@@ -603,9 +603,9 @@ func TestProvisionBackend_EnabledWrongType(t *testing.T) {
603603
resetBackendRegistry()
604604

605605
provisionerCalled := false
606-
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) error {
606+
mockProvisioner := func(ctx context.Context, atmosConfig *schema.AtmosConfiguration, backendConfig map[string]any, authContext *schema.AuthContext) (*ProvisionResult, error) {
607607
provisionerCalled = true
608-
return nil
608+
return &ProvisionResult{}, nil
609609
}
610610

611611
RegisterBackendCreate("s3", mockProvisioner)
@@ -623,7 +623,7 @@ func TestProvisionBackend_EnabledWrongType(t *testing.T) {
623623
},
624624
}
625625

626-
err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
626+
_, err := ProvisionBackend(ctx, atmosConfig, componentConfig, nil)
627627
if tt.shouldError {
628628
require.Error(t, err)
629629
assert.ErrorIs(t, err, errUtils.ErrProvisioningNotConfigured)

0 commit comments

Comments
 (0)