feat(apis): add typed "patch" operatorAction to the Command contract

[matthyx]

Summary

• Adds OperatorActionPatch ("patch") to OperatorActionType, plus Patch/PatchType fields on OperatorActionArgs, matching the wire shape kubescape/operator PR uniq id to support multi jira #410 (feat(remediation): add generic patch action for arbitrary workload patches kubescape/operator#410) already validates and tests against.
• Removes the need for the operator's workaround: a local OperatorActionPatch constant and reading patch/patchType directly off the raw Command.Args map instead of through the typed struct.
• Purely the typed contract addition — no patch semantics, validation, or safety rails here; those stay in the operator repo (mainhandler/remediators/patch.go's Plan/canonicalizePatch). IsDryRun, ToArgs, and OperatorActionArgsFromMap are unchanged.
• Adds docs/features/operatoraction-patch-type.md per this repo's docs convention, including a flagged rollout hazard: the operator's extractPatchArgs currently also accepts an object-shaped patch value (not just a string), but handleOperatorAction parses the whole args map through the new typed struct first and will hard-error on an object-shaped patch once its armoapi-go pin picks this up. The operator-side fix (switch extractPatchArgs to read args.Patch/args.PatchType and require patch be a string) needs to land in the same bump — noted in the doc, not addressed in this PR since it's operator-repo scope.

Test plan

• go build ./...
• go test ./apis/... — includes new TestOperatorActionArgsRoundTripPatch, with explicit wire-key (patch/patchType) assertions
• go test ./... (full suite)

🤖 Generated with Claude Code

https://claude.ai/code/session_01KQHhDvFHPdpBmh2NyKxLLM

AI-skills: oh-my-claudecode:cancel | cmds: /oh-my-claudecode:autopilot

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 4fd63d52-467a-4358-bbce-0f663ed1d92c

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟡 Changes recommended

Adding Patch string introduces a backward-incompatible deserialization failure when patch arrives as an object, which can break consumers during rollout without an explicit compatibility strategy.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds first-class typed support for the operator’s "patch" remediation action to the apis command contract so producers/consumers can use OperatorActionArgs instead of reading patch/patchType from the raw Command.Args map.

Changes:

• Introduces OperatorActionPatch ("patch") in OperatorActionType.
• Extends OperatorActionArgs with Patch / PatchType fields (wire keys patch / patchType).
• Adds documentation and a round-trip unit test asserting the expected wire keys.

File summaries

File	Description
docs/features/operatoraction-patch-type.md	Documents the new typed "patch" operator action shape and rollout considerations.
apis/operatoraction.go	Adds the new OperatorActionPatch constant and Patch/PatchType fields on OperatorActionArgs.
apis/operatoraction_test.go	Adds a round-trip test for "patch" args and verifies wire keys.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Coverage Report for CI Build 34093429286

Warning

No base build found for commit 9392029 on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 48.443%

Details

• Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	5171
Covered Lines:	2505
Line Coverage:	48.44%
Coverage Strength:	6.05 hits per line

---

💛 - Coveralls

[jnathangreeg]

Reviewed against the consumer side — kubescape/operator#410 at a7fe5a2 — since this contract is only meaningful in terms of what the operator accepts.

The Go change itself is minimal and internally correct: ToArgs / OperatorActionArgsFromMap are generic marshal/unmarshal and need no change, both new fields are omitempty, the "strategic" / "merge" values in the godoc match the operator's PatchTypeStrategic / PatchTypeMerge constants exactly, nothing else in this repo switches on OperatorActionType, and the absent bson tags are consistent with the rest of the struct.

One thing is worth changing before this merges (the string typing), plus two documentation/example points.

[jnathangreeg]

Typing Patch as string turns a wire shape the operator already supports into a hard failure of the whole args parse. The operator's extractPatchArgs has an object branch (it re-marshals a map[string]any patch), but handleOperatorAction calls OperatorActionArgsFromMap as its first statement — so an object-shaped patch fails json.Unmarshal into this field and the command dies before extractPatchArgs is ever reached. And it isn't only the patch key that's lost: the entire args parse errors, so target, dryRun and reason go with it.

The PR body handles this with a documented two-repo rollout order, but nothing enforces it — bumping the armoapi-go pin in the operator compiles clean and only breaks at runtime, for whichever producer is still sending objects.

A tolerant UnmarshalJSON on this field (accept a JSON string or an object, re-marshalling the object into the string) makes the change genuinely additive and lets the two repos move independently.

[jnathangreeg]

This is the only worked example of a patch payload in the contract library, and it's one the operator will always reject: rejectContainerEscalation denylists container image unconditionally, so "image":"api:v2" (with Reason: "bump image to patched version") can never be applied. The test still passes because it only asserts JSON round-tripping.

Producers will copy this. Suggest the seccompProfile body this feature actually motivates, so the example is both a valid contract sample and an appliable patch.

[jnathangreeg]

"arbitrary" overstates it, and the RFC-6902 restriction is the only limit mentioned. The sole consumer also enforces a 256 KiB size cap, a non-empty-object shape, a 4-kind target allowlist, and a ~12-field escalation denylist (volumes, serviceAccountName, hostNetwork, metadata.ownerReferences, container image / privileged / capabilities.add, …). None of that is visible to someone writing a producer against this contract — worth either summarising the limits here or pointing at the operator-side validation.

[jnathangreeg]

Approving — 41938cbe addresses all three points, and the PatchBody shape is the right fix: the object case now round-trips instead of failing the whole args parse, so the two repos can move independently and the rollout ordering in the PR body stops being load-bearing.

Verified locally at 41938cbe: go test ./apis/... passes, go vet ./apis/... and go build ./... are clean, and no non-test code outside operatoraction.go touches .Patch, so the string → PatchBody change costs nothing elsewhere. The new TestOperatorActionArgsFromMapPatchAcceptsObjectShape asserts the part that matters — that target and patchType still parse alongside an object-shaped patch, not just the patch body itself.

One non-blocking nit inline, worth folding in whenever you next touch the file.

[jnathangreeg]

Non-blocking: this final branch accepts anything that isn't a JSON string, not just objects. Probed it at this commit:

input ["a"]   -> err=<nil> value="[\"a\"]"
input 123     -> err=<nil> value="123"
input true    -> err=<nil> value="true"

Meanwhile the field godoc still says "Must be object-shaped: RFC 6902 JSON Patch arrays are not supported" — which the old plain string field did enforce, since an array failed the unmarshal outright. Nothing breaks in practice (the operator's remediator rejects non-objects with a clear message, which is arguably a better error than a parse failure), but the type no longer backs the comment's claim.

Either if trimmed[0] != '{' { return fmt.Errorf(...) } here, or soften the godoc to say shape is validated by the consumer. No need to hold the merge for it.

[matthyx]

Good catch — fixed in 1342c40: the fallback branch now only accepts {-prefixed (object) tokens, rejecting arrays/numbers/booleans with an explicit error instead of silently accepting them. Doc comment already matched this intent; now the code backs it too.

[jnathangreeg]

Re-approving at 1342c40d — my earlier approval was dismissed by the push, so this replaces it.

The nit is properly closed, not papered over. Verified by probing UnmarshalJSON directly at this commit:

{"a":1}          -> ok, stored verbatim
"{\"a\":1}"      -> ok (pre-encoded string form)
["a"]            -> error: patch must be a JSON string or a JSON object, got "[\"a\"]"
123              -> error
true             -> error
null             -> "" (empty, as intended)
  {"a":1}        -> ok (leading whitespace handled)

go build ./..., go vet ./apis/... and go test ./apis/... are all clean, the rename to OperatorActionPatchBody leaves no stale PatchBody references anywhere in Go or docs, and it does read better next to OperatorActionTarget / OperatorActionSelector.

One thing I'd deliberately leave alone: the string branch still accepts any JSON string, so a pre-encoded "[1,2]" passes this layer. That's identical to the behaviour of the original plain string field, and validating the content of the encoded body belongs to the operator's remediator rather than here — noting it only so it isn't mistaken for an oversight later. No action needed.