General clean up to the /internal/workflow pkg (#668)

The `/internal/workflow` package contains data structures for workflow handling that were previously split across the code base. This change applies some best practices by unexporting unnecessarily exported names and tweaks some algorithms to simplify logic. Finally it removes functions that aren't used anywhere in the project.

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
 }