-
Notifications
You must be signed in to change notification settings - Fork 76
feat: add aws_security_group_inline_rules rule #793
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
Changes from 5 commits
bdef144
ee1be9d
60404ab
330ea4f
89e39a0
b074a48
3384269
56a023e
58a22de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
# 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. | ||
kayman-mk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## 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. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur. (aws_security_group_egress_and_ingress_blocks_deprecated) | ||
|
||
on test.tf line 4: | ||
4: egress { | ||
|
||
Notice: Replace this ingress block with aws_vpc_security_group_ingress_rule. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur. (aws_security_group_egress_and_ingress_blocks_deprecated) | ||
|
||
on test.tf line 11: | ||
11: ingress { | ||
|
||
``` | ||
|
||
## 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. | ||
kayman-mk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These sentences appear to be copied from official references, but they feel a bit odd to include in this section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to be a duplication. Paragraph reworked. |
||
|
||
## 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_ingress_rule` in the same way. |
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" { | ||
kayman-mk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
runner.EmitIssue( | ||
r, | ||
fmt.Sprintf("Replace this %s block with aws_vpc_security_group_%s_rule. Otherwise, rule conflicts, perpetual differences, and result in rules being overwritten may occur.", block.Type, block.Type), | ||
kayman-mk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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_egress_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) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.