feat: add scanfailure types for scan failure reporting#620
Conversation
Move scan failure report types (ScanFailureReport, ScanFailureCase, WorkloadIdentifier) to armoapi-go as the canonical home for API contract types. Previously in kubescape/messaging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Go file introduces type definitions for scan failure reporting in a scanner system. It defines a ScanFailureCase enum with four failure types, a WorkloadIdentifier struct to identify affected Kubernetes workloads, and a ScanFailureReport struct that contains per-image failure information including customer ID, workloads, image tag, failure reason, and timestamp. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new scanfailure package defining shared data structures for reporting container scan failures (including failure categorization and affected workload context) so downstream services can consume a consistent schema.
Changes:
- Introduces
ScanFailureCaseenum with aString()helper for human-readable descriptions. - Adds
WorkloadIdentifierandScanFailureReportstructs with JSON/BSON tags for transport/storage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // ScanFailureCVE — have SBOM, can't match against vulnerability DBs. | ||
| ScanFailureCVE ScanFailureCase = 1 | ||
| // ScanFailureSBOMGeneration — can't build SBOM from image. | ||
| ScanFailureSBOMGeneration ScanFailureCase = 2 | ||
| // ScanFailureOOMKilled — scanner process was OOM-killed. | ||
| ScanFailureOOMKilled ScanFailureCase = 3 | ||
| // ScanFailureBackendPost — scan succeeded but results couldn't be posted. | ||
| ScanFailureBackendPost ScanFailureCase = 4 | ||
| ) | ||
|
|
||
| // String returns a human-readable description of the failure case. | ||
| func (f ScanFailureCase) String() string { | ||
| switch f { |
There was a problem hiding this comment.
Fixed in efa4ba6. Added ScanFailureUnknown = 0 as explicit zero value.
| case ScanFailureSBOMGeneration: | ||
| return "SBOM generation failed" | ||
| case ScanFailureOOMKilled: | ||
| return "Scanner process OOM killed" |
There was a problem hiding this comment.
Fixed in efa4ba6. Changed to "OOM-killed" to match the constant name ScanFailureOOMKilled.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scanfailure/types.go (1)
37-42: Avoid long-term schema drift betweenWorkloadIdentifierandarmotypes.Resource.Line 37-Line 42 introduce a second workload identity model with overlapping semantics (
cluster/namespace/kind/name). This increases drift risk across services. Consider adding explicit conversion helpers to/fromarmotypes.Resourceso there is one canonical internal shape even if wire tags differ.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scanfailure/types.go` around lines 37 - 42, WorkloadIdentifier duplicates armotypes.Resource shape; add explicit conversion helpers to avoid schema drift by implementing two functions (e.g., WorkloadIdentifier.ToArmResource() armotypes.Resource and NewWorkloadIdentifierFromArm(res armotypes.Resource) WorkloadIdentifier) that map ClusterName/Namespace/WorkloadKind/WorkloadName ↔ corresponding armotypes.Resource fields, place them alongside the WorkloadIdentifier type (or in a nearby conversions file), and use these helpers across codepaths so the canonical internal representation remains armotypes.Resource while WorkloadIdentifier only handles wire/tag differences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scanfailure/types.go`:
- Around line 48-59: Add a Validate method on ScanFailureReport that enforces
the documented invariants: if IsRegistryScan is true then Workloads must be
empty/nil and RegistryName must be non-empty, and if IsRegistryScan is false
then RegistryName must be empty (or optional) and Workloads must be non-empty;
the method should return an error describing which invariant failed (use
ScanFailureReport.Validate) and be called wherever reports are constructed or
serialized (e.g., before sending or marshalling) so malformed reports cannot be
emitted. Ensure the checks handle nil/zero slices and trim/check RegistryName
for emptiness and use clear error messages mentioning the field names.
---
Nitpick comments:
In `@scanfailure/types.go`:
- Around line 37-42: WorkloadIdentifier duplicates armotypes.Resource shape; add
explicit conversion helpers to avoid schema drift by implementing two functions
(e.g., WorkloadIdentifier.ToArmResource() armotypes.Resource and
NewWorkloadIdentifierFromArm(res armotypes.Resource) WorkloadIdentifier) that
map ClusterName/Namespace/WorkloadKind/WorkloadName ↔ corresponding
armotypes.Resource fields, place them alongside the WorkloadIdentifier type (or
in a nearby conversions file), and use these helpers across codepaths so the
canonical internal representation remains armotypes.Resource while
WorkloadIdentifier only handles wire/tag differences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08a748c8-e4aa-4e46-9ff2-f58a03f5e1be
📒 Files selected for processing (1)
scanfailure/types.go
| type ScanFailureReport struct { | ||
| CustomerGUID string `json:"customerGUID" bson:"customerGUID"` | ||
| Workloads []WorkloadIdentifier `json:"workloads,omitempty" bson:"workloads,omitempty"` | ||
| ImageTag string `json:"imageTag" bson:"imageTag"` | ||
| FailureCase ScanFailureCase `json:"failureCase" bson:"failureCase"` | ||
| FailureReason string `json:"failureReason" bson:"failureReason"` | ||
| Timestamp time.Time `json:"timestamp" bson:"timestamp"` | ||
|
|
||
| // Registry scan context (no workloads). | ||
| RegistryName string `json:"registryName,omitempty" bson:"registryName,omitempty"` | ||
| IsRegistryScan bool `json:"isRegistryScan,omitempty" bson:"isRegistryScan,omitempty"` | ||
| } |
There was a problem hiding this comment.
Enforce registry/workload payload invariants in code, not only comments.
Line 47 documents strict rules (registry scan => no workloads + registryName set), but Line 49-Line 59 does not enforce them. Invalid combinations can be serialized and sent downstream.
Proposed guardrail (`Validate`) to prevent malformed reports
package scanfailure
-import "time"
+import (
+ "fmt"
+ "time"
+)
@@
type ScanFailureReport struct {
@@
IsRegistryScan bool `json:"isRegistryScan,omitempty" bson:"isRegistryScan,omitempty"`
}
+
+func (r ScanFailureReport) Validate() error {
+ if r.IsRegistryScan {
+ if r.RegistryName == "" {
+ return fmt.Errorf("registryName must be set when isRegistryScan is true")
+ }
+ if len(r.Workloads) > 0 {
+ return fmt.Errorf("workloads must be empty when isRegistryScan is true")
+ }
+ return nil
+ }
+ if len(r.Workloads) == 0 {
+ return fmt.Errorf("workloads must be set when isRegistryScan is false")
+ }
+ if r.RegistryName != "" {
+ return fmt.Errorf("registryName must be empty when isRegistryScan is false")
+ }
+ return nil
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scanfailure/types.go` around lines 48 - 59, Add a Validate method on
ScanFailureReport that enforces the documented invariants: if IsRegistryScan is
true then Workloads must be empty/nil and RegistryName must be non-empty, and if
IsRegistryScan is false then RegistryName must be empty (or optional) and
Workloads must be non-empty; the method should return an error describing which
invariant failed (use ScanFailureReport.Validate) and be called wherever reports
are constructed or serialized (e.g., before sending or marshalling) so malformed
reports cannot be emitted. Ensure the checks handle nil/zero slices and
trim/check RegistryName for emptiness and use clear error messages mentioning
the field names.
There was a problem hiding this comment.
Keeping the type as a plain data struct for now. Validation logic (registry vs workload invariants) lives in the consuming services (event-ingester, kubevuln) where the context for those rules exists. Adding it here would couple the contract type to service-level business rules. Happy to revisit if we see validation bugs downstream.
There was a problem hiding this comment.
@kooomix, that's a reasonable design boundary — keeping contract types as plain structs and pushing invariant enforcement into the services that own the construction context is a well-established pattern. Acknowledged, and thanks for the clear explanation.
✏️ Learnings added
Learnt from: kooomix
Repo: armosec/armoapi-go PR: 620
File: scanfailure/types.go:50-61
Timestamp: 2026-03-15T08:43:31.934Z
Learning: In the `armoapi-go` repository, the `scanfailure` package (file `scanfailure/types.go`) intentionally defines `ScanFailureReport` and related types as plain data structs with no validation logic. Registry vs. workload invariants (e.g., `IsRegistryScan=true` requires `RegistryName` set and empty `Workloads`) are enforced in the consuming services (`event-ingester`, `kubevuln`), not in the contract type itself. Do not suggest adding a `Validate()` method or other business-rule enforcement to this package.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Address review comments: add explicit zero-value constant for Go safety, and fix "OOM killed" → "OOM-killed" inconsistency in String(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
scanfailurepackage with scan failure report types (ScanFailureReport,ScanFailureCase,WorkloadIdentifier)/k8s/v2/vulnScanFailure(careportreceiver endpoint)kubescape/messagingtoarmoapi-goas the canonical home for API contract typesContext
Part of SUB-7074 (scan failure notifications). kubevuln (B.2) and node-agent (B.5) will import these types to report scan failures. All consuming repos already depend on armoapi-go — no new dependency edges.
Test plan
go build ./scanfailure/...passesSummary by CodeRabbit