From 6986646835fb08e64fe3454f56be543d6348c8af Mon Sep 17 00:00:00 2001 From: x-color <36035885+x-color@users.noreply.github.com> Date: Thu, 14 Jul 2022 23:36:28 +0900 Subject: [PATCH] Add aws_security_group_invalid_protocol rule (#356) * Add aws_security_group_invalid_protocol rule * Fix allowed protocols * Update README.md.tmpl * Update README.md Co-authored-by: Kazuma Watanabe --- docs/rules/README.md | 1 + docs/rules/README.md.tmpl | 1 + .../aws_security_group_invalid_protocol.md | 35 ++++++ rules/aws_security_group_invalid_protocol.go | 111 +++++++++++++++++ ...ws_security_group_invalid_protocol_test.go | 117 ++++++++++++++++++ rules/provider.go | 1 + 6 files changed, 266 insertions(+) create mode 100644 docs/rules/aws_security_group_invalid_protocol.md create mode 100644 rules/aws_security_group_invalid_protocol.go create mode 100644 rules/aws_security_group_invalid_protocol_test.go diff --git a/docs/rules/README.md b/docs/rules/README.md index 1b583ea2..b3508f28 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -51,6 +51,7 @@ These rules warn of possible errors that can occur at `terraform apply`. Rules m |aws_s3_bucket_invalid_acl|Disallow invalid ACL rule for S3 bucket||✔| |aws_s3_bucket_invalid_region|Disallow invalid region for S3 bucket||✔| |aws_spot_fleet_request_invalid_excess_capacity_termination_policy|Disallow invalid excess capacity termination policy||✔| +|[aws_security_group_invalid_protocol](aws_security_group_invalid_protocol.md)|Disallow using invalid protocol||✔| |[aws_security_group_rule_invalid_protocol](aws_security_group_rule_invalid_protocol.md)|Disallow using invalid protocol||✔| ### Best Practices/Naming Conventions diff --git a/docs/rules/README.md.tmpl b/docs/rules/README.md.tmpl index 3f560030..0f9fa3fc 100644 --- a/docs/rules/README.md.tmpl +++ b/docs/rules/README.md.tmpl @@ -51,6 +51,7 @@ These rules warn of possible errors that can occur at `terraform apply`. Rules m |aws_s3_bucket_invalid_acl|Disallow invalid ACL rule for S3 bucket||✔| |aws_s3_bucket_invalid_region|Disallow invalid region for S3 bucket||✔| |aws_spot_fleet_request_invalid_excess_capacity_termination_policy|Disallow invalid excess capacity termination policy||✔| +|[aws_security_group_invalid_protocol](aws_security_group_invalid_protocol.md)|Disallow using invalid protocol||✔| |[aws_security_group_rule_invalid_protocol](aws_security_group_rule_invalid_protocol.md)|Disallow using invalid protocol||✔| ### Best Practices/Naming Conventions diff --git a/docs/rules/aws_security_group_invalid_protocol.md b/docs/rules/aws_security_group_invalid_protocol.md new file mode 100644 index 00000000..b64ff692 --- /dev/null +++ b/docs/rules/aws_security_group_invalid_protocol.md @@ -0,0 +1,35 @@ +# aws_security_group_invalid_protocol + +Disallow using invalid protocol. + +## Example + +```hcl +resource "aws_security_group" "sample" { + description = "test security group" + + ingress { + from_port = 443 + to_port = 443 + protocol = "https" + } +} +``` + +``` +$ tflint +1 issue(s) found: + +Error: "https" is an invalid protocol. It allows "tcp", "udp" or "-1"(all). (aws_security_group_invalid_protocol) + + on terraform.tf line 7: + 7: protocol = "https" +``` + +## Why + +Apply will fail. (Plan will succeed with the invalid value though) + +## How To Fix + +Select valid protocol according to the [document](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_SecurityGroupRule.html) diff --git a/rules/aws_security_group_invalid_protocol.go b/rules/aws_security_group_invalid_protocol.go new file mode 100644 index 00000000..7df9e1a0 --- /dev/null +++ b/rules/aws_security_group_invalid_protocol.go @@ -0,0 +1,111 @@ +package rules + +import ( + "fmt" + "strconv" + "strings" + + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/terraform-linters/tflint-ruleset-aws/project" +) + +// AwsSecurityGroupInvalidProtocolRule checks whether "protocol" in "ingress" or "egress" has invalid value +type AwsSecurityGroupInvalidProtocolRule struct { + tflint.DefaultRule + + resourceType string + protocols map[string]struct{} +} + +// NewAwsSecurityGroupInvalidProtocolRule returns new rule with default attributes +func NewAwsSecurityGroupInvalidProtocolRule() *AwsSecurityGroupInvalidProtocolRule { + return &AwsSecurityGroupInvalidProtocolRule{ + resourceType: "aws_security_group", + protocols: map[string]struct{}{ + "all": {}, + "tcp": {}, + "udp": {}, + "icmp": {}, + "icmpv6": {}, + }, + } +} + +// Name returns the rule name +func (r *AwsSecurityGroupInvalidProtocolRule) Name() string { + return "aws_security_group_invalid_protocol" +} + +// Enabled returns whether the rule is enabled by default +func (r *AwsSecurityGroupInvalidProtocolRule) Enabled() bool { + return true +} + +// Severity returns the rule severity +func (r *AwsSecurityGroupInvalidProtocolRule) Severity() tflint.Severity { + return tflint.ERROR +} + +// Link returns the rule reference link +func (r *AwsSecurityGroupInvalidProtocolRule) Link() string { + return project.ReferenceLink(r.Name()) +} + +// Check checks whether "protocol" in "ingress" or "egress" has invalid value +func (r *AwsSecurityGroupInvalidProtocolRule) Check(runner tflint.Runner) error { + resources, err := runner.GetResourceContent(r.resourceType, &hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "ingress", + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "protocol"}, + }, + }, + }, + { + Type: "egress", + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "protocol"}, + }, + }, + }, + }, + }, nil) + if err != nil { + return err + } + + for _, resource := range resources.Blocks { + for _, rule := range resource.Body.Blocks { + attribute, exists := rule.Body.Attributes["protocol"] + if !exists { + continue + } + + var protocol string + err := runner.EvaluateExpr(attribute.Expr, &protocol, nil) + + err = runner.EnsureNoError(err, func() error { + if _, err := strconv.Atoi(protocol); err == nil { + return nil + } + if _, ok := r.protocols[strings.ToLower(protocol)]; !ok { + runner.EmitIssue( + r, + fmt.Sprintf("\"%s\" is an invalid protocol.", protocol), + attribute.Expr.Range(), + ) + } + return nil + }) + if err != nil { + return err + } + } + } + + return nil +} diff --git a/rules/aws_security_group_invalid_protocol_test.go b/rules/aws_security_group_invalid_protocol_test.go new file mode 100644 index 00000000..2f5589d4 --- /dev/null +++ b/rules/aws_security_group_invalid_protocol_test.go @@ -0,0 +1,117 @@ +package rules + +import ( + "testing" + + hcl "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/helper" +) + +func Test_AwsSecurityGroupInvalidProtocol(t *testing.T) { + cases := []struct { + Name string + Content string + Expected helper.Issues + }{ + { + Name: "valid protocols", + Content: ` +resource "aws_security_group" "this" { + ingress { + protocol = "tcp" + } + egress { + protocol = "-1" + } +} +`, + Expected: helper.Issues{}, + }, + { + Name: "valid protocol with upper case (for Terraform v0.11 and older)", + Content: ` +resource "aws_security_group" "this" { + ingress { + protocol = "TCP" + } +} +`, + Expected: helper.Issues{}, + }, + { + Name: "invalid ingress.protocol", + Content: ` +resource "aws_security_group" "this" { + ingress { + protocol = "http" + } +} +`, + Expected: helper.Issues{ + { + Rule: NewAwsSecurityGroupInvalidProtocolRule(), + Message: "\"http\" is an invalid protocol.", + Range: hcl.Range{ + Filename: "resource.tf", + Start: hcl.Pos{Line: 4, Column: 20}, + End: hcl.Pos{Line: 4, Column: 26}, + }, + }, + }, + }, + { + Name: "invalid egress.protocol", + Content: ` +resource "aws_security_group" "this" { + egress { + protocol = "https" + } +} +`, + Expected: helper.Issues{ + { + Rule: NewAwsSecurityGroupInvalidProtocolRule(), + Message: "\"https\" is an invalid protocol.", + Range: hcl.Range{ + Filename: "resource.tf", + Start: hcl.Pos{Line: 4, Column: 20}, + End: hcl.Pos{Line: 4, Column: 27}, + }, + }, + }, + }, + { + Name: "invalid protocol with upper case (for Terraform v0.11 and older)", + Content: ` +resource "aws_security_group" "this" { + ingress { + protocol = "HTTP" + } +} +`, + Expected: helper.Issues{ + { + Rule: NewAwsSecurityGroupInvalidProtocolRule(), + Message: "\"HTTP\" is an invalid protocol.", + Range: hcl.Range{ + Filename: "resource.tf", + Start: hcl.Pos{Line: 4, Column: 20}, + End: hcl.Pos{Line: 4, Column: 26}, + }, + }, + }, + }, + } + + rule := NewAwsSecurityGroupInvalidProtocolRule() + + 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 56976a0f..e886ce95 100644 --- a/rules/provider.go +++ b/rules/provider.go @@ -38,6 +38,7 @@ var manualRules = []tflint.Rule{ NewAwsIAMGroupPolicyTooLongRule(), NewAwsAcmCertificateLifecycleRule(), NewAwsElasticBeanstalkEnvironmentInvalidNameFormatRule(), + NewAwsSecurityGroupInvalidProtocolRule(), NewAwsSecurityGroupRuleInvalidProtocolRule(), }