feat: add aws_security_group_inline_rules rule (#793)

* add rule AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule

* Update README.md

* Update README.md.tmpl

* update docs

* Update aws_security_group_egress_and_ingress_blocks_deprecated.md

* Update rules/aws_security_group_egress_and_ingress_blocks_deprecated.go

Co-authored-by: Kazuma Watanabe <[email protected]>

* address review comments

* docs reworked

---------

Co-authored-by: Kazuma Watanabe <[email protected]>

Author: kayman-mk

docs/rules/README.md

@@ -75,6 +75,7 @@ These rules enforce best practices and naming conventions:
 |[aws_lambda_function_deprecated_runtime](aws_lambda_function_deprecated_runtime.md)|Disallow deprecated runtimes for Lambda Function|✔|
 |[aws_resource_missing_tags](aws_resource_missing_tags.md)|Require specific tags for all AWS resource types that support them||
 |[aws_s3_bucket_name](aws_s3_bucket_name.md)|Ensures all S3 bucket names match the naming rules|✔|
+|[aws_security_group_inline_rules](aws_security_group_inline_rules.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource||
 |[aws_security_group_rule_deprecated](aws_security_group_rule_deprecated.md)|Disallow using `aws_security_group_rule` resource||
 |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags||
 

docs/rules/README.md.tmpl

@@ -75,6 +75,7 @@ These rules enforce best practices and naming conventions:
 |[aws_lambda_function_deprecated_runtime](aws_lambda_function_deprecated_runtime.md)|Disallow deprecated runtimes for Lambda Function|✔|
 |[aws_resource_missing_tags](aws_resource_missing_tags.md)|Require specific tags for all AWS resource types that support them||
 |[aws_s3_bucket_name](aws_s3_bucket_name.md)|Ensures all S3 bucket names match the naming rules|✔|
+|[aws_security_group_inline_rules](aws_security_group_inline_rules.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource||
 |[aws_security_group_rule_deprecated](aws_security_group_rule_deprecated.md)|Disallow using `aws_security_group_rule` resource||
 |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags||
 

docs/rules/aws_security_group_inline_rules.md

@@ -0,0 +1,65 @@
+# aws_security_group_inline_rules
+
+Disallow `ingress` and `egress` arguments of the `aws_security_group` resource.
+
+
+## Example
+
+```hcl
+resource "aws_security_group" "foo" {
+  name = "test"
+
+  egress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  ingress {
+    from_port   = 443
+    to_port     = 443
+    protocol    = "tcp"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+}
+```
+
+```
+$ tflint
+2 issue(s) found:
+
+Notice: Replace this egress block with aws_vpc_security_group_egress_rule. (aws_security_group_inline_rules)
+
+  on test.tf line 4:
+   4:   egress {
+   
+Notice: Replace this ingress block with aws_vpc_security_group_ingress_rule. (aws_security_group_inline_rules)
+
+  on test.tf line 11:
+   11:   ingress {
+   
+```
+
+## Why
+
+In-line rules are difficult to manage and maintain, especially when multiple CIDR blocks are used. They lack unique IDs, tags, and descriptions, which makes it hard to identify and manage them.
+
+See [best practices](https://registry.terraform.io/providers/hashicorp/aws/5.82.2/docs/resources/security_group).
+
+## How To Fix
+
+Replace an `egress` block by
+
+```hcl
+resource "aws_vpc_security_group_egress_rule" "example" {
+  security_group_id = aws_security_group.example.id
+
+  cidr_ipv4   = "0.0.0.0/0"
+  from_port   = 443
+  ip_protocol = "tcp"
+  to_port     = 443
+}
+```
+
+using the attributes according to your code. For `ingress` blocks use `aws_vpc_security_group_ingress_rule` in the same way.

rules/aws_security_group_inline_rules.go

@@ -0,0 +1,67 @@
+package rules
+
+import (
+	"fmt"
+	"github.com/terraform-linters/tflint-plugin-sdk/hclext"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	"github.com/terraform-linters/tflint-ruleset-aws/project"
+)
+
+// AwsSecurityGroupInlineRulesRule checks that egress and ingress blocks are not used in aws_security_group
+type AwsSecurityGroupInlineRulesRule struct {
+	tflint.DefaultRule
+
+	resourceType string
+}
+
+// NewAwsSecurityGroupInlineRulesRule returns new rule with default attributes
+func NewAwsSecurityGroupInlineRulesRule() *AwsSecurityGroupInlineRulesRule {
+	return &AwsSecurityGroupInlineRulesRule{
+		resourceType: "aws_security_group",
+	}
+}
+
+// Name returns the rule name
+func (r *AwsSecurityGroupInlineRulesRule) Name() string {
+	return "aws_security_group_inline_rules"
+}
+
+// Enabled returns whether the rule is enabled by default
+func (r *AwsSecurityGroupInlineRulesRule) Enabled() bool {
+	return false
+}
+
+// Severity returns the rule severity
+func (r *AwsSecurityGroupInlineRulesRule) Severity() tflint.Severity {
+	return tflint.WARNING
+}
+
+// Link returns the rule reference link
+func (r *AwsSecurityGroupInlineRulesRule) Link() string {
+	return project.ReferenceLink(r.Name())
+}
+
+// Check that no egress and ingress blocks are used in a aws_security_group
+func (r *AwsSecurityGroupInlineRulesRule) Check(runner tflint.Runner) error {
+	resources, err := runner.GetResourceContent(r.resourceType, &hclext.BodySchema{
+		Blocks: []hclext.BlockSchema{
+			{Type: "egress"},
+			{Type: "ingress"},
+		},
+	}, nil)
+	if err != nil {
+		return err
+	}
+
+	for _, resource := range resources.Blocks {
+		for _, block := range resource.Body.Blocks {
+			runner.EmitIssue(
+				r,
+				fmt.Sprintf("Replace this %s block with aws_vpc_security_group_%s_rule.", block.Type, block.Type),
+				block.DefRange,
+			)
+		}
+	}
+
+	return nil
+}

rules/aws_security_group_inline_rules_test.go

@@ -0,0 +1,90 @@
+package rules
+
+import (
+	"testing"
+
+	hcl "github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_AwsSecurityGroupInlineRulesRule(t *testing.T) {
+	cases := []struct {
+		Name     string
+		Content  string
+		Expected helper.Issues
+	}{
+		{
+			Name: "finds deprecated ingress block",
+			Content: `
+resource "aws_security_group" "test" {
+  name = "test"
+
+  ingress {
+	from_port   = 0
+	to_port     = 0
+	protocol    = "-1"
+	cidr_blocks = ["0.0.0.0/0"]
+  }
+}
+`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsSecurityGroupInlineRulesRule(),
+					Message: "Replace this ingress block with aws_vpc_security_group_ingress_rule.",
+					Range: hcl.Range{
+						Filename: "resource.tf",
+						Start:    hcl.Pos{Line: 5, Column: 3},
+						End:      hcl.Pos{Line: 5, Column: 10},
+					},
+				},
+			},
+		},
+		{
+			Name: "finds deprecated egress block",
+			Content: `
+resource "aws_security_group" "test" {
+  name = "test"
+
+  egress {
+	from_port   = 0
+	to_port     = 0
+	protocol    = "-1"
+	cidr_blocks = ["0.0.0.0/0"]
+  }
+}
+`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsSecurityGroupInlineRulesRule(),
+					Message: "Replace this egress block with aws_vpc_security_group_egress_rule.",
+					Range: hcl.Range{
+						Filename: "resource.tf",
+						Start:    hcl.Pos{Line: 5, Column: 3},
+						End:      hcl.Pos{Line: 5, Column: 9},
+					},
+				},
+			},
+		},
+		{
+			Name: "everything is fine",
+			Content: `
+resource "aws_security_group" "test" {
+  name = "test"
+}
+`,
+			Expected: helper.Issues{},
+		},
+	}
+
+	rule := NewAwsSecurityGroupInlineRulesRule()
+
+	for _, tc := range cases {
+		runner := helper.TestRunner(t, map[string]string{"resource.tf": tc.Content})
+
+		if err := rule.Check(runner); err != nil {
+			t.Fatalf("Unexpected error occurred: %s", err)
+		}
+
+		helper.AssertIssues(t, tc.Expected, runner.Issues)
+	}
+}

rules/provider.go

@@ -41,6 +41,7 @@ var manualRules = []tflint.Rule{
 	NewAwsSecurityGroupInvalidProtocolRule(),
 	NewAwsSecurityGroupRuleInvalidProtocolRule(),
 	NewAwsProviderMissingDefaultTagsRule(),
+	NewAwsSecurityGroupInlineRulesRule(),
 	NewAwsSecurityGroupRuleDeprecatedRule(),
 }