Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validation #1508

Merged
merged 4 commits into from
Aug 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions workflow/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
// 1) check if was supplied as argument. if so use the supplied value from arg
// 2) if not, use default value.
// 3) if no default value, it is an error
tmpl = tmpl.DeepCopy()
for i, inParam := range tmpl.Inputs.Parameters {
newTmpl := tmpl.DeepCopy()
for i, inParam := range newTmpl.Inputs.Parameters {
if inParam.Default != nil {
// first set to default value
inParam.Value = inParam.Default
Expand All @@ -259,12 +259,12 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
if inParam.Value == nil {
return nil, errors.Errorf(errors.CodeBadRequest, "inputs.parameters.%s was not supplied", inParam.Name)
}
tmpl.Inputs.Parameters[i] = inParam
newTmpl.Inputs.Parameters[i] = inParam
}

// Performs substitutions of input artifacts
newInputArtifacts := make([]wfv1.Artifact, len(tmpl.Inputs.Artifacts))
for i, inArt := range tmpl.Inputs.Artifacts {
newInputArtifacts := make([]wfv1.Artifact, len(newTmpl.Inputs.Artifacts))
for i, inArt := range newTmpl.Inputs.Artifacts {
// if artifact has hard-wired location, we prefer that
if inArt.HasLocation() {
newInputArtifacts[i] = inArt
Expand All @@ -288,9 +288,10 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
newInputArtifacts[i] = inArt
}
}
tmpl.Inputs.Artifacts = newInputArtifacts
newTmpl.Inputs.Artifacts = newInputArtifacts

return substituteParams(newTmpl, globalParams, localParams)

return substituteParams(tmpl, globalParams, localParams)
}

// substituteParams returns a new copy of the template with global, pod, and input parameters substituted
Expand Down
43 changes: 26 additions & 17 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
ctx.globalParams[common.GlobalVarWorkflowNamespace] = placeholderValue
ctx.globalParams[common.GlobalVarWorkflowUID] = placeholderValue
for _, param := range ctx.wf.Spec.Arguments.Parameters {
ctx.globalParams["workflow.parameters."+param.Name] = placeholderValue
if param.Name != "" {
if param.Value != nil {
ctx.globalParams["workflow.parameters."+param.Name] = *param.Value
} else {
ctx.globalParams["workflow.parameters."+param.Name] = placeholderValue
}
}
}

for k := range ctx.wf.ObjectMeta.Annotations {
Expand All @@ -96,7 +102,7 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
if entryTmpl == nil {
return errors.Errorf(errors.CodeBadRequest, "spec.entrypoint template '%s' undefined", ctx.wf.Spec.Entrypoint)
}
err = ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments)
err = ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments, map[string]interface{}{})
if err != nil {
return err
}
Expand All @@ -107,15 +113,15 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
}
// now when validating onExit, {{workflow.status}} is now available as a global
ctx.globalParams[common.GlobalVarWorkflowStatus] = placeholderValue
err = ctx.validateTemplate(exitTmpl, ctx.wf.Spec.Arguments)
err = ctx.validateTemplate(exitTmpl, ctx.wf.Spec.Arguments, map[string]interface{}{})
if err != nil {
return err
}
}
return nil
}

func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments) error {
func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments, extraScope map[string]interface{}) error {
_, ok := ctx.results[tmpl.Name]
if ok {
// we already processed this template
Expand All @@ -125,7 +131,7 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu
if err := validateTemplateType(tmpl); err != nil {
return err
}
scope, err := validateInputs(tmpl)
scope, err := validateInputs(tmpl, extraScope)
if err != nil {
return err
}
Expand All @@ -147,34 +153,34 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu
}
}

_, err = common.ProcessArgs(tmpl, args, ctx.globalParams, localParams, true)
newTmpl, err := common.ProcessArgs(tmpl, args, ctx.globalParams, localParams, true)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s %s", tmpl.Name, err)
}
for globalVar, val := range ctx.globalParams {
scope[globalVar] = val
}
switch tmpl.GetType() {
switch newTmpl.GetType() {
case wfv1.TemplateTypeSteps:
err = ctx.validateSteps(scope, tmpl)
err = ctx.validateSteps(scope, newTmpl)
case wfv1.TemplateTypeDAG:
err = ctx.validateDAG(scope, tmpl)
err = ctx.validateDAG(scope, newTmpl)
default:
err = validateLeaf(scope, tmpl)
err = validateLeaf(scope, newTmpl)
}
if err != nil {
return err
}
err = validateOutputs(scope, tmpl)
err = validateOutputs(scope, newTmpl)
if err != nil {
return err
}
err = ctx.validateBaseImageOutputs(tmpl)
err = ctx.validateBaseImageOutputs(newTmpl)
if err != nil {
return err
}
if tmpl.ArchiveLocation != nil {
err = validateArtifactLocation("templates.archiveLocation", *tmpl.ArchiveLocation)
if newTmpl.ArchiveLocation != nil {
err = validateArtifactLocation("templates.archiveLocation", *newTmpl.ArchiveLocation)
if err != nil {
return err
}
Expand All @@ -200,7 +206,7 @@ func validateTemplateType(tmpl *wfv1.Template) error {
return nil
}

func validateInputs(tmpl *wfv1.Template) (map[string]interface{}, error) {
func validateInputs(tmpl *wfv1.Template, extraScope map[string]interface{}) (map[string]interface{}, error) {
err := validateWorkflowFieldNames(tmpl.Inputs.Parameters)
if err != nil {
return nil, errors.Errorf(errors.CodeBadRequest, "templates.%s.inputs.parameters%s", tmpl.Name, err.Error())
Expand All @@ -210,6 +216,9 @@ func validateInputs(tmpl *wfv1.Template) (map[string]interface{}, error) {
return nil, errors.Errorf(errors.CodeBadRequest, "templates.%s.inputs.artifacts%s", tmpl.Name, err.Error())
}
scope := make(map[string]interface{})
for name, value := range extraScope {
scope[name] = value
}
for _, param := range tmpl.Inputs.Parameters {
scope[fmt.Sprintf("inputs.parameters.%s", param.Name)] = true
}
Expand Down Expand Up @@ -429,7 +438,7 @@ func (ctx *wfValidationCtx) validateSteps(scope map[string]interface{}, tmpl *wf
if err != nil {
return err
}
err = ctx.validateTemplate(childTmpl, step.Arguments)
err = ctx.validateTemplate(childTmpl, step.Arguments, scope)
if err != nil {
return err
}
Expand Down Expand Up @@ -787,7 +796,7 @@ func (ctx *wfValidationCtx) validateDAG(scope map[string]interface{}, tmpl *wfv1
return err
}
taskTmpl := ctx.wf.GetTemplate(task.Template)
err = ctx.validateTemplate(taskTmpl, task.Arguments)
err = ctx.validateTemplate(taskTmpl, task.Arguments, scope)
if err != nil {
return err
}
Expand Down
14 changes: 7 additions & 7 deletions workflow/validate/validate_dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ spec:
- name: A
template: echo
arguments:
parameters:
parameters:
- name: message
value: val
- name: B
Expand Down Expand Up @@ -154,14 +154,14 @@ spec:
- name: A
template: echo
arguments:
parameters:
parameters:
- name: message
value: val
- name: B
dependencies: [A]
template: echo
arguments:
parameters:
parameters:
- name: message
value: "{{tasks.A.outputs.parameters.hosts}}"
- name: C
Expand Down Expand Up @@ -199,14 +199,14 @@ spec:
- name: A
template: echo
arguments:
parameters:
parameters:
- name: message
value: val
- name: B
dependencies: [A]
template: echo
arguments:
parameters:
parameters:
- name: message
value: "{{tasks.A.outputs.parameters.hosts}}"
- name: C
Expand Down Expand Up @@ -246,7 +246,7 @@ spec:
command: [echo, generate]
outputs:
artifacts:
- name: hosts
- name: generated_hosts
path: /etc/hosts

- name: echo
Expand Down Expand Up @@ -282,7 +282,7 @@ spec:
value: val
artifacts:
- name: passthrough
from: "{{tasks.A.outputs.artifacts.hosts}}"
from: "{{tasks.A.outputs.artifacts.generated_hosts}}"
`

func TestDAGArtifactResolution(t *testing.T) {
Expand Down
60 changes: 58 additions & 2 deletions workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ var stepOutputReferences = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: hello-world-
generateName: step-output-ref-
spec:
entrypoint: whalesay
templates:
Expand Down Expand Up @@ -267,11 +267,67 @@ spec:
value: "{{steps.one.outparam}}"
`

func TestStepReference(t *testing.T) {
func TestStepOutputReference(t *testing.T) {
err := validate(stepOutputReferences)
assert.Nil(t, err)
}

var stepArtReferences = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: step-art-ref-
spec:
entrypoint: stepref
templates:
- name: generate
container:
image: alpine:3.7
command: [echo, generate]
outputs:
artifacts:
- name: generated_hosts
path: /etc/hosts

- name: echo
inputs:
parameters:
- name: message
artifacts:
- name: passthrough
path: /tmp/passthrough
container:
image: alpine:3.7
command: [echo, "{{inputs.parameters.message}}"]
outputs:
parameters:
- name: hosts
valueFrom:
path: /etc/hosts
artifacts:
- name: someoutput
path: /tmp/passthrough

- name: stepref
steps:
- - name: one
template: generate
- - name: two
template: echo
arguments:
parameters:
- name: message
value: val
artifacts:
- name: passthrough
from: "{{steps.one.outputs.artifacts.generated_hosts}}"
`

func TestStepArtReference(t *testing.T) {
err := validate(stepArtReferences)
assert.Nil(t, err)
}

var unsatisfiedParam = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
Expand Down