Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add aws_security_group_inline_rules rule #793

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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||

Expand Down
1 change: 1 addition & 0 deletions docs/rules/README.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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||

Expand Down
65 changes: 65 additions & 0 deletions docs/rules/aws_security_group_inline_rules.md
Original file line number Diff line number Diff line change
@@ -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.
67 changes: 67 additions & 0 deletions rules/aws_security_group_inline_rules.go
Original file line number Diff line number Diff line change
@@ -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
}
90 changes: 90 additions & 0 deletions rules/aws_security_group_inline_rules_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
1 change: 1 addition & 0 deletions rules/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var manualRules = []tflint.Rule{
NewAwsSecurityGroupInvalidProtocolRule(),
NewAwsSecurityGroupRuleInvalidProtocolRule(),
NewAwsProviderMissingDefaultTagsRule(),
NewAwsSecurityGroupInlineRulesRule(),
NewAwsSecurityGroupRuleDeprecatedRule(),
}

Expand Down
Loading