Merge pull request #21 from chrisgavin/workflow-validation-warnings

Make workflow validation more robust and report invalid workflows as warnings.

Author: chrisgavin

internal/locator/repository-workflow-locator.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/chrisgavin/gh-dispatch/internal/workflow"
 	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
 )
 
 func ListWorkflowsInRepository() (map[string]workflow.Workflow, error) {
@@ -43,7 +44,8 @@ func ListWorkflowsInRepository() (map[string]workflow.Workflow, error) {
 		}
 		loaded, err := workflow.ReadWorkflow(entry.Name(), bytes)
 		if err != nil {
-			return nil, errors.Wrap(err, "Unable to read workflow.")
+			log.Warnf("Workflow \"%s\" is invalid: %s", entry.Name(), err)
+			continue
 		}
 		if !loaded.Dispatchable {
 			continue

internal/workflow/workflow.go

@@ -25,7 +25,7 @@ func ReadWorkflow(name string, rawWorkflow []byte) (*Workflow, error) {
 	parsed := make(map[string]interface{})
 	err := yaml.Unmarshal(rawWorkflow, &parsed)
 	if err != nil {
-		return nil, errors.Wrap(err, "Unable to parse workflow.")
+		return nil, errors.Wrap(err, "Unable to parse workflow as YAML.")
 	}
 	if on, ok := parsed["on"]; ok {
 		switch typedOn := on.(type) {
@@ -41,28 +41,38 @@ func ReadWorkflow(name string, rawWorkflow []byte) (*Workflow, error) {
 			for event, eventConfiguration := range typedOn {
 				if event == workflowDispatch {
 					workflow.Dispatchable = true
-					// TODO: How many of the switches can be converted to type assertions and do we need to handle errors?
-					switch typedEventConfiguration := eventConfiguration.(type) {
-					case map[interface{}]interface{}:
+					if eventConfiguration != nil {
+						typedEventConfiguration, ok := eventConfiguration.(map[interface{}]interface{})
+						if !ok {
+							return nil, errors.Errorf("Workflow dispatch configuration had unexpected type %T.", eventConfiguration)
+						}
 						if inputs, ok := typedEventConfiguration["inputs"]; ok {
-							switch typedInputs := inputs.(type) {
-							case map[interface{}]interface{}:
-								for inputName, inputConfiguration := range typedInputs {
-									input := Input{
-										Name: inputName.(string),
-									}
-									if inputDescription, ok := inputConfiguration.(map[interface{}]interface{})["description"]; ok {
-										input.Description = inputDescription.(string)
+							typedInputs, ok := inputs.(map[interface{}]interface{})
+							if !ok {
+								return nil, errors.Errorf("Workflow dispatch configuration inputs had unexpected type %T.", inputs)
+							}
+							for inputName, inputConfiguration := range typedInputs {
+								typedInputConfiguration, ok := inputConfiguration.(map[interface{}]interface{})
+								if !ok {
+									return nil, errors.Errorf("Input configuration for %s had unexpected type %T.", inputName, inputConfiguration)
+								}
+								input := Input{
+									Name: inputName.(string),
+								}
+								if inputDescription, ok := typedInputConfiguration["description"]; ok {
+									input.Description, ok = inputDescription.(string)
+									if !ok {
+										return nil, errors.Errorf("Input description for %s had unexpected type %T.", inputName, inputDescription)
 									}
-									workflow.Inputs = append(workflow.Inputs, input)
 								}
+								workflow.Inputs = append(workflow.Inputs, input)
 							}
 						}
 					}
 				}
 			}
 		default:
-			return nil, errors.Errorf("Unable to parse workflow \"on\" clause. Unexpected type %T.", on) // TODO: Should we error here?
+			return nil, errors.Errorf("Unable to parse workflow \"on\" clause. Unexpected type %T.", on)
 		}
 	}
 	return &workflow, nil