Skip to content

refactor(rulemanager): reuse armoapi-go profileDataRequired schema#834

Open
matthyx wants to merge 2 commits into
mainfrom
feat/dedup-profiledata-to-armoapi
Open

refactor(rulemanager): reuse armoapi-go profileDataRequired schema#834
matthyx wants to merge 2 commits into
mainfrom
feat/dedup-profiledata-to-armoapi

Conversation

@matthyx

@matthyx matthyx commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Two commits:

  1. Schema dedup — replaces node-agent's duplicate profileDataRequired schema with type aliases to the canonical armoapi-go types (ProfileDataRequired/ProfileDataField/ProfileDataPattern). Defining the matcher once in armoapi-go — imported by node-agent (query side), storage (generation side / rule-aware collapse), and the backend (rules in MongoDB) — guarantees it can't drift between the side that records a profile and the side that queries it. Shape change: a profile-data surface is now a pointer (Opens *ProfileDataField); a nil pointer means "not declared" (the role the old FieldRequirement.Declared bool played). mergeField updated to pointer + nil-check; the schema's own test moves to armoapi-go.

  2. Dependency fix — resolves the transitive conflicts from the kubescape/storage v0.0.282 bump using additive replace directives only, leaving the three frozen replaces (syft, inspektor-gadget, cilium/ebpf) and the storage pin untouched.

Deps

  • armoapi-gov0.0.719 (profileDataRequired schema + UnionOpenProtection).
  • kubescape/storagev0.0.282.

Dependency resolution (was the blocker — now fixed)

The storage v0.0.282 bump pulled a newer transitive stack that broke the build against the frozen forks:

  • opencontainers/runtime-spec was forced to v1.3.0, which changed LinuxPids.Limit from int64 to *int64; containerd v1.7.32's oci/spec_opts.go assigns an int64 and stopped compiling.
  • anchore/stereoscope was forced to v0.1.22, whose docker/podman clients need a docker API (client.New, client.PingOptions) newer than the pinned docker v28.5.2; the frozen kubescape/syft fork expects stereoscope v0.1.9.

Resolved with two additive replaces pinning these transitives back to the versions the frozen set already uses:

replace github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.2.1
replace github.com/anchore/stereoscope => github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385

(runtime-spec v1.2.1 is what origin/main used; the stereoscope pseudo-version is the one the kubescape/syft v1.32.0-ks.2 fork requires.) The three frozen replaces and storage v0.0.282 are unchanged.

Verified

  • go build ./... — clean.
  • go test ./... -run='^$' — all test binaries compile.
  • Unit tests pass for the affected packages: pkg/objectcache/containerprofilecache (projection / mergeField) and pkg/rulemanager/cel/libraries/applicationprofile (was_path_opened).

Related

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR contains a schema type migration where ProfileDataRequired, FieldRequirement, and PatternObject are converted from local struct definitions to type aliases pointing to upstream armotypes equivalents, eliminating local custom marshaling logic. Projection compile code is adapted to work with pointer-based FieldRequirement. Additionally, the Go toolchain is bumped to 1.25.8 and the entire dependency graph is refreshed with newer versions across direct and indirect requires.

Changes

Type Schema Migration to armotypes

Layer / File(s) Summary
Profile data type alias migration
pkg/rulemanager/types/v1/profiledata.go
ProfileDataRequired, FieldRequirement, and PatternObject are converted to type aliases to armotypes equivalents. All custom JSON/YAML marshaling methods and local validation logic (including "known fields" enforcement and pattern object validation) are removed and delegated to the upstream library.
Projection compile refactoring for pointer FieldRequirement
pkg/objectcache/containerprofilecache/projection_compile.go, pkg/objectcache/containerprofilecache/projection_compile_test.go
mergeField function signature is updated to accept *typesv1.FieldRequirement instead of a value type, with explicit nil-awareness replacing the prior Declared boolean check. Test helpers fieldReqAll and fieldReqPatterns are updated to return pointers with the corresponding fields set, removing the prior Declared: true initialization.

Dependency Updates

Layer / File(s) Summary
Go toolchain and direct dependency updates
go.mod
Go toolchain is bumped to 1.25.8. Direct requires are refreshed for github.com/anchore/syft, github.com/armosec/armoapi-go, github.com/google/go-containerregistry, github.com/kubescape/k8s-interface, github.com/kubescape/storage, and github.com/sirupsen/logrus (to plain v1.9.4 tag). go.opentelemetry.io/otel/log/logtest is added as a direct require. gopkg.in/yaml.v3 moves from direct to indirect requires. modernc.org/sqlite is bumped.
Indirect dependency updates
go.mod
Large set of indirect transitive dependencies are refreshed: container tooling (github.com/containerd/*, github.com/docker/*), cryptography (github.com/ProtonMail/go-crypto, github.com/Microsoft/hcsshim), Kubernetes modules (k8s.io/* at v0.35.0), data formats (gopkg.in/yaml.v3 now indirect, github.com/github/go-spdx/v2, github.com/bmatcuk/doublestar/v4), and various supporting libraries. go.opentelemetry.io/otel/log/logtest is removed from the indirect list.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A schema freed from local chains,
Now points to armotypes' terrain,
Pointers dance where booleans once led,
Dependencies refreshed, upgraded ahead,
Clean code, clear path, lighter than bread!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main refactoring work: replacing duplicate profileDataRequired schema with type aliases to armoapi-go, which is the primary change across all modified files.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/dedup-profiledata-to-armoapi

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@go.mod`:
- Line 188: The go.mod currently pulls github.com/containerd/containerd/v2 at
v2.2.1 which is in the vulnerable range for GHSA-fqw6-gf59-qr4w; update the
dependency graph so containerd is at least v2.2.4 (the first patched release).
Concretely, in go.mod either update the direct dependency that transitively
requires github.com/containerd/containerd/v2 to a version that resolves to
v2.2.4+ or add a module replace/require for github.com/containerd/containerd/v2
=> github.com/containerd/containerd/v2 v2.2.4 (or later) and run go mod tidy to
regenerate go.sum and ensure the new version is used.
- Line 209: The go.mod currently pins the indirect module
github.com/docker/docker v28.5.2+incompatible which contains HIGH-severity
vulnerabilities; update the dependency to at least v29.3.1 by replacing or
bumping github.com/docker/docker to >= v29.3.1 (and ideally a later patched
release) and then run go mod tidy to resolve the graph—if the direct introducer
is github.com/kubescape/node-agent/pkg/containerwatcher/v2 or
github.com/inspektor-gadget/inspektor-gadget/pkg/container-utils/docker, update
those modules (or add a replace directive for github.com/docker/docker) so the
resolved version is >= v29.3.1 and re-run tests/build to ensure compatibility.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe36ae3a-968a-4a99-9fab-e6b8c2f19cdd

📥 Commits

Reviewing files that changed from the base of the PR and between 7edbd72 and 7bb0c7a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • go.mod
  • pkg/objectcache/containerprofilecache/projection_compile.go
  • pkg/objectcache/containerprofilecache/projection_compile_test.go
  • pkg/rulemanager/types/v1/profiledata.go
  • pkg/rulemanager/types/v1/profiledata_test.go
💤 Files with no reviewable changes (1)
  • pkg/rulemanager/types/v1/profiledata_test.go

Comment thread go.mod Outdated
github.com/containerd/containerd v1.7.32 // indirect
github.com/containerd/containerd/api v1.9.0 // indirect
github.com/containerd/containerd/api v1.10.0 // indirect
github.com/containerd/containerd/v2 v2.2.1 // indirect

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for containerd security advisories and latest version

gh api graphql -f query='
{
  securityVulnerabilities(first: 10, ecosystem: GO, package: "github.com/containerd/containerd/v2") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Repository: kubescape/node-agent

Length of output: 2602


Update github.com/containerd/containerd/v2 to fix GHSA-fqw6-gf59-qr4w (runAsNonRoot evasion).

go.mod currently pins github.com/containerd/containerd/v2 v2.2.1 (indirect), which falls in the vulnerable range >= 2.1.0-beta.0, < 2.2.4; the first patched version is 2.2.4. Update your dependency graph (via updating the direct dependency chain or adding a replace) to pull in containerd/v2 >= 2.2.4.

🧰 Tools
🪛 OSV Scanner (2.3.8)

[HIGH] 188-188: github.com/containerd/containerd/v2 2.2.1: containerd user ID handling bypass allows runAsNonRoot evasion

(GHSA-fqw6-gf59-qr4w)

🤖 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 `@go.mod` at line 188, The go.mod currently pulls
github.com/containerd/containerd/v2 at v2.2.1 which is in the vulnerable range
for GHSA-fqw6-gf59-qr4w; update the dependency graph so containerd is at least
v2.2.4 (the first patched release). Concretely, in go.mod either update the
direct dependency that transitively requires github.com/containerd/containerd/v2
to a version that resolves to v2.2.4+ or add a module replace/require for
github.com/containerd/containerd/v2 => github.com/containerd/containerd/v2
v2.2.4 (or later) and run go mod tidy to regenerate go.sum and ensure the new
version is used.

Source: Linters/SAST tools

Comment thread go.mod
github.com/docker/cli v29.2.0+incompatible // indirect
github.com/docker/cli v29.3.0+incompatible // indirect
github.com/docker/distribution v2.8.3+incompatible // indirect
github.com/docker/docker v28.5.2+incompatible // indirect

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for Docker security advisories and patched versions

gh api graphql -f query='
{
  securityVulnerabilities(first: 15, ecosystem: GO, package: "github.com/docker/docker") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

echo "---"
echo "Checking which direct dependencies pull in docker/docker:"
rg -n "github.com/docker/docker" go.mod

Repository: kubescape/node-agent

Length of output: 3625


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== go.mod entries =="
rg -n "github.com/docker/docker" go.mod

echo
echo "== go mod why (dependency chain) =="
go mod why -m github.com/docker/docker

echo
echo "== modules that import/require docker/docker (small graph slice) =="
# Print only relevant edges around docker/docker
go mod graph | rg "github.com/docker/docker@" | head -n 50 || true

Repository: kubescape/node-agent

Length of output: 8415


major: Update vulnerable github.com/docker/docker v28.5.2+incompatible (indirect) pulled via inspektor-gadget

go.mod includes github.com/docker/docker v28.5.2+incompatible // indirect and go mod why -m github.com/docker/docker traces it to github.com/kubescape/node-agent/pkg/containerwatcher/v2github.com/inspektor-gadget/inspektor-gadget/pkg/container-utils/dockergithub.com/docker/docker/api/types/container.

GitHub advisory data shows HIGH issues affecting this version:

  • AuthZ plugin bypass with oversized request bodies: fixed in 29.3.1 (< 29.3.1)
  • docker cp race condition (bind mount redirection to host path): affects <= 28.5.2; no patched version listed in advisory data
  • PUT /containers/{id}/archive executes container binary on the host: affects <= 28.5.2; no patched version listed in advisory data

Update github.com/docker/docker to >= 29.3.1 at minimum, and bump further once the upstream patched versions for the docker cp and PUT .../archive HIGH issues are identified.

🧰 Tools
🪛 OSV Scanner (2.3.8)

[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Moby has an Off-by-one error in its plugin privilege validation in github.com/docker/docker

(GO-2026-4883)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Moby has AuthZ plugin bypass when provided oversized request bodies in github.com/docker/docker

(GO-2026-4887)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Moby has an Off-by-one error in its plugin privilege validation

(GHSA-pxq6-2prw-chj9)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Docker: Race condition in docker cp allows bind mount redirection to host path

(GHSA-rg2x-37c3-w2rh)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Docker: Race condition in docker cp allows creation of arbitrary empty files on the host via symlink swap

(GHSA-vp62-88p7-qqf5)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Moby has AuthZ plugin bypass when provided oversized request bodies

(GHSA-x744-4wpc-v9h2)


[HIGH] 209-209: github.com/docker/docker 28.5.2+incompatible: Docker: PUT /containers/{id}/archive executes container binary on the host

(GHSA-x86f-5xw2-fm2r)

🤖 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 `@go.mod` at line 209, The go.mod currently pins the indirect module
github.com/docker/docker v28.5.2+incompatible which contains HIGH-severity
vulnerabilities; update the dependency to at least v29.3.1 by replacing or
bumping github.com/docker/docker to >= v29.3.1 (and ideally a later patched
release) and then run go mod tidy to resolve the graph—if the direct introducer
is github.com/kubescape/node-agent/pkg/containerwatcher/v2 or
github.com/inspektor-gadget/inspektor-gadget/pkg/container-utils/docker, update
those modules (or add a replace directive for github.com/docker/docker) so the
resolved version is >= v29.3.1 and re-run tests/build to ensure compatibility.

Source: Linters/SAST tools

Replace node-agent's duplicate profileDataRequired schema
(ProfileDataRequired/FieldRequirement/PatternObject + custom marshalling) with
type aliases to the canonical armoapi-go/armotypes types
(ProfileDataRequired/ProfileDataField/ProfileDataPattern). Defining the matcher
once in armoapi-go — imported by node-agent (query side), storage (generation
side / rule-aware collapse), and the backend (rules in MongoDB) — guarantees it
can never drift between the side that records a profile and the side that
queries it.

Shape change: a profile-data surface is now a *pointer* (Opens *ProfileDataField);
a nil pointer means "this rule does not declare this surface", the role the old
FieldRequirement.Declared bool played. mergeField is updated accordingly
(pointer + nil check); the schema's own test moves to armoapi-go.

Deps:
- armoapi-go -> v0.0.719 (profileDataRequired schema + UnionOpenProtection).
- kubescape/storage -> v0.0.282.

Note: the storage v0.0.282 bump pulls a newer SBOM/containerd stack
(stereoscope v0.1.22, containerd/api v1.10.0) that does not yet compile against
node-agent's pinned containerd v1.7.32 and kubescape/syft fork — a separate
coordinated dependency update, tracked independently of this schema refactor.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@matthyx matthyx force-pushed the feat/dedup-profiledata-to-armoapi branch from 7bb0c7a to d972a07 Compare June 11, 2026 08:26
…places only)

Bumping kubescape/storage to v0.0.282 pulled a newer transitive stack that broke
the build against the frozen forks/replaces:

- opencontainers/runtime-spec was forced to v1.3.0, which changed
  LinuxPids.Limit from int64 to *int64; containerd v1.7.32's oci/spec_opts.go
  assigns an int64 and no longer compiles.
- anchore/stereoscope was forced to v0.1.22, whose docker/podman clients need a
  docker client API (client.New, client.PingOptions) newer than the pinned
  docker v28.5.2; the frozen kubescape/syft fork expects stereoscope v0.1.9.

Resolve with two additive replace directives pinning these transitives back to
the versions the frozen set already uses (runtime-spec v1.2.1 — the version
origin/main used — and the stereoscope v0.1.9 pseudo-version the kubescape/syft
fork requires). No change to the three frozen replaces (syft, inspektor-gadget,
cilium/ebpf) or to the storage v0.0.282 pin.

go build ./... is clean, all test binaries compile, and the affected unit tests
(containerprofilecache projection, applicationprofile CEL) pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@matthyx

matthyx commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Review — no merge blockers. CI is fully green (build-and-push + all 24 component tests) and the refactor is sound. One non-blocking gap and a deps note below.

✅ Verified

  • Pointer migration is internally consistent. In armoapi-go v0.0.719, ProfileDataRequired.Opens … IngressAddresses are all *ProfileDataField, so mergeField(dst, src *typesv1.FieldRequirement) with the src == nil check correctly takes over the old Declared bool's role, and the call sites need no edit (the field type itself went value→pointer). The FieldRequirement = armotypes.ProfileDataField / PatternObject = armotypes.ProfileDataPattern aliases line up with the upstream definitions.
  • Dependency resolution. The two additive replaces (runtime-spec → v1.2.1, stereoscope → v0.1.9-…ef061ea) pin the transitives back to what the frozen syft/inspektor-gadget/ebpf set already uses; the three frozen replaces and the storage v0.0.282 pin are untouched.

⚠️ Non-blocking — load-time validation is now silently weaker

The deleted local schema rejected malformed profileDataRequired at unmarshal time: unknown top-level surface keys, unknown keys inside a pattern object, and multi-key/empty pattern objects all errored on decode. armoapi-go folds that logic into an explicit ProfileDataRequired.Validate() — whose own doc calls it "the single source of truth for both the rulelibrary lint and node-agent's load-time check" — but node-agent never calls Validate() on the load path (rule_manager.go / config.go only check presence/profileDependency, not schema).

Net effect: ProfileDataField shape errors are still caught (its UnmarshalJSON/UnmarshalYAML survive in armoapi-go), but a typo'd surface name (e.g. opnes:) or a malformed pattern object is now silently dropped instead of rejected — which can quietly empty a rule's projection and turn into a missed detection. The upstream rulelibrary lint still validates, so this is defense-in-depth rather than a correctness break. If you want to preserve node-agent's prior strictness, add a pdr.Validate() call where rules are loaded.

ℹ️ On CodeRabbit's two security findings

docker v28.5.2 (HIGH) and the containerd GHSA-fqw6-gf59-qr4w note are transitive and not introduced by this PR — docker stays pinned at v28.5.2 by the frozen syft/docker set, unchanged here. Out of scope for this refactor; worth tracking separately rather than blocking the merge.

@github-actions

Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.209 0.204 -2.2%
Peak CPU (cores) 0.224 0.214 -4.6%
Avg Memory (MiB) 329.727 318.546 -3.4%
Peak Memory (MiB) 332.254 326.387 -1.8%
Dedup Effectiveness

No data available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant