fix: ordering of functionality for setting and evaluating label expressions

workflow/controller/operator.go

@@ -482,22 +482,30 @@ func (woc *wfOperationCtx) operate(ctx context.Context) {
 	}
 }
 
+// set Labels and Annotations for the Workflow
+// Also, since we're setting Labels and Annotations we need to find any
+// parameters formatted as "workflow.labels.<param>" or "workflow.annotations.<param>"
+// and perform substitution
 func (woc *wfOperationCtx) updateWorkflowMetadata() error {
+	updatedParams := make(common.Parameters)
 	if md := woc.execWf.Spec.WorkflowMetadata; md != nil {
-		if woc.wf.ObjectMeta.Labels == nil {
-			woc.wf.ObjectMeta.Labels = make(map[string]string)
+		if woc.wf.Labels == nil {
+			woc.wf.Labels = make(map[string]string)
 		}
 		for n, v := range md.Labels {
 			woc.wf.Labels[n] = v
 			woc.globalParams["workflow.labels."+n] = v
+			updatedParams["workflow.labels."+n] = v
 		}
-		if woc.wf.ObjectMeta.Annotations == nil {
-			woc.wf.ObjectMeta.Annotations = make(map[string]string)
+		if woc.wf.Annotations == nil {
+			woc.wf.Annotations = make(map[string]string)
 		}
 		for n, v := range md.Annotations {
 			woc.wf.Annotations[n] = v
 			woc.globalParams["workflow.annotations."+n] = v
+			updatedParams["workflow.annotations."+n] = v
 		}
+
 		env := env.GetFuncMap(template.EnvMap(woc.globalParams))
 		for n, f := range md.LabelsFrom {
 			r, err := expr.Eval(f.Expression, env)
@@ -510,8 +518,16 @@ func (woc *wfOperationCtx) updateWorkflowMetadata() error {
 			}
 			woc.wf.Labels[n] = v
 			woc.globalParams["workflow.labels."+n] = v
+			updatedParams["workflow.labels."+n] = v
 		}
 		woc.updated = true
+
+		// Now we need to do any substitution that involves these labels
+		err := woc.substituteGlobalVariables(updatedParams)
+		if err != nil {
+			return err
+		}
+
 	}
 	return nil
 }
@@ -570,6 +586,23 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument
 			woc.globalParams["workflow.parameters."+param.Name] = param.Value.String()
 		}
 	}
+	if woc.wf.Status.Outputs != nil {
+		for _, param := range woc.wf.Status.Outputs.Parameters {
+			if param.HasValue() {
+				woc.globalParams["workflow.outputs.parameters."+param.Name] = param.GetValue()
+			}
+		}
+	}
+
+	/////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Set global parameters based on Labels and Annotations, both those that are defined in the execWf.ObjectMeta
+	// and those that are defined in the execWf.Spec.WorkflowMetadata
+	// Note: we no longer set globalParams based on LabelsFrom expressions here since they may themselves use parameters
+	// and thus will need to be evaluated later based on the evaluation of those parameters
+	/////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+	md := woc.execWf.Spec.WorkflowMetadata
+
 	if workflowAnnotations, err := json.Marshal(woc.wf.ObjectMeta.Annotations); err == nil {
 		woc.globalParams[common.GlobalVarWorkflowAnnotations] = string(workflowAnnotations)
 	}
@@ -580,15 +613,30 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument
 		woc.globalParams[common.GlobalVarWorkflowLabels] = string(workflowLabels)
 	}
 	for k, v := range woc.wf.ObjectMeta.Labels {
-		woc.globalParams["workflow.labels."+k] = v
+		// if the Label will get overridden by a LabelsFrom expression later, don't set it now
+		if md != nil {
+			_, existsLabelsFrom := md.LabelsFrom[k]
+			if !existsLabelsFrom {
+				woc.globalParams["workflow.labels."+k] = v
+			}
+		} else {
+			woc.globalParams["workflow.labels."+k] = v
+		}
 	}
-	if woc.wf.Status.Outputs != nil {
-		for _, param := range woc.wf.Status.Outputs.Parameters {
-			if param.HasValue() {
-				woc.globalParams["workflow.outputs.parameters."+param.Name] = param.GetValue()
+
+	if md != nil {
+		for n, v := range md.Labels {
+			// if the Label will get overridden by a LabelsFrom expression later, don't set it now
+			_, existsLabelsFrom := md.LabelsFrom[n]
+			if !existsLabelsFrom {
+				woc.globalParams["workflow.labels."+n] = v
 			}
 		}
+		for n, v := range md.Annotations {
+			woc.globalParams["workflow.annotations."+n] = v
+		}
 	}
+
 	return nil
 }
 
@@ -3532,16 +3580,17 @@ func (woc *wfOperationCtx) setExecWorkflow(ctx context.Context) error {
 		woc.markWorkflowFailed(ctx, fmt.Sprintf("failed to set global parameters: %s", err.Error()))
 		return err
 	}
+
+	err = woc.substituteGlobalVariables(woc.globalParams)
+	if err != nil {
+		return err
+	}
 	if woc.wf.Status.Phase == wfv1.WorkflowUnknown {
 		if err := woc.updateWorkflowMetadata(); err != nil {
 			woc.markWorkflowError(ctx, err)
 			return err
 		}
 	}
-	err = woc.substituteGlobalVariables()
-	if err != nil {
-		return err
-	}
 
 	// runtime value will be set after the substitution, otherwise will not be reflected from stored wf spec
 	woc.setGlobalRuntimeParameters()
@@ -3647,7 +3696,7 @@ func (woc *wfOperationCtx) mergedTemplateDefaultsInto(originalTmpl *wfv1.Templat
 	return nil
 }
 
-func (woc *wfOperationCtx) substituteGlobalVariables() error {
+func (woc *wfOperationCtx) substituteGlobalVariables(params common.Parameters) error {
 	execWfSpec := woc.execWf.Spec
 
 	// To Avoid the stale Global parameter value substitution to templates.
@@ -3659,7 +3708,7 @@ func (woc *wfOperationCtx) substituteGlobalVariables() error {
 		return err
 	}
 
-	resolveSpec, err := template.Replace(string(wfSpec), woc.globalParams, true)
+	resolveSpec, err := template.Replace(string(wfSpec), params, true)
 	if err != nil {
 		return err
 	}

workflow/controller/operator_test.go

@@ -3780,7 +3780,7 @@ func TestNestedOptionalOutputArtifacts(t *testing.T) {
 	assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase)
 }
 
-//  TestPodSpecLogForFailedPods tests PodSpec logging configuration
+// TestPodSpecLogForFailedPods tests PodSpec logging configuration
 func TestPodSpecLogForFailedPods(t *testing.T) {
 	wf := wfv1.MustUnmarshalWorkflow(helloWorldWf)
 	cancel, controller := newController(wf)
@@ -3798,7 +3798,7 @@ func TestPodSpecLogForFailedPods(t *testing.T) {
 	}
 }
 
-//  TestPodSpecLogForAllPods tests  PodSpec logging configuration
+// TestPodSpecLogForAllPods tests  PodSpec logging configuration
 func TestPodSpecLogForAllPods(t *testing.T) {
 	cancel, controller := newController()
 	defer cancel()
@@ -7067,8 +7067,8 @@ func TestSubstituteGlobalVariablesLabelsAnnotations(t *testing.T) {
 			// entire template referenced; value not contained in WorkflowTemplate or Workflow
 			workflow:              "@testdata/workflow-sub-test-1.yaml",
 			workflowTemplate:      "@testdata/workflow-template-sub-test-1.yaml",
-			expectedMutexName:     "{{workflow.labels.mutex-name}}",
-			expectedSchedulerName: "{{workflow.annotations.scheduler-name}}",
+			expectedMutexName:     "{{workflow.labels.mutexName}}",
+			expectedSchedulerName: "{{workflow.annotations.schedulerName}}",
 		},
 		{
 			// entire template referenced; value is in Workflow.Labels
@@ -7106,6 +7106,14 @@ func TestSubstituteGlobalVariablesLabelsAnnotations(t *testing.T) {
 			expectedMutexName:     "myMutex",
 			expectedSchedulerName: "myScheduler",
 		},
+		{
+			// this checks that we can use a sprig expression to set a label (using workflowMetadata.labelsFrom)
+			// and the result of that label can also be evaluated in the spec
+			workflow:              "@testdata/workflow-sub-test-6.yaml",
+			workflowTemplate:      "@testdata/workflow-template-sub-test-2.yaml",
+			expectedMutexName:     "wfMetadataScheduler",
+			expectedSchedulerName: "wfMetadataScheduler",
+		},
 	}
 
 	for _, tt := range tests {

workflow/controller/testdata/workflow-sub-test-1.yaml

@@ -8,5 +8,5 @@ spec:
     name: workflow-template-submittable
   synchronization:
     mutex:
-      name:  "{{workflow.labels.mutex-name}}"
-  schedulerName:  "{{workflow.annotations.scheduler-name}}"
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"

workflow/controller/testdata/workflow-sub-test-2.yaml

@@ -4,13 +4,13 @@ metadata:
   generateName: workflow-template-hello-world-
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   workflowTemplateRef:
     name: workflow-template-submittable
   synchronization:
     mutex:
-      name:  "{{workflow.labels.mutex-name}}"
-  schedulerName:  "{{workflow.annotations.scheduler-name}}"
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"

workflow/controller/testdata/workflow-sub-test-3.yaml

@@ -4,18 +4,18 @@ metadata:
   generateName: workflow-template-hello-world-
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   workflowTemplateRef:
     name: workflow-template-submittable
   synchronization:
     mutex:
-      name:  "{{workflow.labels.mutex-name}}"
-  schedulerName:  "{{workflow.annotations.scheduler-name}}"
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"
   workflowMetadata:
     labels:
-      mutex-name: wfMetadataMutex
+      mutexName: wfMetadataMutex
     annotations:
-      scheduler-name: wfMetadataScheduler
+      schedulerName: wfMetadataScheduler

workflow/controller/testdata/workflow-sub-test-4.yaml

@@ -2,18 +2,18 @@ apiVersion: argoproj.io/v1alpha1
 kind: Workflow
 metadata:
   generateName: workflow-template-hello-world-
-  namespace: test
+  namespace: argo
   labels:
-    mutex-name: myMutex
+    mutexName: myOverrideMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   workflowTemplateRef:
     name: workflow-template-submittable
   synchronization:
     mutex:
-      name:  "{{workflow.labels.mutex-name}}"
-  schedulerName:  "{{workflow.annotations.scheduler-name}}"
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"
   workflowMetadata:
     annotations:
-      scheduler-name: wfMetadataScheduler
+      schedulerName: wfMetadataScheduler

workflow/controller/testdata/workflow-sub-test-5.yaml

@@ -4,9 +4,9 @@ metadata:
   generateName: workflow-template-hello-world-
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   entrypoint: myTemplate
   templates:
@@ -18,5 +18,5 @@ spec:
               template: whalesay-template
   synchronization:
     mutex:
-      name:  "{{workflow.labels.mutex-name}}"
-  schedulerName:  "{{workflow.annotations.scheduler-name}}"
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"

workflow/controller/testdata/workflow-sub-test-6.yaml

@@ -0,0 +1,22 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: workflow-template-hello-world-
+  namespace: test
+  labels:
+    mutexName: myMutex
+  annotations:
+    schedulerName: myScheduler
+spec:
+  workflowTemplateRef:
+    name: workflow-template-submittable
+  synchronization:
+    mutex:
+      name:  "{{workflow.labels.mutexName}}"
+  schedulerName:  "{{workflow.annotations.schedulerName}}"
+  workflowMetadata:
+    labelsFrom:
+      mutexName: 
+        expression: "{{= sprig.quote(sprig.trim( workflow.annotations.schedulerName )) }}"
+    annotations:
+      schedulerName: wfMetadataScheduler

workflow/controller/testdata/workflow-template-sub-test-1.yaml

@@ -4,9 +4,9 @@ metadata:
   name: workflow-template-submittable
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   entrypoint: whalesay-template
   templates:

workflow/controller/testdata/workflow-template-sub-test-2.yaml

@@ -4,9 +4,9 @@ metadata:
   name: workflow-template-submittable
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   entrypoint: whalesay-template
   templates:
@@ -17,6 +17,6 @@ spec:
         args: ['hello']
   workflowMetadata:
     labels:
-      mutex-name: wfMetadataTemplateMutex
+      mutexName: wfMetadataTemplateMutex
     annotations:
-      scheduler-name: wfMetadataTemplateScheduler
+      schedulerName: wfMetadataTemplateScheduler

workflow/controller/testdata/workflow-template-sub-test-3.yaml

@@ -2,11 +2,11 @@ apiVersion: argoproj.io/v1alpha1
 kind: WorkflowTemplate
 metadata:
   name: workflow-template-submittable
-  namespace: test
+  namespace: argo
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   arguments:
     parameters:
@@ -21,7 +21,7 @@ spec:
         args: ['hello']
   workflowMetadata:
     labelsFrom:
-      mutex-name:
+      mutexName:
         expression: workflow.parameters.mutex
     annotations:
-      scheduler-name: wfMetadataTemplateScheduler
+      schedulerName: wfMetadataTemplateScheduler

workflow/controller/testdata/workflow-template-sub-test-4.yaml

@@ -4,9 +4,9 @@ metadata:
   name: workflow-template-submittable
   namespace: test
   labels:
-    mutex-name: myMutex
+    mutexName: myMutex
   annotations:
-    scheduler-name: myScheduler
+    schedulerName: myScheduler
 spec:
   templates:
     - name: whalesay-template
@@ -16,6 +16,6 @@ spec:
         args: ['hello']
   workflowMetadata:
     labels:
-      mutex-name: wfMetadataTemplateMutex
+      mutexName: wfMetadataTemplateMutex
     annotations:
-      scheduler-name: wfMetadataTemplateScheduler
+      schedulerName: wfMetadataTemplateScheduler