feat(catalog): add discovery block for custom module paths #5030
Conversation
|
@shuv0id is attempting to deploy a commit to the Gruntwork Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds discovery-based catalog configuration and configurable module search paths, gates discovery behind a new catalog-discovery experiment, plumbs module path options through NewRepo via functional RepoOpt, updates catalog loading to aggregate discoveries, and adds tests, fixtures, and docs for the feature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant CatalogSvc as CatalogService
participant RepoFactory as NewRepoFunc
participant Repo
rect rgb(235,245,255)
note left of CatalogSvc: Legacy flow (no discoveries)
Client->>CatalogSvc: Load(cfg with top-level URLs)
CatalogSvc->>RepoFactory: NewRepoFunc(cloneURL, path)
RepoFactory->>Repo: NewRepo(... default modulePaths)
Repo->>Repo: FindModules() // searches "modules"
Repo-->>CatalogSvc: modules
CatalogSvc-->>Client: aggregated modules
end
rect rgb(255,245,235)
note left of CatalogSvc: Discovery-enabled flow
Client->>CatalogSvc: Load(cfg with Discoveries)
CatalogSvc->>CatalogSvc: check catalog-discovery experiment
CatalogSvc->>CatalogSvc: loadModulesFromDiscoveries()
loop per Discovery
CatalogSvc->>RepoFactory: NewRepoFunc(cloneURL, path, WithModulePaths)
RepoFactory->>Repo: NewRepo(..., apply opts -> modulePaths)
Repo->>Repo: FindModules() // searches configured modulePaths
Repo-->>CatalogSvc: modules
end
CatalogSvc-->>Client: aggregated & deduped modules
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Questions / Notes for Maintainers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-monitoring/README.md (1)
17-50: Module source path example may need clarification for test context.Line 19 shows a module source path:
git::https://github.com/example-org/terraform-modules.git//platform/terraform-aws-monitoring. For a test fixture inplatform-modules/, consider whether this example should reference a structure that aligns more directly with how the test demonstrates the discovery feature, or if an inline comment clarifying this is a generic example would help future maintainers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
cli/commands/catalog/catalog_test.go(1 hunks)cli/commands/catalog/tui/model_test.go(1 hunks)config/catalog.go(2 hunks)config/catalog_test.go(1 hunks)docs-starlight/src/content/docs/04-reference/04-experiments.md(2 hunks)internal/experiment/experiment.go(2 hunks)internal/services/catalog/catalog.go(4 hunks)internal/services/catalog/catalog_test.go(7 hunks)internal/services/catalog/module/repo.go(4 hunks)internal/services/catalog/module/repo_test.go(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/HEAD(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/config(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-security-group/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-vpc/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-eks/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-monitoring/README.md(1 hunks)test/fixtures/catalog/config5.hcl(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
config/catalog_test.gocli/commands/catalog/catalog_test.gointernal/services/catalog/module/repo_test.gointernal/experiment/experiment.gointernal/services/catalog/catalog.goconfig/catalog.gointernal/services/catalog/module/repo.gocli/commands/catalog/tui/model_test.gointernal/services/catalog/catalog_test.go
docs-starlight/**/*.md*
⚙️ CodeRabbit configuration file
Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation in
docsto the Starlight + Astro based documentation indocs-starlight. Make sure that thedocs-starlightdocumentation is accurate and up-to-date with thedocsdocumentation, and that any difference between them results in an improvement in thedocs-starlightdocumentation.
Files:
docs-starlight/src/content/docs/04-reference/04-experiments.md
🧠 Learnings (1)
📚 Learning: 2025-02-10T13:36:19.542Z
Learnt from: levkohimins
PR: gruntwork-io/terragrunt#3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
Applied to files:
internal/services/catalog/catalog_test.go
🧬 Code graph analysis (8)
config/catalog_test.go (1)
config/catalog.go (2)
CatalogConfig(46-52)Discovery(41-44)
cli/commands/catalog/catalog_test.go (1)
internal/services/catalog/module/repo.go (2)
RepoOpt(58-58)Repo(43-56)
internal/services/catalog/module/repo_test.go (2)
internal/services/catalog/module/repo.go (2)
NewRepo(60-86)WithModulePaths(90-94)test/helpers/logger/logger.go (1)
CreateLogger(9-14)
internal/services/catalog/catalog.go (6)
internal/services/catalog/module/repo.go (3)
RepoOpt(58-58)Repo(43-56)WithModulePaths(90-94)config/catalog.go (1)
Discovery(41-44)internal/experiment/experiment.go (3)
CatalogDiscovery(39-39)Symlinks(18-18)CAS(25-25)pkg/log/log.go (4)
Warn(37-39)Warnf(92-94)Debugf(72-74)Infof(82-84)util/collections.go (1)
RemoveDuplicatesFromList(87-89)util/hash.go (1)
EncodeBase64Sha1(16-19)
config/catalog.go (1)
util/file.go (1)
FileExists(61-64)
internal/services/catalog/module/repo.go (2)
internal/services/catalog/module/module.go (2)
Modules(24-24)NewModule(37-65)util/file.go (1)
FileExists(61-64)
cli/commands/catalog/tui/model_test.go (1)
internal/services/catalog/module/repo.go (2)
RepoOpt(58-58)Repo(43-56)
internal/services/catalog/catalog_test.go (3)
internal/services/catalog/module/repo.go (3)
RepoOpt(58-58)Repo(43-56)NewRepo(60-86)internal/experiment/experiment.go (3)
NewExperiments(54-89)CatalogDiscovery(39-39)Experiments(49-49)internal/services/catalog/catalog.go (1)
NewCatalogService(70-75)
🔇 Additional comments (25)
internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/config (1)
1-13: Test fixture looks appropriate.The git configuration file is correctly formatted and suitable for testing the custom module path discovery feature. The remote origin points to a Gruntwork terraform module repository, which aligns with the test scenario.
internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-vpc/README.md (1)
1-44: Test fixture README is well-suited for validating discovery in custom module paths.This README serves its purpose as test data for the discovery feature, demonstrating a realistic module structure within the
infra-modules/custom path. The documentation is clear, the Terraform configuration example is valid, and the content appropriately simulates how modules would be documented in repositories with non-default directory structures.internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-monitoring/README.md (1)
1-88: Test fixture verified and working correctly—no issues found.This README is properly referenced and actively used by the test suite to validate custom module path discovery. The test case "single custom path - platform-modules" (in
repo_test.golines 121-134) expects this exact module atplatform-modules/terraform-aws-monitoring, and the fixture documentation appropriately demonstrates realistic module documentation for test purposes.config/catalog_test.go (1)
124-142: LGTM! Test case properly validates Discovery block parsing.The test case correctly validates the new Discovery configuration feature, including both top-level URLs and discovery blocks with custom module paths.
cli/commands/catalog/tui/model_test.go (1)
34-86: LGTM! Signature update maintains consistency across test helpers.The addition of the variadic
optsparameter aligns with the repository options pattern introduced across the codebase, ensuring the mock function signature remains compatible with future extensions.docs-starlight/src/content/docs/04-reference/04-experiments.md (2)
73-73: LGTM! Experiment properly added to active experiments list.
190-229: Excellent documentation for the catalog-discovery experiment.The documentation is comprehensive and includes:
- Clear explanation of what the feature does
- Practical usage example with HCL configuration
- Key features highlighting the capabilities
- Explanation of backward compatibility with standard URLs
test/fixtures/catalog/config5.hcl (1)
1-16: LGTM! Well-structured test fixture for discovery feature.The configuration demonstrates both standard URLs and multiple discovery blocks with custom module paths, providing comprehensive test coverage for the new feature.
internal/experiment/experiment.go (2)
37-39: LGTM! Experiment constant properly defined.The constant follows the established naming convention and includes clear documentation explaining the feature's purpose.
85-87: LGTM! Experiment properly integrated into experiments list.The CatalogDiscovery experiment is correctly added to the default experiments with appropriate default values.
cli/commands/catalog/catalog_test.go (1)
28-28: LGTM! Mock signature properly extended.The variadic
opts ...module.RepoOptparameter correctly aligns with the new repository option pattern introduced across the codebase.internal/services/catalog/module/repo_test.go (1)
84-205: LGTM! Comprehensive test coverage for custom module paths.The test correctly exercises the new
WithModulePathsoption across multiple scenarios (single path, multiple paths, non-existent path). The parallel handling is appropriate—top-level parallelism with sequential subtests to avoid gitdir rename races.config/catalog.go (3)
41-44: LGTM! Discovery struct is well-defined.The struct correctly captures URLs and module paths with appropriate HCL and cty tags for configuration parsing.
54-67: LGTM! String representation provides useful diagnostics.The updated String method provides a helpful summary of discovery blocks and their URLs, aiding in debugging and logging.
87-106: LGTM! Clean normalization helper.The
normalizeURLsfunction correctly handles relative-to-absolute path conversion and is appropriately reused for both top-level URLs and discovery block URLs.internal/services/catalog/catalog_test.go (3)
29-29: LGTM! Mock signatures correctly updated.All
mockNewRepofunctions now accept the variadicopts ...module.RepoOptparameter, maintaining consistency with the new repository option pattern.Also applies to: 108-108, 142-142, 161-161, 193-193
213-289: LGTM! Discovery-enabled test is comprehensive.The test correctly:
- Enables the CatalogDiscovery experiment
- Creates fake repositories with modules in custom paths (tf-modules, infra, terraform)
- Passes RepoOpts through the mock
- Verifies all three modules are discovered across multiple discovery blocks
291-354: LGTM! Discovery-disabled test validates fallback behavior.The test properly verifies that when the CatalogDiscovery experiment is not enabled, only naked URLs are processed and discovery blocks are ignored, maintaining backward compatibility.
internal/services/catalog/module/repo.go (5)
38-38: LGTM! Constant captures default behavior.The
defaultModulePathconstant properly captures the previously hard-coded "modules" directory, maintaining backward compatibility.
52-52: LGTM! Clean option pattern implementation.The
modulePathsfield andRepoOptfunction type follow idiomatic Go patterns for optional configuration.Also applies to: 58-58
60-86: LGTM! NewRepo correctly applies options.The updated signature accepts variadic
RepoOptparameters and applies them after repository initialization, allowing flexible per-repository configuration.
88-94: LGTM! WithModulePaths provides clean option construction.The function follows the option pattern and includes helpful documentation about the default behavior.
99-158: LGTM! FindModules correctly handles multiple paths.The implementation:
- Defaults to
defaultModulePathwhenmodulePathsis empty (maintaining backward compatibility)- Iterates over all configured paths
- Skips non-existent paths gracefully
- Properly handles both absolute and relative module directory paths
internal/services/catalog/catalog.go (2)
96-157: LGTM! Discovery support is well-implemented.The Load method correctly:
- Gates discovery behind the CatalogDiscovery experiment flag
- Warns when discovery blocks exist but the experiment is disabled (Line 118)
- Maintains backward compatibility with naked URLs
- Deduplicates URLs within each discovery
- Delegates to a clean helper function for module loading
159-216: LGTM! Clean helper with proper error handling.The
loadModulesFromDiscoveriesfunction:
- Correctly evaluates experimental feature flags
- Handles empty URLs gracefully (Line 172)
- Conditionally applies
WithModulePathsonly when module paths are configured (Lines 185-187)- Accumulates errors without failing fast, allowing partial success
- Provides informative logging at each stage
Add experimental catalog-discovery feature allowing custom module directory paths via discovery blocks in catalog configuration. Previously, catalog only searched the hardcoded "modules/" directory. This change allows repositories with non-standard layouts (e.g., "tf-modules/", "infrastructure/") to be used in the catalog. Changes: - Add discovery block support to catalog configuration - Add ModulePaths field to config.Discovery struct - Add Repo.WithModulePaths() functional option for custom paths - Support multiple discovery blocks with different configurations - Maintain backward compatibility (naked URLs use default "modules/") - Update tests and documentation Closes gruntwork-io#4632
shuv0id
force-pushed
the
feat/configurable-catalog-module-directory-4632
branch
from
03324a3 to
ca2ab9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
cli/commands/catalog/catalog_test.go(1 hunks)cli/commands/catalog/tui/model_test.go(1 hunks)config/catalog.go(2 hunks)config/catalog_test.go(1 hunks)docs-starlight/src/content/docs/04-reference/04-experiments.md(2 hunks)internal/experiment/experiment.go(2 hunks)internal/services/catalog/catalog.go(4 hunks)internal/services/catalog/catalog_test.go(7 hunks)internal/services/catalog/module/repo.go(4 hunks)internal/services/catalog/module/repo_test.go(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/HEAD(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/config(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-security-group/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-vpc/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-eks/README.md(1 hunks)internal/services/catalog/module/testdata/find_modules_custom_paths/platform-modules/terraform-aws-monitoring/README.md(1 hunks)test/fixtures/catalog/config5.hcl(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-vpc/README.md
🚧 Files skipped from review as they are similar to previous changes (8)
- internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/HEAD
- cli/commands/catalog/catalog_test.go
- config/catalog_test.go
- cli/commands/catalog/tui/model_test.go
- docs-starlight/src/content/docs/04-reference/04-experiments.md
- test/fixtures/catalog/config5.hcl
- internal/services/catalog/module/repo.go
- internal/services/catalog/module/testdata/find_modules_custom_paths/gitdir/config
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
internal/services/catalog/catalog.gointernal/experiment/experiment.gointernal/services/catalog/module/repo_test.goconfig/catalog.gointernal/services/catalog/catalog_test.go
🧠 Learnings (1)
📚 Learning: 2025-02-10T13:36:19.542Z
Learnt from: levkohimins
PR: gruntwork-io/terragrunt#3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
Applied to files:
internal/services/catalog/catalog_test.go
🧬 Code graph analysis (4)
internal/services/catalog/catalog.go (6)
internal/services/catalog/module/repo.go (3)
RepoOpt(58-58)Repo(43-56)WithModulePaths(90-94)config/catalog.go (1)
Discovery(41-44)internal/experiment/experiment.go (4)
Experiments(49-49)CatalogDiscovery(39-39)Symlinks(18-18)CAS(25-25)util/collections.go (1)
RemoveDuplicatesFromList(87-89)util/hash.go (1)
EncodeBase64Sha1(16-19)internal/errors/export.go (1)
Join(17-19)
internal/services/catalog/module/repo_test.go (2)
internal/services/catalog/module/repo.go (2)
NewRepo(60-86)WithModulePaths(90-94)test/helpers/logger/logger.go (1)
CreateLogger(9-14)
config/catalog.go (1)
util/file.go (1)
FileExists(61-64)
internal/services/catalog/catalog_test.go (3)
internal/services/catalog/module/repo.go (3)
RepoOpt(58-58)Repo(43-56)NewRepo(60-86)internal/experiment/experiment.go (3)
NewExperiments(54-89)CatalogDiscovery(39-39)Experiments(49-49)internal/services/catalog/catalog.go (1)
NewCatalogService(70-75)
🔇 Additional comments (8)
internal/services/catalog/module/testdata/find_modules_custom_paths/infra-modules/terraform-aws-security-group/README.md (1)
1-63: Good test fixture for custom-path discovery.This README is well-structured, grammatically sound, and provides clear usage examples. The file appropriately demonstrates a discoverable module in the non-standard
infra-modules/directory, validating the catalog's ability to find modules outside the hardcodedmodules/path—a core goal of this PR.config/catalog.go (2)
41-44: LGTM! Well-designed Discovery configuration structure.The Discovery struct is properly defined with appropriate HCL and cty tags, and its integration into CatalogConfig follows the established patterns in the codebase.
Also applies to: 51-51
87-106: Clean URL normalization with defensive checks.The normalizeURLs helper correctly handles path resolution and preserves non-existent paths as-is, which is appropriate for URLs that might be remote references.
internal/services/catalog/module/repo_test.go (1)
84-205: LGTM! Comprehensive test coverage for custom module paths.The test correctly:
- Uses
nolintdirective with clear justification for non-parallel subtests- Exercises single-path, multi-path, and nonexistent-path scenarios
- Passes
module.WithModulePathsoptions to the repo constructor- Uses
ElementsMatchfor order-independent assertionsThe gitdir rename pattern with defer is safe and appropriate for this test setup.
internal/experiment/experiment.go (1)
37-39: LGTM! Experiment constant properly defined and registered.The CatalogDiscovery experiment follows the established naming and documentation patterns, and is correctly added to the experiments list with StatusOngoing (default state for new experiments).
Also applies to: 85-87
internal/services/catalog/catalog_test.go (2)
213-289: LGTM! Thorough test coverage for discovery-enabled scenario.The test correctly:
- Enables the CatalogDiscovery experiment
- Sets up modules in custom paths (tf-modules, infra, terraform)
- Configures discovery blocks with appropriate module_paths
- Verifies all 3 expected modules are discovered
- Passes repoOpts through to NewRepo correctly
291-354: LGTM! Validates feature gate behavior correctly.This test ensures that when the CatalogDiscovery experiment is disabled, only naked URLs are processed and discovery blocks are ignored. The explicit assertion that the "Compute Module" is not included (lines 350-353) is particularly valuable for preventing regressions.
internal/services/catalog/catalog.go (1)
96-138: LGTM! Discovery-based loading logic is well-structured.The implementation correctly:
- Gates discovery functionality behind the CatalogDiscovery experiment
- Warns users when discovery blocks exist but the feature is disabled (lines 117-119)
- Falls back gracefully to naked URLs when discovery is disabled
- Handles the repoURL override case appropriately (lines 132-138)
The conditional logic for building the discoveries list is sound and maintains backward compatibility.
| func (s *catalogServiceImpl) loadModulesFromDiscoveries(ctx context.Context, l log.Logger, discoveries []config.Discovery) (module.Modules, []error) { | ||
| var ( | ||
| errs []error | ||
| allModules module.Modules | ||
| ) | ||
|
|
||
| // Evaluate experimental features for symlinks and content-addressable storage. | ||
| walkWithSymlinks := s.opts.Experiments.Evaluate(experiment.Symlinks) | ||
| allowCAS := s.opts.Experiments.Evaluate(experiment.CAS) | ||
|
|
||
| for _, discovery := range discoveries { | ||
| for _, currentRepoURL := range discovery.URLs { | ||
| if currentRepoURL == "" { | ||
| l.Warnf("Empty repository URL encountered, skipping.") | ||
| continue | ||
| } | ||
|
|
||
| // Create a unique path in the system's temporary directory for this repository. | ||
| // The path is based on a SHA1 hash of the repository URL to ensure uniqueness and idempotency. | ||
| encodedRepoURL := util.EncodeBase64Sha1(currentRepoURL) | ||
| tempPath := filepath.Join(os.TempDir(), fmt.Sprintf(tempDirFormat, encodedRepoURL)) | ||
|
|
||
| l.Debugf("Processing repository %s in temporary path %s", currentRepoURL, tempPath) | ||
|
|
||
| var repoOpts []module.RepoOpt | ||
|
|
||
| if len(discovery.ModulePaths) > 0 { | ||
| repoOpts = append(repoOpts, module.WithModulePaths(discovery.ModulePaths)) | ||
| } | ||
|
|
||
| // Initialize the repository. This might involve cloning or updating. | ||
| // Use the newRepo function stored in the service instance. | ||
| repo, err := s.newRepo(ctx, l, currentRepoURL, tempPath, walkWithSymlinks, allowCAS, repoOpts...) | ||
| if err != nil { | ||
| l.Errorf("Failed to initialize repository %s: %v", currentRepoURL, err) | ||
|
|
||
| errs = append(errs, err) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| // Find modules within the initialized repository. | ||
| repoModules, err := repo.FindModules(ctx) | ||
| if err != nil { | ||
| l.Errorf("Failed to find modules in repository %s: %v", currentRepoURL, err) | ||
|
|
||
| errs = append(errs, err) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| l.Infof("Found %d module(s) in repository %q", len(repoModules), currentRepoURL) | ||
| allModules = append(allModules, repoModules...) | ||
| } | ||
| } | ||
|
|
||
| return allModules, errs | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Clean helper function with proper error handling.
The loadModulesFromDiscoveries function is well-designed:
- Accumulates errors while continuing to process all repositories
- Conditionally applies ModulePaths via RepoOpt when configured (lines 185-187)
- Provides clear debug and error logging
- Handles empty URLs defensively (lines 171-174)
Note: Temporary directories are created at line 179 but not explicitly cleaned up in this code. Verify that cleanup is handled elsewhere (e.g., OS temp directory cleanup, or cleanup in a calling function/defer).
🏁 Script executed:
#!/bin/bash
# Search for cleanup logic related to catalog temp directories
rg -n -C3 'catalog-.*EncodeBase64Sha1|TempDir.*catalog|RemoveAll.*catalog' --type=goLength of output: 0
🏁 Script executed:
#!/bin/bash
# Find all calls to loadModulesFromDiscoveries to see how temp dirs might be cleaned up
rg -n 'loadModulesFromDiscoveries' --type=go -C5Length of output: 1492
🏁 Script executed:
#!/bin/bash
# Search for RemoveAll, Remove, or os.Temp cleanup patterns in the catalog service
rg -n 'RemoveAll|os\.Remove|defer.*Remove' internal/services/catalog/ --type=go -C3Length of output: 582
🏁 Script executed:
#!/bin/bash
# Look for defer statements in the service that might handle cleanup
rg -n 'defer' internal/services/catalog/ --type=go -C3Length of output: 1363
🏁 Script executed:
#!/bin/bash
# Check the repo struct definition and see if it has cleanup/Close methods
rg -n 'type.*Repo.*struct|func.*\(.*\*Repo\).*Close|func.*\(.*\*Repo\).*Cleanup' internal/services/catalog/module/ --type=go -A5Length of output: 847
🏁 Script executed:
#!/bin/bash
# Search for calls to the Load function to see caller context
rg -n '\.Load\(' --type=go -C4Length of output: 14737
🏁 Script executed:
#!/bin/bash
# Check the complete Load function to see if there's any cleanup logic we missed
rg -n 'func.*Load' internal/services/catalog/catalog.go --type=go -A30Length of output: 1234
Add explicit cleanup of temporary directories to prevent resource leak.
The temporary directories created at line 179 are never cleaned up. While the code correctly accumulates errors and continues processing, it leaves behind orphaned directories in the system's temp folder.
These temp directories should be cleaned up after processing completes. Either:
- Add a
defer os.RemoveAll(tempPath)after the temp path is created and before repository operations begin, or - Add cleanup logic to the repo's Close/Cleanup method and call it after FindModules completes
This prevents accumulation of unused temporary directories and ensures resources are properly released.
🤖 Prompt for AI Agents
In internal/services/catalog/catalog.go around lines 159 to 216, the temporary
directory created at tempPath (line ~179) is never cleaned up, leaking temp
directories; fix by ensuring tempPath is removed after processing each
repository: either call os.RemoveAll(tempPath) in all exit paths (after errors
and after successful FindModules) or invoke the repo cleanup/Close method (which
should remove the tempPath) immediately after you finish using repo (and ensure
you call it on every continue/error path); add the cleanup calls so every loop
iteration always removes the temporary directory before proceeding to the next
URL.
1 participant
Description
Closes #4632.
Add experimental catalog-discovery feature allowing custom module directory paths via discovery blocks in catalog configuration.
Previously, the catalog only searched the hardcoded
modules/directory. This change enables repositories with custom module paths (e.g.,tf-modules/,infrastructure/) to be used in the catalog.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added experimental catalog-discovery feature to support custom module directory paths in catalog configuration.
Migration Guide
No migration required.
Summary by CodeRabbit
New Features
Documentation
Tests