diff --git a/docs/rules/README.md b/docs/rules/README.md index d474d270..637d6318 100644 --- a/docs/rules/README.md +++ b/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|| diff --git a/docs/rules/README.md.tmpl b/docs/rules/README.md.tmpl index 8f4e7347..d1e5d565 100644 --- a/docs/rules/README.md.tmpl +++ b/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|| diff --git a/docs/rules/aws_security_group_inline_rules.md b/docs/rules/aws_security_group_inline_rules.md new file mode 100644 index 00000000..ba79fed1 --- /dev/null +++ b/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. diff --git a/rules/aws_security_group_inline_rules.go b/rules/aws_security_group_inline_rules.go new file mode 100644 index 00000000..31df5d9a --- /dev/null +++ b/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 +} diff --git a/rules/aws_security_group_inline_rules_test.go b/rules/aws_security_group_inline_rules_test.go new file mode 100644 index 00000000..cd287b1a --- /dev/null +++ b/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) + } +} diff --git a/rules/provider.go b/rules/provider.go index 707a2636..3b4dff65 100644 --- a/rules/provider.go +++ b/rules/provider.go @@ -41,6 +41,7 @@ var manualRules = []tflint.Rule{ NewAwsSecurityGroupInvalidProtocolRule(), NewAwsSecurityGroupRuleInvalidProtocolRule(), NewAwsProviderMissingDefaultTagsRule(), + NewAwsSecurityGroupInlineRulesRule(), NewAwsSecurityGroupRuleDeprecatedRule(), }