fix(controller): Empty global output param crashes (argoproj#5931)

Signed-off-by: Saravanan Balasubramanian <[email protected]>

cmd/argo/commands/cron/get.go

@@ -106,10 +106,10 @@ func getCronWorkflowGet(cwf *wfv1.CronWorkflow) string {
  if len(cwf.Spec.WorkflowSpec.Arguments.Parameters) > 0 {
  out += fmt.Sprintf(fmtStr, "Workflow Parameters:", "")
  for _, param := range cwf.Spec.WorkflowSpec.Arguments.Parameters {
- if param.Value == nil {
+ if !param.HasValue() {
  continue
  }
- out += fmt.Sprintf(fmtStr, " "+param.Name+":", *param.Value)
+ out += fmt.Sprintf(fmtStr, " "+param.Name+":", param.GetValue())
  }
  }
  return out

cmd/argo/commands/get.go

@@ -162,7 +162,9 @@ func printWorkflowHelper(wf *wfv1.Workflow, getArgs getFlags) string {
  if len(wf.Status.Outputs.Parameters) > 0 {
  out += fmt.Sprintf(fmtStr, "Output Parameters:", "")
  for _, param := range wf.Status.Outputs.Parameters {
- out += fmt.Sprintf(fmtStr, " "+param.Name+":", *param.Value)
+ if param.HasValue() {
+ out += fmt.Sprintf(fmtStr, " "+param.Name+":", param.GetValue())
+ }
  }
  }
  if len(wf.Status.Outputs.Artifacts) > 0 {

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -774,6 +774,20 @@ type ValueFrom struct {
  Expression string `json:"expression,omitempty" protobuf:"bytes,8,rep,name=expression"`
 }
 
+func (p *Parameter) HasValue() bool {
+ return p.Value != nil || p.Default != nil || p.ValueFrom != nil
+}
+
+func (p *Parameter) GetValue() string {
+ if p.Value != nil {
+ return p.Value.String()
+ }
+ if p.Default != nil {
+ return p.Default.String()
+ }
+ return ""
+}
+
 // SuppliedValueFrom is a placeholder for a value to be filled in directly, either through the CLI, API, etc.
 type SuppliedValueFrom struct{}
 

pkg/apis/workflow/v1alpha1/workflow_types_test.go

@@ -890,3 +890,21 @@ func TestHasChild(t *testing.T) {
  assert.False(t, node.HasChild("c"))
  assert.False(t, node.HasChild(""))
 }
+
+func TestParameterGetValue(t *testing.T) {
+ empty := Parameter{}
+ defaultVal := Parameter{Default: AnyStringPtr("Default")}
+ value := Parameter{Value: AnyStringPtr("Test")}
+
+ valueFrom := Parameter{ValueFrom: &ValueFrom{}}
+ assert.False(t, empty.HasValue())
+ assert.Empty(t, empty.GetValue())
+ assert.True(t, defaultVal.HasValue())
+ assert.NotEmpty(t, defaultVal.GetValue())
+ assert.Equal(t, "Default", defaultVal.GetValue())
+ assert.True(t, value.HasValue())
+ assert.NotEmpty(t, value.GetValue())
+ assert.Equal(t, "Test", value.GetValue())
+ assert.True(t, valueFrom.HasValue())
+
+}

workflow/controller/operator.go

@@ -494,7 +494,9 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument
  }
  if woc.wf.Status.Outputs != nil {
  for _, param := range woc.wf.Status.Outputs.Parameters {
- woc.globalParams["workflow.outputs.parameters."+param.Name] = param.Value.String()
+ if param.HasValue() {
+ woc.globalParams["workflow.outputs.parameters."+param.Name] = param.GetValue()
+ }
  }
  }
 }
@@ -2610,7 +2612,9 @@ func (woc *wfOperationCtx) addParamToGlobalScope(param wfv1.Parameter) {
  return
  }
  paramName := fmt.Sprintf("workflow.outputs.parameters.%s", param.GlobalName)
- woc.globalParams[paramName] = param.Value.String()
+ if param.HasValue() {
+ woc.globalParams[paramName] = param.GetValue()
+ }
  wfUpdated := wfutil.AddParamToGlobalScope(woc.wf, woc.log, param)
  if wfUpdated {
  woc.updated = true

workflow/controller/operator_test.go

@@ -6785,3 +6785,100 @@ func TestMutexWfPendingWithNoPod(t *testing.T) {
  assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
  assert.Equal(t, wfv1.NodePending, woc.wf.Status.Nodes.FindByDisplayName("hello-world-mpdht").Phase)
 }
+
+var wfGlopalArtifactNil = `apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+ name: global-outputs-ttsfq
+ namespace: default
+spec:
+ entrypoint: generate-globals
+ onExit: consume-globals
+ templates:
+ - name: generate-globals
+ steps:
+ - - name: generate
+ template: global-output
+ - - name: consume-globals
+ template: consume-globals
+ - container:
+ args:
+ - sleep 1; exit 1
+ command:
+ - sh
+ - -c
+ image: alpine:3.7
+ name: global-output
+ outputs:
+ artifacts:
+ - globalName: my-global-art
+ name: hello-art
+ path: /tmp/hello_world.txt
+ parameters:
+ - globalName: my-global-param
+ name: hello-param
+ valueFrom:
+ path: /tmp/hello_world.txt
+ - name: consume-globals
+ steps:
+ - - name: consume-global-param
+ template: consume-global-param
+ - arguments:
+ artifacts:
+ - from: '{{workflow.outputs.artifacts.my-global-art}}'
+ name: art
+ name: consume-global-art
+ template: consume-global-art
+ - container:
+ args:
+ - echo {{inputs.parameters.param}}
+ command:
+ - sh
+ - -c
+ image: alpine:3.7
+ inputs:
+ parameters:
+ - name: param
+ value: '{{workflow.outputs.parameters.my-global-param}}'
+ name: consume-global-param
+ - container:
+ args:
+ - cat /art
+ command:
+ - sh
+ - -c
+ image: alpine:3.7
+ inputs:
+ artifacts:
+ - name: art
+ path: /art
+ name: consume-global-art
+`
+
+func TestWFGlobalArtifactNil(t *testing.T) {
+ wf := wfv1.MustUnmarshalWorkflow(wfGlopalArtifactNil)
+ cancel, controller := newController(wf)
+ defer cancel()
+
+ ctx := context.Background()
+ woc := newWorkflowOperationCtx(wf, controller)
+ woc.operate(ctx)
+ makePodsPhase(ctx, woc, apiv1.PodRunning)
+ woc.operate(ctx)
+ makePodsPhase(ctx, woc, apiv1.PodFailed, func(pod *apiv1.Pod) {
+ pod.Annotations[common.AnnotationKeyOutputs] = string("{\"parameters\":[{\"name\":\"hello-param\",\"valueFrom\":{\"path\":\"/tmp/hello_world.txt\"},\"globalName\":\"my-global-param\"}],\"artifacts\":[{\"name\":\"hello-art\",\"path\":\"/tmp/hello_world.txt\",\"globalName\":\"my-global-art\"}]}")
+ pod.Status.ContainerStatuses = []apiv1.ContainerStatus{
+ {
+ Name: "main",
+ State: apiv1.ContainerState{
+ Terminated: &apiv1.ContainerStateTerminated{
+ ExitCode: 1,
+ Message: "",
+ },
+ },
+ },
+ }
+ })
+ woc.operate(ctx)
+ assert.NotPanics(t, func() { woc.operate(ctx) })
+}