fix(report): Result field is optional. #214
Conversation
The code which builds the PolicyReportResult does check if the Result field from the AdmissionRespose type is nil. Causing a nil pointer dereference. This field is optional and it can be nil. Specially if the policy result is "allowed" where there is not further details about the result of the evaluation. This commit fixes this issue by checking if the Result field has something before trying to get the possible error message from the policy. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
internal/report/report_test.go
Outdated
| @@ -235,6 +235,46 @@ func TestNewPolicyReportResult(t *testing.T) { | |||
| }, | |||
| }, | |||
| }, | |||
| { | |||
| name: "Validating policy, allowed response with no message", | |||
There was a problem hiding this comment.
Instead of adding a new test we should modify the existing tests in this file and the mock policy server handler in scanner_test.go to remove the Result when the AdmissionReview is allowed.
There was a problem hiding this comment.
Yeah, I was thinking about that too. But I've decided to keep the old tests just in case I was missing something that you know. As you are not around and leave that for the review process, like know. :)
I've just updated the tests. Please, give another look.
AdmissionResponse does not have the Result field filled when it is allowed by the policy. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
=======================================
Coverage 72.37% 72.37%
=======================================
Files 6 6
Lines 637 637
=======================================
Hits 461 461
Misses 115 115
Partials 61 61
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
The code which builds the PolicyReportResult does check if the Result field from the AdmissionRespose type is nil. Causing a nil pointer dereference. This field is optional and it can be nil. Specially if the policy result is "allowed" where there is not further details about the result of the evaluation.
This commit fixes this issue by checking if the Result field has something before trying to get the possible error message from the policy.
Fix #212
Test
I've added a new unit test to cover this bug.