Skip to content

Commit

Permalink
Fix panic on non-string topic name
Browse files Browse the repository at this point in the history
A panic results in an unhelpful message when running the plugin:

    Failed to check ruleset; error reading from server: EOF

So report an error here instead.

There is potential for similar behaviour if `consume_topics` is not a
list, in which case `AsValueSlice` would panic, but I'm content to not
be too defensive here if no-ones needing that change.

Ticket: DENA-1175
  • Loading branch information
matthewhughes-uw committed Jan 29, 2025
1 parent 8262fab commit 481d650
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
16 changes: 16 additions & 0 deletions rules/msk_app_topics.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,22 @@ func (r *MSKAppTopicsRule) reportExternalTopics(
return fmt.Errorf("evaluating topic names: %w", diags)
}
for _, v := range val.AsValueSlice() {
if v.Type() != cty.String {
err := runner.EmitIssue(
r,
fmt.Sprintf(
"value for '%s' should be a string, but it is a: %s",
attrName,
v.Type().FriendlyName(),
),
topicAttr.Range,
)
if err != nil {
return fmt.Errorf("emitting issue: %w", err)
}
continue
}

name := v.AsString()
if _, ok := moduleTopicNames[name]; !ok {
err := runner.EmitIssue(
Expand Down
25 changes: 25 additions & 0 deletions rules/msk_app_topics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,31 @@ module "consumer" {
},
},
},
{
name: "topic name is not string",
files: map[string]string{
"file.tf": `
resource "kafka_topic" "my_topic" {
name = "my_topic"
}
module "consumer" {
consume_topics = [kafka_topic.my_topic]
}
`,
},
expected: []*helper.Issue{
{
Rule: rule,
Message: "value for 'consume_topics' should be a string, but it is a: object",
Range: hcl.Range{
Filename: "file.tf",
Start: hcl.Pos{Line: 7, Column: 2},
End: hcl.Pos{Line: 7, Column: 41},
},
},
},
},
{
name: "external topic defined outside of consumer/producer",
files: map[string]string{
Expand Down

0 comments on commit 481d650

Please sign in to comment.