refactor: introduce containerImageScanStatusPatch to reduce code dupl…

…ication (#1028)
statnett · Jul 17, 2024 · 3f754db · 3f754db
1 parent 25367f8
commit 3f754db
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 91 deletions.
diff --git a/internal/controller/stas/containerimagescan_controller.go b/internal/controller/stas/containerimagescan_controller.go
@@ -132,20 +132,15 @@ func (r *ContainerImageScanReconciler) reconcile(ctx context.Context, cis *stasv
 		return result, err
 	}
 
-	condition := metav1ac.Condition().
-		WithType(string(kstatus.ConditionReconciling)).
-		WithStatus(metav1.ConditionTrue).
-		WithReason("ScanJobCreated").
-		WithMessage(fmt.Sprintf("Job '%s' created to scan image.", scanJob.Name))
-	patch := newContainerImageStatusPatch(cis)
-	patch.Status.
-		WithConditions(NewConditionsPatch(cis.Status.Conditions, condition)...)
-
-	if err := upgradeStatusManagedFields(ctx, r.Client, cis); err != nil {
-		return result, err
-	}
-
-	return result, r.Status().Patch(ctx, cis, applyPatch{patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner)
+	return result, newContainerImageStatusPatch(cis).
+		withCondition(
+			metav1ac.Condition().
+				WithType(string(kstatus.ConditionReconciling)).
+				WithStatus(metav1.ConditionTrue).
+				WithReason("ScanJobCreated").
+				WithMessage(fmt.Sprintf("Job '%s' created to scan image.", scanJob.Name)),
+		).
+		apply(ctx, r.Client)
 }
 
 func (r *ContainerImageScanReconciler) newScanJob(ctx context.Context, cis *stasv1alpha1.ContainerImageScan) (*batchv1.Job, error) {

diff --git a/internal/controller/stas/containerimagescan_status.go b/internal/controller/stas/containerimagescan_status.go
@@ -1,11 +1,19 @@
 package stas
 
 import (
+	"context"
+	"fmt"
+
+	batchv1 "k8s.io/api/batch/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1ac "k8s.io/client-go/applyconfigurations/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	stasv1alpha1 "github.com/statnett/image-scanner-operator/api/stas/v1alpha1"
 	stasv1alpha1ac "github.com/statnett/image-scanner-operator/internal/client/applyconfiguration/stas/v1alpha1"
 )
 
-func newContainerImageStatusPatch(cis *stasv1alpha1.ContainerImageScan) *stasv1alpha1ac.ContainerImageScanApplyConfiguration {
+func newContainerImageStatusPatch(cis *stasv1alpha1.ContainerImageScan) *containerImageScanStatusPatch {
 	status := stasv1alpha1ac.ContainerImageScanStatus().
 		WithObservedGeneration(cis.Generation).
 		WithLastScanJobUID(cis.Status.LastScanJobUID)
@@ -28,8 +36,80 @@ func newContainerImageStatusPatch(cis *stasv1alpha1.ContainerImageScan) *stasv1a
 		}
 	}
 
-	return stasv1alpha1ac.ContainerImageScan(cis.Name, cis.Namespace).
-		WithStatus(status)
+	return &containerImageScanStatusPatch{
+		cis: cis,
+		patch: stasv1alpha1ac.ContainerImageScan(cis.Name, cis.Namespace).
+			WithStatus(status),
+	}
+}
+
+type containerImageScanStatusPatch struct {
+	cis             *stasv1alpha1.ContainerImageScan
+	patch           *stasv1alpha1ac.ContainerImageScanApplyConfiguration
+	vulnerabilities []stasv1alpha1.Vulnerability
+	minSeverity     *stasv1alpha1.Severity
+}
+
+func (p *containerImageScanStatusPatch) withCondition(c *metav1ac.ConditionApplyConfiguration) *containerImageScanStatusPatch {
+	p.patch.Status.
+		WithConditions(NewConditionsPatch(p.cis.Status.Conditions, c)...)
+	return p
+}
+
+func (p *containerImageScanStatusPatch) withScanJob(job *batchv1.Job) *containerImageScanStatusPatch {
+	p.patch.Status.
+		WithLastScanTime(metav1.Now()).
+		WithLastScanJobUID(job.UID)
+
+	return p
+}
+
+func (p *containerImageScanStatusPatch) withCompletedScanJob(job *batchv1.Job, vulnerabilities []stasv1alpha1.Vulnerability, minSeverity stasv1alpha1.Severity) *containerImageScanStatusPatch {
+	p.minSeverity = &minSeverity
+	p.vulnerabilities = vulnerabilities
+
+	now := metav1.Now()
+	p.patch.Status.
+		WithVulnerabilitySummary(vulnerabilitySummary(vulnerabilities, minSeverity)).
+		WithLastScanTime(now).
+		WithLastScanJobUID(job.UID).
+		WithLastSuccessfulScanTime(now)
+
+	return p
+}
+
+func (p *containerImageScanStatusPatch) apply(ctx context.Context, c client.Client) error {
+	if err := upgradeStatusManagedFields(ctx, c, p.cis); err != nil {
+		return fmt.Errorf("when upgrading status managed fields: %w", err)
+	}
+
+	if p.minSeverity == nil {
+		if err := c.Status().Patch(ctx, p.cis, applyPatch{p.patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner); err != nil {
+			return fmt.Errorf("when patching status: %w", err)
+		}
+
+		return nil
+	}
+
+	var err error
+	// Repeat until resource fits in api-server by increasing minimum severity on failure.
+	for severity := *p.minSeverity; severity <= stasv1alpha1.MaxSeverity; severity++ {
+		p.patch.Status.Vulnerabilities, err = filterVulnerabilities(p.vulnerabilities, severity)
+		if err != nil {
+			return err
+		}
+
+		err = c.Status().Patch(ctx, p.cis, applyPatch{p.patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner)
+		if !isResourceTooLargeError(err) {
+			break
+		}
+	}
+
+	if err != nil {
+		return fmt.Errorf("when patching status: %w", err)
+	}
+
+	return nil
 }
 
 func vulnerabilityPatch(v stasv1alpha1.Vulnerability) *stasv1alpha1ac.VulnerabilityApplyConfiguration {

diff --git a/internal/controller/stas/scan_job_controller.go b/internal/controller/stas/scan_job_controller.go
@@ -160,27 +160,16 @@ func (r *ScanJobReconciler) reconcileCompleteJob(ctx context.Context, job *batch
 
 	err := json.NewDecoderCaseSensitivePreserveInts(log).Decode(&vulnerabilities)
 	if err != nil {
-		condition := metav1ac.Condition().
-			WithType(string(kstatus.ConditionStalled)).
-			WithStatus(metav1.ConditionTrue).
-			WithReason(stasv1alpha1.ReasonScanReportDecodeError).
-			WithMessage(fmt.Sprintf("error decoding scan report JSON from job '%s': %s", job.Name, err))
-		patch := newContainerImageStatusPatch(cis)
-		patch.Status.
-			WithConditions(NewConditionsPatch(cis.Status.Conditions, condition)...).
-			WithLastScanTime(metav1.Now()).
-			WithLastScanJobUID(job.UID)
-
-		if err := upgradeStatusManagedFields(ctx, r.Client, cis); err != nil {
-			return err
-		}
-
-		err = r.Status().Patch(ctx, cis, applyPatch{patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner)
-		if err != nil {
-			logf.FromContext(ctx).Error(err, "when patching status", "condition", condition)
-		}
-
-		return err
+		return newContainerImageStatusPatch(cis).
+			withCondition(
+				metav1ac.Condition().
+					WithType(string(kstatus.ConditionStalled)).
+					WithStatus(metav1.ConditionTrue).
+					WithReason(stasv1alpha1.ReasonScanReportDecodeError).
+					WithMessage(fmt.Sprintf("error decoding scan report JSON from job '%s': %s", job.Name, err)),
+			).
+			withScanJob(job).
+			apply(ctx, r.Client)
 	}
 
 	sort.Sort(stasv1alpha1.BySeverity(vulnerabilities))
@@ -193,38 +182,9 @@ func (r *ScanJobReconciler) reconcileCompleteJob(ctx context.Context, job *batch
 		}
 	}
 
-	return r.updateCISStatus(ctx, job, cis, vulnerabilities, minSeverity)
-}
-
-func (r *ScanJobReconciler) updateCISStatus(ctx context.Context, job *batchv1.Job, cis *stasv1alpha1.ContainerImageScan, vulnerabilities []stasv1alpha1.Vulnerability, minSeverity stasv1alpha1.Severity) error {
-	now := metav1.Now()
-
-	patch := newContainerImageStatusPatch(cis)
-	patch.Status.
-		WithVulnerabilitySummary(vulnerabilitySummary(vulnerabilities, minSeverity)).
-		WithLastScanTime(now).
-		WithLastScanJobUID(job.UID).
-		WithLastSuccessfulScanTime(now)
-
-	if err := upgradeStatusManagedFields(ctx, r.Client, cis); err != nil {
-		return err
-	}
-
-	var err error
-	// Repeat until resource fits in api-server by increasing minimum severity on failure.
-	for severity := minSeverity; severity <= stasv1alpha1.MaxSeverity; severity++ {
-		patch.Status.Vulnerabilities, err = filterVulnerabilities(vulnerabilities, severity)
-		if err != nil {
-			return err
-		}
-
-		err = r.Status().Patch(ctx, cis, applyPatch{patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner)
-		if err == nil || !isResourceTooLargeError(err) {
-			return err
-		}
-	}
-
-	return err
+	return newContainerImageStatusPatch(cis).
+		withCompletedScanJob(job, vulnerabilities, minSeverity).
+		apply(ctx, r.Client)
 }
 
 func isResourceTooLargeError(err error) bool {
@@ -239,27 +199,16 @@ func (r *ScanJobReconciler) reconcileFailedJob(ctx context.Context, job *batchv1
 		return err
 	}
 
-	condition := metav1ac.Condition().
-		WithType(string(kstatus.ConditionStalled)).
-		WithStatus(metav1.ConditionTrue).
-		WithReason("Error").
-		WithMessage(string(logBytes))
-	patch := newContainerImageStatusPatch(cis)
-	patch.Status.
-		WithConditions(NewConditionsPatch(cis.Status.Conditions, condition)...).
-		WithLastScanTime(metav1.Now()).
-		WithLastScanJobUID(job.UID)
-
-	if err := upgradeStatusManagedFields(ctx, r.Client, cis); err != nil {
-		return err
-	}
-
-	err = r.Status().Patch(ctx, cis, applyPatch{patch}, FieldValidationStrict, client.ForceOwnership, fieldOwner)
-	if err != nil {
-		logf.FromContext(ctx).Error(err, "when patching status", "condition", condition)
-	}
-
-	return err
+	return newContainerImageStatusPatch(cis).
+		withCondition(
+			metav1ac.Condition().
+				WithType(string(kstatus.ConditionStalled)).
+				WithStatus(metav1.ConditionTrue).
+				WithReason("Error").
+				WithMessage(string(logBytes)),
+		).
+		withScanJob(job).
+		apply(ctx, r.Client)
 }
 
 func (r *ScanJobReconciler) reconcile() reconcile.Func {