Allow output parameters with .value, not only .valueFrom (argoproj#1336)

Fixed argoproj#1329 Allow output parameters with .value, not only .valueFrom (argoproj#1336)
Duske · Aug 15, 2019 · d90ec0d · d90ec0d
1 parent 40a92b2
commit d90ec0d
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 15 deletions.
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"encoding/json"
 	"fmt"
 	"github.com/argoproj/argo/workflow/config"
 	"strings"
@@ -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

diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
@@ -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) {
@@ -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} {

diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go
@@ -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) {