From bd5f34c921a6691320a76b9c9995b9e74a2693aa Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Wed, 10 Jun 2026 21:16:20 +0200 Subject: [PATCH 1/4] refactor(rulemanager): reuse armoapi-go profileDataRequired schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: Matthias Bertschy --- go.mod | 2 +- .../projection_compile.go | 9 +- .../projection_compile_test.go | 11 +- .../projection_golden_test.go | 8 +- pkg/rulemanager/types/v1/profiledata.go | 229 ++------------- pkg/rulemanager/types/v1/profiledata_test.go | 264 ------------------ 6 files changed, 35 insertions(+), 488 deletions(-) delete mode 100644 pkg/rulemanager/types/v1/profiledata_test.go diff --git a/go.mod b/go.mod index 475afc7f7d..e4342920f1 100644 --- a/go.mod +++ b/go.mod @@ -68,7 +68,6 @@ require ( google.golang.org/grpc v1.82.1 google.golang.org/protobuf v1.36.11 gopkg.in/mcuadros/go-syslog.v2 v2.3.0 - gopkg.in/yaml.v3 v3.0.1 istio.io/pkg v0.0.0-20231221211216-7635388a563e k8s.io/api v0.35.0 k8s.io/apimachinery v0.35.0 @@ -452,6 +451,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.35.0 // indirect k8s.io/apiserver v0.35.0 // indirect k8s.io/cli-runtime v0.35.0 // indirect diff --git a/pkg/objectcache/containerprofilecache/projection_compile.go b/pkg/objectcache/containerprofilecache/projection_compile.go index 74f934b8d7..8c6f6cbd13 100644 --- a/pkg/objectcache/containerprofilecache/projection_compile.go +++ b/pkg/objectcache/containerprofilecache/projection_compile.go @@ -54,9 +54,12 @@ func CompileSpec(rules []typesv1.Rule) objectcache.RuleProjectionSpec { return spec } -// mergeField unions one rule's FieldRequirement into the accumulator FieldSpec. -func mergeField(dst *objectcache.FieldSpec, src typesv1.FieldRequirement) { - if !src.Declared { +// mergeField unions one rule's profile-data field into the accumulator +// FieldSpec. src is nil when the rule does not declare this surface (the role +// the old FieldRequirement.Declared bool played before the schema moved to +// armoapi-go). +func mergeField(dst *objectcache.FieldSpec, src *typesv1.FieldRequirement) { + if src == nil { return } dst.InUse = true diff --git a/pkg/objectcache/containerprofilecache/projection_compile_test.go b/pkg/objectcache/containerprofilecache/projection_compile_test.go index fa73e4c0e8..9fa11d83dc 100644 --- a/pkg/objectcache/containerprofilecache/projection_compile_test.go +++ b/pkg/objectcache/containerprofilecache/projection_compile_test.go @@ -17,14 +17,15 @@ func makeRule(pdr *typesv1.ProfileDataRequired) typesv1.Rule { } } -// fieldReqAll returns a FieldRequirement that requests all entries. -func fieldReqAll() typesv1.FieldRequirement { - return typesv1.FieldRequirement{Declared: true, All: true} +// fieldReqAll returns a FieldRequirement that requests all entries. A non-nil +// pointer marks the surface as declared (the old Declared bool's role). +func fieldReqAll() *typesv1.FieldRequirement { + return &typesv1.FieldRequirement{All: true} } // fieldReqPatterns returns a FieldRequirement with the supplied patterns. -func fieldReqPatterns(patterns ...typesv1.PatternObject) typesv1.FieldRequirement { - return typesv1.FieldRequirement{Declared: true, Patterns: patterns} +func fieldReqPatterns(patterns ...typesv1.PatternObject) *typesv1.FieldRequirement { + return &typesv1.FieldRequirement{Patterns: patterns} } func exactPattern(path string) typesv1.PatternObject { diff --git a/pkg/objectcache/containerprofilecache/projection_golden_test.go b/pkg/objectcache/containerprofilecache/projection_golden_test.go index bea516196c..f22dcb153a 100644 --- a/pkg/objectcache/containerprofilecache/projection_golden_test.go +++ b/pkg/objectcache/containerprofilecache/projection_golden_test.go @@ -119,14 +119,14 @@ func toGolden(pcp *objectcache.ProjectedContainerProfile, tree *callstackcache.C // --- fixture construction helpers --- // declaredAll returns a FieldRequirement declaring the whole surface. -func declaredAll() typesv1.FieldRequirement { - return typesv1.FieldRequirement{Declared: true, All: true} +func declaredAll() *typesv1.FieldRequirement { + return &typesv1.FieldRequirement{All: true} } // declaredPatterns returns a FieldRequirement declaring a set of pattern // selectors (exact / prefix / suffix / contains). -func declaredPatterns(pats ...typesv1.PatternObject) typesv1.FieldRequirement { - return typesv1.FieldRequirement{Declared: true, Patterns: pats} +func declaredPatterns(pats ...typesv1.PatternObject) *typesv1.FieldRequirement { + return &typesv1.FieldRequirement{Patterns: pats} } // linearCallStack builds a single-path identified call stack from an ordered diff --git a/pkg/rulemanager/types/v1/profiledata.go b/pkg/rulemanager/types/v1/profiledata.go index ffb3172d39..2ccfeb5168 100644 --- a/pkg/rulemanager/types/v1/profiledata.go +++ b/pkg/rulemanager/types/v1/profiledata.go @@ -1,214 +1,21 @@ package types -import ( - "encoding/json" - "fmt" - - "gopkg.in/yaml.v3" +import "github.com/armosec/armoapi-go/armotypes" + +// The profileDataRequired schema (the type, its match patterns, and the custom +// JSON/YAML/BSON (un)marshalling) lives in armoapi-go/armotypes — the single +// module imported by every consumer: node-agent (this query side: projection / +// was_path_opened), storage (the generation side: rule-aware collapse), and the +// backend (rules persisted in MongoDB). Defining it once there guarantees the +// matcher can never drift between the side that records a profile and the side +// that queries it. +// +// These aliases preserve node-agent's historical type names. Note the shape +// change versus the old node-agent-local schema: a surface is now a *pointer* +// (ProfileDataRequired.Opens is *ProfileDataField); a nil pointer means "this +// rule does not declare this surface" — the role the old `Declared` bool played. +type ( + ProfileDataRequired = armotypes.ProfileDataRequired + FieldRequirement = armotypes.ProfileDataField + PatternObject = armotypes.ProfileDataPattern ) - -// ProfileDataRequired declares the per-rule profile fields the rule queries. -// Nil means the rule reads no profile data. -type ProfileDataRequired struct { - Opens FieldRequirement `json:"opens" yaml:"opens,omitempty"` - Execs FieldRequirement `json:"execs" yaml:"execs,omitempty"` - Capabilities FieldRequirement `json:"capabilities" yaml:"capabilities,omitempty"` - Syscalls FieldRequirement `json:"syscalls" yaml:"syscalls,omitempty"` - Endpoints FieldRequirement `json:"endpoints" yaml:"endpoints,omitempty"` - EgressDomains FieldRequirement `json:"egressDomains" yaml:"egressDomains,omitempty"` - EgressAddresses FieldRequirement `json:"egressAddresses" yaml:"egressAddresses,omitempty"` - IngressDomains FieldRequirement `json:"ingressDomains" yaml:"ingressDomains,omitempty"` - IngressAddresses FieldRequirement `json:"ingressAddresses" yaml:"ingressAddresses,omitempty"` -} - -var profileDataRequiredKnownFields = map[string]bool{ - "opens": true, "execs": true, "capabilities": true, - "syscalls": true, "endpoints": true, - "egressDomains": true, "egressAddresses": true, - "ingressDomains": true, "ingressAddresses": true, -} - -// UnmarshalJSON rejects unknown fields. -func (p *ProfileDataRequired) UnmarshalJSON(data []byte) error { - *p = ProfileDataRequired{} // reset to avoid stale state if receiver is reused - var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - for k := range raw { - if !profileDataRequiredKnownFields[k] { - return fmt.Errorf("profileDataRequired: unknown field %q", k) - } - } - type plain ProfileDataRequired - return json.Unmarshal(data, (*plain)(p)) -} - -// UnmarshalYAML rejects unknown fields. -func (p *ProfileDataRequired) UnmarshalYAML(value *yaml.Node) error { - *p = ProfileDataRequired{} // reset to avoid stale state if receiver is reused - if value.Kind == yaml.MappingNode { - for i := 0; i < len(value.Content)-1; i += 2 { - key := value.Content[i].Value - if !profileDataRequiredKnownFields[key] { - return fmt.Errorf("profileDataRequired: unknown field %q", key) - } - } - } - type plain ProfileDataRequired - return value.Decode((*plain)(p)) -} - -// FieldRequirement is the per-field declaration. After unmarshalling, exactly -// one of (All, Patterns) is meaningful. Declared=true when the YAML key was -// present, letting the spec compiler distinguish absent-from-this-rule vs -// explicitly declared. -type FieldRequirement struct { - All bool - Patterns []PatternObject - Declared bool -} - -// PatternObject — exactly one of {Exact, Prefix, Suffix, Contains} is non-empty. -// Multi-key or empty objects are rejected at unmarshal time. -type PatternObject struct { - Exact string `json:"exact,omitempty" yaml:"exact,omitempty"` - Prefix string `json:"prefix,omitempty" yaml:"prefix,omitempty"` - Suffix string `json:"suffix,omitempty" yaml:"suffix,omitempty"` - Contains string `json:"contains,omitempty" yaml:"contains,omitempty"` -} - -var patternObjectKnownFields = map[string]bool{ - "exact": true, "prefix": true, "suffix": true, "contains": true, -} - -// UnmarshalJSON rejects unknown fields in a PatternObject so typos in rule -// YAML/JSON are caught at load time rather than silently ignored. -func (p *PatternObject) UnmarshalJSON(data []byte) error { - var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - for k := range raw { - if !patternObjectKnownFields[k] { - return fmt.Errorf("PatternObject: unknown field %q", k) - } - } - type plain PatternObject - return json.Unmarshal(data, (*plain)(p)) -} - -// UnmarshalYAML rejects unknown fields in a PatternObject. -func (p *PatternObject) UnmarshalYAML(value *yaml.Node) error { - if value.Kind == yaml.MappingNode { - for i := 0; i < len(value.Content)-1; i += 2 { - key := value.Content[i].Value - if !patternObjectKnownFields[key] { - return fmt.Errorf("PatternObject: unknown field %q", key) - } - } - } - type plain PatternObject - return value.Decode((*plain)(p)) -} - -// validate checks that exactly one field is set. -func (p PatternObject) validate() error { - count := 0 - if p.Exact != "" { - count++ - } - if p.Prefix != "" { - count++ - } - if p.Suffix != "" { - count++ - } - if p.Contains != "" { - count++ - } - if count == 0 { - return fmt.Errorf("PatternObject must have exactly one non-empty field (exact/prefix/suffix/contains), got none") - } - if count > 1 { - return fmt.Errorf("PatternObject must have exactly one non-empty field (exact/prefix/suffix/contains), got %d", count) - } - return nil -} - -// UnmarshalJSON for FieldRequirement: accepts the string "all" or a non-empty -// JSON array of PatternObject. -func (f *FieldRequirement) UnmarshalJSON(data []byte) error { - *f = FieldRequirement{} // reset to clear any stale All/Patterns before decode - f.Declared = true - - // Try string "all" - var s string - if err := json.Unmarshal(data, &s); err == nil { - if s != "all" { - return fmt.Errorf("FieldRequirement string value must be \"all\", got %q", s) - } - f.All = true - return nil - } - - // Try array of PatternObject - var patterns []PatternObject - if err := json.Unmarshal(data, &patterns); err != nil { - return fmt.Errorf("FieldRequirement must be \"all\" or a list of pattern objects: %w", err) - } - if len(patterns) == 0 { - return fmt.Errorf("FieldRequirement pattern list must be non-empty; use \"all\" to retain all entries") - } - for i, p := range patterns { - if err := p.validate(); err != nil { - return fmt.Errorf("FieldRequirement[%d]: %w", i, err) - } - } - f.Patterns = patterns - return nil -} - -// MarshalJSON for FieldRequirement: emits "all" or the pattern list. -func (f FieldRequirement) MarshalJSON() ([]byte, error) { - if !f.Declared { - return []byte("null"), nil - } - if f.All { - return []byte(`"all"`), nil - } - return json.Marshal(f.Patterns) -} - -// UnmarshalYAML for FieldRequirement: accepts the string "all" or a non-empty -// sequence of pattern objects. -func (f *FieldRequirement) UnmarshalYAML(unmarshal func(any) error) error { - *f = FieldRequirement{} // reset to clear any stale All/Patterns before decode - f.Declared = true - - // Try string first. - var s string - if err := unmarshal(&s); err == nil { - if s != "all" { - return fmt.Errorf("FieldRequirement string value must be \"all\", got %q", s) - } - f.All = true - return nil - } - - // Try slice of PatternObject. - var patterns []PatternObject - if err := unmarshal(&patterns); err != nil { - return fmt.Errorf("FieldRequirement must be \"all\" or a list of pattern objects: %w", err) - } - if len(patterns) == 0 { - return fmt.Errorf("FieldRequirement pattern list must be non-empty; use \"all\" to retain all entries") - } - for i, p := range patterns { - if err := p.validate(); err != nil { - return fmt.Errorf("FieldRequirement[%d]: %w", i, err) - } - } - f.Patterns = patterns - return nil -} diff --git a/pkg/rulemanager/types/v1/profiledata_test.go b/pkg/rulemanager/types/v1/profiledata_test.go deleted file mode 100644 index b8e7b599d4..0000000000 --- a/pkg/rulemanager/types/v1/profiledata_test.go +++ /dev/null @@ -1,264 +0,0 @@ -package types - -import ( - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" -) - -// --- YAML unmarshaling tests --- - -// TestProfileDataRequired_Unmarshal_AllString verifies that the string "all" -// unmarshals to FieldRequirement{Declared:true, All:true}. -func TestProfileDataRequired_Unmarshal_AllString(t *testing.T) { - input := `opens: all` - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(input), &pdr) - require.NoError(t, err) - - assert.True(t, pdr.Opens.Declared, "Declared should be true when field is present in YAML") - assert.True(t, pdr.Opens.All, "All should be true when value is 'all'") - assert.Empty(t, pdr.Opens.Patterns, "Patterns should be empty when value is 'all'") -} - -// TestProfileDataRequired_Unmarshal_Patterns verifies that a list of pattern -// objects unmarshals correctly. -func TestProfileDataRequired_Unmarshal_Patterns(t *testing.T) { - input := ` -opens: - - exact: /bin/sh - - prefix: /usr/ -` - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(input), &pdr) - require.NoError(t, err) - - assert.True(t, pdr.Opens.Declared) - assert.False(t, pdr.Opens.All) - require.Len(t, pdr.Opens.Patterns, 2, "should have two pattern entries") - - // Find exact and prefix entries (order may vary). - var exactFound, prefixFound bool - for _, p := range pdr.Opens.Patterns { - if p.Exact == "/bin/sh" { - exactFound = true - } - if p.Prefix == "/usr/" { - prefixFound = true - } - } - assert.True(t, exactFound, "exact /bin/sh pattern should be present") - assert.True(t, prefixFound, "prefix /usr/ pattern should be present") -} - -// TestProfileDataRequired_Unmarshal_NilField verifies that an omitted field -// results in Declared=false. -func TestProfileDataRequired_Unmarshal_NilField(t *testing.T) { - // Only opens is specified; syscalls is omitted. - input := `opens: all` - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(input), &pdr) - require.NoError(t, err) - - assert.False(t, pdr.Syscalls.Declared, "omitted syscalls field should have Declared=false") - assert.False(t, pdr.Execs.Declared, "omitted execs field should have Declared=false") -} - -// TestProfileDataRequired_Unmarshal_InvalidPattern verifies that a pattern -// object with two fields is rejected at unmarshal time. -func TestProfileDataRequired_Unmarshal_InvalidPattern(t *testing.T) { - input := ` -opens: - - exact: /a - prefix: /b -` - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(input), &pdr) - assert.Error(t, err, "a PatternObject with two fields (exact+prefix) should return an error") -} - -// TestProfileDataRequired_Unmarshal_ValidateSingleField verifies that each -// single-field PatternObject variant is accepted. -func TestProfileDataRequired_Unmarshal_ValidateSingleField(t *testing.T) { - cases := []struct { - name string - input string - }{ - {name: "exact", input: "opens:\n - exact: /bin/sh"}, - {name: "prefix", input: "opens:\n - prefix: /usr/"}, - {name: "suffix", input: "opens:\n - suffix: .log"}, - {name: "contains", input: "opens:\n - contains: http"}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(tc.input), &pdr) - require.NoError(t, err, "single-field pattern %q should be valid", tc.name) - assert.True(t, pdr.Opens.Declared) - require.Len(t, pdr.Opens.Patterns, 1) - }) - } -} - -// TestProfileDataRequired_Unmarshal_TwoFieldsInOneObject verifies that a pattern -// object with more than one non-empty field is rejected. -func TestProfileDataRequired_Unmarshal_TwoFieldsInOneObject(t *testing.T) { - cases := []struct { - name string - input string - }{ - { - name: "exact+prefix", - input: "opens:\n - exact: /a\n prefix: /b", - }, - { - name: "suffix+contains", - input: "opens:\n - suffix: .log\n contains: http", - }, - { - name: "exact+suffix", - input: "opens:\n - exact: /bin/sh\n suffix: .sh", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(tc.input), &pdr) - assert.Error(t, err, "multi-field PatternObject %q should be rejected", tc.name) - }) - } -} - -// TestProfileDataRequired_Unmarshal_AllFields verifies that all field names in -// ProfileDataRequired can be round-tripped from YAML. -func TestProfileDataRequired_Unmarshal_AllFields(t *testing.T) { - input := ` -opens: all -execs: - - prefix: /usr/ -capabilities: all -syscalls: - - contains: read -endpoints: all -egressDomains: - - exact: example.com -egressAddresses: - - prefix: 10.0. -ingressDomains: all -ingressAddresses: - - suffix: .local -` - var pdr ProfileDataRequired - err := yaml.Unmarshal([]byte(input), &pdr) - require.NoError(t, err) - - assert.True(t, pdr.Opens.All) - assert.True(t, pdr.Execs.Declared) - assert.False(t, pdr.Execs.All) - require.Len(t, pdr.Execs.Patterns, 1) - assert.Equal(t, "/usr/", pdr.Execs.Patterns[0].Prefix) - - assert.True(t, pdr.Capabilities.All) - assert.True(t, pdr.Syscalls.Declared) - require.Len(t, pdr.Syscalls.Patterns, 1) - assert.Equal(t, "read", pdr.Syscalls.Patterns[0].Contains) - - assert.True(t, pdr.Endpoints.All) - assert.True(t, pdr.EgressDomains.Declared) - assert.Equal(t, "example.com", pdr.EgressDomains.Patterns[0].Exact) - assert.Equal(t, "10.0.", pdr.EgressAddresses.Patterns[0].Prefix) - assert.True(t, pdr.IngressDomains.All) - assert.Equal(t, ".local", pdr.IngressAddresses.Patterns[0].Suffix) -} - -// --- JSON unmarshaling tests --- - -// TestFieldRequirement_JSON_AllString verifies JSON "all" string unmarshaling. -func TestFieldRequirement_JSON_AllString(t *testing.T) { - data := `{"opens": "all"}` - var pdr ProfileDataRequired - err := json.Unmarshal([]byte(data), &pdr) - require.NoError(t, err) - - assert.True(t, pdr.Opens.Declared) - assert.True(t, pdr.Opens.All) -} - -// TestFieldRequirement_JSON_Patterns verifies JSON pattern list unmarshaling. -func TestFieldRequirement_JSON_Patterns(t *testing.T) { - data := `{"opens": [{"exact": "/bin/sh"}, {"prefix": "/usr/"}]}` - var pdr ProfileDataRequired - err := json.Unmarshal([]byte(data), &pdr) - require.NoError(t, err) - - assert.True(t, pdr.Opens.Declared) - assert.False(t, pdr.Opens.All) - require.Len(t, pdr.Opens.Patterns, 2) -} - -// TestFieldRequirement_JSON_InvalidString verifies that a non-"all" string -// value is rejected. -func TestFieldRequirement_JSON_InvalidString(t *testing.T) { - data := `{"opens": "some"}` - var pdr ProfileDataRequired - err := json.Unmarshal([]byte(data), &pdr) - assert.Error(t, err, `string value other than "all" should be rejected`) -} - -// TestFieldRequirement_JSON_TwoFieldPattern verifies that a multi-field pattern -// object is rejected during JSON unmarshaling. -func TestFieldRequirement_JSON_TwoFieldPattern(t *testing.T) { - data := `{"opens": [{"exact": "/a", "prefix": "/b"}]}` - var pdr ProfileDataRequired - err := json.Unmarshal([]byte(data), &pdr) - assert.Error(t, err, "multi-field PatternObject should be rejected in JSON") -} - -// TestFieldRequirement_MarshalJSON_All verifies that MarshalJSON for All=true -// emits the string "all". -func TestFieldRequirement_MarshalJSON_All(t *testing.T) { - f := FieldRequirement{Declared: true, All: true} - data, err := json.Marshal(f) - require.NoError(t, err) - assert.Equal(t, `"all"`, string(data)) -} - -// TestFieldRequirement_MarshalJSON_NotDeclared verifies that MarshalJSON for -// Declared=false emits null. -func TestFieldRequirement_MarshalJSON_NotDeclared(t *testing.T) { - f := FieldRequirement{Declared: false} - data, err := json.Marshal(f) - require.NoError(t, err) - assert.Equal(t, `null`, string(data)) -} - -// TestFieldRequirement_MarshalJSON_Patterns verifies that MarshalJSON for -// pattern lists emits the correct JSON array. -func TestFieldRequirement_MarshalJSON_Patterns(t *testing.T) { - f := FieldRequirement{ - Declared: true, - Patterns: []PatternObject{ - {Exact: "/bin/sh"}, - {Prefix: "/usr/"}, - }, - } - data, err := json.Marshal(f) - require.NoError(t, err) - assert.Contains(t, string(data), `"exact":"/bin/sh"`) - assert.Contains(t, string(data), `"prefix":"/usr/"`) -} - -// TestPatternObject_Validate_EmptyObject verifies that a PatternObject with no -// fields is rejected. -func TestPatternObject_Validate_EmptyObject(t *testing.T) { - // Use JSON unmarshaling path to trigger validate. - data := `{"opens": [{}]}` - var pdr ProfileDataRequired - err := json.Unmarshal([]byte(data), &pdr) - assert.Error(t, err, "empty PatternObject should be rejected") -} From 09755aa80f0103d841bcb4c8ae03c00609c3dff6 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 14 Sep 2026 19:06:25 +0200 Subject: [PATCH 2/4] fix(rulemanager): validate profileDataRequired and reject unknown keys on load Reject unknown surface keys and unknown ProfileDataPattern fields when converting unstructured Rule CRDs before conversion discards them, call ProfileDataRequired.Validate() after conversion, and reject invalid rule declarations in RuleCreator. Signed-off-by: Matthias Bertschy --- pkg/rulemanager/ruleswatcher/watcher.go | 43 +++++++ pkg/rulemanager/ruleswatcher/watcher_test.go | 107 ++++++++++++++++ pkg/rulemanager/types/v1/profiledata.go | 123 ++++++++++++++++++- pkg/rulemanager/types/v1/profiledata_test.go | 96 +++++++++++++++ 4 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 pkg/rulemanager/ruleswatcher/watcher_test.go create mode 100644 pkg/rulemanager/types/v1/profiledata_test.go diff --git a/pkg/rulemanager/ruleswatcher/watcher.go b/pkg/rulemanager/ruleswatcher/watcher.go index 45782beb23..47c186c213 100644 --- a/pkg/rulemanager/ruleswatcher/watcher.go +++ b/pkg/rulemanager/ruleswatcher/watcher.go @@ -2,6 +2,7 @@ package ruleswatcher import ( "context" + "fmt" "os" "github.com/Masterminds/semver/v3" @@ -118,14 +119,56 @@ func (w *RulesWatcherImpl) InitialSync(ctx context.Context) error { } func unstructuredToRules(obj *unstructured.Unstructured) (*typesv1.Rules, error) { + if err := validateRawProfileDataInUnstructured(obj); err != nil { + return nil, err + } + rule := &typesv1.Rules{} if err := k8sruntime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &rule); err != nil { return nil, err } + for _, r := range rule.Spec.Rules { + if r.ProfileDataRequired != nil { + if err := r.ProfileDataRequired.Validate(); err != nil { + return nil, fmt.Errorf("rule %q invalid profileDataRequired: %w", r.ID, err) + } + } + } + return rule, nil } +func validateRawProfileDataInUnstructured(obj *unstructured.Unstructured) error { + if obj == nil || obj.Object == nil { + return nil + } + spec, ok := obj.Object["spec"].(map[string]any) + if !ok { + return nil + } + rulesRaw, ok := spec["rules"].([]any) + if !ok { + return nil + } + for i, r := range rulesRaw { + ruleMap, ok := r.(map[string]any) + if !ok { + continue + } + if pdr, exists := ruleMap["profileDataRequired"]; exists && pdr != nil { + if err := typesv1.ValidateRawProfileDataRequired(pdr); err != nil { + ruleID, _ := ruleMap["id"].(string) + if ruleID != "" { + return fmt.Errorf("rule %q: %w", ruleID, err) + } + return fmt.Errorf("rule[%d]: %w", i, err) + } + } + } + return nil +} + // isAgentVersionCompatible checks if the current agent version satisfies the given requirement // using semantic versioning constraints. Returns true if compatible, false otherwise. func isAgentVersionCompatible(requirement string) bool { diff --git a/pkg/rulemanager/ruleswatcher/watcher_test.go b/pkg/rulemanager/ruleswatcher/watcher_test.go new file mode 100644 index 0000000000..0521f46fb8 --- /dev/null +++ b/pkg/rulemanager/ruleswatcher/watcher_test.go @@ -0,0 +1,107 @@ +package ruleswatcher + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestUnstructuredToRules_ProfileDataRequired(t *testing.T) { + t.Run("valid profileDataRequired", func(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]any{ + "spec": map[string]any{ + "rules": []any{ + map[string]any{ + "id": "R0001", + "profileDataRequired": map[string]any{ + "opens": "all", + "execs": []any{ + map[string]any{"exact": "/bin/sh"}, + }, + }, + }, + }, + }, + }, + } + + rules, err := unstructuredToRules(obj) + require.NoError(t, err) + require.NotNil(t, rules.Spec.Rules[0].ProfileDataRequired) + require.NotNil(t, rules.Spec.Rules[0].ProfileDataRequired.Opens) + assert.True(t, rules.Spec.Rules[0].ProfileDataRequired.Opens.All) + require.NotNil(t, rules.Spec.Rules[0].ProfileDataRequired.Execs) + require.Len(t, rules.Spec.Rules[0].ProfileDataRequired.Execs.Patterns, 1) + assert.Equal(t, "/bin/sh", rules.Spec.Rules[0].ProfileDataRequired.Execs.Patterns[0].Exact) + }) + + t.Run("rejects unknown surface key", func(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]any{ + "spec": map[string]any{ + "rules": []any{ + map[string]any{ + "id": "R0001", + "profileDataRequired": map[string]any{ + "unknownSurface": "all", + }, + }, + }, + }, + }, + } + + _, err := unstructuredToRules(obj) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown field "unknownSurface"`) + }) + + t.Run("rejects unknown pattern key", func(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]any{ + "spec": map[string]any{ + "rules": []any{ + map[string]any{ + "id": "R0001", + "profileDataRequired": map[string]any{ + "opens": []any{ + map[string]any{"exct": "/bin/sh"}, + }, + }, + }, + }, + }, + }, + } + + _, err := unstructuredToRules(obj) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown field "exct"`) + }) + + t.Run("rejects invalid pattern object with multiple fields", func(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]any{ + "spec": map[string]any{ + "rules": []any{ + map[string]any{ + "id": "R0001", + "profileDataRequired": map[string]any{ + "opens": []any{ + map[string]any{"exact": "/bin/sh", "prefix": "/usr/"}, + }, + }, + }, + }, + }, + }, + } + + _, err := unstructuredToRules(obj) + require.Error(t, err) + assert.Contains(t, err.Error(), "exactly one of {exact, prefix, suffix, contains} must be set") + }) +} diff --git a/pkg/rulemanager/types/v1/profiledata.go b/pkg/rulemanager/types/v1/profiledata.go index 2ccfeb5168..bd9f57447c 100644 --- a/pkg/rulemanager/types/v1/profiledata.go +++ b/pkg/rulemanager/types/v1/profiledata.go @@ -1,6 +1,12 @@ package types -import "github.com/armosec/armoapi-go/armotypes" +import ( + "fmt" + "reflect" + "strings" + + "github.com/armosec/armoapi-go/armotypes" +) // The profileDataRequired schema (the type, its match patterns, and the custom // JSON/YAML/BSON (un)marshalling) lives in armoapi-go/armotypes — the single @@ -19,3 +25,118 @@ type ( FieldRequirement = armotypes.ProfileDataField PatternObject = armotypes.ProfileDataPattern ) + +var ( + // KnownProfileDataSurfaces and KnownProfileDataPatternFields are extracted + // dynamically from the canonical armotypes structs so node-agent never + // duplicates the field lists and automatically inherits any new surfaces. + KnownProfileDataSurfaces = extractJSONFieldNames(reflect.TypeOf(armotypes.ProfileDataRequired{})) + KnownProfileDataPatternFields = extractJSONFieldNames(reflect.TypeOf(armotypes.ProfileDataPattern{})) +) + +func extractJSONFieldNames(t reflect.Type) map[string]bool { + if t.Kind() == reflect.Pointer { + t = t.Elem() + } + m := make(map[string]bool, t.NumField()) + for i := 0; i < t.NumField(); i++ { + tag := t.Field(i).Tag.Get("json") + if tag == "" || tag == "-" { + continue + } + name, _, _ := strings.Cut(tag, ",") + if name != "" { + m[name] = true + } + } + return m +} + +// ValidateRawProfileDataRequired inspects raw, untyped profileDataRequired +// definitions (e.g. from an unstructured CRD or JSON/YAML map) before conversion +// to armotypes.ProfileDataRequired discards unknown keys. +func ValidateRawProfileDataRequired(raw any) error { + if raw == nil { + return nil + } + rawMap, ok := toStringMap(raw) + if !ok { + return fmt.Errorf("profileDataRequired must be a map, got %T", raw) + } + + for k, v := range rawMap { + if !KnownProfileDataSurfaces[k] { + return fmt.Errorf("profileDataRequired: unknown field %q", k) + } + if v == nil { + continue + } + if err := validateRawProfileDataField(k, v); err != nil { + return err + } + } + return nil +} + +func validateRawProfileDataField(surface string, val any) error { + if s, ok := val.(string); ok { + if s != "all" { + return fmt.Errorf("profileDataRequired.%s: string value must be \"all\", got %q", surface, s) + } + return nil + } + + slice, ok := toSlice(val) + if !ok { + return fmt.Errorf("profileDataRequired.%s: expected \"all\" or pattern list, got %T", surface, val) + } + if len(slice) == 0 { + return fmt.Errorf("profileDataRequired.%s: pattern list must not be empty", surface) + } + + for i, pat := range slice { + patMap, ok := toStringMap(pat) + if !ok { + return fmt.Errorf("profileDataRequired.%s[%d]: pattern must be an object, got %T", surface, i, pat) + } + if len(patMap) == 0 { + return fmt.Errorf("profileDataRequired.%s[%d]: empty pattern object", surface, i) + } + for pk := range patMap { + if !KnownProfileDataPatternFields[pk] { + return fmt.Errorf("profileDataRequired.%s[%d]: unknown field %q", surface, i, pk) + } + } + } + return nil +} + +func toStringMap(v any) (map[string]any, bool) { + if m, ok := v.(map[string]any); ok { + return m, true + } + val := reflect.ValueOf(v) + if val.Kind() == reflect.Map { + res := make(map[string]any, val.Len()) + for _, key := range val.MapKeys() { + res[fmt.Sprint(key.Interface())] = val.MapIndex(key).Interface() + } + return res, true + } + return nil, false +} + +func toSlice(v any) ([]any, bool) { + if s, ok := v.([]any); ok { + return s, true + } + val := reflect.ValueOf(v) + if val.Kind() == reflect.Slice { + res := make([]any, val.Len()) + for i := 0; i < val.Len(); i++ { + res[i] = val.Index(i).Interface() + } + return res, true + } + return nil, false +} diff --git a/pkg/rulemanager/types/v1/profiledata_test.go b/pkg/rulemanager/types/v1/profiledata_test.go new file mode 100644 index 0000000000..ff7e6fda34 --- /dev/null +++ b/pkg/rulemanager/types/v1/profiledata_test.go @@ -0,0 +1,96 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateRawProfileDataRequired(t *testing.T) { + tests := []struct { + name string + raw any + wantErr bool + errMsg string + }{ + { + name: "nil is valid", + raw: nil, + wantErr: false, + }, + { + name: "valid all and patterns", + raw: map[string]any{ + "opens": "all", + "execs": []any{ + map[string]any{"exact": "/bin/sh"}, + map[string]any{"prefix": "/usr/"}, + }, + }, + wantErr: false, + }, + { + name: "unknown surface key", + raw: map[string]any{ + "open": "all", + }, + wantErr: true, + errMsg: `unknown field "open"`, + }, + { + name: "unknown pattern key", + raw: map[string]any{ + "opens": []any{ + map[string]any{"exct": "/bin/sh"}, + }, + }, + wantErr: true, + errMsg: `unknown field "exct"`, + }, + { + name: "invalid string value", + raw: map[string]any{ + "opens": "none", + }, + wantErr: true, + errMsg: `string value must be "all"`, + }, + { + name: "empty pattern list", + raw: map[string]any{ + "opens": []any{}, + }, + wantErr: true, + errMsg: "pattern list must not be empty", + }, + { + name: "empty pattern object", + raw: map[string]any{ + "opens": []any{map[string]any{}}, + }, + wantErr: true, + errMsg: "empty pattern object", + }, + { + name: "non-map input", + raw: "all", + wantErr: true, + errMsg: "must be a map", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateRawProfileDataRequired(tt.raw) + if tt.wantErr { + require.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + require.NoError(t, err) + } + }) + } +} From a473a27ecdd62b0fea52bb17aa86d304ff604f04 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Tue, 15 Sep 2026 07:49:40 +0200 Subject: [PATCH 3/4] test(ruleswatcher): verify default-rules.yaml loads with valid profileDataRequired Signed-off-by: Matthias Bertschy --- pkg/rulemanager/ruleswatcher/watcher_test.go | 30 ++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pkg/rulemanager/ruleswatcher/watcher_test.go b/pkg/rulemanager/ruleswatcher/watcher_test.go index 0521f46fb8..5a354bd6ca 100644 --- a/pkg/rulemanager/ruleswatcher/watcher_test.go +++ b/pkg/rulemanager/ruleswatcher/watcher_test.go @@ -1,11 +1,14 @@ package ruleswatcher import ( + "os" "testing" + typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/yaml" ) func TestUnstructuredToRules_ProfileDataRequired(t *testing.T) { @@ -105,3 +108,30 @@ func TestUnstructuredToRules_ProfileDataRequired(t *testing.T) { assert.Contains(t, err.Error(), "exactly one of {exact, prefix, suffix, contains} must be set") }) } + +func TestUnstructuredToRules_DefaultRulesYAML(t *testing.T) { + data, err := os.ReadFile("../../../tests/chart/templates/node-agent/default-rules.yaml") + require.NoError(t, err) + + var objMap map[string]any + err = yaml.Unmarshal(data, &objMap) + require.NoError(t, err) + + obj := &unstructured.Unstructured{Object: objMap} + rules, err := unstructuredToRules(obj) + require.NoError(t, err) + require.NotEmpty(t, rules.Spec.Rules) + + var r0001 *typesv1.Rule + for i := range rules.Spec.Rules { + if rules.Spec.Rules[i].ID == "R0001" { + r0001 = &rules.Spec.Rules[i] + break + } + } + require.NotNil(t, r0001) + require.NotNil(t, r0001.ProfileDataRequired) + require.NotNil(t, r0001.ProfileDataRequired.Execs) + assert.True(t, r0001.ProfileDataRequired.Execs.All) +} + From 298b0131aa2ae90459e2e019676ee799e9c332f2 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Tue, 15 Sep 2026 08:27:27 +0200 Subject: [PATCH 4/4] ci(tests): resolve latest storage release in storage-tag.sh Signed-off-by: Matthias Bertschy --- tests/scripts/storage-tag.sh | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/scripts/storage-tag.sh b/tests/scripts/storage-tag.sh index 8db14a5ef2..b3c3958502 100755 --- a/tests/scripts/storage-tag.sh +++ b/tests/scripts/storage-tag.sh @@ -1,13 +1,23 @@ -#/bin/bash -curl -s https://raw.githubusercontent.com/kubescape/helm-charts/main/charts/kubescape-operator/values.yaml -o values.yaml -DYNAMIC_TAG=$(yq '.storage.image.tag' < values.yaml | tr -d '"') -rm -rf values.yaml +#!/bin/bash +set -eo pipefail -# Floor: node-agent's own go.mod-pinned kubescape/storage client version. When +# 1. Latest release tag from kubescape/storage repository (using git ls-remote to avoid GitHub API rate limits) +LATEST_TAG=$(git ls-remote --tags --refs https://github.com/kubescape/storage.git 2>/dev/null | awk -F/ '{print $3}' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -n1 || true) + +# 2. Pinned tag from helm-charts +curl -s https://raw.githubusercontent.com/kubescape/helm-charts/main/charts/kubescape-operator/values.yaml -o values.yaml 2>/dev/null || true +DYNAMIC_TAG="" +if [ -f values.yaml ]; then + DYNAMIC_TAG=$(yq '.storage.image.tag' < values.yaml 2>/dev/null | tr -d '"' || true) + rm -f values.yaml +fi + +# 3. Floor: node-agent's own go.mod-pinned kubescape/storage client version. When # kubescape/helm-charts' pinned server image (fetched above) lags behind what # this repo's client library needs, component-tests would silently regress -# (an older server drops CRD fields the newer client sets). Take the newer of -# the two so CI never runs against a server that predates our client. -FLOOR_TAG=$(go list -m -f '{{.Version}}' github.com/kubescape/storage) +# (an older server drops CRD fields the newer client sets). Take the newest +# available tag so CI runs against the latest server release. +FLOOR_TAG=$(go list -m -f '{{.Version}}' github.com/kubescape/storage 2>/dev/null || true) + +printf '%s\n%s\n%s\n' "$LATEST_TAG" "$DYNAMIC_TAG" "$FLOOR_TAG" | grep -v '^$' | sort -V | tail -n1 -printf '%s\n%s\n' "$DYNAMIC_TAG" "$FLOOR_TAG" | sort -V | tail -n1