Skip to content

Commit 2a065e8

Browse files
ostermanclaudeaknysh
authored
feat: version-aware JIT source provisioning with TTL-based cleanup (#2010)
* fix: JIT source provisioning now takes precedence over local components When both source.uri and provision.workdir.enabled are configured on a component, the JIT source provisioner now always runs, even if a local component already exists. This ensures that source + workdir provisioning always vendors from the remote source to the workdir path, respecting the version specified in stack config rather than using a potentially stale local component. Added regression test to verify source provisioning takes precedence when both local component and source config are present. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat: version-aware JIT source provisioning with TTL-based cleanup - Implement intelligent re-provisioning for remote sources based on version/URI changes - Add incremental local sync with per-file checksum comparison using SyncDir - Support TTL-based cleanup for stale workdirs via duration parsing - Move workdir metadata from flat file to .atmos/metadata.json for better organization - Track source_uri, source_version, and last_accessed timestamps - Add new CLI flags: --expired, --ttl, --dry-run for workdir clean command - Update workdir list and show commands with version and access information - Extract duration parsing to new pkg/duration package for reusability Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Reduce cyclomatic and cognitive complexity in workdir/source packages - Extract helper functions to reduce function complexity: - duration.go: Use maps for unit multipliers and keywords, extract parseInteger/parseWithSuffix/parseKeyword - provision_hook.go: Extract isNonEmptyDir and checkMetadataChanges - clean.go: Extract checkWorkdirExpiry, getLastAccessedTime, getModTimeFromEntry - fs.go: Extract syncSourceToDest, fileNeedsCopy, deleteRemovedFiles - workdir.go: Extract validateComponentPath, computeContentHash, create localMetadataParams struct - Pass localMetadataParams by pointer to avoid hugeParam warning Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Update source-provisioning example to use demo-library Replace terraform-null-label (which is a module, not a component) with demo-library components that can actually be run with terraform apply. The example now demonstrates both source types: - weather: LOCAL source (../demo-library/weather) - ipinfo: REMOTE source (github.com/cloudposse/atmos//examples/demo-library/ipinfo) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments for JIT source provisioning - Add duration overflow guard in ParseDuration (Comment #6) - Fix non-workdir re-provisioning: skip metadata check for non-workdir targets (Comments #7, #11) - Detect version removal: trigger re-provisioning when version is removed (Comments #8, #14) - Fix blog date 2025 → 2026 (Comments #9, #16) - Surface metadata read failures as warnings in ListWorkdirs (Comment #10) - Add periods to comment block in needsProvisioning (Comment #12) - Treat .atmos-only directories as empty in isNonEmptyDir (Comment #13) - Skip .atmos during source walk in syncSourceToDest (Comment #15) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Update golden snapshot for atmos describe config Add the new provision.workdir section to the expected output, matching the new JIT source provisioning configuration schema. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address additional PR review comments - Guard against int64 overflow in parseWithSuffix (Comment #2) - Branch metadata writing by source type - local vs remote (Comment #3) - Add permission checks to fileNeedsCopy for mode changes (Comment #4) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Add tests to improve coverage for workdir and source provisioning Adds comprehensive tests for: - CleanExpiredWorkdirs with mock filesystem - formatDuration for human-readable output - getLastAccessedTime with atime fallback to mtime - checkWorkdirExpiry with valid/corrupt/missing metadata - isLocalSource for local vs remote URI detection Also fixes linter issues: - godot: Fix comment in duration.go - revive: Refactor formatWithOptions to map-based dispatch Addresses CodeRabbit comment #1 requesting improved patch coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Return wrapped error from ReadMetadata instead of warning Changes error handling in ListWorkdirs to return a wrapped error when ReadMetadata fails, surfacing permission/corruption issues to callers. Directories without metadata (metadata == nil) still skip silently. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Add metadata_test.go with tests for UpdateLastAccessed and readMetadataUnlocked - Add buildLocalMetadata tests covering all timestamp preservation branches - Add cleanExpiredWorkdirs and CleanExpiredWorkdirs tests - Fix ListWorkdirs to skip invalid metadata instead of failing entire operation - Fix zero timestamp display to show "-" instead of "0001-01-01" - Fix isLocalSource to use filepath.IsAbs for Windows path support - Fix godot lint issues in log_utils.go Coverage improvements: - pkg/provisioner/workdir: 82.1% -> 88.1% - cmd/terraform/workdir: 58.7% -> 92.2% (function coverage) - UpdateLastAccessed: 0% -> 84.2% - readMetadataUnlocked: 0% -> 100% - buildLocalMetadata: 57% -> 100% - cleanExpiredWorkdirs: 0% -> 100% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Add JIT source provisioning tests for destroy and init commands Add test coverage to confirm that JIT source provisioning correctly takes precedence over local components for all terraform subcommands, not just plan. These tests verify that when: - source.uri is configured - provision.workdir.enabled: true - A local component exists at components/terraform/<component>/ The workdir is populated from the remote source, NOT copied from the local component. This confirms the fix in ExecuteTerraform() works universally for destroy and init commands. Uses table-driven test pattern to avoid code duplication. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Expand JIT source tests to cover all terraform subcommands Expand table-driven test to verify JIT source provisioning works for all 22 terraform subcommands that operate on a component with a stack: Core execution: apply, deploy, destroy, init, workspace State/resource: console, force-unlock, get, graph, import, output, refresh, show, state, taint, untaint Validation/info: metadata, modules, providers, test, validate All commands correctly trigger JIT source provisioning when: - source.uri is configured - provision.workdir.enabled: true - A local component exists The workdir is populated from remote source, not local component. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Make tests fail-fast instead of silently skipping when files don't exist - Verify context.tf exists (proving remote source was used) - Assert main.tf does NOT exist (proving local component wasn't copied) - Remove unused strings import - Update roadmap with JIT source provisioning precedence milestone - Update vendoring initiative progress from 86% to 89% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Add JIT source provisioning to generate commands (#2019) - Add JIT source provisioning to terraform generate varfile - Add JIT source provisioning to terraform generate backend - Add JIT source provisioning to helmfile generate varfile - Add JIT source provisioning to packer output - Update golden snapshot for secrets-masking_describe_config test The generate commands were missing JIT source provisioning that exists in ExecuteTerraform(), causing them to fail with JIT-vendored components. This fix adds the same pattern to all affected commands. Closes #2019 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Add automatic component refresh milestone to roadmap Add new milestone to Vendoring & Resilience initiative: - "Automatic component refresh on version changes" - Links to PR #2010 and version-aware-jit-provisioning blog post - Update progress from 89% to 95% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Add tests for workdir clean command edge cases - Add tests for workdir show command scenarios - Add duration parsing tests for TTL validation - Add filesystem tests for workdir operations - Add metadata lock tests for Unix file locking Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Windows test compatibility and improve error hint accuracy - Skip permission-based tests on Windows (Unix permissions not supported) - TestFileNeedsCopy_DifferentPermissions - TestCopyFile_PreservesPermissions - TestServiceProvision_WriteMetadataFails (read-only dirs work differently) - Use actual componentPath in error hint instead of hardcoded path Addresses CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address CodeRabbit review comments - Wrap auto-provision error with ErrSourceProvision sentinel (packer_output.go) - Add error wrapping with ErrWorkdirMetadata in Windows metadata loader - Document circular import limitation preventing cmd.NewTestKit usage Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Use runtime.GOOS instead of os.Getenv for Windows detection GOOS is a compile-time constant, not a runtime environment variable. os.Getenv("GOOS") returns empty unless explicitly set. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: Ignore flaky kubernetes.io URLs in link checker The kubernetes.io domain frequently has connection failures/timeouts in CI, causing spurious link check failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve JIT source test assertions with explicit failure Instead of silently passing when main.tf doesn't exist, the tests now: - Explicitly fail if main.tf exists (unexpected) - Read and check for LOCAL_VERSION_MARKER to provide better diagnostics - Use t.Fatalf to fail fast with clear error messages Addresses CodeRabbit feedback about test assertion clarity. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: improve coverage for JIT source provisioning Add comprehensive tests for: - pkg/provisioner/workdir/metadata.go: - MetadataPath function - WriteMetadata with all fields populated - ReadMetadata new location priority over legacy - UpdateLastAccessed preserves all fields - pkg/provisioner/workdir/clean.go: - checkWorkdirExpiry for expired/non-expired workdirs - getModTimeFromEntry - findExpiredWorkdirs with mixed workdirs - CleanExpiredWorkdirs with empty base path - Clean with Expired option precedence - formatDuration edge cases - pkg/provisioner/workdir/fs.go: - DefaultPathFilter.Match with patterns - SyncDir with nested directories - SyncDir updating changed files - pkg/provisioner/source/provision_hook.go: - checkMetadataChanges with version scenarios - isNonEmptyDir edge cases - needsProvisioning for non-workdir targets - writeWorkdirMetadata source type detection - writeWorkdirMetadata preserving ContentHash Coverage improvements: - workdir package: ~79% → 92.5% - source package: ~76% → 83.6% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> Co-authored-by: aknysh <andriy.knysh@gmail.com>
1 parent 2779cf9 commit 2a065e8

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+4578
-364
lines changed

cmd/terraform/workdir/mock_workdir_manager_test.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/terraform/workdir/workdir_clean.go

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,19 @@ var cleanCmd = &cobra.Command{
2121
Long: `Remove component working directories.
2222
2323
Use --all to clean all workdirs, or specify a component and stack to clean a specific workdir.
24+
Use --expired with --ttl to clean only workdirs that haven't been accessed within the TTL.
2425
Workdirs are ephemeral and can be regenerated by running 'atmos terraform init'.`,
2526
Example: ` # Clean a specific workdir
2627
atmos terraform workdir clean vpc --stack dev
2728
2829
# Clean all workdirs
29-
atmos terraform workdir clean --all`,
30+
atmos terraform workdir clean --all
31+
32+
# Clean expired workdirs (not accessed in 7 days)
33+
atmos terraform workdir clean --expired --ttl=7d
34+
35+
# Dry run - show what would be cleaned
36+
atmos terraform workdir clean --expired --ttl=24h --dry-run`,
3037
Args: cobra.MaximumNArgs(1),
3138
RunE: func(cmd *cobra.Command, args []string) error {
3239
defer perf.Track(atmosConfigPtr, "workdir.clean.RunE")()
@@ -37,28 +44,48 @@ Workdirs are ephemeral and can be regenerated by running 'atmos terraform init'.
3744
return err
3845
}
3946
all := v.GetBool("all")
47+
expired := v.GetBool("expired")
48+
ttl := v.GetString("ttl")
49+
dryRun := v.GetBool("dry-run")
4050
stack := v.GetString("stack")
4151

4252
// Validate arguments.
43-
if all && len(args) > 0 {
44-
return errUtils.Build(errUtils.ErrWorkdirClean).
45-
WithExplanation("Cannot specify both --all and a component").
46-
WithHint("Use --all alone to clean all workdirs, or specify a component and stack").
47-
Err()
48-
}
49-
50-
if !all && len(args) == 0 {
51-
return errUtils.Build(errUtils.ErrWorkdirClean).
52-
WithExplanation("Either --all or a component is required").
53-
WithHint("Use --all to clean all workdirs, or specify a component with --stack").
54-
Err()
55-
}
56-
57-
if !all && stack == "" {
58-
return errUtils.Build(errUtils.ErrWorkdirClean).
59-
WithExplanation("Stack is required when cleaning a specific workdir").
60-
WithHint("Use --stack to specify the stack").
61-
Err()
53+
if expired {
54+
// --expired mode: requires --ttl, ignores --all and component.
55+
if ttl == "" {
56+
return errUtils.Build(errUtils.ErrWorkdirClean).
57+
WithExplanation("TTL is required when using --expired").
58+
WithHint("Specify a TTL like --ttl=7d or --ttl=24h").
59+
Err()
60+
}
61+
if all || len(args) > 0 {
62+
return errUtils.Build(errUtils.ErrWorkdirClean).
63+
WithExplanation("Cannot use --all or component with --expired").
64+
WithHint("Use --expired --ttl=<duration> alone to clean expired workdirs").
65+
Err()
66+
}
67+
} else {
68+
// Non-expired mode: standard validation.
69+
if all && len(args) > 0 {
70+
return errUtils.Build(errUtils.ErrWorkdirClean).
71+
WithExplanation("Cannot specify both --all and a component").
72+
WithHint("Use --all alone to clean all workdirs, or specify a component and stack").
73+
Err()
74+
}
75+
76+
if !all && len(args) == 0 {
77+
return errUtils.Build(errUtils.ErrWorkdirClean).
78+
WithExplanation("Either --all, --expired, or a component is required").
79+
WithHint("Use --all to clean all workdirs, --expired --ttl to clean stale workdirs, or specify a component with --stack").
80+
Err()
81+
}
82+
83+
if !all && len(args) > 0 && stack == "" {
84+
return errUtils.Build(errUtils.ErrWorkdirClean).
85+
WithExplanation("Stack is required when cleaning a specific workdir").
86+
WithHint("Use --stack to specify the stack").
87+
Err()
88+
}
6289
}
6390

6491
// Initialize config with global flags (--base-path, --config, etc.).
@@ -71,7 +98,10 @@ Workdirs are ephemeral and can be regenerated by running 'atmos terraform init'.
7198
Err()
7299
}
73100

74-
// Clean workdirs.
101+
// Clean workdirs based on mode.
102+
if expired {
103+
return cleanExpiredWorkdirs(&atmosConfig, ttl, dryRun)
104+
}
75105
if all {
76106
return cleanAllWorkdirs(&atmosConfig)
77107
}
@@ -103,14 +133,27 @@ func cleanSpecificWorkdir(atmosConfig *schema.AtmosConfiguration, component, sta
103133
return nil
104134
}
105135

136+
func cleanExpiredWorkdirs(atmosConfig *schema.AtmosConfiguration, ttl string, dryRun bool) error {
137+
defer perf.Track(atmosConfig, "workdir.cleanExpiredWorkdirs")()
138+
139+
// Use the workdir manager's CleanExpiredWorkdirs.
140+
return workdirManager.CleanExpiredWorkdirs(atmosConfig, ttl, dryRun)
141+
}
142+
106143
func init() {
107144
cleanCmd.DisableFlagParsing = false
108145

109146
// Create parser with functional options.
110147
cleanParser = flags.NewStandardParser(
111148
flags.WithStackFlag(),
112149
flags.WithBoolFlag("all", "a", false, "Clean all workdirs"),
150+
flags.WithBoolFlag("expired", "e", false, "Clean only expired workdirs (requires --ttl)"),
151+
flags.WithStringFlag("ttl", "t", "", "TTL for expired cleanup (e.g., 7d, 24h, weekly)"),
152+
flags.WithBoolFlag("dry-run", "n", false, "Show what would be cleaned without deleting"),
113153
flags.WithEnvVars("all", "ATMOS_WORKDIR_CLEAN_ALL"),
154+
flags.WithEnvVars("expired", "ATMOS_WORKDIR_CLEAN_EXPIRED"),
155+
flags.WithEnvVars("ttl", "ATMOS_WORKDIR_TTL"),
156+
flags.WithEnvVars("dry-run", "ATMOS_WORKDIR_DRY_RUN"),
114157
)
115158

116159
// Register flags with the command.

cmd/terraform/workdir/workdir_clean_cmd_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,3 +404,135 @@ func TestCleanSpecificWorkdir_VariousInputs(t *testing.T) {
404404
})
405405
}
406406
}
407+
408+
func TestCleanExpiredWorkdirs_Success(t *testing.T) {
409+
ctrl := gomock.NewController(t)
410+
defer ctrl.Finish()
411+
412+
mock := NewMockWorkdirManager(ctrl)
413+
mock.EXPECT().CleanExpiredWorkdirs(gomock.Any(), "7d", false).Return(nil)
414+
415+
// Save and restore.
416+
original := workdirManager
417+
defer func() { workdirManager = original }()
418+
SetWorkdirManager(mock)
419+
420+
atmosConfig := &schema.AtmosConfiguration{}
421+
err := cleanExpiredWorkdirs(atmosConfig, "7d", false)
422+
assert.NoError(t, err)
423+
}
424+
425+
func TestCleanExpiredWorkdirs_DryRun(t *testing.T) {
426+
ctrl := gomock.NewController(t)
427+
defer ctrl.Finish()
428+
429+
mock := NewMockWorkdirManager(ctrl)
430+
mock.EXPECT().CleanExpiredWorkdirs(gomock.Any(), "30d", true).Return(nil)
431+
432+
// Save and restore.
433+
original := workdirManager
434+
defer func() { workdirManager = original }()
435+
SetWorkdirManager(mock)
436+
437+
atmosConfig := &schema.AtmosConfiguration{}
438+
err := cleanExpiredWorkdirs(atmosConfig, "30d", true)
439+
assert.NoError(t, err)
440+
}
441+
442+
func TestCleanExpiredWorkdirs_Error(t *testing.T) {
443+
ctrl := gomock.NewController(t)
444+
defer ctrl.Finish()
445+
446+
mock := NewMockWorkdirManager(ctrl)
447+
expectedErr := errUtils.Build(errUtils.ErrWorkdirClean).
448+
WithExplanation("invalid TTL").
449+
Err()
450+
mock.EXPECT().CleanExpiredWorkdirs(gomock.Any(), "invalid", false).Return(expectedErr)
451+
452+
// Save and restore.
453+
original := workdirManager
454+
defer func() { workdirManager = original }()
455+
SetWorkdirManager(mock)
456+
457+
atmosConfig := &schema.AtmosConfiguration{}
458+
err := cleanExpiredWorkdirs(atmosConfig, "invalid", false)
459+
assert.Error(t, err)
460+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
461+
}
462+
463+
// RunE validation tests - test the command entry point validation logic.
464+
465+
// resetViperForTest resets viper flags for test isolation.
466+
// Note: cmd.NewTestKit cannot be used here due to circular import between
467+
// cmd/terraform/workdir and cmd packages.
468+
func resetViperForTest(t *testing.T) {
469+
t.Helper()
470+
v := viper.GetViper()
471+
v.Set("all", false)
472+
v.Set("expired", false)
473+
v.Set("ttl", "")
474+
v.Set("dry-run", false)
475+
v.Set("stack", "")
476+
}
477+
478+
func TestCleanCmd_RunE_ExpiredWithoutTTL(t *testing.T) {
479+
resetViperForTest(t)
480+
v := viper.GetViper()
481+
v.Set("expired", true)
482+
v.Set("ttl", "") // No TTL provided.
483+
484+
err := cleanCmd.RunE(cleanCmd, []string{})
485+
assert.Error(t, err)
486+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
487+
}
488+
489+
func TestCleanCmd_RunE_ExpiredWithAll(t *testing.T) {
490+
resetViperForTest(t)
491+
v := viper.GetViper()
492+
v.Set("expired", true)
493+
v.Set("ttl", "7d")
494+
v.Set("all", true) // Cannot use --all with --expired.
495+
496+
err := cleanCmd.RunE(cleanCmd, []string{})
497+
assert.Error(t, err)
498+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
499+
}
500+
501+
func TestCleanCmd_RunE_ExpiredWithComponent(t *testing.T) {
502+
resetViperForTest(t)
503+
v := viper.GetViper()
504+
v.Set("expired", true)
505+
v.Set("ttl", "7d")
506+
507+
err := cleanCmd.RunE(cleanCmd, []string{"vpc"}) // Component arg with --expired.
508+
assert.Error(t, err)
509+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
510+
}
511+
512+
func TestCleanCmd_RunE_ActualCall_AllWithComponent(t *testing.T) {
513+
resetViperForTest(t)
514+
v := viper.GetViper()
515+
v.Set("all", true)
516+
517+
err := cleanCmd.RunE(cleanCmd, []string{"vpc"}) // --all with component arg.
518+
assert.Error(t, err)
519+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
520+
}
521+
522+
func TestCleanCmd_RunE_ActualCall_NoFlagsNoArgs(t *testing.T) {
523+
resetViperForTest(t)
524+
525+
err := cleanCmd.RunE(cleanCmd, []string{}) // No flags, no args.
526+
assert.Error(t, err)
527+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
528+
}
529+
530+
func TestCleanCmd_RunE_ActualCall_ComponentWithoutStack(t *testing.T) {
531+
resetViperForTest(t)
532+
v := viper.GetViper()
533+
v.Set("stack", "") // No stack provided.
534+
535+
err := cleanCmd.RunE(cleanCmd, []string{"vpc"}) // Component without --stack.
536+
assert.Error(t, err)
537+
assert.ErrorIs(t, err, errUtils.ErrWorkdirClean)
538+
}

0 commit comments

Comments
 (0)