Reviewing files that changed from the base of the PR and between 06f5f3b and 82caa05.

• internal/exec/copy_glob.go (1 hunks)

• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Summary

internal/exec/copy_glob.go (2)

1-13: Well-structured imports and package organization!

Good choice using the optimized otiai10/copy library for unfiltered copying operations.

---

94-137: Solid implementation of recursive directory copying!

Good job handling symlinks and maintaining proper error propagation.

Reviewing files that changed from the base of the PR and between 8b0a3b0 and b468fb2.

• internal/exec/error.go (1 hunks)
• internal/exec/go_getter_utils.go (4 hunks)
• internal/exec/oci_utils.go (1 hunks)
• internal/exec/tar_utils.go (1 hunks)
• internal/exec/validate_component.go (2 hunks)
• internal/exec/validate_stacks.go (5 hunks)
• internal/exec/vendor.go (1 hunks)
• internal/exec/vendor_component_utils.go (6 hunks)
• internal/exec/vendor_model.go (8 hunks)
• internal/exec/vendor_model_component.go (0 hunks)
• internal/exec/vendor_utils.go (6 hunks)
• internal/exec/vendor_utils_test.go (3 hunks)
• pkg/config/config.go (5 hunks)
• pkg/config/utils.go (2 hunks)
• pkg/schema/schema.go (2 hunks)
• pkg/vender/component_vendor_test.go (2 hunks)
• pkg/vender/vendor_config_test.go (4 hunks)

• internal/exec/vendor_model_component.go

• GitHub Check: Summary

pkg/config/utils.go (10)

329-356: Updated environment variable handling with improved logging.

The transition from u.LogDebug to the structured log.Debug method from Charmbracelet provides better logging context by explicitly naming environment variables. The schema configuration is now properly set using a map approach with explicit types.

---

403-432: Good refactoring of command line argument processing.

Breaking down the large function into smaller, focused helper functions improves maintainability and readability. The function signature change to pass configAndStacksInfo by reference is a good optimization for memory usage.

---

434-440: Base path configuration looks good.

Simple helper function with proper error handling pattern.

---

442-452: Terraform configuration handling looks good.

Clear logging with context improves debuggability.

---

454-464: Helmfile configuration handling looks good.

Follows consistent pattern with the other configuration helpers.

---

466-472: Stacks configuration handling looks good.

Maintains the consistent pattern established in other helper functions.

---

474-504: Feature flags configuration handling looks good.

Proper error handling for boolean parsing and consistent logging pattern.

---

506-526: Schema directories configuration looks good.

The use of explicit schema structs improves type safety and clarity.

---

528-542: Logging configuration handling looks good.

Validates log level before setting it, showing good defensive programming.

---

544-551: Settings configuration handling looks good.

Follows consistent pattern with other configuration helpers.

pkg/vender/component_vendor_test.go (4)

33-33: Passing atmosConfig by reference improves consistency.

Changed to pass the configuration object by reference instead of by value, consistent with the updates in other parts of the codebase.

---

38-38: Passing componentConfig.Spec by reference maintains consistency.

Now passing the component spec by reference which aligns with the parameter changes in the function definition.

---

53-53: Consistent reference passing for second test case.

Ensures consistency in how configuration is passed between functions.

---

58-58: Consistent reference passing for component execution.

Maintains the pattern of passing configuration objects by reference.

pkg/vender/vendor_config_test.go (5)

55-55: Updated to pass atmosConfig by reference.

Changed to use the address-of operator (&) which matches the updated function signature.

---

92-92: Consistent parameter passing in component config test.

Ensures that the atmosConfig is passed by reference throughout the test.

---

106-106: Reference passing in no-config test case.

Maintains consistency with the updated function signatures.

---

111-111: Reference passing in component test.

Follows the pattern established across the codebase of passing configuration by reference.

---

134-134: Consistent reference passing in final test case.

Ensures all test cases follow the same pattern of passing configuration by reference.

internal/exec/validate_component.go (3)

209-209: Updated schema path access to use GetResourcePath method.

Now uses the GetResourcePath method to retrieve the base path for JSON schema validation, which is more flexible than direct struct field access.

---

213-213: Consistent schema path access for OPA validation.

Uses the same GetResourcePath method for OPA schema paths, maintaining consistency with the JSON schema case.

---

240-240: Updated module paths to use GetResourcePath for OPA.

Ensures consistent path resolution for OPA module paths using the GetResourcePath method.

internal/exec/vendor_utils_test.go (13)

4-4: Importing errors package.

This import is standard for error handling. No issues here.

---

10-10: Adding testify/require dependency.

This is a good practice for writing clean and readable tests.

---

23-27: Cleaning up environment variables after tests.

This deferred cleanup is a solid move to ensure test isolation and avoid side effects.

---

65-65: Using the pointer version of ReadAndProcessVendorConfigFile.

Pointer usage promotes consistency with other signatures. Looks fine.

---

69-73: Added documentation for TestExecuteVendorPull.

Clear docstrings are helpful to understand test coverage and intentions.

---

74-85: Unset environment variables conditionally.

It's good to ensure the environment is clean before proceeding.

---

86-97: Capturing and restoring working directory.

Preserving the original working directory is a neat solution to avoid residual changes after tests.

---

99-104: Switching to a target directory.

This step is straightforward and effectively sets up the test environment.

---

105-114: Flag initialization and command execution.

Registering flags and invoking the vendor pull command is straightforward.

---

120-147: List of expected files.

Defining file paths in one place is helpful for clarity.

---

148-152: Verifying file existence and deleting state files.

Test calls are consistent and help validate vendoring outcomes.

---

176-185: verifyFileExists function.

Implementation is straightforward, returning early if any file is missing.

---

187-193: deleteStateFiles function.

Simple approach for cleanup of generated vendor files.

pkg/config/config.go (4)

10-10: Imported strings package.

This enables efficient string handling below.

---

83-90: Converting Schemas to a map.

This flexible map-based schema definition better accommodates multiple registry entries.

---

263-263: Calling atmosConfig.ProcessSchemas().

Ensures schemas are processed immediately after unmarshal, aiding consistency.

---

396-412: setLogConfig function.

Adjusts log settings from environment variables and flags. The logic is straightforward and consistent with typical override patterns.

internal/exec/validate_stacks.go (13)

14-14: Introducing structured logging.

Using charmbracelet/log for better clarity.

---

58-60: Overriding the 'atmos' schema registry.

This helps maintain user-provided schema paths at runtime.

---

108-109: Beginning switch statement for manifest checks.

Improves clarity over multiple if-else conditions.

---

116-117: Assigning the embedded schema path.

A fallback path for the manifest is a handy approach.

---

118-118: Logging default schema usage.

Good trace message for diagnosing schema resolution.

---

119-119: Direct file existence check.

Simple approach to confirm the user-specified manifest.

---

120-120: Relative path check.

Verifies the combined path from base config.

---

122-122: Handling remote schema with isURL.

Ensures we fetch from the network if specified.

---

127-127: Error fallback for missing schemas.

Direct feedback for misconfigurations or typos.

---

133-133: User instructions in error.

Guidance on possible ways to configure the schema manifest.

---

390-390: downloadSchemaFromURL signature uses pointer.

Maintains consistency with the rest of the codebase's pointer usage.

---

391-391: Extracting manifestURL from config.

In line with the central approach to read from the 'atmos' registry.

---

392-392: manifestSchema retrieval from atmosConfig.

Follows the standardized pattern for fetching registry entries.

internal/exec/tar_utils.go (11)

11-12: Adding structured logger and errors.

This aligns with the rest of the logging approach and standard error usage.

---

15-15: Defining ErrInvalidFilePath.

Helpful for identifying unsafe extractions.

---

17-21: Refactoring extractTarball signature.

Accepting an io.Reader broadens input options (e.g., streaming data).

---

23-26: untar routine.

Centralizing tar extraction logic for clarity.

---

28-35: Reading tar headers.

Clear error handling with structured logging. This approach handles partial extractions gracefully.

---

37-38: Detecting potential directory traversal.

Skipping suspicious paths helps mitigate security risks.

---

40-43: Delegating to processTarHeader.

Separating logic into functions enhances readability.

---

45-45: Returning upon successful untar.

No major concerns.

---

49-70: processTarHeader with security checks.

Validates and cleans file paths, skipping or halting on invalid attempts.

---

72-78: createDirectory function.

Minimal approach to ensure directories exist.

---

80-105: createFileFromTar handles file creation and permissions.

Permissions are adjusted safely. Consider logging if setuid/setgid bits are encountered and stripped. Otherwise, all good.

internal/exec/error.go (1)

9-14: Review the unused constants.

Static analysis flags these constants (progressWidth, getterTimeout, componentTempDirPermissions, wrapErrFmtWithDetails, timeFormatBase) as unused. Confirm whether you still need them or remove them if they are no longer relevant.

🧰 Tools

🪛 golangci-lint (1.64.8)

[error] 9-9: const progressWidth is unused

(unused)

---

[error] 10-10: const getterTimeout is unused

(unused)

---

[error] 11-11: const componentTempDirPermissions is unused

(unused)

---

[error] 12-12: const wrapErrFmtWithDetails is unused

(unused)

---

[error] 13-13: const timeFormatBase is unused

(unused)

internal/exec/oci_utils.go (2)

24-25: Error declaration looks good.

Defining ErrNoLayers here improves code readability and consistency for layer-check scenarios.

---

56-57: Confirm artifact mismatch handling.

checkArtifactType logs a warning but does not return an error on mismatch. Verify if the code should fail when the artifact type deviates from the expected value.

internal/exec/go_getter_utils.go (3)

73-123: Confirm shallow clone behavior.

The code automatically sets depth=1 unless an explicit value is present. Ensure this does not break workflows needing a full clone.

---

257-265: Validate subdir adjustment logic.

adjustSubdir appends //. if no subdirectory is detected. Confirm there are no edge cases where multiple slashes or alternative paths might bypass or misapply this logic.

---

335-349: Refine symlink removal to handle nested structures.

Removing symlinks in nested directories may require skipping subdirectories once a symlink is removed. Consider using a deeper check or WalkDir.

 func removeSymlinks(root string) error {
-	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+	return filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if info.Mode()&os.ModeSymlink != 0 {
+		if d.Type()&os.ModeSymlink != 0 {
 			log.Debug("Removing symlink", "path", path)
 			return os.Remove(path)
 		}
 		return nil
 	})
 }

internal/exec/vendor_utils.go (12)

22-40: StderrLogger initialization and new error variables look good.

These additions provide more descriptive error handling and a clean separation of stderr logging. This approach helps keep stdout streams free of diagnostic messages, which is a valuable design choice for vendoring processes.

---

42-50: Struct naming clarifies usage.

Using processTargetsParams and executeVendorOptions effectively encapsulates parameters for source processing and vendor execution. This fosters modular design and readability.

---

113-142: Double-star glob usage is correct.

Here, doublestar.Glob is a good choice for matching files in directories. The check for empty matches and sorting them ensures stable ordering for merges. Keep this approach for consistency with your vendoring logic.

---

145-181: Avoid duplicate components.

mergeVendorConfigFiles checks for duplicates by component name, throwing early errors if encountered. This is good practice. Just ensure you also consider or handle case sensitivity if that emerges as a future requirement.

---

222-254: Tag-based filtering logic is sound.

validateTagsAndComponents ensures that only specified tags or components pass through. This effectively guards against extraneous sources. Be sure to keep the intersection logic thoroughly tested for new tag behavior.

---

323-348: processTargets function is concise and readable.

Shared logic for creating the pkgAtmosVendor struct is well-organized. The parameter struct fosters maintainability, and the function adheres to single-responsibility nicely.

---

350-396: Recursively processing imports is well-structured.

processVendorImports helpfully prevents circular imports with the allImports check. Good use of error messages to clarify the cause of duplicate imports.

---

406-418: Guard against missing fields.

validateSourceFields checks for required fields: Source and Targets. The explicit error strings help. Consider verifying even more potential user mistakes, like empty strings in component.

---

427-461: Solid path traversal checks.

determineSourceType carefully handles .. in URIs and ensures local paths are joined properly. This helps mitigate path traversal risks. Keep verifying that other allowable schemes are handled as expected.

---

481-517: Skip function usage is constructive.

Using generateSkipFunction to unify exclusion/inclusion logic is wise. This ensures consistent behavior across vendoring operations. Just be cautious when patterns overlap in both included and excluded sets.

---

523-535: Exclusion pattern logic is correct.

shouldExcludeFile checks each excluded path pattern with the helpful debug statement. The code is straightforward. If additional patterns or advanced matching are introduced, ensure performance remains acceptable.

---

537-559: Inclusion logic complements exclusion checks.

shouldIncludeFile ensures that if there are any included_paths, a file must match at least one. The fallback of including all if no patterns are specified is a sensible default.

internal/exec/vendor_model.go (13)

14-14: New logger import is consistent with existing usage.

Importing charmbracelet/log aligns well with the chosen logging strategy. It helps ensure a uniform logging style.

---

26-40: Readable constants and styling blocks.

Defining tempDirPermissions, progressBarWidth, and maxWidth as constants clarifies usage. The style variables (checkMark, xMark, etc.) keep UI code centralized and consistent.

---

42-46: installedPkgMsg transition is straightforward.

Encapsulating installation result details in installedPkgMsg fosters easier message handling. The approach to pass package name and error is clear and maintainable.

---

98-126: Consider a fallback logging statement for the TTY check.

executeVendorModel handles TTY vs non-TTY scenarios gracefully, defaulting to a fallback mode. This keeps normal usage interactive and still logs essential info if TTY is unavailable.

---

128-178: Well-structured generic factory.

newModelVendor uses a type parameter to distinguish pkgComponentVendor from pkgAtmosVendor. This approach is a strong example of generics usage for code reuse, simplifying the creation of unified models.

---

192-194: Window size logic is safe.

Ensuring m.width never exceeds maxWidth helps preserve layout consistency when rendering progress in narrower terminals. This is a handy boundary check.

---

197-199: Non-blocking exit with ctrl+c.

handleKeyPress gracefully quits on typical terminal exit keys. The code is easy to follow and aligns with standard TUI patterns.

---

225-269: Comprehensive install result handling.

handleInstalledPkgMsg increments failedPkg if errors occur. The logic to print mark, maintain progress, and decide next steps is a well-defined workflow. Good job ensuring logs are used if TTY is absent.

---

271-292: logNonNTYFinalStatus is a useful helper.

It avoids repetition in non-TTY contexts, summarizing final results succinctly. Keep an eye on the formatting options if users desire more verbose logs.

---

356-387: Installer function covers all package types.

(*pkgAtmosVendor).installer does a nice job branching by pkgType. The go-getter usage for remote packages is clearly separated from local copies. If more package types appear, consider a plugin approach to keep this code lean.

---

398-411: Secure temporary directory creation.

createTempDir with os.MkdirTemp is best practice, and applying restricted permissions is an important security measure. Great job.

---

413-418: newInstallError helps unify error reporting.

Keeping error creation in a single place promotes consistency for failed installs, including naming the package in the error message.

---

420-437: Overall robust fallback in ExecuteInstall.

If neither atmosPackage nor componentPackage is set, the code returns a clear error. This guards against unexpected usage and keeps the contract explicit.

pkg/schema/schema.go (8)

4-5: New imports are consistent with existing structure.

Using encoding/json and gopkg.in/yaml.v3 is aligned with the approach for schema marshalling/unmarshalling. This unifies standard Go libraries for JSON tasks.

Also applies to: 7-7

---

12-44: Expanded AtmosConfiguration fields facilitate flexible stack definitions.

The introduction and reorganization of fields, alongside the Schemas map, allow more dynamic CLI configuration. Ensure that each new field is properly tested in typical user scenarios.

---

46-57: GetSchemaRegistry method ensures safe retrieval.

This avoids panics by returning a zero struct if not found. Good pattern for user-friendly API calls.

---

59-70: Graceful fallback in GetResourcePath.

Similar to GetSchemaRegistry, returning an empty ResourcePath if not found is consistent. This fosters error-free usage in dependent code.

---

72-118: UnmarshalYAML carefully handles multiple structure possibilities.

The fallback to storing raw YAML node is an inclusive approach. This ensures minimal data loss if certain structures don't match strongly typed fields.

---

120-128: ProcessSchemas systematically processes known keys.

Separating resource-based keys (cue, opa, jsonschema) from manifest-based keys keeps logic simpler and maintainable. This is a clean design choice.

---

148-165: processResourceSchema also reuses JSON marshalling.

Consistent approach with processManifestSchemas. Good to keep resource path logic separate from manifest logic for clarity.

---

603-611: Schema types unify resource references.

ResourcePath and SchemaRegistry add structure around different schema styles. If owners want to store more metadata, these types are well-placed for extension in the future.

internal/exec/vendor.go (9)

5-9: Imports look concise.

All newly introduced imports appear broadly relevant to the vendoring logic (e.g. strings, errors, cobra, pflag, etc.). This is coherent and doesn't present immediate concerns.

Also applies to: 12-13

---

15-21: Clear and descriptive error grouping.

Declaring these error variables here facilitates maintainability and provides tailored messages for vendor operations.

---

24-25: Minimal function wrapper.

The function simply delegates to ExecuteVendorPullCommand, which is good for clarity and discoverability.

---

30-32: Gracefully handling not implemented command.

Returning a structured error here helps communicate the missing functionality.

---

34-41: Well-defined flags struct.

Encapsulating parameters into a VendorFlags struct improves code organization and clarity for command parsing.

---

43-72: Robust command execution flow.

1. The function integrates CLI configuration, parses flags, and handles distinct scenarios (component vs. stack vs. config file).
2. Very readable, but ensure consistent test coverage across paths (e.g., stack usage).

Would you like me to provide a script that checks references to ExecuteVendorPullCommand to confirm that all code paths are tested and no extraneous calls exist?

---

115-121: Thoughtful fallback for 'Everything' flag.

Automatically setting Everything if no other flags were provided is a nice convenience.

---

123-137: Flag validation is straightforward.

Checks are well-ordered and produce clear error messages that guide the user.

---

174-196: Single-entry function for component vendoring.

Pulling config and then delegating to component vendoring flow is cohesive. Ensure you handle edge cases when the folder or config is missing.

internal/exec/vendor_component_utils.go (17)

13-14: Improved error definitions.

Centralizing error variables (e.g., ErrMissingMixinURI, ErrFolderNotFound, etc.) clarifies failure modes for mixins, directories, etc.

Also applies to: 18-19, 29-41

---

44-55: findComponentConfigFile: descriptive error usage.

Returning a combined error with contextual path fosters debugging clarity.

---

57-72: Component vendoring config reading.

The switch on componentType is straightforward, though consider gracefully handling future expansions for additional component types.

Suggest verifying that all references to “helmfile” and “terraform” in your codebase are enumerated. Would you like a script to confirm that no references to other component types exist?

---

82-83: Directory existence check.

Clear error if the target directory is missing. This is a good developer experience.

---

100-101: Kind validation for vendor config.

Strictly checking for "ComponentVendorConfig" guards against accidental usage of the wrong file type.

---

107-115: Descriptive documentation for internal vendor functions.

The docstrings provide a thorough overview of supported protocols and relevant references. This fosters maintainability.

---

125-130: Configurable copy options.

Your usage of Skip and the skip function approach is flexible for future expansions.

---

158-181: Skip function design.

1. The logic covers .git directories, excluded paths, and included paths, combining them with clarity.
2. Good usage of double-star and ignoring matched patterns.

---

184-197: Pattern matching for excludes.

Your design is consistent with typical glob usage, returning early upon a single match.

---

199-220: Pattern matching for includes.

1. Checking any match to “include” a file is intuitive.
2. The debug logs are descriptive.

---

223-283: Component vendoring logic.

Process for building a list of pkgComponentVendor items and executing them is well-structured. This promotes a flexible pipeline for main component + mixins.

---

285-309: handleLocalFileScheme logic.

Falling back to a local path if it can be resolved is helpful. Clean path usage prevents confusion on different OS.

---

311-366: Mixin processing.

1. The function gracefully checks for essential fields (Uri, Filename).
2. Good approach skipping mixin installation when a local file is discovered.

---

368-384: Templated URI approach.

Dynamic building of mixin URIs is flexible. Minimal error references ensure clarity.

---

386-427: Download flow with TUI state handling.

The approach to handle both IsComponent and IsMixins is well-structured, returning an appropriate message.

---

429-467: Install component sequence.

Temporary directory usage is practical for clean file operations. The fallback to GoGetterGet or processOciImage is consistent.

---

469-489: Local system handling.

This function clearly identifies local file copying while preserving symlinks.

Reviewing files that changed from the base of the PR and between daf9c22 and 276438c.

• go.sum is excluded by !**/*.sum

• go.mod (1 hunks)
• internal/exec/copy_glob.go (1 hunks)
• internal/exec/copy_glob_test.go (1 hunks)
• internal/exec/error.go (1 hunks)
• internal/exec/go_getter_utils.go (4 hunks)
• internal/exec/vendor_model.go (3 hunks)
• internal/exec/vendor_utils.go (1 hunks)
• internal/exec/yaml_func_include.go (1 hunks)
• pkg/utils/url_utils.go (1 hunks)
• tests/cli_test.go (3 hunks)
• tests/fixtures/scenarios/vendor-globs/atmos.yaml (1 hunks)
• tests/fixtures/scenarios/vendor-globs/vendor.yaml (1 hunks)
• tests/fixtures/scenarios/vendor/vendor.yaml (1 hunks)
• tests/test-cases/demo-globs.yaml (1 hunks)
• website/docs/core-concepts/vendor/vendor-manifest.mdx (2 hunks)

• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Summary

tests/fixtures/scenarios/vendor/vendor.yaml (1)

54-54: Good addition of trailing newline.

Adding the empty line at the end of the file follows best practices for file formatting. This ensures proper handling by version control systems and text editors.

internal/exec/vendor_utils.go (1)

22-23: Good addition of dedicated stderr logger.

Adding a dedicated logger for stderr helps keep stdout clean from detailed messaging during vendoring operations. This improves the user experience by separating different types of output.

go.mod (1)

87-87:

Details

✅ Verification successful

Added test dependency for mocking.

The addition of the gomonkey library is appropriate for testing the enhanced vendoring functionality with proper mocking capabilities.

---

🏁 Script executed:

#!/bin/bash
# Verify that the gomonkey library is being used in tests and not in production code
echo "Checking for gomonkey usage in test files..."
rg "github.com/agiledragon/gomonkey" --type go | grep "_test.go"

echo "Checking that gomonkey is not imported in non-test files..."
rg "github.com/agiledragon/gomonkey" --type go | grep -v "_test.go" || echo "Good: No usage of gomonkey in non-test files"

Length of output: 525

---

Dependency Usage Verified: gomonkey Is Correctly Scoped to Tests

• The gomonkey dependency in go.mod (line 87) is confirmed to be used exclusively in test files (e.g., internal/exec/copy_glob_test.go).
• No instances of gomonkey are found in non-test files, ensuring the dependency is appropriately isolated for testing purposes.
• The addition of this test dependency effectively supports enhanced vendoring functionality with proper mocking mechanisms.

pkg/utils/url_utils.go (1)

1-21: Well-implemented auth masking utility.

This function provides good security by masking sensitive authentication credentials in URLs, which is important when logging or displaying URLs that might contain passwords.

A few observations:

1. The implementation properly handles error cases when parsing the URL
2. The function correctly checks if user credentials exist before replacing them
3. The masked output uses a consistent placeholder ("xxx") for both username and password

This aligns well with the PR objective of improving URL handling for repository cloning.

internal/exec/yaml_func_include.go (1)

50-50:

Details

✅ Verification successful

Updated function call to match new signature

The change from passing atmosConfig directly to passing &atmosConfig (a pointer) aligns with the updated function signature in go_getter_utils.go. Passing a pointer is more efficient for large structs and allows the function to potentially modify the config if needed.

Run this script to verify that all calls to this function have been updated:

---

🏁 Script executed:

#!/bin/bash
# Find all calls to DownloadDetectFormatAndParseFile that don't pass a pointer
rg "DownloadDetectFormatAndParseFile\([^&]" --type go

Length of output: 194

---

Verified pointer usage in DownloadDetectFormatAndParseFile() is correct

• The call in internal/exec/yaml_func_include.go now passes &atmosConfig, which aligns with the updated function signature in internal/exec/go_getter_utils.go.
• A repository-wide search confirmed that no other call sites pass a non-pointer argument.

Everything looks good.

tests/fixtures/scenarios/vendor-globs/atmos.yaml (1)

11-18: Good implementation of stacks with glob patterns

The stacks configuration correctly implements inclusion and exclusion patterns using glob syntax, which aligns with the PR objective of enhancing glob pattern support.

tests/test-cases/demo-globs.yaml (1)

1-60: Comprehensive test case for glob pattern vendoring

This test case thoroughly checks that the atmos vendor pull command works correctly with glob patterns by verifying both the presence and absence of specific files. The test aligns with the PR objective of enhancing glob pattern support and fixing vendoring issues with symlinks.

The test includes verification for:

1. Files that should be included based on glob patterns
2. Files that should be excluded based on glob patterns
3. Proper execution of the command with exit code 0

internal/exec/vendor_model.go (1)

14-14: Updated log import to charmbracelet/log

Changed the logging library from sirupsen/logrus to charmbracelet/log. This change is consistent with the updated logging approach in the codebase.

tests/fixtures/scenarios/vendor-globs/vendor.yaml (1)

1-44: New test fixture looks good with comprehensive glob pattern examples

This new vendor configuration file provides a good test case for the glob pattern functionality, covering various scenarios: file extension pattern matching, directory exclusions, and path handling variations.

🧰 Tools

🪛 YAMLlint (1.35.1)

[error] 31-31: trailing spaces

(trailing-spaces)

tests/cli_test.go (3)

48-54: Good addition of FileNotExists field to Expectation struct

Adding the FileNotExists field is a logical extension to support negative file existence testing, complementing the existing FileExists field.

---

682-685: Well-integrated verification of non-existent files

The implementation correctly adds verification of non-existent files to the test flow, following the same pattern as the existing file verification.

---

795-807: Robust implementation of file non-existence verification

The verifyFileNotExists function is implemented correctly, with proper error handling for both the case when a file exists that shouldn't and for unexpected errors.

internal/exec/error.go (1)

7-22: Well-defined error variables for consistent error handling

These error variables provide a consistent way to report errors across the exec package, improving error handling standardization.

website/docs/core-concepts/vendor/vendor-manifest.mdx (2)

220-221: Good cross-reference to glob pattern documentation

The updated description for included_paths and excluded_paths now correctly links to the new detailed glob pattern section.

---

501-611: Excellent documentation on glob pattern usage

The new "Vendoring with Globs" section provides comprehensive and clear documentation on how glob patterns work in Atmos, with helpful examples that demonstrate the differences between pattern types and how to effectively use them.

🧰 Tools

🪛 LanguageTool

[style] ~570-~570: Consider using the typographical ellipsis character here instead.
Context: ...ers, use **/demo-library/**/*. Using {...} for Multiple Extensions or Patterns ...

(ELLIPSIS)

---

[style] ~572-~572: Consider using the typographical ellipsis character here instead.
Context: ...le Extensions or Patterns Curly braces {...} allow for expanding multiple patterns...

(ELLIPSIS)

---

[style] ~602-~602: Consider using the typographical ellipsis character here instead.
Context: ... **/demo-library/**/archive/** Using {...} here prevents the need to write two s...

(ELLIPSIS)

---

[style] ~608-~608: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...won't include deeper subdirectories. 3. Use {...} to match multiple extensions or...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

---

[style] ~608-~608: Consider using the typographical ellipsis character here instead.
Context: ...t include deeper subdirectories. 3. Use {...} to match multiple extensions or direc...

(ELLIPSIS)

---

[style] ~611-~611: Consider using the typographical ellipsis character here instead.
Context: ...included_paths, excluded_paths, and {...} expansion, you can precisely control ...

(ELLIPSIS)

internal/exec/go_getter_utils.go (6)

73-123: Improve error coverage
The Detect method handles parsing and token injection thoroughly. Consider adding a test to cover partial parse failures or unexpected edge cases to boost reliability.

---

137-174: Rewrite edge cases
rewriteSCPURL seems robust, but unusual ports or nested subdomains might require extra validation. Check logs for unexpected rewrites in real-world usage.

---

176-184: Path normalization
Consistently normalizing slash usage is good. No major issues found.

---

233-253: Bitbucket username
The approach for retrieving the Bitbucket username is consistent and flexible.

---

270-277: Registering custom detectors
Prepending CustomGitDetector is aligned with go-getter's strip-subdir behavior. Good approach.

---

320-333: CustomGitGetter symlink logic
This correctly delegates to GitGetter then removes symlinks. Make sure to handle cases where a directory is replaced by a symlink.

internal/exec/copy_glob.go (12)

1-41: File-level constants/types
Defining named constants (e.g. shallowCopySuffix) improves readability. No issues found.

---

42-72: copyFile
Preserving permissions and using os.MkdirAll is solid.

---

74-105: Exclude path logic
Implementation effectively handles directory vs file patterns.

---

106-122: Inclusion logic
Mirrors the exclusion approach nicely. Straightforward.

---

144-181: processDirEntry
Thorough checks and recursion. Good error handling.

---

182-194: copyDirRecursive
Simple recursion, no immediate concerns.

---

196-221: shouldSkipPrefixEntry
Logic matches shouldSkipEntry. Looks consistent.

---

259-271: copyDirRecursiveWithPrefix
Clear recursion approach. Smooth.

---

273-305: getMatchesForPattern
Shallow vs recursive logic is carefully handled.

---

307-311: Shallow pattern detection
isShallowPattern is straightforward and correct.

---

312-342: processMatch
Cleans up relative path usage and skipping. No issues noted.

---

344-362: processIncludedPattern
Properly loops through matched files. Good job ignoring errors on invalid patterns.

internal/exec/copy_glob_test.go (36)

49-77: TestCopyFile
Verifies file integrity and permissions.

---

79-87: TestCopyFile_SourceNotExist
Validates missing source error. Good negative path coverage.

---

89-104: TestShouldExcludePath
Checks wildcard patterns for exclusion. Nicely done.

---

106-125: TestShouldExcludePath_Directory
Ensures trailing slash exclusion logic works (Unix-like).

---

127-141: TestShouldExcludePath_Error
Smart approach to invalid pattern coverage.

---

143-158: TestShouldIncludePath
Confirms typical inclusion pattern scenario.

---

160-174: TestShouldIncludePath_Error
Ensures invalid pattern doesn't incorrectly include.

---

176-198: TestShouldSkipEntry
Checks combined exclusion/inclusion logic. Good.

---

200-240: TestCopyDirRecursive
Recursively copies subdirectories/files, verifying structure.

---

242-292: TestProcessDirEntry_Symlink
Verifies skipping symlinks. Good approach.

---

294-316: TestGetMatchesForPattern
Direct matching test. Straightforward verification.

---

318-337: TestGetMatchesForPattern_NoMatches
Ensures no matches scenario is well-handled.

---

339-350: TestGetMatchesForPattern_InvalidPattern
Verifies handling of invalid patterns with errors.

---

352-379: TestGetMatchesForPattern_ShallowNoMatch
Confirms shallow pattern yields empty result when none found.

---

381-416: TestGetMatchesForPattern_RecursiveMatch
Checks star-star patterns for deeper recursion. Good coverage.

---

418-426: TestIsShallowPattern
Quick test for shallow detection.

---

428-457: TestCopyDirRecursiveWithPrefix
Copies directory with prefix. Implementation verified.

---

459-489: TestProcessIncludedPattern
Successfully validates included file patterns.

---

491-506: TestProcessIncludedPattern_Invalid
Ensures invalid pattern doesn't crash the process.

---

508-530: TestProcessMatch_ShallowDirectory
Checks that shallow copy excludes entire directories.

---

532-558: TestProcessMatch_Directory
Verifies full directory copying when shallow is false.

---

560-566: TestProcessMatch_ErrorStat
Simulates an error on os.Stat. Good negative coverage.

---

568-581: TestCopyDirRecursive_ReadDirError
Properly tests read failures to ensure robust handling.

---

583-620: TestCopyToTargetWithPatterns
Validates inclusion/exclusion in a realistic scenario.

---

622-648: TestCopyToTargetWithPatterns_NoPatterns
Confirms fallback to cp.Copy without patterns.

---

650-676: TestCopyToTargetWithPatterns_LocalFileBranch
Ensures correct behavior for local file sources.

---

678-691: TestProcessDirEntry_InfoError
Checks forced error from entry.Info(). Good.

---

693-716: TestCopyFile_FailCreateDir
Simulates an error creating the destination directory.

---

718-749: TestCopyFile_FailChmod
Uses gomonkey to mock os.Chmod. Covers permission failures well.

---

751-764: TestGetMatchesForPattern_GlobError
Injects a simulated glob error to confirm graceful handling.

---

766-790: TestInclusionExclusion_TrailingSlash
Ensures directories are matchable with trailing slash patterns.

---

792-809: TestProcessPrefixEntry_InfoError
Checks error handling behavior for Info().

---

838-878: TestProcessPrefixEntry_FailMkdir
Simulates mkdir failure for prefix-based copy.

---

880-922: TestCopyToTargetWithPatterns_UseCpCopy
Verifies that the cp.Copy path is actually called. Nicely tested.

---

924-941: TestGetMatchesForPattern_ShallowNoMatches
Verifies empty result for shallow patterns with no matches.

---

943-966: TestProcessMatch_RelPathError
Covers relative path errors carefully.

Reviewing files that changed from the base of the PR and between 276438c and 9e546ea.

• internal/exec/copy_glob.go (1 hunks)
• internal/exec/copy_glob_test.go (1 hunks)
• internal/exec/vendor_model.go (3 hunks)

• internal/exec/vendor_model.go

• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Summary

internal/exec/copy_glob.go (4)

48-78: copyFile function looks robust.
It effectively preserves permissions and handles I/O errors well. No issues found.

---

80-108: shouldExcludePath logic is concise.
It properly checks patterns and handles directories with trailing slash checks. Good coverage of edge cases.

---

109-128: shouldIncludePath approach is on point.
Directories pass by default, and file matching uses patterns effectively. This is well done.

---

445-471: ComponentOrMixinsCopy is logically structured.
The distinction between file-to-folder and file-to-file is clear, and directory cleanup prevents conflicts. Nicely done.

internal/exec/copy_glob_test.go (7)

39-48: Useful mock implementation.
The fakeDirEntry struct neatly simulates a directory entry for error handling tests.

---

50-77: TestCopyFile demonstrates strong coverage.
Ensures content and permissions are preserved. No concerns.

---

652-680: Local file branch test is commendable.
Covers the special logic for file-to-file and file-to-folder copying.

---

697-720: Provides negative test coverage for directory creation errors.
Helpful for validating resilience.

---

903-926: Verifying cp.Copy usage through patch is clever.
Nice technique to confirm the fallback copy logic is being invoked.

---

928-946: Shallow no-match test is clear.
Ensures patterns ending with /* but not /** produce an empty result.

---

947-970: TestProcessMatch_RelPathError is well-defined.
Mocks out filepath.Rel to trigger error handling. Good negative scenario coverage.

Reviewing files that changed from the base of the PR and between 9e546ea and be91fae.

• internal/exec/copy_glob.go (1 hunks)

• GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
• GitHub Check: [mock-macos] examples/demo-vendoring
• GitHub Check: [mock-macos] examples/demo-context
• GitHub Check: [mock-macos] examples/demo-component-versions
• GitHub Check: [mock-macos] examples/demo-atlantis
• GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
• GitHub Check: [mock-windows] examples/demo-vendoring
• GitHub Check: [mock-windows] examples/demo-context
• GitHub Check: [mock-windows] examples/demo-component-versions
• GitHub Check: [mock-windows] examples/demo-atlantis
• GitHub Check: [mock-linux] tests/fixtures/scenarios/complete
• GitHub Check: [mock-linux] examples/demo-vendoring
• GitHub Check: [mock-linux] examples/demo-context
• GitHub Check: [localstack] demo-localstack
• GitHub Check: [lint] quick-start-advanced
• GitHub Check: Acceptance Tests (macos-latest, macos)
• GitHub Check: [lint] demo-context
• GitHub Check: [k3s] demo-helmfile
• GitHub Check: Acceptance Tests (windows-latest, windows)
• GitHub Check: Acceptance Tests (ubuntu-latest, linux)
• GitHub Check: [lint] Dockerfile
• GitHub Check: Summary

internal/exec/copy_glob.go (18)

16-28: Constants are well-organized.
They provide clarity by avoiding repeated literals.

---

30-47: Structs provide clear parameter grouping.
No immediate issues; they effectively encapsulate copy parameters for different contexts.

---

80-107: Clear approach for exclusion patterns.
The trailing slash check for directories is helpful. Implementation looks sound.

---

109-128: Inclusion patterns are handled cleanly.
Logic is straightforward, and debug logs assist in troubleshooting. No concerns here.

---

130-148: Skips unwanted entries gracefully.
This is a fine approach to ignoring .git and respecting include/exclude logic. No immediate issues.

---

188-200: Recursive directory copying is concise.
Edge cases are handled by delegated logic in processDirEntry. Looks clean.

---

202-226: Prefix-based exclusion logic.
Matches the approach in shouldExcludePath, ensuring uniform pattern usage. Good job.

---

265-277: Recursive prefix copying is well implemented.
It reuses processPrefixEntry, maintaining a consistent approach.

---

279-311: Glob matching logic is thorough.
Fallback to shallow or deeper matching is well explained and handles no-match scenarios gracefully.

---

313-316: Simple shallow pattern check is on point.
Readable helper function with no issues.

---

318-348: Processes single match suitably.
Directory vs. file logic is clearly separated. This is nicely done.

---

350-368: Robust inclusion pattern handling.
Errors are downgraded to warnings, preventing premature failures when pattern matching is incomplete.

---

370-376: Initializes final target without duplication.
It’s straightforward with separate functions for local vs. non-local.

---

378-391: Local final target creation.
Gracefully handles file-to-folder vs. file-to-file. Looks good.

---

393-398: Non-local target creation.
Simple logic that returns the target path. No concerns.

---

400-405: Local file source logic:
Resolves to a single function call. The debug logs are helpful. No concerns.

---

407-447: Comprehensive copying with patterns.
Logical flow chooses fast copy if no patterns. Inclusion pattern loops seem well-structured.

---

449-475: Handles file-to-folder vs. file-to-file.
Consistent approach that ensures directories exist before copying. No immediate improvements needed.

Reviewing files that changed from the base of the PR and between a58bce1 and f21e958.

• internal/exec/go_getter_utils.go (5 hunks)

• GitHub Check: Acceptance Tests (macos-latest, macos)
• GitHub Check: Acceptance Tests (windows-latest, windows)
• GitHub Check: Acceptance Tests (ubuntu-latest, linux)
• GitHub Check: Summary

internal/exec/go_getter_utils.go (11)

14-14: Logging package update looks good.

The switch to using the charmbracelet/log package makes sense for consistent logging throughout the codebase.

---

67-71: Improved detector with better naming and source tracking.

Renaming from what was likely a GitHub-specific detector to a more general Git detector makes sense, particularly with the source field addition that will help with debugging and subdirectory handling.

---

133-142: Good performance optimization with shallow cloning.

Setting a default depth=1 for shallow cloning is a great optimization that will make vendoring operations much faster. The subdirectory adjustment is also handled nicely.

---

144-149: Secure URL masking before logging.

The code now masks sensitive information in URLs before logging them, which is an excellent security practice to prevent accidental credential leakage.

---

154-165: Clean implementation of subdirectory handling.

The adjustSubdir method is well-structured and cleanly separates this logic, making the code more maintainable. Good use of masking the URL in logs too.

---

169-173: Updated detector registration to support source passing.

The function now passes the original source string to the custom detector, which is needed for accurate subdirectory handling.

---

189-193: Clear documentation of source passing.

The comments explain clearly why the original source is passed to the detector, making the code more maintainable for future developers.

---

219-232: Good implementation of custom git getter.

The implementation properly extends the existing GitGetter and adds symlink removal functionality. This is a clean approach to add functionality while reusing the existing getter.

---

255-255: Improved time duration format.

The change from time.Second*30 to 30*time.Second follows Go's idiomatic style more closely.

---

268-315: Improved documentation of supported schemes.

The updated formatting of the comments for supported schemes makes the documentation more readable and easier to follow.

---

251-251:

Details

✅ Verification successful

Parameter change to pointer type.

The function now accepts a pointer to atmosConfig instead of a value, which is a potential breaking change if callers aren't updated.

Let's verify this change is consistent with all callers:

---

🏁 Script executed:

#!/bin/bash
# Find all calls to DownloadDetectFormatAndParseFile to verify they've been updated

echo "Searching for callsites of DownloadDetectFormatAndParseFile:"
rg -A 2 -B 2 "DownloadDetectFormatAndParseFile\(" --type go

Length of output: 1029

---

Pointer parameter change verified: All call sites are updated.
The call at internal/exec/yaml_func_include.go correctly passes &atmosConfig, so the change is consistent across the codebase.