Skip to content

Commit 8d60225

Browse files
committed
chore: slightly tweak implementation details
Signed-off-by: Hidde Beydals <[email protected]>
1 parent 54286be commit 8d60225

File tree

4 files changed

+92
-70
lines changed

4 files changed

+92
-70
lines changed

internal/controller/promotions/promotions.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -479,8 +479,6 @@ func (r *reconciler) promote(
479479
}
480480
}
481481

482-
creator := parseActorAnnotation(&promo)
483-
484482
promoCtx := directives.PromotionContext{
485483
UIBaseURL: r.cfg.APIServerBaseURL,
486484
WorkDir: filepath.Join(os.TempDir(), "promotion-"+string(workingPromo.UID)),
@@ -493,7 +491,7 @@ func (r *reconciler) promote(
493491
StepExecutionMetadata: promo.Status.StepExecutionMetadata,
494492
State: directives.State(workingPromo.Status.GetState()),
495493
Vars: workingPromo.Spec.Vars,
496-
Creator: creator,
494+
Actor: parseCreateActorAnnotation(&promo),
497495
}
498496
if err := os.Mkdir(promoCtx.WorkDir, 0o700); err == nil {
499497
// If we're working with a fresh directory, we should start the promotion
@@ -650,13 +648,18 @@ func (r *reconciler) terminatePromotion(
650648
return nil
651649
}
652650

653-
func parseActorAnnotation(promo *kargoapi.Promotion) string {
654-
creator := "N/A"
655-
actorAnnotation, ok := promo.ObjectMeta.Annotations[kargoapi.AnnotationKeyCreateActor]
656-
if ok {
657-
substrings := strings.Split(actorAnnotation, ":")
658-
if len(substrings) == 2 {
659-
creator = substrings[1]
651+
// parseCreateActorAnnotation extracts the v1alpha1.AnnotationKeyCreateActor
652+
// value from the Promotion's annotations and returns it. If the value contains
653+
// a colon, it is split and the second part is returned. Otherwise, the entire
654+
// value or an empty string is returned.
655+
func parseCreateActorAnnotation(promo *kargoapi.Promotion) string {
656+
var creator string
657+
if v, ok := promo.Annotations[kargoapi.AnnotationKeyCreateActor]; ok {
658+
if v != kargoapi.EventActorUnknown {
659+
creator = v
660+
}
661+
if parts := strings.Split(v, ":"); len(parts) == 2 {
662+
creator = parts[1]
660663
}
661664
}
662665
return creator

internal/controller/promotions/promotions_test.go

Lines changed: 69 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package promotions
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"testing"
78
"time"
89

@@ -335,60 +336,6 @@ func TestReconcile(t *testing.T) {
335336
}
336337
}
337338

338-
func Test_parseActorAnnotation(t *testing.T) {
339-
tests := []struct {
340-
name string
341-
promo *kargoapi.Promotion
342-
expectedCreator string
343-
}{
344-
{
345-
name: "basic case",
346-
promo: &kargoapi.Promotion{
347-
ObjectMeta: metav1.ObjectMeta{
348-
Name: "fake-promo",
349-
Namespace: "fake-namespace",
350-
Annotations: map[string]string{
351-
kargoapi.AnnotationKeyCreateActor: "email:fake-actor",
352-
},
353-
},
354-
},
355-
expectedCreator: "fake-actor",
356-
},
357-
{
358-
359-
name: "no annotation",
360-
promo: &kargoapi.Promotion{
361-
ObjectMeta: metav1.ObjectMeta{
362-
Name: "fake-promo",
363-
Namespace: "fake-namespace",
364-
},
365-
},
366-
expectedCreator: "N/A",
367-
},
368-
{
369-
name: "invalid",
370-
promo: &kargoapi.Promotion{
371-
ObjectMeta: metav1.ObjectMeta{
372-
Name: "fake-promo",
373-
Namespace: "fake-namespace",
374-
Annotations: map[string]string{
375-
kargoapi.AnnotationKeyCreateActor: "abc123",
376-
},
377-
},
378-
},
379-
expectedCreator: "N/A",
380-
},
381-
}
382-
383-
for _, tc := range tests {
384-
385-
t.Run(tc.name, func(t *testing.T) {
386-
result := parseActorAnnotation(tc.promo)
387-
require.Equal(t, tc.expectedCreator, result)
388-
})
389-
}
390-
}
391-
392339
func Test_reconciler_terminatePromotion(t *testing.T) {
393340
scheme := k8sruntime.NewScheme()
394341
require.NoError(t, kargoapi.SchemeBuilder.AddToScheme(scheme))
@@ -516,6 +463,74 @@ func Test_reconciler_terminatePromotion(t *testing.T) {
516463
}
517464
}
518465

466+
func Test_parseCreateActorAnnotation(t *testing.T) {
467+
tests := []struct {
468+
name string
469+
promo *kargoapi.Promotion
470+
want string
471+
}{
472+
{
473+
name: "basic case",
474+
promo: &kargoapi.Promotion{
475+
ObjectMeta: metav1.ObjectMeta{
476+
Name: "fake-promo",
477+
Namespace: "fake-namespace",
478+
Annotations: map[string]string{
479+
kargoapi.AnnotationKeyCreateActor: fmt.Sprintf(
480+
"%s%s", kargoapi.EventActorEmailPrefix, "fake-actor",
481+
),
482+
},
483+
},
484+
},
485+
want: "fake-actor",
486+
},
487+
{
488+
name: "single element",
489+
promo: &kargoapi.Promotion{
490+
ObjectMeta: metav1.ObjectMeta{
491+
Name: "fake-promo",
492+
Namespace: "fake-namespace",
493+
Annotations: map[string]string{
494+
kargoapi.AnnotationKeyCreateActor: kargoapi.EventActorAdmin,
495+
},
496+
},
497+
},
498+
want: kargoapi.EventActorAdmin,
499+
},
500+
{
501+
name: "unknown actor",
502+
promo: &kargoapi.Promotion{
503+
ObjectMeta: metav1.ObjectMeta{
504+
Name: "fake-promo",
505+
Namespace: "fake-namespace",
506+
Annotations: map[string]string{
507+
kargoapi.AnnotationKeyCreateActor: kargoapi.EventActorUnknown,
508+
},
509+
},
510+
},
511+
want: "",
512+
},
513+
{
514+
515+
name: "no annotation",
516+
promo: &kargoapi.Promotion{
517+
ObjectMeta: metav1.ObjectMeta{
518+
Name: "fake-promo",
519+
Namespace: "fake-namespace",
520+
},
521+
},
522+
want: "",
523+
},
524+
}
525+
526+
for _, tt := range tests {
527+
t.Run(tt.name, func(t *testing.T) {
528+
result := parseCreateActorAnnotation(tt.promo)
529+
require.Equal(t, tt.want, result)
530+
})
531+
}
532+
}
533+
519534
// nolint: unparam
520535
func newPromo(namespace, name, stage string,
521536
phase kargoapi.PromotionPhase,

internal/directives/promotions.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ type PromotionContext struct {
8181
Vars []kargoapi.PromotionVariable
8282
// Secrets is a map of secrets that can be used by the PromotionSteps.
8383
Secrets map[string]map[string]string
84-
// Creator is the name of the actor triggering the Promotion.
85-
Creator string
84+
// Actor is the name of the actor triggering the Promotion.
85+
Actor string
8686
}
8787

8888
// PromotionStep describes a single step in a user-defined promotion process.
@@ -196,7 +196,11 @@ func (s *PromotionStep) BuildEnv(
196196
"project": promoCtx.Project,
197197
"promotion": promoCtx.Promotion,
198198
"stage": promoCtx.Stage,
199-
"creator": promoCtx.Creator,
199+
"meta": map[string]any{
200+
"promotion": map[string]any{
201+
"actor": promoCtx.Actor,
202+
},
203+
},
200204
},
201205
}
202206

internal/directives/promotions_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,19 +207,19 @@ func TestPromotionStep_GetConfig(t *testing.T) {
207207
Project: "fake-project",
208208
Stage: "fake-stage",
209209
Promotion: "fake-promotion",
210-
Creator: "fake-creator",
210+
Actor: "fake-creator",
211211
},
212212
rawCfg: []byte(`{
213213
"project": "${{ ctx.project }}",
214214
"stage": "${{ ctx.stage }}",
215215
"promotion": "${{ ctx.promotion }}",
216-
"creator": "${{ ctx.creator }}"
216+
"actor": "${{ ctx.meta.promotion.actor }}"
217217
}`),
218218
expectedCfg: Config{
219219
"project": "fake-project",
220220
"stage": "fake-stage",
221221
"promotion": "fake-promotion",
222-
"creator": "fake-creator",
222+
"actor": "fake-creator",
223223
},
224224
},
225225
{

0 commit comments

Comments
 (0)