fix(ci): failing terraform fmt (#1158) #1159
Conversation
WalkthroughThe workflow file has been updated to include a new step in the lint job. The GitHub Actions file now calls the Changes
Sequence Diagram(s)sequenceDiagram
participant GA as GitHub Actions
participant TS as Terraform Setup (hashicorp/setup-terraform@v3)
participant LP as Lint Process (make lint-examples)
GA->>TS: Invoke Terraform setup step
TS-->>GA: Terraform environment configured
GA->>LP: Execute linting process
LP-->>GA: Lint results returned
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've completed my review and didn't find any issues... but I did find this dog.
/ \__
( @\___
/ O
/ (_____/
/_____/ UNeed a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Documentation ✅ Logging ✅ Error Handling ✅ Readability ✅ Design ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
.github/workflows/test.yml
Outdated
| - name: terraform fmt | ||
| uses: hashicorp/setup-terraform@v3 | ||
| run: | | ||
| make tflint |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Separate Terraform setup from linter execution.
The current step combines a uses (for hashicorp/setup-terraform@v3) with a run command (make tflint), which is not valid in a single step. This configuration may cause unexpected behavior. I recommend splitting this into two distinct steps. Additionally, the step name “terraform fmt” is misleading since it actually executes make tflint; consider renaming it to reflect its purpose more accurately (e.g., “Setup Terraform for linting” and “Run TFLint”).
- - name: terraform fmt
- uses: hashicorp/setup-terraform@v3
- run: |
- make tflint
+ - name: Setup Terraform for linting
+ uses: hashicorp/setup-terraform@v3
+ - name: Run TFLint
+ run: make tflint📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: terraform fmt | |
| uses: hashicorp/setup-terraform@v3 | |
| run: | | |
| make tflint | |
| - name: Setup Terraform for linting | |
| uses: hashicorp/setup-terraform@v3 | |
| - name: Run TFLint | |
| run: make tflint |
🧰 Tools
🪛 actionlint (1.7.4)
33-33: this step is for running action since it contains at least one of "uses", "with" keys, but also contains "run" key which is used for running shell command
(syntax-check)
There was a problem hiding this comment.
I don't think the proposed name is better. In this case the right fix would be to rename the tflint step to lint-examples as the only thing it does is to check if the examples terraform syntax is correct.
Then the step in CI could be "Setup Terraform for linting examples"
There was a problem hiding this comment.
🧩 Analysis chain
@dmacvicar You make an excellent point about the naming. Let me check what the tflint target actually does in the Makefile to better understand its purpose.
🏁 Script executed:
#!/bin/bash
# Let's look at the Makefile to understand what the tflint target does
cat Makefile | grep -A 10 "tflint:"
Length of output: 205
@dmacvicar You're absolutely right. After checking the Makefile, I can see that the tflint target is specifically running terraform fmt on the examples directory to check formatting, not performing general linting.
Your suggested naming is much more accurate and descriptive of what the step actually does. Here's the corrected solution that both fixes the technical issue and uses better naming:
- - name: terraform fmt
- uses: hashicorp/setup-terraform@v3
- run: |
- make tflint
+ - name: Setup Terraform for linting examples
+ uses: hashicorp/setup-terraform@v3
+ - name: Check Terraform formatting in examples
+ run: make tflintFor even better clarity, you might consider renaming the Makefile target from tflint to lint-examples in a future update, as you suggested.
Description by Korbit AI
What change is being made?
Update the CI workflow to properly set up Terraform for linting examples and rename the
tflinttarget tolint-examplesin the Makefile.Why are these changes being made?
The change addresses the issue of failing Terraform formatting checks by ensuring Terraform is properly set up for linting and enhances clarity by renaming the target to accurately represent its function of linting examples. This improves maintainability and correctness of the CI process.
Summary by CodeRabbit