Detect and fail upon unknown fields during argo submit & lint (resolves

#892)

CHANGELOG.md

@@ -40,6 +40,7 @@ and may be removed/ignored in a future release.
 + Support submission of workflows from json files (issue #926)
 + Support submission of workflows from stdin (issue #926)
 + Prometheus metrics and telemetry (issue #896) (@bbc88ks)
++ Detect and fail upon unknown fields during argo submit & lint (issue #892)
 * Remove installer/uninstaller (issue #928)
 * Update golang compiler to v1.10.3
 * Update k8s dependencies to v1.10 and client-go to v7.0

Gopkg.toml

@@ -56,3 +56,7 @@ required = [
 [[constraint]]
   name = "github.com/prometheus/client_golang"
   version = "0.8.0"
+
+[[constraint]]
+  name = "github.com/ghodss/yaml"
+  branch = "master"

cmd/argo/commands/common.go

@@ -122,15 +122,19 @@ func ansiFormat(s string, codes ...int) string {
 var yamlSeparator = regexp.MustCompile("\\n---")
 
 // splitYAMLFile is a helper to split a body into multiple workflow objects
-func splitYAMLFile(body []byte) ([]wfv1.Workflow, error) {
+func splitYAMLFile(body []byte, strict bool) ([]wfv1.Workflow, error) {
 	manifestsStrings := yamlSeparator.Split(string(body), -1)
 	manifests := make([]wfv1.Workflow, 0)
 	for _, manifestStr := range manifestsStrings {
 		if strings.TrimSpace(manifestStr) == "" {
 			continue
 		}
 		var wf wfv1.Workflow
-		err := yaml.Unmarshal([]byte(manifestStr), &wf)
+		var opts []yaml.JSONOpt
+		if strict {
+			opts = append(opts, yaml.DisallowUnknownFields)
+		}
+		err := yaml.Unmarshal([]byte(manifestStr), &wf, opts...)
 		if wf.Kind != "" && wf.Kind != workflow.Kind {
 			// If we get here, it was a k8s manifest which was not of type 'Workflow'
 			// We ignore these since we only care about validating Workflow manifests.

cmd/argo/commands/lint.go

@@ -14,6 +14,9 @@ import (
 )
 
 func NewLintCommand() *cobra.Command {
+	var (
+		strict bool
+	)
 	var command = &cobra.Command{
 		Use:   "lint (DIRECTORY | FILE1 FILE2 FILE3...)",
 		Short: "validate a directory or specific workflow YAML files",
@@ -30,7 +33,7 @@ func NewLintCommand() *cobra.Command {
 					os.Exit(1)
 				}
 				fmt.Printf("Verifying all yaml files in directory: %s\n", args[0])
-				err = lintYAMLDir(args[0])
+				err = lintYAMLDir(args[0], strict)
 			} else {
 				yamlFiles := make([]string, 0)
 				for _, filePath := range args {
@@ -41,7 +44,7 @@ func NewLintCommand() *cobra.Command {
 					yamlFiles = append(yamlFiles, filePath)
 				}
 				for _, yamlFile := range yamlFiles {
-					err = lintYAMLFile(yamlFile)
+					err = lintYAMLFile(yamlFile, strict)
 					if err != nil {
 						break
 					}
@@ -53,10 +56,11 @@ func NewLintCommand() *cobra.Command {
 			fmt.Printf("YAML validated\n")
 		},
 	}
+	command.Flags().BoolVar(&strict, "strict", true, "perform strict workflow validatation")
 	return command
 }
 
-func lintYAMLDir(dirPath string) error {
+func lintYAMLDir(dirPath string, strict bool) error {
 	walkFunc := func(path string, info os.FileInfo, err error) error {
 		if info == nil || info.IsDir() {
 			return nil
@@ -65,18 +69,18 @@ func lintYAMLDir(dirPath string) error {
 		if fileExt != ".yaml" && fileExt != ".yml" {
 			return nil
 		}
-		return lintYAMLFile(path)
+		return lintYAMLFile(path, strict)
 	}
 	return filepath.Walk(dirPath, walkFunc)
 }
 
 // lintYAMLFile lints multiple workflow manifest in a single yaml file. Ignores non-workflow manifests
-func lintYAMLFile(filePath string) error {
+func lintYAMLFile(filePath string, strict bool) error {
 	body, err := ioutil.ReadFile(filePath)
 	if err != nil {
 		return errors.Errorf(errors.CodeBadRequest, "Can't read from file: %s, err: %v", filePath, err)
 	}
-	workflows, err := splitYAMLFile(body)
+	workflows, err := splitYAMLFile(body, strict)
 	if err != nil {
 		return errors.Errorf(errors.CodeBadRequest, "%s failed to parse: %v", filePath, err)
 	}

cmd/argo/commands/submit.go

@@ -23,6 +23,7 @@ type submitFlags struct {
 	parameters     []string // --parameter
 	output         string   // --output
 	wait           bool     // --wait
+	strict         bool     // --strict
 	serviceAccount string   // --serviceaccount
 }
 
@@ -47,6 +48,7 @@ func NewSubmitCommand() *cobra.Command {
 	command.Flags().StringArrayVarP(&submitArgs.parameters, "parameter", "p", []string{}, "pass an input parameter")
 	command.Flags().StringVarP(&submitArgs.output, "output", "o", "", "Output format. One of: name|json|yaml|wide")
 	command.Flags().BoolVarP(&submitArgs.wait, "wait", "w", false, "wait for the workflow to complete")
+	command.Flags().BoolVar(&submitArgs.strict, "strict", false, "perform strict workflow validation")
 	command.Flags().StringVar(&submitArgs.serviceAccount, "serviceaccount", "", "run all pods in the workflow using specified serviceaccount")
 	command.Flags().StringVar(&submitArgs.instanceID, "instanceid", "", "submit with a specific controller's instance id label")
 	return command
@@ -61,7 +63,7 @@ func SubmitWorkflows(filePaths []string, submitArgs *submitFlags) {
 		if err != nil {
 			log.Fatal(err)
 		}
-		workflows = unmarshalWorkflows(body)
+		workflows = unmarshalWorkflows(body, submitArgs.strict)
 	} else {
 		for _, filePath := range filePaths {
 			var body []byte
@@ -82,7 +84,7 @@ func SubmitWorkflows(filePaths []string, submitArgs *submitFlags) {
 					log.Fatal(err)
 				}
 			}
-			wfs := unmarshalWorkflows(body)
+			wfs := unmarshalWorkflows(body, submitArgs.strict)
 			workflows = append(workflows, wfs...)
 		}
 	}
@@ -102,13 +104,13 @@ func SubmitWorkflows(filePaths []string, submitArgs *submitFlags) {
 }
 
 // unmarshalWorkflows unmarshals the input bytes as either json or yaml
-func unmarshalWorkflows(wfBytes []byte) []wfv1.Workflow {
+func unmarshalWorkflows(wfBytes []byte, strict bool) []wfv1.Workflow {
 	var wf wfv1.Workflow
 	err := json.Unmarshal(wfBytes, &wf)
 	if err == nil {
 		return []wfv1.Workflow{wf}
 	}
-	yamlWfs, err := splitYAMLFile(wfBytes)
+	yamlWfs, err := splitYAMLFile(wfBytes, strict)
 	if err == nil {
 		return yamlWfs
 	}

examples/daemon-nginx.yaml

@@ -21,7 +21,6 @@ spec:
     daemon: true
     container:
       image: nginx:1.13
-      restartPolicy: Always
       readinessProbe:
         httpGet:
           path: /

examples/daemon-step.yaml

@@ -47,7 +47,6 @@ spec:
     daemon: true
     container:
       image: influxdb:1.2
-      restartPolicy: Always
       readinessProbe:
         httpGet:
           path: /ping

examples/dag-daemon-task.yaml

@@ -56,7 +56,6 @@ spec:
     daemon: true
     container:
       image: influxdb:1.2
-      restartPolicy: Always
       readinessProbe:
         httpGet:
           path: /ping

examples/influxdb-ci.yaml

@@ -196,7 +196,6 @@ spec:
     daemon: true
     container:
       image: debian:9.4
-      restartPolicy: Always
       readinessProbe:
         httpGet:
           path: /ping