config: refactor config unmarshal

* Don't use a complex UnmarshalJSON for Task but use specific UnmarshalJSON for
every type that requires custom unmarshaling

internal/config/config.go

@@ -108,11 +108,11 @@ type Task struct {
 	WorkingDir           string                         `json:"working_dir"`
 	Shell                string                         `json:"shell"`
 	User                 string                         `json:"user"`
-	Steps                []interface{}                  `json:"steps"`
+	Steps                Steps                          `json:"steps"`
-	Depends              []*Depend                      `json:"depends"`
+	Depends              Depends                        `json:"depends"`
 	IgnoreFailure        bool                           `json:"ignore_failure"`
 	Approval             bool                           `json:"approval"`
-	When                 *types.When                    `json:"when"`
+	When                 *When                          `json:"when"`
 	DockerRegistriesAuth map[string]*DockerRegistryAuth `json:"docker_registries_auth"`
 }
 
@@ -124,22 +124,29 @@ const (
 	DependConditionOnSkipped DependCondition = "on_skipped"
 )
 
+type Depends []*Depend
+
 type Depend struct {
 	TaskName   string            `json:"task"`
 	Conditions []DependCondition `json:"conditions"`
 }
 
-type Step struct {
+type Step interface{}
+
+type Steps []Step
+
+type BaseStep struct {
 	Type string `json:"type"`
 	Name string `json:"name"`
+	When *When  `json:"when"`
 }
 
 type CloneStep struct {
-	Step `json:",inline"`
+	BaseStep `json:",inline"`
 }
 
 type RunStep struct {
-	Step        `json:",inline"`
+	BaseStep    `json:",inline"`
 	Command     string           `json:"command"`
 	Environment map[string]Value `json:"environment,omitempty"`
 	WorkingDir  string           `json:"working_dir"`
@@ -147,16 +154,26 @@ type RunStep struct {
 	User        string           `json:"user"`
 }
 
-type ValueType int
+type SaveToWorkspaceStep struct {
+	BaseStep `json:",inline"`
+	Contents []*SaveContent `json:"contents"`
+}
 
-const (
+type RestoreWorkspaceStep struct {
-	ValueTypeString ValueType = iota
+	BaseStep `json:",inline"`
-	ValueTypeFromVariable
+	DestDir  string `json:"dest_dir"`
-)
+}
 
-type Value struct {
+type SaveCacheStep struct {
-	Type  ValueType
+	BaseStep `json:",inline"`
-	Value string
+	Key      string         `json:"key"`
+	Contents []*SaveContent `json:"contents"`
+}
+
+type RestoreCacheStep struct {
+	BaseStep `json:",inline"`
+	Keys     []string `json:"keys"`
+	DestDir  string   `json:"dest_dir"`
 }
 
 type SaveContent struct {
@@ -165,141 +182,101 @@ type SaveContent struct {
 	Paths     []string `json:"paths"`
 }
 
-type SaveToWorkspaceStep struct {
+func (s *Steps) UnmarshalJSON(b []byte) error {
-	Step     `json:",inline"`
+	var stepsRaw []json.RawMessage
-	Contents []*SaveContent `json:"contents"`
+	if err := json.Unmarshal(b, &stepsRaw); err != nil {
-}
-
-type RestoreWorkspaceStep struct {
-	Step    `json:",inline"`
-	DestDir string `json:"dest_dir"`
-}
-
-type SaveCacheStep struct {
-	Step     `json:",inline"`
-	Key      string         `json:"key"`
-	Contents []*SaveContent `json:"contents"`
-}
-
-type RestoreCacheStep struct {
-	Step    `json:",inline"`
-	Keys    []string `json:"keys"`
-	DestDir string   `json:"dest_dir"`
-}
-
-func (t *Task) UnmarshalJSON(b []byte) error {
-	type when struct {
-		Branch interface{} `json:"branch"`
-		Tag    interface{} `json:"tag"`
-		Ref    interface{} `json:"ref"`
-	}
-
-	type runtask struct {
-		Name                 string                         `json:"name"`
-		Runtime              *Runtime                       `json:"runtime"`
-		Environment          map[string]Value               `json:"environment,omitempty"`
-		WorkingDir           string                         `json:"working_dir"`
-		Shell                string                         `json:"shell"`
-		User                 string                         `json:"user"`
-		Steps                []map[string]interface{}       `json:"steps"`
-		Depends              []interface{}                  `json:"depends"`
-		IgnoreFailure        bool                           `json:"ignore_failure"`
-		Approval             bool                           `json:"approval"`
-		When                 *when                          `json:"when"`
-		DockerRegistriesAuth map[string]*DockerRegistryAuth `json:"docker_registries_auth"`
-	}
-
-	var tr *runtask
-
-	if err := json.Unmarshal(b, &tr); err != nil {
 		return err
 	}
 
-	t.Name = tr.Name
+	steps := make(Steps, len(stepsRaw))
-	t.Runtime = tr.Runtime
+	for i, stepRaw := range stepsRaw {
-	t.Environment = tr.Environment
+		var step interface{}
-	t.WorkingDir = tr.WorkingDir
-	t.Shell = tr.Shell
-	t.User = tr.User
-	t.IgnoreFailure = tr.IgnoreFailure
-	t.Approval = tr.Approval
-	t.DockerRegistriesAuth = tr.DockerRegistriesAuth
 
-	steps := make([]interface{}, len(tr.Steps))
+		var stepMap map[string]json.RawMessage
-	for i, stepEntry := range tr.Steps {
+		if err := json.Unmarshal(stepRaw, &stepMap); err != nil {
-		if _, ok := stepEntry["type"]; ok {
+			return err
-			// handle default step definition using format { type: "steptype", other steps fields }
+		}
-			stepType, ok := stepEntry["type"].(string)
+		// handle default step definition using format { type: "steptype", other steps fields }
+		if _, ok := stepMap["type"]; ok {
+			var stepTypeI interface{}
+			if err := json.Unmarshal(stepMap["type"], &stepTypeI); err != nil {
+				return err
+			}
+			stepType, ok := stepTypeI.(string)
 			if !ok {
 				return errors.Errorf("step type at index %d must be a string", i)
 			}
-			o, err := json.Marshal(stepEntry)
+
-			if err != nil {
-				return err
-			}
 			switch stepType {
 			case "clone":
 				var s CloneStep
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
+					return err
+				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 
 			case "run":
 				var s RunStep
-				if err := json.Unmarshal(o, &s); err != nil {
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
 					return err
 				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 
 			case "save_to_workspace":
 				var s SaveToWorkspaceStep
-				if err := json.Unmarshal(o, &s); err != nil {
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
 					return err
 				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 
 			case "restore_workspace":
 				var s RestoreWorkspaceStep
-				if err := json.Unmarshal(o, &s); err != nil {
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
 					return err
 				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 
 			case "save_cache":
 				var s SaveCacheStep
-				if err := json.Unmarshal(o, &s); err != nil {
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
 					return err
 				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 
 			case "restore_cache":
 				var s RestoreCacheStep
-				if err := json.Unmarshal(o, &s); err != nil {
+				if err := json.Unmarshal(stepRaw, &s); err != nil {
 					return err
 				}
 				s.Type = stepType
-				steps[i] = &s
+				step = &s
 			default:
 				return errors.Errorf("unknown step type: %s", stepType)
 			}
 		} else {
 			// handle simpler (for yaml not for json) steps definition using format "steptype": { stepSpecification }
-			if len(stepEntry) > 1 {
+			if len(stepMap) > 1 {
 				return errors.Errorf("wrong steps description at index %d: more than one step name per list entry", i)
 			}
-			for stepType, stepSpec := range stepEntry {
+			for stepType, stepSpecRaw := range stepMap {
-				o, err := json.Marshal(stepSpec)
+				var stepSpec interface{}
-				if err != nil {
+				if err := json.Unmarshal(stepSpecRaw, &stepSpec); err != nil {
 					return err
 				}
+
 				switch stepType {
 				case "clone":
 					var s CloneStep
+					if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
+						return err
+					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 
 				case "run":
 					var s RunStep
@@ -307,62 +284,78 @@ func (t *Task) UnmarshalJSON(b []byte) error {
 					case string:
 						s.Command = stepSpec.(string)
 					default:
-						if err := json.Unmarshal(o, &s); err != nil {
+						if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
 							return err
 						}
 					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 
 				case "save_to_workspace":
 					var s SaveToWorkspaceStep
-					if err := json.Unmarshal(o, &s); err != nil {
+					if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
 						return err
 					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 
 				case "restore_workspace":
 					var s RestoreWorkspaceStep
-					if err := json.Unmarshal(o, &s); err != nil {
+					if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
 						return err
 					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 
 				case "save_cache":
 					var s SaveCacheStep
-					if err := json.Unmarshal(o, &s); err != nil {
+					if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
 						return err
 					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 
 				case "restore_cache":
 					var s RestoreCacheStep
-					if err := json.Unmarshal(o, &s); err != nil {
+					if err := json.Unmarshal(stepSpecRaw, &s); err != nil {
 						return err
 					}
 					s.Type = stepType
-					steps[i] = &s
+					step = &s
 				default:
 					return errors.Errorf("unknown step type: %s", stepType)
 				}
 			}
 		}
+
+		steps[i] = step
 	}
 
-	t.Steps = steps
+	*s = steps
 
-	depends := make([]*Depend, len(tr.Depends))
+	return nil
-	for i, dependEntry := range tr.Depends {
+}
+
+func (d *Depends) UnmarshalJSON(b []byte) error {
+	var dependsRaw []json.RawMessage
+
+	if err := json.Unmarshal(b, &dependsRaw); err != nil {
+		return err
+	}
+
+	depends := make([]*Depend, len(dependsRaw))
+	for i, dependRaw := range dependsRaw {
+		var dependi interface{}
+		if err := json.Unmarshal(dependRaw, &dependi); err != nil {
+			return err
+		}
 		var depend *Depend
 		isSimpler := false
-		switch de := dependEntry.(type) {
+		switch de := dependi.(type) {
 		// handle simpler (for yaml) depends definition using format "taskname":
 		case string:
 			depend = &Depend{
-				TaskName: dependEntry.(string),
+				TaskName: dependi.(string),
 			}
 		case map[string]interface{}:
 			if len(de) == 1 {
@@ -378,11 +371,7 @@ func (t *Task) UnmarshalJSON(b []byte) error {
 			}
 			if !isSimpler {
 				// handle default depends definition using format "task": "taskname", conditions: [ list of conditions ]
-				o, err := json.Marshal(dependEntry)
+				if err := json.Unmarshal(dependRaw, &depend); err != nil {
-				if err != nil {
-					return err
-				}
-				if err := json.Unmarshal(o, &depend); err != nil {
 					return err
 				}
 			} else {
@@ -392,11 +381,7 @@ func (t *Task) UnmarshalJSON(b []byte) error {
 				}
 				type deplist map[string][]DependCondition
 				var dl deplist
-				o, err := json.Marshal(dependEntry)
+				if err := json.Unmarshal(dependRaw, &dl); err != nil {
-				if err != nil {
-					return err
-				}
-				if err := json.Unmarshal(o, &dl); err != nil {
 					return err
 				}
 				if len(dl) != 1 {
@@ -416,40 +401,23 @@ func (t *Task) UnmarshalJSON(b []byte) error {
 		depends[i] = depend
 	}
 
-	t.Depends = depends
+	*d = depends
-
-	if tr.When != nil {
-		w := &types.When{}
-
-		var err error
-
-		if tr.When.Branch != nil {
-			w.Branch, err = parseWhenConditions(tr.When.Branch)
-			if err != nil {
-				return err
-			}
-		}
-
-		if tr.When.Tag != nil {
-			w.Tag, err = parseWhenConditions(tr.When.Tag)
-			if err != nil {
-				return err
-			}
-		}
-
-		if tr.When.Ref != nil {
-			w.Ref, err = parseWhenConditions(tr.When.Ref)
-			if err != nil {
-				return err
-			}
-		}
-
-		t.When = w
-	}
 
 	return nil
 }
 
+type ValueType int
+
+const (
+	ValueTypeString ValueType = iota
+	ValueTypeFromVariable
+)
+
+type Value struct {
+	Type  ValueType
+	Value string
+}
+
 func (val *Value) UnmarshalJSON(b []byte) error {
 	var ival interface{}
 	if err := json.Unmarshal(b, &ival); err != nil {
@@ -477,6 +445,46 @@ func (val *Value) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+type When types.When
+
+type when struct {
+	Branch interface{} `json:"branch"`
+	Tag    interface{} `json:"tag"`
+	Ref    interface{} `json:"ref"`
+}
+
+func (w *When) UnmarshalJSON(b []byte) error {
+	var wi *when
+	if err := json.Unmarshal(b, &wi); err != nil {
+		return err
+	}
+
+	var err error
+
+	if wi.Branch != nil {
+		w.Branch, err = parseWhenConditions(wi.Branch)
+		if err != nil {
+			return err
+		}
+	}
+
+	if wi.Tag != nil {
+		w.Tag, err = parseWhenConditions(wi.Tag)
+		if err != nil {
+			return err
+		}
+	}
+
+	if wi.Ref != nil {
+		w.Ref, err = parseWhenConditions(wi.Ref)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func parseWhenConditions(wi interface{}) (*types.WhenConditions, error) {
 	w := &types.WhenConditions{}
 

internal/config/config_test.go

@@ -307,24 +307,24 @@ func TestParseOutput(t *testing.T) {
 								WorkingDir: defaultWorkingDir,
 								Shell:      "",
 								User:       "",
-								Steps: []interface{}{
+								Steps: Steps{
-									&CloneStep{Step: Step{Type: "clone"}},
+									&CloneStep{BaseStep: BaseStep{Type: "clone"}},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "command01",
 										},
 										Command: "command01",
 									},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "name different than command",
 										},
 										Command: "command02",
 									},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "command03",
 										},
@@ -335,27 +335,27 @@ func TestParseOutput(t *testing.T) {
 										},
 									},
 									&SaveCacheStep{
-										Step:     Step{Type: "save_cache"},
+										BaseStep: BaseStep{Type: "save_cache"},
 										Key:      "cache-{{ arch }}",
 										Contents: []*SaveContent{&SaveContent{SourceDir: "/go/pkg/mod/cache", Paths: []string{"**"}}},
 									},
-									&CloneStep{Step: Step{Type: "clone"}},
+									&CloneStep{BaseStep: BaseStep{Type: "clone"}},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "command01",
 										},
 										Command: "command01",
 									},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "name different than command",
 										},
 										Command: "command02",
 									},
 									&RunStep{
-										Step: Step{
+										BaseStep: BaseStep{
 											Type: "run",
 											Name: "command03",
 										},
@@ -366,14 +366,14 @@ func TestParseOutput(t *testing.T) {
 										},
 									},
 									&SaveCacheStep{
-										Step:     Step{Type: "save_cache"},
+										BaseStep: BaseStep{Type: "save_cache"},
 										Key:      "cache-{{ arch }}",
 										Contents: []*SaveContent{&SaveContent{SourceDir: "/go/pkg/mod/cache", Paths: []string{"**"}}},
 									},
 								},
 								IgnoreFailure: false,
 								Approval:      false,
-								When: &types.When{
+								When: &When{
 									Branch: &types.WhenConditions{
 										Include: []types.WhenCondition{
 											{Type: types.WhenConditionTypeSimple, Match: "master"},
@@ -413,8 +413,8 @@ func TestParseOutput(t *testing.T) {
 									},
 								},
 								WorkingDir: defaultWorkingDir,
-								Steps:      []interface{}{},
+								Steps:      nil,
-								Depends:    []*Depend{},
+								Depends:    nil,
 							},
 							&Task{
 								Name: "task03",
@@ -428,8 +428,8 @@ func TestParseOutput(t *testing.T) {
 									},
 								},
 								WorkingDir: defaultWorkingDir,
-								Steps:      []interface{}{},
+								Steps:      nil,
-								Depends:    []*Depend{},
+								Depends:    nil,
 							},
 							&Task{
 								Name: "task04",
@@ -443,8 +443,8 @@ func TestParseOutput(t *testing.T) {
 									},
 								},
 								WorkingDir: defaultWorkingDir,
-								Steps:      []interface{}{},
+								Steps:      nil,
-								Depends:    []*Depend{},
+								Depends:    nil,
 							},
 						},
 					},

internal/runconfig/runconfig.go

@@ -48,6 +48,17 @@ func genRuntime(c *config.Config, ce *config.Runtime, variables map[string]strin
 	}
 }
 
+func whenFromConfigWhen(cw *config.When) *types.When {
+	if cw == nil {
+		return nil
+	}
+	return &types.When{
+		Branch: cw.Branch,
+		Tag:    cw.Tag,
+		Ref:    cw.Ref,
+	}
+}
+
 func stepFromConfigStep(csi interface{}, variables map[string]string) interface{} {
 	switch cs := csi.(type) {
 	case *config.CloneStep:
@@ -183,7 +194,7 @@ func GenRunConfigTasks(uuid util.UUIDGenerator, c *config.Config, runName string
 	rcts := map[string]*rstypes.RunConfigTask{}
 
 	for _, ct := range cr.Tasks {
-		include := types.MatchWhen(ct.When, branch, tag, ref)
+		include := types.MatchWhen(whenFromConfigWhen(ct.When), branch, tag, ref)
 
 		steps := make([]interface{}, len(ct.Steps))
 		for i, cpts := range ct.Steps {

internal/runconfig/runconfig_test.go

@@ -19,11 +19,12 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/sorintlab/agola/internal/config"
 	rstypes "github.com/sorintlab/agola/internal/services/runservice/types"
 	"github.com/sorintlab/agola/internal/services/types"
 	"github.com/sorintlab/agola/internal/util"
+
+	"github.com/google/go-cmp/cmp"
 	errors "golang.org/x/xerrors"
 )
 
@@ -673,23 +674,23 @@ func TestGenRunConfig(t *testing.T) {
 								WorkingDir: "",
 								Shell:      "",
 								User:       "",
-								Steps: []interface{}{
+								Steps: config.Steps{
 									&config.RunStep{
-										Step: config.Step{
+										BaseStep: config.BaseStep{
 											Type: "run",
 											Name: "command01",
 										},
 										Command: "command01",
 									},
 									&config.RunStep{
-										Step: config.Step{
+										BaseStep: config.BaseStep{
 											Type: "run",
 											Name: "name different than command",
 										},
 										Command: "command02",
 									},
 									&config.RunStep{
-										Step: config.Step{
+										BaseStep: config.BaseStep{
 											Type: "run",
 											Name: "command03",
 										},
@@ -704,7 +705,7 @@ func TestGenRunConfig(t *testing.T) {
 								Depends:       []*config.Depend{},
 								IgnoreFailure: false,
 								Approval:      false,
-								When: &types.When{
+								When: &config.When{
 									Branch: &types.WhenConditions{Include: []types.WhenCondition{{Match: "master"}}},
 									Tag:    &types.WhenConditions{Include: []types.WhenCondition{{Match: "v1.x"}, {Match: "v2.x"}}},
 									Ref: &types.WhenConditions{
@@ -781,9 +782,9 @@ func TestGenRunConfig(t *testing.T) {
 										},
 									},
 								},
-								Steps: []interface{}{
+								Steps: config.Steps{
 									&config.RunStep{
-										Step: config.Step{
+										BaseStep: config.BaseStep{
 											Type: "run",
 											Name: "command01",
 										},
@@ -867,9 +868,9 @@ func TestGenRunConfig(t *testing.T) {
 										},
 									},
 								},
-								Steps: []interface{}{
+								Steps: config.Steps{
 									&config.RunStep{
-										Step: config.Step{
+										BaseStep: config.BaseStep{
 											Type: "run",
 											Name: "command01",
 										},