fix: tolerate expression template runtime failures when allowUnresolved is true. Fixes #15832, #15824 (cherry-pick #15839 for 4.0) (#15850)

Signed-off-by: Alan Clucas <alan@clucas.org>
Co-authored-by: Alan Clucas <alan@clucas.org>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: 3 people

test/e2e/functional/dag-when-expr-skip.yaml

@@ -0,0 +1,35 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: dag-when-expr-skip-
+spec:
+  entrypoint: main
+  arguments:
+    parameters:
+      - name: data
+        value: ''
+  templates:
+    - name: main
+      dag:
+        tasks:
+          - name: A
+            template: echo
+            arguments:
+              parameters:
+                - name: message
+                  value: A
+          - name: B
+            dependencies: [A]
+            when: "{{= workflow.parameters.data != '' }}"
+            template: echo
+            arguments:
+              parameters:
+                - name: message
+                  value: "{{= jsonpath(workflow.parameters.data, '$.id') }}"
+    - name: echo
+      inputs:
+        parameters:
+          - name: message
+      container:
+        image: argoproj/argosay:v2
+        args: ["echo", "{{inputs.parameters.message}}"]

test/e2e/functional/steps-when-expr-filter.yaml

@@ -0,0 +1,39 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: steps-when-expr-filter-
+spec:
+  entrypoint: main
+  templates:
+    - name: main
+      inputs:
+        parameters:
+          - name: test
+            value: 'true'
+          - name: list
+            value: "{{= concat(['always'], inputs.parameters.test == 'true' ? ['test'] : []) | toJSON() }}"
+      steps:
+        - - name: fst
+            template: run
+            when: |
+              "{{= get(item, 'type') ?? 'always' }}"
+              in
+              ("{{= inputs.parameters.list | fromJSON() | join('","') }}","")
+            withParam: |
+              [
+                { "name": "first", "type": "" },
+                { "name": "second", "type": "always" },
+                { "name": "third", "type": "test" },
+                { "name": "fourth" }
+              ]
+            arguments:
+              parameters:
+                - name: name
+                  value: "{{ item.name }}{{ inputs.parameters.list }}"
+    - name: run
+      inputs:
+        parameters:
+          - name: name
+      container:
+        image: argoproj/argosay:v2
+        args: ["echo", "{{inputs.parameters.name}}"]

test/e2e/functional_test.go

@@ -511,6 +511,54 @@ func (s *FunctionalSuite) TestDAGSkippedOutputRef() {
 		})
 }
 
+func (s *FunctionalSuite) TestStepsWhenExprFilter() {
+	s.Given().
+		Workflow("@functional/steps-when-expr-filter.yaml").
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow().
+		Then().
+		ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
+			assert.Equal(t, wfv1.WorkflowSucceeded, status.Phase)
+			// "first" has type "" which matches the empty string in the list
+			node0 := status.Nodes.FindByDisplayName("fst(0:name:first,type:)")
+			if assert.NotNil(t, node0) {
+				assert.Equal(t, wfv1.NodeSucceeded, node0.Phase)
+			}
+			// "second" has type "always" which is in the list
+			node1 := status.Nodes.FindByDisplayName("fst(1:name:second,type:always)")
+			if assert.NotNil(t, node1) {
+				assert.Equal(t, wfv1.NodeSucceeded, node1.Phase)
+			}
+			// "third" has type "test" which is in the list (since test=true)
+			node2 := status.Nodes.FindByDisplayName("fst(2:name:third,type:test)")
+			if assert.NotNil(t, node2) {
+				assert.Equal(t, wfv1.NodeSucceeded, node2.Phase)
+			}
+			// "fourth" has no type, defaults to "always" via ??
+			node3 := status.Nodes.FindByDisplayName("fst(3:name:fourth)")
+			if assert.NotNil(t, node3) {
+				assert.Equal(t, wfv1.NodeSucceeded, node3.Phase)
+			}
+		})
+}
+
+func (s *FunctionalSuite) TestDAGWhenExprSkip() {
+	s.Given().
+		Workflow("@functional/dag-when-expr-skip.yaml").
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow().
+		Then().
+		ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
+			assert.Equal(t, wfv1.WorkflowSucceeded, status.Phase)
+			nodeB := status.Nodes.FindByDisplayName("B")
+			if assert.NotNil(t, nodeB) {
+				assert.Equal(t, wfv1.NodeSkipped, nodeB.Phase)
+			}
+		})
+}
+
 // 128M is for argo executor
 func (s *FunctionalSuite) TestPendingRetryWorkflow() {
 	s.Given().

util/template/expression_template.go

@@ -78,8 +78,10 @@ func expressionReplaceStrict(ctx context.Context, w io.Writer, expression string
 	}
 
 	// If we have missing identifiers but they are NOT strict, we allow unresolved.
-	// If we have NO missing identifiers, we enforce resolution (to catch runtime errors).
-	allowUnresolved := len(missingIdentifiers) > 0
+	// If we have NO missing identifiers, we enforce resolution (to catch runtime errors),
+	// unless the caller allows unresolved (strictRegex == nil), in which case runtime
+	// failures are tolerated and the expression is left unresolved for later evaluation.
+	allowUnresolved := len(missingIdentifiers) > 0 || strictRegex == nil
 	return expressionReplaceCore(ctx, w, expression, env, allowUnresolved)
 }
 

workflow/controller/dag_test.go

@@ -4120,3 +4120,71 @@ func TestDAGSkippedOutputRef(t *testing.T) {
 	require.NotNil(t, nodeC, "stage-c should be created even though stage-b was skipped")
 	assert.Equal(t, wfv1.NodePending, nodeC.Phase)
 }
+
+var dagWhenExprSkipEval = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: dag-when-expr-skip-eval-
+spec:
+  entrypoint: main
+  arguments:
+    parameters:
+      - name: data
+        value: ''
+  templates:
+  - name: main
+    dag:
+      tasks:
+      - name: A
+        template: echo
+        when: "false"
+        arguments:
+          parameters:
+          - name: message
+            value: A
+      - name: B
+        dependencies: [A]
+        when: "{{= workflow.parameters.data != '' }}"
+        template: echo
+        arguments:
+          parameters:
+          - name: message
+            value: "{{= jsonpath(workflow.parameters.data, '$.id') }}"
+  - name: echo
+    inputs:
+      parameters:
+      - name: message
+    container:
+      image: alpine:3.23
+      command: [echo, "{{inputs.parameters.message}}"]
+`
+
+// TestDAGWhenExprSkipEval verifies that expression templates in a DAG task's arguments are not
+// evaluated when the task's "when" clause evaluates to false.
+// Scenario: workflow parameter "data" is empty. Task B has when: "{{= workflow.parameters.data != ” }}"
+// which evaluates to false. B's argument uses jsonpath(workflow.parameters.data, '$.id') which would
+// fail on an empty string. The expression should not be evaluated since the task will be skipped.
+// Currently fails because SubstituteParams evaluates all expression templates in the entire DAG
+// template before individual task "when" conditions are checked.
+func TestDAGWhenExprSkipEval(t *testing.T) {
+	ctx := logging.TestContext(t.Context())
+	cancel, controller := newController(ctx)
+	defer cancel()
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+
+	wf := wfv1.MustUnmarshalWorkflow(dagWhenExprSkipEval)
+	wf, err := wfcset.Create(ctx, wf, metav1.CreateOptions{})
+	require.NoError(t, err)
+	woc := newWorkflowOperationCtx(ctx, wf, controller)
+
+	woc.operate(ctx)
+
+	// Workflow should succeed: B's when clause evaluates to false so B should be skipped
+	// without evaluating B's argument expressions.
+	assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase)
+
+	nodeB := woc.wf.Status.Nodes.FindByDisplayName("B")
+	require.NotNil(t, nodeB)
+	assert.Equal(t, wfv1.NodeSkipped, nodeB.Phase)
+}

workflow/controller/steps_test.go

@@ -719,3 +719,90 @@ func TestStepsWhenSkipNoRequeue(t *testing.T) {
 	require.NotNil(t, nodeB)
 	assert.Equal(t, wfv1.NodeSkipped, nodeB.Phase)
 }
+
+var stepsWhenExprWithParamFilter = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: steps-when-expr-filter-
+spec:
+  entrypoint: main
+  templates:
+    - name: main
+      inputs:
+        parameters:
+          - name: test
+            value: 'true'
+          - name: list
+            value: "{{= concat(['always'], inputs.parameters.test == 'true' ? ['test'] : []) | toJSON() }}"
+      steps:
+        - - name: fst
+            template: run
+            when: |
+              "{{= get(item, 'type') ?? 'always' }}"
+              in
+              ("{{= inputs.parameters.list | fromJSON() | join('","') }}","")
+            withParam: |
+              [
+                { "name": "first", "type": "" },
+                { "name": "second", "type": "always" },
+                { "name": "third", "type": "test" },
+                { "name": "fourth" }
+              ]
+            arguments:
+              parameters:
+                - name: name
+                  value: "{{ item.name }}{{ inputs.parameters.list }}"
+    - name: run
+      inputs:
+        parameters:
+          - name: name
+      container:
+        image: alpine:3.23
+        command: [echo]
+        args: ["{{inputs.parameters.name}}"]
+`
+
+// TestStepsWhenExprWithParamFilter verifies that expression templates work correctly
+// in a steps workflow with withParam expansion and a when clause that filters items
+// using expression functions (concat, get, ??, toJSON, fromJSON, join).
+// This mirrors a real-world pattern where a dynamic list parameter controls which
+// withParam items execute.
+func TestStepsWhenExprWithParamFilter(t *testing.T) {
+	ctx := logging.TestContext(t.Context())
+	cancel, controller := newController(ctx)
+	defer cancel()
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+
+	wf := wfv1.MustUnmarshalWorkflow(stepsWhenExprWithParamFilter)
+	wf, err := wfcset.Create(ctx, wf, metav1.CreateOptions{})
+	require.NoError(t, err)
+	woc := newWorkflowOperationCtx(ctx, wf, controller)
+
+	woc.operate(ctx)
+
+	// Workflow is Running because pods haven't completed, but we can verify:
+	// 1. No error occurred during expression evaluation
+	// 2. All 4 items were expanded and scheduled (none were incorrectly skipped/errored)
+	assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
+
+	// "first" has type "" which matches empty string in the filter list
+	node0 := woc.wf.Status.Nodes.FindByDisplayName("fst(0:name:first,type:)")
+	require.NotNil(t, node0)
+	assert.Equal(t, wfv1.NodePending, node0.Phase)
+
+	// "second" has type "always" which is in the filter list
+	node1 := woc.wf.Status.Nodes.FindByDisplayName("fst(1:name:second,type:always)")
+	require.NotNil(t, node1)
+	assert.Equal(t, wfv1.NodePending, node1.Phase)
+
+	// "third" has type "test" which is in the filter list (test=true)
+	node2 := woc.wf.Status.Nodes.FindByDisplayName("fst(2:name:third,type:test)")
+	require.NotNil(t, node2)
+	assert.Equal(t, wfv1.NodePending, node2.Phase)
+
+	// "fourth" has no type, defaults to "always" via ?? operator
+	node3 := woc.wf.Status.Nodes.FindByDisplayName("fst(3:name:fourth)")
+	require.NotNil(t, node3)
+	assert.Equal(t, wfv1.NodePending, node3.Phase)
+}