Skip to content

Commit

Permalink
fix(controller): support float for param value (argoproj#4490)
Browse files Browse the repository at this point in the history
Signed-off-by: Arghya Sadhu <[email protected]>
  • Loading branch information
arghya88 committed Nov 16, 2020
1 parent 4bacbc1 commit 176d890
Show file tree
Hide file tree
Showing 21 changed files with 592 additions and 561 deletions.
52 changes: 52 additions & 0 deletions pkg/apis/workflow/v1alpha1/anystring.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package v1alpha1

import (
"encoding/json"
"fmt"
"strconv"
)

// * It's JSON type is just string.
// * It will unmarshall int64, int32, float64, float32, boolean, a plain string and represents it as string.
// * It will marshall back to string - marshalling is not symmetric.
type AnyString string

func ParseAnyString(val interface{}) AnyString {
return AnyString(fmt.Sprintf("%v", val))
}

func AnyStringPtr(val interface{}) *AnyString {
i := ParseAnyString(val)
return &i
}

func (i *AnyString) UnmarshalJSON(value []byte) error {
var v interface{}
err := json.Unmarshal(value, &v)
if err != nil {
return err
}
switch v := v.(type) {
case float64:
*i = AnyString(strconv.FormatFloat(v, 'f', -1, 64))
case float32:
*i = AnyString(strconv.FormatFloat(float64(v), 'f', -1, 32))
case int64:
*i = AnyString(strconv.FormatInt(v, 10))
case int32:
*i = AnyString(strconv.FormatInt(int64(v), 10))
case bool:
*i = AnyString(strconv.FormatBool(v))
case string:
*i = AnyString(v)
}
return nil
}

func (i AnyString) MarshalJSON() ([]byte, error) {
return json.Marshal(string(i))
}

func (i AnyString) String() string {
return string(i)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,57 +7,88 @@ import (
"github.com/stretchr/testify/assert"
)

func TestInt64OrString(t *testing.T) {
func TestAnyString(t *testing.T) {
t.Run("Empty", func(t *testing.T) {
x := Int64OrStringPtr("")
x := AnyStringPtr("")
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `""`, string(data), "string value has quotes")
}
i := Int64OrStringPtr("")
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte(`""`))
if assert.NoError(t, err) {
assert.Equal(t, Int64OrStringPtr(""), i)
assert.Equal(t, AnyStringPtr(""), i)
}
assert.Equal(t, "", i.String(), "string value does not have quotes")
})
t.Run("String", func(t *testing.T) {
x := Int64OrStringPtr("my-string")
x := AnyStringPtr("my-string")
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"my-string"`, string(data), "string value has quotes")
}
i := Int64OrStringPtr("")
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte(`"my-string"`))
if assert.NoError(t, err) {
assert.Equal(t, Int64OrStringPtr("my-string"), i)
assert.Equal(t, AnyStringPtr("my-string"), i)
}
assert.Equal(t, "my-string", i.String(), "string value does not have quotes")
})
t.Run("StringNumber", func(t *testing.T) {
x := Int64OrStringPtr(1)
x := AnyStringPtr(1)
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"1"`, string(data), "number value has quotes")
}
i := Int64OrStringPtr("")
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte(`"1"`))
if assert.NoError(t, err) {
assert.Equal(t, Int64OrStringPtr("1"), i)
assert.Equal(t, AnyStringPtr("1"), i)
}
assert.Equal(t, "1", i.String(), "number value does not have quotes")
})
t.Run("LargeNumber", func(t *testing.T) {
x := ParseInt64OrString(881217801864)
x := ParseAnyString(881217801864)
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"881217801864"`, string(data))
}
i := Int64OrStringPtr("")
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte("881217801864"))
if assert.NoError(t, err) {
assert.Equal(t, Int64OrStringPtr("881217801864"), i)
assert.Equal(t, AnyStringPtr("881217801864"), i)
}
assert.Equal(t, "881217801864", i.String())
})
t.Run("FloatNumber", func(t *testing.T) {
x := ParseAnyString(0.2)
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"0.2"`, string(data))
}
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte("0.2"))
if assert.NoError(t, err) {
assert.Equal(t, AnyStringPtr("0.2"), i)
}
assert.Equal(t, "0.2", i.String())
})
t.Run("Boolean", func(t *testing.T) {
x := ParseAnyString(true)
data, err := json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"true"`, string(data))
}
x = ParseAnyString(false)
data, err = json.Marshal(x)
if assert.NoError(t, err) {
assert.Equal(t, `"false"`, string(data))
}
i := AnyStringPtr("")
err = i.UnmarshalJSON([]byte("true"))
if assert.NoError(t, err) {
assert.Equal(t, AnyStringPtr("true"), i)
}
assert.Equal(t, "true", i.String())
})
}
925 changes: 462 additions & 463 deletions pkg/apis/workflow/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

51 changes: 0 additions & 51 deletions pkg/apis/workflow/v1alpha1/int64orstr.go

This file was deleted.

8 changes: 4 additions & 4 deletions pkg/apis/workflow/v1alpha1/workflow_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,11 +701,11 @@ type Parameter struct {
Name string `json:"name" protobuf:"bytes,1,opt,name=name"`

// Default is the default value to use for an input parameter if a value was not supplied
Default *Int64OrString `json:"default,omitempty" protobuf:"bytes,2,opt,name=default"`
Default *AnyString `json:"default,omitempty" protobuf:"bytes,2,opt,name=default"`

// Value is the literal value to use for the parameter.
// If specified in the context of an input parameter, the value takes precedence over any passed values
Value *Int64OrString `json:"value,omitempty" protobuf:"bytes,3,opt,name=value"`
Value *AnyString `json:"value,omitempty" protobuf:"bytes,3,opt,name=value"`

// ValueFrom is the source for the output parameter's value
ValueFrom *ValueFrom `json:"valueFrom,omitempty" protobuf:"bytes,4,opt,name=valueFrom"`
Expand All @@ -715,7 +715,7 @@ type Parameter struct {
GlobalName string `json:"globalName,omitempty" protobuf:"bytes,5,opt,name=globalName"`

// Enum holds a list of string values to choose from, for the actual value of the parameter
Enum []Int64OrString `json:"enum,omitempty" protobuf:"bytes,6,rep,name=enum"`
Enum []AnyString `json:"enum,omitempty" protobuf:"bytes,6,rep,name=enum"`
}

// ValueFrom describes a location in which to obtain the value to a parameter
Expand All @@ -740,7 +740,7 @@ type ValueFrom struct {
Supplied *SuppliedValueFrom `json:"supplied,omitempty" protobuf:"bytes,6,opt,name=supplied"`

// Default specifies a value to be used if retrieving the value from the specified source fails
Default *Int64OrString `json:"default,omitempty" protobuf:"bytes,5,opt,name=default"`
Default *AnyString `json:"default,omitempty" protobuf:"bytes,5,opt,name=default"`
}

// SuppliedValueFrom is a placeholder for a value to be filled in directly, either through the CLI, API, etc.
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/event/dispatch/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (o *Operation) dispatch(wfeb wfv1.WorkflowEventBinding, nameSuffix string)
if err != nil {
return nil, fmt.Errorf("failed to evaluate workflow template parameter \"%s\" expression: %w", p.Name, err)
}
wf.Spec.Arguments.Parameters = append(wf.Spec.Arguments.Parameters, wfv1.Parameter{Name: p.Name, Value: wfv1.Int64OrStringPtr(result)})
wf.Spec.Arguments.Parameters = append(wf.Spec.Arguments.Parameters, wfv1.Parameter{Name: p.Name, Value: wfv1.AnyStringPtr(result)})
}
}
wf, err = client.ArgoprojV1alpha1().Workflows(wfeb.Namespace).Create(wf)
Expand Down
2 changes: 1 addition & 1 deletion server/event/dispatch/operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestNewOperation(t *testing.T) {
assert.Equal(t, "my-instanceid", wf.Labels[common.LabelKeyControllerInstanceID])
assert.Equal(t, "my-sub", wf.Labels[common.LabelKeyCreator])
assert.Contains(t, wf.Labels, common.LabelKeyWorkflowEventBinding)
assert.Equal(t, []wfv1.Parameter{{Name: "my-param", Value: wfv1.Int64OrStringPtr(`foo`)}}, wf.Spec.Arguments.Parameters)
assert.Equal(t, []wfv1.Parameter{{Name: "my-param", Value: wfv1.AnyStringPtr(`foo`)}}, wf.Spec.Arguments.Parameters)
}
}
assert.Equal(t, "Warning WorkflowEventBindingError failed to dispatch event: failed to evaluate workflow template expression: unexpected token EOF (1:1)", <-recorder.Events)
Expand Down
2 changes: 1 addition & 1 deletion util/printer/workflow-printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestPrintWorkflows(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "my-wf", Namespace: "my-ns", CreationTimestamp: metav1.Time{Time: now}},
Spec: wfv1.WorkflowSpec{
Arguments: wfv1.Arguments{Parameters: []wfv1.Parameter{
{Name: "my-param", Value: wfv1.Int64OrStringPtr("my-value")},
{Name: "my-param", Value: wfv1.AnyStringPtr("my-value")},
}},
Priority: pointer.Int32Ptr(2),
Templates: []wfv1.Template{
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestConfigMapCacheSave(t *testing.T) {
var MockParamValue string = "Hello world"
var MockParam = wfv1.Parameter{
Name: "hello",
Value: wfv1.Int64OrStringPtr(MockParamValue),
Value: wfv1.AnyStringPtr(MockParamValue),
}
cancel, controller := newController()
defer cancel()
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2231,7 +2231,7 @@ func getTemplateOutputsFromScope(tmpl *wfv1.Template, scope *wfScope) (*wfv1.Out
return nil, err
}
}
param.Value = wfv1.Int64OrStringPtr(val)
param.Value = wfv1.AnyStringPtr(val)
param.ValueFrom = nil
outputs.Parameters = append(outputs.Parameters, param)
}
Expand Down
6 changes: 3 additions & 3 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2061,7 +2061,7 @@ func TestWorkflowSpecParam(t *testing.T) {
func TestAddGlobalParamToScope(t *testing.T) {
woc := newWoc()
woc.globalParams = make(map[string]string)
testVal := wfv1.Int64OrStringPtr("test-value")
testVal := wfv1.AnyStringPtr("test-value")
param := wfv1.Parameter{
Name: "test-param",
Value: testVal,
Expand All @@ -2079,7 +2079,7 @@ func TestAddGlobalParamToScope(t *testing.T) {
assert.Equal(t, testVal.String(), woc.globalParams["workflow.outputs.parameters.global-param"])

// Change the value and verify it is reflected in workflow outputs
newValue := wfv1.Int64OrStringPtr("new-value")
newValue := wfv1.AnyStringPtr("new-value")
param.Value = newValue
woc.addParamToGlobalScope(param)
assert.Equal(t, 1, len(woc.wf.Status.Outputs.Parameters))
Expand Down Expand Up @@ -4451,7 +4451,7 @@ func TestConfigMapCacheSaveOperate(t *testing.T) {
woc := newWorkflowOperationCtx(wf, controller)
sampleOutputs := wfv1.Outputs{
Parameters: []wfv1.Parameter{
{Name: "hello", Value: wfv1.Int64OrStringPtr("foobar")},
{Name: "hello", Value: wfv1.AnyStringPtr("foobar")},
},
}

Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/operator_wfdefault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestWFDefaultWithWFTAndWf(t *testing.T) {
t.Run("SubmitComplexWorkflowRefWithArguments", func(t *testing.T) {
param := wfv1.Parameter{
Name: "Test",
Value: wfv1.Int64OrStringPtr("welcome"),
Value: wfv1.AnyStringPtr("welcome"),
}
art := wfv1.Artifact{
Name: "TestA",
Expand Down
4 changes: 2 additions & 2 deletions workflow/controller/operator_workflow_template_ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestWorkflowTemplateRefWithArgs(t *testing.T) {
args := []wfv1.Parameter{
{
Name: "param1",
Value: wfv1.Int64OrStringPtr("test"),
Value: wfv1.AnyStringPtr("test"),
},
}
wf.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)
Expand All @@ -51,7 +51,7 @@ func TestWorkflowTemplateRefWithWorkflowTemplateArgs(t *testing.T) {
args := []wfv1.Parameter{
{
Name: "param1",
Value: wfv1.Int64OrStringPtr("test"),
Value: wfv1.AnyStringPtr("test"),
},
}
wftmpl.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ func TestVolumesPodSubstitution(t *testing.T) {
inputParameters := []wfv1.Parameter{
{
Name: "volume-name",
Value: wfv1.Int64OrStringPtr("test-name"),
Value: wfv1.AnyStringPtr("test-name"),
},
}

Expand Down
Loading

0 comments on commit 176d890

Please sign in to comment.