Refactor evaluate yaml node do not alter nested nodes (#1761)

* refactor: EvaluateYamlNode do not alter nested nodes

* fix build error

* fix op

* fix lint

* ...

* fixup

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

pkg/runner/expression.go

@@ -158,67 +158,117 @@ func (ee expressionEvaluator) evaluate(ctx context.Context, in string, defaultSt
 	return evaluated, err
 }
 
-func (ee expressionEvaluator) evaluateScalarYamlNode(ctx context.Context, node *yaml.Node) error {
+func (ee expressionEvaluator) evaluateScalarYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
 	var in string
 	if err := node.Decode(&in); err != nil {
-		return err
+		return nil, err
 	}
 	if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
-		return nil
+		return nil, nil
 	}
 	expr, _ := rewriteSubExpression(ctx, in, false)
 	res, err := ee.evaluate(ctx, expr, exprparser.DefaultStatusCheckNone)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	return node.Encode(res)
+	ret := &yaml.Node{}
+	if err := ret.Encode(res); err != nil {
+		return nil, err
+	}
+	return ret, err
 }
 
-func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node *yaml.Node) error {
+func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
+	var ret *yaml.Node = nil
 	// GitHub has this undocumented feature to merge maps, called insert directive
 	insertDirective := regexp.MustCompile(`\${{\s*insert\s*}}`)
-	for i := 0; i < len(node.Content)/2; {
+	for i := 0; i < len(node.Content)/2; i++ {
+		changed := func() error {
+			if ret == nil {
+				ret = &yaml.Node{}
+				if err := ret.Encode(node); err != nil {
+					return err
+				}
+				ret.Content = ret.Content[:i*2]
+			}
+			return nil
+		}
 		k := node.Content[i*2]
 		v := node.Content[i*2+1]
-		if err := ee.EvaluateYamlNode(ctx, v); err != nil {
+		ev, err := ee.evaluateYamlNodeInternal(ctx, v)
-			return err
+		if err != nil {
+			return nil, err
+		}
+		if ev != nil {
+			if err := changed(); err != nil {
+				return nil, err
+			}
+		} else {
+			ev = v
 		}
 		var sk string
 		// Merge the nested map of the insert directive
 		if k.Decode(&sk) == nil && insertDirective.MatchString(sk) {
-			node.Content = append(append(node.Content[:i*2], v.Content...), node.Content[(i+1)*2:]...)
+			if ev.Kind != yaml.MappingNode {
-			i += len(v.Content) / 2
+				return nil, fmt.Errorf("failed to insert node %v into mapping %v unexpected type %v expected MappingNode", ev, node, ev.Kind)
-		} else {
+			}
-			if err := ee.EvaluateYamlNode(ctx, k); err != nil {
+			if err := changed(); err != nil {
-				return err
+				return nil, err
+			}
+			ret.Content = append(ret.Content, ev.Content...)
+		} else {
+			ek, err := ee.evaluateYamlNodeInternal(ctx, k)
+			if err != nil {
+				return nil, err
+			}
+			if ek != nil {
+				if err := changed(); err != nil {
+					return nil, err
+				}
+			} else {
+				ek = k
+			}
+			if ret != nil {
+				ret.Content = append(ret.Content, ek, ev)
 			}
-			i++
 		}
 	}
-	return nil
+	return ret, nil
 }
 
-func (ee expressionEvaluator) evaluateSequenceYamlNode(ctx context.Context, node *yaml.Node) error {
+func (ee expressionEvaluator) evaluateSequenceYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
-	for i := 0; i < len(node.Content); {
+	var ret *yaml.Node = nil
+	for i := 0; i < len(node.Content); i++ {
 		v := node.Content[i]
 		// Preserve nested sequences
 		wasseq := v.Kind == yaml.SequenceNode
-		if err := ee.EvaluateYamlNode(ctx, v); err != nil {
+		ev, err := ee.evaluateYamlNodeInternal(ctx, v)
-			return err
+		if err != nil {
+			return nil, err
 		}
-		// GitHub has this undocumented feature to merge sequences / arrays
+		if ev != nil {
-		// We have a nested sequence via evaluation, merge the arrays
+			if ret == nil {
-		if v.Kind == yaml.SequenceNode && !wasseq {
+				ret = &yaml.Node{}
-			node.Content = append(append(node.Content[:i], v.Content...), node.Content[i+1:]...)
+				if err := ret.Encode(node); err != nil {
-			i += len(v.Content)
+					return nil, err
-		} else {
+				}
-			i++
+				ret.Content = ret.Content[:i]
+			}
+			// GitHub has this undocumented feature to merge sequences / arrays
+			// We have a nested sequence via evaluation, merge the arrays
+			if ev.Kind == yaml.SequenceNode && !wasseq {
+				ret.Content = append(ret.Content, ev.Content...)
+			} else {
+				ret.Content = append(ret.Content, ev)
+			}
+		} else if ret != nil {
+			ret.Content = append(ret.Content, v)
 		}
 	}
-	return nil
+	return ret, nil
 }
 
-func (ee expressionEvaluator) EvaluateYamlNode(ctx context.Context, node *yaml.Node) error {
+func (ee expressionEvaluator) evaluateYamlNodeInternal(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
 	switch node.Kind {
 	case yaml.ScalarNode:
 		return ee.evaluateScalarYamlNode(ctx, node)
@@ -227,10 +277,21 @@ func (ee expressionEvaluator) EvaluateYamlNode(ctx context.Context, node *yaml.N
 	case yaml.SequenceNode:
 		return ee.evaluateSequenceYamlNode(ctx, node)
 	default:
-		return nil
+		return nil, nil
 	}
 }
 
+func (ee expressionEvaluator) EvaluateYamlNode(ctx context.Context, node *yaml.Node) error {
+	ret, err := ee.evaluateYamlNodeInternal(ctx, node)
+	if err != nil {
+		return err
+	}
+	if ret != nil {
+		return ret.Decode(node)
+	}
+	return nil
+}
+
 func (ee expressionEvaluator) Interpolate(ctx context.Context, in string) string {
 	if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
 		return in