Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve handling of ExcludeFromCopy configuration in Terragrunt #3816

Merged

Conversation

Excoriate
Copy link
Contributor

Description

This PR addresses a subtle configuration handling issue in Terragrunt's source downloading mechanism, specifically related to the ExcludeFromCopy configuration. It fixes #3767 (ar at least, contributes to fixing/closing it)

Problem Statement

In the previous implementation, there was a bug in how ExcludeFromCopy was being processed during source downloading. The original code incorrectly assigned the exclusion configuration, potentially leading to unexpected file copying behavior. Somehow, this wast not caught by the existing tests.

Key Changes

  • Added comprehensive test case TestUpdateGettersExcludeFromCopy to validate ExcludeFromCopy configuration handling
  • Ensured FileCopyGetter correctly respects the ExcludeFromCopy configuration
  • Improved test coverage for configuration-driven file exclusion scenarios

Rationale

The bug in PR #3766 highlighted a some sort of weakness in terragrunt's configuration handling (in the download-source logic):

  • Incorrect assignment of configuration values
  • Lack of explicit test coverage for edge cases
  • Potential for silent misconfigurations during module source downloading

Technical Details

  • Added unit test with two scenarios:
    1. Handling nil ExcludeFromCopy configuration
    2. Handling non-nil ExcludeFromCopy with specific exclusion patterns
  • Verified that UpdateGetters correctly populates FileCopyGetter with exclusion configuration
  • Ensured no regressions in existing source downloading functionality

TODOs

Read the Gruntwork contribution guidelines.

  • Add comprehensive test case
  • Update documentation (if necessary)
  • Run relevant tests successfully
  • Ensure 3rd party code adheres to license policy

Release Notes

Added enhanced test coverage and fixed configuration handling for ExcludeFromCopy in Terragrunt source downloading mechanism.

Migration Guide

No breaking changes. Users can continue to use exclude_from_copy configuration as before.

- Add a new test case to verify the correct behavior of `UpdateGetters` with `ExcludeFromCopy` configuration
- Ensure that the `FileCopyGetter` correctly respects the `ExcludeFromCopy` configuration when downloading Terraform sources
- Update the `UpdateGetters` function to pass the `TerragruntConfig` to the `FileCopyGetter` for proper handling of the `ExcludeFromCopy` configuration
@Excoriate Excoriate requested a review from yhakbar January 30, 2025 15:07
The changes in this commit remove an unnecessary comment from the `TestUpdateGettersExcludeFromCopy` test in the `download_source_test.go` file. This comment was not providing any valuable information and was cluttering the test code.
Copy link
Collaborator

@yhakbar yhakbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving our coverage!

@yhakbar yhakbar merged commit d5bc03c into gruntwork-io:main Jan 31, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Testing for exclude_from_copy
2 participants