Skip to content

Commit 7d5c74e

Browse files
rbeuque74rbeuque74
authored
fix: values: templating values were incorrectly copied after a clone (#227)
values.Values were incorrectly copied after a values.Clone, because the deepclone library were considering a json.Number as a string, instead of treating it as a number. Consequently, every usage of the string instead of a json.Number was incorrect. Signed-off-by: Romain Beuque <556072+rbeuque74@users.noreply.github.com>
1 parent 47d2c36 commit 7d5c74e

File tree

5 files changed

+153
-10
lines changed

5 files changed

+153
-10
lines changed

‎engine/step/step.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,14 @@ func Run(st *Step, baseConfig map[string]json.RawMessage, stepValues *values.Val
358358
return
359359
}
360360

361-
preHookValues := stepValues.Clone()
361+
preHookValues, err := stepValues.Clone()
362+
if err != nil {
363+
st.State = StateFatalError
364+
st.Error = err.Error()
365+
go noopStep(st, stepChan)
366+
return
367+
}
368+
362369
var preHookWg sync.WaitGroup
363370
if prehook != nil {
364371
preHookExecution, err := st.generateExecution(*prehook, baseConfig, stepValues, shutdownCtx)

‎engine/values/values.go‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import (
1212
"github.com/gofrs/uuid"
1313
"github.com/juju/errors"
1414
"github.com/ovh/utask"
15+
"github.com/ovh/utask/pkg/utils"
1516
"github.com/robertkrimen/otto"
16-
"github.com/ybriffa/deepcopy"
1717
)
1818

1919
// keys to store/retrieve data from a Values struct
@@ -47,7 +47,7 @@ type Variable struct {
4747
Name string `json:"name"`
4848
Expression string `json:"expression"`
4949
Value interface{} `json:"value"`
50-
evalCachedResult interface{} `json:"-"`
50+
evalCachedResult interface{}
5151
}
5252

5353
// NewValues instantiates a new Values holder,
@@ -78,12 +78,38 @@ func NewValues() *Values {
7878
}
7979

8080
// Clone duplicates the values object
81-
func (v *Values) Clone() *Values {
81+
func (v *Values) Clone() (*Values, error) {
8282
n := NewValues()
83-
for key, value := range v.m {
84-
n.m[key] = deepcopy.Copy(value)
83+
84+
for key, val := range v.m {
85+
if val == nil {
86+
continue
87+
}
88+
89+
ba, err := utils.JSONMarshal(val)
90+
if err != nil {
91+
return nil, err
92+
}
93+
94+
switch key {
95+
case VarKey:
96+
newobj := &map[string]*Variable{}
97+
err = utils.JSONnumberUnmarshal(bytes.NewReader(ba), newobj)
98+
if err != nil {
99+
return nil, err
100+
}
101+
n.m[key] = *newobj
102+
default:
103+
var newobj interface{}
104+
err = utils.JSONnumberUnmarshal(bytes.NewReader(ba), &newobj)
105+
if err != nil {
106+
return nil, err
107+
}
108+
n.m[key] = newobj
109+
}
85110
}
86-
return n
111+
112+
return n, nil
87113
}
88114

89115
// SetInput stores a task's inputs in Values

‎engine/values/values_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package values_test
22

33
import (
4+
"fmt"
5+
"strings"
46
"testing"
57

68
"github.com/ghodss/yaml"
79
"github.com/maxatome/go-testdeep/td"
810
"github.com/ovh/utask/engine/values"
11+
"github.com/ovh/utask/pkg/utils"
912
)
1013

1114
func TestTmpl(t *testing.T) {
@@ -37,3 +40,113 @@ func TestTmpl(t *testing.T) {
3740
td.CmpNil(t, err)
3841
td.Cmp(t, string(output), "example.org")
3942
}
43+
44+
func TestJsonNumber(t *testing.T) {
45+
input := `
46+
{
47+
"step": {
48+
"first": {
49+
"output": {
50+
"my-payload": {
51+
"foo": 0
52+
}
53+
}
54+
}
55+
}
56+
}
57+
`
58+
59+
assert, require := td.AssertRequire(t)
60+
61+
obj := map[string]map[string]map[string]interface{}{}
62+
err := utils.JSONnumberUnmarshal(strings.NewReader(input), &obj)
63+
require.Nil(err)
64+
65+
v := values.NewValues()
66+
v.SetOutput("first", obj["step"]["first"]["output"])
67+
v.SetVariables([]values.Variable{
68+
{
69+
Name: "foobar0",
70+
Value: "ok",
71+
},
72+
{
73+
Name: "foobar1",
74+
Value: "{{ `ok` }}",
75+
},
76+
{
77+
Name: "foobar2",
78+
Expression: `var foo = {"foo":"bar"};
79+
foo.foo;`,
80+
},
81+
})
82+
83+
output := v.GetOutput("first")
84+
outputMap, ok := output.(map[string]interface{})
85+
require.True(ok)
86+
innerOutputMap, ok := outputMap["my-payload"].(map[string]interface{})
87+
require.True(ok)
88+
89+
assert.Cmp(fmt.Sprintf("%T", innerOutputMap["foo"]), "json.Number")
90+
91+
outputba, err := v.Apply("{{ eval `foobar0` }}", nil, "")
92+
require.Nil(err)
93+
assert.Cmp(string(outputba), "ok")
94+
95+
outputba, err = v.Apply("{{ eval `foobar1` }}", nil, "")
96+
require.Nil(err)
97+
assert.Cmp(string(outputba), "ok")
98+
99+
outputba, err = v.Apply("{{ evalCache `foobar2` }}", nil, "")
100+
require.Nil(err)
101+
assert.Cmp(string(outputba), "bar")
102+
103+
newV, err := v.Clone()
104+
require.Nil(err, "received unexpected error: %s", err)
105+
106+
newOutput := newV.GetOutput("first")
107+
newOutputMap, ok := newOutput.(map[string]interface{})
108+
require.True(ok)
109+
newInnerOutputMap, ok := newOutputMap["my-payload"].(map[string]interface{})
110+
require.True(ok)
111+
112+
assert.Cmp(fmt.Sprintf("%T", newInnerOutputMap["foo"]), "json.Number")
113+
114+
outputba, err = newV.Apply("{{ eval `foobar0` }}", nil, "")
115+
require.Nil(err)
116+
assert.Cmp(string(outputba), "ok")
117+
118+
outputba, err = newV.Apply("{{ eval `foobar1` }}", nil, "")
119+
require.Nil(err)
120+
assert.Cmp(string(outputba), "ok")
121+
122+
outputba, err = newV.Apply("{{ evalCache `foobar2` }}", nil, "")
123+
require.Nil(err)
124+
assert.Cmp(string(outputba), "bar")
125+
126+
// with iterator now
127+
iterator := map[string]map[string][]string{
128+
"foo": map[string][]string{
129+
"bar": []string{"foobar", "buzz"},
130+
},
131+
}
132+
v.SetIterator(iterator)
133+
134+
outputba, err = v.Apply("{{ index .iterator.foo.bar 1 }}", nil, "")
135+
require.Nil(err)
136+
assert.Cmp(string(outputba), "buzz")
137+
138+
newV, err = v.Clone()
139+
require.Nil(err, "received unexpected error: %s", err)
140+
141+
newOutput = newV.GetOutput("first")
142+
newOutputMap, ok = newOutput.(map[string]interface{})
143+
require.True(ok)
144+
newInnerOutputMap, ok = newOutputMap["my-payload"].(map[string]interface{})
145+
require.True(ok)
146+
147+
assert.Cmp(fmt.Sprintf("%T", newInnerOutputMap["foo"]), "json.Number")
148+
149+
outputba, err = newV.Apply("{{ index .iterator.foo.bar 1 }}", nil, "")
150+
require.Nil(err)
151+
assert.Cmp(string(outputba), "buzz")
152+
}

‎go.mod‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ require (
5454
github.com/tidwall/gjson v1.6.0
5555
github.com/tidwall/pretty v1.0.1 // indirect
5656
github.com/wI2L/fizz v0.13.4
57-
github.com/ybriffa/deepcopy v0.0.0-20200601125345-88c31e59651d
5857
github.com/ybriffa/go-http-digest-auth-client v0.6.3
5958
github.com/ziutek/mymysql v1.5.4 // indirect
6059
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9

‎go.sum‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,6 @@ github.com/wI2L/fizz v0.13.4 h1:4G48MVzMXSC4SmWagQlgSnUgDrZ5eLr43RutM2dnM8Q=
349349
github.com/wI2L/fizz v0.13.4/go.mod h1:R7UIV/FdYUnOZbfkTMwoV9zpnqzZSRl2CaxWUMpwDuo=
350350
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
351351
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
352-
github.com/ybriffa/deepcopy v0.0.0-20200601125345-88c31e59651d h1:CMtYkTYEOI9CmV8yfd7FMqJXdDpwu8NSxDwXtXMbY2o=
353-
github.com/ybriffa/deepcopy v0.0.0-20200601125345-88c31e59651d/go.mod h1:SMxjudgXLmHFX1pP4suwVNURE3VHtRinVMii3OTJjGs=
354352
github.com/ybriffa/go-http-digest-auth-client v0.6.3 h1:s8r2tg2eqVtQ94Vy2gtCp+kV97RjQd0XeUbH7dOhw3w=
355353
github.com/ybriffa/go-http-digest-auth-client v0.6.3/go.mod h1:gs7qI0Vksu7hyGo5lrXM8uOlWuC6qCRlfhonZ14exsY=
356354
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

0 commit comments

Comments
 (0)