Add rule to ensure unique TLS app names

Duplicate names can cause all sorts of issues, see tickets for details

Ticket: DENA-991

Author: matthewhughes-uw

README.md

@@ -23,6 +23,7 @@ plugin "uw-kafka-config" {
 | [`msk_app_topics`](rules/msk_app_topics.md)         | Requires apps consume from and produce to only topics define in their module.                                                    |
 | [`msk_topic_name`](rules/msk_topic_name.md)         | Requires defined topics in a module to belong to that team.                                                                      |
 | [`msk_topic_config`](rules/msk_topic_config.md)     | Checks the configuration for MSK topics                                                                                          |
+| [`msk_unique_app_names`](rules/msk_unique_app_names.md) | Checks that TLS app names are unique |
 
 
 ## Building the plugin

main.go

@@ -20,6 +20,7 @@ func main() {
 				&rules.MSKAppTopicsRule{},
 				&rules.MSKTopicNameRule{},
 				&rules.MSKTopicConfigRule{},
+				&rules.MSKUniqueAppNamesRule{},
 			},
 		},
 	})

rules/msk_unique_app_names.go

@@ -0,0 +1,123 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/hcl/v2/gohcl"
+	"github.com/terraform-linters/tflint-plugin-sdk/hclext"
+	"github.com/terraform-linters/tflint-plugin-sdk/logger"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+)
+
+const commonNameAttribute = "cert_common_name"
+
+type MSKUniqueAppNamesRule struct {
+	tflint.DefaultRule
+}
+
+func (r *MSKUniqueAppNamesRule) Name() string {
+	return "msk_unique_app_names"
+}
+
+func (r *MSKUniqueAppNamesRule) Enabled() bool {
+	return true
+}
+
+func (r *MSKUniqueAppNamesRule) Link() string {
+	return ReferenceLink(r.Name())
+}
+
+func (r *MSKUniqueAppNamesRule) Severity() tflint.Severity {
+	return tflint.ERROR
+}
+
+func (r *MSKUniqueAppNamesRule) Check(runner tflint.Runner) error {
+	isRoot, err := isRootModule(runner)
+	if err != nil {
+		return err
+	}
+	if !isRoot {
+		logger.Debug("skipping child module")
+		return nil
+	}
+
+	TLSAppModules, err := getTLSAppModules(runner)
+	if err != nil {
+		return err
+	}
+
+	return r.reportDuplicateTLSAppNames(runner, TLSAppModules)
+}
+
+func getTLSAppModules(runner tflint.Runner) (hclext.Blocks, error) {
+	modules, err := runner.GetModuleContent(
+		&hclext.BodySchema{
+			Blocks: []hclext.BlockSchema{
+				{
+					Type:       "module",
+					LabelNames: []string{"name"},
+					Body: &hclext.BodySchema{
+						Attributes: []hclext.AttributeSchema{
+							{Name: commonNameAttribute},
+						},
+					},
+				},
+			},
+		},
+		nil,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("getting modules: %w", err)
+	}
+
+	var TLSAppModules hclext.Blocks
+	for _, moduleBlock := range modules.Blocks {
+		if _, ok := moduleBlock.Body.Attributes[commonNameAttribute]; ok {
+			TLSAppModules = append(TLSAppModules, moduleBlock)
+		}
+	}
+
+	return TLSAppModules, nil
+}
+
+type tlsAppName struct {
+	attr *hclext.Attribute
+	name string
+}
+
+func (r *MSKUniqueAppNamesRule) reportDuplicateTLSAppNames(runner tflint.Runner, tlsAppModules hclext.Blocks) error {
+	seenNames := map[string]struct{}{}
+	duplicateNames := []tlsAppName{}
+	for _, appModule := range tlsAppModules {
+		appNameAttr := appModule.Body.Attributes[commonNameAttribute]
+
+		var appName string
+		diags := gohcl.DecodeExpression(appNameAttr.Expr, nil, &appName)
+		if diags.HasErrors() {
+			return fmt.Errorf("decoding expression for attribute %s: %w", commonNameAttribute, diags)
+		}
+
+		if _, ok := seenNames[appName]; ok {
+			duplicateNames = append(duplicateNames, tlsAppName{attr: appNameAttr, name: appName})
+			continue
+		}
+
+		seenNames[appName] = struct{}{}
+	}
+
+	for _, appName := range duplicateNames {
+		if err := runner.EmitIssue(
+			r,
+			fmt.Sprintf(
+				"'%s' must be unique across a module, but '%s' has already been seen",
+				commonNameAttribute,
+				appName.name,
+			),
+			appName.attr.Range,
+		); err != nil {
+			return fmt.Errorf("emitting issue: %w", err)
+		}
+	}
+
+	return nil
+}

rules/msk_unique_app_names.md

@@ -0,0 +1,43 @@
+# `msk_unique_app_names`
+
+## Requirements
+
+Requires all modules using the `tls-app` in our Kafka cluster config provide a
+unique name for the `cert_common_name`. This is because this name is used to
+identify the ACLs for the modules.
+
+## Example
+
+### Bad example
+
+``` hcl
+module "my_team_example_producer" {
+  source           = "../../../modules/tls-app"
+  produce_topics   = [kafka_topic.some_topic.name]
+  cert_common_name = "pubsub/example-app"
+}
+
+module "my_team_example_consumer" {
+  source           = "../../../modules/tls-app"
+  consume_topics   = [kafka_topic.some_topic.name]
+  # BAD: cert_common_name is same as module above
+  cert_common_name = "pubsub/example-app"
+}
+```
+
+### Good example
+
+``` hcl
+module "my_team_example_producer" {
+  source           = "../../../modules/tls-app"
+  produce_topics   = [kafka_topic.some_topic.name]
+  cert_common_name = "pubsub/example-producer"
+}
+
+module "my_team_example_consumer" {
+  source           = "../../../modules/tls-app"
+  consume_topics   = [kafka_topic.some_topic.name]
+  # GOOD: cert_common_name is unique
+  cert_common_name = "pubsub/example-consumer"
+}
+```

rules/msk_unique_app_names_test.go

@@ -0,0 +1,141 @@
+package rules
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/stretchr/testify/require"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_MSKUniqueAppNamesRule(t *testing.T) {
+	rule := &MSKUniqueAppNamesRule{}
+
+	for _, tc := range []struct {
+		name     string
+		files    map[string]string
+		expected helper.Issues
+	}{
+		{
+			name: "reports duplicate app names in same file",
+			files: map[string]string{
+				"file.tf": `
+module "first_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+
+module "second_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+`,
+			},
+			expected: []*helper.Issue{
+				{
+					Rule:    rule,
+					Message: "'cert_common_name' must be unique across a module, but 'my-namespace/my-app' has already been seen",
+					Range: hcl.Range{
+						Filename: "file.tf",
+						Start:    hcl.Pos{Line: 9, Column: 3},
+						End:      hcl.Pos{Line: 9, Column: 43},
+					},
+				},
+			},
+		},
+		{
+			name: "reports duplicate app names across files",
+			files: map[string]string{
+				"first.tf": `
+module "first_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+`,
+				"second.tf": `
+module "second_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+`,
+			},
+			expected: []*helper.Issue{
+				{
+					Rule:    rule,
+					Message: "'cert_common_name' must be unique across a module, but 'my-namespace/my-app' has already been seen",
+					Range: hcl.Range{
+						Filename: "second.tf",
+						Start:    hcl.Pos{Line: 4, Column: 3},
+						End:      hcl.Pos{Line: 4, Column: 43},
+					},
+				},
+			},
+		},
+		{
+			name: "reports repeated duplicate app names",
+			files: map[string]string{
+				"file.tf": `
+module "first_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+
+module "second_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+
+module "third_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/my-app"
+}
+`,
+			},
+			expected: []*helper.Issue{
+				{
+					Rule:    rule,
+					Message: "'cert_common_name' must be unique across a module, but 'my-namespace/my-app' has already been seen",
+					Range: hcl.Range{
+						Filename: "file.tf",
+						Start:    hcl.Pos{Line: 9, Column: 3},
+						End:      hcl.Pos{Line: 9, Column: 43},
+					},
+				},
+				{
+					Rule:    rule,
+					Message: "'cert_common_name' must be unique across a module, but 'my-namespace/my-app' has already been seen",
+					Range: hcl.Range{
+						Filename: "file.tf",
+						Start:    hcl.Pos{Line: 14, Column: 3},
+						End:      hcl.Pos{Line: 14, Column: 43},
+					},
+				},
+			},
+		},
+		{
+			name: "Reports nothing with all unique names",
+			files: map[string]string{
+				"file.tf": `
+module "first_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/first-app"
+}
+
+module "second_app" {
+  source           = "../../../modules/tls-app"
+  cert_common_name = "my-namespace/second-app"
+}
+`,
+			},
+			expected: []*helper.Issue{},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			runner := helper.TestRunner(t, tc.files)
+
+			require.NoError(t, rule.Check(runner))
+
+			helper.AssertIssues(t, tc.expected, runner.Issues)
+		})
+	}
+}