feat(webhook): set actor for Promotion abort request

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

api/v1alpha1/promotion_helpers.go

@@ -2,6 +2,7 @@ package v1alpha1
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -25,6 +26,10 @@ const (
 
 // AbortPromotionRequest is a request payload with an optional actor field which
 // can be used to annotate a Promotion using the AnnotationKeyAbort annotation.
+//
+// +protobuf=false
+// +k8s:deepcopy-gen=false
+// +k8s:openapi-gen=false
 type AbortPromotionRequest struct {
 	// Action is the action to take on the Promotion to abort it.
 	Action AbortAction `json:"action,omitempty" protobuf:"bytes,1,opt,name=action"`
@@ -35,6 +40,32 @@ type AbortPromotionRequest struct {
 	ControlPlane bool `json:"controlPlane,omitempty" protobuf:"varint,3,opt,name=controlPlane"`
 }
 
+// Equals returns true if the AbortPromotionRequest is equal to the other
+// AbortPromotionRequest, false otherwise. Two VerificationRequests are equal
+// if their Action, Actor, and ControlPlane fields are equal.
+func (r *AbortPromotionRequest) Equals(other *AbortPromotionRequest) bool {
+	if r == nil && other == nil {
+		return true
+	}
+	if r == nil || other == nil {
+		return false
+	}
+	return r.Action == other.Action && r.Actor == other.Actor && r.ControlPlane == other.ControlPlane
+}
+
+// String returns the JSON string representation of the AbortPromotionRequest,
+// or an empty string if the AbortPromotionRequest is nil or has an empty Action.
+func (r *AbortPromotionRequest) String() string {
+	if r == nil || r.Action == "" {
+		return ""
+	}
+	b, _ := json.Marshal(r)
+	if b == nil {
+		return ""
+	}
+	return string(b)
+}
+
 // GetPromotion returns a pointer to the Promotion resource specified by the
 // namespacedName argument. If no such resource is found, nil is returned
 // instead.

api/v1alpha1/promotion_helpers_test.go

@@ -14,6 +14,92 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+func TestAbortPromotionRequest_Equals(t *testing.T) {
+	tests := []struct {
+		name     string
+		r1       *AbortPromotionRequest
+		r2       *AbortPromotionRequest
+		expected bool
+	}{
+		{
+			name:     "both nil",
+			r1:       nil,
+			r2:       nil,
+			expected: true,
+		},
+		{
+			name:     "one nil",
+			r1:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: false},
+			r2:       nil,
+			expected: false,
+		},
+		{
+			name:     "other nil",
+			r1:       nil,
+			r2:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: false},
+			expected: false,
+		},
+		{
+			name:     "different actions",
+			r1:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: false},
+			r2:       &AbortPromotionRequest{Action: "other-action", Actor: "fake-actor", ControlPlane: false},
+			expected: false,
+		},
+		{
+			name:     "different actors",
+			r1:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: true},
+			r2:       &AbortPromotionRequest{Action: "fake-action", Actor: "other-actor", ControlPlane: true},
+			expected: false,
+		},
+		{
+			name:     "different control plane flags",
+			r1:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: true},
+			r2:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: false},
+			expected: false,
+		},
+		{
+			name:     "equal",
+			r1:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: true},
+			r2:       &AbortPromotionRequest{Action: "fake-action", Actor: "fake-actor", ControlPlane: true},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.r1.Equals(tt.r2), tt.expected)
+		})
+	}
+}
+
+func TestAbortPromotionRequest_String(t *testing.T) {
+	t.Run("abort request is nil", func(t *testing.T) {
+		var r *AbortPromotionRequest
+		require.Empty(t, r.String())
+	})
+
+	t.Run("abort request is empty", func(t *testing.T) {
+		r := &AbortPromotionRequest{}
+		require.Empty(t, r.String())
+	})
+
+	t.Run("abort request has empty action", func(t *testing.T) {
+		r := &AbortPromotionRequest{
+			Action: "",
+		}
+		require.Empty(t, r.String())
+	})
+
+	t.Run("abort request has data", func(t *testing.T) {
+		r := &AbortPromotionRequest{
+			Action:       "foo",
+			Actor:        "fake-actor",
+			ControlPlane: true,
+		}
+		require.Equal(t, `{"action":"foo","actor":"fake-actor","controlPlane":true}`, r.String())
+	})
+}
+
 func TestGetPromotion(t *testing.T) {
 	scheme := k8sruntime.NewScheme()
 	require.NoError(t, SchemeBuilder.AddToScheme(scheme))

internal/controller/promotions/promotions_test.go

@@ -440,7 +440,8 @@ func Test_reconciler_terminatePromotion(t *testing.T) {
 				recorder:    recorder,
 			}
 
-			err := r.terminatePromotion(context.Background(), tt.req.DeepCopy(), tt.promo, tt.freight)
+			req := tt.req
+			err := r.terminatePromotion(context.Background(), &req, tt.promo, tt.freight)
 			tt.assertions(t, recorder, tt.promo, err)
 		})
 	}

internal/webhook/promotion/webhook.go

@@ -139,20 +139,28 @@ func (w *webhook) Default(ctx context.Context, obj runtime.Object) error {
 	if promo.Annotations == nil {
 		promo.Annotations = make(map[string]string, 1)
 	}
-	if req.Operation == admissionv1.Create {
+
+	switch req.Operation {
+	case admissionv1.Create:
 		// Set actor as an admission request's user info when the promotion is created
 		// to allow controllers to track who created it.
 		if !w.isRequestFromKargoControlplaneFn(req) {
 			promo.Annotations[kargoapi.AnnotationKeyCreateActor] =
 				kargoapi.FormatEventKubernetesUserActor(req.UserInfo)
 		}
-	} else if req.Operation == admissionv1.Update {
+
+		// Enrich the annotation with the actor and control plane information.
+		w.setAbortAnnotationActor(req, nil, promo)
+	case admissionv1.Update:
 		// Ensure actor annotation immutability
 		if oldActor, ok := oldPromo.Annotations[kargoapi.AnnotationKeyCreateActor]; ok {
 			promo.Annotations[kargoapi.AnnotationKeyCreateActor] = oldActor
 		} else {
 			delete(promo.Annotations, kargoapi.AnnotationKeyCreateActor)
 		}
+
+		// Enrich the annotation with the actor and control plane information.
+		w.setAbortAnnotationActor(req, oldPromo, promo)
 	}
 
 	stage, err := w.getStageFn(
@@ -372,3 +380,24 @@ func (w *webhook) recordPromotionCreatedEvent(
 		actor,
 	)
 }
+
+func (w *webhook) setAbortAnnotationActor(req admission.Request, old, new *kargoapi.Promotion) {
+	if abortReq, ok := kargoapi.AbortPromotionAnnotationValue(new.Annotations); ok {
+		var oldAbortReq *kargoapi.AbortPromotionRequest
+		if old != nil {
+			oldAbortReq, _ = kargoapi.AbortPromotionAnnotationValue(old.Annotations)
+		}
+		// If the abort request has changed, enrich the annotation with the
+		// actor and control plane information.
+		if old == nil || oldAbortReq == nil || !abortReq.Equals(oldAbortReq) {
+			abortReq.ControlPlane = w.isRequestFromKargoControlplaneFn(req)
+			if !abortReq.ControlPlane {
+				// If the abort request is not from the control plane, then it's
+				// from a specific Kubernetes user. Without this check we would
+				// overwrite the actor field set by the control plane.
+				abortReq.Actor = kargoapi.FormatEventKubernetesUserActor(req.UserInfo)
+			}
+			new.Annotations[kargoapi.AnnotationKeyAbort] = abortReq.String()
+		}
+	}
+}