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 <[email protected]>

Author: x-color

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

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

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)

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
+}

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)
+	}
+}

rules/provider.go

@@ -38,6 +38,7 @@ var manualRules = []tflint.Rule{
 	NewAwsIAMGroupPolicyTooLongRule(),
 	NewAwsAcmCertificateLifecycleRule(),
 	NewAwsElasticBeanstalkEnvironmentInvalidNameFormatRule(),
+	NewAwsSecurityGroupInvalidProtocolRule(),
 	NewAwsSecurityGroupRuleInvalidProtocolRule(),
 }