feat(api): return more info about workflow template parsing and execu…

…tion errors (#3850)

* feat(api): return more info about workflow template parsing and execution errors

* feat(api,ui): return error for template parsing with data

* feat(ui): display template error in code with message

* fix(api): use multierror instead of error slice

* fix(ui,api): returns template parsing error for push and add removable args on template ui editor

* fix(api): code review

* fix(api): code review

engine/api/templates.go

@@ -155,6 +155,11 @@ func (api *API) postTemplateHandler() service.Handler {
 			return err
 		}
 
+		// execute template with no instance only to check if parsing is ok
+		if _, err := workflowtemplate.Execute(&t, nil); err != nil {
+			return err
+		}
+
 		// duplicate couple of group id and slug will failed with sql constraint
 		if err := workflowtemplate.Insert(api.mustDB(), &t); err != nil {
 			return err
@@ -232,6 +237,11 @@ func (api *API) putTemplateHandler() service.Handler {
 		new := sdk.WorkflowTemplate(*old)
 		new.Update(data)
 
+		// execute template with no instance only to check if parsing is ok
+		if _, err := workflowtemplate.Execute(&new, nil); err != nil {
+			return err
+		}
+
 		if err := workflowtemplate.Update(api.mustDB(), &new); err != nil {
 			return err
 		}

engine/api/workflowtemplate/execute.go

@@ -6,6 +6,8 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
+	"regexp"
+	"strconv"
 	"strings"
 	"text/template"
 
@@ -38,85 +40,156 @@ func prepareParams(wt *sdk.WorkflowTemplate, r sdk.WorkflowTemplateRequest) inte
 	return m
 }
 
-func executeTemplate(t string, data map[string]interface{}) (string, error) {
-	tmpl, err := template.New(fmt.Sprintf("template")).Delims("[[", "]]").Parse(t)
+func parseTemplate(templateType string, number int, t string) (*template.Template, error) {
+	var id string
+	switch templateType {
+	case "workflow":
+		id = templateType
+	default:
+		id = fmt.Sprintf("%s.%d", templateType, number)
+	}
+
+	tmpl, err := template.New(id).Delims("[[", "]]").Parse(t)
 	if err != nil {
-		return "", sdk.WrapError(err, "cannot parse workflow template")
+		reg := regexp.MustCompile(`template: ([0-9a-zA-Z.]+):([0-9]+): (.*)$`)
+		submatch := reg.FindStringSubmatch(err.Error())
+		if len(submatch) != 4 {
+			return nil, sdk.WithStack(err)
+		}
+		line, err := strconv.Atoi(submatch[2])
+		if err != nil {
+			return nil, sdk.WithStack(err)
+		}
+		return nil, sdk.WithStack(sdk.WorkflowTemplateError{
+			Type:    templateType,
+			Number:  number,
+			Line:    line,
+			Message: submatch[3],
+		})
 	}
+	return tmpl, nil
+}
 
+func executeTemplate(tmpl *template.Template, data map[string]interface{}) (string, error) {
 	var buffer bytes.Buffer
 	if err := tmpl.Execute(&buffer, data); err != nil {
-		return "", sdk.WrapError(err, "cannot execute workflow template")
+		return "", sdk.WithStack(err)
 	}
-
 	return buffer.String(), nil
 }
 
+func decodeTemplateValue(value string) (string, error) {
+	v, err := base64.StdEncoding.DecodeString(value)
+	if err != nil {
+		return "", sdk.NewError(sdk.ErrWrongRequest, err)
+	}
+	return string(v), nil
+}
+
 // Execute returns yaml file from template.
-func Execute(wt *sdk.WorkflowTemplate, i *sdk.WorkflowTemplateInstance) (sdk.WorkflowTemplateResult, error) {
-	data := map[string]interface{}{
-		"id":     i.ID,
-		"name":   i.Request.WorkflowName,
-		"params": prepareParams(wt, i.Request),
+func Execute(wt *sdk.WorkflowTemplate, instance *sdk.WorkflowTemplateInstance) (sdk.WorkflowTemplateResult, error) {
+	result := sdk.WorkflowTemplateResult{
+		Pipelines:    make([]string, len(wt.Pipelines)),
+		Applications: make([]string, len(wt.Applications)),
+		Environments: make([]string, len(wt.Environments)),
 	}
 
-	v, err := base64.StdEncoding.DecodeString(wt.Value)
-	if err != nil {
-		return sdk.WorkflowTemplateResult{}, sdk.WrapError(err, "cannot parse workflow template")
+	var data map[string]interface{}
+	if instance != nil {
+		data = map[string]interface{}{
+			"id":     instance.ID,
+			"name":   instance.Request.WorkflowName,
+			"params": prepareParams(wt, instance.Request),
+		}
 	}
 
-	out, err := executeTemplate(string(v), data)
+	var multiErr sdk.MultiError
+
+	v, err := decodeTemplateValue(wt.Value)
 	if err != nil {
-		return sdk.WorkflowTemplateResult{}, err
+		return result, err
 	}
-
-	res := sdk.WorkflowTemplateResult{
-		Workflow:     out,
-		Pipelines:    make([]string, len(wt.Pipelines)),
-		Applications: make([]string, len(wt.Applications)),
-		Environments: make([]string, len(wt.Environments)),
+	if tmpl, err := parseTemplate("workflow", 0, v); err != nil {
+		multiErr.Append(err)
+	} else {
+		if data != nil {
+			result.Workflow, err = executeTemplate(tmpl, data)
+			if err != nil {
+				return result, err
+			}
+		}
 	}
 
 	for i, p := range wt.Pipelines {
-		v, err := base64.StdEncoding.DecodeString(p.Value)
+		v, err := decodeTemplateValue(p.Value)
 		if err != nil {
-			return sdk.WorkflowTemplateResult{}, sdk.WrapError(err, "cannot parse pipeline template")
+			return result, err
 		}
 
-		out, err := executeTemplate(string(v), data)
-		if err != nil {
-			return sdk.WorkflowTemplateResult{}, err
+		if tmpl, err := parseTemplate("pipeline", i, v); err != nil {
+			multiErr.Append(err)
+		} else {
+			result.Pipelines[i], err = executeTemplate(tmpl, data)
+			if err != nil {
+				return result, err
+			}
 		}
-		res.Pipelines[i] = out
 	}
 
 	for i, a := range wt.Applications {
-		v, err := base64.StdEncoding.DecodeString(a.Value)
+		v, err := decodeTemplateValue(a.Value)
 		if err != nil {
-			return sdk.WorkflowTemplateResult{}, sdk.WrapError(err, "cannot parse application template")
+			return result, err
 		}
 
-		out, err := executeTemplate(string(v), data)
-		if err != nil {
-			return sdk.WorkflowTemplateResult{}, err
+		if tmpl, err := parseTemplate("application", i, v); err != nil {
+			multiErr.Append(err)
+		} else {
+			if data != nil {
+				result.Applications[i], err = executeTemplate(tmpl, data)
+				if err != nil {
+					return result, err
+				}
+			}
 		}
-		res.Applications[i] = out
 	}
 
 	for i, e := range wt.Environments {
-		v, err := base64.StdEncoding.DecodeString(e.Value)
+		v, err := decodeTemplateValue(e.Value)
 		if err != nil {
-			return sdk.WorkflowTemplateResult{}, sdk.WrapError(err, "cannot parse environment template")
+			return result, err
 		}
 
-		out, err := executeTemplate(string(v), data)
-		if err != nil {
-			return sdk.WorkflowTemplateResult{}, err
+		if tmpl, err := parseTemplate("environment", i, v); err != nil {
+			multiErr.Append(err)
+		} else {
+			if data != nil {
+				result.Environments[i], err = executeTemplate(tmpl, data)
+				if err != nil {
+					return result, err
+				}
+			}
+		}
+	}
+
+	if !multiErr.IsEmpty() {
+		var errs []sdk.WorkflowTemplateError
+		causes := make([]string, len(multiErr))
+		for i, err := range multiErr {
+			cause := sdk.Cause(err)
+			if e, ok := cause.(sdk.WorkflowTemplateError); ok {
+				errs = append(errs, e)
+			}
+			causes[i] = cause.Error()
 		}
-		res.Environments[i] = out
+		return result, sdk.NewErrorFrom(sdk.Error{
+			ID:     sdk.ErrCannotParseTemplate.ID,
+			Status: sdk.ErrCannotParseTemplate.Status,
+			Data:   errs,
+		}, strings.Join(causes, ", "))
 	}
 
-	return res, nil
+	return result, nil
 }
 
 // Tar returns in buffer the a tar file that contains all generated stuff in template result.

engine/api/workflowtemplate/execute_test.go

@@ -124,3 +124,57 @@ values:
 		type: string
 		value: value1`, res.Environments[0])
 }
+
+func TestExecuteTemplateWithError(t *testing.T) {
+	tmpl := &sdk.WorkflowTemplate{
+		ID: 42,
+		Parameters: []sdk.WorkflowTemplateParameter{
+			{Key: "withDeploy", Type: sdk.ParameterTypeBoolean, Required: true},
+			{Key: "deployWhen", Type: sdk.ParameterTypeString},
+			{Key: "repo", Type: sdk.ParameterTypeRepository},
+		},
+		Value: base64.StdEncoding.EncodeToString([]byte(`
+name: [[.name]
+description: Test simple workflow with error
+version: v1.0`)),
+		Pipelines: []sdk.PipelineTemplate{{
+			Value: base64.StdEncoding.EncodeToString([]byte(`
+version: v1.0
+name: Pipeline-[[error .id]]
+stages:
+- Stage 1`)),
+		}},
+		Applications: []sdk.ApplicationTemplate{{
+			Value: base64.StdEncoding.EncodeToString([]byte(`
+version: v1.0
+name: [[`)),
+		}},
+		Environments: []sdk.EnvironmentTemplate{{
+			Value: base64.StdEncoding.EncodeToString([]byte(`
+name: Environment-[[if .id]]`)),
+		}},
+	}
+
+	_, err := workflowtemplate.Execute(tmpl, nil)
+	assert.NotNil(t, err)
+	e := sdk.ExtractHTTPError(err, "")
+	assert.Equal(t, sdk.ErrCannotParseTemplate.ID, e.ID)
+	errs := []sdk.WorkflowTemplateError{{
+		Type:    "workflow",
+		Line:    2,
+		Message: "unexpected \"]\" in operand",
+	}, {
+		Type:    "pipeline",
+		Line:    3,
+		Message: "function \"error\" not defined",
+	}, {
+		Type:    "application",
+		Line:    3,
+		Message: "unexpected unclosed action in command",
+	}, {
+		Type:    "environment",
+		Line:    2,
+		Message: "unexpected EOF",
+	}}
+	assert.Equal(t, errs, e.Data)
+}

engine/api/workflowtemplate/import.go

@@ -108,6 +108,11 @@ func Push(db gorp.SqlExecutor, u *sdk.User, tr *tar.Reader) ([]sdk.Message, *sdk
 	new := sdk.WorkflowTemplate(*old)
 	new.Update(wt)
 
+	// execute template with no instance only to check if parsing is ok
+	if _, err := Execute(&new, nil); err != nil {
+		return nil, nil, err
+	}
+
 	if err := Update(db, &new); err != nil {
 		return nil, nil, err
 	}

sdk/error.go

@@ -173,6 +173,7 @@ var (
 	ErrWorkflowNodeRootUpdate                 = Error{ID: 156, Status: http.StatusBadRequest}
 	ErrWorkflowAlreadyAsCode                  = Error{ID: 157, Status: http.StatusBadRequest}
 	ErrNoDBMigrationID                        = Error{ID: 158, Status: http.StatusNotFound}
+	ErrCannotParseTemplate                    = Error{ID: 159, Status: http.StatusBadRequest}
 )
 
 var errorsAmericanEnglish = map[int]string{
@@ -328,6 +329,7 @@ var errorsAmericanEnglish = map[int]string{
 	ErrWorkflowNodeRootUpdate.ID:                 "Unable to update/delete the root node of your workflow",
 	ErrWorkflowAlreadyAsCode.ID:                  "Workflow is already as-code or there is already a pull-request to transform it",
 	ErrNoDBMigrationID.ID:                        "ID does not exist in table gorp_migration",
+	ErrCannotParseTemplate.ID:                    "Cannot parse workflow template",
 }
 
 var errorsFrench = map[int]string{
@@ -483,6 +485,7 @@ var errorsFrench = map[int]string{
 	ErrWorkflowNodeRootUpdate.ID:                 "Impossible de mettre à jour ou supprimer le noeud racine du workflow",
 	ErrWorkflowAlreadyAsCode.ID:                  "Le workflow est déjà as-code ou il y a déjà une pull-request pour le transformer",
 	ErrNoDBMigrationID.ID:                        "Cet id n'existe pas dans la table gorp_migrations",
+	ErrCannotParseTemplate.ID:                    "Impossible de parser le modèle de workflow",
 }
 
 var errorsLanguages = []map[int]string{
@@ -492,11 +495,12 @@ var errorsLanguages = []map[int]string{
 
 // Error type.
 type Error struct {
-	ID         int    `json:"id"`
-	Status     int    `json:"-"`
-	Message    string `json:"message"`
-	UUID       string `json:"uuid,omitempty"`
-	StackTrace string `json:"stack_trace,omitempty"`
+	ID         int         `json:"id"`
+	Status     int         `json:"-"`
+	Message    string      `json:"message"`
+	Data       interface{} `json:"data"`
+	UUID       string      `json:"uuid,omitempty"`
+	StackTrace string      `json:"stack_trace,omitempty"`
 	from       string
 }
 

sdk/workflow_template.go

@@ -3,6 +3,7 @@ package sdk
 import (
 	"database/sql/driver"
 	json "encoding/json"
+	"fmt"
 	"strings"
 
 	"github.com/ovh/cds/sdk/slug"
@@ -352,3 +353,15 @@ func WorkflowTemplateInstancesToWorkflowTemplateIDs(wtis []*WorkflowTemplateInst
 	}
 	return ids
 }
+
+// WorkflowTemplateError contains info about template parsing error.
+type WorkflowTemplateError struct {
+	Type    string `json:"type"`
+	Number  int    `json:"number"`
+	Line    int    `json:"line"`
+	Message string `json:"message"`
+}
+
+func (w WorkflowTemplateError) Error() string {
+	return fmt.Sprintf("error '%s' in %s.%d at line %d", w.Message, w.Type, w.Number, w.Line)
+}

ui/src/app/model/workflow-template.model.ts

@@ -53,3 +53,10 @@ export class WorkflowTemplateInstance {
     workflow_template_version: number;
     request: WorkflowTemplateRequest;
 }
+
+export class WorkflowTemplateError {
+    type: string;
+    number: number;
+    line: number;
+    message: string;
+}

ui/src/app/shared/diff/item/diff.item.component.ts

@@ -12,8 +12,8 @@ export class Mode {
     styleUrls: ['./diff.item.scss']
 })
 export class DiffItemComponent implements OnChanges {
-    @ViewChild('codeLeft') codeLeft;
-    @ViewChild('codeRight') codeRight;
+    @ViewChild('codeLeft') codeLeft: any;
+    @ViewChild('codeRight') codeRight: any;
     @Input() original: string;
     @Input() updated: string;
     @Input() mode: Mode = Mode.UNIFIED;

ui/src/app/views/admin/hook-task/show/hook-task.show.component.ts

@@ -90,7 +90,7 @@ export class HookTaskShowComponent {
     }
 
     selectExecution(e: TaskExecution) {
-        return _ => {
+        return () => {
             this.selectedExecution = e
             this.selectedExecutionBody = null;
             if (e.webhook) {