Skip to content

Commit

Permalink
feat: add aws_security_group_rule_deprecated rule (#790)
Browse files Browse the repository at this point in the history
* generate rule

* finish coding

* finish docs

* amend docs

* add rule name to output

* Update docs/rules/aws_security_group_rule_deprecated.md

Co-authored-by: Ben Drucker <bvdrucker@gmail.com>

* fix review issues

* get the resource only

* Update README.md

* Update README.md

* Update README.md.tmpl

* Update README.md

---------

Co-authored-by: Ben Drucker <bvdrucker@gmail.com>
  • Loading branch information
kayman-mk and bendrucker authored Dec 30, 2024
1 parent 1538e44 commit ed4ff6e
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 0 deletions.
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_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||

### SDK-based Validations
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_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||

### SDK-based Validations
Expand Down
30 changes: 30 additions & 0 deletions docs/rules/aws_security_group_rule_deprecated.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# aws_security_group_rule_deprecated

The `aws_security_group_rule` resource should be replaced with `aws_vpc_security_group_egress_rule` or `aws_vpc_security_group_ingress_rule`. It lacks support of unique IDs, tags, and descriptions, and has difficulties managing multiple CIDR blocks.

## Example

```hcl
resource "aws_security_group_rule" "foo" {
security_group_id = "sg-12345678"
type = "ingress"
}
```

```sh
❯ tflint
1 issue(s) found:

Warning: Consider using aws_vpc_security_group_egress_rule or aws_vpc_security_group_ingress_rule instead. (aws_security_group_rule_deprecated)

on bastion.tf line 4:
4: resource "aws_security_group_rule" "foo" {
```
## Why
Avoid using the [`aws_security_group_rule`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) resource because it has difficulties managing multiple CIDR blocks and historically lacks 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.
## How To Fix
Depending on `type`, you can fix the issue by using either `aws_vpc_security_group_egress_rule` or `aws_vpc_security_group_ingress_rule`.
61 changes: 61 additions & 0 deletions rules/aws_security_group_rule_deprecated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package rules

import (
"github.com/terraform-linters/tflint-plugin-sdk/hclext"
"github.com/terraform-linters/tflint-plugin-sdk/tflint"
"github.com/terraform-linters/tflint-ruleset-aws/project"
)

// AwsSecurityGroupRuleDeprecatedRule checks that aws_security_group_rule is not used
type AwsSecurityGroupRuleDeprecatedRule struct {
tflint.DefaultRule

resourceType string
attributeName string
}

// NewAwsSecurityGroupRuleDeprecatedRule returns new rule with default attributes
func NewAwsSecurityGroupRuleDeprecatedRule() *AwsSecurityGroupRuleDeprecatedRule {
return &AwsSecurityGroupRuleDeprecatedRule{
resourceType: "aws_security_group_rule",
attributeName: "security_group_id",
}
}

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

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

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

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

// Check that aws_security_group_rule resource is not used
func (r *AwsSecurityGroupRuleDeprecatedRule) Check(runner tflint.Runner) error {
resources, err := runner.GetResourceContent(r.resourceType, &hclext.BodySchema{}, nil)
if err != nil {
return err
}

for _, resource := range resources.Blocks {
runner.EmitIssue(
r,
"Consider using aws_vpc_security_group_egress_rule or aws_vpc_security_group_ingress_rule instead.",
resource.DefRange,
)
}

return nil
}
57 changes: 57 additions & 0 deletions rules/aws_security_group_rule_deprecated_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package rules

import (
"testing"

hcl "github.com/hashicorp/hcl/v2"
"github.com/terraform-linters/tflint-plugin-sdk/helper"
)

func Test_AwsSecurityGroupRuleDeprecated(t *testing.T) {
cases := []struct {
Name string
Content string
Expected helper.Issues
}{
{
Name: "resource is used",
Content: `
resource "aws_security_group_rule" "test" {
security_group_id = "sg-12345678"
}
`,
Expected: helper.Issues{
{
Rule: NewAwsSecurityGroupRuleDeprecatedRule(),
Message: "Consider using aws_vpc_security_group_egress_rule or aws_vpc_security_group_ingress_rule instead.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 2, Column: 1},
End: hcl.Pos{Line: 2, Column: 42},
},
},
},
},
{
Name: "everything is fine",
Content: `
resource "aws_security_group" "test" {
name = "test"
}
`,
Expected: helper.Issues{},
},
}

rule := NewAwsSecurityGroupRuleDeprecatedRule()

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(),
NewAwsSecurityGroupRuleDeprecatedRule(),
}

// Rules is a list of all rules
Expand Down

0 comments on commit ed4ff6e

Please sign in to comment.