Write own yaml2json func (#570)

* fix regression of #384 
 * add more tests

Makefile

@@ -71,7 +71,7 @@ test-frontend: frontend-dependencies
 test-lib:
 	$(DOCKER_RUN) go test -race -timeout 30s $(shell go list ./... | grep -v '/cmd\|/agent\|/cli\|/server')
 
-test: test-agent test-server test-server-datastore test-cli test-frontend test-lib
+test: test-agent test-server test-server-datastore test-cli test-lib test-frontend
 
 build-frontend:
 	(cd web/; yarn install --frozen-lockfile; yarn build)

pipeline/frontend/yaml/compiler/params.go

@@ -6,61 +6,94 @@ import (
 	"strconv"
 	"strings"
 
-	json "github.com/woodpecker-ci/woodpecker/shared/yml"
-
 	"gopkg.in/yaml.v3"
+
+	"github.com/woodpecker-ci/woodpecker/shared/yml"
 )
 
 // paramsToEnv uses reflection to convert a map[string]interface to a list
 // of environment variables.
-func paramsToEnv(from map[string]interface{}, to map[string]string) error {
+func paramsToEnv(from map[string]interface{}, to map[string]string) (err error) {
+	if to == nil {
+		return fmt.Errorf("no map to write to")
+	}
 	for k, v := range from {
-		if v == nil {
+		if v == nil || len(k) == 0 {
 			continue
 		}
+		to[sanitizeParamKey(k)], err = sanitizeParamValue(v)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
 
+func sanitizeParamKey(k string) string {
+	return "PLUGIN_" +
+		strings.ToUpper(
+			strings.ReplaceAll(k, ".", "_"),
+		)
+}
+
+func isComplex(t reflect.Kind) bool {
+	switch t {
+	case reflect.Bool,
+		reflect.String,
+		reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Float32, reflect.Float64:
+		return false
+	default:
+		return true
+	}
+}
+
+func sanitizeParamValue(v interface{}) (string, error) {
 	t := reflect.TypeOf(v)
 	vv := reflect.ValueOf(v)
 
-		k = "PLUGIN_" + strings.ToUpper(k)
-
 	switch t.Kind() {
 	case reflect.Bool:
-			to[k] = strconv.FormatBool(vv.Bool())
+		return strconv.FormatBool(vv.Bool()), nil
 
 	case reflect.String:
-			to[k] = vv.String()
+		return vv.String(), nil
 
 	case reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int8:
-			to[k] = fmt.Sprintf("%v", vv.Int())
+		return fmt.Sprintf("%v", vv.Int()), nil
 
 	case reflect.Float32, reflect.Float64:
-			to[k] = fmt.Sprintf("%v", vv.Float())
+		return fmt.Sprintf("%v", vv.Float()), nil
 
 	case reflect.Map:
-			yml, _ := yaml.Marshal(vv.Interface())
-			out, _ := json.Yml2Json(yml)
-			to[k] = string(out)
+		ymlOut, _ := yaml.Marshal(vv.Interface())
+		out, _ := yml.ToJSON(ymlOut)
+		return string(out), nil
 
-		case reflect.Slice:
+	case reflect.Slice, reflect.Array:
+		if !isComplex(t.Elem().Kind()) {
+			in := make([]string, vv.Len())
+			for i := 0; i < vv.Len(); i++ {
+				var err error
+				if in[i], err = sanitizeParamValue(vv.Index(i).Interface()); err != nil {
+					return "", err
+				}
+			}
+			return strings.Join(in, ","), nil
+		}
+
+		// it's complex use yml.ToJSON
+		fallthrough
+
+	default:
 		out, err := yaml.Marshal(vv.Interface())
 		if err != nil {
-				return err
+			return "", err
 		}
-
-			var in []string
-			err = yaml.Unmarshal(out, &in)
-			if err == nil {
-				to[k] = strings.Join(in, ",")
-			} else {
-				out, err = json.Yml2Json(out)
+		out, err = yml.ToJSON(out)
 		if err != nil {
-					return err
+			return "", err
 		}
-				to[k] = string(out)
+		return string(out), nil
 	}
 }
-	}
-
-	return nil
-}

pipeline/frontend/yaml/compiler/params_test.go

@@ -13,18 +13,22 @@ func TestParamsToEnv(t *testing.T) {
 		"int":          1,
 		"float":        1.2,
 		"bool":         true,
-		"map":     map[string]string{"hello": "world"},
 		"slice":        []int{1, 2, 3},
+		"map":          map[string]string{"hello": "world"},
 		"complex":      []struct{ Name string }{{"Jack"}, {"Jill"}},
+		"complex2":     struct{ Name string }{"Jack"},
+		"from.address": "noreply@example.com",
 	}
 	want := map[string]string{
 		"PLUGIN_STRING":       "stringz",
 		"PLUGIN_INT":          "1",
 		"PLUGIN_FLOAT":        "1.2",
 		"PLUGIN_BOOL":         "true",
-		"PLUGIN_MAP":     `{"hello":"world"}`,
 		"PLUGIN_SLICE":        "1,2,3",
+		"PLUGIN_MAP":          `{"hello":"world"}`,
 		"PLUGIN_COMPLEX":      `[{"name":"Jack"},{"name":"Jill"}]`,
+		"PLUGIN_COMPLEX2":     `{"name":"Jack"}`,
+		"PLUGIN_FROM_ADDRESS": "noreply@example.com",
 	}
 	got := map[string]string{}
 	assert.NoError(t, paramsToEnv(from, got))

shared/yml/yml.go

@@ -4,65 +4,72 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"strconv"
 
 	"gopkg.in/yaml.v3"
 )
 
-// Source: https://github.com/icza/dyno/blob/f1bafe5d99965c48cc9d5c7cf024eeb495facc1e/dyno.go#L563-L601
-// License: Apache 2.0 - Copyright 2017 Andras Belicza
-// ConvertMapI2MapS walks the given dynamic object recursively, and
-// converts maps with interface{} key type to maps with string key type.
-// This function comes handy if you want to marshal a dynamic object into
-// JSON where maps with interface{} key type are not allowed.
-//
-// Recursion is implemented into values of the following types:
-//   -map[interface{}]interface{}
-//   -map[string]interface{}
-//   -[]interface{}
-//
-// When converting map[interface{}]interface{} to map[string]interface{},
+// toJSON convert gopkg.in/yaml.v3 nodes to object that can be serialized as json
 // fmt.Sprint() with default formatting is used to convert the key to a string key.
-func convertMapI2MapS(v interface{}) interface{} {
-	switch x := v.(type) {
-	case map[interface{}]interface{}:
-		m := map[string]interface{}{}
-		for k, v2 := range x {
-			switch k2 := k.(type) {
-			case string: // Fast check if it's already a string
-				m[k2] = convertMapI2MapS(v2)
-			default:
-				m[fmt.Sprint(k)] = convertMapI2MapS(v2)
-			}
-		}
-		v = m
+func toJSON(node *yaml.Node) (interface{}, error) {
+	switch node.Kind {
+	case yaml.DocumentNode:
+		return toJSON(node.Content[0])
 
-	case []interface{}:
-		for i, v2 := range x {
-			x[i] = convertMapI2MapS(v2)
+	case yaml.SequenceNode:
+		val := make([]interface{}, len(node.Content))
+		var err error
+		for i := range node.Content {
+			if val[i], err = toJSON(node.Content[i]); err != nil {
+				return nil, err
 			}
+		}
+		return val, nil
 
-	case map[string]interface{}:
-		for k, v2 := range x {
-			x[k] = convertMapI2MapS(v2)
+	case yaml.MappingNode:
+		if (len(node.Content) % 2) != 0 {
+			return nil, fmt.Errorf("broken mapping node")
 		}
-	}
-
-	return v
-}
-
-func Yml2Json(data []byte) (j []byte, err error) {
-	m := make(map[interface{}]interface{})
-	err = yaml.Unmarshal(data, &m)
+		val := make(map[string]interface{}, len(node.Content)%2)
+		for i := len(node.Content); i > 1; i = i - 2 {
+			k, err := toJSON(node.Content[i-2])
 			if err != nil {
 				return nil, err
 			}
+			if val[fmt.Sprint(k)], err = toJSON(node.Content[i-1]); err != nil {
+				return nil, err
+			}
+		}
+		return val, nil
 
-	j, err = json.Marshal(convertMapI2MapS(m))
-	if err != nil {
+	case yaml.ScalarNode:
+		switch node.Tag {
+		case nullTag:
+			return nil, nil
+		case boolTag:
+			return strconv.ParseBool(node.Value)
+		case intTag:
+			return strconv.ParseInt(node.Value, 10, 64)
+		case floatTag:
+			return strconv.ParseFloat(node.Value, 64)
+		}
+		return node.Value, nil
+	}
+
+	return nil, fmt.Errorf("do not support yaml node kind '%v'", node.Kind)
+}
+
+func ToJSON(data []byte) ([]byte, error) {
+	m := &yaml.Node{}
+	if err := yaml.Unmarshal(data, m); err != nil {
 		return nil, err
 	}
 
-	return j, nil
+	d, err := toJSON(m)
+	if err != nil {
+		return nil, err
+	}
+	return json.Marshal(d)
 }
 
 func LoadYmlFileAsJSON(path string) (j []byte, err error) {
@@ -71,10 +78,24 @@ func LoadYmlFileAsJSON(path string) (j []byte, err error) {
 		return nil, err
 	}
 
-	j, err = Yml2Json(data)
+	j, err = ToJSON(data)
 	if err != nil {
 		return nil, err
 	}
 
 	return j, nil
 }
+
+// Source: https://github.com/go-yaml/yaml/blob/3e3283e801afc229479d5fc68aa41df1137b8394/resolve.go#L70-L81
+const (
+	nullTag  = "!!null"
+	boolTag  = "!!bool"
+	intTag   = "!!int"
+	floatTag = "!!float"
+	// strTag       = "!!str"       // we dont have to parse it
+	// timestampTag = "!!timestamp" // TODO: do we have to parse this?
+	// seqTag       = "!!seq"       // TODO: do we have to parse this?
+	// mapTag       = "!!map"       // TODO: do we have to parse this?
+	// binaryTag    = "!!binary"    // TODO: do we have to parse this?
+	// mergeTag     = "!!merge"     // TODO: do we have to parse this?
+)

shared/yml/yml_test.go

@@ -0,0 +1,45 @@
+package yml
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestToJSON(t *testing.T) {
+	tests := []struct {
+		yaml string
+		json string
+	}{{
+		yaml: `- name: Jack
+- name: Jill
+`,
+		json: `[{"name":"Jack"},{"name":"Jill"}]`,
+	}, {
+		yaml: `name: Jack`,
+		json: `{"name":"Jack"}`,
+	}, {
+		yaml: `name: Jack
+job: Butcher
+`,
+		json: `{"job":"Butcher","name":"Jack"}`,
+	}, {
+		yaml: `- name: Jack
+  job: Butcher
+- name: Jill
+  job: Cook
+  obj:
+    empty: false
+    data: |
+      some data 123
+      with new line
+`,
+		json: `[{"job":"Butcher","name":"Jack"},{"job":"Cook","name":"Jill","obj":{"data":"some data 123\nwith new line\n","empty":false}}]`,
+	}}
+
+	for _, tc := range tests {
+		result, err := ToJSON([]byte(tc.yaml))
+		assert.NoError(t, err)
+		assert.EqualValues(t, tc.json, string(result))
+	}
+}