From bdef1448c69c98c3f19fff2ff57f09063af6aec3 Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Mon, 16 Dec 2024 10:41:28 +0100 Subject: [PATCH 1/8] add rule AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule --- ...up_egress_and_ingress_blocks_deprecated.md | 57 ++++++++++++ ...up_egress_and_ingress_blocks_deprecated.go | 71 +++++++++++++++ ...ress_and_ingress_blocks_deprecated_test.go | 90 +++++++++++++++++++ rules/provider.go | 1 + 4 files changed, 219 insertions(+) create mode 100644 docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md create mode 100644 rules/aws_security_group_egress_and_ingress_blocks_deprecated.go create mode 100644 rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go diff --git a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md new file mode 100644 index 00000000..da03facf --- /dev/null +++ b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md @@ -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. diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go new file mode 100644 index 00000000..09963521 --- /dev/null +++ b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go @@ -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 +} diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go new file mode 100644 index 00000000..c1a90ff1 --- /dev/null +++ b/rules/aws_security_group_egress_and_ingress_blocks_deprecated_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_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) + } +} diff --git a/rules/provider.go b/rules/provider.go index 4d450b64..4318c300 100644 --- a/rules/provider.go +++ b/rules/provider.go @@ -40,6 +40,7 @@ var manualRules = []tflint.Rule{ NewAwsSecurityGroupInvalidProtocolRule(), NewAwsSecurityGroupRuleInvalidProtocolRule(), NewAwsProviderMissingDefaultTagsRule(), + NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule(), } // Rules is a list of all rules From ee1be9d6734f7f9139e1d5b42774240e662791aa Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Mon, 16 Dec 2024 10:44:59 +0100 Subject: [PATCH 2/8] Update README.md --- docs/rules/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rules/README.md b/docs/rules/README.md index af6892dd..8a93c2d2 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -74,6 +74,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_egress_and_ingress_blocks_deprecated](aws_security_group_egress_and_ingress_blocks_deprecated.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags|| ### SDK-based Validations From 60404ab20f0b13344e43041dbf5e4bbabfff93ad Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Mon, 16 Dec 2024 10:45:42 +0100 Subject: [PATCH 3/8] Update README.md.tmpl --- docs/rules/README.md.tmpl | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rules/README.md.tmpl b/docs/rules/README.md.tmpl index f197f6fe..797c9020 100644 --- a/docs/rules/README.md.tmpl +++ b/docs/rules/README.md.tmpl @@ -74,6 +74,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_egress_and_ingress_blocks_deprecated](aws_security_group_egress_and_ingress_blocks_deprecated.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags|| ### SDK-based Validations From 330ea4f89e50fe60f9a75064d59898aebfefb2d8 Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Mon, 16 Dec 2024 10:51:15 +0100 Subject: [PATCH 4/8] update docs --- ...rity_group_egress_and_ingress_blocks_deprecated.md | 11 ++++++++++- ...rity_group_egress_and_ingress_blocks_deprecated.go | 2 +- ...group_egress_and_ingress_blocks_deprecated_test.go | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md index da03facf..addfc762 100644 --- a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md +++ b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md @@ -26,9 +26,18 @@ resource "aws_security_group" "foo" { ``` $ tflint +2 issue(s) found: -// TODO: Write the output when inspects the above code +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 diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go index 09963521..05a53716 100644 --- a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go +++ b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go @@ -58,7 +58,7 @@ func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Check(runner tfli 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), + 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), block.DefRange, ) } else { diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go index c1a90ff1..b327d0eb 100644 --- a/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go +++ b/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go @@ -56,7 +56,7 @@ resource "aws_security_group" "test" { 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.", + 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}, From 89e39a0f8f38605afa97cb3457fb5f1716538fcc Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Mon, 16 Dec 2024 10:55:28 +0100 Subject: [PATCH 5/8] Update aws_security_group_egress_and_ingress_blocks_deprecated.md --- .../aws_security_group_egress_and_ingress_blocks_deprecated.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md index addfc762..9ecad46b 100644 --- a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md +++ b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md @@ -63,4 +63,4 @@ resource "aws_vpc_security_group_egress_rule" "example" { using the attributes according to your code. -`ingress` blocks are replaced by `aws_vpc_security_group_egress_rule` in the same way. +`ingress` blocks are replaced by `aws_vpc_security_group_ingress_rule` in the same way. From b074a48c1225b7498a08bb7e8a8857f43d0bd8a3 Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Fri, 10 Jan 2025 09:51:45 +0100 Subject: [PATCH 6/8] Update rules/aws_security_group_egress_and_ingress_blocks_deprecated.go Co-authored-by: Kazuma Watanabe --- .../aws_security_group_egress_and_ingress_blocks_deprecated.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go index 05a53716..9bb777f9 100644 --- a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go +++ b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go @@ -58,7 +58,7 @@ func (r *AwsSecurityGroupEgressAndIngressBlocksDeprecatedRule) Check(runner tfli if block.Type == "egress" || block.Type == "ingress" { 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), + fmt.Sprintf("Replace this %s block with aws_vpc_security_group_%s_rule", block.Type, block.Type), block.DefRange, ) } else { From 3384269527a680f9e9860e09519e4a0eb99c2a2d Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Fri, 10 Jan 2025 12:02:58 +0100 Subject: [PATCH 7/8] address review comments --- docs/rules/README.md | 2 +- docs/rules/README.md.tmpl | 2 +- ...up_egress_and_ingress_blocks_deprecated.md | 66 ----------------- docs/rules/aws_security_group_inline_rules.md | 66 +++++++++++++++++ ...up_egress_and_ingress_blocks_deprecated.go | 71 ------------------- rules/aws_security_group_inline_rules.go | 67 +++++++++++++++++ ...> aws_security_group_inline_rules_test.go} | 12 ++-- rules/provider.go | 2 +- 8 files changed, 142 insertions(+), 146 deletions(-) delete mode 100644 docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md create mode 100644 docs/rules/aws_security_group_inline_rules.md delete mode 100644 rules/aws_security_group_egress_and_ingress_blocks_deprecated.go create mode 100644 rules/aws_security_group_inline_rules.go rename rules/{aws_security_group_egress_and_ingress_blocks_deprecated_test.go => aws_security_group_inline_rules_test.go} (75%) diff --git a/docs/rules/README.md b/docs/rules/README.md index 8a93c2d2..2f1e3c0d 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -74,7 +74,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_egress_and_ingress_blocks_deprecated](aws_security_group_egress_and_ingress_blocks_deprecated.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| +|[aws_security_group_inline_rules](aws_security_group_inline_rules.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags|| ### SDK-based Validations diff --git a/docs/rules/README.md.tmpl b/docs/rules/README.md.tmpl index 797c9020..2e93d528 100644 --- a/docs/rules/README.md.tmpl +++ b/docs/rules/README.md.tmpl @@ -74,7 +74,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_egress_and_ingress_blocks_deprecated](aws_security_group_egress_and_ingress_blocks_deprecated.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| +|[aws_security_group_inline_rules](aws_security_group_inline_rules.md)|Disallow `ingress` and `egress` arguments of the `aws_security_group` resource|| |[aws_provider_missing_default_tags](aws_provider_missing_default_tags.md)|Require specific tags for all AWS providers default tags|| ### SDK-based Validations diff --git a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md b/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md deleted file mode 100644 index 9ecad46b..00000000 --- a/docs/rules/aws_security_group_egress_and_ingress_blocks_deprecated.md +++ /dev/null @@ -1,66 +0,0 @@ -# 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 -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. - -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_ingress_rule` in the same way. 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..8c9e515f --- /dev/null +++ b/docs/rules/aws_security_group_inline_rules.md @@ -0,0 +1,66 @@ +# 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 + +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. 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. + +`ingress` blocks are replaced by `aws_vpc_security_group_ingress_rule` in the same way. diff --git a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go b/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go deleted file mode 100644 index 9bb777f9..00000000 --- a/rules/aws_security_group_egress_and_ingress_blocks_deprecated.go +++ /dev/null @@ -1,71 +0,0 @@ -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_%s_rule", block.Type, block.Type), - block.DefRange, - ) - } else { - return fmt.Errorf("unexpected resource type: %s", block.Type) - } - } - } - - return nil -} 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_egress_and_ingress_blocks_deprecated_test.go b/rules/aws_security_group_inline_rules_test.go similarity index 75% rename from rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go rename to rules/aws_security_group_inline_rules_test.go index b327d0eb..cd287b1a 100644 --- a/rules/aws_security_group_egress_and_ingress_blocks_deprecated_test.go +++ b/rules/aws_security_group_inline_rules_test.go @@ -7,7 +7,7 @@ import ( "github.com/terraform-linters/tflint-plugin-sdk/helper" ) -func Test_AwsSecurityGroupEgressAndIngressBlocksDeprecated(t *testing.T) { +func Test_AwsSecurityGroupInlineRulesRule(t *testing.T) { cases := []struct { Name string Content string @@ -29,8 +29,8 @@ resource "aws_security_group" "test" { `, 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.", + 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}, @@ -55,8 +55,8 @@ resource "aws_security_group" "test" { `, 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.", + 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}, @@ -76,7 +76,7 @@ resource "aws_security_group" "test" { }, } - rule := NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule() + rule := NewAwsSecurityGroupInlineRulesRule() for _, tc := range cases { runner := helper.TestRunner(t, map[string]string{"resource.tf": tc.Content}) diff --git a/rules/provider.go b/rules/provider.go index 4318c300..c36fbc92 100644 --- a/rules/provider.go +++ b/rules/provider.go @@ -40,7 +40,7 @@ var manualRules = []tflint.Rule{ NewAwsSecurityGroupInvalidProtocolRule(), NewAwsSecurityGroupRuleInvalidProtocolRule(), NewAwsProviderMissingDefaultTagsRule(), - NewAwsSecurityGroupEgressAndIngressBlocksDeprecatedRule(), + NewAwsSecurityGroupInlineRulesRule(), } // Rules is a list of all rules From 58a22de203ec0ed7a8aed5146b07bea349e986c2 Mon Sep 17 00:00:00 2001 From: Matthias Kay Date: Fri, 10 Jan 2025 12:20:45 +0100 Subject: [PATCH 8/8] docs reworked --- docs/rules/aws_security_group_inline_rules.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/rules/aws_security_group_inline_rules.md b/docs/rules/aws_security_group_inline_rules.md index 8c9e515f..ba79fed1 100644 --- a/docs/rules/aws_security_group_inline_rules.md +++ b/docs/rules/aws_security_group_inline_rules.md @@ -43,8 +43,9 @@ Notice: Replace this ingress block with aws_vpc_security_group_ingress_rule. (aw ## 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. See [best practices](https://registry.terraform.io/providers/hashicorp/aws/5.82.2/docs/resources/security_group). +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 @@ -61,6 +62,4 @@ resource "aws_vpc_security_group_egress_rule" "example" { } ``` -using the attributes according to your code. - -`ingress` blocks are replaced by `aws_vpc_security_group_ingress_rule` in the same way. +using the attributes according to your code. For `ingress` blocks use `aws_vpc_security_group_ingress_rule` in the same way.