Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom rules that report on the absence of aggregates #1260

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions bundle/regal/main/main.rego
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,23 @@ aggregate[category_title] contains entry if {
config.for_rule(category, title).level != "ignore"
not config.excluded_file(category, title, input.regal.file.name)

some entry in data.custom.regal.rules[category][title].aggregate
entries := _mark_if_empty(data.custom.regal.rules[category][title].aggregate)

category_title := concat("/", [category, title])

some entry in entries
}

# a custom aggregate rule may not come back with entries, but we still need
# to register the fact that it was called so that we know to call the
# aggregate_report for the same rule later
#
# for these cases we just return an empty map, and let the aggregator on the Go
# side handle this case
_mark_if_empty(entries) := {{}} if {
count(entries) == 0
} else := entries

# METADATA
# description: Check bundled rules using aggregated data
# schemas:
Expand Down Expand Up @@ -187,7 +199,7 @@ aggregate_report contains violation if {
not config.excluded_file(category, title, input.regal.file.name)

input_for_rule := object.remove(
object.union(input, {"aggregate": input.aggregates_internal[key]}),
object.union(input, {"aggregate": _null_to_empty(input.aggregates_internal[key])}),
["aggregates_internal"],
)

Expand All @@ -211,3 +223,7 @@ _ignored(violation, directives) if {
ignored_rules := directives[util.to_location_object(violation.location).row + 1]
violation.title in ignored_rules
}

_null_to_empty(x) := [] if {
x == null
} else := x
Comment on lines +227 to +229
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like a code golf opportunity, but I'm not in the mood right now 😉

24 changes: 24 additions & 0 deletions e2e/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,30 @@ func TestAggregatesAreCollectedAndUsed(t *testing.T) {
t.Errorf("expected 1 violation, got %d", rep.Summary.NumViolations)
}
})

t.Run("custom policy where nothing aggregate is a violation", func(t *testing.T) {
stdout, stderr := bytes.Buffer{}, bytes.Buffer{}

err := regal(&stdout, &stderr)("lint", "--format", "json", "--rules",
basedir+filepath.FromSlash("/custom/regal/rules/testcase/empty_aggregate/"),
basedir+filepath.FromSlash("/two_policies"))

expectExitCode(t, err, 3, &stdout, &stderr)

if exp, act := "", stderr.String(); exp != act {
t.Errorf("expected stderr %q, got %q", exp, act)
}

var rep report.Report

if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil {
t.Fatalf("expected JSON response, got %v", stdout.String())
}

if rep.Summary.NumViolations != 1 {
t.Errorf("expected 1 violation, got %d", rep.Summary.NumViolations)
}
})
}

func TestLintAggregateIgnoreDirective(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# METADATA
# description: |
# Test to ensure a custom rule that aggregated no data is still reported
# related_resources:
# - description: issue
# ref: https://github.com/StyraInc/regal/issues/1259
package custom.regal.rules.testcase.empty_aggregates

import rego.v1

import data.regal.result

aggregate contains result.aggregate(rego.metadata.chain(), {}) if {
input.nope
}

aggregate_report contains violation if {
count(input.aggregate) == 0

violation := result.fail(rego.metadata.chain(), {})
}
15 changes: 14 additions & 1 deletion pkg/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,20 @@ func (l Linter) lintWithRegoRules(
regoReport.Notices = append(regoReport.Notices, result.Notices...)

for k := range result.Aggregates {
regoReport.Aggregates[k] = append(regoReport.Aggregates[k], result.Aggregates[k]...)
// Custom aggregate rules that have been invoked but not returned any data
// will return an empty map to signal that they have been called, and that
// the aggregate report for this rule should be invoked even when no data
// was aggregated. This because the absence of data is exactly what some rules
// will want to report on.
for _, agg := range result.Aggregates[k] {
if len(agg) == 0 {
if _, ok := regoReport.Aggregates[k]; !ok {
regoReport.Aggregates[k] = make([]report.Aggregate, 0)
}
} else {
regoReport.Aggregates[k] = append(regoReport.Aggregates[k], agg)
}
}
}

for k := range result.IgnoreDirectives {
Expand Down