fix: add parent step id in composite action step id (#1268)

when running nested composite actions, step ids were repeating
leading to errors in parsing the output. this patch adds the
parent step id to ste stepID field.

Co-authored-by: Björn Brauer <bjoern.brauer@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>

Co-authored-by: Björn Brauer <bjoern.brauer@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

pkg/runner/action_composite.go

@@ -62,6 +62,7 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action
 		Env:              env,
 		Masks:            parent.Masks,
 		ExtraPath:        parent.ExtraPath,
+		Parent:           parent,
 	}
 
 	return compositerc
@@ -137,11 +138,15 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 			}
 		}
 
-		preSteps = append(preSteps, step.pre())
+		stepID := step.getStepModel().ID
+		stepPre := rc.newCompositeCommandExecutor(step.pre())
+		preSteps = append(preSteps, newCompositeStepLogExecutor(stepPre, stepID))
 
 		steps = append(steps, func(ctx context.Context) error {
+			ctx = WithCompositeStepLogger(ctx, stepID)
 			logger := common.Logger(ctx)
-			err := step.main()(ctx)
+			err := rc.newCompositeCommandExecutor(step.main())(ctx)
+
 			if err != nil {
 				logger.Errorf("%v", err)
 				common.SetJobError(ctx, err)
@@ -154,19 +159,24 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 
 		// run the post executor in reverse order
 		if postExecutor != nil {
-			postExecutor = step.post().Finally(postExecutor)
+			stepPost := rc.newCompositeCommandExecutor(step.post())
+			postExecutor = newCompositeStepLogExecutor(stepPost, stepID)
+			stepPost.Finally(postExecutor)
 		} else {
-			postExecutor = step.post()
+			stepPost := rc.newCompositeCommandExecutor(step.post())
+			postExecutor = newCompositeStepLogExecutor(stepPost, stepID)
 		}
 	}
 
 	steps = append(steps, common.JobError)
 	return &compositeSteps{
-		pre: rc.newCompositeCommandExecutor(common.NewPipelineExecutor(preSteps...)),
-		main: rc.newCompositeCommandExecutor(func(ctx context.Context) error {
+		pre: func(ctx context.Context) error {
+			return common.NewPipelineExecutor(preSteps...)(common.WithJobErrorContainer(ctx))
+		},
+		main: func(ctx context.Context) error {
 			return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx))
-		}),
-		post: rc.newCompositeCommandExecutor(postExecutor),
+		},
+		post: postExecutor,
 	}
 }
 
@@ -194,3 +204,19 @@ func (rc *RunContext) newCompositeCommandExecutor(executor common.Executor) comm
 		return executor(ctx)
 	}
 }
+
+func newCompositeStepLogExecutor(runStep common.Executor, stepID string) common.Executor {
+	return func(ctx context.Context) error {
+		ctx = WithCompositeStepLogger(ctx, stepID)
+		logger := common.Logger(ctx)
+		err := runStep(ctx)
+		if err != nil {
+			logger.Errorf("%v", err)
+			common.SetJobError(ctx, err)
+		} else if ctx.Err() != nil {
+			logger.Errorf("%v", ctx.Err())
+			common.SetJobError(ctx, ctx.Err())
+		}
+		return nil
+	}
+}

pkg/runner/logger.go

@@ -96,10 +96,27 @@ func WithCompositeLogger(ctx context.Context, masks *[]string) context.Context {
 	return common.WithLogger(ctx, common.Logger(ctx).WithFields(logrus.Fields{}).WithContext(ctx))
 }
 
+func WithCompositeStepLogger(ctx context.Context, stepID string) context.Context {
+	val := common.Logger(ctx)
+	stepIDs := make([]string, 0)
+
+	if logger, ok := val.(*logrus.Entry); ok {
+		if oldStepIDs, ok := logger.Data["stepID"].([]string); ok {
+			stepIDs = append(stepIDs, oldStepIDs...)
+		}
+	}
+
+	stepIDs = append(stepIDs, stepID)
+
+	return common.WithLogger(ctx, common.Logger(ctx).WithFields(logrus.Fields{
+		"stepID": stepIDs,
+	}).WithContext(ctx))
+}
+
 func withStepLogger(ctx context.Context, stepID string, stepName string, stageName string) context.Context {
 	rtn := common.Logger(ctx).WithFields(logrus.Fields{
 		"step":   stepName,
-		"stepID": stepID,
+		"stepID": []string{stepID},
 		"stage":  stageName,
 	})
 	return common.WithLogger(ctx, rtn)