Skip to content

Commit

Permalink
Allow output parameters with .value, not only .valueFrom (argoproj#1336)
Browse files Browse the repository at this point in the history
Fixed argoproj#1329 Allow output parameters with .value, not only .valueFrom (argoproj#1336)
  • Loading branch information
Ark-kun authored and sarabala1979 committed Jul 30, 2019
1 parent 781d3b8 commit e03287b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 15 deletions.
38 changes: 38 additions & 0 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controller

import (
"encoding/json"
"fmt"
"github.com/argoproj/argo/workflow/config"
"strings"
Expand Down Expand Up @@ -1008,6 +1009,43 @@ func TestResolveIOPathPlaceholders(t *testing.T) {
assert.Equal(t, []string{"sh", "-c", "head -n 3 <\"/inputs/text/data\" | tee \"/outputs/text/data\" | wc -l > \"/outputs/actual-lines-count/data\""}, pods.Items[0].Spec.Containers[1].Command)
}

var outputValuePlaceholders = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: output-value-placeholders-wf
spec:
entrypoint: tell-pod-name
templates:
- name: tell-pod-name
outputs:
parameters:
- name: pod-name
value: "{{pod.name}}"
container:
image: busybox
`

func TestResolvePlaceholdersInOutputValues(t *testing.T) {
wf := unmarshalWF(outputValuePlaceholders)
woc := newWoc(*wf)
woc.controller.Config.ArtifactRepository.S3 = new(S3ArtifactRepository)
woc.operate()
assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase)
pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{})
assert.Nil(t, err)
assert.True(t, len(pods.Items) > 0, "pod was not created successfully")

templateString := pods.Items[0].ObjectMeta.Annotations["workflows.argoproj.io/template"]
var template wfv1.Template
err = json.Unmarshal([]byte(templateString), &template)
assert.Nil(t, err)
parameterValue := template.Outputs.Parameters[0].Value
assert.NotNil(t, parameterValue)
assert.NotEmpty(t, *parameterValue)
assert.Equal(t, "output-value-placeholders-wf", *parameterValue)
}

var resourceTemplate = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
Expand Down
36 changes: 22 additions & 14 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,19 +559,21 @@ func validateOutputs(scope map[string]interface{}, tmpl *wfv1.Template) error {
if err != nil {
return err
}
tmplType := tmpl.GetType()
switch tmplType {
case wfv1.TemplateTypeContainer, wfv1.TemplateTypeScript:
if param.ValueFrom.Path == "" {
return errors.Errorf(errors.CodeBadRequest, "%s.path must be specified for %s templates", paramRef, tmplType)
}
case wfv1.TemplateTypeResource:
if param.ValueFrom.JQFilter == "" && param.ValueFrom.JSONPath == "" {
return errors.Errorf(errors.CodeBadRequest, "%s .jqFilter or jsonPath must be specified for %s templates", paramRef, tmplType)
}
case wfv1.TemplateTypeDAG, wfv1.TemplateTypeSteps:
if param.ValueFrom.Parameter == "" {
return errors.Errorf(errors.CodeBadRequest, "%s.parameter must be specified for %s templates", paramRef, tmplType)
if param.ValueFrom != nil {
tmplType := tmpl.GetType()
switch tmplType {
case wfv1.TemplateTypeContainer, wfv1.TemplateTypeScript:
if param.ValueFrom.Path == "" {
return errors.Errorf(errors.CodeBadRequest, "%s.path must be specified for %s templates", paramRef, tmplType)
}
case wfv1.TemplateTypeResource:
if param.ValueFrom.JQFilter == "" && param.ValueFrom.JSONPath == "" {
return errors.Errorf(errors.CodeBadRequest, "%s .jqFilter or jsonPath must be specified for %s templates", paramRef, tmplType)
}
case wfv1.TemplateTypeDAG, wfv1.TemplateTypeSteps:
if param.ValueFrom.Parameter == "" {
return errors.Errorf(errors.CodeBadRequest, "%s.parameter must be specified for %s templates", paramRef, tmplType)
}
}
}
if param.GlobalName != "" && !isParameter(param.GlobalName) {
Expand Down Expand Up @@ -631,8 +633,14 @@ func (ctx *wfValidationCtx) validateBaseImageOutputs(tmpl *wfv1.Template) error

// validateOutputParameter verifies that only one of valueFrom is defined in an output
func validateOutputParameter(paramRef string, param *wfv1.Parameter) error {
if param.ValueFrom != nil && param.Value != nil {
return errors.Errorf(errors.CodeBadRequest, "%s has both valueFrom and value specified. Choose one.", paramRef)
}
if param.Value != nil {
return nil
}
if param.ValueFrom == nil {
return errors.Errorf(errors.CodeBadRequest, "%s.valueFrom not specified", paramRef)
return errors.Errorf(errors.CodeBadRequest, "%s does not have valueFrom or value specified", paramRef)
}
paramTypes := 0
for _, value := range []string{param.ValueFrom.Path, param.ValueFrom.JQFilter, param.ValueFrom.JSONPath, param.ValueFrom.Parameter} {
Expand Down
2 changes: 1 addition & 1 deletion workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ func TestInvalidOutputParam(t *testing.T) {
}
err = validate(invalidOutputMissingValueFrom)
if assert.NotNil(t, err) {
assert.Contains(t, err.Error(), "valueFrom not specified")
assert.Contains(t, err.Error(), "does not have valueFrom or value specified")
}
err = validate(invalidOutputMultipleValueFrom)
if assert.NotNil(t, err) {
Expand Down

0 comments on commit e03287b

Please sign in to comment.