fix(executor): Fix resource patch when not providing flags. Fixes #5310…

… (#5311)

workflow/executor/resource.go

@@ -59,6 +59,14 @@ func (we *WorkflowExecutor) ExecResource(action string, manifestPath string, fla
 }
 
 func (we *WorkflowExecutor) getKubectlArguments(action string, manifestPath string, flags []string) ([]string, error) {
+	buff, err := ioutil.ReadFile(manifestPath)
+	if err != nil {
+		return []string{}, errors.New(errors.CodeBadRequest, err.Error())
+	}
+	if len(buff) == 0 && len(flags) == 0 {
+		return []string{}, errors.New(errors.CodeBadRequest, "Must provide at least one of flags or manifest.")
+	}
+
 	args := []string{
 		action,
 	}
@@ -69,11 +77,6 @@ func (we *WorkflowExecutor) getKubectlArguments(action string, manifestPath stri
 		output = "name"
 	}
 
-	buff, err := ioutil.ReadFile(manifestPath)
-	if err != nil {
-		return []string{}, errors.New(errors.CodeBadRequest, err.Error())
-	}
-
 	if action == "patch" {
 		mergeStrategy := "strategic"
 		if we.Template.Resource.MergeStrategy != "" {
@@ -97,8 +100,6 @@ func (we *WorkflowExecutor) getKubectlArguments(action string, manifestPath stri
 	if len(buff) != 0 && action != "patch" {
 		args = append(args, "-f")
 		args = append(args, manifestPath)
-	} else if len(flags) <= 0 {
-		return []string{}, errors.New(errors.CodeBadRequest, "Must provide at least one of flags or manifest.")
 	}
 	args = append(args, "-o")
 	args = append(args, output)

workflow/executor/resource_test.go

@@ -1,6 +1,8 @@
 package executor
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -13,6 +15,7 @@ import (
 // TestResourceFlags tests whether Resource Flags
 // are properly passed to `kubectl` command
 func TestResourceFlags(t *testing.T) {
+	manifestPath := "../../examples/hello-world.yaml"
 	fakeClientset := fake.NewSimpleClientset()
 	fakeFlags := []string{"--fake=true"}
 
@@ -34,8 +37,51 @@ func TestResourceFlags(t *testing.T) {
 		ExecutionControl:   nil,
 		RuntimeExecutor:    &mockRuntimeExecutor,
 	}
-	args, err := we.getKubectlArguments("fake", "../../examples/hello-world.yaml", fakeFlags)
-
-	assert.Nil(t, err)
+	args, err := we.getKubectlArguments("fake", manifestPath, fakeFlags)
+	assert.NoError(t, err)
 	assert.Contains(t, args, fakeFlags[0])
+
+	_, err = we.getKubectlArguments("fake", manifestPath, nil)
+	assert.NoError(t, err)
+	_, err = we.getKubectlArguments("fake", "unknown-location", fakeFlags)
+	assert.EqualError(t, err, "open unknown-location: no such file or directory")
+
+	emptyFile, err := ioutil.TempFile("/tmp", "empty-manifest")
+	assert.NoError(t, err)
+	defer func() { _ = os.Remove(emptyFile.Name()) }()
+	_, err = we.getKubectlArguments("fake", emptyFile.Name(), nil)
+	assert.EqualError(t, err, "Must provide at least one of flags or manifest.")
+}
+
+// TestResourcePatchFlags tests whether Resource Flags
+// are properly passed to `kubectl patch` command
+func TestResourcePatchFlags(t *testing.T) {
+	fakeClientset := fake.NewSimpleClientset()
+	manifestPath := "../../examples/hello-world.yaml"
+	buff, err := ioutil.ReadFile(manifestPath)
+	assert.NoError(t, err)
+	fakeFlags := []string{"patch", "--type", "strategic", "-p", string(buff), "-o", "json"}
+
+	mockRuntimeExecutor := mocks.ContainerRuntimeExecutor{}
+
+	template := wfv1.Template{
+		Resource: &wfv1.ResourceTemplate{
+			Action: "patch",
+			Flags:  fakeFlags,
+		},
+	}
+
+	we := WorkflowExecutor{
+		PodName:            fakePodName,
+		Template:           template,
+		ClientSet:          fakeClientset,
+		Namespace:          fakeNamespace,
+		PodAnnotationsPath: fakeAnnotations,
+		ExecutionControl:   nil,
+		RuntimeExecutor:    &mockRuntimeExecutor,
+	}
+	args, err := we.getKubectlArguments("patch", manifestPath, nil)
+
+	assert.NoError(t, err)
+	assert.Equal(t, args, fakeFlags)
 }