From 412ff1c41196cb602aa7bb98a39e8ec90c08ada5 Mon Sep 17 00:00:00 2001
From: Tianchu Zhao <evantczhao@gmail.com>
Date: Wed, 23 Feb 2022 02:42:19 +1100
Subject: [PATCH] feat(controller): skip resolve artifact when when evaluates
 to fals one on withsequence (#7950)

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
---
 workflow/controller/dag.go           |  2 +-
 workflow/controller/dag_test.go      | 10 ++++++++++
 workflow/controller/operator.go      | 13 +++++++++++--
 workflow/controller/operator_test.go |  2 +-
 workflow/controller/steps.go         |  2 +-
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/workflow/controller/dag.go b/workflow/controller/dag.go
index 00fc56a13e7a..4df118f33b2b 100644
--- a/workflow/controller/dag.go
+++ b/workflow/controller/dag.go
@@ -708,7 +708,7 @@ func expandTask(task wfv1.DAGTask) ([]wfv1.DAGTask, error) {
 	expandedTasks := make([]wfv1.DAGTask, 0)
 	for i, item := range items {
 		var newTask wfv1.DAGTask
-		newTaskName, err := processItem(tmpl, task.Name, i, item, &newTask)
+		newTaskName, err := processItem(tmpl, task.Name, i, item, &newTask, task.When)
 		if err != nil {
 			return nil, err
 		}
diff --git a/workflow/controller/dag_test.go b/workflow/controller/dag_test.go
index 6e96d3435dda..7723ef5a2127 100644
--- a/workflow/controller/dag_test.go
+++ b/workflow/controller/dag_test.go
@@ -155,6 +155,16 @@ spec:
           artifacts:
           - name: message
             from: "{{tasks.generate-artifact.outputs.artifacts.hello-art}}"
+      - name: sequence-param
+        template: print-message
+        dependencies: [generate-artifact]
+        when: "false"
+        arguments:
+          artifacts:
+          - name: message
+            from: "{{tasks.generate-artifact.outputs.artifacts.hello-art}}"
+        withSequence:
+          count: "5"
 
   - name: whalesay
     container:
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 00b5f0378b95..a5bb2112297e 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -2994,7 +2994,7 @@ func parseStringToDuration(durationString string) (time.Duration, error) {
 	return suspendDuration, nil
 }
 
-func processItem(tmpl template.Template, name string, index int, item wfv1.Item, obj interface{}) (string, error) {
+func processItem(tmpl template.Template, name string, index int, item wfv1.Item, obj interface{}, whenCondition string) (string, error) {
 	replaceMap := make(map[string]string)
 	var newName string
 
@@ -3035,7 +3035,16 @@ func processItem(tmpl template.Template, name string, index int, item wfv1.Item,
 	default:
 		return "", errors.Errorf(errors.CodeBadRequest, "withItems[%d] expected string, number, list, or map. received: %v", index, item)
 	}
-	newStepStr, err := tmpl.Replace(replaceMap, false)
+	var newStepStr string
+	// If when is not parameterised and evaluated to false, we are not executing nor resolving artifact,
+	// we allow parameter substitution to be Unresolved
+	// The parameterised when will get handle by the task-expansion
+	proceed, err := shouldExecute(whenCondition)
+	if err == nil && !proceed {
+		newStepStr, err = tmpl.Replace(replaceMap, true)
+	} else {
+		newStepStr, err = tmpl.Replace(replaceMap, false)
+	}
 	if err != nil {
 		return "", err
 	}
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index 7d2602973073..85d7b40c205a 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -5623,7 +5623,7 @@ func Test_processItem(t *testing.T) {
 
 	var newTask wfv1.DAGTask
 	tmpl, _ := template.NewTemplate(string(taskBytes))
-	newTaskName, err := processItem(tmpl, "task-name", 0, items[0], &newTask)
+	newTaskName, err := processItem(tmpl, "task-name", 0, items[0], &newTask, "")
 	if assert.NoError(t, err) {
 		assert.Equal(t, `task-name(0:json:{"list":[0,"1"],"number":2,"string":"foo"},list:[0,"1"],number:2,string:foo)`, newTaskName)
 	}
diff --git a/workflow/controller/steps.go b/workflow/controller/steps.go
index 2acfe5434816..aa477140f443 100644
--- a/workflow/controller/steps.go
+++ b/workflow/controller/steps.go
@@ -490,7 +490,7 @@ func (woc *wfOperationCtx) expandStep(step wfv1.WorkflowStep) ([]wfv1.WorkflowSt
 
 	for i, item := range items {
 		var newStep wfv1.WorkflowStep
-		newStepName, err := processItem(t, step.Name, i, item, &newStep)
+		newStepName, err := processItem(t, step.Name, i, item, &newStep, step.When)
 		if err != nil {
 			return nil, err
 		}