fix: Allow empty strings in valueFrom.default (argoproj#2805)

highb · Apr 23, 2020 · 510e11b · 510e11b
1 parent d7f41ac
commit 510e11b
Show file tree

Hide file tree

Showing 7 changed files with 466 additions and 423 deletions.
diff --git a/pkg/apis/workflow/v1alpha1/generated.pb.go b/pkg/apis/workflow/v1alpha1/generated.pb.go
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -596,7 +596,7 @@ type ValueFrom struct {
  Parameter string `json:"parameter,omitempty" protobuf:"bytes,4,opt,name=parameter"`
 
  // Default specifies a value to be used if retrieving the value from the specified source fails
- Default string `json:"default,omitempty" protobuf:"bytes,5,opt,name=default"`
+ Default *string `json:"default,omitempty" protobuf:"bytes,5,opt,name=default"`
 }
 
 // Artifact indicates an artifact to place at a specified path

diff --git a/pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -1871,8 +1871,8 @@ func getTemplateOutputsFromScope(tmpl *wfv1.Template, scope *wfScope) (*wfv1.Out
  val, err := scope.resolveParameter(param.ValueFrom.Parameter)
  if err != nil {
  // We have a default value to use instead of returning an error
- if param.ValueFrom.Default != "" {
- val = param.ValueFrom.Default
+ if param.ValueFrom.Default != nil {
+ val = *param.ValueFrom.Default
  } else {
  return nil, err
  }

diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go
@@ -443,8 +443,8 @@ func (we *WorkflowExecutor) SaveParameters() error {
  output, err = we.RuntimeExecutor.GetFileContents(mainCtrID, param.ValueFrom.Path)
  if err != nil {
  // We have a default value to use instead of returning an error
- if param.ValueFrom.Default != "" {
- output = param.ValueFrom.Default
+ if param.ValueFrom.Default != nil {
+ output = *param.ValueFrom.Default
  } else {
  return err
  }
@@ -455,8 +455,8 @@ func (we *WorkflowExecutor) SaveParameters() error {
  out, err := ioutil.ReadFile(mountedPath)
  if err != nil {
  // We have a default value to use instead of returning an error
- if param.ValueFrom.Default != "" {
- output = param.ValueFrom.Default
+ if param.ValueFrom.Default != nil {
+ output = *param.ValueFrom.Default
  } else {
  return err
  }

diff --git a/workflow/executor/executor_test.go b/workflow/executor/executor_test.go
@@ -107,6 +107,7 @@ func TestIsBaseImagePath(t *testing.T) {
 }
 
 func TestDefaultParameters(t *testing.T) {
+ defaultValue := "Default Value"
  fakeClientset := fake.NewSimpleClientset()
  mockRuntimeExecutor := mocks.ContainerRuntimeExecutor{}
  templateWithOutParam := wfv1.Template{
@@ -115,7 +116,7 @@ func TestDefaultParameters(t *testing.T) {
  {
  Name: "my-out",
  ValueFrom: &wfv1.ValueFrom{
- Default: "Default Value",
+ Default: &defaultValue,
  Path: "/path",
  },
  },
@@ -137,3 +138,36 @@ func TestDefaultParameters(t *testing.T) {
  assert.NoError(t, err)
  assert.Equal(t, "Default Value", *we.Template.Outputs.Parameters[0].Value)
 }
+
+func TestDefaultParametersEmptyString(t *testing.T) {
+ defaultValue := ""
+ fakeClientset := fake.NewSimpleClientset()
+ mockRuntimeExecutor := mocks.ContainerRuntimeExecutor{}
+ templateWithOutParam := wfv1.Template{
+ Outputs: wfv1.Outputs{
+ Parameters: []wfv1.Parameter{
+ {
+ Name: "my-out",
+ ValueFrom: &wfv1.ValueFrom{
+ Default: &defaultValue,
+ Path: "/path",
+ },
+ },
+ },
+ },
+ }
+ we := WorkflowExecutor{
+ PodName: fakePodName,
+ Template: templateWithOutParam,
+ ClientSet: fakeClientset,
+ Namespace: fakeNamespace,
+ PodAnnotationsPath: fakeAnnotations,
+ ExecutionControl: nil,
+ RuntimeExecutor: &mockRuntimeExecutor,
+ mainContainerID: fakeContainerID,
+ }
+ mockRuntimeExecutor.On("GetFileContents", fakeContainerID, "/path").Return("", fmt.Errorf("file not found"))
+ err := we.SaveParameters()
+ assert.NoError(t, err)
+ assert.Equal(t, "", *we.Template.Outputs.Parameters[0].Value)
+}
diff --git a/workflow/executor/resource.go b/workflow/executor/resource.go
@@ -328,8 +328,8 @@ func (we *WorkflowExecutor) SaveResourceParameters(resourceNamespace string, res
  out, err := cmd.Output()
  if err != nil {
  // We have a default value to use instead of returning an error
- if param.ValueFrom.Default != "" {
- out = []byte(param.ValueFrom.Default)
+ if param.ValueFrom.Default != nil {
+ out = []byte(*param.ValueFrom.Default)
  } else {
  if exErr, ok := err.(*exec.ExitError); ok {
  log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr))