Skip to content

Commit

Permalink
add rule AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule
Browse files Browse the repository at this point in the history
  • Loading branch information
kayman-mk committed Dec 16, 2024
1 parent 158193b commit bdef144
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# aws_security_group_egress_and_ingress_blocks_deprecated

Avoid using the `ingress` and `egress` arguments of the `aws_security_group` resource to configure in-line rules, as they have difficulties managing multiple CIDR blocks and lack unique IDs, tags, and descriptions. To prevent these issues, follow the current best practice of using the `aws_vpc_security_group_egress_rule` and `aws_vpc_security_group_ingress_rule` resources, with one CIDR block per rule.

## 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
// TODO: Write the output when inspects the above code
```

## Why

Refrain from using the `ingress` and `egress` arguments of the `aws_security_group` resource for in-line rules, as they have difficulties managing multiple CIDR blocks and historically lack unique IDs, tags, and descriptions. To prevent these issues, follow the best practice of using the `aws_vpc_security_group_egress_rule` and `aws_vpc_security_group_ingress_rule` resources, with one CIDR block per rule.

Avoid using the `aws_security_group` resource with in-line rules (using the ingress and egress arguments) alongside the `aws_vpc_security_group_egress_rule`, `aws_vpc_security_group_ingress_rule`, or `aws_security_group_rule` resources. This practice can lead to rule conflicts, perpetual differences, and rules being overwritten.

## 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.

`ingress` blocks are replaced by `aws_vpc_security_group_egress_rule` in the same way.
71 changes: 71 additions & 0 deletions rules/aws_security_group_egress_and_ingress_blocks_deprecated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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"
)

// AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule checks that egress and ingress blocks are not used in aws_security_group
type AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule struct {
tflint.DefaultRule

resourceType string
}

// NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule returns new rule with default attributes
func NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule() *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule {
return &AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule{
resourceType: "aws_security_group",
}
}

// Name returns the rule name
func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Name() string {
return "aws_security_group_egress_and_ingress_blocks_deprecated"
}

// Enabled returns whether the rule is enabled by default
func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Enabled() bool {
return false
}

// Severity returns the rule severity
func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Severity() tflint.Severity {
return tflint.WARNING
}

// Link returns the rule reference link
func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Link() string {
return project.ReferenceLink(r.Name())
}

// Check that no egress and ingress blocks are used in a aws_security_group
func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) 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 {
if block.Type == "egress" || block.Type == "ingress" {
runner.EmitIssue(
r,
fmt.Sprintf("Replace this %s block with aws_vpc_security_group_ingress_rule. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur.", block.Type),
block.DefRange,
)
} else {
return fmt.Errorf("unexpected resource type: %s", block.Type)
}
}
}

return nil
}
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_AwsSecurityGroupEgressAndIngressBlocksDeprecated(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: NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule(),
Message: "Replace this ingress block with aws_vpc_security_group_ingress_rule. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur.",
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: NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule(),
Message: "Replace this egress block with aws_vpc_security_group_ingress_rule. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur.",
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 := NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule()

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 @@ -40,6 +40,7 @@ var manualRules = []tflint.Rule{
NewAwsSecurityGroupInvalidProtocolRule(),
NewAwsSecurityGroupRuleInvalidProtocolRule(),
NewAwsProviderMissingDefaultTagsRule(),
NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule(),
}

// Rules is a list of all rules
Expand Down

0 comments on commit bdef144

Please sign in to comment.