Fix empty action field on IRI-mapped events#1500
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBucket, machine, and volume event mappers now ensure Kubernetes event actions are non-empty by substituting ChangesEvent action fallback fix
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
poollet/bucketpoollet/bem/bem.go (1)
56-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the action-fallback logic to a shared helper.
The identical 4-line fallback (
action := Spec.Action; if action == "" { action = Spec.Reason }) is duplicated verbatim inmem.goandvem.go. A small shared helper (e.g.,eventutils.ResolveAction(action, reason string) string) would avoid drift if the fallback logic needs to change later.Also note: if both
Spec.ActionandSpec.Reasonare empty,actionremains empty and the event could still be rejected byevents.k8s.io/v1. This appears to be accepted as out-of-scope per the PR description (follow-up planned for provider-utils/providers), so just flagging for awareness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@poollet/bucketpoollet/bem/bem.go` around lines 56 - 63, The action fallback logic in the event emission path is duplicated across bem, mem, and vem, so extract the repeated Spec.Action/Spec.Reason handling into a shared helper such as eventutils.ResolveAction(action, reason string) string. Update the Eventf call sites in the relevant event builders to use that helper instead of inlining the fallback, keeping the behavior unchanged for now and preserving the existing acceptance of empty action as out of scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@poollet/bucketpoollet/bem/bem.go`:
- Around line 56-63: The action fallback logic in the event emission path is
duplicated across bem, mem, and vem, so extract the repeated
Spec.Action/Spec.Reason handling into a shared helper such as
eventutils.ResolveAction(action, reason string) string. Update the Eventf call
sites in the relevant event builders to use that helper instead of inlining the
fallback, keeping the behavior unchanged for now and preserving the existing
acceptance of empty action as out of scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0942a1ec-f61f-420b-bbef-f9c643fa2bd6
📒 Files selected for processing (6)
poollet/bucketpoollet/bem/bem.gopoollet/bucketpoollet/bem/bem_test.gopoollet/machinepoollet/mem/mem.gopoollet/machinepoollet/mem/mem_test.gopoollet/volumepoollet/vem/vem.gopoollet/volumepoollet/vem/vem_test.go
The events.k8s.io/v1 API server rejects events with an empty action field. Providers do not always populate Spec.Action on IRI events, causing the poollet event mappers to push invalid events. Fall back to Spec.Reason when Spec.Action is empty. Contributes to #1489 Signed-off-by: Felix Riegger <felix.riegger@sap.com>
ef8b149 to
bd2d47f
Compare
|
What should happen if |
Works also, if there is nor Reason. It also makes it obvious, if we miss to change the sources to send a proper Action. Signed-off-by: Felix Riegger <felix.riegger@sap.com>
Good point, changed the implementation to fallback to |
The events.k8s.io/v1 API server rejects events with an empty action field. Providers do not always populate Spec.Action on IRI events, causing the poollet event mappers to push invalid events. Fall back to Spec.Reason when Spec.Action is empty.
Additional changes will update provider-utils and providers to correctly send the action field.
Contributes to #1489
Summary by CodeRabbit
actionvalue.actionnow defaults toUnknown.Reason,Message, andTypeare preserved whileActionis set toUnknownwhen the source action is empty.