clean: cleaned up code for getting/setting parameters.

pkg/types.go

@@ -2,6 +2,8 @@ package v1
 
 import (
 	"encoding/json"
+	"github.com/onepanelio/core/pkg/util/mapping"
+	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
 	"strings"
 	"time"
@@ -71,13 +73,13 @@ func (wt *WorkflowTemplate) GetManifestBytes() []byte {
 }
 
 func (wt *WorkflowTemplate) GetParametersKeyString() (map[string]string, error) {
-	mapping := make(map[interface{}]interface{})
+	root := make(map[interface{}]interface{})
 
-	if err := yaml.Unmarshal(wt.GetManifestBytes(), mapping); err != nil {
+	if err := yaml.Unmarshal(wt.GetManifestBytes(), root); err != nil {
 		return nil, err
 	}
 
-	arguments, ok := mapping["arguments"]
+	arguments, ok := root["arguments"]
 	if !ok {
 		return nil, nil
 	}
@@ -97,6 +99,10 @@ func (wt *WorkflowTemplate) GetParametersKeyString() (map[string]string, error)
 		return nil, nil
 	}
 
+	if len(parametersAsArray) == 0 {
+		delete(root, arguments)
+	}
+
 	result := make(map[string]string)
 	for index, parameter := range parametersAsArray {
 		parameterMap, ok := parameter.(map[interface{}]interface{})
@@ -146,6 +152,46 @@ func (wt *WorkflowTemplate) GetWorkflowManifestBytes() ([]byte, error) {
 	return json.Marshal(wt.ArgoWorkflowTemplate)
 }
 
+func (wt *WorkflowTemplate) FormatManifest() (string, error) {
+	manifestMap, err := mapping.NewFromYamlString(wt.Manifest)
+	if err != nil {
+		log.WithFields(log.Fields{
+			"Method": "FormatManifest",
+			"Step":   "NewFromYamlString",
+			"Error":  err.Error(),
+		}).Error("FormatManifest Workflow Template failed.")
+
+		return "", nil
+	}
+
+	manifestMap, err = manifestMap.GetChildMap("spec")
+	if err != nil {
+		log.WithFields(log.Fields{
+			"Method": "FormatManifest",
+			"Step":   "GetChildMap",
+			"Error":  err.Error(),
+		}).Error("GetChildMap Workflow Template failed.")
+
+		return "", nil
+	}
+	manifestMap.PruneEmpty()
+
+	if wt.ArgoWorkflowTemplate != nil {
+		AddWorkflowTemplateParametersFromAnnotations(manifestMap, wt.ArgoWorkflowTemplate.Annotations)
+	}
+
+	manifestBytes, err := manifestMap.ToYamlBytes()
+	if err != nil {
+		log.WithFields(log.Fields{
+			"Method": "FormatManifest",
+			"Step":   "ToYamlBytes",
+			"Error":  err.Error(),
+		}).Error("ToYamlBytes Workflow Template failed.")
+	}
+
+	return string(manifestBytes), nil
+}
+
 const (
 	WorfklowPending   WorkflowExecutionPhase = "Pending"
 	WorfklowRunning   WorkflowExecutionPhase = "Running"
@@ -273,38 +319,29 @@ func WrapSpecInK8s(data []byte) ([]byte, error) {
 	return finalBytes, nil
 }
 
-func RemoveAllButSpec(manifest []byte) (map[interface{}]interface{}, error) {
-	mapping := make(map[interface{}]interface{})
-
-	if err := yaml.Unmarshal(manifest, mapping); err != nil {
-		return nil, err
+func AddWorkflowTemplateParametersFromAnnotations(spec mapping.Mapping, annotations map[string]string) {
+	if spec == nil || len(annotations) == 0 {
+		return
 	}
 
-	deleteEmptyValuesMapping(mapping)
-
-	spec, ok := mapping["spec"].(map[interface{}]interface{})
-	if !ok {
-		return nil, nil
+	arguments, err := spec.GetChildMap("arguments")
+	if err != nil {
+		return
 	}
 
-	return spec, nil
-}
-
-func AddWorkflowTemplateParametersFromAnnotations(spec map[interface{}]interface{}, annotations map[string]string) {
-	// TODO put in parameters here, decoding as you go.
-	// @todo don't forget to clean up the code and centralize it somewhere.
-	// maybe we should have a manifest builder or something.
-
-	spec["arguments"] = make(map[interface{}]interface{})
-	arguments := spec["arguments"].(map[interface{}]interface{})
-	arguments["parameters"] = make([]interface{}, 0)
-	parameters := make([]interface{}, len(annotations))
+	parameters, err := arguments.GetChildMap("parameters")
+	if err != nil {
+		return
+	}
 
 	for _, value := range annotations {
-		data := make(map[interface{}]interface{})
-		err := yaml.Unmarshal([]byte(value), data)
+		data, err := mapping.NewFromYamlString(value)
 		if err != nil {
-			// todo log
+			log.WithFields(log.Fields{
+				"Method": "AddWorkflowTemplateParametersFromAnnotations",
+				"Step":   "NewFromYamlString",
+				"Error":  err.Error(),
+			}).Error("Error with AddWorkflowTemplateParametersFromAnnotations")
 			continue
 		}
 
@@ -313,55 +350,12 @@ func AddWorkflowTemplateParametersFromAnnotations(spec map[interface{}]interface
 		if ok {
 			order = orderValue.(int)
 			delete(data, "order")
+
+			if order >= 0 && order < len(parameters) {
 				parameters[order] = data
 			}
 		}
+	}
 
 	arguments["parameters"] = parameters
 }
-
-// Returns the number of keys in the map
-func deleteEmptyValuesMapping(mapping map[interface{}]interface{}) int {
-	keys := 0
-	for key, value := range mapping {
-		keys++
-		valueAsMapping, ok := value.(map[interface{}]interface{})
-		if ok {
-			if deleteEmptyValuesMapping(valueAsMapping) == 0 {
-				delete(mapping, key)
-			}
-		}
-
-		valueAsArray, ok := value.([]interface{})
-		if ok {
-			deleteEmptyValuesArray(valueAsArray)
-		}
-
-		valueAsString, ok := value.(string)
-		if ok && valueAsString == "" {
-			delete(mapping, key)
-		}
-	}
-
-	return keys
-}
-
-// Returns the number of items in the array.
-func deleteEmptyValuesArray(values []interface{}) int {
-	count := 0
-	for _, value := range values {
-		count++
-
-		valueAsMapping, ok := value.(map[interface{}]interface{})
-		if ok {
-			deleteEmptyValuesMapping(valueAsMapping)
-		}
-
-		valueAsArray, ok := value.([]interface{})
-		if ok {
-			deleteEmptyValuesArray(valueAsArray)
-		}
-	}
-
-	return count
-}

pkg/util/mapping/mapping.go

@@ -0,0 +1,109 @@
+package mapping
+
+import (
+	"fmt"
+	"gopkg.in/yaml.v2"
+)
+
+type Mapping map[interface{}]interface{}
+
+func New() Mapping {
+	return make(Mapping)
+}
+
+func NewFromYamlBytes(data []byte) (Mapping, error) {
+	mapping := New()
+
+	if err := yaml.Unmarshal(data, mapping); err != nil {
+		return nil, err
+	}
+
+	return mapping, nil
+}
+
+func NewFromYamlString(data string) (Mapping, error) {
+	return NewFromYamlBytes([]byte(data))
+}
+
+// Marshals the mapping using yaml
+// if mapping is nil, an empty byte array is returned.
+func (m Mapping) ToYamlBytes() ([]byte, error) {
+	if m == nil {
+		return make([]byte, 0), nil
+	}
+
+	return yaml.Marshal(m)
+}
+
+// Removes all the empty values in the mapping, going in depth.
+func (m Mapping) PruneEmpty() {
+	if m == nil {
+		return
+	}
+
+	deleteEmptyValuesMapping(m)
+}
+
+// Attempts the get the Mapping under key.
+// If it is not a mapping, an error is returned.
+// If it does not exist, it is created and returned.
+func (m Mapping) GetChildMap(key interface{}) (Mapping, error) {
+	subMapping, ok := m[key]
+	if !ok {
+		subMapping = New()
+		m[key] = subMapping
+	}
+
+	asMap, ok := subMapping.(Mapping)
+	if !ok {
+		return nil, fmt.Errorf("%v is not a Mapping", key)
+	}
+
+	return asMap, nil
+}
+
+// Returns the number of keys in the map
+func deleteEmptyValuesMapping(mapping Mapping) int {
+	keys := 0
+	for key, value := range mapping {
+		keys++
+		valueAsMapping, ok := value.(Mapping)
+		if ok {
+			if deleteEmptyValuesMapping(valueAsMapping) == 0 {
+				delete(mapping, key)
+			}
+		}
+
+		valueAsArray, ok := value.([]interface{})
+		if ok {
+			deleteEmptyValuesArray(valueAsArray)
+		}
+
+		valueAsString, ok := value.(string)
+		if ok && valueAsString == "" {
+			delete(mapping, key)
+		}
+	}
+
+	return keys
+}
+
+// Returns the number of items in the array.
+func deleteEmptyValuesArray(values []interface{}) int {
+	count := 0
+	for _, value := range values {
+		count++
+
+		valueAsMapping, ok := value.(Mapping)
+		if ok {
+			deleteEmptyValuesMapping(valueAsMapping)
+		}
+
+		valueAsArray, ok := value.([]interface{})
+		if ok {
+			deleteEmptyValuesArray(valueAsArray)
+		}
+	}
+
+	return count
+}

server/workflow_server.go

@@ -4,9 +4,7 @@ import (
 	"context"
 	"github.com/onepanelio/core/pkg/util"
 	"github.com/onepanelio/core/server/converter"
-	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
-	"gopkg.in/yaml.v2"
 	"math"
 	"sort"
 	"strings"
@@ -46,35 +44,7 @@ func apiWorkflowExecution(wf *v1.WorkflowExecution) (workflow *api.WorkflowExecu
 	}
 
 	if wf.WorkflowTemplate != nil {
-		wftManifestMap, err := v1.RemoveAllButSpec(wf.WorkflowTemplate.GetManifestBytes())
-		if err != nil {
-			log.WithFields(log.Fields{
-				"Method": "apiWorkflowExecution",
-				"Step":   "RemoveAllButSpec",
-				"Error":  err.Error(),
-			}).Error("Invalid status.")
-		}
-		if wf.WorkflowTemplate.ArgoWorkflowTemplate != nil {
-			v1.AddWorkflowTemplateParametersFromAnnotations(wftManifestMap, wf.WorkflowTemplate.ArgoWorkflowTemplate.Annotations)
-		}
-
-		manifestBytes := []byte{}
-		if wftManifestMap != nil {
-			manifestBytes, err = yaml.Marshal(wftManifestMap)
-			if err != nil {
-				// todo log
-			}
-		}
-
-		workflow.WorkflowTemplate = &api.WorkflowTemplate{
-			Uid:        wf.WorkflowTemplate.UID,
-			CreatedAt:  wf.WorkflowTemplate.CreatedAt.UTC().Format(time.RFC3339),
-			Name:       wf.WorkflowTemplate.Name,
-			Version:    wf.WorkflowTemplate.Version,
-			Manifest:   string(manifestBytes),
-			IsLatest:   wf.WorkflowTemplate.IsLatest,
-			IsArchived: wf.WorkflowTemplate.IsArchived,
-		}
+		workflow.WorkflowTemplate = apiWorkflowTemplate(wf.WorkflowTemplate)
 	}
 
 	return

server/workflow_template_server.go

@@ -8,8 +8,6 @@ import (
 	"github.com/onepanelio/core/pkg/util/label"
 	"github.com/onepanelio/core/server/auth"
 	"github.com/onepanelio/core/server/converter"
-	log "github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v2"
 	"strings"
 	"time"
 )
@@ -21,24 +19,9 @@ func NewWorkflowTemplateServer() *WorkflowTemplateServer {
 }
 
 func apiWorkflowTemplate(wft *v1.WorkflowTemplate) *api.WorkflowTemplate {
-	manifestMap, err := v1.RemoveAllButSpec(wft.GetManifestBytes())
+	manifest, err := wft.FormatManifest()
 	if err != nil {
-		log.WithFields(log.Fields{
-			"Method": "apiWorkflowTemplate",
-			"Step":   "RemoveAllButSpec",
-			"Error":  err.Error(),
-		}).Error("Get Workflow Template failed.")
-	}
-
-	if wft.ArgoWorkflowTemplate != nil {
-		v1.AddWorkflowTemplateParametersFromAnnotations(manifestMap, wft.ArgoWorkflowTemplate.Annotations)
-	}
-	manifestBytes := []byte{}
-	if manifestMap != nil {
-		manifestBytes, err = yaml.Marshal(manifestMap)
-		if err != nil {
-			// todo log it
-		}
+		manifest = ""
 	}
 
 	return &api.WorkflowTemplate{
@@ -46,7 +29,7 @@ func apiWorkflowTemplate(wft *v1.WorkflowTemplate) *api.WorkflowTemplate {
 		CreatedAt:  wft.CreatedAt.UTC().Format(time.RFC3339),
 		Name:       wft.Name,
 		Version:    wft.Version,
-		Manifest:   string(manifestBytes),
+		Manifest:   manifest,
 		IsLatest:   wft.IsLatest,
 		IsArchived: wft.IsArchived,
 	}