Failure vs successes counts are inconsistent

[consolethinks]

Conftest doesn't count passed tests in the total amount of tests, or number of passed tests when the policy is not in the main namespace. It comes across as if the test doesn't even exist when it passes.

If, for example, I have a common namespace and a safety namespace, in addition to the main namespace, and I either pass --all-namespaces or list them separately using --namespace [name], only the policies in the main namespace will have its passed policies counted, any other namespaces' passed policies will be ignored from the total and passed counts.

[jalseth]

Hi @consolethinks I'm not able to reproduce this issue with conftest v0.33.2. Please provide snippets and the exact conftest command you are using so I can reproduce the issue.

[consolethinks]

After doing some testing, it's not related to namespaces after all. One policy in a specific namespace seems to be the culprit.

To reproduce my issue, I'm giving an example terraform plan outputted as json:
tfplan.json.zip

And here are two policies:

package common

key_val_valid_pascal_case(key, val) {
    is_pascal_case(key)
    is_pascal_case(val)
}

is_pascal_case(string) {
    re_match(`^([A-Z][a-z0-9]+)+`, string)
}


deny[msg] {
    changeset := input.resource_changes[_]
    tags := changeset.change.after.tags
    some key
    not is_pascal_case(tags[key])
    msg := sprintf("general - Non-pascal case value in tags - %v - key: %v, value: %v", [changeset.address, key, tags[key]])
}

deny[msg] {
    false
    msg := ""
}

The following command is used:
conftest test tfplan.json --policy [folder containing policies] --all-namespaces

If the first policy fails for multiple resources, it will make Conftest say that there are 0 passing tests. If it is disablbed (by renaming the first deny to something else for example), Conftest will show one passing test.

[jalseth]

Thank you for the extra info, I have been able to reproduce the issue though the cause is not immediately clear.

[jalseth]

I've spent a little time digging and better understand the issue now. The root of the problem is due to the nature of OPA/Rego and the fact that rules are queried by name and the results are unioned. While conftest can know that there are multiple deny rules in a given policy source, it cannot know which of those caused violations as it only queries the single data.<namespace>.deny rule and all results are merged within the OPA engine. Because a single deny[msg] can produce multiple violations (as is the case here), conftest cannot meaningfully keep track of the count of failures vs. successes. I think this is a #wontfix as it is just how OPA works at a fundamental level.

The best way forward if these counts matter to your use case is to have only one deny rule per namespace and have the namespace be scoped to a specific policy violation. I've linked the relevant parts of the code below. Let me know if you have any questions.

conftest/policy/engine.go

Lines 314 to 346 in 3aada8c

	ruleQuery := fmt.Sprintf("data.%s.%s", namespace, rule)
	ruleQueryResult, err := e.query(ctx, config, ruleQuery)
	if err != nil {
	return output.CheckResult{}, fmt.Errorf("query rule: %w", err)
	}
	var failures []output.Result
	var warnings []output.Result
	for _, ruleResult := range ruleQueryResult.Results {
	// Exceptions have already been accounted for in the exception query so
	// we skip them here to avoid doubling the result.
	if len(exceptions) > 0 {
	continue
	}
	if ruleResult.Passed() {
	successes++
	continue
	}
	if isFailure(rule) {
	failures = append(failures, ruleResult)
	} else {
	warnings = append(warnings, ruleResult)
	}
	}
	checkResult.Failures = append(checkResult.Failures, failures...)
	checkResult.Warnings = append(checkResult.Warnings, warnings...)
	checkResult.Exceptions = append(checkResult.Exceptions, exceptions...)
	checkResult.Queries = append(checkResult.Queries, exceptionQueryResult, ruleQueryResult)

conftest/output/standard.go

Lines 58 to 105 in 3aada8c

	var totalFailures int
	var totalExceptions int
	var totalWarnings int
	var totalSuccesses int
	var totalSkipped int
	for _, result := range results {
	var indicator string
	var namespace string
	if result.FileName == "-" {
	indicator = "-"
	} else {
	indicator = fmt.Sprintf("- %s", result.FileName)
	}
	if result.Namespace == "-" {
	namespace = "-"
	} else {
	namespace = fmt.Sprintf("- %s -", result.Namespace)
	}
	totalPolicies := result.Successes + len(result.Warnings) + len(result.Failures) + len(result.Exceptions) + len(result.Skipped)
	if totalPolicies == 0 {
	fmt.Fprintln(s.Writer, colorizer.Colorize("?", aurora.WhiteFg), indicator, namespace, "no policies found")
	continue
	}
	for _, warning := range result.Warnings {
	fmt.Fprintln(s.Writer, colorizer.Colorize("WARN", aurora.YellowFg), indicator, namespace, warning.Message)
	}
	for _, failure := range result.Failures {
	fmt.Fprintln(s.Writer, colorizer.Colorize("FAIL", aurora.RedFg), indicator, namespace, failure.Message)
	}
	if !s.SuppressExceptions {
	for _, exception := range result.Exceptions {
	fmt.Fprintln(s.Writer, colorizer.Colorize("EXCP", aurora.CyanFg), indicator, namespace, exception.Message)
	}
	}
	totalFailures += len(result.Failures)
	totalExceptions += len(result.Exceptions)
	totalWarnings += len(result.Warnings)
	totalSkipped += len(result.Skipped)
	totalSuccesses += result.Successes
	}
	totalTests := totalFailures + totalExceptions + totalWarnings + totalSuccesses + totalSkipped