Skip to content

Commit

Permalink
Add rule to ensure consume_groups is prefixed
Browse files Browse the repository at this point in the history
Ticket: DENA-1034
  • Loading branch information
matthewhughes-uw committed Nov 6, 2024
1 parent cab51dc commit 37a051d
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ plugin "uw-kafka-config" {
| [`msk_topic_config`](rules/msk_topic_config.md) | Checks the configuration for MSK topics |
| [`msk_topic_config_comments`](rules/msk_topic_config_comments.md) | Checks the comments for topic configurations expressed in millis |
| [`msk_unique_app_names`](rules/msk_unique_app_names.md) | Checks that TLS app names are unique |
| [`msk_app_consume_groups`](rules/msk_app_consume_groups.md) | Checks that TLS app consume groups are prefixed with a team name |


## Building the plugin
Expand Down
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func main() {
&rules.MSKAppTopicsRule{},
&rules.MSKTopicNameRule{},
&rules.MSKTopicConfigRule{},
&rules.MSKAppConsumeGroupsRule{},
// keep the comments rule after the config one, as the config one might remove some properties checked by the comments one
&rules.MSKTopicConfigCommentsRule{},
&rules.MSKUniqueAppNamesRule{},
Expand Down
116 changes: 116 additions & 0 deletions rules/msk_app_consume_groups.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package rules

import (
"fmt"
"strings"

"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 (
consumeGroupAttrName = "consume_groups"
consumeGroupSepChar = "."
)

type MSKAppConsumeGroupsRule struct {
tflint.DefaultRule
}

func (r *MSKAppConsumeGroupsRule) Name() string {
return "msk_app_consume_groups"
}

func (r *MSKAppConsumeGroupsRule) Enabled() bool {
return true
}

func (r *MSKAppConsumeGroupsRule) Link() string {
return ReferenceLink(r.Name())
}

func (r *MSKAppConsumeGroupsRule) Severity() tflint.Severity {
return tflint.ERROR
}

func (r *MSKAppConsumeGroupsRule) Check(runner tflint.Runner) error {
isRoot, err := isRootModule(runner)
if err != nil {
return err
}
if !isRoot {
logger.Debug("skipping child module")
return nil
}

appBlocks, err := getTLSApps(runner)
if err != nil {
return err
}

return r.validateConsumeGroups(runner, appBlocks)
}

func getTLSApps(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: consumeGroupAttrName},
},
},
},
},
},
nil,
)
if err != nil {
return nil, fmt.Errorf("getting modules: %w", err)
}

var appBlocks hclext.Blocks
for _, block := range modules.Blocks {
_, ok := block.Body.Attributes[consumeGroupAttrName]
if !ok {
logger.Debug("skipping block, doesn't have 'consume_group' attribute", "labels", block.Labels)
continue
}
appBlocks = append(appBlocks, block)
}

return appBlocks, nil
}

func (r *MSKAppConsumeGroupsRule) validateConsumeGroups(runner tflint.Runner, appBlocks hclext.Blocks) error {
for _, block := range appBlocks {
consumeGroupAttr := block.Body.Attributes[consumeGroupAttrName]

var consumeGroupNames []string
if err := runner.EvaluateExpr(consumeGroupAttr.Expr, &consumeGroupNames, nil); err != nil {
return fmt.Errorf("decoding attribute '%s': %v", consumeGroupAttrName, err)
}
for _, name := range consumeGroupNames {
if !strings.Contains(name, consumeGroupSepChar) {
err := runner.EmitIssue(
r,
fmt.Sprintf(
"'%s' must be prefixed with the name of the team using it, but '%s' is not",
consumeGroupAttrName,
name,
),
consumeGroupAttr.Range,
)
if err != nil {
return fmt.Errorf("emitting issue: %w", err)
}
}
}
}

return nil
}
42 changes: 42 additions & 0 deletions rules/msk_app_consume_groups.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# `msk_app_consume_groups`

Requires that any element of the `consume_groups` in a `tls-app` be prefixed wit
the name of the team using it, e.g. `my-team.my-consumer-group`

This is because MSK is a multi-tenant environment and we want to know to which
team a consumer group belongs. Additionally, in kafka-ui, access is given to
consumer groups based on the team prefixes.

## Examples

### Bad example

``` hcl
module "my_indexer" {
source = "../../../modules/tls-app"
cert_common_name = "some-team/indexer"
consume_topics = ["some-team.example"]
consume_groups = [
# BAD: group name not prefixed with consuming team
"some-example-consumer"
]
}
```

### Good example

The simplest solution is to just add your team name as the prefix:

``` hcl
module "my_indexer" {
source = "../../../modules/tls-app"
cert_common_name = "some-team/indexer"
consume_topics = ["some-team.example"]
consume_groups = [
# GOOD: group name prefixed with team
"some-team.some-example-consumer"
]
}
```
93 changes: 93 additions & 0 deletions rules/msk_app_consume_groups_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package rules

import (
"testing"

"github.com/hashicorp/hcl/v2"
"github.com/stretchr/testify/require"
"github.com/terraform-linters/tflint-plugin-sdk/helper"
)

func Test_MSKAppConsumeGroupsRule(t *testing.T) {
rule := &MSKAppConsumeGroupsRule{}

for _, tc := range []struct {
name string
files map[string]string
expected helper.Issues
}{
{
name: "single bad entry",
files: map[string]string{
"file.tf": `
module "my-app" {
consume_groups = ["my-bad-group"]
}
`,
},
expected: []*helper.Issue{
{
Rule: rule,
Message: "'consume_groups' must be prefixed with the name of the team using it, but 'my-bad-group' is not",
Range: hcl.Range{
Filename: "file.tf",
Start: hcl.Pos{Line: 3, Column: 2},
End: hcl.Pos{Line: 3, Column: 35},
},
},
},
},
{
name: "multiple bad entres",
files: map[string]string{
"file.tf": `
module "my-app" {
consume_groups = [
"my-bad-group1",
"my-bad-group2",
]
}
`,
},
expected: []*helper.Issue{
{
Rule: rule,
Message: "'consume_groups' must be prefixed with the name of the team using it, but 'my-bad-group1' is not",
Range: hcl.Range{
Filename: "file.tf",
Start: hcl.Pos{Line: 3, Column: 2},
End: hcl.Pos{Line: 6, Column: 3},
},
},
{
Rule: rule,
Message: "'consume_groups' must be prefixed with the name of the team using it, but 'my-bad-group2' is not",
Range: hcl.Range{
Filename: "file.tf",
Start: hcl.Pos{Line: 3, Column: 2},
End: hcl.Pos{Line: 6, Column: 3},
},
},
},
},
{
name: "no issue on valid names",
files: map[string]string{
"file.tf": `
module "my-app" {
consume_groups = ["my-team.my-group1, my-team.my-group2"]
}
`,
},
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)
})
}
}

0 comments on commit 37a051d

Please sign in to comment.