From 6ae0c5cbc40ff7ff73cab916327be98f31016e80 Mon Sep 17 00:00:00 2001 From: Tianxin Dong Date: Fri, 30 Dec 2022 18:26:36 +0800 Subject: [PATCH] Feat: add set value and use it for inputs (#112) Signed-off-by: FogDong Signed-off-by: FogDong --- controllers/workflow_test.go | 2 +- pkg/cue/model/sets/utils.go | 11 +-- pkg/cue/model/value/value.go | 93 +++++++++++++++++++ pkg/cue/model/value/value_test.go | 145 ++++++++++++++++++++++++++++++ pkg/cue/utils.go | 6 +- pkg/cue/utils_test.go | 2 +- pkg/hooks/data_passing.go | 9 +- pkg/hooks/data_passing_test.go | 19 ++++ pkg/providers/kube/handle.go | 2 +- pkg/tasks/custom/task_test.go | 10 +-- 10 files changed, 281 insertions(+), 18 deletions(-) diff --git a/controllers/workflow_test.go b/controllers/workflow_test.go index 3a946b6..27163cf 100644 --- a/controllers/workflow_test.go +++ b/controllers/workflow_test.go @@ -361,7 +361,7 @@ var _ = Describe("Test Workflow", func() { WorkflowStepBase: v1alpha1.WorkflowStepBase{ Name: "step2", Type: "test-apply", - Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","message":"test"}`)}, Inputs: v1alpha1.StepInputs{ { From: "message", diff --git a/pkg/cue/model/sets/utils.go b/pkg/cue/model/sets/utils.go index d0b0291..9fc7560 100644 --- a/pkg/cue/model/sets/utils.go +++ b/pkg/cue/model/sets/utils.go @@ -70,7 +70,8 @@ func lookUp(node ast.Node, paths ...string) (ast.Node, error) { return nil, notFoundErr } -func lookUpAll(node ast.Node, paths ...string) []ast.Node { +// LookUpAll look up all the nodes by paths +func LookUpAll(node ast.Node, paths ...string) []ast.Node { if len(paths) == 0 { return []ast.Node{node} } @@ -81,7 +82,7 @@ func lookUpAll(node ast.Node, paths ...string) []ast.Node { for _, decl := range x.Decls { nnode := lookField(decl, key) if nnode != nil { - nodes = append(nodes, lookUpAll(nnode, paths[1:]...)...) + nodes = append(nodes, LookUpAll(nnode, paths[1:]...)...) } } @@ -89,13 +90,13 @@ func lookUpAll(node ast.Node, paths ...string) []ast.Node { for _, elt := range x.Elts { nnode := lookField(elt, key) if nnode != nil { - nodes = append(nodes, lookUpAll(nnode, paths[1:]...)...) + nodes = append(nodes, LookUpAll(nnode, paths[1:]...)...) } } case *ast.ListLit: for index, elt := range x.Elts { if strconv.Itoa(index) == key { - return lookUpAll(elt, paths[1:]...) + return LookUpAll(elt, paths[1:]...) } } } @@ -136,7 +137,7 @@ func doBuiltinFunc(root ast.Node, pathSel ast.Expr, do func(values []ast.Node) ( if len(paths) == 0 { return nil, errors.New("path resolve error") } - values := lookUpAll(root, paths...) + values := LookUpAll(root, paths...) return do(values) } diff --git a/pkg/cue/model/value/value.go b/pkg/cue/model/value/value.go index ae5aaf6..32e2627 100644 --- a/pkg/cue/model/value/value.go +++ b/pkg/cue/model/value/value.go @@ -328,6 +328,79 @@ func (val *Value) FillValueByScript(x *Value, path string) error { return nil } +func setValue(orig ast.Node, expr ast.Expr, selectors []cue.Selector) error { + if len(selectors) == 0 { + return nil + } + key := selectors[0] + selectors = selectors[1:] + switch x := orig.(type) { + case *ast.ListLit: + if key.Type() != cue.IndexLabel { + return fmt.Errorf("invalid key type %s in list lit", key.Type()) + } + if len(selectors) == 0 { + for key.Index() >= len(x.Elts) { + x.Elts = append(x.Elts, ast.NewStruct()) + } + x.Elts[key.Index()] = expr + return nil + } + return setValue(x.Elts[key.Index()], expr, selectors) + case *ast.StructLit: + if len(x.Elts) == 0 || (key.Type() == cue.StringLabel && len(sets.LookUpAll(x, key.String())) == 0) { + if len(selectors) == 0 { + x.Elts = append(x.Elts, &ast.Field{ + Label: ast.NewString(key.String()), + Value: expr, + }) + } else { + x.Elts = append(x.Elts, &ast.Field{ + Label: ast.NewString(key.String()), + Value: ast.NewStruct(), + }) + } + return setValue(x.Elts[len(x.Elts)-1].(*ast.Field).Value, expr, selectors) + } + for i := range x.Elts { + switch elem := x.Elts[i].(type) { + case *ast.Field: + if len(selectors) == 0 { + if key.Type() == cue.StringLabel && strings.Trim(sets.LabelStr(elem.Label), `"`) == strings.Trim(key.String(), `"`) { + x.Elts[i].(*ast.Field).Value = expr + return nil + } + } + if key.Type() == cue.StringLabel && strings.Trim(sets.LabelStr(elem.Label), `"`) == strings.Trim(key.String(), `"`) { + return setValue(x.Elts[i].(*ast.Field).Value, expr, selectors) + } + default: + return fmt.Errorf("not support type %T", elem) + } + } + default: + return fmt.Errorf("not support type %T", orig) + } + return nil +} + +// SetValueByScript set the value v at the given script path. +// nolint:staticcheck +func (val *Value) SetValueByScript(v *Value, path ...string) error { + cuepath := FieldPath(path...) + selectors := cuepath.Selectors() + node := val.CueValue().Syntax(cue.ResolveReferences(true)) + if err := setValue(node, v.CueValue().Syntax(cue.ResolveReferences(true)).(ast.Expr), selectors); err != nil { + return err + } + b, err := format.Node(node) + if err != nil { + return err + } + val.v = val.r.CompileBytes(b) + return nil +} + // CueValue return cue.Value func (val *Value) CueValue() cue.Value { return val.v @@ -348,6 +421,26 @@ func (val *Value) FillObject(x interface{}, paths ...string) error { return nil } +// SetObject set the value with object x at the given path. +func (val *Value) SetObject(x interface{}, paths ...string) error { + insert := &Value{ + r: val.r, + } + switch v := x.(type) { + case *Value: + if v.r != val.r { + return errors.New("filled value not created with same Runtime") + } + insert.v = v.v + case ast.Expr: + cueV := val.r.BuildExpr(v) + insert.v = cueV + default: + return fmt.Errorf("not support type %T", x) + } + return val.SetValueByScript(insert, paths...) +} + // LookupValue reports the value at a path starting from val func (val *Value) LookupValue(paths ...string) (*Value, error) { v := val.v.LookupPath(FieldPath(paths...)) diff --git a/pkg/cue/model/value/value_test.go b/pkg/cue/model/value/value_test.go index 20e63b3..df9505e 100644 --- a/pkg/cue/model/value/value_test.go +++ b/pkg/cue/model/value/value_test.go @@ -1094,6 +1094,151 @@ a: b: c: [{x: 100}, {x: 101}, {x: 102}]`, } } +func TestSetByScript(t *testing.T) { + testCases := []struct { + name string + raw string + path string + v string + expected string + }{ + { + name: "insert array", + raw: `a: ["hello"]`, + path: "a[0]", + v: `"world"`, + expected: `a: ["world"] +`}, + { + name: "insert array2", + raw: `a: ["hello"]`, + path: "a[1]", + v: `"world"`, + expected: `a: ["hello", "world"] +`}, + { + name: "insert array3", + raw: `a: b: [{x: 100}]`, + path: "a.b[0]", + v: `{name: "foo"}`, + expected: `a: { + b: [{ + name: "foo" + }] +} +`}, + { + name: "insert struct", + raw: `a: {b: "hello"}`, + path: "a.b", + v: `"world"`, + expected: `a: { + b: "world" +} +`}, + { + name: "insert struct2", + raw: `a: {b: "hello"}, c: {d: "world"}`, + path: "c.d", + v: `"hello"`, + expected: `a: { + b: "hello" +} +c: { + d: "hello" +} +`}, + { + name: "insert array to array", + raw: ` +a: b: c: [{x: 100}, {x: 101}, {x: 102}]`, + path: "a.b.c[0].value", + v: `"foo"`, + expected: `a: { + b: { + c: [{ + x: 100 + value: "foo" + }, { + x: 101 + }, { + x: 102 + }] + } +} +`, + }, + { + name: "insert nest array ", + raw: `a: b: [{x: y:[{name: "key"}]}]`, + path: "a.b[0].x.y[0].value", + v: `"foo"`, + expected: `a: { + b: [{ + x: { + y: [{ + name: "key" + value: "foo" + }] + } + }] +} +`, + }, + { + name: "insert without array", + raw: `a: b: [{x: y:[{name: "key"}]}]`, + path: "a.c.x", + v: `"foo"`, + expected: `a: { + b: [{ + x: { + y: [{ + name: "key" + }] + } + }] + c: { + x: "foo" + } +} +`, + }, + { + name: "path with string index", + raw: `a: b: [{x: y:[{name: "key"}]}]`, + path: "a.c[\"x\"]", + v: `"foo"`, + expected: `a: { + b: [{ + x: { + y: [{ + name: "key" + }] + } + }] + c: { + x: "foo" + } +} +`, + }, + } + + for _, tCase := range testCases { + r := require.New(t) + v, err := NewValue(tCase.raw, nil, "") + r.NoError(err) + val, err := v.MakeValue(tCase.v) + r.NoError(err) + err = v.SetValueByScript(val, tCase.path) + r.NoError(err, tCase.name) + s, err := v.String() + r.NoError(err) + r.Equal(s, tCase.expected, tCase.name) + } +} + func TestSubstituteInStruct(t *testing.T) { base := ` value: { diff --git a/pkg/cue/utils.go b/pkg/cue/utils.go index 9955321..6999bd1 100644 --- a/pkg/cue/utils.go +++ b/pkg/cue/utils.go @@ -79,8 +79,8 @@ func FillUnstructuredObject(v *value.Value, obj runtime.Unstructured, paths ...s return v.FillObject(expr, paths...) } -// SubstituteUnstructuredObject substitute runtime.Unstructured to *value.Value -func SubstituteUnstructuredObject(v *value.Value, obj runtime.Unstructured, path string) error { +// SetUnstructuredObject set runtime.Unstructured to *value.Value +func SetUnstructuredObject(v *value.Value, obj runtime.Unstructured, path string) error { var buf bytes.Buffer if err := unstructured.UnstructuredJSONScheme.Encode(obj, &buf); err != nil { return v.FillObject(err.Error(), "err") @@ -89,5 +89,5 @@ func SubstituteUnstructuredObject(v *value.Value, obj runtime.Unstructured, path if err != nil { return v.FillObject(err.Error(), "err") } - return v.SubstituteInStruct(expr, path) + return v.SetObject(expr, path) } diff --git a/pkg/cue/utils_test.go b/pkg/cue/utils_test.go index 4312ee3..4d755a2 100644 --- a/pkg/cue/utils_test.go +++ b/pkg/cue/utils_test.go @@ -145,7 +145,7 @@ func TestSubstituteUnstructuredObject(t *testing.T) { r := require.New(t) value, err := value.NewValue(`object:{"test": "test"}`, nil, "") r.NoError(err) - err = SubstituteUnstructuredObject(value, testcase.obj, "object") + err = SetUnstructuredObject(value, testcase.obj, "object") r.NoError(err) json, err := value.CueValue().MarshalJSON() r.NoError(err) diff --git a/pkg/hooks/data_passing.go b/pkg/hooks/data_passing.go index 26c025e..e7d9f3f 100644 --- a/pkg/hooks/data_passing.go +++ b/pkg/hooks/data_passing.go @@ -41,8 +41,13 @@ func Input(ctx wfContext.Context, paramValue *value.Value, step v1alpha1.Workflo } } if input.ParameterKey != "" { - if err := paramValue.FillValueByScript(inputValue, strings.Join([]string{"parameter", input.ParameterKey}, ".")); err != nil { - return err + if err := paramValue.SetValueByScript(inputValue, strings.Join([]string{"parameter", input.ParameterKey}, ".")); err != nil || paramValue.Error() != nil { + if err != nil { + return err + } + if paramValue.Error() != nil { + return paramValue.Error() + } } } } diff --git a/pkg/hooks/data_passing_test.go b/pkg/hooks/data_passing_test.go index a8ea8d6..c16a724 100644 --- a/pkg/hooks/data_passing_test.go +++ b/pkg/hooks/data_passing_test.go @@ -54,6 +54,25 @@ func TestInput(t *testing.T) { s, err := result.String() r.NoError(err) r.Equal(s, `99 +`) + // test set value + paramValue, err = wfCtx.MakeParameter(`parameter: {myscore: "test"}`) + r.NoError(err) + err = Input(wfCtx, paramValue, v1alpha1.WorkflowStep{ + WorkflowStepBase: v1alpha1.WorkflowStepBase{ + DependsOn: []string{"mystep"}, + Inputs: v1alpha1.StepInputs{{ + From: "foo.score", + ParameterKey: "myscore", + }}, + }, + }) + r.NoError(err) + result, err = paramValue.LookupValue("parameter", "myscore") + r.NoError(err) + s, err = result.String() + r.NoError(err) + r.Equal(s, `99 `) paramValue, err = wfCtx.MakeParameter(`context: {name: "test"}`) r.NoError(err) diff --git a/pkg/providers/kube/handle.go b/pkg/providers/kube/handle.go index dee6cca..0c29572 100644 --- a/pkg/providers/kube/handle.go +++ b/pkg/providers/kube/handle.go @@ -166,7 +166,7 @@ func (h *provider) Apply(ctx monitorContext.Context, wfCtx wfContext.Context, v if err := h.handlers.Apply(deployCtx, cluster, WorkflowResourceCreator, workload); err != nil { return err } - return cue.SubstituteUnstructuredObject(v, workload, "value") + return cue.SetUnstructuredObject(v, workload, "value") } // ApplyInParallel create or update CRs in parallel. diff --git a/pkg/tasks/custom/task_test.go b/pkg/tasks/custom/task_test.go index 54b6b05..9d73223 100644 --- a/pkg/tasks/custom/task_test.go +++ b/pkg/tasks/custom/task_test.go @@ -205,7 +205,7 @@ close({ steps := []v1alpha1.WorkflowStep{ { WorkflowStepBase: v1alpha1.WorkflowStepBase{ - Name: "input-err", + Name: "input-replace", Type: "ok", Properties: &runtime.RawExtension{Raw: []byte(` {"score": {"x": 101}} @@ -262,11 +262,11 @@ close({ r.NoError(err) status, operation, _ := run.Run(wfCtx, &types.TaskRunOptions{}) switch step.Name { - case "input-err": - r.Equal(status.Message, "parameter.score.x: conflicting values 100 and 101") + case "input-replace": + r.Equal(status.Message, "") r.Equal(operation.Waiting, false) - r.Equal(status.Phase, v1alpha1.WorkflowStepPhaseFailed) - r.Equal(status.Reason, types.StatusReasonInput) + r.Equal(status.Phase, v1alpha1.WorkflowStepPhaseSucceeded) + r.Equal(status.Reason, "") case "input": r.Equal(status.Message, "get input from [podIP]: failed to lookup value: var(path=podIP) not exist") r.Equal(operation.Waiting, false)