Skip to content

Preview/TFlint#98

Open
andrescastellanos-coderabbit wants to merge 2 commits intomainfrom
Preview/TFlint
Open

Preview/TFlint#98
andrescastellanos-coderabbit wants to merge 2 commits intomainfrom
Preview/TFlint

Conversation

@andrescastellanos-coderabbit
Copy link
Copy Markdown

@andrescastellanos-coderabbit andrescastellanos-coderabbit commented Apr 27, 2026

Screenshot 2026-04-28 at 1 54 35 PM

Demonstrates the TFLint sandboxer tool. Five .tf files (main, variables, outputs, modules, legacy) for an AWS-centric module, with intentionally embedded violations across 11 bundled terraform-* rules: terraform_required_version, terraform_required_providers, terraform_typed_variables, terraform_documented_variables, terraform_documented_outputs, terraform_unused_declarations, terraform_naming_convention, terraform_module_pinned_source, terraform_module_version, terraform_deprecated_index, terraform_deprecated_interpolation, terraform_empty_list_equality. .coderabbit.yml disables Trivy and Checkov to keep .tf findings tagged TFLint rather than shadowed by the other IaC scanners. Caveat: per CodeRabbit's TFLint docs, the wrapper writes a safe override that enables only the bundled terraform ruleset — AWS/GCP/Azure provider plugins are stripped at runtime, so provider-specific rule IDs (e.g. aws_instance_invalid_type) won't appear. This PR demonstrates the bundled terraform-* rules only.

Summary by CodeRabbit

  • New Features

    • Provisioned cloud infrastructure: compute instances, storage bucket, managed database, networking, and DNS.
    • Added outputs for instance IDs, public IPs, bucket name, and service role ARN for integrations.
  • Chores

    • Added configuration for linting/review tools and infrastructure lint rules to enforce best practices.

…les across 11 categories, disable Trivy and Checkov to avoid shadowing
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

Adds a CodeRabbit config and a multi-file Terraform configuration: provider, EC2 instances, S3 bucket, IAM role, conditional EIP/Route53/Security Rule, module declarations, variables, outputs, and TFLint configuration.

Changes

Cohort / File(s) Summary
Review Tool Configuration
/.coderabbit.yml
Adds CodeRabbit review settings: enables tflint, disables trivy and checkov.
Core Terraform: provider, resources, outputs, variables
tflint/main.tf, tflint/outputs.tf, tflint/variables.tf
Adds AWS provider (region = us-east-1), two aws_instance resources, an aws_s3_bucket, an aws_iam_role, related outputs (instance IDs/IPs, bucket name, sensitive role ARN, computed_name, all_instance_ids) and variables/locals (environment, region, instance_type, admin_email, tags, subnets, computed_name).
Legacy & Conditional Networking
tflint/legacy.tf
Adds conditional aws_eip (based on var.subnets), Route53 A record mapping demoapp.example.com to EIP, SSH aws_security_group_rule created per subnet, and outputs first_subnet and primary_eip.
Module declarations
tflint/modules.tf
Adds module blocks: vpc (terraform-aws-modules/vpc/aws), rds (terraform-aws-modules/rds/aws @ 5.9.0), plus shared_helpers, internal_observability (datadog subpath), and a local legacy_networking module.
TFLint config
/.tflint.hcl
Adds TFLint configuration enabling recommended plugin and many lint rules (provider/version, docs/typing, unused declarations, naming, module pinning, deprecated patterns, empty list equality, unused required providers).

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Terraform (local)
    participant Provider as AWS Provider
    participant EC2 as EC2
    participant S3 as S3
    participant IAM as IAM
    participant Route53 as Route53

    Dev->>Provider: init & plan
    Provider->>EC2: create aws_instance (MyInstance, WebServer2)
    Provider->>S3: create aws_s3_bucket (ArtifactsBucket)
    Provider->>IAM: create aws_iam_role (ServiceRole)
    alt var.subnets != []
        Provider->>Provider: create aws_eip.Web[*]
        Provider->>Route53: create aws_route53_record.Apex -> map demoapp.example.com to EIP
        Provider->>EC2: create aws_security_group_rule.AllowSsh per subnet
    end
    Provider->>Dev: return outputs (instance IDs/IPs, bucket, role ARN, first_subnet, primary_eip)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 I hopped through lines with tiny feet,

Modules stitched and outputs neat.
Buckets, roles, and IPs in tow,
A rabbit’s patch where infra grows—
A small commit, a quiet beat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Preview/TFlint' is vague and overly abbreviated; it does not clearly convey the main change (adding Terraform configuration files with intentional TFLint violations for testing). Consider a more descriptive title such as 'Add Terraform configuration with TFLint rule violations for testing' to better communicate the PR's purpose.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Preview/TFlint

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07f463687b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .coderabbit.yml
Comment on lines +3 to +6
trivy:
enabled: false
checkov:
enabled: false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore security scanners in default CodeRabbit config

Re-enable these tools in the committed .coderabbit.yml baseline because this change disables Trivy and Checkov for all future PRs in the repo, not just this TFLint preview. That creates a persistent blind spot for IaC/container/security findings and can let regressions merge unnoticed; if the suppression is only for a demo run, it should be scoped to that run instead of committed as the repository default.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (6)
tflint/outputs.tf (2)

1-16: Add descriptions for public outputs to make module contracts clearer.

Only computed_name is documented; the other exported outputs should also include description.

Also applies to: 23-25

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/outputs.tf` around lines 1 - 16, The outputs instance_id,
instance_public_ip, artifacts_bucket, and service_role_arn lack description
fields; update each output block (output "instance_id", output
"instance_public_ip", output "artifacts_bucket", output "service_role_arn") to
include a clear description = "..." string explaining what that output
represents (e.g., instance ID, public IP, S3 bucket name, IAM role ARN) while
preserving sensitive = true for service_role_arn and existing values so the
module contract is explicit.

24-24: Use modern splat syntax ([*]) instead of legacy .*.

Line 24 should use aws_instance.WebServer2[*].id.

Suggested fix
 output "all_instance_ids" {
-  value = aws_instance.WebServer2.*.id
+  value = aws_instance.WebServer2[*].id
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/outputs.tf` at line 24, The output uses the legacy splat operator `.*`
on the resource reference; update the value to use Terraform's modern splat
syntax by replacing the `aws_instance.WebServer2.*.id` reference with the new
form that uses `[*]` so the expression reads like the modern splat for the
resource `aws_instance.WebServer2` and returns all ids.
tflint/main.tf (1)

4-6: Use var.region instead of a hardcoded provider region.

region = "us-east-1" bypasses your input variable and makes environment overrides harder.

Suggested fix
 provider "aws" {
-  region = "us-east-1"
+  region = var.region
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/main.tf` around lines 4 - 6, Replace the hardcoded provider region in
the aws provider block by using the input variable (change region = "us-east-1"
to region = var.region) in the provider "aws" configuration; also ensure a
Terraform input variable named region is defined (variable "region") so callers
can override it via tfvars/ENV or defaults.
tflint/modules.tf (1)

27-29: Relative module source is brittle outside this repo layout.

If this module is executed from Terraform Cloud/remote runners or moved in-repo, ../../shared-modules/networking can break resolution.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/modules.tf` around lines 27 - 29, The module "legacy_networking" uses
a brittle relative source ("../../shared-modules/networking") which will break
in Terraform Cloud/remote runners or if the repo layout changes; change the
module "legacy_networking" declaration to reference a stable remote source (for
example a VCS URL with a ref, a registry identifier, or a module published to
your private/module registry) instead of the relative path, and update any
callers to use that new source string so resolution works from remote/Cloud
runs.
tflint/variables.tf (1)

17-21: Remove or gate unused declarations to reduce configuration noise.

unused_legacy_setting and unused_local appear unreferenced and can confuse module consumers.

Also applies to: 34-37

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/variables.tf` around lines 17 - 21, The two unused variable
declarations (unused_legacy_setting and unused_local) are unused and should be
removed or gated; either delete the variable blocks for unused_legacy_setting
and unused_local from variables.tf, or add a single controlling feature flag
(e.g., enable_legacy_settings) and move those variables behind that gating so
they're only declared/used when the flag is true, updating any module docs and
defaults accordingly to avoid surprising consumers.
tflint/legacy.tf (1)

3-14: Replace legacy interpolation and empty-list equality with native expressions.

Line 3, Line 8, Line 13, Line 27, and Line 28 use deprecated/legacy expression styles.

Suggested fix
 resource "aws_eip" "Web" {
   count    = length(var.subnets) > 0 ? 1 : 0
-  instance = "${aws_instance.MyInstance.id}"
+  instance = aws_instance.MyInstance.id
   domain   = "vpc"
 }

 resource "aws_route53_record" "Apex" {
-  count   = var.subnets == [] ? 0 : 1
+  count   = length(var.subnets) == 0 ? 0 : 1
   zone_id = "Z123456ABCDEFG"
   name    = "demoapp.example.com"
   type    = "A"
   ttl     = 60
-  records = ["${aws_eip.Web.*.public_ip[0]}"]
+  records = [aws_eip.Web[0].public_ip]
 }

 locals {
-  primary_region = "${var.region}"
-  environment    = "${var.environment}"
+  primary_region = var.region
+  environment    = var.environment
 }

Also applies to: 26-29

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 3 - 14, Replace legacy interpolations and the
empty-list equality with native expressions: remove "${...}" wrappers and use
direct references (e.g., change instance = "${aws_instance.MyInstance.id}" to
instance = aws_instance.MyInstance.id and change records =
["${aws_eip.Web.*.public_ip[0]}"] to records = [aws_eip.Web[0].public_ip] or the
appropriate index/reference for aws_eip.Web), and replace the var.subnets == []
conditional used for count with a native check like length(var.subnets) == 0
(e.g., count = length(var.subnets) == 0 ? 0 : 1) for the aws_route53_record
"Apex" resource.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tflint/legacy.tf`:
- Around line 31-33: The output "first_subnet" is indexing var.subnets directly
which will fail when var.subnets is empty; update the output to guard access by
checking length(var.subnets) (or using a conditional) and return a safe fallback
(e.g. null or empty string) when the list is empty so the expression never
attempts var.subnets[0]; modify the output block named "first_subnet" to use
that conditional guard around var.subnets[0].
- Around line 35-37: The output "primary_eip" currently indexes
aws_eip.Web.*.public_ip[0] which will raise an index out of range if aws_eip.Web
has count = 0; update the output "primary_eip" to guard against an empty splat
result by using a conditional that checks the list length (or uses a safe
default) and returns the first public_ip only when aws_eip.Web.* is non-empty,
otherwise return a sensible fallback (empty string or null) so the output never
attempts to index an empty list.
- Around line 16-24: The current aws_security_group_rule "AllowSsh" uses count
and deprecated interpolation which creates identical duplicate rules when
var.subnets has multiple items; either remove count to create a single ingress
rule (keep cidr_blocks = ["0.0.0.0/0"] and set security_group_id =
aws_instance.MyInstance.vpc_security_group_ids[0] using direct expression
without "${...}"), or if you truly need one rule per subnet convert to for_each
= { for s in var.subnets: s => s } and set cidr_blocks = [each.value] and
security_group_id = aws_instance.MyInstance.vpc_security_group_ids[0] (again
using direct references) so each generated rule is unique and avoids
DuplicatePermission errors.

In `@tflint/main.tf`:
- Around line 8-20: Replace the hardcoded instance types and AMI IDs in the
aws_instance resources with variables: update resource "aws_instance"
"MyInstance" and resource "aws_instance" "WebServer2" to reference
var.instance_type and var.ami instead of "t3.medium" and the literal AMI; add a
new variable "ami" (with description and optional default) and ensure the
existing instance_type variable is used (var.instance_type); keep existing tags
like Name unchanged so only the ami and instance_type references are changed.
- Around line 22-24: The S3 bucket resource aws_s3_bucket.ArtifactsBucket
currently hardcodes a globally-unique name which will collide; change its bucket
attribute to use the existing environment variable (var.environment) so the name
is parameterized (e.g., follow the computed_name/local pattern you already use)
and update any references to aws_s3_bucket.ArtifactsBucket if they assume the
old fixed name.
- Around line 1-2: The terraform block is empty and must declare version
constraints; update the terraform { } block to include a required_version
constraint (e.g. required_version = ">= 1.5.0") and a required_providers map
specifying each provider with its source and version constraints (e.g.
required_providers = { aws = { source = "hashicorp/aws" version = ">= 4.0.0" } }
), using the terraform block symbol to locate where to add these fields and
ensure you choose appropriate provider names and version ranges for your
project.

In `@tflint/modules.tf`:
- Around line 19-25: The Git module sources are unpinned which can lead to
unexpected updates; update the module "shared_helpers" and module
"internal_observability" source URLs by appending immutable refs (e.g.
?ref=v1.2.3 for shared_helpers and ?ref=v2.4.1 for internal_observability) so
the source fields in the module "shared_helpers" and module
"internal_observability" blocks point to specific semantic-version tags or
commit SHAs instead of the branch/HEAD.

In `@tflint/variables.tf`:
- Around line 1-32: The variables file is missing explicit types for variables
that are used and contains unused variables; add type = "string" to the
environment variable and add an explicit collection type (e.g., type =
list(string)) to subnets to match how they are referenced, and then review and
either remove or explicitly type the unused variables instance_type,
admin_email, and tags (or mark them as deprecated) so the config is clear;
update the declarations for environment, subnets, and adjust or remove
instance_type, admin_email, and tags accordingly, keeping unused_legacy_setting
as-is.

---

Nitpick comments:
In `@tflint/legacy.tf`:
- Around line 3-14: Replace legacy interpolations and the empty-list equality
with native expressions: remove "${...}" wrappers and use direct references
(e.g., change instance = "${aws_instance.MyInstance.id}" to instance =
aws_instance.MyInstance.id and change records =
["${aws_eip.Web.*.public_ip[0]}"] to records = [aws_eip.Web[0].public_ip] or the
appropriate index/reference for aws_eip.Web), and replace the var.subnets == []
conditional used for count with a native check like length(var.subnets) == 0
(e.g., count = length(var.subnets) == 0 ? 0 : 1) for the aws_route53_record
"Apex" resource.

In `@tflint/main.tf`:
- Around line 4-6: Replace the hardcoded provider region in the aws provider
block by using the input variable (change region = "us-east-1" to region =
var.region) in the provider "aws" configuration; also ensure a Terraform input
variable named region is defined (variable "region") so callers can override it
via tfvars/ENV or defaults.

In `@tflint/modules.tf`:
- Around line 27-29: The module "legacy_networking" uses a brittle relative
source ("../../shared-modules/networking") which will break in Terraform
Cloud/remote runners or if the repo layout changes; change the module
"legacy_networking" declaration to reference a stable remote source (for example
a VCS URL with a ref, a registry identifier, or a module published to your
private/module registry) instead of the relative path, and update any callers to
use that new source string so resolution works from remote/Cloud runs.

In `@tflint/outputs.tf`:
- Around line 1-16: The outputs instance_id, instance_public_ip,
artifacts_bucket, and service_role_arn lack description fields; update each
output block (output "instance_id", output "instance_public_ip", output
"artifacts_bucket", output "service_role_arn") to include a clear description =
"..." string explaining what that output represents (e.g., instance ID, public
IP, S3 bucket name, IAM role ARN) while preserving sensitive = true for
service_role_arn and existing values so the module contract is explicit.
- Line 24: The output uses the legacy splat operator `.*` on the resource
reference; update the value to use Terraform's modern splat syntax by replacing
the `aws_instance.WebServer2.*.id` reference with the new form that uses `[*]`
so the expression reads like the modern splat for the resource
`aws_instance.WebServer2` and returns all ids.

In `@tflint/variables.tf`:
- Around line 17-21: The two unused variable declarations (unused_legacy_setting
and unused_local) are unused and should be removed or gated; either delete the
variable blocks for unused_legacy_setting and unused_local from variables.tf, or
add a single controlling feature flag (e.g., enable_legacy_settings) and move
those variables behind that gating so they're only declared/used when the flag
is true, updating any module docs and defaults accordingly to avoid surprising
consumers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: 50198756-03a9-4691-a232-ec819a2cdc25

📥 Commits

Reviewing files that changed from the base of the PR and between ef04b8d and 07f4636.

📒 Files selected for processing (6)
  • .coderabbit.yml
  • tflint/legacy.tf
  • tflint/main.tf
  • tflint/modules.tf
  • tflint/outputs.tf
  • tflint/variables.tf

Comment thread tflint/legacy.tf
Comment on lines +16 to +24
resource "aws_security_group_rule" "AllowSsh" {
count = "${length(var.subnets)}"
type = "ingress"
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
security_group_id = "${aws_instance.MyInstance.vpc_security_group_ids[0]}"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "legacy.tf" -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 96


🏁 Script executed:

find . -name "variables.tf" -type f | head -5

Repository: coderabbitai/coderabbit-pr-review

Length of output: 99


🌐 Web query:

AWS security group rule duplicate identical rules behavior terraform

💡 Result:

AWS rejects attempts to add duplicate identical security group rules via the EC2 API (AuthorizeSecurityGroupIngress/AuthorizeSecurityGroupEgress), returning InvalidPermission.Duplicate error. AWS canonicalizes CIDRs, so non-canonical forms matching existing canonical rules also fail as duplicates. In Terraform: - aws_security_group with inline ingress/egress assumes exclusive management of all rules; mixing with aws_security_group_rule causes conflicts and overwrites. - aws_security_group_rule (legacy) supports multiple CIDRs/sources per rule but has issues with tags/descriptions (pre-unique rule IDs), state drift, and duplicates during applies (e.g., count/for_each index shifts, dynamic values). Often fails with duplicate warnings during parallel create/delete. - Best practice: Use aws_vpc_security_group_ingress_rule and aws_vpc_security_group_egress_rule (modern resources). One source per rule, leverages unique rule IDs, stable with for_each on maps(avoiding list index churn), prevents duplicates. Example for idempotency: resource "aws_vpc_security_group_ingress_rule" "example" { for_each = var.ingress_rules # map(object({...})) with stable keys security_group_id = aws_security_group.example.id ip_protocol = each.value.protocol from_port = each.value.from_port to_port = each.value.to_port cidr_ipv4 = each.value.cidr_ipv4 # or referenced_security_group_id description = each.value.description } Adding rules only affects new ones; existing unchanged. Import existing via security_group_rule_id if needed. Do not mix resource types on same SG. Use modules like terraform-aws-modules/security-group/aws for convenience.

Citations:


🏁 Script executed:

cat -n ./tflint/legacy.tf | sed -n '1,40p'

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1227


🏁 Script executed:

cat -n ./tflint/variables.tf | grep -A 5 "var.subnets"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

cat -n ./tflint/variables.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 953


count creates duplicate SSH rules that will fail on apply when var.subnets has multiple items.

When var.subnets contains 2 or more items, this creates N identical ingress rules with the same CIDR block and port, which AWS rejects with a DuplicatePermission error. Additionally, the deprecated interpolation syntax "${...}" should be modernized.

Suggested fix
 resource "aws_security_group_rule" "AllowSsh" {
-  count             = "${length(var.subnets)}"
+  count             = length(var.subnets) > 0 ? 1 : 0
   type              = "ingress"
   from_port         = 22
   to_port           = 22
   protocol          = "tcp"
   cidr_blocks       = ["0.0.0.0/0"]
-  security_group_id = "${aws_instance.MyInstance.vpc_security_group_ids[0]}"
+  security_group_id = aws_instance.MyInstance.vpc_security_group_ids[0]
 }
📝 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.

Suggested change
resource "aws_security_group_rule" "AllowSsh" {
count = "${length(var.subnets)}"
type = "ingress"
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
security_group_id = "${aws_instance.MyInstance.vpc_security_group_ids[0]}"
}
resource "aws_security_group_rule" "AllowSsh" {
count = length(var.subnets) > 0 ? 1 : 0
type = "ingress"
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
security_group_id = aws_instance.MyInstance.vpc_security_group_ids[0]
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 16 - 24, The current aws_security_group_rule
"AllowSsh" uses count and deprecated interpolation which creates identical
duplicate rules when var.subnets has multiple items; either remove count to
create a single ingress rule (keep cidr_blocks = ["0.0.0.0/0"] and set
security_group_id = aws_instance.MyInstance.vpc_security_group_ids[0] using
direct expression without "${...}"), or if you truly need one rule per subnet
convert to for_each = { for s in var.subnets: s => s } and set cidr_blocks =
[each.value] and security_group_id =
aws_instance.MyInstance.vpc_security_group_ids[0] (again using direct
references) so each generated rule is unique and avoids DuplicatePermission
errors.

Comment thread tflint/legacy.tf
Comment on lines +31 to +33
output "first_subnet" {
value = "${var.subnets[0]}"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "(legacy\.tf|variables\.tf)" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 114


🏁 Script executed:

# Check if the files exist and examine their content
if [ -f "tflint/variables.tf" ]; then
  echo "=== tflint/variables.tf ===" 
  cat -n "tflint/variables.tf"
fi

Repository: coderabbitai/coderabbit-pr-review

Length of output: 981


🏁 Script executed:

if [ -f "tflint/legacy.tf" ]; then
  echo "=== tflint/legacy.tf ===" 
  cat -n "tflint/legacy.tf"
fi

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1252


🌐 Web query:

Terraform accessing index 0 of empty list error behavior

💡 Result:

In Terraform, attempting to access index 0 (or any index) of an empty list or tuple results in a runtime error during plan or apply phases. The error message is typically "Error: Invalid index" followed by details like "on [file.tf] line [X], in resource [name]: [Y]: [list_ref][0] is empty tuple" and "The given key does not identify an element in this collection value: the collection has no elements." This occurs because Terraform performs static validation and evaluates expressions even when guarded by count or conditional logic that should skip them if the list is empty. Direct indexing like mylist[0] fails immediately if length(mylist) == 0. Common scenarios include: - Module outputs returning empty lists when count=0 (e.g., random_pet.names[].id when no resources created). - Resource attributes referencing empty dynamic lists. - Splat expressions (.*) on zero-count resources producing empty tuples. Workarounds to handle empty lists safely: 1. Use try: try(mylist[0], "default") - attempts access, falls back if invalid. 2. Conditional with length: length(mylist) > 0 ? mylist[0] : "default". 3. For optional resources: set count = length(mylist) and avoid fixed indexing. Note: The element(list, index) function wraps around with modulo for out-of-bounds (non-empty lists) but also fails on empty lists. The index(list, value) function errors if value not found. This behavior is consistent across Terraform versions, including recent ones as of 2026.

Citations:


Add a guard clause to prevent index access on empty list.

var.subnets[0] will crash when var.subnets is empty (default is [] in tflint/variables.tf, line 31). This output needs to safely handle the empty case.

Suggested fix
 output "first_subnet" {
-  value = "${var.subnets[0]}"
+  value = length(var.subnets) > 0 ? var.subnets[0] : null
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 31 - 33, The output "first_subnet" is indexing
var.subnets directly which will fail when var.subnets is empty; update the
output to guard access by checking length(var.subnets) (or using a conditional)
and return a safe fallback (e.g. null or empty string) when the list is empty so
the expression never attempts var.subnets[0]; modify the output block named
"first_subnet" to use that conditional guard around var.subnets[0].

Comment thread tflint/legacy.tf
Comment on lines +35 to +37
output "primary_eip" {
value = "${aws_eip.Web.*.public_ip[0]}"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "legacy.tf" -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 96


🏁 Script executed:

sed -n '30,40p' ./tflint/legacy.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 202


🌐 Web query:

Terraform splat operator count = 0 empty resource index behavior

💡 Result:

In Terraform, when a resource or data source has count = 0, no instances are created. The splat operator (both legacy .* and modern []) applied to such a resource returns an empty list/tuple (length 0). This empty list behavior is the standard and expected outcome, allowing safe usage in expressions like length(aws_instance.example[].id) which evaluates to 0, or join(",", aws_instance.example[].id) which returns an empty string. It enables conditional logic, such as setting another resource's count = length(data.example[].outputs) to 0. Direct indexing like aws_instance.example[0].id fails with an error ("empty tuple" or "index out of range") because no instances exist. Use splat expressions to handle both zero and positive counts gracefully. Official documentation confirms splat on non-collections (including empty resource lists) yields empty tuples, and multiple sources demonstrate this produces [] with length = 0 (e.g., GitHub issue #26526 shows outputs = [], length = 0; StackOverflow example with join returns empty string; HashiCorp Discuss and issues consistently describe empty list return). This holds for modern Terraform versions (post-0.12, including 2026 releases), with early bugs around planning fixed by 0.14.

Citations:


primary_eip output will fail when aws_eip.Web has count = 0.

The splat operator aws_eip.Web.* returns an empty list when count is 0, and indexing into an empty list with [0] causes an "index out of range" error at runtime.

Use a conditional to safely handle the empty case:

Suggested fix
 output "primary_eip" {
-  value = "${aws_eip.Web.*.public_ip[0]}"
+  value = length(aws_eip.Web) > 0 ? aws_eip.Web[0].public_ip : null
 }
📝 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.

Suggested change
output "primary_eip" {
value = "${aws_eip.Web.*.public_ip[0]}"
}
output "primary_eip" {
value = length(aws_eip.Web) > 0 ? aws_eip.Web[0].public_ip : null
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 35 - 37, The output "primary_eip" currently
indexes aws_eip.Web.*.public_ip[0] which will raise an index out of range if
aws_eip.Web has count = 0; update the output "primary_eip" to guard against an
empty splat result by using a conditional that checks the list length (or uses a
safe default) and returns the first public_ip only when aws_eip.Web.* is
non-empty, otherwise return a sensible fallback (empty string or null) so the
output never attempts to index an empty list.

Comment thread tflint/main.tf
Comment on lines +1 to +2
terraform {
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "\.tf$" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 165


🏁 Script executed:

# Check for tflint directory and main.tf
find . -name "main.tf" -o -name "variables.tf" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 116


🏁 Script executed:

# Read the specific files mentioned in the review
if [ -f "tflint/main.tf" ]; then
  echo "=== tflint/main.tf ===" && cat -n "tflint/main.tf"
fi

if [ -f "tflint/variables.tf" ]; then
  echo -e "\n=== tflint/variables.tf ===" && cat -n "tflint/variables.tf"
fi

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1985


🌐 Web query:

Terraform version 1.5.0 AWS provider 5.0 current stable versions April 2026

💡 Result:

As of April 27, 2026, the current stable version of Terraform is 1.14.9, released on April 20, 2026. The current stable version of the HashiCorp AWS provider is 6.42.0, released on April 22, 2026. Note that Terraform 1.15.0 is in release candidate stage (rc3 on April 22, 2026) and not yet stable.

Citations:


🌐 Web query:

Hashicorp Terraform latest stable version 2026

💡 Result:

The latest stable version of HashiCorp Terraform as of 2026-04-27 is 1.14.9, released on April 20, 2026. This is confirmed by the official HashiCorp install page listing it as the latest version with binaries available, the GitHub releases page topping the list with 1.14.9 followed by pre-release versions of 1.15.x (rc3 on April 22, 2026), and the dedicated release page and CHANGELOG for v1.14.9 documenting it as a stable bug-fix release.

Citations:


🌐 Web query:

AWS Terraform provider latest stable version 2026

💡 Result:

The latest stable version of the HashiCorp AWS Terraform provider is 6.42.0.

Citations:


Add required_version and required_providers constraints.

An empty terraform {} block allows unbounded Terraform/provider versions, which can break plans unexpectedly.

Suggested fix
 terraform {
+  required_version = ">= 1.10.0, < 2.0.0"
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 6.0"
+    }
+  }
 }
📝 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.

Suggested change
terraform {
}
terraform {
required_version = ">= 1.10.0, < 2.0.0"
required_providers {
aws = {
source = "hashicorp/aws"
version = "~> 6.0"
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/main.tf` around lines 1 - 2, The terraform block is empty and must
declare version constraints; update the terraform { } block to include a
required_version constraint (e.g. required_version = ">= 1.5.0") and a
required_providers map specifying each provider with its source and version
constraints (e.g. required_providers = { aws = { source = "hashicorp/aws"
version = ">= 4.0.0" } } ), using the terraform block symbol to locate where to
add these fields and ensure you choose appropriate provider names and version
ranges for your project.

Comment thread tflint/main.tf
Comment on lines +8 to +20
resource "aws_instance" "MyInstance" {
ami = "ami-0c55b159cbfafe1f0"
instance_type = "t3.medium"

tags = {
Name = "demoapp-web"
}
}

resource "aws_instance" "WebServer2" {
ami = "ami-0c55b159cbfafe1f0"
instance_type = "t3.medium"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd tflint && git ls-files | grep -E "\.tf$|\.tfvars$"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 130


🏁 Script executed:

cat -n tflint/main.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1057


🏁 Script executed:

ls -la tflint/

Repository: coderabbitai/coderabbit-pr-review

Length of output: 480


🏁 Script executed:

cat -n tflint/variables.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 953


Use the instance_type variable instead of hardcoding, and add a variable for AMI to support dynamic configuration.

The instance_type variable is defined but not referenced in either resource—both hardcode "t3.medium" instead of using var.instance_type. Additionally, AMI IDs are hardcoded without parameterization, which blocks drift management and makes AMI updates require code changes. Define an ami variable and update both resources to reference var.instance_type and var.ami.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/main.tf` around lines 8 - 20, Replace the hardcoded instance types and
AMI IDs in the aws_instance resources with variables: update resource
"aws_instance" "MyInstance" and resource "aws_instance" "WebServer2" to
reference var.instance_type and var.ami instead of "t3.medium" and the literal
AMI; add a new variable "ami" (with description and optional default) and ensure
the existing instance_type variable is used (var.instance_type); keep existing
tags like Name unchanged so only the ami and instance_type references are
changed.

Comment thread tflint/main.tf
Comment on lines +22 to +24
resource "aws_s3_bucket" "ArtifactsBucket" {
bucket = "demoapp-artifacts-prod"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.tf" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 175


🏁 Script executed:

fd -e tf -e json -e yaml | grep -E "(main|terraform)" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 92


🏁 Script executed:

cat -n tflint/main.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1057


🏁 Script executed:

cat -n tflint/variables.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 953


🏁 Script executed:

cat -n tflint/modules.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 937


🏁 Script executed:

cat -n tflint/legacy.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1227


🏁 Script executed:

rg -A 3 "aws_s3_bucket" tflint/

Repository: coderabbitai/coderabbit-pr-review

Length of output: 374


🏁 Script executed:

rg "bucket\s*=" tflint/

Repository: coderabbitai/coderabbit-pr-review

Length of output: 128


🌐 Web query:

AWS S3 bucket naming rules global uniqueness across accounts

💡 Result:

Yes, AWS S3 general purpose bucket names must be globally unique across all AWS accounts within the same partition (e.g., aws for Standard Regions). By default, they exist in a shared global namespace, meaning once a bucket with a name is created by any account in the partition, no other account can create a bucket with that name until it is deleted. Partitions include aws (Standard), aws-cn (China), aws-us-gov (GovCloud US), and aws-eusc (European Sovereign Cloud). Optionally, buckets can be created in an account-specific regional namespace (with suffix like -accountid-region-an), which are unique only to that account and not subject to global uniqueness.

Citations:


Use the environment variable for the S3 bucket name to avoid global naming collisions.

AWS S3 bucket names are globally unique across all AWS accounts. The hardcoded name demoapp-artifacts-prod will fail creation when deploying to other environments or accounts. Use the existing environment variable to parameterize the bucket name:

bucket = "demoapp-artifacts-${var.environment}"

This matches the pattern already established in your computed_name local.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/main.tf` around lines 22 - 24, The S3 bucket resource
aws_s3_bucket.ArtifactsBucket currently hardcodes a globally-unique name which
will collide; change its bucket attribute to use the existing environment
variable (var.environment) so the name is parameterized (e.g., follow the
computed_name/local pattern you already use) and update any references to
aws_s3_bucket.ArtifactsBucket if they assume the old fixed name.

Comment thread tflint/modules.tf
Comment on lines +19 to +25
module "shared_helpers" {
source = "git::https://github.com/demoapp/terraform-shared-helpers.git"
}

module "internal_observability" {
source = "git::ssh://git@github.com/demoapp/terraform-observability.git//modules/datadog"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "modules.tf" -o -name "modules.tf" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 97


🌐 Web query:

Terraform Git module sources ref pinning best practices

💡 Result:

Best practices for pinning Terraform Git module sources emphasize using the ref query parameter in the source URL to specify immutable references, preventing unexpected changes during terraform init. Key recommendations: 1. Prefer Git tags: Use semantic version tags (e.g., v1.2.0) as they are immutable and human-readable. Example: module "vpc" { source = "git::https://example.com/vpc.git?ref=v1.2.0" } 2. Use commit SHAs for precision: For exact reproducibility, pin to a full commit hash (e.g., ?ref=abc123...). Example: module "storage" { source = "git::https://example.com/storage.git?ref=51d462976d84fdea54b47d80dcabbf680badcdb8" } 3. Avoid branches in production: Branches like main or develop move with new commits, leading to non-reproducible builds. Only use for development. 4. Tagging strategy: For monorepos or multiple modules, use prefixed tags (e.g., vpc-v2.3.1). Create annotated tags: git tag -a v1.2.0 -m "Release v1.2.0" git push origin v1.2.0 5. Shallow clones for large repos: Add ?depth=1 to speed up cloning while preserving ref pinning: source = "git::https://example.com/repo.git//modules/vpc?ref=v1.2.0&amp;depth=1" Note: .terraform.lock.hcl locks providers but not modules; pinning must be explicit in module blocks. Always review changes before upgrading refs with terraform init -upgrade.

Citations:


🏁 Script executed:

cat -n ./tflint/modules.tf | sed -n '15,30p'

Repository: coderabbitai/coderabbit-pr-review

Length of output: 552


Pin Git module sources with immutable refs (?ref=).

Both Git-based modules are unpinned, so branch/default HEAD changes can silently alter infrastructure behavior. Pin to specific semantic version tags or commit SHAs:

  • shared_helpers: Add ?ref=v1.2.3 (or your target version)
  • internal_observability: Add ?ref=v2.4.1 (or your target version)

Note: .terraform.lock.hcl does not lock modules, only providers—pinning must be explicit in module source URLs.

Suggested fix
 module "shared_helpers" {
-  source = "git::https://github.com/demoapp/terraform-shared-helpers.git"
+  source = "git::https://github.com/demoapp/terraform-shared-helpers.git?ref=v1.2.3"
 }

 module "internal_observability" {
-  source = "git::ssh://git@github.com/demoapp/terraform-observability.git//modules/datadog"
+  source = "git::ssh://git@github.com/demoapp/terraform-observability.git//modules/datadog?ref=v2.4.1"
 }
📝 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.

Suggested change
module "shared_helpers" {
source = "git::https://github.com/demoapp/terraform-shared-helpers.git"
}
module "internal_observability" {
source = "git::ssh://git@github.com/demoapp/terraform-observability.git//modules/datadog"
}
module "shared_helpers" {
source = "git::https://github.com/demoapp/terraform-shared-helpers.git?ref=v1.2.3"
}
module "internal_observability" {
source = "git::ssh://git@github.com/demoapp/terraform-observability.git//modules/datadog?ref=v2.4.1"
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/modules.tf` around lines 19 - 25, The Git module sources are unpinned
which can lead to unexpected updates; update the module "shared_helpers" and
module "internal_observability" source URLs by appending immutable refs (e.g.
?ref=v1.2.3 for shared_helpers and ?ref=v2.4.1 for internal_observability) so
the source fields in the module "shared_helpers" and module
"internal_observability" blocks point to specific semantic-version tags or
commit SHAs instead of the branch/HEAD.

Comment thread tflint/variables.tf
Comment on lines +1 to +32
variable "environment" {
default = "production"
}

variable "region" {
type = string
default = "us-east-1"
}

variable "instance_type" {
default = "t3.medium"
}

variable "admin_email" {
}

variable "unused_legacy_setting" {
type = string
description = "Legacy toggle retained during migration. Not currently consumed."
default = "off"
}

variable "tags" {
default = {
Project = "demoapp"
Owner = "platform-team"
}
}

variable "subnets" {
default = []
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the variables.tf file
find . -name "variables.tf" -type f | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 99


🏁 Script executed:

# Check if tflint directory exists and list its contents
ls -la tflint/ 2>/dev/null || echo "tflint directory not found"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 480


🏁 Script executed:

# Search for any references to these variables in the codebase
rg "(environment|instance_type|admin_email|tags|subnets)" --type tf -B 2 -A 2 | head -100

Repository: coderabbitai/coderabbit-pr-review

Length of output: 2814


🏁 Script executed:

# Read the complete variables.tf file to see all content
cat -n tflint/variables.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 953


🏁 Script executed:

# Check if admin_email is referenced anywhere in the codebase
rg "admin_email" --type tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 122


🏁 Script executed:

# Check how instance_type and tags variables are actually used
rg "var\.instance_type|var\.tags" --type tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

# Check actual usage of var.instance_type (not hardcoded instance_type)
rg "var\.instance_type" --type tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

# Check actual usage of var.tags (not local tags)
rg "var\.tags" --type tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

# Check all references to admin_email variable
rg "var\.admin_email" --type tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

# Let me check all variable references comprehensively
rg "var\." --type tf -o | sort | uniq -c

Repository: coderabbitai/coderabbit-pr-review

Length of output: 140


🏁 Script executed:

# Check which variables are actually declared vs used
echo "=== Variables declared ===" && \
rg '^variable' --type tf -A 0 && \
echo -e "\n=== Actual var references ===" && \
rg 'var\.[a-z_]+' --type tf -o -h | sort | uniq

Repository: coderabbitai/coderabbit-pr-review

Length of output: 9722


🏁 Script executed:

# Simpler check - look for actual variable references more carefully
grep -r "var\." tflint/ --include="*.tf"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 481


🏁 Script executed:

# Check module declarations to see if these variables are passed to modules
cat tflint/modules.tf

Repository: coderabbitai/coderabbit-pr-review

Length of output: 734


Add explicit type declarations for variables actually used in the configuration.

environment and subnets are actively referenced in the configuration but lack explicit types. Additionally, instance_type, admin_email, and tags are declared but never referenced via var.* expressions—they should be reviewed for removal rather than typed.

The variable region already has type = string declared.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/variables.tf` around lines 1 - 32, The variables file is missing
explicit types for variables that are used and contains unused variables; add
type = "string" to the environment variable and add an explicit collection type
(e.g., type = list(string)) to subnets to match how they are referenced, and
then review and either remove or explicitly type the unused variables
instance_type, admin_email, and tags (or mark them as deprecated) so the config
is clear; update the declarations for environment, subnets, and adjust or remove
instance_type, admin_email, and tags accordingly, keeping unused_legacy_setting
as-is.

…undled rules and explicit tflint enable in .coderabbit.yml
@andrescastellanos-coderabbit
Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (4)
tflint/legacy.tf (1)

26-29: Remove unnecessary deprecated interpolation in locals.

Direct variable references don't require "${...}" wrapping since Terraform 0.12.

Suggested fix
 locals {
-  primary_region = "${var.region}"
-  environment    = "${var.environment}"
+  primary_region = var.region
+  environment    = var.environment
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 26 - 29, The locals block uses deprecated
string interpolation for variables; update the locals declaration by removing
the "${...}" wrappers and reference variables directly (change local names
primary_region and environment to use var.region and var.environment without
interpolation) so primary_region = var.region and environment = var.environment
inside the locals block.
tflint/outputs.tf (2)

1-11: Add descriptions to document outputs.

The terraform_documented_outputs rule requires descriptions. Adding descriptions improves maintainability and helps consumers understand what each output represents.

Suggested fix
 output "instance_id" {
+  description = "The ID of the primary EC2 instance."
   value = aws_instance.MyInstance.id
 }

 output "instance_public_ip" {
+  description = "The public IP address of the primary EC2 instance."
   value = aws_instance.MyInstance.public_ip
 }

 output "artifacts_bucket" {
+  description = "The name of the S3 bucket for build artifacts."
   value = aws_s3_bucket.ArtifactsBucket.bucket
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/outputs.tf` around lines 1 - 11, Each Terraform output block (output
"instance_id", output "instance_public_ip", output "artifacts_bucket") is
missing a description required by the terraform_documented_outputs rule; update
each output to include a descriptive description string that explains the value
(e.g., "ID of aws_instance.MyInstance", "Public IP of aws_instance.MyInstance",
"Name of aws_s3_bucket.ArtifactsBucket used for artifacts") so consumers
understand what aws_instance.MyInstance.id, aws_instance.MyInstance.public_ip
and aws_s3_bucket.ArtifactsBucket.bucket represent.

23-25: Add description and consider simplifying the splat expression.

The output lacks a description. Also, since aws_instance.WebServer2 has no count or for_each, the splat *.id returns a single-element list. Consider using .id directly or adding count/for_each if multiple instances are intended.

Suggested fix
 output "all_instance_ids" {
+  description = "List of IDs for all WebServer2 instances."
-  value = aws_instance.WebServer2.*.id
+  value = [aws_instance.WebServer2.id]
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/outputs.tf` around lines 23 - 25, The output "all_instance_ids" is
missing a description and uses a splat on aws_instance.WebServer2 that returns a
single-element list because the resource has no count/for_each; add a
descriptive description for the output and either change value =
aws_instance.WebServer2.*.id to value = aws_instance.WebServer2.id to return a
single id, or if multiple instances are intended, convert the
aws_instance.WebServer2 resource to use count/for_each and keep the splat to
return a list.
tflint/variables.tf (1)

34-37: Unused local value unused_local.

The terraform_unused_declarations rule will flag unused_local since it's never referenced. If this is intentional for demonstration purposes, consider adding a comment.

Suggested fix (if keeping for demo)
 locals {
   computed_name = "demoapp-${var.environment}"
+  # Intentionally unused - demonstrates terraform_unused_declarations rule
   unused_local  = "this local is never referenced anywhere"
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/variables.tf` around lines 34 - 37, The local block defines an unused
variable `unused_local` which triggers terraform_unused_declarations; either
remove `unused_local` from the `locals` block or keep it intentionally and add
an explanatory comment next to `unused_local` (or a module-level comment)
indicating it’s a demo placeholder so the linter can be satisfied; locate the
`locals` block containing `computed_name` and `unused_local` to apply the
change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tflint/legacy.tf`:
- Around line 1-5: The aws_eip resource aws_eip.Web uses deprecated
interpolation syntax for the instance attribute; replace the quoted
interpolation "${aws_instance.MyInstance.id}" with a direct reference by setting
instance = aws_instance.MyInstance.id (remove the surrounding quotes and ${}),
ensuring the instance attribute points directly to aws_instance.MyInstance.id.
- Around line 7-14: The resource aws_route53_record "Apex" uses an empty-list
equality and deprecated interpolation; change the count expression from
var.subnets == [] ? 0 : 1 to length(var.subnets) == 0 ? 0 : 1, and replace the
interpolated records = ["${aws_eip.Web.*.public_ip[0]}"] with a direct reference
such as records = [aws_eip.Web[0].public_ip] (adjust the index if you intend a
different instance).

In `@tflint/main.tf`:
- Around line 4-6: The AWS provider block currently hardcodes region =
"us-east-1"; replace that with the project variable by referencing the Terraform
input variable (e.g. variable "region") in the provider block (provider "aws")
so the provider reads the region from var.region instead of a fixed string;
update any related examples or documentation to show passing the region variable
if needed.

In `@tflint/modules.tf`:
- Around line 27-29: module "legacy_networking" references a non-existent local
path "../../shared-modules/networking" which will break terraform init; either
replace the source value in module "legacy_networking" with the correct remote
source (registry or git URL) used by other modules or add the missing
shared-modules/networking directory to the repo; update the module declaration
for "legacy_networking" accordingly so the source points to a valid location.
- Around line 1-9: The module block module "vpc" using source
"terraform-aws-modules/vpc/aws" must be pinned to a specific module version to
avoid pulling breaking changes; update the module block by adding a version
attribute with an appropriate constraint (for example a fixed version like
"x.y.z" or a constrained range like "~> x.y") to the module "vpc" declaration so
terraform init will use a known module release.

In `@tflint/variables.tf`:
- Around line 14-15: The variable declaration variable "admin_email" is defined
as required but not used; either remove the variable block for "admin_email" or
make it optional by adding a sensible default and a description (e.g., default =
"" and description = "admin contact email") and update any modules/resources
that should consume it; ensure no references remain to "admin_email" before
removing or, if intended for future use, include the default and description so
it is not required at plan/apply time.

---

Nitpick comments:
In `@tflint/legacy.tf`:
- Around line 26-29: The locals block uses deprecated string interpolation for
variables; update the locals declaration by removing the "${...}" wrappers and
reference variables directly (change local names primary_region and environment
to use var.region and var.environment without interpolation) so primary_region =
var.region and environment = var.environment inside the locals block.

In `@tflint/outputs.tf`:
- Around line 1-11: Each Terraform output block (output "instance_id", output
"instance_public_ip", output "artifacts_bucket") is missing a description
required by the terraform_documented_outputs rule; update each output to include
a descriptive description string that explains the value (e.g., "ID of
aws_instance.MyInstance", "Public IP of aws_instance.MyInstance", "Name of
aws_s3_bucket.ArtifactsBucket used for artifacts") so consumers understand what
aws_instance.MyInstance.id, aws_instance.MyInstance.public_ip and
aws_s3_bucket.ArtifactsBucket.bucket represent.
- Around line 23-25: The output "all_instance_ids" is missing a description and
uses a splat on aws_instance.WebServer2 that returns a single-element list
because the resource has no count/for_each; add a descriptive description for
the output and either change value = aws_instance.WebServer2.*.id to value =
aws_instance.WebServer2.id to return a single id, or if multiple instances are
intended, convert the aws_instance.WebServer2 resource to use count/for_each and
keep the splat to return a list.

In `@tflint/variables.tf`:
- Around line 34-37: The local block defines an unused variable `unused_local`
which triggers terraform_unused_declarations; either remove `unused_local` from
the `locals` block or keep it intentionally and add an explanatory comment next
to `unused_local` (or a module-level comment) indicating it’s a demo placeholder
so the linter can be satisfied; locate the `locals` block containing
`computed_name` and `unused_local` to apply the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: 7138f444-5c35-419a-a8c7-eb4f60224e8f

📥 Commits

Reviewing files that changed from the base of the PR and between ef04b8d and 101d037.

📒 Files selected for processing (7)
  • .coderabbit.yml
  • .tflint.hcl
  • tflint/legacy.tf
  • tflint/main.tf
  • tflint/modules.tf
  • tflint/outputs.tf
  • tflint/variables.tf

Comment thread tflint/legacy.tf
Comment on lines +1 to +5
resource "aws_eip" "Web" {
count = length(var.subnets) > 0 ? 1 : 0
instance = "${aws_instance.MyInstance.id}"
domain = "vpc"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove deprecated interpolation syntax.

Line 3 uses "${aws_instance.MyInstance.id}" which is deprecated since Terraform 0.12. Use direct references instead.

Suggested fix
 resource "aws_eip" "Web" {
   count    = length(var.subnets) > 0 ? 1 : 0
-  instance = "${aws_instance.MyInstance.id}"
+  instance = aws_instance.MyInstance.id
   domain   = "vpc"
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 1 - 5, The aws_eip resource aws_eip.Web uses
deprecated interpolation syntax for the instance attribute; replace the quoted
interpolation "${aws_instance.MyInstance.id}" with a direct reference by setting
instance = aws_instance.MyInstance.id (remove the surrounding quotes and ${}),
ensuring the instance attribute points directly to aws_instance.MyInstance.id.

Comment thread tflint/legacy.tf
Comment on lines +7 to +14
resource "aws_route53_record" "Apex" {
count = var.subnets == [] ? 0 : 1
zone_id = "Z123456ABCDEFG"
name = "demoapp.example.com"
type = "A"
ttl = 60
records = ["${aws_eip.Web.*.public_ip[0]}"]
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace empty list comparison with length() check.

Line 8 uses var.subnets == [] which triggers terraform_empty_list_equality. Use length(var.subnets) == 0 for consistency with Line 2. Also, Line 13 uses deprecated interpolation.

Suggested fix
 resource "aws_route53_record" "Apex" {
-  count   = var.subnets == [] ? 0 : 1
+  count   = length(var.subnets) > 0 ? 1 : 0
   zone_id = "Z123456ABCDEFG"
   name    = "demoapp.example.com"
   type    = "A"
   ttl     = 60
-  records = ["${aws_eip.Web.*.public_ip[0]}"]
+  records = [aws_eip.Web[0].public_ip]
 }
📝 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.

Suggested change
resource "aws_route53_record" "Apex" {
count = var.subnets == [] ? 0 : 1
zone_id = "Z123456ABCDEFG"
name = "demoapp.example.com"
type = "A"
ttl = 60
records = ["${aws_eip.Web.*.public_ip[0]}"]
}
resource "aws_route53_record" "Apex" {
count = length(var.subnets) > 0 ? 1 : 0
zone_id = "Z123456ABCDEFG"
name = "demoapp.example.com"
type = "A"
ttl = 60
records = [aws_eip.Web[0].public_ip]
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/legacy.tf` around lines 7 - 14, The resource aws_route53_record "Apex"
uses an empty-list equality and deprecated interpolation; change the count
expression from var.subnets == [] ? 0 : 1 to length(var.subnets) == 0 ? 0 : 1,
and replace the interpolated records = ["${aws_eip.Web.*.public_ip[0]}"] with a
direct reference such as records = [aws_eip.Web[0].public_ip] (adjust the index
if you intend a different instance).

Comment thread tflint/main.tf
Comment on lines +4 to +6
provider "aws" {
region = "us-east-1"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use the region variable instead of hardcoding.

A region variable is defined in variables.tf but not used here. This makes the configuration inflexible for multi-region deployments.

Suggested fix
 provider "aws" {
-  region = "us-east-1"
+  region = var.region
 }
📝 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.

Suggested change
provider "aws" {
region = "us-east-1"
}
provider "aws" {
region = var.region
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/main.tf` around lines 4 - 6, The AWS provider block currently
hardcodes region = "us-east-1"; replace that with the project variable by
referencing the Terraform input variable (e.g. variable "region") in the
provider block (provider "aws") so the provider reads the region from var.region
instead of a fixed string; update any related examples or documentation to show
passing the region variable if needed.

Comment thread tflint/modules.tf
Comment on lines +1 to +9
module "vpc" {
source = "terraform-aws-modules/vpc/aws"

name = "demoapp-vpc"
cidr = "10.0.0.0/16"

azs = ["us-east-1a", "us-east-1b"]
public_subnets = ["10.0.1.0/24", "10.0.2.0/24"]
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

What is the latest version of terraform-aws-modules/vpc/aws module?

💡 Result:

The latest version of the terraform-aws-modules/vpc/aws module is 6.6.1, released on April 2, 2026.

Citations:


Pin the VPC module to a specific version.

The terraform-aws-modules/vpc/aws module lacks a version constraint. Without pinning, terraform init may pull breaking changes from newer module versions.

Suggested fix
 module "vpc" {
   source = "terraform-aws-modules/vpc/aws"
+  version = "6.6.1"
 
   name = "demoapp-vpc"
📝 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.

Suggested change
module "vpc" {
source = "terraform-aws-modules/vpc/aws"
name = "demoapp-vpc"
cidr = "10.0.0.0/16"
azs = ["us-east-1a", "us-east-1b"]
public_subnets = ["10.0.1.0/24", "10.0.2.0/24"]
}
module "vpc" {
source = "terraform-aws-modules/vpc/aws"
version = "6.6.1"
name = "demoapp-vpc"
cidr = "10.0.0.0/16"
azs = ["us-east-1a", "us-east-1b"]
public_subnets = ["10.0.1.0/24", "10.0.2.0/24"]
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/modules.tf` around lines 1 - 9, The module block module "vpc" using
source "terraform-aws-modules/vpc/aws" must be pinned to a specific module
version to avoid pulling breaking changes; update the module block by adding a
version attribute with an appropriate constraint (for example a fixed version
like "x.y.z" or a constrained range like "~> x.y") to the module "vpc"
declaration so terraform init will use a known module release.

Comment thread tflint/modules.tf
Comment on lines +27 to +29
module "legacy_networking" {
source = "../../shared-modules/networking"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the referenced local module path exists
if [ -d "shared-modules/networking" ]; then
  echo "Local module path exists at repository root level"
  ls -la shared-modules/networking/
elif [ -d "../shared-modules/networking" ]; then
  echo "Local module path exists one level up"
else
  echo "WARNING: Local module path not found - this will cause terraform init to fail"
  # Search for any networking module in the repo
  fd -t d "networking" | head -5
fi

Repository: coderabbitai/coderabbit-pr-review

Length of output: 155


🏁 Script executed:

# Check directory structure around tflint/
ls -la tflint/ | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 480


🏁 Script executed:

# Search for all terraform module references in the codebase
rg "source\s*=" -A 1 --type tf | head -30

Repository: coderabbitai/coderabbit-pr-review

Length of output: 594


🏁 Script executed:

# Check if shared-modules exists anywhere in the repo
fd -t d "shared-modules"

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

# List all directories in repository root
ls -la / 2>/dev/null || echo "root not accessible"; cd / && ls -d */ 2>/dev/null | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 843


🏁 Script executed:

# Check the actual repository root structure
git ls-files | head -30

Repository: coderabbitai/coderabbit-pr-review

Length of output: 203


The relative path ../../shared-modules/networking does not exist and will cause terraform init to fail.

The referenced directory shared-modules/networking is not present in the repository at any level. All other module sources in this file reference remote registries or Git repositories, making this the only local module reference and a blocking issue.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/modules.tf` around lines 27 - 29, module "legacy_networking"
references a non-existent local path "../../shared-modules/networking" which
will break terraform init; either replace the source value in module
"legacy_networking" with the correct remote source (registry or git URL) used by
other modules or add the missing shared-modules/networking directory to the
repo; update the module declaration for "legacy_networking" accordingly so the
source points to a valid location.

Comment thread tflint/variables.tf
Comment on lines +14 to +15
variable "admin_email" {
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if admin_email is actually used anywhere
echo "=== Searching for admin_email references ==="
rg -n "var\.admin_email|admin_email" --type tf

echo -e "\n=== All variable references in tflint/ ==="
rg -o "var\.[a-z_]+" tflint/ | sort | uniq -c | sort -rn

Repository: coderabbitai/coderabbit-pr-review

Length of output: 372


Remove unused required variable admin_email.

The variable is declared with no default (making it required) but is never referenced anywhere in the configuration. Remove it if unnecessary, or add a default value and description if it's intended for future use.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tflint/variables.tf` around lines 14 - 15, The variable declaration variable
"admin_email" is defined as required but not used; either remove the variable
block for "admin_email" or make it optional by adding a sensible default and a
description (e.g., default = "" and description = "admin contact email") and
update any modules/resources that should consume it; ensure no references remain
to "admin_email" before removing or, if intended for future use, include the
default and description so it is not required at plan/apply time.

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.

1 participant