chore(api): simplify annotation patch helper

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

api/v1alpha1/helpers.go

@@ -3,23 +3,22 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func refreshObject(
+func patchObjectAnnotation(
 	ctx context.Context,
 	c client.Client,
 	obj client.Object,
-	nowFunc func() time.Time,
+	key, value string,
 ) error {
 	patchBytes := []byte(
 		fmt.Sprintf(
 			`{"metadata":{"annotations":{"%s":"%s"}}}`,
-			AnnotationKeyRefresh,
-			nowFunc().UTC().Format(time.RFC3339),
+			key,
+			value,
 		),
 	)
 	patch := client.RawPatch(types.MergePatchType, patchBytes)

api/v1alpha1/helpers_test.go

@@ -3,7 +3,6 @@ package v1alpha1
 import (
 	"context"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -12,7 +11,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
-func Test_refreshObject(t *testing.T) {
+func Test_patchObjectAnnotation(t *testing.T) {
 	scheme := k8sruntime.NewScheme()
 	require.NoError(t, SchemeBuilder.AddToScheme(scheme))
 
@@ -23,13 +22,12 @@ func Test_refreshObject(t *testing.T) {
 			WithObjects(obj...).
 			Build()
 	}
-	mockNow := func() time.Time {
-		return time.Date(2023, 11, 2, 0, 0, 0, 0, time.UTC)
-	}
+
 	testCases := map[string]struct {
 		obj         client.Object
-		cli         client.Client
-		nowFunc     func() time.Time
+		client      client.Client
+		key         string
+		value       string
 		errExpected bool
 	}{
 		"stage": {
@@ -39,31 +37,33 @@ func Test_refreshObject(t *testing.T) {
 					Name:      "stage",
 				},
 			},
-			cli: newFakeClient(&Stage{
+			client: newFakeClient(&Stage{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
 					Name:      "stage",
 				},
 			}),
-			nowFunc: mockNow,
+			key:   "key",
+			value: "value",
 		},
-		"stage with refresh annotation key": {
+		"stage with existing annotation": {
 			obj: &Stage{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
 					Name:      "stage",
 				},
 			},
-			cli: newFakeClient(&Stage{
+			client: newFakeClient(&Stage{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
 					Name:      "stage",
 					Annotations: map[string]string{
-						AnnotationKeyRefresh: time.Date(2023, 11, 1, 0, 0, 0, 0, time.UTC).Format(time.RFC3339),
+						"key": "value",
 					},
 				},
 			}),
-			nowFunc: mockNow,
+			key:   "key",
+			value: "value2",
 		},
 		"non-existing stage": {
 			obj: &Stage{
@@ -72,23 +72,22 @@ func Test_refreshObject(t *testing.T) {
 					Name:      "stage",
 				},
 			},
-			cli:         newFakeClient(),
-			nowFunc:     mockNow,
+			client:      newFakeClient(),
+			key:         "key",
+			value:       "value",
 			errExpected: true,
 		},
 	}
 	for name, tc := range testCases {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
-			err := refreshObject(context.Background(), tc.cli, tc.obj, tc.nowFunc)
+			err := patchObjectAnnotation(context.Background(), tc.client, tc.obj, tc.key, tc.value)
 			if tc.errExpected {
 				require.Error(t, err)
 				return
 			}
-			require.Contains(t, tc.obj.GetAnnotations(), AnnotationKeyRefresh)
-			actual, err := time.Parse(time.RFC3339, tc.obj.GetAnnotations()[AnnotationKeyRefresh])
-			require.NoError(t, err)
-			require.Equal(t, tc.nowFunc(), actual)
+			require.Contains(t, tc.obj.GetAnnotations(), tc.key)
+			require.Equal(t, tc.obj.GetAnnotations()[tc.key], tc.value)
 		})
 	}
 }

api/v1alpha1/promotion_helpers.go

@@ -48,7 +48,7 @@ func RefreshPromotion(
 			Name:      namespacedName.Name,
 		},
 	}
-	if err := refreshObject(ctx, c, promo, time.Now); err != nil {
+	if err := patchObjectAnnotation(ctx, c, promo, AnnotationKeyRefresh, time.Now().Format(time.RFC3339)); err != nil {
 		return nil, fmt.Errorf("refresh: %w", err)
 	}
 	return promo, nil

api/v1alpha1/stage_helpers.go

@@ -49,7 +49,7 @@ func RefreshStage(
 			Name:      namespacedName.Name,
 		},
 	}
-	if err := refreshObject(ctx, c, stage, time.Now); err != nil {
+	if err := patchObjectAnnotation(ctx, c, stage, AnnotationKeyRefresh, time.Now().Format(time.RFC3339)); err != nil {
 		return nil, fmt.Errorf("refresh: %w", err)
 	}
 	return stage, nil
@@ -108,18 +108,7 @@ func ReverifyStageFreight(
 		return fmt.Errorf("stage verification info has no ID")
 	}
 
-	patchBytes := []byte(
-		fmt.Sprintf(
-			`{"metadata":{"annotations":{"%s":"%s"}}}`,
-			AnnotationKeyReverify,
-			curFreight.VerificationInfo.ID,
-		),
-	)
-	patch := client.RawPatch(types.MergePatchType, patchBytes)
-	if err := c.Patch(ctx, stage, patch); err != nil {
-		return fmt.Errorf("patch annotation: %w", err)
-	}
-	return nil
+	return patchObjectAnnotation(ctx, c, stage, AnnotationKeyReverify, curFreight.VerificationInfo.ID)
 }
 
 // ClearStageReverify is called by the Stage controller to clear the
@@ -183,18 +172,7 @@ func AbortStageFreightVerification(
 		return fmt.Errorf("stage verification info has no ID")
 	}
 
-	patchBytes := []byte(
-		fmt.Sprintf(
-			`{"metadata":{"annotations":{"%s":"%s"}}}`,
-			AnnotationKeyAbort,
-			stage.Status.CurrentFreight.VerificationInfo.ID,
-		),
-	)
-	patch := client.RawPatch(types.MergePatchType, patchBytes)
-	if err := c.Patch(ctx, stage, patch); err != nil {
-		return fmt.Errorf("patch annotation: %w", err)
-	}
-	return nil
+	return patchObjectAnnotation(ctx, c, stage, AnnotationKeyAbort, curFreight.VerificationInfo.ID)
 }
 
 // ClearStageAbort is called by the Stage controller to clear the

api/v1alpha1/warehouse_helpers.go

@@ -48,7 +48,7 @@ func RefreshWarehouse(
 			Name:      namespacedName.Name,
 		},
 	}
-	if err := refreshObject(ctx, c, warehouse, time.Now); err != nil {
+	if err := patchObjectAnnotation(ctx, c, warehouse, AnnotationKeyRefresh, time.Now().Format(time.RFC3339)); err != nil {
 		return nil, fmt.Errorf("refresh: %w", err)
 	}
 	return warehouse, nil