From 5edd1639003b940814aa47207a65d3f2744f753e Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Fri, 1 Oct 2021 14:57:29 +0200 Subject: [PATCH 01/17] Add support for hook priorities Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scan_types.go | 4 + .../execution/v1/scancompletionhook_types.go | 6 + .../execution/v1/zz_generated.deepcopy.go | 5 + ....securecodebox.io_scancompletionhooks.yaml | 10 + .../execution.securecodebox.io_scans.yaml | 31 ++ .../execution/scans/hook_reconciler.go | 414 +++++++++--------- .../execution/scans/scan_controller.go | 4 +- ....securecodebox.io_scancompletionhooks.yaml | 10 + .../execution.securecodebox.io_scans.yaml | 31 ++ 9 files changed, 305 insertions(+), 210 deletions(-) diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index 6bddae27e5..9ae468fd33 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -84,6 +84,8 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` ReadAndWriteHookStatus []HookStatus `json:"readAndWriteHookStatus,omitempty"` + + ReadOnlyHookStatus []HookStatus `json:"readOnlyHookStatus,omitempty"` } // HookState Describes the State of a Hook on a Scan @@ -101,6 +103,8 @@ type HookStatus struct { HookName string `json:"hookName"` State HookState `json:"state"` JobName string `json:"jobName,omitempty"` + Priority int `json:"priority"` + Type HookType `json:"type"` } // FindingStats contains the general stats about the results of the scan diff --git a/operator/apis/execution/v1/scancompletionhook_types.go b/operator/apis/execution/v1/scancompletionhook_types.go index 2ce5136450..9d057702a4 100644 --- a/operator/apis/execution/v1/scancompletionhook_types.go +++ b/operator/apis/execution/v1/scancompletionhook_types.go @@ -30,6 +30,11 @@ type ScanCompletionHookSpec struct { // Defines weather the hook should be able to change the findings or is run in a read only mode. Type HookType `json:"type"` + // Defines the priority of the hook. Higher priority hooks run before low priority hooks. Hooks with identical priority will be launched in parallel. + // +kubebuilder:default=0 + // +kubebuilder:validation:Optional + Priority int `json:"priority"` + // Image is the container image for the hooks kubernetes job Image string `json:"image,omitempty"` // ImagePullSecrets used to access private hooks images @@ -60,6 +65,7 @@ type ScanCompletionHookStatus struct { // +kubebuilder:object:root=true // +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type`,description="ScanCompletionHook Type" +// +kubebuilder:printcolumn:name="Priority",type=string,JSONPath=`.spec.priority`,description="ScanCompletionHook Priority" // +kubebuilder:printcolumn:name="Image",type=string,JSONPath=`.spec.image`,description="ScanCompletionHook Image" // ScanCompletionHook is the Schema for the ScanCompletionHooks API diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index ebe1b4f245..bab8c2fddb 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -474,6 +474,11 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { *out = make([]HookStatus, len(*in)) copy(*out, *in) } + if in.ReadOnlyHookStatus != nil { + in, out := &in.ReadOnlyHookStatus, &out.ReadOnlyHookStatus + *out = make([]HookStatus, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ScanStatus. diff --git a/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml b/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml index 5758ef4520..c3bf1a7fda 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml @@ -24,6 +24,10 @@ spec: jsonPath: .spec.type name: Type type: string + - description: ScanCompletionHook Priority + jsonPath: .spec.priority + name: Priority + type: string - description: ScanCompletionHook Image jsonPath: .spec.image name: Image @@ -174,6 +178,12 @@ spec: type: string type: object type: array + priority: + default: 0 + description: Defines the priority of the hook. Higher priority hooks + run before low priority hooks. Hooks with identical priority will + be launched in parallel. + type: integer serviceAccountName: description: ServiceAccountName Name of the serviceAccount Name used. Should only be used if your hook needs specifc RBAC Access. Otherwise diff --git a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml index cc38bfe839..29eb2ab2d5 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml @@ -1775,12 +1775,43 @@ spec: type: string jobName: type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array + readOnlyHookStatus: + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer state: description: HookState Describes the State of a Hook on a Scan type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string required: - hookName + - priority - state + - type type: object type: array state: diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 1420e79065..f89bd2c859 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sort" ) func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { @@ -32,27 +33,24 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { r.Log.Info("Found ScanCompletionHooks", "ScanCompletionHooks", len(scanCompletionHooks.Items)) - readAndWriteHooks := []executionv1.ScanCompletionHook{} - // filter all ReadAndWriteHooks in the scamCompletionHooks list for _, hook := range scanCompletionHooks.Items { + hookStatus := executionv1.HookStatus{ + HookName: hook.Name, + State: executionv1.Pending, + Priority: hook.Spec.Priority, + Type: hook.Spec.Type, + } if hook.Spec.Type == executionv1.ReadAndWrite { - readAndWriteHooks = append(readAndWriteHooks, hook) + scan.Status.ReadAndWriteHookStatus = append(scan.Status.ReadAndWriteHookStatus, hookStatus) + } else if hook.Spec.Type == executionv1.ReadOnly { + scan.Status.ReadOnlyHookStatus = append(scan.Status.ReadOnlyHookStatus, hookStatus) } } - r.Log.Info("Found ReadAndWriteHooks", "ReadAndWriteHooks", len(readAndWriteHooks)) - - hookStatus := []executionv1.HookStatus{} - - for _, hook := range readAndWriteHooks { - hookStatus = append(hookStatus, executionv1.HookStatus{ - HookName: hook.Name, - State: executionv1.Pending, - }) - } + r.Log.Info("Found ReadAndWrite Hooks", "Hooks", len(scan.Status.ReadAndWriteHookStatus)) + r.Log.Info("Found ReadOnlyHooks", "Hooks", len(scan.Status.ReadOnlyHookStatus)) scan.Status.State = "ReadAndWriteHookProcessing" - scan.Status.ReadAndWriteHookStatus = hookStatus if err := r.Status().Update(ctx, scan); err != nil { r.Log.Error(err, "unable to update Scan status") @@ -62,250 +60,266 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { return nil } -func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error { - // Get the first Hook Status which is not completed. - ctx := context.Background() - var nonCompletedHook *executionv1.HookStatus - - for _, hook := range scan.Status.ReadAndWriteHookStatus { +func getNonCompletedHooks(hooks []executionv1.HookStatus) []executionv1.HookStatus { + var nonCompletedHooks []executionv1.HookStatus + for _, hook := range hooks { if hook.State != executionv1.Completed { - nonCompletedHook = &hook - break + nonCompletedHooks = append(nonCompletedHooks, hook) } } + return nonCompletedHooks +} + +func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error { + ctx := context.Background() + + nonCompletedHooks := getNonCompletedHooks(scan.Status.ReadAndWriteHookStatus) // If nil then all hooks are done - if nonCompletedHook == nil { - scan.Status.State = "ReadAndWriteHookCompleted" + if len(nonCompletedHooks) == 0 { + r.Log.Info("All ReadAndWriteHooks have been completed", "ScanName", scan.Name) + scan.Status.State = "ReadOnlyHookProcessing" if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "unable to update Scan status") - return err - } - return nil - } - - switch nonCompletedHook.State { - case executionv1.Pending: - rawFileURL, err := r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) - if err != nil { + r.Log.Error(err, "Unable to update Scan status") return err } - findingsFileURL, err := r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) + } else if len(nonCompletedHooks) > 0 { + highestPriorityHooks, err := r.getHighestPriorityHooks(nonCompletedHooks, 1) if err != nil { return err } - rawFileUploadURL, err := r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) + updatedHook, err := r.processHook(scan, highestPriorityHooks[0]) if err != nil { return err } - findingsUploadURL, err := r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) - if err != nil { - return err + + // Update the corresponding status in the Scan object + for i, hookStatus := range scan.Status.ReadAndWriteHookStatus { + if hookStatus.HookName == updatedHook.HookName { + scan.Status.ReadAndWriteHookStatus[i] = *updatedHook + break + } } - var hook executionv1.ScanCompletionHook - if err := r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook); err != nil { - r.Log.Error(err, "Failed to get ReadAndWrite Hook for HookStatus") - return err + if updatedHook.State == executionv1.Failed || updatedHook.State == executionv1.Cancelled { + scan.Status.State = "Errored" + scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", updatedHook.HookName, updatedHook.JobName) } - jobs, err := r.getJobsForScan(scan, client.MatchingLabels{ - "securecodebox.io/job-type": "read-and-write-hook", - "securecodebox.io/hook-name": nonCompletedHook.HookName, - }) - if err != nil { + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "Unable to update Scan status") return err } - if len(jobs.Items) > 0 { - // Job already exists - return nil - } + } - jobName, err := r.createJobForHook( - &hook, - scan, - []string{ - rawFileURL, - findingsFileURL, - rawFileUploadURL, - findingsUploadURL, - }, - ) + return nil +} - // Update the currently executed hook status to "InProgress" - err = r.updateHookStatus(scan, executionv1.HookStatus{ - HookName: nonCompletedHook.HookName, - JobName: jobName, - State: executionv1.InProgress, - }) - return err - case executionv1.InProgress: - jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ - "securecodebox.io/job-type": "read-and-write-hook", - "securecodebox.io/hook-name": nonCompletedHook.HookName, - }) - if err != nil { - r.Log.Error(err, "Failed to check job status for ReadAndWrite Hook") +func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { + ctx := context.Background() + + var nonCompletedHooks = getNonCompletedHooks(scan.Status.ReadOnlyHookStatus) + + // If nil then all hooks are done + if len(nonCompletedHooks) == 0 { + r.Log.Info("Marked scan as done as all hooks have completed", "ScanName", scan.Name) + scan.Status.State = "Done" + var now = metav1.Now() + scan.Status.FinishedAt = &now + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "Unable to update Scan status") return err } - switch jobStatus { - case completed: - // Job is completed => set current Hook to completed - err = r.updateHookStatus(scan, executionv1.HookStatus{ - HookName: nonCompletedHook.HookName, - JobName: nonCompletedHook.JobName, - State: executionv1.Completed, - }) + } else if len(nonCompletedHooks) > 0 { + highestPriorityHooks, err := r.getHighestPriorityHooks(nonCompletedHooks, 0) + if err != nil { return err - case incomplete: - // Still waiting for job to finish - return nil - case failed: - for i, hookStatus := range scan.Status.ReadAndWriteHookStatus { - if hookStatus.HookName == nonCompletedHook.HookName { - scan.Status.ReadAndWriteHookStatus[i].State = executionv1.Failed - } else if hookStatus.State == executionv1.Pending { - scan.Status.ReadAndWriteHookStatus[i].State = executionv1.Cancelled + } + + for _, hook := range highestPriorityHooks { + updatedHook, err := r.processHook(scan, hook) + if err != nil { + return err + } + + for i, hookStatus := range scan.Status.ReadOnlyHookStatus { + if hookStatus.HookName == updatedHook.HookName { + scan.Status.ReadOnlyHookStatus[i] = *updatedHook + break } } - scan.Status.State = "Errored" - scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", nonCompletedHook.HookName, nonCompletedHook.JobName) - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "unable to update Scan status") - return err + + if updatedHook.State == executionv1.Failed || updatedHook.State == executionv1.Cancelled { + scan.Status.State = "Errored" + scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadOnly Hook '%s' in job '%s'. Check the logs of the hook for more information.", updatedHook.HookName, updatedHook.JobName) + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "unable to update Scan status") + return err + } + return nil } } + + r.Log.Info("All ReadOnly have been completed", "ScanName", scan.Name) + scan.Status.State = "ReadOnlyHookProcessing" + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "Unable to update Scan status") + return err + } } return nil } -func containsJobForHook(jobs *batch.JobList, hook executionv1.ScanCompletionHook) bool { - if len(jobs.Items) == 0 { - return false +func (r *ScanReconciler) getHighestPriorityHooks(hooks []executionv1.HookStatus, maxConcurrent int) ([]executionv1.HookStatus, error) { + if len(hooks) == 0 { + return hooks, nil } - for _, job := range jobs.Items { - if job.ObjectMeta.Labels["securecodebox.io/hook-name"] == hook.Name { - return true + // Sort from high to low priority + sort.Slice(hooks, func(i, j int) bool { + return hooks[i].Priority > hooks[j].Priority + }) + + // Find hooks with the highest priority + var highestPriorityHooks []executionv1.HookStatus + highestPriorityHooks = append(highestPriorityHooks, hooks[0]) + if len(hooks) > 1 { + for _, hook := range hooks[1:] { + // Check if we've reached + if maxConcurrent == len(highestPriorityHooks) { + break + } + if hook.Priority == highestPriorityHooks[0].Priority { + highestPriorityHooks = append(highestPriorityHooks, hook) + } else if hook.Priority < highestPriorityHooks[0].Priority { + break + } } } - return false + return highestPriorityHooks, nil } -func (r *ScanReconciler) startReadOnlyHooks(scan *executionv1.Scan) error { +func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook executionv1.HookStatus) (*executionv1.HookStatus, error) { ctx := context.Background() - var scanCompletionHooks executionv1.ScanCompletionHookList - - if err := r.List(ctx, &scanCompletionHooks, client.InNamespace(scan.Namespace)); err != nil { - r.Log.V(7).Info("Unable to fetch ScanCompletionHooks") - return err + var jobType string + if nonCompletedHook.Type == executionv1.ReadOnly { + jobType = "read-only-hook" + } else if nonCompletedHook.Type == executionv1.ReadAndWrite { + jobType = "read-and-write-hook" + } else { + return nil, nil } - r.Log.Info("Found ScanCompletionHooks", "ScanCompletionHooks", len(scanCompletionHooks.Items)) + r.Log.Info(fmt.Sprintf("Processing %s", jobType), "Hook", nonCompletedHook) - readOnlyHooks := []executionv1.ScanCompletionHook{} - // filter all ReadOnlyHooks in the scamCompletionHooks list - for _, hook := range scanCompletionHooks.Items { - if hook.Spec.Type == executionv1.ReadOnly { - readOnlyHooks = append(readOnlyHooks, hook) + switch nonCompletedHook.State { + case executionv1.Pending: + var hook executionv1.ScanCompletionHook + if err := r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook); err != nil { + r.Log.Error(err, "Failed to get ReadAndWrite Hook for HookStatus") + return nil, err } - } - - r.Log.Info("Found ReadOnlyHooks", "ReadOnlyHooks", len(readOnlyHooks)) - // If the readOnlyHooks list is empty, nothing more to do - if len(readOnlyHooks) == 0 { - r.Log.Info("Marked scan as done as without running ReadOnly hooks as non were configured", "ScanName", scan.Name) - scan.Status.State = "Done" - var now metav1.Time = metav1.Now() - scan.Status.FinishedAt = &now - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err + jobs, err := r.getJobsForScan(scan, client.MatchingLabels{ + "securecodebox.io/job-type": jobType, + "securecodebox.io/hook-name": nonCompletedHook.HookName, + }) + if err != nil { + return nil, err } - return nil - } - - // Get all read-only-hooks for scan to later check that they weren't already created - jobs, err := r.getJobsForScan(scan, client.MatchingLabels{ - "securecodebox.io/job-type": "read-only-hook", - }) - if err != nil { - return err - } - - for _, hook := range readOnlyHooks { - // Check if hook was already executed - if containsJobForHook(jobs, hook) == true { - r.Log.V(4).Info("Skipping creation of job for hook '%s' as it already exists", hook.Name) - // Job was already created - continue + if len(jobs.Items) > 0 { + // Job already exists + return &nonCompletedHook, nil } rawFileURL, err := r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err + return &nonCompletedHook, err } findingsFileURL, err := r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err + return &nonCompletedHook, err } - jobName, err := r.createJobForHook( - &hook, - scan, - []string{ - rawFileURL, - findingsFileURL, - }, - ) + var jobName string + switch hook.Spec.Type { + case executionv1.ReadOnly: + jobName, err = r.createJobForHook( + &hook, + scan, + []string{ + rawFileURL, + findingsFileURL, + }, + ) + if err != nil { + return &nonCompletedHook, err + } + break + case executionv1.ReadAndWrite: + rawFileUploadURL, err := r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) + if err != nil { + return &nonCompletedHook, err + } + findingsUploadURL, err := r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) + if err != nil { + return &nonCompletedHook, err + } + + jobName, err = r.createJobForHook( + &hook, + scan, + []string{ + rawFileURL, + findingsFileURL, + rawFileUploadURL, + findingsUploadURL, + }, + ) + if err != nil { + return &nonCompletedHook, err + } + break + default: + return nil, nil + } + // Update the currently executed hook job name and status to "InProgress" + nonCompletedHook.JobName = jobName + nonCompletedHook.State = executionv1.InProgress + return &nonCompletedHook, err + case executionv1.InProgress: + jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ + "securecodebox.io/job-type": jobType, + "securecodebox.io/hook-name": nonCompletedHook.HookName, + }) if err != nil { - r.Log.Error(err, "Unable to create Job for ReadOnlyHook", "job", jobName) - return err + r.Log.Error(err, "Failed to check job status for Hook") + return &nonCompletedHook, err } - } - scan.Status.State = "ReadOnlyHookProcessing" - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err - } - r.Log.Info("Started ReadOnlyHook", "ReadOnlyHookCount", len(readOnlyHooks)) - return nil -} - -func (r *ScanReconciler) checkIfReadOnlyHookIsCompleted(scan *executionv1.Scan) error { - ctx := context.Background() - readOnlyHookCompletion, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{"securecodebox.io/job-type": "read-only-hook"}) - if err != nil { - return err - } + switch jobStatus { + case completed: + // Job is completed => set current Hook to completed + nonCompletedHook.State = executionv1.Completed + return &nonCompletedHook, err + case incomplete: + // Still waiting for job to finish + return &nonCompletedHook, nil + case failed: + if nonCompletedHook.State == executionv1.Pending { + nonCompletedHook.State = executionv1.Cancelled + } else { + nonCompletedHook.State = executionv1.Failed + } - if readOnlyHookCompletion == completed { - r.Log.V(7).Info("All ReadOnlyHooks have completed") - scan.Status.State = "Done" - var now metav1.Time = metav1.Now() - scan.Status.FinishedAt = &now - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err - } - } else if readOnlyHookCompletion == failed { - r.Log.Info("At least one ReadOnlyHook failed") - scan.Status.State = "Errored" - scan.Status.ErrorDescription = "At least one ReadOnlyHook failed, check the hooks kubernetes jobs related to the scan for more details." - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err + return &nonCompletedHook, nil } } - // ReadOnlyHook(s) are still running. At least some of them are. - // Waiting until all are done. - return nil + return &nonCompletedHook, nil } func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { @@ -452,17 +466,3 @@ func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, } return job.Name, nil } - -func (r *ScanReconciler) updateHookStatus(scan *executionv1.Scan, hookStatus executionv1.HookStatus) error { - for i, hook := range scan.Status.ReadAndWriteHookStatus { - if hook.HookName == hookStatus.HookName { - scan.Status.ReadAndWriteHookStatus[i] = hookStatus - break - } - } - if err := r.Status().Update(context.Background(), scan); err != nil { - r.Log.Error(err, "unable to update Scan status") - return err - } - return nil -} diff --git a/operator/controllers/execution/scans/scan_controller.go b/operator/controllers/execution/scans/scan_controller.go index 7a8e2a11b9..8a17ff6951 100644 --- a/operator/controllers/execution/scans/scan_controller.go +++ b/operator/controllers/execution/scans/scan_controller.go @@ -101,10 +101,8 @@ func (r *ScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. err = r.setHookStatus(&scan) case "ReadAndWriteHookProcessing": err = r.executeReadAndWriteHooks(&scan) - case "ReadAndWriteHookCompleted": - err = r.startReadOnlyHooks(&scan) case "ReadOnlyHookProcessing": - err = r.checkIfReadOnlyHookIsCompleted(&scan) + err = r.executeReadOnlyHooks(&scan) } if err != nil { return ctrl.Result{}, err diff --git a/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml b/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml index 1c47676fa5..c2232230a9 100644 --- a/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml +++ b/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml @@ -25,6 +25,10 @@ spec: jsonPath: .spec.type name: Type type: string + - description: ScanCompletionHook Priority + jsonPath: .spec.priority + name: Priority + type: string - description: ScanCompletionHook Image jsonPath: .spec.image name: Image @@ -175,6 +179,12 @@ spec: type: string type: object type: array + priority: + default: 0 + description: Defines the priority of the hook. Higher priority hooks + run before low priority hooks. Hooks with identical priority will + be launched in parallel. + type: integer serviceAccountName: description: ServiceAccountName Name of the serviceAccount Name used. Should only be used if your hook needs specifc RBAC Access. Otherwise diff --git a/operator/crds/execution.securecodebox.io_scans.yaml b/operator/crds/execution.securecodebox.io_scans.yaml index 398dfd7f4e..9c30573b01 100644 --- a/operator/crds/execution.securecodebox.io_scans.yaml +++ b/operator/crds/execution.securecodebox.io_scans.yaml @@ -1782,12 +1782,43 @@ spec: type: string jobName: type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array + readOnlyHookStatus: + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer state: description: HookState Describes the State of a Hook on a Scan type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string required: - hookName + - priority - state + - type type: object type: array state: From 6d6fa10c5ea29a75a70109c53fc0e6c200c3b985 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Fri, 1 Oct 2021 23:50:28 +0200 Subject: [PATCH 02/17] Introduce heap based priority queue and use reference everywhere for optimizations Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scan_types.go | 4 +- .../execution/v1/zz_generated.deepcopy.go | 20 ++- .../execution/scans/hook_reconciler.go | 149 +++++++----------- operator/utils/priorityqueue.go | 58 +++++++ operator/utils/priorityqueue_test.go | 57 +++++++ 5 files changed, 191 insertions(+), 97 deletions(-) create mode 100644 operator/utils/priorityqueue.go create mode 100644 operator/utils/priorityqueue_test.go diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index 9ae468fd33..59c5d6e564 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -83,9 +83,9 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` - ReadAndWriteHookStatus []HookStatus `json:"readAndWriteHookStatus,omitempty"` + ReadAndWriteHookStatus []*HookStatus `json:"readAndWriteHookStatus,omitempty"` - ReadOnlyHookStatus []HookStatus `json:"readOnlyHookStatus,omitempty"` + ReadOnlyHookStatus []*HookStatus `json:"readOnlyHookStatus,omitempty"` } // HookState Describes the State of a Hook on a Scan diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index bab8c2fddb..cc5c70cd7d 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -471,13 +471,25 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { in.Findings.DeepCopyInto(&out.Findings) if in.ReadAndWriteHookStatus != nil { in, out := &in.ReadAndWriteHookStatus, &out.ReadAndWriteHookStatus - *out = make([]HookStatus, len(*in)) - copy(*out, *in) + *out = make([]*HookStatus, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(HookStatus) + **out = **in + } + } } if in.ReadOnlyHookStatus != nil { in, out := &in.ReadOnlyHookStatus, &out.ReadOnlyHookStatus - *out = make([]HookStatus, len(*in)) - copy(*out, *in) + *out = make([]*HookStatus, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(HookStatus) + **out = **in + } + } } } diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index f89bd2c859..0ab6ee78be 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -5,6 +5,7 @@ package scancontrollers import ( + "container/heap" "context" "fmt" @@ -18,7 +19,6 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sort" ) func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { @@ -41,9 +41,9 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { Type: hook.Spec.Type, } if hook.Spec.Type == executionv1.ReadAndWrite { - scan.Status.ReadAndWriteHookStatus = append(scan.Status.ReadAndWriteHookStatus, hookStatus) + scan.Status.ReadAndWriteHookStatus = append(scan.Status.ReadAndWriteHookStatus, &hookStatus) } else if hook.Spec.Type == executionv1.ReadOnly { - scan.Status.ReadOnlyHookStatus = append(scan.Status.ReadOnlyHookStatus, hookStatus) + scan.Status.ReadOnlyHookStatus = append(scan.Status.ReadOnlyHookStatus, &hookStatus) } } @@ -60,20 +60,24 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { return nil } -func getNonCompletedHooks(hooks []executionv1.HookStatus) []executionv1.HookStatus { - var nonCompletedHooks []executionv1.HookStatus - for _, hook := range hooks { +func getNonCompletedHookPriorityQueue(hooks *[]*executionv1.HookStatus) util.PriorityQueue { + var priorityQueue = make(util.PriorityQueue, 0) + for _, hook := range *hooks { if hook.State != executionv1.Completed { - nonCompletedHooks = append(nonCompletedHooks, hook) + priorityQueueItem := util.PriorityQueueItem{ + Value: hook, + Priority: hook.Priority, + } + heap.Push(&priorityQueue, &priorityQueueItem) } } - return nonCompletedHooks + return priorityQueue } func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error { ctx := context.Background() - nonCompletedHooks := getNonCompletedHooks(scan.Status.ReadAndWriteHookStatus) + nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadAndWriteHookStatus) // If nil then all hooks are done if len(nonCompletedHooks) == 0 { @@ -84,27 +88,17 @@ func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error return err } } else if len(nonCompletedHooks) > 0 { - highestPriorityHooks, err := r.getHighestPriorityHooks(nonCompletedHooks, 1) - if err != nil { - return err - } + priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) + hook := priorityQueueItem.Value.(*executionv1.HookStatus) - updatedHook, err := r.processHook(scan, highestPriorityHooks[0]) + err := r.processHook(scan, hook) if err != nil { return err } - // Update the corresponding status in the Scan object - for i, hookStatus := range scan.Status.ReadAndWriteHookStatus { - if hookStatus.HookName == updatedHook.HookName { - scan.Status.ReadAndWriteHookStatus[i] = *updatedHook - break - } - } - - if updatedHook.State == executionv1.Failed || updatedHook.State == executionv1.Cancelled { + if hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { scan.Status.State = "Errored" - scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", updatedHook.HookName, updatedHook.JobName) + scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) } if err := r.Status().Update(ctx, scan); err != nil { @@ -119,7 +113,7 @@ func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { ctx := context.Background() - var nonCompletedHooks = getNonCompletedHooks(scan.Status.ReadOnlyHookStatus) + nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadOnlyHookStatus) // If nil then all hooks are done if len(nonCompletedHooks) == 0 { @@ -132,77 +126,50 @@ func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { return err } } else if len(nonCompletedHooks) > 0 { - highestPriorityHooks, err := r.getHighestPriorityHooks(nonCompletedHooks, 0) - if err != nil { - return err - } + for { + priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) + hook := priorityQueueItem.Value.(*executionv1.HookStatus) - for _, hook := range highestPriorityHooks { - updatedHook, err := r.processHook(scan, hook) + err := r.processHook(scan, hook) if err != nil { return err } - for i, hookStatus := range scan.Status.ReadOnlyHookStatus { - if hookStatus.HookName == updatedHook.HookName { - scan.Status.ReadOnlyHookStatus[i] = *updatedHook - break - } - } - - if updatedHook.State == executionv1.Failed || updatedHook.State == executionv1.Cancelled { + if hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { scan.Status.State = "Errored" - scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadOnly Hook '%s' in job '%s'. Check the logs of the hook for more information.", updatedHook.HookName, updatedHook.JobName) + scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadOnly Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) if err := r.Status().Update(ctx, scan); err != nil { r.Log.Error(err, "unable to update Scan status") return err } return nil } - } - r.Log.Info("All ReadOnly have been completed", "ScanName", scan.Name) - scan.Status.State = "ReadOnlyHookProcessing" - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err + // Only process hooks with identical priority in parallel + if nonCompletedHooks.Len() == 0 || + nonCompletedHooks.Peek().(*util.PriorityQueueItem).Priority < priorityQueueItem.Priority { + break + } } } - return nil -} - -func (r *ScanReconciler) getHighestPriorityHooks(hooks []executionv1.HookStatus, maxConcurrent int) ([]executionv1.HookStatus, error) { - if len(hooks) == 0 { - return hooks, nil + scan.Status.State = "Completed" + for _, hook := range nonCompletedHooks { + if hook.Value.(*executionv1.HookStatus).State != executionv1.Completed { + scan.Status.State = "ReadOnlyHookProcessing" + break + } } - // Sort from high to low priority - sort.Slice(hooks, func(i, j int) bool { - return hooks[i].Priority > hooks[j].Priority - }) - - // Find hooks with the highest priority - var highestPriorityHooks []executionv1.HookStatus - highestPriorityHooks = append(highestPriorityHooks, hooks[0]) - if len(hooks) > 1 { - for _, hook := range hooks[1:] { - // Check if we've reached - if maxConcurrent == len(highestPriorityHooks) { - break - } - if hook.Priority == highestPriorityHooks[0].Priority { - highestPriorityHooks = append(highestPriorityHooks, hook) - } else if hook.Priority < highestPriorityHooks[0].Priority { - break - } - } + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "Unable to update Scan status") + return err } - return highestPriorityHooks, nil + return nil } -func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook executionv1.HookStatus) (*executionv1.HookStatus, error) { +func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error { ctx := context.Background() var jobType string @@ -211,7 +178,7 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex } else if nonCompletedHook.Type == executionv1.ReadAndWrite { jobType = "read-and-write-hook" } else { - return nil, nil + return nil } r.Log.Info(fmt.Sprintf("Processing %s", jobType), "Hook", nonCompletedHook) @@ -221,7 +188,7 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex var hook executionv1.ScanCompletionHook if err := r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook); err != nil { r.Log.Error(err, "Failed to get ReadAndWrite Hook for HookStatus") - return nil, err + return err } jobs, err := r.getJobsForScan(scan, client.MatchingLabels{ @@ -229,20 +196,20 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex "securecodebox.io/hook-name": nonCompletedHook.HookName, }) if err != nil { - return nil, err + return err } if len(jobs.Items) > 0 { // Job already exists - return &nonCompletedHook, nil + return nil } rawFileURL, err := r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return &nonCompletedHook, err + return err } findingsFileURL, err := r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return &nonCompletedHook, err + return err } var jobName string @@ -257,17 +224,17 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex }, ) if err != nil { - return &nonCompletedHook, err + return err } break case executionv1.ReadAndWrite: rawFileUploadURL, err := r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return &nonCompletedHook, err + return err } findingsUploadURL, err := r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return &nonCompletedHook, err + return err } jobName, err = r.createJobForHook( @@ -281,16 +248,16 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex }, ) if err != nil { - return &nonCompletedHook, err + return err } break default: - return nil, nil + return nil } // Update the currently executed hook job name and status to "InProgress" nonCompletedHook.JobName = jobName nonCompletedHook.State = executionv1.InProgress - return &nonCompletedHook, err + return err case executionv1.InProgress: jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, @@ -298,16 +265,16 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex }) if err != nil { r.Log.Error(err, "Failed to check job status for Hook") - return &nonCompletedHook, err + return err } switch jobStatus { case completed: // Job is completed => set current Hook to completed nonCompletedHook.State = executionv1.Completed - return &nonCompletedHook, err + return err case incomplete: // Still waiting for job to finish - return &nonCompletedHook, nil + return nil case failed: if nonCompletedHook.State == executionv1.Pending { nonCompletedHook.State = executionv1.Cancelled @@ -315,11 +282,11 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook ex nonCompletedHook.State = executionv1.Failed } - return &nonCompletedHook, nil + return nil } } - return &nonCompletedHook, nil + return nil } func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { diff --git a/operator/utils/priorityqueue.go b/operator/utils/priorityqueue.go new file mode 100644 index 0000000000..2f58262fa2 --- /dev/null +++ b/operator/utils/priorityqueue.go @@ -0,0 +1,58 @@ +package utils + +import ( + "container/heap" +) + +// Taken (mostly) from official example https://pkg.go.dev/container/heap + +// An PriorityQueueItem is something we manage in a priority queue. +type PriorityQueueItem struct { + Value interface{} + Priority int // The Priority of the item in the queue. + Index int // The Index of the item in the heap. +} + +// A PriorityQueue implements heap.Interface and holds Items. +type PriorityQueue []*PriorityQueueItem + +func (pq PriorityQueue) Len() int { return len(pq) } + +func (pq PriorityQueue) Less(i, j int) bool { + // We want Pop to give us the highest, not lowest, priority so we use greater than here. + return pq[i].Priority > pq[j].Priority +} + +func (pq PriorityQueue) Swap(i, j int) { + pq[i], pq[j] = pq[j], pq[i] + pq[i].Index = i + pq[j].Index = j +} + +func (pq *PriorityQueue) Push(x interface{}) { + n := len(*pq) + item := x.(*PriorityQueueItem) + item.Index = n + *pq = append(*pq, item) +} + +func (pq *PriorityQueue) Peek() interface{} { + return (*pq)[0] +} + +func (pq *PriorityQueue) Pop() interface{} { + old := *pq + n := len(old) + item := old[n-1] + old[n-1] = nil // avoid memory leak + item.Index = -1 // for safety + *pq = old[0 : n-1] + return item +} + +// update modifies the priority and Value of an PriorityQueueItem in the queue. +func (pq *PriorityQueue) update(item *PriorityQueueItem, value interface{}, priority int) { + item.Value = value + item.Priority = priority + heap.Fix(pq, item.Index) +} diff --git a/operator/utils/priorityqueue_test.go b/operator/utils/priorityqueue_test.go new file mode 100644 index 0000000000..a955fdf78f --- /dev/null +++ b/operator/utils/priorityqueue_test.go @@ -0,0 +1,57 @@ +package utils + +import ( + "container/heap" + "fmt" + "testing" +) + +var items = []PriorityQueueItem{ + { + Value: "banana", + Priority: 3, + }, + { + Value: "apple", + Priority: 2, + }, + { + Value: "pear", + Priority: 4, + }, +} + +func TestQueue(t *testing.T) { + pq := make(PriorityQueue, 0) + for i, _ := range items { + heap.Push(&pq, &items[i]) + } + + // Take the items out; they should arrive in decreasing priority order. + var item1peek = pq.Peek() + var item1 = heap.Pop(&pq).(*PriorityQueueItem) + if item1peek != item1 { + t.Error(fmt.Errorf("peek didn't equal pop")) + } + if item1.Value != items[2].Value { + t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item1.Value, item1.Priority, items[2].Value, items[2].Priority)) + } + + var item2peek = pq.Peek() + var item2 = heap.Pop(&pq).(*PriorityQueueItem) + if item2peek != item2 { + t.Error(fmt.Errorf("peek didn't equal pop")) + } + if item2.Value != items[0].Value { + t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item2.Value, item2.Priority, items[0].Value, items[0].Priority)) + } + + var item3peek = pq.Peek() + var item3 = heap.Pop(&pq).(*PriorityQueueItem) + if item3peek != item3 { + t.Error(fmt.Errorf("peek didn't equal pop")) + } + if item3.Value != items[1].Value { + t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item3.Value, item3.Priority, items[1].Value, items[1].Priority)) + } +} From b90a567e27a8591e88034a0d454700419f6c7a35 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Sat, 2 Oct 2021 12:14:57 +0200 Subject: [PATCH 03/17] pq usage optimization (less append calls) Signed-off-by: Jop Zitman pq usage optimization (less append calls) Signed-off-by: Jop Zitman --- operator/controllers/execution/scans/hook_reconciler.go | 8 ++++++-- operator/utils/priorityqueue_test.go | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 0ab6ee78be..96b0eb78e5 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -61,16 +61,20 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { } func getNonCompletedHookPriorityQueue(hooks *[]*executionv1.HookStatus) util.PriorityQueue { - var priorityQueue = make(util.PriorityQueue, 0) + var priorityQueue = make(util.PriorityQueue, len(*hooks)) + var completedHooks = 0 for _, hook := range *hooks { if hook.State != executionv1.Completed { priorityQueueItem := util.PriorityQueueItem{ Value: hook, Priority: hook.Priority, } - heap.Push(&priorityQueue, &priorityQueueItem) + priorityQueue[completedHooks] = &priorityQueueItem + completedHooks++ } } + priorityQueue = priorityQueue[:completedHooks] + heap.Init(&priorityQueue) return priorityQueue } diff --git a/operator/utils/priorityqueue_test.go b/operator/utils/priorityqueue_test.go index a955fdf78f..fa6b9f4ee2 100644 --- a/operator/utils/priorityqueue_test.go +++ b/operator/utils/priorityqueue_test.go @@ -22,10 +22,12 @@ var items = []PriorityQueueItem{ } func TestQueue(t *testing.T) { - pq := make(PriorityQueue, 0) + pq := make(PriorityQueue, len(items) + 5) // Test overallocation for i, _ := range items { - heap.Push(&pq, &items[i]) + pq[i] = &items[i] } + pq = pq[:len(items)] + heap.Init(&pq) // Take the items out; they should arrive in decreasing priority order. var item1peek = pq.Peek() From 27d5d966a67aeb0029f7b6f8e02fff29852a0e7a Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Sat, 2 Oct 2021 13:06:22 +0200 Subject: [PATCH 04/17] More "global" error handling Signed-off-by: Jop Zitman --- .../execution/scans/hook_reconciler.go | 135 +++++++++--------- 1 file changed, 65 insertions(+), 70 deletions(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 96b0eb78e5..7e5f898a6f 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -83,35 +83,39 @@ func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadAndWriteHookStatus) + var err error // If nil then all hooks are done if len(nonCompletedHooks) == 0 { r.Log.Info("All ReadAndWriteHooks have been completed", "ScanName", scan.Name) scan.Status.State = "ReadOnlyHookProcessing" - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err - } } else if len(nonCompletedHooks) > 0 { priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) hook := priorityQueueItem.Value.(*executionv1.HookStatus) - err := r.processHook(scan, hook) - if err != nil { - return err - } - - if hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { + err = r.processHook(scan, hook) + if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { scan.Status.State = "Errored" scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) } + } - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err + if err == nil && scan.Status.State == "ReadAndWriteHookProcessing" { + // Check if there are now still incomplete hooks + scan.Status.State = "ReadOnlyHookProcessing" + for _, hook := range scan.Status.ReadAndWriteHookStatus { + if hook.State != executionv1.Completed { + scan.Status.State = "ReadAndWriteHookProcessing" + break + } } } - return nil + if sErr := r.Status().Update(ctx, scan); sErr != nil { + r.Log.Error(sErr, "Unable to update Scan status") + return sErr + } + + return err } func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { @@ -119,34 +123,23 @@ func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadOnlyHookStatus) + var err error // If nil then all hooks are done if len(nonCompletedHooks) == 0 { r.Log.Info("Marked scan as done as all hooks have completed", "ScanName", scan.Name) scan.Status.State = "Done" var now = metav1.Now() scan.Status.FinishedAt = &now - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err - } } else if len(nonCompletedHooks) > 0 { for { priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) hook := priorityQueueItem.Value.(*executionv1.HookStatus) - err := r.processHook(scan, hook) - if err != nil { - return err - } - - if hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { + err = r.processHook(scan, hook) + if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { scan.Status.State = "Errored" scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadOnly Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "unable to update Scan status") - return err - } - return nil + break } // Only process hooks with identical priority in parallel @@ -157,20 +150,23 @@ func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { } } - scan.Status.State = "Completed" - for _, hook := range nonCompletedHooks { - if hook.Value.(*executionv1.HookStatus).State != executionv1.Completed { - scan.Status.State = "ReadOnlyHookProcessing" - break + if err == nil && scan.Status.State == "ReadOnlyHookProcessing" { + // Check if there are now still incomplete hooks + scan.Status.State = "Completed" + for _, hook := range scan.Status.ReadOnlyHookStatus { + if hook.State != executionv1.Completed { + scan.Status.State = "ReadOnlyHookProcessing" + break + } } } - if err := r.Status().Update(ctx, scan); err != nil { - r.Log.Error(err, "Unable to update Scan status") - return err + if sErr := r.Status().Update(ctx, scan); sErr != nil { + r.Log.Error(sErr, "Unable to update Scan status") + return sErr } - return nil + return err } func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error { @@ -181,39 +177,41 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e jobType = "read-only-hook" } else if nonCompletedHook.Type == executionv1.ReadAndWrite { jobType = "read-and-write-hook" - } else { - return nil } r.Log.Info(fmt.Sprintf("Processing %s", jobType), "Hook", nonCompletedHook) + var err error switch nonCompletedHook.State { case executionv1.Pending: var hook executionv1.ScanCompletionHook - if err := r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook); err != nil { + err = r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook) + if err != nil { r.Log.Error(err, "Failed to get ReadAndWrite Hook for HookStatus") - return err + break } - jobs, err := r.getJobsForScan(scan, client.MatchingLabels{ + var jobs *batch.JobList + jobs, err = r.getJobsForScan(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, "securecodebox.io/hook-name": nonCompletedHook.HookName, }) if err != nil { - return err + break } if len(jobs.Items) > 0 { - // Job already exists - return nil + break // Job already exists } - rawFileURL, err := r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) + var rawFileURL string + rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err + break } - findingsFileURL, err := r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) + var findingsFileURL string + findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err + break } var jobName string @@ -227,18 +225,16 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e findingsFileURL, }, ) - if err != nil { - return err - } - break case executionv1.ReadAndWrite: - rawFileUploadURL, err := r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) + var rawFileUploadURL string + rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err + break } - findingsUploadURL, err := r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) + var findingsUploadURL string + findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err + break } jobName, err = r.createJobForHook( @@ -251,46 +247,45 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e findingsUploadURL, }, ) - if err != nil { - return err - } + } + if err != nil { break - default: - return nil } + // Update the currently executed hook job name and status to "InProgress" nonCompletedHook.JobName = jobName nonCompletedHook.State = executionv1.InProgress return err case executionv1.InProgress: - jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ + var jobStatus jobCompletionType + jobStatus, err = r.checkIfJobIsCompleted(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, "securecodebox.io/hook-name": nonCompletedHook.HookName, }) if err != nil { r.Log.Error(err, "Failed to check job status for Hook") - return err + break } switch jobStatus { case completed: // Job is completed => set current Hook to completed nonCompletedHook.State = executionv1.Completed - return err case incomplete: // Still waiting for job to finish - return nil case failed: if nonCompletedHook.State == executionv1.Pending { nonCompletedHook.State = executionv1.Cancelled } else { nonCompletedHook.State = executionv1.Failed } - - return nil } } - return nil + if err != nil { + r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook) + } + + return err } func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { From 120137f86f6d00aee071ca57aa6e40d2ef68b72c Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Sat, 2 Oct 2021 13:20:24 +0200 Subject: [PATCH 05/17] Fix log msg condition Signed-off-by: Jop Zitman --- operator/controllers/execution/scans/hook_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 7e5f898a6f..daa87a8da2 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -281,7 +281,7 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e } } - if err != nil { + if err == nil { r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook) } From a0f2c92a91110db856246ee5486536c716a850a3 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Fri, 8 Oct 2021 14:18:56 +0200 Subject: [PATCH 06/17] Reimplement hook priorities Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scan_types.go | 4 +- .../execution/v1/zz_generated.deepcopy.go | 15 +-- .../execution.securecodebox.io_scans.yaml | 49 +++------ .../execution/scans/hook_reconciler.go | 103 ++++-------------- .../execution/scans/scan_controller.go | 6 +- .../execution.securecodebox.io_scans.yaml | 49 +++------ ...{priorityqueue.go => hookpriorityqueue.go} | 47 ++++++-- operator/utils/hookpriorityqueue_test.go | 90 +++++++++++++++ operator/utils/priorityqueue_test.go | 59 ---------- 9 files changed, 179 insertions(+), 243 deletions(-) rename operator/utils/{priorityqueue.go => hookpriorityqueue.go} (52%) create mode 100644 operator/utils/hookpriorityqueue_test.go delete mode 100644 operator/utils/priorityqueue_test.go diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index 59c5d6e564..69a77d15f0 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -83,9 +83,7 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` - ReadAndWriteHookStatus []*HookStatus `json:"readAndWriteHookStatus,omitempty"` - - ReadOnlyHookStatus []*HookStatus `json:"readOnlyHookStatus,omitempty"` + HookStatuses []*HookStatus `json:"hookStatuses,omitempty"` } // HookState Describes the State of a Hook on a Scan diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index cc5c70cd7d..9b2e48bb90 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -469,19 +469,8 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { *out = (*in).DeepCopy() } in.Findings.DeepCopyInto(&out.Findings) - if in.ReadAndWriteHookStatus != nil { - in, out := &in.ReadAndWriteHookStatus, &out.ReadAndWriteHookStatus - *out = make([]*HookStatus, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(HookStatus) - **out = **in - } - } - } - if in.ReadOnlyHookStatus != nil { - in, out := &in.ReadOnlyHookStatus, &out.ReadOnlyHookStatus + if in.HookStatuses != nil { + in, out := &in.HookStatuses, &out.HookStatuses *out = make([]*HookStatus, len(*in)) for i := range *in { if (*in)[i] != nil { diff --git a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml index 29eb2ab2d5..fd1b995b6d 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml @@ -1756,42 +1756,7 @@ spec: parser & hooks) has been marked as "Done" format: date-time type: string - rawResultDownloadLink: - description: RawResultDownloadLink link to download the raw result - file from. Valid for 7 days - type: string - rawResultFile: - description: RawResultFile Filename of the result file of the scanner. - e.g. `nmap-result.xml` - type: string - rawResultType: - description: RawResultType determines which kind of ParseDefinition - will be used to turn the raw results of the scanner into findings - type: string - readAndWriteHookStatus: - items: - properties: - hookName: - type: string - jobName: - type: string - priority: - type: integer - state: - description: HookState Describes the State of a Hook on a Scan - type: string - type: - description: HookType Defines weather the hook should be able - to change the findings or is run in a read only mode. - type: string - required: - - hookName - - priority - - state - - type - type: object - type: array - readOnlyHookStatus: + hookStatuses: items: properties: hookName: @@ -1814,6 +1779,18 @@ spec: - type type: object type: array + rawResultDownloadLink: + description: RawResultDownloadLink link to download the raw result + file from. Valid for 7 days + type: string + rawResultFile: + description: RawResultFile Filename of the result file of the scanner. + e.g. `nmap-result.xml` + type: string + rawResultType: + description: RawResultType determines which kind of ParseDefinition + will be used to turn the raw results of the scanner into findings + type: string state: type: string type: object diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index daa87a8da2..98fcc69315 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -34,23 +34,16 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { r.Log.Info("Found ScanCompletionHooks", "ScanCompletionHooks", len(scanCompletionHooks.Items)) for _, hook := range scanCompletionHooks.Items { - hookStatus := executionv1.HookStatus{ + hookStatus := &executionv1.HookStatus{ HookName: hook.Name, State: executionv1.Pending, - Priority: hook.Spec.Priority, Type: hook.Spec.Type, + Priority: hook.Spec.Priority, } - if hook.Spec.Type == executionv1.ReadAndWrite { - scan.Status.ReadAndWriteHookStatus = append(scan.Status.ReadAndWriteHookStatus, &hookStatus) - } else if hook.Spec.Type == executionv1.ReadOnly { - scan.Status.ReadOnlyHookStatus = append(scan.Status.ReadOnlyHookStatus, &hookStatus) - } + scan.Status.HookStatuses = append(scan.Status.HookStatuses, hookStatus) } - r.Log.Info("Found ReadAndWrite Hooks", "Hooks", len(scan.Status.ReadAndWriteHookStatus)) - r.Log.Info("Found ReadOnlyHooks", "Hooks", len(scan.Status.ReadOnlyHookStatus)) - - scan.Status.State = "ReadAndWriteHookProcessing" + scan.Status.State = "HookProcessing" if err := r.Status().Update(ctx, scan); err != nil { r.Log.Error(err, "unable to update Scan status") @@ -60,68 +53,15 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { return nil } -func getNonCompletedHookPriorityQueue(hooks *[]*executionv1.HookStatus) util.PriorityQueue { - var priorityQueue = make(util.PriorityQueue, len(*hooks)) - var completedHooks = 0 - for _, hook := range *hooks { - if hook.State != executionv1.Completed { - priorityQueueItem := util.PriorityQueueItem{ - Value: hook, - Priority: hook.Priority, - } - priorityQueue[completedHooks] = &priorityQueueItem - completedHooks++ - } - } - priorityQueue = priorityQueue[:completedHooks] - heap.Init(&priorityQueue) - return priorityQueue -} - -func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error { - ctx := context.Background() - - nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadAndWriteHookStatus) - - var err error - // If nil then all hooks are done - if len(nonCompletedHooks) == 0 { - r.Log.Info("All ReadAndWriteHooks have been completed", "ScanName", scan.Name) - scan.Status.State = "ReadOnlyHookProcessing" - } else if len(nonCompletedHooks) > 0 { - priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) - hook := priorityQueueItem.Value.(*executionv1.HookStatus) - - err = r.processHook(scan, hook) - if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { - scan.Status.State = "Errored" - scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadAndWrite Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) - } - } - - if err == nil && scan.Status.State == "ReadAndWriteHookProcessing" { - // Check if there are now still incomplete hooks - scan.Status.State = "ReadOnlyHookProcessing" - for _, hook := range scan.Status.ReadAndWriteHookStatus { - if hook.State != executionv1.Completed { - scan.Status.State = "ReadAndWriteHookProcessing" - break - } - } - } - - if sErr := r.Status().Update(ctx, scan); sErr != nil { - r.Log.Error(sErr, "Unable to update Scan status") - return sErr - } - - return err -} - -func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { +func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { ctx := context.Background() - nonCompletedHooks := getNonCompletedHookPriorityQueue(&scan.Status.ReadOnlyHookStatus) + nonCompletedHooks := util.PriorityQueueFromSlice( + &scan.Status.HookStatuses, + func(hook *executionv1.HookStatus) bool { + return hook.State != executionv1.Completed + }, + ) var err error // If nil then all hooks are done @@ -132,30 +72,33 @@ func (r *ScanReconciler) executeReadOnlyHooks(scan *executionv1.Scan) error { scan.Status.FinishedAt = &now } else if len(nonCompletedHooks) > 0 { for { - priorityQueueItem := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem) - hook := priorityQueueItem.Value.(*executionv1.HookStatus) + hook := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem).Value err = r.processHook(scan, hook) if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { scan.Status.State = "Errored" - scan.Status.ErrorDescription = fmt.Sprintf("Failed to execute ReadOnly Hook '%s' in job '%s'. Check the logs of the hook for more information.", hook.HookName, hook.JobName) + 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) + break + } + + // ReadAndWrite hooks may not be run in parallel + if nonCompletedHooks.Len() == 0 || hook.Type == executionv1.ReadAndWrite { break } - // Only process hooks with identical priority in parallel - if nonCompletedHooks.Len() == 0 || - nonCompletedHooks.Peek().(*util.PriorityQueueItem).Priority < priorityQueueItem.Priority { + next := nonCompletedHooks.Peek().(*util.PriorityQueueItem).Value + if next.Priority < hook.Priority { break } } } - if err == nil && scan.Status.State == "ReadOnlyHookProcessing" { + if err == nil && scan.Status.State == "HookProcessing" { // Check if there are now still incomplete hooks scan.Status.State = "Completed" - for _, hook := range scan.Status.ReadOnlyHookStatus { + for _, hook := range scan.Status.HookStatuses { if hook.State != executionv1.Completed { - scan.Status.State = "ReadOnlyHookProcessing" + scan.Status.State = "HookProcessing" break } } diff --git a/operator/controllers/execution/scans/scan_controller.go b/operator/controllers/execution/scans/scan_controller.go index 8a17ff6951..0707934a03 100644 --- a/operator/controllers/execution/scans/scan_controller.go +++ b/operator/controllers/execution/scans/scan_controller.go @@ -99,10 +99,8 @@ func (r *ScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. err = r.checkIfParsingIsCompleted(&scan) case "ParseCompleted": err = r.setHookStatus(&scan) - case "ReadAndWriteHookProcessing": - err = r.executeReadAndWriteHooks(&scan) - case "ReadOnlyHookProcessing": - err = r.executeReadOnlyHooks(&scan) + case "HookProcessing": + err = r.executeHooks(&scan) } if err != nil { return ctrl.Result{}, err diff --git a/operator/crds/execution.securecodebox.io_scans.yaml b/operator/crds/execution.securecodebox.io_scans.yaml index 9c30573b01..2ab8ba2615 100644 --- a/operator/crds/execution.securecodebox.io_scans.yaml +++ b/operator/crds/execution.securecodebox.io_scans.yaml @@ -1763,42 +1763,7 @@ spec: parser & hooks) has been marked as "Done" format: date-time type: string - rawResultDownloadLink: - description: RawResultDownloadLink link to download the raw result - file from. Valid for 7 days - type: string - rawResultFile: - description: RawResultFile Filename of the result file of the scanner. - e.g. `nmap-result.xml` - type: string - rawResultType: - description: RawResultType determines which kind of ParseDefinition - will be used to turn the raw results of the scanner into findings - type: string - readAndWriteHookStatus: - items: - properties: - hookName: - type: string - jobName: - type: string - priority: - type: integer - state: - description: HookState Describes the State of a Hook on a Scan - type: string - type: - description: HookType Defines weather the hook should be able - to change the findings or is run in a read only mode. - type: string - required: - - hookName - - priority - - state - - type - type: object - type: array - readOnlyHookStatus: + hookStatuses: items: properties: hookName: @@ -1821,6 +1786,18 @@ spec: - type type: object type: array + rawResultDownloadLink: + description: RawResultDownloadLink link to download the raw result + file from. Valid for 7 days + type: string + rawResultFile: + description: RawResultFile Filename of the result file of the scanner. + e.g. `nmap-result.xml` + type: string + rawResultType: + description: RawResultType determines which kind of ParseDefinition + will be used to turn the raw results of the scanner into findings + type: string state: type: string type: object diff --git a/operator/utils/priorityqueue.go b/operator/utils/hookpriorityqueue.go similarity index 52% rename from operator/utils/priorityqueue.go rename to operator/utils/hookpriorityqueue.go index 2f58262fa2..3552f9b076 100644 --- a/operator/utils/priorityqueue.go +++ b/operator/utils/hookpriorityqueue.go @@ -2,13 +2,14 @@ package utils import ( "container/heap" + executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" ) // Taken (mostly) from official example https://pkg.go.dev/container/heap // An PriorityQueueItem is something we manage in a priority queue. type PriorityQueueItem struct { - Value interface{} + Value *executionv1.HookStatus Priority int // The Priority of the item in the queue. Index int // The Index of the item in the heap. } @@ -19,8 +20,19 @@ type PriorityQueue []*PriorityQueueItem func (pq PriorityQueue) Len() int { return len(pq) } func (pq PriorityQueue) Less(i, j int) bool { - // We want Pop to give us the highest, not lowest, priority so we use greater than here. - return pq[i].Priority > pq[j].Priority + // We want Pop to give us the highest, not lowest, priority so we invert result + + if pq[i].Priority == pq[j].Priority { + if pq[i].Value.Type == pq[j].Value.Type { + // Prefer hook if it is in progress + return pq[i].Value.State == executionv1.InProgress + } else { + // Prefer ReadAndWriteHooks + return pq[i].Value.Type == executionv1.ReadAndWrite + } + } else { + return pq[i].Priority > pq[j].Priority + } } func (pq PriorityQueue) Swap(i, j int) { @@ -36,10 +48,6 @@ func (pq *PriorityQueue) Push(x interface{}) { *pq = append(*pq, item) } -func (pq *PriorityQueue) Peek() interface{} { - return (*pq)[0] -} - func (pq *PriorityQueue) Pop() interface{} { old := *pq n := len(old) @@ -50,9 +58,24 @@ func (pq *PriorityQueue) Pop() interface{} { return item } -// update modifies the priority and Value of an PriorityQueueItem in the queue. -func (pq *PriorityQueue) update(item *PriorityQueueItem, value interface{}, priority int) { - item.Value = value - item.Priority = priority - heap.Fix(pq, item.Index) +func (pq *PriorityQueue) Peek() interface{} { + return (*pq)[0] +} + +func PriorityQueueFromSlice(hooks *[]*executionv1.HookStatus, conditional func(hook *executionv1.HookStatus) bool) PriorityQueue { + var priorityQueue = make(PriorityQueue, len(*hooks)) + var completedHooks = 0 + for _, hook := range *hooks { + if conditional(hook) { + priorityQueueItem := PriorityQueueItem{ + Value: hook, + Priority: hook.Priority, + } + priorityQueue[completedHooks] = &priorityQueueItem + completedHooks++ + } + } + priorityQueue = priorityQueue[:completedHooks] + heap.Init(&priorityQueue) + return priorityQueue } diff --git a/operator/utils/hookpriorityqueue_test.go b/operator/utils/hookpriorityqueue_test.go new file mode 100644 index 0000000000..fd5ee364b8 --- /dev/null +++ b/operator/utils/hookpriorityqueue_test.go @@ -0,0 +1,90 @@ +package utils + +import ( + "container/heap" + "fmt" + executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" + "testing" +) + +func TestQueuePriority(t *testing.T) { + var items = []*executionv1.HookStatus{ + { + HookName: "banana", + Priority: 3, + State: executionv1.Pending, + }, + { + HookName: "apple", + Priority: 2, + State: executionv1.InProgress, + }, + { + HookName: "pear", + Priority: 4, + State: executionv1.Failed, + }, + { + HookName: "kiwi", + Priority: 4, + State: executionv1.Completed, + }, + } + + pq := PriorityQueueFromSlice(&items, hookIsNotCompleted) + + // Take the items out; they should arrive in decreasing priority order. + assertTopEqual(t, &pq, items[2]) + assertTopEqual(t, &pq, items[0]) + assertTopEqual(t, &pq, items[1]) +} + +func TestQueuePool(t *testing.T) { + var hookStatuses = []*executionv1.HookStatus{ + { + HookName: "banana", + State: executionv1.InProgress, + Type: executionv1.ReadOnly, + Priority: 1, + }, + { + HookName: "apple", + State: executionv1.InProgress, + Type: executionv1.ReadAndWrite, + Priority: 1, + }, + { + HookName: "pear", + State: executionv1.Pending, + Type: executionv1.ReadAndWrite, + Priority: 1, + }, + } + + pq := PriorityQueueFromSlice(&hookStatuses, hookIsNotCompleted) + + // Take the items out; they should arrive in decreasing priority order. + assertTopEqual(t, &pq, hookStatuses[1]) + assertTopEqual(t, &pq, hookStatuses[2]) + assertTopEqual(t, &pq, hookStatuses[0]) +} + +func hookIsNotCompleted(hook *executionv1.HookStatus) bool { + return hook.State != executionv1.Completed +} + +func assertTopEqual(t *testing.T, queue *PriorityQueue, expected *executionv1.HookStatus) { + assertEqual(t, queue.Peek().(*PriorityQueueItem).Value, expected) + assertEqual(t, heap.Pop(queue).(*PriorityQueueItem).Value, expected) +} + +func assertEqual(t *testing.T, received *executionv1.HookStatus, expected *executionv1.HookStatus) { + if received != expected { + t.Fatal( + fmt.Errorf("received '%s' '%s' priority '%d' hook '%s' expecting '%s' '%s' priority '%d' hook '%s'", + received.State, received.Type, received.Priority, received.HookName, + expected.State, expected.Type, received.Priority, expected.HookName, + ), + ) + } +} diff --git a/operator/utils/priorityqueue_test.go b/operator/utils/priorityqueue_test.go deleted file mode 100644 index fa6b9f4ee2..0000000000 --- a/operator/utils/priorityqueue_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package utils - -import ( - "container/heap" - "fmt" - "testing" -) - -var items = []PriorityQueueItem{ - { - Value: "banana", - Priority: 3, - }, - { - Value: "apple", - Priority: 2, - }, - { - Value: "pear", - Priority: 4, - }, -} - -func TestQueue(t *testing.T) { - pq := make(PriorityQueue, len(items) + 5) // Test overallocation - for i, _ := range items { - pq[i] = &items[i] - } - pq = pq[:len(items)] - heap.Init(&pq) - - // Take the items out; they should arrive in decreasing priority order. - var item1peek = pq.Peek() - var item1 = heap.Pop(&pq).(*PriorityQueueItem) - if item1peek != item1 { - t.Error(fmt.Errorf("peek didn't equal pop")) - } - if item1.Value != items[2].Value { - t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item1.Value, item1.Priority, items[2].Value, items[2].Priority)) - } - - var item2peek = pq.Peek() - var item2 = heap.Pop(&pq).(*PriorityQueueItem) - if item2peek != item2 { - t.Error(fmt.Errorf("peek didn't equal pop")) - } - if item2.Value != items[0].Value { - t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item2.Value, item2.Priority, items[0].Value, items[0].Priority)) - } - - var item3peek = pq.Peek() - var item3 = heap.Pop(&pq).(*PriorityQueueItem) - if item3peek != item3 { - t.Error(fmt.Errorf("peek didn't equal pop")) - } - if item3.Value != items[1].Value { - t.Error(fmt.Errorf("received %s with priority %d expecting %s with priority %d", item3.Value, item3.Priority, items[1].Value, items[1].Priority)) - } -} From 0c12033cecb0f3e71cc79fbe37f2f878393b8d62 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Mon, 11 Oct 2021 12:53:40 +0200 Subject: [PATCH 07/17] Split up processHook function Signed-off-by: Jop Zitman --- .../execution/scans/hook_reconciler.go | 174 +++++++++--------- 1 file changed, 85 insertions(+), 89 deletions(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 98fcc69315..9a7153e05e 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -113,8 +113,6 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { } func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error { - ctx := context.Background() - var jobType string if nonCompletedHook.Type == executionv1.ReadOnly { jobType = "read-only-hook" @@ -127,110 +125,108 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e var err error switch nonCompletedHook.State { case executionv1.Pending: - var hook executionv1.ScanCompletionHook - err = r.Get(ctx, types.NamespacedName{Name: nonCompletedHook.HookName, Namespace: scan.Namespace}, &hook) - if err != nil { - r.Log.Error(err, "Failed to get ReadAndWrite Hook for HookStatus") - break - } + err = r.processPendingHook(scan, nonCompletedHook, jobType) + case executionv1.InProgress: + err = r.processInProgressHook(scan, nonCompletedHook, jobType) + } - var jobs *batch.JobList - jobs, err = r.getJobsForScan(scan, client.MatchingLabels{ - "securecodebox.io/job-type": jobType, - "securecodebox.io/hook-name": nonCompletedHook.HookName, - }) - if err != nil { - break - } - if len(jobs.Items) > 0 { - break // Job already exists - } + if err == nil { + r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook) + } - var rawFileURL string - rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) - if err != nil { - break - } - var findingsFileURL string - findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) - if err != nil { - break - } + return err +} - var jobName string - switch hook.Spec.Type { - case executionv1.ReadOnly: - jobName, err = r.createJobForHook( - &hook, - scan, - []string{ - rawFileURL, - findingsFileURL, - }, - ) - case executionv1.ReadAndWrite: - var rawFileUploadURL string - rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) - if err != nil { - break - } - var findingsUploadURL string - findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) - if err != nil { - break - } +func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) error { + ctx := context.Background() - jobName, err = r.createJobForHook( - &hook, - scan, - []string{ - rawFileURL, - findingsFileURL, - rawFileUploadURL, - findingsUploadURL, - }, - ) - } - if err != nil { - break - } + var hook executionv1.ScanCompletionHook + var err error + err = r.Get(ctx, types.NamespacedName{Name: pendingHook.HookName, Namespace: scan.Namespace}, &hook) + if err != nil { + r.Log.Error(err, "Failed to get Hook for HookStatus") + return err + } - // Update the currently executed hook job name and status to "InProgress" - nonCompletedHook.JobName = jobName - nonCompletedHook.State = executionv1.InProgress + var jobs *batch.JobList + jobs, err = r.getJobsForScan(scan, client.MatchingLabels{ + "securecodebox.io/job-type": jobType, + "securecodebox.io/hook-name": pendingHook.HookName, + }) + if err != nil || len(jobs.Items) > 0 { return err - case executionv1.InProgress: - var jobStatus jobCompletionType - jobStatus, err = r.checkIfJobIsCompleted(scan, client.MatchingLabels{ - "securecodebox.io/job-type": jobType, - "securecodebox.io/hook-name": nonCompletedHook.HookName, - }) + } + + var rawFileURL string + rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) + if err != nil { + return err + } + var findingsFileURL string + findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) + if err != nil { + return err + } + + var args = []string{ + rawFileURL, + findingsFileURL, + } + if hook.Spec.Type == executionv1.ReadAndWrite { + var rawFileUploadURL string + rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - r.Log.Error(err, "Failed to check job status for Hook") - break + return err } - switch jobStatus { - case completed: - // Job is completed => set current Hook to completed - nonCompletedHook.State = executionv1.Completed - case incomplete: - // Still waiting for job to finish - case failed: - if nonCompletedHook.State == executionv1.Pending { - nonCompletedHook.State = executionv1.Cancelled - } else { - nonCompletedHook.State = executionv1.Failed - } + var findingsUploadURL string + findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) + if err != nil { + return err } + args = append(args, rawFileUploadURL, findingsUploadURL) } + var jobName string + jobName, err = r.createJobForHook( + &hook, + scan, + args, + ) + if err == nil { - r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook) + // Update the currently executed hook job name and status to "InProgress" + pendingHook.JobName = jobName + pendingHook.State = executionv1.InProgress } return err } +func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) error { + jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ + "securecodebox.io/job-type": jobType, + "securecodebox.io/hook-name": inProgressHook.HookName, + }) + if err != nil { + r.Log.Error(err, "Failed to check job status for Hook") + return err + } + switch jobStatus { + case completed: + // Job is completed => set current Hook to completed + inProgressHook.State = executionv1.Completed + case incomplete: + // Still waiting for job to finish + case failed: + if inProgressHook.State == executionv1.Pending { + inProgressHook.State = executionv1.Cancelled + } else { + inProgressHook.State = executionv1.Failed + } + } + return nil +} + func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { ctx := context.Background() From bd6219d299419de681ff25ae510ab170bd578945 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Mon, 11 Oct 2021 12:58:29 +0200 Subject: [PATCH 08/17] Update priority description Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scancompletionhook_types.go | 2 +- .../execution.securecodebox.io_scancompletionhooks.yaml | 7 ++++--- .../execution.securecodebox.io_scancompletionhooks.yaml | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/operator/apis/execution/v1/scancompletionhook_types.go b/operator/apis/execution/v1/scancompletionhook_types.go index 9d057702a4..d7beb87ee8 100644 --- a/operator/apis/execution/v1/scancompletionhook_types.go +++ b/operator/apis/execution/v1/scancompletionhook_types.go @@ -30,7 +30,7 @@ type ScanCompletionHookSpec struct { // Defines weather the hook should be able to change the findings or is run in a read only mode. Type HookType `json:"type"` - // Defines the priority of the hook. Higher priority hooks run before low priority hooks. Hooks with identical priority will be launched in parallel. + // Higher priority hooks run before low priority hooks. Within a priority class ReadAndWrite hooks are started before ReadOnly hooks, ReadAndWrite hooks wil be launched in serial, and ReadOnly hooks will be launched in parallel. // +kubebuilder:default=0 // +kubebuilder:validation:Optional Priority int `json:"priority"` diff --git a/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml b/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml index c3bf1a7fda..2179549b24 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scancompletionhooks.yaml @@ -180,9 +180,10 @@ spec: type: array priority: default: 0 - description: Defines the priority of the hook. Higher priority hooks - run before low priority hooks. Hooks with identical priority will - be launched in parallel. + description: Higher priority hooks run before low priority hooks. + Within a priority class ReadAndWrite hooks are started before ReadOnly + hooks, ReadAndWrite hooks wil be launched in serial, and ReadOnly + hooks will be launched in parallel. type: integer serviceAccountName: description: ServiceAccountName Name of the serviceAccount Name used. diff --git a/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml b/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml index c2232230a9..1c33cfb36e 100644 --- a/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml +++ b/operator/crds/execution.securecodebox.io_scancompletionhooks.yaml @@ -181,9 +181,10 @@ spec: type: array priority: default: 0 - description: Defines the priority of the hook. Higher priority hooks - run before low priority hooks. Hooks with identical priority will - be launched in parallel. + description: Higher priority hooks run before low priority hooks. + Within a priority class ReadAndWrite hooks are started before ReadOnly + hooks, ReadAndWrite hooks wil be launched in serial, and ReadOnly + hooks will be launched in parallel. type: integer serviceAccountName: description: ServiceAccountName Name of the serviceAccount Name used. From 983385104df2b7bb5ee23bf19c45fff322f7a359 Mon Sep 17 00:00:00 2001 From: Jannik Hollenbach Date: Tue, 12 Oct 2021 23:00:48 +0200 Subject: [PATCH 09/17] Refactor hook prio handling to use a pre calculated list --- .../cascading/v1/zz_generated.deepcopy.go | 1 + operator/apis/execution/v1/scan_types.go | 2 +- .../execution/v1/zz_generated.deepcopy.go | 11 +- .../execution.securecodebox.io_scans.yaml | 45 ++-- .../execution/scans/hook_reconciler.go | 146 +++++------ operator/go.mod | 2 + operator/go.sum | 43 ++- operator/utils/hookpriorityqueue.go | 81 ------ operator/utils/hookpriorityqueue_test.go | 90 ------- operator/utils/orderedhookgroups.go | 104 ++++++++ operator/utils/orderedhookgroups_test.go | 245 ++++++++++++++++++ operator/utils/suite_test.go | 20 ++ 12 files changed, 498 insertions(+), 292 deletions(-) delete mode 100644 operator/utils/hookpriorityqueue.go delete mode 100644 operator/utils/hookpriorityqueue_test.go create mode 100644 operator/utils/orderedhookgroups.go create mode 100644 operator/utils/orderedhookgroups_test.go create mode 100644 operator/utils/suite_test.go diff --git a/operator/apis/cascading/v1/zz_generated.deepcopy.go b/operator/apis/cascading/v1/zz_generated.deepcopy.go index 6778e87db6..b14043e774 100644 --- a/operator/apis/cascading/v1/zz_generated.deepcopy.go +++ b/operator/apis/cascading/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // SPDX-FileCopyrightText: 2021 iteratec GmbH diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index 69a77d15f0..7e3a8ebf75 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -83,7 +83,7 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` - HookStatuses []*HookStatus `json:"hookStatuses,omitempty"` + OrderedHookStatuses [][]HookStatus `json:"orderedHookStatuses,omitempty"` } // HookState Describes the State of a Hook on a Scan diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index 9b2e48bb90..f97044457f 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // SPDX-FileCopyrightText: 2021 iteratec GmbH @@ -469,14 +470,14 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { *out = (*in).DeepCopy() } in.Findings.DeepCopyInto(&out.Findings) - if in.HookStatuses != nil { - in, out := &in.HookStatuses, &out.HookStatuses - *out = make([]*HookStatus, len(*in)) + if in.OrderedHookStatuses != nil { + in, out := &in.OrderedHookStatuses, &out.OrderedHookStatuses + *out = make([][]HookStatus, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] - *out = new(HookStatus) - **out = **in + *out = make([]HookStatus, len(*in)) + copy(*out, *in) } } } diff --git a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml index fd1b995b6d..5f8cb52b62 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml @@ -1756,28 +1756,31 @@ spec: parser & hooks) has been marked as "Done" format: date-time type: string - hookStatuses: + orderedHookStatuses: items: - properties: - hookName: - type: string - jobName: - type: string - priority: - type: integer - state: - description: HookState Describes the State of a Hook on a Scan - type: string - type: - description: HookType Defines weather the hook should be able - to change the findings or is run in a read only mode. - type: string - required: - - hookName - - priority - - state - - type - type: object + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a + Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array type: array rawResultDownloadLink: description: RawResultDownloadLink link to download the raw result diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 9a7153e05e..a0d0e4749b 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -5,11 +5,11 @@ package scancontrollers import ( - "container/heap" "context" "fmt" executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" + "github.com/secureCodeBox/secureCodeBox/operator/utils" util "github.com/secureCodeBox/secureCodeBox/operator/utils" batch "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -33,16 +33,8 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { r.Log.Info("Found ScanCompletionHooks", "ScanCompletionHooks", len(scanCompletionHooks.Items)) - for _, hook := range scanCompletionHooks.Items { - hookStatus := &executionv1.HookStatus{ - HookName: hook.Name, - State: executionv1.Pending, - Type: hook.Spec.Type, - Priority: hook.Spec.Priority, - } - scan.Status.HookStatuses = append(scan.Status.HookStatuses, hookStatus) - } - + orderedHookStatus := util.FromUnorderedList(scanCompletionHooks.Items) + scan.Status.OrderedHookStatuses = orderedHookStatus scan.Status.State = "HookProcessing" if err := r.Status().Update(ctx, scan); err != nil { @@ -56,50 +48,25 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { ctx := context.Background() - nonCompletedHooks := util.PriorityQueueFromSlice( - &scan.Status.HookStatuses, - func(hook *executionv1.HookStatus) bool { - return hook.State != executionv1.Completed - }, - ) + err, currentHooks := utils.CurrentHookGroup(scan.Status.OrderedHookStatuses) - var err error - // If nil then all hooks are done - if len(nonCompletedHooks) == 0 { - r.Log.Info("Marked scan as done as all hooks have completed", "ScanName", scan.Name) - scan.Status.State = "Done" - var now = metav1.Now() - scan.Status.FinishedAt = &now - } else if len(nonCompletedHooks) > 0 { - for { - hook := heap.Pop(&nonCompletedHooks).(*util.PriorityQueueItem).Value - - err = r.processHook(scan, hook) - if err != nil || hook.State == executionv1.Failed || hook.State == executionv1.Cancelled { + if err != nil && scan.Status.State == "Errored" { + r.Log.V(8).Info("Skipping hook execution as it already contains failed hooks.") + return nil + } else if err != nil { + scan.Status.State = "Errored" + scan.Status.ErrorDescription = fmt.Sprintf("Hook execution failed for a unknown hook. Check the scan.status.hookStatus field for more details") + } else if err == nil && currentHooks == nil { + // No hooks left to execute + scan.Status.State = "Completed" + } else { + for i, hook := range currentHooks { + err, status := r.processHook(scan, &hook) + currentHooks[i] = status + + if err != nil { scan.Status.State = "Errored" 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) - break - } - - // ReadAndWrite hooks may not be run in parallel - if nonCompletedHooks.Len() == 0 || hook.Type == executionv1.ReadAndWrite { - break - } - - next := nonCompletedHooks.Peek().(*util.PriorityQueueItem).Value - if next.Priority < hook.Priority { - break - } - } - } - - if err == nil && scan.Status.State == "HookProcessing" { - // Check if there are now still incomplete hooks - scan.Status.State = "Completed" - for _, hook := range scan.Status.HookStatuses { - if hook.State != executionv1.Completed { - scan.Status.State = "HookProcessing" - break } } } @@ -108,11 +75,10 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { r.Log.Error(sErr, "Unable to update Scan status") return sErr } - return err } -func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error { +func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) (error, executionv1.HookStatus) { var jobType string if nonCompletedHook.Type == executionv1.ReadOnly { jobType = "read-only-hook" @@ -120,32 +86,36 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e jobType = "read-and-write-hook" } - r.Log.Info(fmt.Sprintf("Processing %s", jobType), "Hook", nonCompletedHook) + r.Log.Info("Processing hook", "hook", nonCompletedHook, "jobType", jobType) - var err error switch nonCompletedHook.State { case executionv1.Pending: - err = r.processPendingHook(scan, nonCompletedHook, jobType) + err, status := r.processPendingHook(scan, nonCompletedHook, jobType) + if err == nil { + r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) + } + return err, status case executionv1.InProgress: - err = r.processInProgressHook(scan, nonCompletedHook, jobType) - } - - if err == nil { - r.Log.Info(fmt.Sprintf("Processed %s", jobType), "Hook", nonCompletedHook) + err, status := r.processInProgressHook(scan, nonCompletedHook, jobType) + if err == nil { + r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) + } + return err, status } - - return err + return nil, *nonCompletedHook.DeepCopy() } -func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) error { +func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) { ctx := context.Background() var hook executionv1.ScanCompletionHook + status := pendingHook.DeepCopy() + var err error err = r.Get(ctx, types.NamespacedName{Name: pendingHook.HookName, Namespace: scan.Namespace}, &hook) if err != nil { r.Log.Error(err, "Failed to get Hook for HookStatus") - return err + return err, *status } var jobs *batch.JobList @@ -153,19 +123,25 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook "securecodebox.io/job-type": jobType, "securecodebox.io/hook-name": pendingHook.HookName, }) - if err != nil || len(jobs.Items) > 0 { - return err + if err != nil { + return err, *status + } + if len(jobs.Items) > 0 { + // job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values + status.JobName = jobs.Items[0].Name + status.State = executionv1.InProgress + return nil, *status } var rawFileURL string rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err + return err, *status } var findingsFileURL string findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err + return err, *status } var args = []string{ @@ -176,12 +152,12 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook var rawFileUploadURL string rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err + return err, *status } var findingsUploadURL string findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err + return err, *status } args = append(args, rawFileUploadURL, findingsUploadURL) } @@ -194,37 +170,41 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook ) if err == nil { - // Update the currently executed hook job name and status to "InProgress" - pendingHook.JobName = jobName - pendingHook.State = executionv1.InProgress + // job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values + status := pendingHook.DeepCopy() + status.JobName = jobName + status.State = executionv1.InProgress + return nil, *status } - return err + return err, *status } -func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) error { +func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) { + status := inProgressHook.DeepCopy() + jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, "securecodebox.io/hook-name": inProgressHook.HookName, }) if err != nil { r.Log.Error(err, "Failed to check job status for Hook") - return err + return err, *status } switch jobStatus { case completed: // Job is completed => set current Hook to completed - inProgressHook.State = executionv1.Completed + status.State = executionv1.Completed case incomplete: // Still waiting for job to finish case failed: - if inProgressHook.State == executionv1.Pending { - inProgressHook.State = executionv1.Cancelled + if status.State == executionv1.Pending { + status.State = executionv1.Cancelled } else { - inProgressHook.State = executionv1.Failed + status.State = executionv1.Failed } } - return nil + return nil, *status } func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { diff --git a/operator/go.mod b/operator/go.mod index 7471c518cc..b122b0761b 100644 --- a/operator/go.mod +++ b/operator/go.mod @@ -9,6 +9,8 @@ go 1.15 require ( github.com/go-logr/logr v0.4.0 github.com/minio/minio-go/v7 v7.0.10 + github.com/onsi/ginkgo v1.16.5 + github.com/onsi/gomega v1.16.0 k8s.io/api v0.20.2 k8s.io/apimachinery v0.20.2 k8s.io/client-go v0.20.2 diff --git a/operator/go.sum b/operator/go.sum index 464218bdd3..f03b7396ed 100644 --- a/operator/go.sum +++ b/operator/go.sum @@ -130,6 +130,7 @@ github.com/go-openapi/spec v0.19.3/go.mod h1:FpwSN1ksY1eteniUU7X0N/BgJ7a4WvBFVA8 github.com/go-openapi/swag v0.19.2/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= +github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.1 h1:DqDEcV5aeaTmdFBePNpYsp3FlcVH/2ISVVM9Qf8PSls= @@ -158,8 +159,10 @@ github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:W github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0= github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= -github.com/golang/protobuf v1.4.3 h1:JjCZWpVbqXDqFVmTfYWEVTMIYrL/NPdPSCHPJ0T/raM= github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= +github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= +github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= +github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= @@ -167,8 +170,9 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -290,22 +294,26 @@ github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8m github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= -github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= +github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= +github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= -github.com/onsi/ginkgo v1.14.1 h1:jMU0WaQrP0a/YAEq8eJmJKjBoMs+pClEr1vDMlM/Do4= github.com/onsi/ginkgo v1.14.1/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= +github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= +github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= -github.com/onsi/gomega v1.10.2 h1:aY/nuoWlKJud2J6U0E3NWsjlg+0GtwXxgEqthRdzlcs= github.com/onsi/gomega v1.10.2/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= +github.com/onsi/gomega v1.16.0 h1:6gjqkI8iiRHMvdccRJM8rVKjCWk6ZIm6FTm3ddIe4/c= +github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= @@ -384,6 +392,7 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.5/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= @@ -478,8 +487,10 @@ golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= -golang.org/x/net v0.0.0-20201110031124-69a78807bb2b h1:uwuIcX0g4Yl1NC5XAz37xsr2lTtcqevgzYNVt49waME= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/net v0.0.0-20210428140749-89ef3d95e781 h1:DzZ89McO9/gWPsQXS/FVKAlG02ZjaQ6AlZRBimEYOd0= +golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -492,6 +503,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -528,15 +540,20 @@ golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200615200032-f1bc736245b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201112073958-5cba982894dd h1:5CtCZbICpIOFdgO940moixOPjc0178IU44m4EjOO5IY= golang.org/x/sys v0.0.0-20201112073958-5cba982894dd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da h1:b3NXsE2LusjYGGjL5bxEVZZORm/YEFFrWFjR8eFrw/c= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= +golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -583,8 +600,9 @@ golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200224181240-023911ca70b2/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200616133436-c1934b75d054 h1:HHeAlu5H9b71C+Fx0K+1dGgVFN1DM1/wz4aoGOA5qS8= golang.org/x/tools v0.0.0-20200616133436-c1934b75d054/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e h1:4nW4NLDYnU28ojHaHO8OVxFHk/aQ33U01a9cjED+pzE= +golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -645,8 +663,10 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2 google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= -google.golang.org/protobuf v1.25.0 h1:Ejskq+SyPohKW+1uil0JJMtmHCgJPJ/qWTxr8qp+R4c= google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= +google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= +google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk= +google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -671,8 +691,9 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 h1:tQIYjPdBoyREyB9XMu+nnTclpTYkz2zFM+lzLJFO4gQ= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/operator/utils/hookpriorityqueue.go b/operator/utils/hookpriorityqueue.go deleted file mode 100644 index 3552f9b076..0000000000 --- a/operator/utils/hookpriorityqueue.go +++ /dev/null @@ -1,81 +0,0 @@ -package utils - -import ( - "container/heap" - executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" -) - -// Taken (mostly) from official example https://pkg.go.dev/container/heap - -// An PriorityQueueItem is something we manage in a priority queue. -type PriorityQueueItem struct { - Value *executionv1.HookStatus - Priority int // The Priority of the item in the queue. - Index int // The Index of the item in the heap. -} - -// A PriorityQueue implements heap.Interface and holds Items. -type PriorityQueue []*PriorityQueueItem - -func (pq PriorityQueue) Len() int { return len(pq) } - -func (pq PriorityQueue) Less(i, j int) bool { - // We want Pop to give us the highest, not lowest, priority so we invert result - - if pq[i].Priority == pq[j].Priority { - if pq[i].Value.Type == pq[j].Value.Type { - // Prefer hook if it is in progress - return pq[i].Value.State == executionv1.InProgress - } else { - // Prefer ReadAndWriteHooks - return pq[i].Value.Type == executionv1.ReadAndWrite - } - } else { - return pq[i].Priority > pq[j].Priority - } -} - -func (pq PriorityQueue) Swap(i, j int) { - pq[i], pq[j] = pq[j], pq[i] - pq[i].Index = i - pq[j].Index = j -} - -func (pq *PriorityQueue) Push(x interface{}) { - n := len(*pq) - item := x.(*PriorityQueueItem) - item.Index = n - *pq = append(*pq, item) -} - -func (pq *PriorityQueue) Pop() interface{} { - old := *pq - n := len(old) - item := old[n-1] - old[n-1] = nil // avoid memory leak - item.Index = -1 // for safety - *pq = old[0 : n-1] - return item -} - -func (pq *PriorityQueue) Peek() interface{} { - return (*pq)[0] -} - -func PriorityQueueFromSlice(hooks *[]*executionv1.HookStatus, conditional func(hook *executionv1.HookStatus) bool) PriorityQueue { - var priorityQueue = make(PriorityQueue, len(*hooks)) - var completedHooks = 0 - for _, hook := range *hooks { - if conditional(hook) { - priorityQueueItem := PriorityQueueItem{ - Value: hook, - Priority: hook.Priority, - } - priorityQueue[completedHooks] = &priorityQueueItem - completedHooks++ - } - } - priorityQueue = priorityQueue[:completedHooks] - heap.Init(&priorityQueue) - return priorityQueue -} diff --git a/operator/utils/hookpriorityqueue_test.go b/operator/utils/hookpriorityqueue_test.go deleted file mode 100644 index fd5ee364b8..0000000000 --- a/operator/utils/hookpriorityqueue_test.go +++ /dev/null @@ -1,90 +0,0 @@ -package utils - -import ( - "container/heap" - "fmt" - executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" - "testing" -) - -func TestQueuePriority(t *testing.T) { - var items = []*executionv1.HookStatus{ - { - HookName: "banana", - Priority: 3, - State: executionv1.Pending, - }, - { - HookName: "apple", - Priority: 2, - State: executionv1.InProgress, - }, - { - HookName: "pear", - Priority: 4, - State: executionv1.Failed, - }, - { - HookName: "kiwi", - Priority: 4, - State: executionv1.Completed, - }, - } - - pq := PriorityQueueFromSlice(&items, hookIsNotCompleted) - - // Take the items out; they should arrive in decreasing priority order. - assertTopEqual(t, &pq, items[2]) - assertTopEqual(t, &pq, items[0]) - assertTopEqual(t, &pq, items[1]) -} - -func TestQueuePool(t *testing.T) { - var hookStatuses = []*executionv1.HookStatus{ - { - HookName: "banana", - State: executionv1.InProgress, - Type: executionv1.ReadOnly, - Priority: 1, - }, - { - HookName: "apple", - State: executionv1.InProgress, - Type: executionv1.ReadAndWrite, - Priority: 1, - }, - { - HookName: "pear", - State: executionv1.Pending, - Type: executionv1.ReadAndWrite, - Priority: 1, - }, - } - - pq := PriorityQueueFromSlice(&hookStatuses, hookIsNotCompleted) - - // Take the items out; they should arrive in decreasing priority order. - assertTopEqual(t, &pq, hookStatuses[1]) - assertTopEqual(t, &pq, hookStatuses[2]) - assertTopEqual(t, &pq, hookStatuses[0]) -} - -func hookIsNotCompleted(hook *executionv1.HookStatus) bool { - return hook.State != executionv1.Completed -} - -func assertTopEqual(t *testing.T, queue *PriorityQueue, expected *executionv1.HookStatus) { - assertEqual(t, queue.Peek().(*PriorityQueueItem).Value, expected) - assertEqual(t, heap.Pop(queue).(*PriorityQueueItem).Value, expected) -} - -func assertEqual(t *testing.T, received *executionv1.HookStatus, expected *executionv1.HookStatus) { - if received != expected { - t.Fatal( - fmt.Errorf("received '%s' '%s' priority '%d' hook '%s' expecting '%s' '%s' priority '%d' hook '%s'", - received.State, received.Type, received.Priority, received.HookName, - expected.State, expected.Type, received.Priority, expected.HookName, - ), - ) - } -} diff --git a/operator/utils/orderedhookgroups.go b/operator/utils/orderedhookgroups.go new file mode 100644 index 0000000000..b0f0ba6e21 --- /dev/null +++ b/operator/utils/orderedhookgroups.go @@ -0,0 +1,104 @@ +// SPDX-FileCopyrightText: 2021 iteratec GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "fmt" + "sort" + + executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" +) + +func CurrentHookGroup(orderedHookGroup [][]executionv1.HookStatus) (error, []executionv1.HookStatus) { + for _, group := range orderedHookGroup { + for _, hookStatus := range group { + switch hookStatus.State { + case executionv1.Pending: + return nil, group + case executionv1.InProgress: + return nil, group + case executionv1.Failed: + return fmt.Errorf("Hook %s failed to be executed.", hookStatus.HookName), nil + case executionv1.Cancelled: + return fmt.Errorf("Hook %s was cancelled while it was executed.", hookStatus.HookName), nil + case executionv1.Completed: + // continue to next group + } + } + } + + return nil, nil +} + +func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.HookStatus { + // convert ScanCompletionHook objects to HookStatus objects + hookStatuses := mapHookToHookStatus(hooks) + + // Group hookStatuses into a map by their prio class + hooksByPrioClass := map[int][]executionv1.HookStatus{} + // keep a list of existing classes + prioClasses := []int{} + for _, hookStatus := range hookStatuses { + prio := hookStatus.Priority + + if _, ok := hooksByPrioClass[prio]; ok { + hooksByPrioClass[prio] = append(hooksByPrioClass[prio], hookStatus) + } else { + hooksByPrioClass[prio] = []executionv1.HookStatus{hookStatus} + prioClasses = append(prioClasses, prio) + } + } + fmt.Printf("Prio Classes: %d \n", len(prioClasses)) + + // sort prio classes in decending order + sort.Slice(prioClasses, func(i, j int) bool { + return prioClasses[i] > prioClasses[j] + }) + + groups := [][]executionv1.HookStatus{} + for _, prioClass := range prioClasses { + fmt.Printf("Ordering hooks for prio class %d into %d ordering groups \n", prioClass, len(orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass]))) + groups = append(groups, orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass])...) + } + + return groups +} + +func mapHookToHookStatus(hooks []executionv1.ScanCompletionHook) []executionv1.HookStatus { + hookStatuses := []executionv1.HookStatus{} + + for _, hook := range hooks { + hookStatuses = append(hookStatuses, executionv1.HookStatus{ + HookName: hook.Name, + State: executionv1.Pending, + Priority: hook.Spec.Priority, + Type: hook.Spec.Type, + }) + } + + return hookStatuses +} + +func orderHookStatusesInsideAPrioClass(hookStatuses []executionv1.HookStatus) [][]executionv1.HookStatus { + groups := [][]executionv1.HookStatus{} + readOnlyGroups := []executionv1.HookStatus{} + for _, hookStatus := range hookStatuses { + switch hookStatus.Type { + case executionv1.ReadAndWrite: + groups = append(groups, []executionv1.HookStatus{ + hookStatus, + }) + case executionv1.ReadOnly: + readOnlyGroups = append(readOnlyGroups, hookStatus) + } + } + + // Append the ReadOnly Hook Group at the end if existent + if len(readOnlyGroups) != 0 { + groups = append(groups, readOnlyGroups) + } + + return groups +} diff --git a/operator/utils/orderedhookgroups_test.go b/operator/utils/orderedhookgroups_test.go new file mode 100644 index 0000000000..cae3a78417 --- /dev/null +++ b/operator/utils/orderedhookgroups_test.go @@ -0,0 +1,245 @@ +// SPDX-FileCopyrightText: 2021 iteratec GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" + corev1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func createHook(name string, hookType executionv1.HookType, prio int) executionv1.ScanCompletionHook { + return executionv1.ScanCompletionHook{ + ObjectMeta: corev1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: executionv1.ScanCompletionHookSpec{ + Type: hookType, + Priority: prio, + }, + } +} + +var _ = Describe("HookOrderingGroup Creation", func() { + Context("HookOrderingGroup Creation / Sorting (Single Prio)", func() { + It("Should always place ReadAndWrite Hooks into different Groups", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("rw-1", executionv1.ReadAndWrite, 0), + createHook("rw-2", executionv1.ReadAndWrite, 0), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(orderedHookGroups).To(HaveLen(2), "Should create two groups") + + Expect(orderedHookGroups[0]).To(HaveLen(1), "groups should contain one entry") + Expect(orderedHookGroups[1]).To(HaveLen(1), "groups should contain one entry") + }) + + It("Should place place ReadOnly Hooks into the same groups", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("ro-1", executionv1.ReadOnly, 0), + createHook("ro-2", executionv1.ReadOnly, 0), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(orderedHookGroups).To(HaveLen(1)) + Expect(orderedHookGroups[0]).To(HaveLen(2)) + }) + + It("Should handle mixed hook types", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("rw-1", executionv1.ReadAndWrite, 0), + createHook("ro-1", executionv1.ReadOnly, 0), + createHook("rw-2", executionv1.ReadAndWrite, 0), + createHook("ro-2", executionv1.ReadOnly, 0), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(len(orderedHookGroups)).To(Equal(3)) + + Expect(len(orderedHookGroups[0])).To(Equal(1)) + Expect(len(orderedHookGroups[1])).To(Equal(1)) + Expect(len(orderedHookGroups[2])).To(Equal(2)) + }) + }) + + Context("HookOrderingGroup Creation / Sorting (Different Priorities)", func() { + It("Should always place ReadAndWrite Hooks into different Groups", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("rw-1", executionv1.ReadAndWrite, 0), + createHook("rw-2", executionv1.ReadAndWrite, 1), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(orderedHookGroups).To(HaveLen(2), "Should create two groups") + + Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + { + {HookName: "rw-2", State: "Pending", JobName: "", Priority: 1, Type: "ReadAndWrite"}, + }, + { + {HookName: "rw-1", State: "Pending", JobName: "", Priority: 0, Type: "ReadAndWrite"}, + }, + })) + }) + + It("Should order ro hooks properly when they have different priorities", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("ro-1", executionv1.ReadOnly, 4), + createHook("ro-2", executionv1.ReadOnly, 2), + createHook("ro-3", executionv1.ReadOnly, 3), + createHook("ro-4", executionv1.ReadOnly, 1), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + { + {HookName: "ro-3", State: "Pending", JobName: "", Priority: 3, Type: "ReadOnly"}, + }, + { + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 2, Type: "ReadOnly"}, + }, + { + {HookName: "ro-4", State: "Pending", JobName: "", Priority: 1, Type: "ReadOnly"}, + }, + })) + }) + + It("Should order a mix of ro & rw hooks properly when they have different priorities", func() { + hooks := []executionv1.ScanCompletionHook{ + createHook("ro-1", executionv1.ReadOnly, 4), + createHook("ro-2", executionv1.ReadOnly, 4), + createHook("rw-1", executionv1.ReadAndWrite, 4), + createHook("rw-2", executionv1.ReadAndWrite, 4), + createHook("ro-3", executionv1.ReadOnly, 2), + createHook("rw-3", executionv1.ReadAndWrite, 2), + createHook("ro-4", executionv1.ReadOnly, 3), + createHook("ro-5", executionv1.ReadOnly, 1), + } + + orderedHookGroups := FromUnorderedList(hooks) + + Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + { + {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "rw-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + { + {HookName: "ro-4", State: "Pending", JobName: "", Priority: 3, Type: "ReadOnly"}, + }, + { + {HookName: "rw-3", State: "Pending", JobName: "", Priority: 2, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-3", State: "Pending", JobName: "", Priority: 2, Type: "ReadOnly"}, + }, + { + {HookName: "ro-5", State: "Pending", JobName: "", Priority: 1, Type: "ReadOnly"}, + }, + })) + }) + }) +}) + +var _ = Describe("HookOrderingGroup Retrival", func() { + Context("Current() should return the group of hooks which should be executed at the moment", func() { + It("Should return the first if all hooks are pending", func() { + err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + { + {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + }) + + Expect(err).To(BeNil()) + Expect(currentHookGroup).To(Equal( + []executionv1.HookStatus{ + {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + )) + }) + + It("Should return the first group if it consists of hooks currently in progress", func() { + err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + { + {HookName: "rw-1", State: "InProgress", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + }) + + Expect(err).To(BeNil()) + Expect(currentHookGroup).To(Equal( + []executionv1.HookStatus{ + {HookName: "rw-1", State: "InProgress", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + )) + }) + + It("Should return the second group if the first group is completed", func() { + err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + { + {HookName: "rw-1", State: "Completed", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + }) + + Expect(err).To(BeNil()) + Expect(currentHookGroup).To(Equal( + []executionv1.HookStatus{ + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + )) + }) + + It("Should return nil if the first group failed", func() { + err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + { + {HookName: "rw-1", State: "Failed", JobName: "", Priority: 4, Type: "ReadAndWrite"}, + }, + { + {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, + }, + }) + + Expect(err).To(MatchError("Hook rw-1 failed to be executed.")) + Expect(currentHookGroup).To(BeNil()) + }) + + It("Should return nil if no hooks are configured", func() { + err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{}) + + Expect(err).To(BeNil()) + Expect(currentHookGroup).To(BeNil()) + }) + }) +}) diff --git a/operator/utils/suite_test.go b/operator/utils/suite_test.go new file mode 100644 index 0000000000..74d032e36f --- /dev/null +++ b/operator/utils/suite_test.go @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: 2021 iteratec GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestGinko(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, + "Utils Suite", + ) +} From 1ff2dfb1d412af7ccdd4be51d8c87fc0a11f8bb6 Mon Sep 17 00:00:00 2001 From: Jannik Hollenbach Date: Tue, 12 Oct 2021 23:04:24 +0200 Subject: [PATCH 10/17] Remove prints --- operator/utils/orderedhookgroups.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/operator/utils/orderedhookgroups.go b/operator/utils/orderedhookgroups.go index b0f0ba6e21..0a488f2252 100644 --- a/operator/utils/orderedhookgroups.go +++ b/operator/utils/orderedhookgroups.go @@ -50,7 +50,6 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.H prioClasses = append(prioClasses, prio) } } - fmt.Printf("Prio Classes: %d \n", len(prioClasses)) // sort prio classes in decending order sort.Slice(prioClasses, func(i, j int) bool { @@ -59,7 +58,6 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.H groups := [][]executionv1.HookStatus{} for _, prioClass := range prioClasses { - fmt.Printf("Ordering hooks for prio class %d into %d ordering groups \n", prioClass, len(orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass]))) groups = append(groups, orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass])...) } From 168e6d333d4a442a7582262a94bb4d10188c047a Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Wed, 13 Oct 2021 10:30:44 +0200 Subject: [PATCH 11/17] Update HookStatuses by reference instead. It already has side-effects (i.e. cluster updates) so it's more consistent to update in-place. Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scan_types.go | 2 +- .../execution/v1/zz_generated.deepcopy.go | 12 +++-- .../execution/scans/hook_reconciler.go | 53 +++++++++---------- operator/utils/orderedhookgroups.go | 24 ++++----- operator/utils/orderedhookgroups_test.go | 22 ++++---- 5 files changed, 57 insertions(+), 56 deletions(-) diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index 7e3a8ebf75..a7fa6d0f1a 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -83,7 +83,7 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` - OrderedHookStatuses [][]HookStatus `json:"orderedHookStatuses,omitempty"` + OrderedHookStatuses [][]*HookStatus `json:"orderedHookStatuses,omitempty"` } // HookState Describes the State of a Hook on a Scan diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index f97044457f..e0146b6442 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -472,12 +472,18 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { in.Findings.DeepCopyInto(&out.Findings) if in.OrderedHookStatuses != nil { in, out := &in.OrderedHookStatuses, &out.OrderedHookStatuses - *out = make([][]HookStatus, len(*in)) + *out = make([][]*HookStatus, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] - *out = make([]HookStatus, len(*in)) - copy(*out, *in) + *out = make([]*HookStatus, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(HookStatus) + **out = **in + } + } } } } diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index a0d0e4749b..2b8da76d4d 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -60,9 +60,8 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { // No hooks left to execute scan.Status.State = "Completed" } else { - for i, hook := range currentHooks { - err, status := r.processHook(scan, &hook) - currentHooks[i] = status + for _, hook := range currentHooks { + err = r.processHook(scan, hook) if err != nil { scan.Status.State = "Errored" @@ -78,7 +77,7 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { return err } -func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) (error, executionv1.HookStatus) { +func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *executionv1.HookStatus) error { var jobType string if nonCompletedHook.Type == executionv1.ReadOnly { jobType = "read-only-hook" @@ -90,58 +89,57 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e switch nonCompletedHook.State { case executionv1.Pending: - err, status := r.processPendingHook(scan, nonCompletedHook, jobType) + err := r.processPendingHook(scan, nonCompletedHook, jobType) if err == nil { r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) } - return err, status + return err case executionv1.InProgress: - err, status := r.processInProgressHook(scan, nonCompletedHook, jobType) + err := r.processInProgressHook(scan, nonCompletedHook, jobType) if err == nil { r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) } - return err, status + return err } - return nil, *nonCompletedHook.DeepCopy() + return nil } -func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) { +func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, status *executionv1.HookStatus, jobType string) error { ctx := context.Background() var hook executionv1.ScanCompletionHook - status := pendingHook.DeepCopy() var err error - err = r.Get(ctx, types.NamespacedName{Name: pendingHook.HookName, Namespace: scan.Namespace}, &hook) + err = r.Get(ctx, types.NamespacedName{Name: status.HookName, Namespace: scan.Namespace}, &hook) if err != nil { r.Log.Error(err, "Failed to get Hook for HookStatus") - return err, *status + return err } var jobs *batch.JobList jobs, err = r.getJobsForScan(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, - "securecodebox.io/hook-name": pendingHook.HookName, + "securecodebox.io/hook-name": status.HookName, }) if err != nil { - return err, *status + return err } if len(jobs.Items) > 0 { // job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values status.JobName = jobs.Items[0].Name status.State = executionv1.InProgress - return nil, *status + return nil } var rawFileURL string rawFileURL, err = r.PresignedGetURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err, *status + return err } var findingsFileURL string findingsFileURL, err = r.PresignedGetURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err, *status + return err } var args = []string{ @@ -152,12 +150,12 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook var rawFileUploadURL string rawFileUploadURL, err = r.PresignedPutURL(scan.UID, scan.Status.RawResultFile, defaultPresignDuration) if err != nil { - return err, *status + return err } var findingsUploadURL string findingsUploadURL, err = r.PresignedPutURL(scan.UID, "findings.json", defaultPresignDuration) if err != nil { - return err, *status + return err } args = append(args, rawFileUploadURL, findingsUploadURL) } @@ -171,25 +169,22 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, pendingHook if err == nil { // job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values - status := pendingHook.DeepCopy() status.JobName = jobName status.State = executionv1.InProgress - return nil, *status + return nil } - return err, *status + return err } -func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgressHook *executionv1.HookStatus, jobType string) (error, executionv1.HookStatus) { - status := inProgressHook.DeepCopy() - +func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, status *executionv1.HookStatus, jobType string) error { jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{ "securecodebox.io/job-type": jobType, - "securecodebox.io/hook-name": inProgressHook.HookName, + "securecodebox.io/hook-name": status.HookName, }) if err != nil { r.Log.Error(err, "Failed to check job status for Hook") - return err, *status + return err } switch jobStatus { case completed: @@ -204,7 +199,7 @@ func (r *ScanReconciler) processInProgressHook(scan *executionv1.Scan, inProgres status.State = executionv1.Failed } } - return nil, *status + return nil } func (r *ScanReconciler) createJobForHook(hook *executionv1.ScanCompletionHook, scan *executionv1.Scan, cliArgs []string) (string, error) { diff --git a/operator/utils/orderedhookgroups.go b/operator/utils/orderedhookgroups.go index 0a488f2252..b734b9aedc 100644 --- a/operator/utils/orderedhookgroups.go +++ b/operator/utils/orderedhookgroups.go @@ -11,7 +11,7 @@ import ( executionv1 "github.com/secureCodeBox/secureCodeBox/operator/apis/execution/v1" ) -func CurrentHookGroup(orderedHookGroup [][]executionv1.HookStatus) (error, []executionv1.HookStatus) { +func CurrentHookGroup(orderedHookGroup [][]*executionv1.HookStatus) (error, []*executionv1.HookStatus) { for _, group := range orderedHookGroup { for _, hookStatus := range group { switch hookStatus.State { @@ -32,12 +32,12 @@ func CurrentHookGroup(orderedHookGroup [][]executionv1.HookStatus) (error, []exe return nil, nil } -func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.HookStatus { +func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]*executionv1.HookStatus { // convert ScanCompletionHook objects to HookStatus objects hookStatuses := mapHookToHookStatus(hooks) // Group hookStatuses into a map by their prio class - hooksByPrioClass := map[int][]executionv1.HookStatus{} + hooksByPrioClass := map[int][]*executionv1.HookStatus{} // keep a list of existing classes prioClasses := []int{} for _, hookStatus := range hookStatuses { @@ -46,7 +46,7 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.H if _, ok := hooksByPrioClass[prio]; ok { hooksByPrioClass[prio] = append(hooksByPrioClass[prio], hookStatus) } else { - hooksByPrioClass[prio] = []executionv1.HookStatus{hookStatus} + hooksByPrioClass[prio] = []*executionv1.HookStatus{hookStatus} prioClasses = append(prioClasses, prio) } } @@ -56,7 +56,7 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.H return prioClasses[i] > prioClasses[j] }) - groups := [][]executionv1.HookStatus{} + groups := [][]*executionv1.HookStatus{} for _, prioClass := range prioClasses { groups = append(groups, orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass])...) } @@ -64,11 +64,11 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]executionv1.H return groups } -func mapHookToHookStatus(hooks []executionv1.ScanCompletionHook) []executionv1.HookStatus { - hookStatuses := []executionv1.HookStatus{} +func mapHookToHookStatus(hooks []executionv1.ScanCompletionHook) []*executionv1.HookStatus { + hookStatuses := []*executionv1.HookStatus{} for _, hook := range hooks { - hookStatuses = append(hookStatuses, executionv1.HookStatus{ + hookStatuses = append(hookStatuses, &executionv1.HookStatus{ HookName: hook.Name, State: executionv1.Pending, Priority: hook.Spec.Priority, @@ -79,13 +79,13 @@ func mapHookToHookStatus(hooks []executionv1.ScanCompletionHook) []executionv1.H return hookStatuses } -func orderHookStatusesInsideAPrioClass(hookStatuses []executionv1.HookStatus) [][]executionv1.HookStatus { - groups := [][]executionv1.HookStatus{} - readOnlyGroups := []executionv1.HookStatus{} +func orderHookStatusesInsideAPrioClass(hookStatuses []*executionv1.HookStatus) [][]*executionv1.HookStatus { + groups := [][]*executionv1.HookStatus{} + readOnlyGroups := []*executionv1.HookStatus{} for _, hookStatus := range hookStatuses { switch hookStatus.Type { case executionv1.ReadAndWrite: - groups = append(groups, []executionv1.HookStatus{ + groups = append(groups, []*executionv1.HookStatus{ hookStatus, }) case executionv1.ReadOnly: diff --git a/operator/utils/orderedhookgroups_test.go b/operator/utils/orderedhookgroups_test.go index cae3a78417..acc5bd42ab 100644 --- a/operator/utils/orderedhookgroups_test.go +++ b/operator/utils/orderedhookgroups_test.go @@ -82,7 +82,7 @@ var _ = Describe("HookOrderingGroup Creation", func() { Expect(orderedHookGroups).To(HaveLen(2), "Should create two groups") - Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + Expect(orderedHookGroups).To(Equal([][]*executionv1.HookStatus{ { {HookName: "rw-2", State: "Pending", JobName: "", Priority: 1, Type: "ReadAndWrite"}, }, @@ -102,7 +102,7 @@ var _ = Describe("HookOrderingGroup Creation", func() { orderedHookGroups := FromUnorderedList(hooks) - Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + Expect(orderedHookGroups).To(Equal([][]*executionv1.HookStatus{ { {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, }, @@ -132,7 +132,7 @@ var _ = Describe("HookOrderingGroup Creation", func() { orderedHookGroups := FromUnorderedList(hooks) - Expect(orderedHookGroups).To(Equal([][]executionv1.HookStatus{ + Expect(orderedHookGroups).To(Equal([][]*executionv1.HookStatus{ { {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, @@ -163,7 +163,7 @@ var _ = Describe("HookOrderingGroup Creation", func() { var _ = Describe("HookOrderingGroup Retrival", func() { Context("Current() should return the group of hooks which should be executed at the moment", func() { It("Should return the first if all hooks are pending", func() { - err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + err, currentHookGroup := CurrentHookGroup([][]*executionv1.HookStatus{ { {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, @@ -175,14 +175,14 @@ var _ = Describe("HookOrderingGroup Retrival", func() { Expect(err).To(BeNil()) Expect(currentHookGroup).To(Equal( - []executionv1.HookStatus{ + []*executionv1.HookStatus{ {HookName: "rw-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, )) }) It("Should return the first group if it consists of hooks currently in progress", func() { - err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + err, currentHookGroup := CurrentHookGroup([][]*executionv1.HookStatus{ { {HookName: "rw-1", State: "InProgress", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, @@ -194,14 +194,14 @@ var _ = Describe("HookOrderingGroup Retrival", func() { Expect(err).To(BeNil()) Expect(currentHookGroup).To(Equal( - []executionv1.HookStatus{ + []*executionv1.HookStatus{ {HookName: "rw-1", State: "InProgress", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, )) }) It("Should return the second group if the first group is completed", func() { - err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + err, currentHookGroup := CurrentHookGroup([][]*executionv1.HookStatus{ { {HookName: "rw-1", State: "Completed", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, @@ -213,7 +213,7 @@ var _ = Describe("HookOrderingGroup Retrival", func() { Expect(err).To(BeNil()) Expect(currentHookGroup).To(Equal( - []executionv1.HookStatus{ + []*executionv1.HookStatus{ {HookName: "ro-1", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, {HookName: "ro-2", State: "Pending", JobName: "", Priority: 4, Type: "ReadOnly"}, }, @@ -221,7 +221,7 @@ var _ = Describe("HookOrderingGroup Retrival", func() { }) It("Should return nil if the first group failed", func() { - err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{ + err, currentHookGroup := CurrentHookGroup([][]*executionv1.HookStatus{ { {HookName: "rw-1", State: "Failed", JobName: "", Priority: 4, Type: "ReadAndWrite"}, }, @@ -236,7 +236,7 @@ var _ = Describe("HookOrderingGroup Retrival", func() { }) It("Should return nil if no hooks are configured", func() { - err, currentHookGroup := CurrentHookGroup([][]executionv1.HookStatus{}) + err, currentHookGroup := CurrentHookGroup([][]*executionv1.HookStatus{}) Expect(err).To(BeNil()) Expect(currentHookGroup).To(BeNil()) From d8ccfdb20601bb0a264156a3ada4ca8be6dc7ad8 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Wed, 13 Oct 2021 10:32:30 +0200 Subject: [PATCH 12/17] Update logging Signed-off-by: Jop Zitman --- .../controllers/execution/scans/hook_reconciler.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 2b8da76d4d..820fb9ad2e 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -89,17 +89,9 @@ func (r *ScanReconciler) processHook(scan *executionv1.Scan, nonCompletedHook *e switch nonCompletedHook.State { case executionv1.Pending: - err := r.processPendingHook(scan, nonCompletedHook, jobType) - if err == nil { - r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) - } - return err + return r.processPendingHook(scan, nonCompletedHook, jobType) case executionv1.InProgress: - err := r.processInProgressHook(scan, nonCompletedHook, jobType) - if err == nil { - r.Log.Info("Created job for hook", "hook", nonCompletedHook, "jobType", jobType) - } - return err + return r.processInProgressHook(scan, nonCompletedHook, jobType) } return nil } @@ -171,6 +163,7 @@ func (r *ScanReconciler) processPendingHook(scan *executionv1.Scan, status *exec // job was already started, setting status to correct jobName and state to ensure it's not overwritten with wrong values status.JobName = jobName status.State = executionv1.InProgress + r.Log.Info("Created job for hook", "hook", status) return nil } From f8f606f156984409397b986b587b820ce49bebd7 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Wed, 13 Oct 2021 19:42:40 +0200 Subject: [PATCH 13/17] Revert final state name to `Done` Signed-off-by: Jop Zitman --- operator/controllers/execution/scans/hook_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 820fb9ad2e..65e7b97c03 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -58,7 +58,7 @@ func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { scan.Status.ErrorDescription = fmt.Sprintf("Hook execution failed for a unknown hook. Check the scan.status.hookStatus field for more details") } else if err == nil && currentHooks == nil { // No hooks left to execute - scan.Status.State = "Completed" + scan.Status.State = "Done" } else { for _, hook := range currentHooks { err = r.processHook(scan, hook) From 6a8bd46cc44e04dbba1964b1d7475818696684f7 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Thu, 14 Oct 2021 16:30:52 +0200 Subject: [PATCH 14/17] Update Helm CRDs Signed-off-by: Jop Zitman --- .../execution.securecodebox.io_scans.yaml | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/operator/crds/execution.securecodebox.io_scans.yaml b/operator/crds/execution.securecodebox.io_scans.yaml index b6bce1e648..f8d331071b 100644 --- a/operator/crds/execution.securecodebox.io_scans.yaml +++ b/operator/crds/execution.securecodebox.io_scans.yaml @@ -1771,28 +1771,31 @@ spec: parser & hooks) has been marked as "Done" format: date-time type: string - hookStatuses: + orderedHookStatuses: items: - properties: - hookName: - type: string - jobName: - type: string - priority: - type: integer - state: - description: HookState Describes the State of a Hook on a Scan - type: string - type: - description: HookType Defines weather the hook should be able - to change the findings or is run in a read only mode. - type: string - required: - - hookName - - priority - - state - - type - type: object + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a + Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array type: array rawResultDownloadLink: description: RawResultDownloadLink link to download the raw result From 05c3464df708cd0f885145d3f82ec93acbaaaac7 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Fri, 22 Oct 2021 20:52:59 +0200 Subject: [PATCH 15/17] Implement migration mechanism for scans that were processing hooks while upgrading Signed-off-by: Jop Zitman --- operator/apis/execution/v1/scan_types.go | 2 + .../execution/v1/zz_generated.deepcopy.go | 5 ++ .../execution.securecodebox.io_scans.yaml | 23 ++++++++ .../execution/scans/hook_reconciler.go | 59 +++++++++++++++++++ .../execution/scans/scan_controller.go | 6 ++ .../execution.securecodebox.io_scans.yaml | 23 ++++++++ operator/utils/orderedhookgroups.go | 4 +- 7 files changed, 120 insertions(+), 2 deletions(-) diff --git a/operator/apis/execution/v1/scan_types.go b/operator/apis/execution/v1/scan_types.go index d85d483e3d..b15c2efe80 100644 --- a/operator/apis/execution/v1/scan_types.go +++ b/operator/apis/execution/v1/scan_types.go @@ -99,6 +99,8 @@ type ScanStatus struct { Findings FindingStats `json:"findings,omitempty"` + ReadAndWriteHookStatus []HookStatus `json:"readAndWriteHookStatus,omitempty"` + OrderedHookStatuses [][]*HookStatus `json:"orderedHookStatuses,omitempty"` } diff --git a/operator/apis/execution/v1/zz_generated.deepcopy.go b/operator/apis/execution/v1/zz_generated.deepcopy.go index adddf0b403..7b78c7bc4b 100644 --- a/operator/apis/execution/v1/zz_generated.deepcopy.go +++ b/operator/apis/execution/v1/zz_generated.deepcopy.go @@ -477,6 +477,11 @@ func (in *ScanStatus) DeepCopyInto(out *ScanStatus) { *out = (*in).DeepCopy() } in.Findings.DeepCopyInto(&out.Findings) + if in.ReadAndWriteHookStatus != nil { + in, out := &in.ReadAndWriteHookStatus, &out.ReadAndWriteHookStatus + *out = make([]HookStatus, len(*in)) + copy(*out, *in) + } if in.OrderedHookStatuses != nil { in, out := &in.OrderedHookStatuses, &out.OrderedHookStatuses *out = make([][]*HookStatus, len(*in)) diff --git a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml index 608c5a3c95..97192db466 100644 --- a/operator/config/crd/bases/execution.securecodebox.io_scans.yaml +++ b/operator/config/crd/bases/execution.securecodebox.io_scans.yaml @@ -2898,6 +2898,29 @@ spec: description: RawResultType determines which kind of ParseDefinition will be used to turn the raw results of the scanner into findings type: string + readAndWriteHookStatus: + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array state: type: string type: object diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index 65e7b97c03..e99c9322f5 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -45,6 +45,65 @@ func (r *ScanReconciler) setHookStatus(scan *executionv1.Scan) error { return nil } +func (r *ScanReconciler) migrateHookStatus(scan *executionv1.Scan) error { + ctx := context.Background() + var scanCompletionHooks executionv1.ScanCompletionHookList + r.Log.Info("Starting hook Status field migrations", "ReadAndWriteHookStatus", scan.Status.ReadAndWriteHookStatus) + + if err := r.List(ctx, &scanCompletionHooks, client.InNamespace(scan.Namespace)); err != nil { + r.Log.V(7).Info("Unable to fetch ScanCompletionHooks") + return err + } + + // Add new fields to old ReadAndWriteHookStatus object and convert to pointers + strSlice := make([]*executionv1.HookStatus, len(scan.Status.ReadAndWriteHookStatus)) + for i := range scan.Status.ReadAndWriteHookStatus { + strSlice[i] = scan.Status.ReadAndWriteHookStatus[i].DeepCopy() // Keep original ReadAndWriteHookStatus field + strSlice[i].Priority = 0 + strSlice[i].Type = executionv1.ReadAndWrite + r.Log.Info("Converted ReadAndWrite hook Status", "Original", scan.Status.ReadAndWriteHookStatus[i], "New", strSlice[i]) + } + + // Construct new ReadOnly HookStatus for OrderedHookStatuses + var readOnlyHooks []*executionv1.HookStatus + for _, hook := range scanCompletionHooks.Items { + if hook.Spec.Type == executionv1.ReadOnly { + hookStatus := &executionv1.HookStatus{ + HookName: hook.Name, + Priority: 0, + Type: executionv1.ReadOnly, + } + + if scan.Status.State == "ReadAndWriteHookProcessing" || scan.Status.State == "ReadAndWriteHookCompleted" { + // ReadOnly hooks should not have started yet, so mark them all as pending + hookStatus.State = executionv1.Pending + } else if scan.Status.State == "ReadOnlyHookProcessing" { + // Had already started ReadOnly hooks and should now check status. + // No status for ReadOnly in old CRD, so mark everything as InProgress and let processInProgressHook update it later. + hookStatus.State = executionv1.InProgress + } + + r.Log.Info("Retrieved new ReadOnly hook Status", "New", hookStatus) + + readOnlyHooks = append(readOnlyHooks, hookStatus) + } + } + + scan.Status.OrderedHookStatuses = util.OrderHookStatusesInsideAPrioClass(append(readOnlyHooks, strSlice...)) + scan.Status.State = "HookProcessing" + + if err := r.Status().Update(ctx, scan); err != nil { + r.Log.Error(err, "unable to update Scan status") + return err + } + + r.Log.Info("Finished hook Status field migrations. ReadOnly hook statuses will be updated later.", + "ReadAndWriteHookStatus", scan.Status.ReadAndWriteHookStatus, + "OrderedHookStatuses", scan.Status.OrderedHookStatuses) + + return nil +} + func (r *ScanReconciler) executeHooks(scan *executionv1.Scan) error { ctx := context.Background() diff --git a/operator/controllers/execution/scans/scan_controller.go b/operator/controllers/execution/scans/scan_controller.go index 395d704a8c..19039b94bc 100644 --- a/operator/controllers/execution/scans/scan_controller.go +++ b/operator/controllers/execution/scans/scan_controller.go @@ -101,6 +101,12 @@ func (r *ScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. err = r.setHookStatus(&scan) case "HookProcessing": err = r.executeHooks(&scan) + case "ReadAndWriteHookProcessing": + fallthrough + case "ReadAndWriteHookCompleted": + fallthrough + case "ReadOnlyHookProcessing": + err = r.migrateHookStatus(&scan) } if err != nil { return ctrl.Result{}, err diff --git a/operator/crds/execution.securecodebox.io_scans.yaml b/operator/crds/execution.securecodebox.io_scans.yaml index 608c5a3c95..97192db466 100644 --- a/operator/crds/execution.securecodebox.io_scans.yaml +++ b/operator/crds/execution.securecodebox.io_scans.yaml @@ -2898,6 +2898,29 @@ spec: description: RawResultType determines which kind of ParseDefinition will be used to turn the raw results of the scanner into findings type: string + readAndWriteHookStatus: + items: + properties: + hookName: + type: string + jobName: + type: string + priority: + type: integer + state: + description: HookState Describes the State of a Hook on a Scan + type: string + type: + description: HookType Defines weather the hook should be able + to change the findings or is run in a read only mode. + type: string + required: + - hookName + - priority + - state + - type + type: object + type: array state: type: string type: object diff --git a/operator/utils/orderedhookgroups.go b/operator/utils/orderedhookgroups.go index b734b9aedc..f6be0a6df5 100644 --- a/operator/utils/orderedhookgroups.go +++ b/operator/utils/orderedhookgroups.go @@ -58,7 +58,7 @@ func FromUnorderedList(hooks []executionv1.ScanCompletionHook) [][]*executionv1. groups := [][]*executionv1.HookStatus{} for _, prioClass := range prioClasses { - groups = append(groups, orderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass])...) + groups = append(groups, OrderHookStatusesInsideAPrioClass(hooksByPrioClass[prioClass])...) } return groups @@ -79,7 +79,7 @@ func mapHookToHookStatus(hooks []executionv1.ScanCompletionHook) []*executionv1. return hookStatuses } -func orderHookStatusesInsideAPrioClass(hookStatuses []*executionv1.HookStatus) [][]*executionv1.HookStatus { +func OrderHookStatusesInsideAPrioClass(hookStatuses []*executionv1.HookStatus) [][]*executionv1.HookStatus { groups := [][]*executionv1.HookStatus{} readOnlyGroups := []*executionv1.HookStatus{} for _, hookStatus := range hookStatuses { From 71293e1801e108c031a36a45fa2413c3be7bb2c8 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Mon, 1 Nov 2021 14:02:31 +0100 Subject: [PATCH 16/17] Add migrations to scans which had already finished. Unfortunately can't set the field to `nil` as Kubernetes complains about missing fields priority and type (on non existing elements...) Signed-off-by: Jop Zitman --- operator/controllers/execution/scans/hook_reconciler.go | 7 ++++++- operator/controllers/execution/scans/scan_controller.go | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/operator/controllers/execution/scans/hook_reconciler.go b/operator/controllers/execution/scans/hook_reconciler.go index e99c9322f5..bba58a7673 100644 --- a/operator/controllers/execution/scans/hook_reconciler.go +++ b/operator/controllers/execution/scans/hook_reconciler.go @@ -81,6 +81,9 @@ func (r *ScanReconciler) migrateHookStatus(scan *executionv1.Scan) error { // Had already started ReadOnly hooks and should now check status. // No status for ReadOnly in old CRD, so mark everything as InProgress and let processInProgressHook update it later. hookStatus.State = executionv1.InProgress + } else if scan.Status.State == "Done" { + // Had completely finished + hookStatus.State = executionv1.Completed } r.Log.Info("Retrieved new ReadOnly hook Status", "New", hookStatus) @@ -90,7 +93,9 @@ func (r *ScanReconciler) migrateHookStatus(scan *executionv1.Scan) error { } scan.Status.OrderedHookStatuses = util.OrderHookStatusesInsideAPrioClass(append(readOnlyHooks, strSlice...)) - scan.Status.State = "HookProcessing" + if scan.Status.State != "Done" { + scan.Status.State = "HookProcessing" + } if err := r.Status().Update(ctx, scan); err != nil { r.Log.Error(err, "unable to update Scan status") diff --git a/operator/controllers/execution/scans/scan_controller.go b/operator/controllers/execution/scans/scan_controller.go index 19039b94bc..82d4ccd262 100644 --- a/operator/controllers/execution/scans/scan_controller.go +++ b/operator/controllers/execution/scans/scan_controller.go @@ -81,6 +81,12 @@ func (r *ScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. // Handle Finalizer if the scan is getting deleted if !scan.ObjectMeta.DeletionTimestamp.IsZero() { + // Check if this Scan has not yet been converted to new CRD + if scan.Status.OrderedHookStatuses == nil && scan.Status.ReadAndWriteHookStatus != nil && scan.Status.State == "Done" { + if err := r.migrateHookStatus(&scan); err != nil { + return ctrl.Result{}, err + } + } if err := r.handleFinalizer(&scan); err != nil { r.Log.Error(err, "Failed to run Scan Finalizer") return ctrl.Result{}, err From ab00c6cbe87af5194011f879f482938923c65b4d Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Mon, 1 Nov 2021 15:26:57 +0100 Subject: [PATCH 17/17] Add priority field to Hook charts Signed-off-by: Jop Zitman --- hooks/cascading-scans/README.md | 1 + hooks/cascading-scans/templates/cascading-scans-hook.yaml | 1 + hooks/cascading-scans/values.yaml | 3 +++ hooks/finding-post-processing/README.md | 1 + .../templates/finding-post-processing-hook.yaml | 1 + hooks/finding-post-processing/values.yaml | 3 +++ hooks/generic-webhook/README.md | 1 + hooks/generic-webhook/templates/webhook-hook.yaml | 1 + hooks/generic-webhook/values.yaml | 3 +++ hooks/notification/README.md | 1 + hooks/notification/templates/notification-hook.yaml | 1 + hooks/notification/values.yaml | 3 +++ hooks/persistence-defectdojo/README.md | 3 ++- .../persistence-defectdojo/templates/persistence-provider.yaml | 3 ++- hooks/persistence-defectdojo/values.yaml | 3 +++ hooks/persistence-elastic/README.md | 1 + hooks/persistence-elastic/templates/persistence-provider.yaml | 1 + hooks/persistence-elastic/values.yaml | 3 +++ hooks/update-field/README.md | 1 + hooks/update-field/templates/update-field-hook.yaml | 1 + hooks/update-field/values.yaml | 3 +++ 21 files changed, 37 insertions(+), 2 deletions(-) diff --git a/hooks/cascading-scans/README.md b/hooks/cascading-scans/README.md index 97165e0fdc..45ed5b4e57 100644 --- a/hooks/cascading-scans/README.md +++ b/hooks/cascading-scans/README.md @@ -162,6 +162,7 @@ zap-http zap-baseline-scan non-invasive medium |-----|------|---------|-------------| | hook.image.repository | string | `"docker.io/securecodebox/hook-cascading-scans"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | The image Tag defaults to the charts version if not defined. | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | ## License diff --git a/hooks/cascading-scans/templates/cascading-scans-hook.yaml b/hooks/cascading-scans/templates/cascading-scans-hook.yaml index 167e5fe7f0..53d7ccc752 100644 --- a/hooks/cascading-scans/templates/cascading-scans-hook.yaml +++ b/hooks/cascading-scans/templates/cascading-scans-hook.yaml @@ -9,6 +9,7 @@ metadata: labels: {{- include "cascading-scans.labels" . | nindent 4 }} spec: + priority: {{ .Values.hook.priority }} type: ReadOnly image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" imagePullSecrets: diff --git a/hooks/cascading-scans/values.yaml b/hooks/cascading-scans/values.yaml index 32aa4e38ac..79f8952aad 100644 --- a/hooks/cascading-scans/values.yaml +++ b/hooks/cascading-scans/values.yaml @@ -14,5 +14,8 @@ hook: # @default -- defaults to the charts version tag: null + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null diff --git a/hooks/finding-post-processing/README.md b/hooks/finding-post-processing/README.md index bb6569b684..001e390a36 100644 --- a/hooks/finding-post-processing/README.md +++ b/hooks/finding-post-processing/README.md @@ -89,6 +89,7 @@ The `override` field specifies the desired fields and values that need to be upd |-----|------|---------|-------------| | hook.image.repository | string | `"docker.io/securecodebox/hook-finding-post-processing"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | The image Tag defaults to the charts version if not defined. | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | | rules | list | `[]` | | diff --git a/hooks/finding-post-processing/templates/finding-post-processing-hook.yaml b/hooks/finding-post-processing/templates/finding-post-processing-hook.yaml index bd7bf7eaad..9541fe8be5 100644 --- a/hooks/finding-post-processing/templates/finding-post-processing-hook.yaml +++ b/hooks/finding-post-processing/templates/finding-post-processing-hook.yaml @@ -9,6 +9,7 @@ metadata: labels: {{- include "finding-post-processing.labels" . | nindent 4 }} spec: + priority: {{ .Values.hook.priority }} type: ReadAndWrite image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" env: diff --git a/hooks/finding-post-processing/values.yaml b/hooks/finding-post-processing/values.yaml index 9ae75ea875..768a8fbbef 100644 --- a/hooks/finding-post-processing/values.yaml +++ b/hooks/finding-post-processing/values.yaml @@ -30,5 +30,8 @@ hook: # @default -- defaults to the charts version tag: null + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null diff --git a/hooks/generic-webhook/README.md b/hooks/generic-webhook/README.md index 9efaad1dfb..55e72d4744 100644 --- a/hooks/generic-webhook/README.md +++ b/hooks/generic-webhook/README.md @@ -57,6 +57,7 @@ Kubernetes: `>=v1.11.0-0` |-----|------|---------|-------------| | hook.image.repository | string | `"docker.io/securecodebox/hook-generic-webhook"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | The image Tag defaults to the charts version if not defined. | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | | webhookUrl | string | `"http://example.com"` | The URL of your WebHook endpoint | diff --git a/hooks/generic-webhook/templates/webhook-hook.yaml b/hooks/generic-webhook/templates/webhook-hook.yaml index 945d97a082..9a3bbc2b7a 100644 --- a/hooks/generic-webhook/templates/webhook-hook.yaml +++ b/hooks/generic-webhook/templates/webhook-hook.yaml @@ -9,6 +9,7 @@ metadata: labels: {{- include "generic-webhook.labels" . | nindent 4 }} spec: + priority: {{ .Values.hook.priority }} type: ReadOnly image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" ttlSecondsAfterFinished: {{ .Values.hook.ttlSecondsAfterFinished }} diff --git a/hooks/generic-webhook/values.yaml b/hooks/generic-webhook/values.yaml index deae4c2eb3..146b3a55e2 100644 --- a/hooks/generic-webhook/values.yaml +++ b/hooks/generic-webhook/values.yaml @@ -17,5 +17,8 @@ hook: # @default -- defaults to the charts version tag: null + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null diff --git a/hooks/notification/README.md b/hooks/notification/README.md index 5b3811b26a..2c77f0841a 100644 --- a/hooks/notification/README.md +++ b/hooks/notification/README.md @@ -308,6 +308,7 @@ To fill your template with data we provide the following objects. | hook.image.pullPolicy | string | `"IfNotPresent"` | Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images | | hook.image.repository | string | `"docker.io/securecodebox/hook-notification"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | Image tag | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | | notificationChannels[0].endPoint | string | `"SOME_ENV_KEY"` | | | notificationChannels[0].name | string | `"slack"` | | diff --git a/hooks/notification/templates/notification-hook.yaml b/hooks/notification/templates/notification-hook.yaml index 5d1fd2700a..33568fb23e 100644 --- a/hooks/notification/templates/notification-hook.yaml +++ b/hooks/notification/templates/notification-hook.yaml @@ -7,6 +7,7 @@ kind: ScanCompletionHook metadata: name: {{ include "notification-hook.fullname" . }} spec: + priority: {{ .Values.hook.priority }} type: ReadOnly imagePullPolicy: "{{ .Values.hook.image.pullPolicy }}" image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" diff --git a/hooks/notification/values.yaml b/hooks/notification/values.yaml index b541537e36..7fa042f1ff 100644 --- a/hooks/notification/values.yaml +++ b/hooks/notification/values.yaml @@ -16,6 +16,9 @@ hook: # -- Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images pullPolicy: IfNotPresent + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null diff --git a/hooks/persistence-defectdojo/README.md b/hooks/persistence-defectdojo/README.md index 60462903b7..3dcaf95b9b 100644 --- a/hooks/persistence-defectdojo/README.md +++ b/hooks/persistence-defectdojo/README.md @@ -217,7 +217,8 @@ spec: | defectdojo.url | string | `"http://defectdojo-django.default.svc"` | Url to the DefectDojo Instance | | hook.image.pullPolicy | string | `"IfNotPresent"` | Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images | | hook.image.repository | string | `"docker.io/securecodebox/hook-persistence-defectdojo"` | Hook image repository | -| hook.image.tag | string | `nil` | Container image tag | +| hook.image.tag | string | defaults to the charts version | Container image tag | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | ## License [![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0) diff --git a/hooks/persistence-defectdojo/templates/persistence-provider.yaml b/hooks/persistence-defectdojo/templates/persistence-provider.yaml index 8d03afccf0..f633c5d16d 100644 --- a/hooks/persistence-defectdojo/templates/persistence-provider.yaml +++ b/hooks/persistence-defectdojo/templates/persistence-provider.yaml @@ -2,7 +2,7 @@ # # SPDX-License-Identifier: Apache-2.0 -apiVersion: "execution.securecodebox.io/v1" +apiVersion: execution.securecodebox.io/v1 kind: ScanCompletionHook metadata: name: {{ include "persistence-defectdojo.fullname" . }} @@ -10,6 +10,7 @@ metadata: {{- include "persistence-defectdojo.labels" . | nindent 4 }} type: Unstructured spec: + priority: {{ .Values.hook.priority }} {{- if .Values.defectdojo.syncFindingsBack }} type: ReadAndWrite {{- else }} diff --git a/hooks/persistence-defectdojo/values.yaml b/hooks/persistence-defectdojo/values.yaml index 894c82c74b..2925edc594 100644 --- a/hooks/persistence-defectdojo/values.yaml +++ b/hooks/persistence-defectdojo/values.yaml @@ -16,6 +16,9 @@ hook: # -- Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images pullPolicy: IfNotPresent + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + defectdojo: # -- Syncs back (two way sync) all imported findings from DefectDojo to SCB Findings Store. When set to false the hook will only import the findings to DefectDojo (one way sync). syncFindingsBack: true diff --git a/hooks/persistence-elastic/README.md b/hooks/persistence-elastic/README.md index c2094c39f2..4284635768 100644 --- a/hooks/persistence-elastic/README.md +++ b/hooks/persistence-elastic/README.md @@ -82,6 +82,7 @@ the [Luxon documentation](https://moment.github.io/luxon/docs/manual/formatting. | fullnameOverride | string | `""` | | | hook.image.repository | string | `"docker.io/securecodebox/hook-persistence-elastic"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | The image Tag defaults to the charts version if not defined. | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | | imagePullSecrets | list | `[]` | | | indexAppendNamespace | bool | `true` | Define if the name of the namespace where this hook is deployed to must be added to the index name. The namespace can be used to separate index by tenants (namespaces). | diff --git a/hooks/persistence-elastic/templates/persistence-provider.yaml b/hooks/persistence-elastic/templates/persistence-provider.yaml index 4040c9c37d..0c4e496088 100644 --- a/hooks/persistence-elastic/templates/persistence-provider.yaml +++ b/hooks/persistence-elastic/templates/persistence-provider.yaml @@ -10,6 +10,7 @@ metadata: {{- include "persistence-elastic.labels" . | nindent 4 }} type: Structured spec: + priority: {{ .Values.hook.priority }} type: ReadOnly image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" ttlSecondsAfterFinished: {{ .Values.hook.ttlSecondsAfterFinished }} diff --git a/hooks/persistence-elastic/values.yaml b/hooks/persistence-elastic/values.yaml index 10e691287c..c16a33098c 100644 --- a/hooks/persistence-elastic/values.yaml +++ b/hooks/persistence-elastic/values.yaml @@ -86,5 +86,8 @@ hook: # @default -- defaults to the charts version tag: null + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null diff --git a/hooks/update-field/README.md b/hooks/update-field/README.md index 56746b6516..1bf23484fe 100644 --- a/hooks/update-field/README.md +++ b/hooks/update-field/README.md @@ -64,6 +64,7 @@ helm upgrade --install ufh secureCodeBox/update-field --set attribute.name="cate | attribute.value | string | `"my-own-category"` | The value of the attribute you want to add to each finding result | | hook.image.repository | string | `"docker.io/securecodebox/hook-update-field"` | Hook image repository | | hook.image.tag | string | defaults to the charts version | The image Tag defaults to the charts version if not defined. | +| hook.priority | int | `0` | Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. | | hook.ttlSecondsAfterFinished | string | `nil` | Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ | ## License diff --git a/hooks/update-field/templates/update-field-hook.yaml b/hooks/update-field/templates/update-field-hook.yaml index 5acc24e9e3..aa1941120d 100644 --- a/hooks/update-field/templates/update-field-hook.yaml +++ b/hooks/update-field/templates/update-field-hook.yaml @@ -9,6 +9,7 @@ metadata: labels: {{- include "update-field.labels" . | nindent 4 }} spec: + priority: {{ .Values.hook.priority }} type: ReadAndWrite image: "{{ .Values.hook.image.repository }}:{{ .Values.hook.image.tag | default .Chart.Version }}" ttlSecondsAfterFinished: {{ .Values.hook.ttlSecondsAfterFinished }} diff --git a/hooks/update-field/values.yaml b/hooks/update-field/values.yaml index 60ad92daec..d69d86997a 100644 --- a/hooks/update-field/values.yaml +++ b/hooks/update-field/values.yaml @@ -20,5 +20,8 @@ hook: # @default -- defaults to the charts version tag: null + # -- Hook priority. Higher priority Hooks are guaranteed to execute before low priority Hooks. + priority: 0 + # hook.ttlSecondsAfterFinished -- Seconds after which the kubernetes job for the hook will be deleted. Requires the Kubernetes TTLAfterFinished controller: https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ ttlSecondsAfterFinished: null