-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: TFLINT before_hook is not recognizing TFLINT_CONFIG_FILE environment variable #5290
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
base: main
Are you sure you want to change the base?
fix: TFLINT before_hook is not recognizing TFLINT_CONFIG_FILE environment variable #5290
Conversation
When running tflint as a before_hook, the TFLINT_CONFIG_FILE environment variable was not being recognized. This fix adds a check for the environment variable after checking for --config argument and before searching for .tflint.hcl files in the project hierarchy. The config file lookup now follows this priority: 1. --config argument in hook execute command 2. TFLINT_CONFIG_FILE environment variable 3. .tflint.hcl file in project directories Closes gruntwork-io#5286 Signed-off-by: majiayu000 <[email protected]>
|
@majiayu000 is attempting to deploy a commit to the Gruntwork Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdded environment variable support for TFLint configuration files in Terragrunt's before_hook. The implementation introduces a new constant Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tflint/tflint.go (1)
56-56: Consider making the log message more generic.The log message says "Using .tflint.hcl file" but
configFilemight not be named.tflint.hclwhen sourced from the environment variable or--configargument. Consider a more generic message like "Using tflint config file at %s" for clarity.🔎 Proposed fix
- l.Debugf("Using .tflint.hcl file in %s", configFile) + l.Debugf("Using tflint config file at %s", configFile)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/integration_tflint_test.gotflint/tflint.go
🧰 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:
tflint/tflint.gotest/integration_tflint_test.go
🧬 Code graph analysis (2)
tflint/tflint.go (1)
pkg/log/log.go (1)
Debugf(72-74)
test/integration_tflint_test.go (2)
test/integration_windows_test.go (1)
CopyEnvironmentWithTflint(239-264)test/helpers/package.go (2)
RemoveFolder(981-989)RunTerragruntCommand(1043-1047)
🔇 Additional comments (3)
tflint/tflint.go (2)
30-31: LGTM! Good practice to define the environment variable name as an exported constant.This makes the code more maintainable and provides clear documentation of the environment variable name.
38-44: LGTM! Environment variable check correctly implements the priority order.The logic correctly positions the environment variable check between the hook argument check and the project file search, matching the priority described in the PR objectives:
- --config argument (line 37)
- TFLINT_CONFIG_FILE environment variable (lines 39-43)
- Project file search (lines 46-54)
The implementation properly validates the environment variable value is non-empty and provides helpful debug logging.
test/integration_tflint_test.go (1)
199-231: LGTM! Comprehensive test validates the environment variable feature.The test is well-structured and effectively validates the TFLINT_CONFIG_FILE environment variable support:
Smart fixture choice: Uses
testFixtureTflintNoConfigFilewhich lacks a.tflint.hclfile and would normally fail, proving that the environment variable is the source of the config.Good isolation: Uses
t.Setenvto ensure the environment variable is scoped to this test only, preventing interference with other tests.Comprehensive assertions: Verifies the debug log message, the config path in the command arguments, and successful execution.
The test coverage aligns well with the implementation in
tflint/tflint.goand confirms the feature works as intended.
Fixes #5286
Changes
Config file lookup priority
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Updated TFLint before_hook to honor TFLINT_CONFIG_FILE env var.
Migration Guide
N/A