From 69ba8886d96c63408849de2d3b26226780fbe20d Mon Sep 17 00:00:00 2001 From: nakabonne Date: Fri, 22 Jan 2021 14:59:10 +0900 Subject: [PATCH 1/3] Allow several types to be used by Event watcher --- pkg/app/piped/eventwatcher/BUILD.bazel | 10 ++- pkg/app/piped/eventwatcher/eventwatcher.go | 34 +++++--- .../piped/eventwatcher/eventwatcher_test.go | 86 +++++++++++++++++++ pkg/yamlprocessor/yamlprocessor.go | 7 +- pkg/yamlprocessor/yamlprocessor_test.go | 21 ++++- 5 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 pkg/app/piped/eventwatcher/eventwatcher_test.go diff --git a/pkg/app/piped/eventwatcher/BUILD.bazel b/pkg/app/piped/eventwatcher/BUILD.bazel index 2e74e37523..2bffb447d5 100644 --- a/pkg/app/piped/eventwatcher/BUILD.bazel +++ b/pkg/app/piped/eventwatcher/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -13,3 +13,11 @@ go_library( "@org_uber_go_zap//:go_default_library", ], ) + +go_test( + name = "go_default_test", + size = "small", + srcs = ["eventwatcher_test.go"], + embed = [":go_default_library"], + deps = ["@com_github_stretchr_testify//assert:go_default_library"], +) diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go index e63ffa622f..47cd8232fb 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher.go +++ b/pkg/app/piped/eventwatcher/eventwatcher.go @@ -222,16 +222,9 @@ func (w *watcher) modifyFiles(ctx context.Context, event *config.EventWatcherEve if err != nil { return nil, fmt.Errorf("failed to get value at %s in %s: %w", r.YAMLField, r.File, err) } - var value string - switch vv := v.(type) { - case string: - value = vv - case int: - value = strconv.Itoa(vv) - case float64: - value = strconv.FormatFloat(vv, 'f', -1, 64) - default: - return nil, fmt.Errorf("unknown type of value is defined at %s in %s", r.YAMLField, r.File) + value, err := convertStr(v) + if err != nil { + return nil, fmt.Errorf("a value of unknown type (%v) is defined at %s in %s", err, r.YAMLField, r.File) } if latestEvent.Data == value { // Already up-to-date. @@ -260,3 +253,24 @@ func (w *watcher) modifyFiles(ctx context.Context, event *config.EventWatcherEve message: commitMsg, }, nil } + +// convertStr converts a given value into a string. +func convertStr(value interface{}) (out string, err error) { + switch v := value.(type) { + case string: + out = v + case int: + out = strconv.Itoa(v) + case int64: + out = strconv.FormatInt(v, 10) + case uint64: + out = strconv.FormatUint(v, 10) + case float64: + out = strconv.FormatFloat(v, 'f', -1, 64) + case bool: + out = strconv.FormatBool(v) + default: + return "", fmt.Errorf("%T", v) + } + return +} diff --git a/pkg/app/piped/eventwatcher/eventwatcher_test.go b/pkg/app/piped/eventwatcher/eventwatcher_test.go new file mode 100644 index 0000000000..4076fe0354 --- /dev/null +++ b/pkg/app/piped/eventwatcher/eventwatcher_test.go @@ -0,0 +1,86 @@ +// Copyright 2021 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package eventwatcher + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetValue(t *testing.T) { + testcases := []struct { + name string + value interface{} + want string + wantErr bool + }{ + { + name: "string", + value: "value", + want: "value", + wantErr: false, + }, + { + name: "int", + value: 1, + want: "1", + wantErr: false, + }, + { + name: "int64", + value: int64(1), + want: "1", + wantErr: false, + }, + { + name: "uint64", + value: uint64(1), + want: "1", + wantErr: false, + }, + { + name: "uint64", + value: uint64(1), + want: "1", + wantErr: false, + }, + { + name: "float64", + value: 1.1, + want: "1.1", + wantErr: false, + }, + { + name: "bool", + value: true, + want: "true", + wantErr: false, + }, + { + name: "map", + value: make(map[string]interface{}), + want: "", + wantErr: true, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + got, err := convertStr(tc.value) + assert.Equal(t, tc.wantErr, err != nil) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/yamlprocessor/yamlprocessor.go b/pkg/yamlprocessor/yamlprocessor.go index 56644d9810..7084dda56a 100644 --- a/pkg/yamlprocessor/yamlprocessor.go +++ b/pkg/yamlprocessor/yamlprocessor.go @@ -24,7 +24,8 @@ import ( "github.com/goccy/go-yaml/parser" ) -// GetValue gives back the value placed at a given path. +// GetValue gives back the value placed at a given path. The type of +// returned value can be string, int64, uint64, float64, bool, and []interface{}. // // The path requires to start with "$" which represents the root element. // Available operators are: @@ -48,6 +49,10 @@ func GetValue(yml []byte, path string) (interface{}, error) { return nil, err } + // NOTE: Validate value in YAML before actual reading + // because it panics if: + // - the value is -0. + // - unexistence path given. var value interface{} if err := p.Read(bytes.NewReader(yml), &value); err != nil { return nil, err diff --git a/pkg/yamlprocessor/yamlprocessor_test.go b/pkg/yamlprocessor/yamlprocessor_test.go index 254a68a0bb..2ac8b70ee4 100644 --- a/pkg/yamlprocessor/yamlprocessor_test.go +++ b/pkg/yamlprocessor/yamlprocessor_test.go @@ -79,6 +79,13 @@ func TestGetValue(t *testing.T) { want: uint64(1), wantErr: false, }, + { + name: "given a int64 path", + yml: "foo: -1", + path: "$.foo", + want: int64(-1), + wantErr: false, + }, { name: "given a float64 path", yml: "foo: 1.5", @@ -87,7 +94,7 @@ func TestGetValue(t *testing.T) { wantErr: false, }, { - name: "given a array path", + name: "given an array path", yml: ` foo: - bar: 1`, @@ -96,7 +103,17 @@ foo: wantErr: false, }, { - name: "given a array path with wildcard", + name: "given an entire array path", + yml: ` +foo: +- bar: 1 +- baz: 2`, + path: "$.foo", + want: []interface{}{map[string]interface{}{"bar": uint64(1)}, map[string]interface{}{"baz": uint64(2)}}, + wantErr: false, + }, + { + name: "given an entire array path with wildcard", yml: ` foo: - bar: 1 From 3fb9d68be99aa6cdfe59c07ff20aef7c9385a74a Mon Sep 17 00:00:00 2001 From: nakabonne Date: Fri, 22 Jan 2021 15:09:14 +0900 Subject: [PATCH 2/3] Replace NOTE with TODO --- pkg/yamlprocessor/yamlprocessor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/yamlprocessor/yamlprocessor.go b/pkg/yamlprocessor/yamlprocessor.go index 7084dda56a..ed209a3656 100644 --- a/pkg/yamlprocessor/yamlprocessor.go +++ b/pkg/yamlprocessor/yamlprocessor.go @@ -49,7 +49,7 @@ func GetValue(yml []byte, path string) (interface{}, error) { return nil, err } - // NOTE: Validate value in YAML before actual reading + // TODO: Validate value in YAML before actual reading // because it panics if: // - the value is -0. // - unexistence path given. From 2863def509b732fdef81ea5228a88189fb322621 Mon Sep 17 00:00:00 2001 From: nakabonne Date: Fri, 22 Jan 2021 15:49:42 +0900 Subject: [PATCH 3/3] Fix nits --- pkg/app/piped/eventwatcher/eventwatcher.go | 4 ++-- pkg/app/piped/eventwatcher/eventwatcher_test.go | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go index 47cd8232fb..bd1022047d 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher.go +++ b/pkg/app/piped/eventwatcher/eventwatcher.go @@ -224,7 +224,7 @@ func (w *watcher) modifyFiles(ctx context.Context, event *config.EventWatcherEve } value, err := convertStr(v) if err != nil { - return nil, fmt.Errorf("a value of unknown type (%v) is defined at %s in %s", err, r.YAMLField, r.File) + return nil, fmt.Errorf("a value of unknown type is defined at %s in %s: %w", err, r.YAMLField, r.File) } if latestEvent.Data == value { // Already up-to-date. @@ -270,7 +270,7 @@ func convertStr(value interface{}) (out string, err error) { case bool: out = strconv.FormatBool(v) default: - return "", fmt.Errorf("%T", v) + err = fmt.Errorf("failed to convert %T into string", v) } return } diff --git a/pkg/app/piped/eventwatcher/eventwatcher_test.go b/pkg/app/piped/eventwatcher/eventwatcher_test.go index 4076fe0354..a1d43261f8 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher_test.go +++ b/pkg/app/piped/eventwatcher/eventwatcher_test.go @@ -51,12 +51,6 @@ func TestGetValue(t *testing.T) { want: "1", wantErr: false, }, - { - name: "uint64", - value: uint64(1), - want: "1", - wantErr: false, - }, { name: "float64", value: 1.1,