From 9c2b12d170663672aa7ca4dcc6732159d1a02ad9 Mon Sep 17 00:00:00 2001
From: Miltiadis Alexis <18004241+miltalex@users.noreply.github.com>
Date: Thu, 15 Aug 2024 13:19:34 +0200
Subject: [PATCH] fix: avoid exit handler nil pointer when missing outputs.
 Fixes #13445 (#13448)

Signed-off-by: Miltiadis Alexis <alexmiltiadis@gmail.com>
---
 .../exit-handler-fail-missing-output.yaml     | 39 +++++++++++++++++
 test/e2e/functional_test.go                   | 42 ++++++++++++++-----
 workflow/controller/exit_handler.go           |  6 ++-
 3 files changed, 76 insertions(+), 11 deletions(-)
 create mode 100644 test/e2e/expectedfailures/exit-handler-fail-missing-output.yaml

diff --git a/test/e2e/expectedfailures/exit-handler-fail-missing-output.yaml b/test/e2e/expectedfailures/exit-handler-fail-missing-output.yaml
new file mode 100644
index 000000000000..c62fe4beb9b5
--- /dev/null
+++ b/test/e2e/expectedfailures/exit-handler-fail-missing-output.yaml
@@ -0,0 +1,39 @@
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: exit-handler-fail-missing-output-
+  labels:
+    test: test
+spec:
+  entrypoint: failure-workflow
+  templates:
+    - name: failure-workflow
+      steps:
+        - - name: step1
+            template: intentional-fail
+            hooks:
+              exit:
+                template: lifecycle-hook
+                arguments:
+                  parameters:
+                    - name: hello-param
+                      value: '{{steps.step1.outputs.parameters.hello-param}}'
+    - name: intentional-fail
+      outputs:
+        parameters:
+          - name: hello-param
+            valueFrom:
+              path: /tmp/hello_world.txt
+      container:
+        image: alpine:latest
+        command: ["sh", "-c"]
+        args: ["echo intentional failure; exit 1"]
+    - name: lifecycle-hook
+      inputs:
+        parameters:
+          - name: hello-param
+      container:
+        image: busybox
+        command: [echo]
+        args: ["Hello param: {{inputs.parameters.hello-param}}"]
diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go
index 30e872f1b707..f96a38717d4f 100644
--- a/test/e2e/functional_test.go
+++ b/test/e2e/functional_test.go
@@ -109,7 +109,6 @@ func (s *FunctionalSuite) TestWhenExpressions() {
 }
 
 func (s *FunctionalSuite) TestJSONVariables() {
-
 	s.Given().
 		Workflow("@testdata/json-variables.yaml").
 		When().
@@ -1053,9 +1052,7 @@ spec:
 				}
 			}
 			assert.True(t, memoHit)
-
 		})
-
 }
 
 func (s *FunctionalSuite) TestStepLevelMemoizeNoOutput() {
@@ -1113,9 +1110,7 @@ spec:
 				}
 			}
 			assert.True(t, memoHit)
-
 		})
-
 }
 
 func (s *FunctionalSuite) TestDAGLevelMemoize() {
@@ -1179,9 +1174,7 @@ spec:
 				}
 			}
 			assert.True(t, memoHit)
-
 		})
-
 }
 
 func (s *FunctionalSuite) TestDAGLevelMemoizeNoOutput() {
@@ -1240,9 +1233,7 @@ spec:
 				}
 			}
 			assert.True(t, memoHit)
-
 		})
-
 }
 
 func (s *FunctionalSuite) TestContainerSetRetryFail() {
@@ -1323,7 +1314,6 @@ func (s *FunctionalSuite) TestEntrypointName() {
 			assert.Equal(t, wfv1.NodeSucceeded, n.Phase)
 			assert.Equal(t, "bar", n.Inputs.Parameters[0].Value.String())
 		})
-
 }
 
 func (s *FunctionalSuite) TestMissingStepsInUI() {
@@ -1372,3 +1362,35 @@ func (s *FunctionalSuite) TestTerminateWorkflowWhileOnExitHandlerRunning() {
 			assert.Equal(t, status.Phase, wfv1.WorkflowFailed)
 		})
 }
+
+// Exit handler ensure when failed steps ensure no crash and output parameter
+func (s *FunctionalSuite) TestWorkflowExitHandlerCrashEnsureNodeIsPresent() {
+	s.Given().
+		Workflow("@expectedfailures/exit-handler-fail-missing-output.yaml").
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow(fixtures.ToBeRunning).
+		WaitForWorkflow(fixtures.ToBeFailed).
+		Then().
+		ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *wfv1.WorkflowStatus) {
+			var hasExitNode bool
+			var exitNodeName string
+
+			for _, node := range status.Nodes {
+				if !node.IsExitNode() {
+					continue
+				}
+				hasExitNode = true
+				exitNodeName = node.DisplayName
+			}
+			assert.True(t, hasExitNode)
+			assert.NotEmpty(t, exitNodeName)
+
+			hookNode := status.Nodes.FindByDisplayName(exitNodeName)
+
+			require.NotNil(t, hookNode)
+			assert.NotNil(t, hookNode.Inputs)
+			require.Len(t, hookNode.Inputs.Parameters, 1)
+			assert.NotNil(t, hookNode.Inputs.Parameters[0].Value)
+		})
+}
diff --git a/workflow/controller/exit_handler.go b/workflow/controller/exit_handler.go
index a5b20d02506a..cd4725cb355d 100644
--- a/workflow/controller/exit_handler.go
+++ b/workflow/controller/exit_handler.go
@@ -57,7 +57,11 @@ func (woc *wfOperationCtx) resolveExitTmplArgument(args wfv1.Arguments, prefix s
 	}
 	if outputs != nil {
 		for _, param := range outputs.Parameters {
-			scope.addParamToScope(fmt.Sprintf("%s.outputs.parameters.%s", prefix, param.Name), param.Value.String())
+			value := ""
+			if param.Value != nil {
+				value = param.Value.String()
+			}
+			scope.addParamToScope(fmt.Sprintf("%s.outputs.parameters.%s", prefix, param.Name), value)
 		}
 		for _, arts := range outputs.Artifacts {
 			scope.addArtifactToScope(fmt.Sprintf("%s.outputs.artifacts.%s", prefix, arts.Name), arts)