fix: Ensure target Workflow hooks not nil

Signed-off-by: toyamagu-2021 <[email protected]>

workflow/util/merge.go

@@ -39,6 +39,9 @@ func MergeTo(patch, target *wfv1.Workflow) error {
 		return err
 	}
 
+	if len(patchHooks) != 0 && target.Spec.Hooks == nil {
+		target.Spec.Hooks = make(map[wfv1.LifecycleEvent]wfv1.LifecycleHook)
+	}
 	for name, hook := range patchHooks {
 		// If the patch hook doesn't exist in target
 		if _, ok := target.Spec.Hooks[name]; !ok {

workflow/util/merge_test.go

@@ -272,6 +272,14 @@ func TestJoinWorkflowMetaData(t *testing.T) {
 	})
 }
 
+var baseNilHookWF = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: workflow-template-hello-world-
+spec:
+`
+
 var baseHookWF = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -284,6 +292,12 @@ spec:
       expression: workflow.status == "Pending"
 `
 
+var patchNilHookWF = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+spec:
+`
+
 var patchHookWF = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -297,14 +311,36 @@ spec:
       expression: workflow.status == "Pending"
 `
 
-// Ensure hook bar ends up in result, but foo is unchanged
 func TestMergeHooks(t *testing.T) {
-	patchHookWf := wfv1.MustUnmarshalWorkflow(patchHookWF)
-	targetHookWf := wfv1.MustUnmarshalWorkflow(baseHookWF)
+	t.Run("NilBase", func(t *testing.T) {
+		patchHookWf := wfv1.MustUnmarshalWorkflow(patchHookWF)
+		targetHookWf := wfv1.MustUnmarshalWorkflow(baseNilHookWF)
 
-	err := MergeTo(patchHookWf, targetHookWf)
-	assert.NoError(t, err)
-	assert.Equal(t, 2, len(targetHookWf.Spec.Hooks))
-	assert.Equal(t, "a", targetHookWf.Spec.Hooks[`foo`].Template)
-	assert.Equal(t, "b", targetHookWf.Spec.Hooks[`bar`].Template)
+		err := MergeTo(patchHookWf, targetHookWf)
+		assert.NoError(t, err)
+		assert.Equal(t, 2, len(targetHookWf.Spec.Hooks))
+		assert.Equal(t, "c", targetHookWf.Spec.Hooks[`foo`].Template)
+		assert.Equal(t, "b", targetHookWf.Spec.Hooks[`bar`].Template)
+	})
+
+	t.Run("NilBaseAndNilPatch", func(t *testing.T) {
+		patchHookWf := wfv1.MustUnmarshalWorkflow(patchNilHookWF)
+		targetHookWf := wfv1.MustUnmarshalWorkflow(baseNilHookWF)
+
+		err := MergeTo(patchHookWf, targetHookWf)
+		assert.NoError(t, err)
+		assert.Nil(t, targetHookWf.Spec.Hooks)
+	})
+
+	// Ensure hook bar ends up in result, but foo is unchanged
+	t.Run("NotNilBaseAndPatch", func(t *testing.T) {
+		patchHookWf := wfv1.MustUnmarshalWorkflow(patchHookWF)
+		targetHookWf := wfv1.MustUnmarshalWorkflow(baseHookWF)
+
+		err := MergeTo(patchHookWf, targetHookWf)
+		assert.NoError(t, err)
+		assert.Equal(t, 2, len(targetHookWf.Spec.Hooks))
+		assert.Equal(t, "a", targetHookWf.Spec.Hooks[`foo`].Template)
+		assert.Equal(t, "b", targetHookWf.Spec.Hooks[`bar`].Template)
+	})
 }