Skip to content

Commit 9833851

Browse files
J12934J12934
committed
Refactor hook prio handling to use a pre calculated list
1 parent bd6219d commit 9833851

12 files changed

Lines changed: 498 additions & 292 deletions

File tree

‎operator/apis/cascading/v1/zz_generated.deepcopy.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎operator/apis/execution/v1/scan_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ type ScanStatus struct {
8383

8484
Findings FindingStats `json:"findings,omitempty"`
8585

86-
HookStatuses []*HookStatus `json:"hookStatuses,omitempty"`
86+
OrderedHookStatuses [][]HookStatus `json:"orderedHookStatuses,omitempty"`
8787
}
8888

8989
// HookState Describes the State of a Hook on a Scan

‎operator/apis/execution/v1/zz_generated.deepcopy.go‎

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎operator/config/crd/bases/execution.securecodebox.io_scans.yaml‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1756,28 +1756,31 @@ spec:
17561756
parser & hooks) has been marked as "Done"
17571757
format: date-time
17581758
type: string
1759-
hookStatuses:
1759+
orderedHookStatuses:
17601760
items:
1761-
properties:
1762-
hookName:
1763-
type: string
1764-
jobName:
1765-
type: string
1766-
priority:
1767-
type: integer
1768-
state:
1769-
description: HookState Describes the State of a Hook on a Scan
1770-
type: string
1771-
type:
1772-
description: HookType Defines weather the hook should be able
1773-
to change the findings or is run in a read only mode.
1774-
type: string
1775-
required:
1776-
- hookName
1777-
- priority
1778-
- state
1779-
- type
1780-
type: object
1761+
items:
1762+
properties:
1763+
hookName:
1764+
type: string
1765+
jobName:
1766+
type: string
1767+
priority:
1768+
type: integer
1769+
state:
1770+
description: HookState Describes the State of a Hook on a
1771+
Scan
1772+
type: string
1773+
type:
1774+
description: HookType Defines weather the hook should be able
1775+
to change the findings or is run in a read only mode.
1776+
type: string
1777+
required:
1778+
- hookName
1779+
- priority
1780+
- state
1781+
- type
1782+
type: object
1783+
type: array
17811784
type: array
17821785
rawResultDownloadLink:
17831786
description: RawResultDownloadLink link to download the raw result

‎operator/controllers/execution/scans/hook_reconciler.go‎

Lines changed: 63 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
package scancontrollers
66

77
import (
8-
"container/heap"
98
"context"
109
"fmt"
1110

1211
executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1"
12+
"github.com/secureCodeBox/secureCodeBox/operator/utils"
1313
util "github.com/secureCodeBox/secureCodeBox/operator/utils"
1414
batch "k8s.io/api/batch/v1"
1515
corev1 "k8s.io/api/core/v1"
@@ -33,16 +33,8 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error {
3333

3434
r.Log.Info("Found ScanCompletionHooks", "ScanCompletionHooks", len(scanCompletionHooks.Items))
3535

36-
for _, hook := range scanCompletionHooks.Items {
37-
hookStatus := &executionv1.HookStatus{
38-
HookName: hook.Name,
39-
State: executionv1.Pending,
40-
Type: hook.Spec.Type,
41-
Priority: hook.Spec.Priority,
42-
}
43-
scan.Status.HookStatuses = append(scan.Status.HookStatuses, hookStatus)
44-
}
45-
36+
orderedHookStatus := util.FromUnorderedList(scanCompletionHooks.Items)
37+
scan.Status.OrderedHookStatuses = orderedHookStatus
4638
scan.Status.State = "HookProcessing"
4739

4840
if err := r.Status().Update(ctx, scan); err != nil {
@@ -56,50 +48,25 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error {
5648
func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error {
5749
ctx := context.Background()
5850

59-
nonCompletedHooks := util.PriorityQueueFromSlice(
60-
&scan.Status.HookStatuses,
61-
func(hook *executionv1.HookStatus) bool {
62-
return hook.State != executionv1.Completed
63-
},
64-
)
51+
err, currentHooks := utils.CurrentHookGroup(scan.Status.OrderedHookStatuses)
6552

66-
var err error
67-
// If nil then all hooks are done
68-
if len(nonCompletedHooks) == 0 {
69-
r.Log.Info("Marked scan as done as all hooks have completed", "ScanName", scan.Name)
70-
scan.Status.State = "Done"
71-
var now = metav1.Now()
72-
scan.Status.FinishedAt = &now
73-
} else if len(nonCompletedHooks) > 0 {
74-
for {
75-
hook := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem).Value
76-
77-
err = r.processHook(scan, hook)
78-
if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled {
53+
if err != nil && scan.Status.State == "Errored" {
54+
r.Log.V(8).Info("Skipping hook execution as it already contains failed hooks.")
55+
return nil
56+
} else if err != nil {
57+
scan.Status.State = "Errored"
58+
scan.Status.ErrorDescription = fmt.Sprintf("Hook execution failed for a unknown hook. Check the scan.status.hookStatus field for more details")
59+
} else if err == nil && currentHooks == nil {
60+
// No hooks left to execute
61+
scan.Status.State = "Completed"
62+
} else {
63+
for i, hook := range currentHooks {
64+
err, status := r.processHook(scan, &hook)
65+
currentHooks[i] = status
66+
67+
if err != nil {
7968
scan.Status.State = "Errored"
8069
scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName)
81-
break
82-
}
83-
84-
// ReadAndWrite hooks may not be run in parallel
85-
if nonCompletedHooks.Len() == 0 || hook.Type == executionv1.ReadAndWrite {
86-
break
87-
}
88-
89-
next := nonCompletedHooks.Peek().(*util.PriorityQueueItem).Value
90-
if next.Priority < hook.Priority {
91-
break
92-
}
93-
}
94-
}
95-
96-
if err == nil && scan.Status.State == "HookProcessing" {
97-
// Check if there are now still incomplete hooks
98-
scan.Status.State = "Completed"
99-
for _, hook := range scan.Status.HookStatuses {
100-
if hook.State != executionv1.Completed {
101-
scan.Status.State = "HookProcessing"
102-
break
10370
}
10471
}
10572
}
@@ -108,64 +75,73 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error {
10875
r.Log.Error(sErr, "Unable to update Scan status")
10976
return sErr
11077
}
111-
11278
return err
11379
}
11480

115-
func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error {
81+
func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) (error, executionv1.HookStatus) {
11682
var jobType string
11783
if nonCompletedHook.Type == executionv1.ReadOnly {
11884
jobType = "read-only-hook"
11985
} else if nonCompletedHook.Type == executionv1.ReadAndWrite {
12086
jobType = "read-and-write-hook"
12187
}
12288

123-
r.Log.Info(fmt.Sprintf("Processing %s", jobType), "Hook", nonCompletedHook)
89+
r.Log.Info("Processing hook", "hook", nonCompletedHook, "jobType", jobType)
12490

125-
var err error
12691
switch nonCompletedHook.State {
12792
case executionv1.Pending:
128-
err = r.processPendingHook(scan, nonCompletedHook, jobType)
93+
err, status := r.processPendingHook(scan, nonCompletedHook, jobType)
94+
if err == nil {
95+
r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType)
96+
}
97+
return err, status
12998
case executionv1.InProgress:
130-
err = r.processInProgressHook(scan, nonCompletedHook, jobType)
131-
}
132-
133-
if err == nil {
134-
r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook)
99+
err, status := r.processInProgressHook(scan, nonCompletedHook, jobType)
100+
if err == nil {
101+
r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType)
102+
}
103+
return err, status
135104
}
136-
137-
return err
105+
return nil, *nonCompletedHook.DeepCopy()
138106
}
139107

140-
func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) error {
108+
func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) {
141109
ctx := context.Background()
142110

143111
var hook executionv1.ScanCompletionHook
112+
status := pendingHook.DeepCopy()
113+
144114
var err error
145115
err = r.Get(ctx, types.NamespacedName{Name: pendingHook.HookName, Namespace: scan.Namespace}, &hook)
146116
if err != nil {
147117
r.Log.Error(err, "Failed to get Hook for HookStatus")
148-
return err
118+
return err, *status
149119
}
150120

151121
var jobs *batch.JobList
152122
jobs, err = r.getJobsForScan(scan, client.MatchingLabels{
153123
"securecodebox.io/job-type": jobType,
154124
"securecodebox.io/hook-name": pendingHook.HookName,
155125
})
156-
if err != nil || len(jobs.Items) > 0 {
157-
return err
126+
if err != nil {
127+
return err, *status
128+
}
129+
if len(jobs.Items) > 0 {
130+
// job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values
131+
status.JobName = jobs.Items[0].Name
132+
status.State = executionv1.InProgress
133+
return nil, *status
158134
}
159135

160136
var rawFileURL string
161137
rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration)
162138
if err != nil {
163-
return err
139+
return err, *status
164140
}
165141
var findingsFileURL string
166142
findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration)
167143
if err != nil {
168-
return err
144+
return err, *status
169145
}
170146

171147
var args = []string{
@@ -176,12 +152,12 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook
176152
var rawFileUploadURL string
177153
rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration)
178154
if err != nil {
179-
return err
155+
return err, *status
180156
}
181157
var findingsUploadURL string
182158
findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration)
183159
if err != nil {
184-
return err
160+
return err, *status
185161
}
186162
args = append(args, rawFileUploadURL, findingsUploadURL)
187163
}
@@ -194,37 +170,41 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook
194170
)
195171

196172
if err == nil {
197-
// Update the currently executed hook job name and status to "InProgress"
198-
pendingHook.JobName = jobName
199-
pendingHook.State = executionv1.InProgress
173+
// job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values
174+
status := pendingHook.DeepCopy()
175+
status.JobName = jobName
176+
status.State = executionv1.InProgress
177+
return nil, *status
200178
}
201179

202-
return err
180+
return err, *status
203181
}
204182

205-
func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) error {
183+
func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) {
184+
status := inProgressHook.DeepCopy()
185+
206186
jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{
207187
"securecodebox.io/job-type": jobType,
208188
"securecodebox.io/hook-name": inProgressHook.HookName,
209189
})
210190
if err != nil {
211191
r.Log.Error(err, "Failed to check job status for Hook")
212-
return err
192+
return err, *status
213193
}
214194
switch jobStatus {
215195
case completed:
216196
// Job is completed => set current Hook to completed
217-
inProgressHook.State = executionv1.Completed
197+
status.State = executionv1.Completed
218198
case incomplete:
219199
// Still waiting for job to finish
220200
case failed:
221-
if inProgressHook.State == executionv1.Pending {
222-
inProgressHook.State = executionv1.Cancelled
201+
if status.State == executionv1.Pending {
202+
status.State = executionv1.Cancelled
223203
} else {
224-
inProgressHook.State = executionv1.Failed
204+
status.State = executionv1.Failed
225205
}
226206
}
227-
return nil
207+
return nil, *status
228208
}
229209

230210
func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) {

‎operator/go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ go 1.15
99
require (
1010
github.com/go-logr/logr v0.4.0
1111
github.com/minio/minio-go/v7 v7.0.10
12+
github.com/onsi/ginkgo v1.16.5
13+
github.com/onsi/gomega v1.16.0
1214
k8s.io/api v0.20.2
1315
k8s.io/apimachinery v0.20.2
1416
k8s.io/client-go v0.20.2

0 commit comments

Comments
 (0)