Skip to content

Commit da6a874

Browse files
holyspectralholyspectral
authored
Merge pull request #749 from Andreagit97/rework-wp-status-5
refactor: rework WP status management [5/n]
2 parents f49f411 + c298091 commit da6a874

14 files changed

Lines changed: 258 additions & 150 deletions

‎api/v1alpha1/policy_node_status.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func (s *WorkloadPolicyStatus) resetPolicyNodeStatus(totalNodes int) {
8686
s.TransitioningNodes = 0
8787
}
8888

89-
func (s *WorkloadPolicyStatus) ProcessPolicyNodeStatus(nodes []PolicyNodeStatus) error {
89+
func (s *WorkloadPolicyStatus) processPolicyNodeStatus(nodes []PolicyNodeStatus) error {
9090
s.resetPolicyNodeStatus(len(nodes))
9191

9292
for _, status := range nodes {

‎api/v1alpha1/policy_node_status_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func TestProcessPolicyNodeStatus(t *testing.T) {
108108
for _, tt := range tests {
109109
t.Run(tt.name, func(t *testing.T) {
110110
wpStatus := WorkloadPolicyStatus{}
111-
wpStatus.ProcessPolicyNodeStatus(tt.nodes)
111+
wpStatus.processPolicyNodeStatus(tt.nodes)
112112
require.Equal(t, tt.expected, wpStatus)
113113
})
114114
}

‎api/v1alpha1/violation_record.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99
)
1010

11-
const MaxViolationRecords = 100
11+
const maxViolationRecords = 100
1212

1313
// ViolationRecord holds the details of a single policy violation.
1414
type ViolationRecord struct {
@@ -76,16 +76,16 @@ func (r ViolationRecord) key() violationRecordKey {
7676
}
7777
}
7878

79-
func (wp *WorkloadPolicy) ClearAllowed() {
79+
func (wp *WorkloadPolicy) clearAllowedViolations() {
8080
wp.Status.Violations = slices.DeleteFunc(wp.Status.Violations, func(v ViolationRecord) bool {
8181
rules := wp.Spec.RulesByContainer[v.ContainerName]
8282
return rules != nil && slices.Contains(rules.Executables.Allowed, v.ExecutablePath)
8383
})
8484
}
8585

86-
// MergeScrapedViolations dedupes scraped violations against the existing list, allocate ids for
86+
// mergeScrapedViolations dedupes scraped violations against the existing list, allocate ids for
8787
// new records and refresh the timestamp/node on matched records.
88-
func (s *WorkloadPolicyStatus) MergeScrapedViolations(scraped []ViolationRecord) {
88+
func (s *WorkloadPolicyStatus) mergeScrapedViolations(scraped []ViolationRecord) {
8989
indexByKey := make(map[violationRecordKey]int, len(s.Violations))
9090
for i, r := range s.Violations {
9191
indexByKey[r.key()] = i
@@ -94,7 +94,12 @@ func (s *WorkloadPolicyStatus) MergeScrapedViolations(scraped []ViolationRecord)
9494
for _, v := range scraped {
9595
key := v.key()
9696
if idx, ok := indexByKey[key]; ok {
97-
s.Violations[idx].Timestamp = v.Timestamp
97+
// We need to overwrite the timestamp only if it is newer.
98+
// if in a same batch we have multiple occurrences of the violation
99+
// we just need to store the one with the highest timestamp.
100+
if v.Timestamp.Time.After(s.Violations[idx].Timestamp.Time) {
101+
s.Violations[idx].Timestamp = v.Timestamp
102+
}
98103
} else {
99104
v.ID = s.ViolationCount
100105
s.Violations = append(s.Violations, v)
@@ -107,12 +112,12 @@ func (s *WorkloadPolicyStatus) MergeScrapedViolations(scraped []ViolationRecord)
107112
return b.Timestamp.Time.Compare(a.Timestamp.Time)
108113
})
109114

110-
if len(s.Violations) > MaxViolationRecords {
111-
s.Violations = s.Violations[:MaxViolationRecords]
115+
if len(s.Violations) > maxViolationRecords {
116+
s.Violations = s.Violations[:maxViolationRecords]
112117
}
113118
}
114119

115-
func (wp *WorkloadPolicy) AcknowledgeViolationsFromAnnotations(now metav1.Time) []AcknowledgedViolationRecord {
120+
func (wp *WorkloadPolicy) acknowledgeViolationsFromAnnotations(now metav1.Time) []AcknowledgedViolationRecord {
116121
annotations := wp.GetAnnotations()
117122
// No annotations -> no violations to acknowledge
118123
if len(annotations) == 0 {
@@ -180,8 +185,8 @@ func (wp *WorkloadPolicy) AcknowledgeViolationsFromAnnotations(now metav1.Time)
180185
return b.AcknowledgedAt.Time.Compare(a.AcknowledgedAt.Time)
181186
})
182187

183-
if len(wp.Status.AcknowledgedViolations) > MaxViolationRecords {
184-
wp.Status.AcknowledgedViolations = wp.Status.AcknowledgedViolations[:MaxViolationRecords]
188+
if len(wp.Status.AcknowledgedViolations) > maxViolationRecords {
189+
wp.Status.AcknowledgedViolations = wp.Status.AcknowledgedViolations[:maxViolationRecords]
185190
}
186191
return ackToReturn
187192
}

‎api/v1alpha1/violation_record_test.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ func (r ViolationRecord) withPodName(name string) ViolationRecord {
2020
return r
2121
}
2222

23+
func (r ViolationRecord) withNodeName(name string) ViolationRecord {
24+
r.NodeName = name
25+
return r
26+
}
27+
2328
func (r ViolationRecord) withContainerName(name string) ViolationRecord {
2429
r.ContainerName = name
2530
return r
@@ -150,7 +155,7 @@ func TestMergeScrapedViolations(t *testing.T) {
150155
withTimestamp(baseTS.Add(time.Duration(101) * time.Minute)),
151156
},
152157
initialStatus: func() WorkloadPolicyStatus {
153-
r := make([]ViolationRecord, MaxViolationRecords)
158+
r := make([]ViolationRecord, maxViolationRecords)
154159
for i := range r {
155160
r[i] = baseViolation.withExecutable(fmt.Sprintf("/%d", i+1)).
156161
withID(int64(i)).
@@ -162,7 +167,7 @@ func TestMergeScrapedViolations(t *testing.T) {
162167
}
163168
}(),
164169
expectedStatus: func() WorkloadPolicyStatus {
165-
r := make([]ViolationRecord, MaxViolationRecords+1)
170+
r := make([]ViolationRecord, maxViolationRecords+1)
166171
for i := range r {
167172
r[i] = baseViolation.withExecutable(fmt.Sprintf("/%d", i+1)).
168173
withID(int64(i)).
@@ -172,7 +177,7 @@ func TestMergeScrapedViolations(t *testing.T) {
172177
return b.Timestamp.Time.Compare(a.Timestamp.Time)
173178
})
174179
return WorkloadPolicyStatus{
175-
Violations: r[:MaxViolationRecords],
180+
Violations: r[:maxViolationRecords],
176181
ViolationCount: 101,
177182
}
178183
}(),
@@ -181,7 +186,7 @@ func TestMergeScrapedViolations(t *testing.T) {
181186

182187
for _, tt := range tests {
183188
t.Run(tt.name, func(t *testing.T) {
184-
tt.initialStatus.MergeScrapedViolations(tt.scraped)
189+
tt.initialStatus.mergeScrapedViolations(tt.scraped)
185190
require.Equal(t, tt.expectedStatus, tt.initialStatus)
186191
})
187192
}
@@ -246,7 +251,7 @@ func TestClearAllowedViolations(t *testing.T) {
246251
Spec: WorkloadPolicySpec{RulesByContainer: tt.rules},
247252
Status: WorkloadPolicyStatus{Violations: tt.violations},
248253
}
249-
wp.ClearAllowed()
254+
wp.clearAllowedViolations()
250255
require.Equal(t, tt.expected, wp.Status.Violations)
251256
})
252257
}
@@ -349,7 +354,7 @@ func TestAcknowledgeViolationsFromAnnotations(t *testing.T) {
349354
violations: []ViolationRecord{newViolation(101)},
350355
// This policy already has an acknowledged violation, that should remain untouched
351356
acknowledged: func() []AcknowledgedViolationRecord {
352-
r := make([]AcknowledgedViolationRecord, MaxViolationRecords)
357+
r := make([]AcknowledgedViolationRecord, maxViolationRecords)
353358
for i := range r {
354359
r[i] = newAck(int64(i), "acknowledged", now)
355360
}
@@ -358,15 +363,15 @@ func TestAcknowledgeViolationsFromAnnotations(t *testing.T) {
358363
wantAnnotations: map[string]string{},
359364
wantViolations: []ViolationRecord{},
360365
wantAcknowledged: func() []AcknowledgedViolationRecord {
361-
r := make([]AcknowledgedViolationRecord, MaxViolationRecords+1)
366+
r := make([]AcknowledgedViolationRecord, maxViolationRecords+1)
362367
for i := range r {
363368
r[i] = newAck(int64(i), "acknowledged", now)
364369
}
365-
r[MaxViolationRecords] = newAck(101, "acknowledged", now)
370+
r[maxViolationRecords] = newAck(101, "acknowledged", now)
366371
slices.SortFunc(r, func(a, b AcknowledgedViolationRecord) int {
367372
return b.AcknowledgedAt.Time.Compare(a.AcknowledgedAt.Time)
368373
})
369-
return r[:MaxViolationRecords]
374+
return r[:maxViolationRecords]
370375
}(),
371376
wantReturned: []AcknowledgedViolationRecord{
372377
newAck(101, "acknowledged", now),
@@ -383,7 +388,7 @@ func TestAcknowledgeViolationsFromAnnotations(t *testing.T) {
383388
},
384389
}
385390

386-
returned := wp.AcknowledgeViolationsFromAnnotations(now)
391+
returned := wp.acknowledgeViolationsFromAnnotations(now)
387392

388393
require.Equal(t, tt.wantAnnotations, wp.GetAnnotations())
389394
require.ElementsMatch(t, tt.wantViolations, wp.Status.Violations)

‎api/v1alpha1/workloadpolicy_test.go‎

Lines changed: 170 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1-
package v1alpha1_test
1+
package v1alpha1
22

33
import (
44
"testing"
5+
"time"
56

6-
"github.com/rancher-sandbox/runtime-enforcer/api/v1alpha1"
7+
"github.com/rancher-sandbox/runtime-enforcer/internal/types/policymode"
78
"github.com/stretchr/testify/require"
89
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
)
1011

1112
func TestWorkloadPolicyNamespacedName(t *testing.T) {
12-
wp := &v1alpha1.WorkloadPolicy{
13+
wp := &WorkloadPolicy{
1314
ObjectMeta: metav1.ObjectMeta{
1415
Namespace: "test-namespace",
1516
Name: "test-name",
@@ -18,3 +19,169 @@ func TestWorkloadPolicyNamespacedName(t *testing.T) {
1819
expected := "test-namespace/test-name"
1920
require.Equal(t, expected, wp.NamespacedName())
2021
}
22+
23+
func TestRecomputeStatus(t *testing.T) {
24+
// This test should simulate a real flow
25+
const (
26+
forgottenBinary = "/usr/bin/cat"
27+
containerName = "example"
28+
workloadName = "example-workload"
29+
workloadKind = "Deployment"
30+
podA = "pod-a"
31+
nodeA = "node-a"
32+
podB = "pod-b"
33+
nodeB = "node-b"
34+
)
35+
baseViolation := ViolationRecord{
36+
ContainerName: containerName,
37+
ExecutablePath: forgottenBinary,
38+
Action: policymode.ProtectString,
39+
WorkloadName: workloadName,
40+
WorkloadKind: workloadKind,
41+
}
42+
podAViolation := baseViolation.withPodName(podA).withNodeName(nodeA)
43+
podBViolation := baseViolation.withPodName(podB).withNodeName(nodeB)
44+
baseTS := time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC)
45+
ts := func(sec int) time.Time {
46+
return baseTS.Add(time.Duration(sec) * time.Second)
47+
}
48+
49+
policy := &WorkloadPolicy{
50+
ObjectMeta: metav1.ObjectMeta{
51+
Generation: 7,
52+
Annotations: map[string]string{},
53+
},
54+
Spec: WorkloadPolicySpec{
55+
RulesByContainer: map[string]*WorkloadPolicyRules{
56+
containerName: {Executables: WorkloadPolicyExecutables{Allowed: []string{
57+
"/usr/bin/sh",
58+
"/usr/bin/ls",
59+
}}},
60+
},
61+
},
62+
Status: WorkloadPolicyStatus{
63+
ViolationCount: 0,
64+
ActiveViolationCount: 0,
65+
},
66+
}
67+
68+
///////////////////////////
69+
// the user forgot a binary "/usr/bin/cat"
70+
///////////////////////////
71+
72+
// If the application is a deployment we expect multiple violations from different nodes
73+
violations := []ViolationRecord{
74+
// The agent should send newest records first
75+
podAViolation.withTimestamp(ts(1)),
76+
podAViolation.withTimestamp(ts(2)),
77+
podBViolation.withTimestamp(ts(3)),
78+
podAViolation.withTimestamp(ts(4)),
79+
}
80+
81+
policy.RecomputeStatus(nil, violations, ts(5))
82+
expectedPolicyStatus := WorkloadPolicyStatus{
83+
ViolationCount: 4,
84+
ActiveViolationCount: 2,
85+
Violations: []ViolationRecord{
86+
podAViolation.withID(0).withTimestamp(ts(4)),
87+
// This has ID 2 because we first face 2 violation for pod-a
88+
podBViolation.withID(2).withTimestamp(ts(3)),
89+
},
90+
ObservedGeneration: 7,
91+
Phase: Ready,
92+
}
93+
require.Equal(t, expectedPolicyStatus, policy.Status)
94+
95+
///////////////////////////
96+
// the user set the policy to monitor
97+
///////////////////////////
98+
99+
// we should receive new violation in monitor mode so we convert them
100+
podAViolationMonitor := podAViolation.withAction(policymode.MonitorString)
101+
podBViolationMonitor := podBViolation.withAction(policymode.MonitorString)
102+
violations = []ViolationRecord{
103+
// The agent should send newest records first
104+
podAViolationMonitor.withTimestamp(ts(20)),
105+
podBViolationMonitor.withTimestamp(ts(21)),
106+
podAViolationMonitor.withTimestamp(ts(22)),
107+
}
108+
policy.RecomputeStatus(nil, violations, ts(24))
109+
expectedPolicyStatus = WorkloadPolicyStatus{
110+
ViolationCount: 7,
111+
ActiveViolationCount: 4,
112+
Violations: []ViolationRecord{
113+
podAViolationMonitor.withID(4).withTimestamp(ts(22)),
114+
podBViolationMonitor.withID(5).withTimestamp(ts(21)),
115+
podAViolation.withID(0).withTimestamp(ts(4)),
116+
podBViolation.withID(2).withTimestamp(ts(3)),
117+
},
118+
ObservedGeneration: 7,
119+
Phase: Ready,
120+
}
121+
require.Equal(t, expectedPolicyStatus, policy.Status)
122+
123+
///////////////////////////
124+
// the user adds the binary into the spec
125+
///////////////////////////
126+
127+
policy.Spec.RulesByContainer[containerName].Executables.Allowed =
128+
append(policy.Spec.RulesByContainer[containerName].Executables.Allowed, forgottenBinary)
129+
policy.Generation = 8
130+
131+
// Some new violations comes in the meantime
132+
violations = []ViolationRecord{
133+
podAViolationMonitor.withTimestamp(ts(40)),
134+
podBViolationMonitor.withTimestamp(ts(41)),
135+
}
136+
policy.RecomputeStatus(nil, violations, ts(42))
137+
expectedPolicyStatus = WorkloadPolicyStatus{
138+
ViolationCount: 9,
139+
ActiveViolationCount: 0,
140+
Violations: []ViolationRecord{},
141+
ObservedGeneration: 8,
142+
Phase: Ready,
143+
}
144+
require.Equal(t, expectedPolicyStatus, policy.Status)
145+
146+
///////////////////////////
147+
// A real violation comes
148+
///////////////////////////
149+
150+
violations = []ViolationRecord{
151+
podAViolation.withTimestamp(ts(100)).withExecutable("/usr/bin/malware"),
152+
}
153+
policy.RecomputeStatus(nil, violations, ts(101))
154+
expectedPolicyStatus = WorkloadPolicyStatus{
155+
ViolationCount: 10,
156+
ActiveViolationCount: 1,
157+
Violations: []ViolationRecord{
158+
podAViolation.withID(9).withTimestamp(ts(100)).withExecutable("/usr/bin/malware"),
159+
},
160+
ObservedGeneration: 8,
161+
Phase: Ready,
162+
}
163+
require.Equal(t, expectedPolicyStatus, policy.Status)
164+
165+
///////////////////////////
166+
// The user acknowledges it
167+
///////////////////////////
168+
169+
policy.Annotations[ViolationAcknowledgePrefix+"9"] = "acknowledged by the user"
170+
acknowledgeTime := ts(120)
171+
policy.RecomputeStatus(nil, nil, acknowledgeTime)
172+
expectedPolicyStatus = WorkloadPolicyStatus{
173+
ViolationCount: 10,
174+
ActiveViolationCount: 0,
175+
Violations: []ViolationRecord{},
176+
AcknowledgedViolations: []AcknowledgedViolationRecord{
177+
{
178+
Violation: podAViolation.withID(9).withTimestamp(ts(100)).withExecutable("/usr/bin/malware"),
179+
Reason: "acknowledged by the user",
180+
AcknowledgedAt: metav1.Time{Time: acknowledgeTime},
181+
},
182+
},
183+
ObservedGeneration: 8,
184+
Phase: Ready,
185+
}
186+
require.Equal(t, expectedPolicyStatus, policy.Status)
187+
}

0 commit comments

Comments
 (0)