feat(kargo): introduce PromotionBuilder

The builder is capable of inflating steps from a (Cluster)PromotionTask into the Promotion it constructs. To support this, a helper method has been added to the `PromotionStep` in the (public) API to generate an alias for the step if none is configured. Signed-off-by: Hidde Beydals <[email protected]>
akuity · Dec 11, 2024 · 9d1386a · 9d1386a
1 parent ffc480b
commit 9d1386a
Show file tree

Hide file tree

Showing 7 changed files with 304 additions and 73 deletions.
diff --git a/api/v1alpha1/promotion_types.go b/api/v1alpha1/promotion_types.go
@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	"fmt"
 	"time"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -224,6 +225,21 @@ type PromotionStep struct {
 	Config *apiextensionsv1.JSON `json:"config,omitempty" protobuf:"bytes,3,opt,name=config"`
 }
 
+// GetAlias returns the As field, or a default value in the form of "step-<i>"
+// or "task-<i>" if the As field is empty. The index i is provided as an
+// argument to this method and should be the index of the PromotionStep in the
+// list it belongs to.
+func (s *PromotionStep) GetAlias(i int) string {
+	switch {
+	case s.As != "":
+		return s.As
+	case s.Task != nil:
+		return fmt.Sprintf("task-%d", i)
+	default:
+		return fmt.Sprintf("step-%d", i)
+	}
+}
+
 // PromotionStatus describes the current state of the transition represented by
 // a Promotion.
 type PromotionStatus struct {

diff --git a/internal/api/promote_downstream_v1alpha1.go b/internal/api/promote_downstream_v1alpha1.go
@@ -137,19 +137,23 @@ func (s *server) PromoteDownstream(
 	promoteErrs := make([]error, 0, len(downstreams))
 	createdPromos := make([]*kargoapi.Promotion, 0, len(downstreams))
 	for _, downstream := range downstreams {
-		newPromo := kargo.NewPromotion(ctx, downstream, freight.Name)
 		if downstream.Spec.PromotionTemplate != nil &&
 			len(downstream.Spec.PromotionTemplate.Spec.Steps) == 0 {
 			// Avoid creating a Promotion if the downstream Stage has no promotion
 			// steps and is therefore a "control flow" Stage.
 			continue
 		}
-		if err := s.createPromotionFn(ctx, &newPromo); err != nil {
+		newPromo, err := kargo.NewPromotionBuilder(s.client).Build(ctx, downstream, freight.Name)
+		if err != nil {
 			promoteErrs = append(promoteErrs, err)
 			continue
 		}
-		s.recordPromotionCreatedEvent(ctx, &newPromo, freight)
-		createdPromos = append(createdPromos, &newPromo)
+		if err = s.createPromotionFn(ctx, newPromo); err != nil {
+			promoteErrs = append(promoteErrs, err)
+			continue
+		}
+		s.recordPromotionCreatedEvent(ctx, newPromo, freight)
+		createdPromos = append(createdPromos, newPromo)
 	}
 
 	res := connect.NewResponse(&svcv1alpha1.PromoteDownstreamResponse{

diff --git a/internal/api/promote_to_stage_v1alpha1.go b/internal/api/promote_to_stage_v1alpha1.go
@@ -98,7 +98,7 @@ func (s *server) PromoteToStage(
 		)
 	}
 
-	if err := s.authorizeFn(
+	if err = s.authorizeFn(
 		ctx,
 		"promote",
 		schema.GroupVersionResource{
@@ -115,13 +115,16 @@ func (s *server) PromoteToStage(
 		return nil, err
 	}
 
-	promotion := kargo.NewPromotion(ctx, *stage, freight.Name)
-	if err := s.createPromotionFn(ctx, &promotion); err != nil {
+	promotion, err := kargo.NewPromotionBuilder(s.client).Build(ctx, *stage, freight.Name)
+	if err != nil {
+		return nil, fmt.Errorf("build promotion: %w", err)
+	}
+	if err := s.createPromotionFn(ctx, promotion); err != nil {
 		return nil, fmt.Errorf("create promotion: %w", err)
 	}
-	s.recordPromotionCreatedEvent(ctx, &promotion, freight)
+	s.recordPromotionCreatedEvent(ctx, promotion, freight)
 	return connect.NewResponse(&svcv1alpha1.PromoteToStageResponse{
-		Promotion: &promotion,
+		Promotion: promotion,
 	}), nil
 }
 

diff --git a/internal/controller/stages/regular_stages.go b/internal/controller/stages/regular_stages.go
@@ -1573,19 +1573,25 @@ func (r *RegularStageReconciler) autoPromoteFreight(
 		}
 
 		// Auto promote the latest available Freight and record an event.
-		promotion := kargo.NewPromotion(ctx, *stage, latestFreight.Name)
-		if err := r.client.Create(ctx, &promotion); err != nil {
+		promotion, err := kargo.NewPromotionBuilder(r.client).Build(ctx, *stage, latestFreight.Name)
+		if err != nil {
+			return newStatus, fmt.Errorf(
+				"error building Promotion for Freight %q in namespace %q: %w",
+				latestFreight.Name, stage.Namespace, err,
+			)
+		}
+		if err = r.client.Create(ctx, promotion); err != nil {
 			return newStatus, fmt.Errorf(
 				"error creating Promotion for Freight %q in namespace %q: %w",
 				latestFreight.Name, stage.Namespace, err,
 			)
 		}
 		r.eventRecorder.AnnotatedEventf(
-			&promotion,
+			promotion,
 			kargoEvent.NewPromotionAnnotations(
 				ctx,
 				kargoapi.FormatEventControllerActor(r.cfg.Name()),
-				&promotion,
+				promotion,
 				&latestFreight,
 			),
 			corev1.EventTypeNormal,

diff --git a/internal/kargo/kargo.go b/internal/kargo/kargo.go
@@ -1,71 +1,13 @@
 package kargo
 
 import (
-	"context"
-	"fmt"
-	"strings"
-
-	"github.com/oklog/ulid/v2"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
-	"github.com/akuity/kargo/internal/api/user"
 	"github.com/akuity/kargo/internal/logging"
 )
 
-const (
-	// maximum length of the stage name used in the promotion name prefix before it exceeds
-	// kubernetes resource name limit of 253
-	// 253 - 1 (.) - 26 (ulid) - 1 (.) - 7 (sha) = 218
-	maxStageNamePrefixLength = 218
-)
-
-// NewPromotion returns a new Promotion from a given stage and freight with our
-// naming convention.
-func NewPromotion(
-	ctx context.Context,
-	stage kargoapi.Stage,
-	freight string,
-) kargoapi.Promotion {
-	shortHash := freight
-	if len(shortHash) > 7 {
-		shortHash = freight[0:7]
-	}
-	shortStageName := stage.Name
-	if len(stage.Name) > maxStageNamePrefixLength {
-		shortStageName = shortStageName[0:maxStageNamePrefixLength]
-	}
-
-	annotations := make(map[string]string, 1)
-	// Put actor information to track on the controller side
-	if u, ok := user.InfoFromContext(ctx); ok {
-		annotations[kargoapi.AnnotationKeyCreateActor] = kargoapi.FormatEventUserActor(u)
-	}
-
-	// ulid.Make() is pseudo-random, not crypto-random, but we don't care.
-	// We just want a unique ID that can be sorted lexicographically
-	promoName := strings.ToLower(fmt.Sprintf("%s.%s.%s", shortStageName, ulid.Make(), shortHash))
-
-	promotion := kargoapi.Promotion{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:        promoName,
-			Namespace:   stage.Namespace,
-			Annotations: annotations,
-		},
-		Spec: kargoapi.PromotionSpec{
-			Stage:   stage.Name,
-			Freight: freight,
-		},
-	}
-	if stage.Spec.PromotionTemplate != nil {
-		promotion.Spec.Vars = stage.Spec.PromotionTemplate.Spec.Vars
-		promotion.Spec.Steps = stage.Spec.PromotionTemplate.Spec.Steps
-	}
-	return promotion
-}
-
 func NewPromoWentTerminalPredicate(logger *logging.Logger) PromoWentTerminal[*kargoapi.Promotion] {
 	return PromoWentTerminal[*kargoapi.Promotion]{
 		logger: logger,

diff --git a/internal/kargo/kargo_test.go b/internal/kargo/kargo_test.go
@@ -37,6 +37,17 @@ func TestNewPromotion(t *testing.T) {
 					Name:      "test",
 					Namespace: "kargo-demo",
 				},
+				Spec: kargoapi.StageSpec{
+					PromotionTemplate: &kargoapi.PromotionTemplate{
+						Spec: kargoapi.PromotionTemplateSpec{
+							Steps: []kargoapi.PromotionStep{
+								{
+									Uses: "fake-step",
+								},
+							},
+						},
+					},
+				},
 			},
 			freight: testFreight,
 			assertions: func(t *testing.T, _ kargoapi.Stage, promo kargoapi.Promotion) {
@@ -53,6 +64,17 @@ func TestNewPromotion(t *testing.T) {
 					Name:      veryLongResourceName,
 					Namespace: "kargo-demo",
 				},
+				Spec: kargoapi.StageSpec{
+					PromotionTemplate: &kargoapi.PromotionTemplate{
+						Spec: kargoapi.PromotionTemplateSpec{
+							Steps: []kargoapi.PromotionStep{
+								{
+									Uses: "fake-step",
+								},
+							},
+						},
+					},
+				},
 			},
 			freight: testFreight,
 			assertions: func(t *testing.T, _ kargoapi.Stage, promo kargoapi.Promotion) {
@@ -65,12 +87,13 @@ func TestNewPromotion(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			promo := NewPromotion(context.TODO(), tc.stage, tc.freight)
+			promo, err := NewPromotionBuilder(nil).Build(context.TODO(), tc.stage, tc.freight)
+			require.NoError(t, err)
 			require.Equal(t, tc.freight, promo.Spec.Freight)
 			require.Equal(t, tc.stage.Name, promo.Spec.Stage)
 			require.Equal(t, tc.freight, promo.Spec.Freight)
 			require.LessOrEqual(t, len(promo.Name), 253)
-			tc.assertions(t, tc.stage, promo)
+			tc.assertions(t, tc.stage, *promo)
 		})
 	}
 }