Add validation checks for volumeMount/artifact path collision and act…

…iveDeadlineSeconds (argoproj#620)

workflow/common/validate.go

@@ -213,6 +213,27 @@ func validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template) error {
  if err != nil {
  return errors.Errorf(errors.CodeBadRequest, "template '%s' %s", tmpl.Name, err.Error())
  }
+ if tmpl.Container != nil {
+ // Ensure there are no collisions with volume mountPaths and artifact load paths
+ mountPaths := make(map[string]string)
+ for i, volMount := range tmpl.Container.VolumeMounts {
+ if prev, ok := mountPaths[volMount.MountPath]; ok {
+ return errors.Errorf(errors.CodeBadRequest, "template '%s' container.volumeMounts[%d].mountPath '%s' already mounted in %s", tmpl.Name, i, volMount.MountPath, prev)
+ }
+ mountPaths[volMount.MountPath] = fmt.Sprintf("container.volumeMounts.%s", volMount.Name)
+ }
+ for i, art := range tmpl.Inputs.Artifacts {
+ if prev, ok := mountPaths[art.Path]; ok {
+ return errors.Errorf(errors.CodeBadRequest, "template '%s' inputs.artifacts[%d].path '%s' already mounted in %s", tmpl.Name, i, art.Path, prev)
+ }
+ mountPaths[art.Path] = fmt.Sprintf("inputs.artifacts.%s", art.Name)
+ }
+ }
+ if tmpl.ActiveDeadlineSeconds != nil {
+ if *tmpl.ActiveDeadlineSeconds <= 0 {
+ return errors.Errorf(errors.CodeBadRequest, "template '%s' activeDeadlineSeconds must be a positive integer > 0", tmpl.Name)
+ }
+ }
  return nil
 }
 

workflow/common/validate_test.go

@@ -11,12 +11,17 @@ import (
 // validate is a test helper to accept YAML as a string and return
 // its validation result.
 func validate(yamlStr string) error {
+ wf := unmarshalWf(yamlStr)
+ return ValidateWorkflow(wf)
+}
+
+func unmarshalWf(yamlStr string) *wfv1.Workflow {
  var wf wfv1.Workflow
  err := yaml.Unmarshal([]byte(yamlStr), &wf)
  if err != nil {
- return err
+ panic(err)
  }
- return ValidateWorkflow(&wf)
+ return &wf
 }
 
 const invalidErr = "is invalid"
@@ -577,3 +582,72 @@ func TestExitHandler(t *testing.T) {
  err = validate(exitHandlerWorkflowStatusOnExit)
  assert.Nil(t, err)
 }
+
+var volumeMountArtifactPathCollision = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+ generateName: path-collision-
+spec:
+ volumeClaimTemplates:
+ - metadata:
+ name: workdir
+ spec:
+ accessModes: [ "ReadWriteOnce" ]
+ resources:
+ requests:
+ storage: 1Gi
+ entrypoint: pass
+ templates:
+ - name: pass
+ inputs:
+ artifacts:
+ - name: argo-source
+ path: /src
+ git:
+ repo: https://github.com/argoproj/argo.git
+ container:
+ image: alpine:latest
+ command: [sh, -c]
+ args: ["exit 0"]
+ volumeMounts:
+ - name: workdir
+ mountPath: /src
+`
+
+func TestVolumeMountArtifactPathCollision(t *testing.T) {
+ // ensure we detect and reject path collisions
+ wf := unmarshalWf(volumeMountArtifactPathCollision)
+ err := ValidateWorkflow(wf)
+ if assert.NotNil(t, err) {
+ assert.Contains(t, err.Error(), "already mounted")
+ }
+ // tweak the mount path and validation should now be successful
+ wf.Spec.Templates[0].Container.VolumeMounts[0].MountPath = "/differentpath"
+ err = ValidateWorkflow(wf)
+ assert.Nil(t, err)
+}
+
+var activeDeadlineSeconds = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+ generateName: active-deadline-seconds-
+spec:
+ entrypoint: pass
+ templates:
+ - name: pass
+ activeDeadlineSeconds: -1
+ container:
+ image: alpine:latest
+ command: [sh, -c]
+ args: ["exit 0"]
+`
+
+func TestValidActiveDeadlineSeconds(t *testing.T) {
+ // ensure {{workflow.status}} is not available when not in exit handler
+ err := validate(activeDeadlineSeconds)
+ if assert.NotNil(t, err) {
+ assert.Contains(t, err.Error(), "activeDeadlineSeconds must be a positive integer > 0")
+ }
+}