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

Copy lock file only when needed #2823

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ben-z
Copy link

@ben-z ben-z commented Dec 3, 2023

Description

Terragrunt copies lock files into the working directory every time Terraform is initialized. This is inconsistent with the Terraform behaviour where the lock file is only written to when changed.

This is important when the working directory is on a readonly filesystem. I work with an organization that does this to prevent unintentional code changes during provisioning.

This PR makes it so that copying the lock file is avoided when the file has no changes.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Updated the lock file copy mechanism to avoid copying when the lock file is unchanged. Useful for using a readonly working directory.

@ben-z ben-z requested a review from denis256 as a code owner December 3, 2023 05:47
@denis256
Copy link
Member

[INFO] Initializing environment for https://github.com/gruntwork-io/pre-commit.
Terraform fmt............................................................Passed
goimports................................................................Failed
- hook id: goimports
- files were modified by this hook

util/file.go
util/file.go

@ben-z
Copy link
Author

ben-z commented Jan 6, 2024

@denis256 I had some trailing whitespaces. Fixed in the latest commit by running pre-commit run --all-files:

pre-commit run --all-files

util/file.go Outdated
return errors.WithStackTrace(destReadErr)
}

if string(sourceContents) == string(destinationContents) {
Copy link
Member

Choose a reason for hiding this comment

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

util/file.go:613:5: stringXbytes: suggestion: bytes.Equal(sourceContents, destinationContents) (gocritic)
        if string(sourceContents) == string(destinationContents) {

Copy link
Author

Choose a reason for hiding this comment

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

Resolved in b3b9287

@denis256
Copy link
Member

denis256 commented Jan 8, 2024

Failed integration tests:

TestRenderJSONConfig
TestLocalDownload
TestLocalDownloadWithHiddenFolder
TestLocalDownloadWithRelativePath
TestLocalWithBackend
TestRemoteDownload
TestRemoteDownloadWithRelativePath
TestRemoteDownloadOverride
TestRemoteWithBackend
TestExcludeDirs
TestIncludeDirs
TestIncludeDirsDependencyConsistencyRegression
TestTerraformRegistryFetchingRootModule
TestTerraformRegistryFetchingRootShorthandModule
TestTerraformRegistryFetchingSubdirModule
TestTerraformRegistryFetchingSubdirWithReferenceModule
TestTerragruntInitHookWithSourceNoBackend
TestTerragruntInitHookWithSourceWithBackend
TestTerragruntCatchErrorsInTerraformExecution
TestTerragruntStdOut

@denis256
Copy link
Member

denis256 commented Jan 8, 2024

Will be also helpful to add an integration test which will track that the lock copy works as expected.

@ben-z
Copy link
Author

ben-z commented Feb 17, 2024

Will be also helpful to add an integration test which will track that the lock copy works as expected.

I added some unit tests. Integration tests seem a lot more involved and I'm not super familiar with Go. Are unit tests sufficient?

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.

None yet

2 participants