General clean up to the /internal/workflow pkg

internal/workflow/controller.go

@@ -112,7 +112,7 @@ func (c *Controller) processNewWorkflow(ctx context.Context, logger logr.Logger,
 		data["Hardware"] = contract
 	}
 
-	tinkWf, _, err := RenderTemplateHardware(stored.Name, ptr.StringValue(tpl.Spec.Data), data)
+	tinkWf, err := renderTemplateHardware(stored.Name, ptr.StringValue(tpl.Spec.Data), data)
 	if err != nil {
 		return reconcile.Result{}, err
 	}

internal/workflow/template_validator.go

@@ -2,10 +2,7 @@ package workflow
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
 	"text/template"
 
 	"github.com/docker/distribution/reference"
@@ -14,108 +11,65 @@ import (
 )
 
 const (
-	errEmptyName              = "name cannot be empty"
-	errInvalidLength          = "name cannot have more than 200 characters: %s"
-	errTemplateInvalidVersion = "invalid template version: %s"
-	errTaskDuplicateName      = "two tasks in a template cannot have same name: %s"
-	errActionDuplicateName    = "two actions in a task cannot have same name: %s"
-	errActionInvalidImage     = "invalid action image: %s"
-	errTemplateParsing        = "failed to parse template with ID %s"
-	errInvalidHardwareAddress = "failed to render template, invalid hardware address: %v"
+	errInvalidLength   = "name cannot be empty or have more than 200 characters: %s"
+	errTemplateParsing = "failed to parse template with ID %s"
 )
 
-// Parse parses the template yaml content into a Workflow.
-func Parse(yamlContent []byte) (*Workflow, error) {
+// parse parses the template yaml content into a Workflow.
+func parse(yamlContent []byte) (*Workflow, error) {
 	var workflow Workflow
 
-	err := yaml.UnmarshalStrict(yamlContent, &workflow)
-	if err != nil {
+	if err := yaml.UnmarshalStrict(yamlContent, &workflow); err != nil {
 		return &Workflow{}, errors.Wrap(err, "parsing yaml data")
 	}
 
-	if err = validate(&workflow); err != nil {
+	if err := validate(&workflow); err != nil {
 		return &Workflow{}, errors.Wrap(err, "validating workflow template")
 	}
 
 	return &workflow, nil
 }
 
-// MustParse parse a slice of bytes to a template. It an error occurs the
-// function triggers a panic. Common utility for testing purpose.
-func MustParse(yamlContent []byte) *Workflow {
-	w, err := Parse(yamlContent)
-	if err != nil {
-		panic(err)
-	}
-	return w
-}
-
-// MustParseFromFile parse a template from a file and it panics if any error is
-// detected. Ideal to be used in testing.
-func MustParseFromFile(path string) *Workflow {
-	content, err := os.ReadFile(filepath.Clean(path))
-	if err != nil {
-		panic(err)
-	}
-	return MustParse(content)
-}
-
-// RenderTemplate renders the workflow template with regard to the given hardware details.
-func RenderTemplate(templateID, templateData string, devices []byte) (string, error) {
-	var hardware map[string]interface{}
-	err := json.Unmarshal(devices, &hardware)
-	if err != nil {
-		err = errors.Wrapf(err, errTemplateParsing, templateID)
-		return "", err
-	}
-
-	_, buf, err := RenderTemplateHardware(templateID, templateData, hardware)
-	if err != nil {
-		return "", err
-	}
-	return buf.String(), nil
-}
-
-// RenderTemplateHardware renders the workflow template and returns the Workflow and the interpolated bytes.
-func RenderTemplateHardware(templateID, templateData string, hardware map[string]interface{}) (*Workflow, *bytes.Buffer, error) {
+// renderTemplateHardware renders the workflow template and returns the Workflow and the interpolated bytes.
+func renderTemplateHardware(templateID, templateData string, hardware map[string]interface{}) (*Workflow, error) {
 	t := template.New("workflow-template").
 		Option("missingkey=error").
 		Funcs(templateFuncs)
+
 	_, err := t.Parse(templateData)
 	if err != nil {
 		err = errors.Wrapf(err, errTemplateParsing, templateID)
-		return nil, nil, err
+		return nil, err
 	}
 
-	buf := new(bytes.Buffer)
-	if err = t.Execute(buf, hardware); err != nil {
+	var buf bytes.Buffer
+	if err := t.Execute(&buf, hardware); err != nil {
 		err = errors.Wrapf(err, errTemplateParsing, templateID)
-		return nil, nil, err
+		return nil, err
 	}
 
-	wf, err := Parse(buf.Bytes())
+	wf, err := parse(buf.Bytes())
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
+
 	for _, task := range wf.Tasks {
 		if task.WorkerAddr == "" {
-			return nil, nil, fmt.Errorf(errInvalidHardwareAddress, hardware)
+			return nil, fmt.Errorf("failed to render template, empty hardware address (%v)", hardware)
 		}
 	}
-	return wf, buf, nil
+
+	return wf, nil
 }
 
 // validate validates a workflow template against certain requirements.
 func validate(wf *Workflow) error {
-	if hasEmptyName(wf.Name) {
-		return errors.New(errEmptyName)
-	}
 	if !hasValidLength(wf.Name) {
 		return errors.Errorf(errInvalidLength, wf.Name)
 	}
 
 	if wf.Version != "0.1" {
-		return errors.Errorf(errTemplateInvalidVersion, wf.Version)
+		return errors.Errorf("invalid template version (%s)", wf.Version)
 	}
 
 	if len(wf.Tasks) == 0 {
@@ -124,52 +78,40 @@ func validate(wf *Workflow) error {
 
 	taskNameMap := make(map[string]struct{})
 	for _, task := range wf.Tasks {
-		if hasEmptyName(task.Name) {
-			return errors.New(errEmptyName)
-		}
 		if !hasValidLength(task.Name) {
 			return errors.Errorf(errInvalidLength, task.Name)
 		}
 
-		_, ok := taskNameMap[task.Name]
-		if ok {
-			return errors.Errorf(errTaskDuplicateName, task.Name)
+		if _, ok := taskNameMap[task.Name]; ok {
+			return errors.Errorf("two tasks in a template cannot have same name (%s)", task.Name)
 		}
 
 		taskNameMap[task.Name] = struct{}{}
 		actionNameMap := make(map[string]struct{})
 		for _, action := range task.Actions {
-			if hasEmptyName(action.Name) {
-				return errors.New(errEmptyName)
-			}
-
 			if !hasValidLength(action.Name) {
 				return errors.Errorf(errInvalidLength, action.Name)
 			}
 
-			if !hasValidImageName(action.Image) {
-				return errors.Errorf(errActionInvalidImage, action.Image)
+			if err := validateImageName(action.Image); err != nil {
+				return errors.Errorf("invalid action image (%s): %v", action.Image, err)
 			}
 
 			_, ok := actionNameMap[action.Name]
 			if ok {
-				return errors.Errorf(errActionDuplicateName, action.Name)
+				return errors.Errorf("two actions in a task cannot have same name: %s", action.Name)
 			}
 			actionNameMap[action.Name] = struct{}{}
 		}
 	}
 	return nil
 }
 
-func hasEmptyName(name string) bool {
-	return name == ""
-}
-
 func hasValidLength(name string) bool {
-	return len(name) < 200
+	return len(name) > 0 && len(name) < 200
 }
 
-func hasValidImageName(name string) bool {
+func validateImageName(name string) error {
 	_, err := reference.ParseNormalizedNamed(name)
-	return err == nil
+	return err
 }