Fix: Node set updating global output parameter updates global. #5699

workflow/controller/operator.go

@@ -2608,31 +2608,11 @@ func (woc *wfOperationCtx) addParamToGlobalScope(param wfv1.Parameter) {
 	if param.GlobalName == "" {
 		return
 	}
-	index := -1
-	if woc.wf.Status.Outputs != nil {
-		for i, gParam := range woc.wf.Status.Outputs.Parameters {
-			if gParam.Name == param.GlobalName {
-				index = i
-				break
-			}
-		}
-	} else {
-		woc.wf.Status.Outputs = &wfv1.Outputs{}
-	}
 	paramName := fmt.Sprintf("workflow.outputs.parameters.%s", param.GlobalName)
 	woc.globalParams[paramName] = param.Value.String()
-	if index == -1 {
-		woc.log.Infof("setting %s: '%s'", paramName, param.Value)
-		gParam := wfv1.Parameter{Name: param.GlobalName, Value: param.Value}
-		woc.wf.Status.Outputs.Parameters = append(woc.wf.Status.Outputs.Parameters, gParam)
+	wfUpdated := wfutil.AddParamToGlobalScope(woc.wf, woc.log, param)
+	if wfUpdated {
 		woc.updated = true
-	} else {
-		prevVal := *woc.wf.Status.Outputs.Parameters[index].Value
-		if prevVal != *param.Value {
-			woc.log.Infof("overwriting %s: '%s' -> '%s'", paramName, woc.wf.Status.Outputs.Parameters[index].Value, param.Value)
-			woc.wf.Status.Outputs.Parameters[index].Value = param.Value
-			woc.updated = true
-		}
 	}
 }
 

workflow/util/util.go

@@ -450,6 +450,39 @@ type SetOperationValues struct {
 	OutputParameters map[string]string
 }
 
+func AddParamToGlobalScope(wf *wfv1.Workflow, log *log.Entry, param wfv1.Parameter) bool {
+	wfUpdated := false
+	if param.GlobalName == "" {
+		return wfUpdated
+	}
+	index := -1
+	if wf.Status.Outputs != nil {
+		for i, gParam := range wf.Status.Outputs.Parameters {
+			if gParam.Name == param.GlobalName {
+				index = i
+				break
+			}
+		}
+	} else {
+		wf.Status.Outputs = &wfv1.Outputs{}
+	}
+	paramName := fmt.Sprintf("workflow.outputs.parameters.%s", param.GlobalName)
+	if index == -1 {
+		log.Infof("setting %s: '%s'", paramName, param.Value)
+		gParam := wfv1.Parameter{Name: param.GlobalName, Value: param.Value}
+		wf.Status.Outputs.Parameters = append(wf.Status.Outputs.Parameters, gParam)
+		wfUpdated = true
+	} else {
+		prevVal := *wf.Status.Outputs.Parameters[index].Value
+		if prevVal != *param.Value {
+			log.Infof("overwriting %s: '%s' -> '%s'", paramName, wf.Status.Outputs.Parameters[index].Value, param.Value)
+			wf.Status.Outputs.Parameters[index].Value = param.Value
+			wfUpdated = true
+		}
+	}
+	return wfUpdated
+}
+
 func updateSuspendedNode(ctx context.Context, wfIf v1alpha1.WorkflowInterface, hydrator hydrator.Interface, workflowName string, nodeFieldSelector string, values SetOperationValues) error {
 	selector, err := fields.ParseSelector(nodeFieldSelector)
 	if err != nil {
@@ -502,6 +535,7 @@ func updateSuspendedNode(ctx context.Context, wfIf v1alpha1.WorkflowInterface, h
 									node.Outputs.Parameters[i].ValueFrom = nil
 									nodeUpdated = true
 									hit = true
+									AddParamToGlobalScope(wf, log.NewEntry(log.StandardLogger()), node.Outputs.Parameters[i])
 									break
 								}
 							}

workflow/util/util_test.go

@@ -410,6 +410,7 @@ status:
           valueFrom:
             supplied: {}
         - name: message2
+          globalName: message-global-param
           valueFrom:
             supplied: {}
       phase: Running
@@ -438,6 +439,11 @@ func TestUpdateSuspendedNode(t *testing.T) {
 		assert.NoError(t, err)
 		err = updateSuspendedNode(ctx, wfIf, hydratorfake.Noop, "suspend-template", "name=suspend-template-kgfn7[0].approve", SetOperationValues{OutputParameters: map[string]string{"message2": "Hello World 2"}})
 		assert.NoError(t, err)
+
+		//make sure global variable was updated
+		wf, err := wfIf.Get(ctx, "suspend-template", metav1.GetOptions{})
+		assert.NoError(t, err)
+		assert.Equal(t, "Hello World 2", wf.Status.Outputs.Parameters[0].Value.String())
 	}
 
 	noSpaceWf := wfv1.MustUnmarshalWorkflow(susWorkflow)