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

fix: Reintroduce symlink work behind experiment flag #3672

Merged
merged 12 commits into from
Dec 18, 2024

Conversation

yhakbar
Copy link
Collaborator

@yhakbar yhakbar commented Dec 16, 2024

Description

Closes #3654.

We have other experiments like the following that need to be pulled into this system if we don't have any problems with the design:

To ensure backwards compatibility, when they get pulled in, the existing flags that they use will be added to a strict control that warns users they are using a legacy experiment control mechanism, and should switch to using this new system.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added new experiment system. This system allows you to enable experimental features of Terragrunt before they're stable.
Removed Symlink support by default. Users have to opt-in to use it via the new experiment system.

Migration Guide

Users using the current ability to find symlinked units in their stacks will now need to explicitly enable the symlinks experiment to continue using this feature.

e.g.

terragrunt run-all plan --experiment symlinks

For information on how to use the new experiment system, see the Experiments documentation (which will go live once this PR is merged).

Copy link
Contributor

@levkohimins levkohimins left a comment

Choose a reason for hiding this comment

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

Left a few recommendations.

cli/commands/catalog/action.go Outdated Show resolved Hide resolved
internal/experiment/experiment.go Outdated Show resolved Hide resolved
@@ -146,7 +179,7 @@ func (src Source) WriteVersionFile() error {
// 1. Always download source URLs pointing to local file paths.
// 2. Only download source URLs pointing to remote paths if /T/W/H doesn't already exist or, if it does exist, if the
// version number in /T/W/H/.terragrunt-source-version doesn't match the current version.
func NewSource(source string, downloadDir string, workingDir string, logger log.Logger) (*Source, error) {
func NewSource(source string, downloadDir string, workingDir string, logger log.Logger, walkWithSymlinks bool) (*Source, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Can we add a structure to pass data in? more args will make complicated to understand values

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you don't mind, I'd like to punt on that. I think we should eventually just switch to options from a variadic argument.

@yhakbar yhakbar merged commit d2a0285 into main Dec 18, 2024
5 of 6 checks passed
@yhakbar yhakbar deleted the fix/reintroduce-symlink-work-behind-experiment-flag branch December 18, 2024 14:25
@jasonwbarnett
Copy link

jasonwbarnett commented Dec 18, 2024

Thanks so much for addressing this so quickly and thoughtfully. I really appreciate the effort to put this behind a feature flag—it’s a great solution that balances flexibility and safety, even though it’s more work on your end. Your responsiveness and attention to detail are what make contributing to this project such a great experience. 🚀

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.

Revert Recent Symlink Work and Reintroduce Behind Experiment Flag
4 participants