No s3 global endpoint rule

rules/aws_s3_no_global_endpoint.go

@@ -0,0 +1,88 @@
+package rules
+
+import (
+	"strings"
+
+	hcl "github.com/hashicorp/hcl/v2"
+	hclsyntax "github.com/hashicorp/hcl/v2/hclsyntax"
+	lang "github.com/terraform-linters/tflint-plugin-sdk/terraform/lang"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	"github.com/terraform-linters/tflint-ruleset-aws/project"
+)
+
+// AwsS3NoGlobalEndpointRule checks whether deprecated s3 global endpoint is used
+type AwsS3NoGlobalEndpointRule struct {
+	tflint.DefaultRule
+}
+
+// NewAwsS3NoGlobalEndpointRule returns new rule with default attributes
+func NewAwsS3NoGlobalEndpointRule() *AwsS3NoGlobalEndpointRule {
+	return &AwsS3NoGlobalEndpointRule{}
+}
+
+// Name returns the rule name
+func (r *AwsS3NoGlobalEndpointRule) Name() string {
+	return "aws_s3_no_global_endpoint"
+}
+
+// Enabled returns whether the rule is enabled by default
+func (r *AwsS3NoGlobalEndpointRule) Enabled() bool {
+	return false
+}
+
+// Severity returns the rule severity
+func (r *AwsS3NoGlobalEndpointRule) Severity() tflint.Severity {
+	return tflint.WARNING
+}
+
+// Link returns the rule reference link
+func (r *AwsS3NoGlobalEndpointRule) Link() string {
+	return project.ReferenceLink(r.Name())
+}
+
+func isS3BucketResourceOrData(str string) bool {
+	return strings.HasPrefix(str, "aws_s3_bucket") || strings.HasPrefix(str, "data.aws_s3_bucket")
+}
+
+func getRealRange(srcRange hcl.Range) hcl.Range {
+	// ref.SourceRange doesn't include the bucket_domain_name attribute which is 19 characters long
+	srcRange.End.Column += 19
+	return srcRange
+}
+
+// Check checks whether the deprecated s3 global endpoint is used
+func (r *AwsS3NoGlobalEndpointRule) Check(runner tflint.Runner) error {
+	var err error
+	runner.WalkExpressions(tflint.ExprWalkFunc(func(expr hcl.Expression) hcl.Diagnostics {
+		_, isTemplateExpr := expr.(*hclsyntax.TemplateExpr)
+		if isTemplateExpr {
+			// TODO: check sth like .s3.amazonaws.com inside template expression ?
+
+			// "${aws_s3_bucket.test.bucket_domain_name}" would report the same error twice if we didn't return here
+			return nil
+		}
+
+		refs := lang.ReferencesInExpr(expr)
+
+		if len(refs) != 1 {
+			return nil
+		}
+
+		ref := refs[0]
+
+		// lang.ReferencesInExpr(expr) is broken when the reference contains subexpression ?
+		// e.g. aws_s3_bucket.test[length(var.bucket_names) - 1]
+		// TODO: seems like a niche case, should we handle this ?
+		if isS3BucketResourceOrData(ref.Subject.String()) && len(ref.Remaining) == 1 && ref.Remaining[0].(hcl.TraverseAttr).Name == "bucket_domain_name" {
+			// ref.SourceRange doesn't include the bucket_domain_name attribute which is 19 characters long
+			srcRange := getRealRange(ref.SourceRange)
+			err = runner.EmitIssue(r, "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead", srcRange)
+		}
+
+		// TODO: handle foreach, count, dynamic blocks
+
+		return nil
+	}))
+
+	return err
+}

rules/aws_s3_no_global_endpoint_test.go

@@ -0,0 +1,274 @@
+package rules
+
+import (
+	"testing"
+
+	hcl "github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_AwsS3NoGlobalEndpoint(t *testing.T) {
+	filename := "resource.tf"
+
+	cases := []struct {
+		Name     string
+		Content  string
+		Expected helper.Issues
+	}{
+		{
+			Name: "unrelated expressions",
+			Content: `
+output "test1" {
+  value = var.whatever
+}
+
+output "test2" {
+  value = "testing"
+}
+
+output "test3" {
+  value = aws_iam_role.test.arn
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "regional endpoint used",
+			Content: `
+output "test" {
+  value = aws_s3_bucket.test.bucket_regional_domain_name
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "global endpoint used",
+			Content: `
+output "test" {
+  value = aws_s3_bucket.test.bucket_domain_name
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 11},
+						End:      hcl.Pos{Line: 3, Column: 48},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint used from aws_s3_bucket data source",
+			Content: `
+output "test" {
+  value = data.aws_s3_bucket.test.bucket_domain_name
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 11},
+						End:      hcl.Pos{Line: 3, Column: 53},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint interpolation",
+			Content: `
+output "test" {
+  value = "interpolation test: ${aws_s3_bucket.test.bucket_domain_name}"
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 34},
+						End:      hcl.Pos{Line: 3, Column: 71},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint interpolation multiple expressions",
+			Content: `
+output "test" {
+  value = "interpolation test: ${aws_s3_bucket.test.bucket_domain_name} ${var.whatever}"
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 34},
+						End:      hcl.Pos{Line: 3, Column: 71},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint with count",
+			Content: `
+output "test1" {
+  value = aws_s3_bucket.test[0].bucket_domain_name
+}
+
+output "test2" {
+  value = aws_s3_bucket.test[length(var.bucket_names) - 1].bucket_domain_name
+}
+`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 11},
+						End:      hcl.Pos{Line: 3, Column: 51},
+					},
+				},
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 7, Column: 11},
+						End:      hcl.Pos{Line: 7, Column: 78},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint in a for expression",
+			Content: `
+output "test" {
+  value = {
+    for bucket_name, bucket in aws_s3_bucket.test: bucket_name => bucket.bucket_domain_name
+  }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 4, Column: 67},
+						End:      hcl.Pos{Line: 4, Column: 92},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint with foreach",
+			Content: `
+resource "aws_s3_bucket" "test" {
+  for_each = toset(var.bucket_names)
+
+  bucket = each.key
+}
+
+resource "aws_ssm_parameter" "test" {
+  for_each = aws_s3_bucket.test
+
+  name  = each.value.id
+  type  = "String"
+  value = each.value.bucket_domain_name
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 13, Column: 11},
+						End:      hcl.Pos{Line: 13, Column: 40},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint in a dynamic block",
+			Content: `
+resource "aws_s3_bucket" "test" {
+  for_each = toset(var.bucket_names)
+
+  bucket = each.key
+}
+
+resource "aws_cloudfront_distribution" "test" {
+  dynamic "origin" {
+    for_each = aws_s3_bucket.test
+
+    content {
+      origin_id   = origin.value.id
+      domain_name = origin.value.bucket_domain_name
+    }
+  }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 14, Column: 11},
+						End:      hcl.Pos{Line: 14, Column: 40},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint literal",
+			Content: `
+output "test" {
+  value = "test.s3.amazonaws.com"
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 12},
+						End:      hcl.Pos{Line: 3, Column: 33},
+					},
+				},
+			},
+		},
+		{
+			Name: "global endpoint literal with interpolation",
+			Content: `
+output "test" {
+  value = "${var.bucket_name}.s3.amazonaws.com"
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewAwsS3NoGlobalEndpointRule(),
+					Message: "`bucket_domain_name` returns the legacy s3 global endpoint, use `bucket_regional_domain_name` instead",
+					Range: hcl.Range{
+						Filename: filename,
+						Start:    hcl.Pos{Line: 3, Column: 12},
+						End:      hcl.Pos{Line: 3, Column: 47},
+					},
+				},
+			},
+		},
+	}
+
+	rule := NewAwsS3NoGlobalEndpointRule()
+
+	for _, tc := range cases {
+		runner := helper.TestRunner(t, map[string]string{filename: tc.Content})
+
+		if err := rule.Check(runner); err != nil {
+			t.Fatalf("Unexpected error occurred: %s", err)
+		}
+
+		helper.AssertIssues(t, tc.Expected, runner.Issues)
+	}
+}