From 16fdb9cfbbcfd896a424d750b83b2c3421811c8c Mon Sep 17 00:00:00 2001 From: phani Date: Mon, 3 May 2021 17:23:48 -0700 Subject: [PATCH] Separate api layer from cmd/function and validate inputs (#264) * Validate input cases for Search and Replace * Suggested changes * Update package name --- functions/go/search-replace/main.go | 32 +++------ .../{ => searchreplace}/pathparser.go | 2 +- .../{ => searchreplace}/pathparser_test.go | 2 +- .../putcommentcases_test.go | 2 +- .../{ => searchreplace}/search_replace.go | 69 +++++++++++++++++-- .../search_replace_test.go | 26 ++++++- .../searchreplacecases_test.go | 49 ++++++++++++- .../{ => searchreplace}/walk.go | 2 +- 8 files changed, 146 insertions(+), 38 deletions(-) rename functions/go/search-replace/{ => searchreplace}/pathparser.go (99%) rename functions/go/search-replace/{ => searchreplace}/pathparser_test.go (98%) rename functions/go/search-replace/{ => searchreplace}/putcommentcases_test.go (99%) rename functions/go/search-replace/{ => searchreplace}/search_replace.go (84%) rename functions/go/search-replace/{ => searchreplace}/search_replace_test.go (79%) rename functions/go/search-replace/{ => searchreplace}/searchreplacecases_test.go (92%) rename functions/go/search-replace/{ => searchreplace}/walk.go (98%) diff --git a/functions/go/search-replace/main.go b/functions/go/search-replace/main.go index 258a6ab57..4d5e1f72b 100644 --- a/functions/go/search-replace/main.go +++ b/functions/go/search-replace/main.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/search-replace/searchreplace" "github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/search-replace/generated" "sigs.k8s.io/kustomize/kyaml/fn/framework" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" @@ -36,7 +37,7 @@ func main() { } } -// run resolves the function params and runs the function on resources +// run resolves the function params from input ResourceList and runs the function on resources func run(resourceList *framework.ResourceList) ([]framework.Item, error) { sr, err := getSearchReplaceParams(resourceList.FunctionConfig) if err != nil { @@ -52,8 +53,8 @@ func run(resourceList *framework.ResourceList) ([]framework.Item, error) { } // getSearchReplaceParams retrieve the search parameters from input config -func getSearchReplaceParams(fc interface{}) (SearchReplace, error) { - var fcd SearchReplace +func getSearchReplaceParams(fc interface{}) (searchreplace.SearchReplace, error) { + var fcd searchreplace.SearchReplace f, ok := fc.(map[string]interface{}) if !ok { return fcd, fmt.Errorf("function config %#v is not valid", fc) @@ -63,32 +64,15 @@ func getSearchReplaceParams(fc interface{}) (SearchReplace, error) { return fcd, fmt.Errorf("failed to parse input from function config: %w", err) } - decode(rn, &fcd) - return fcd, nil -} - -// decode decodes the input yaml node into SearchReplace struct -func decode(rn *kyaml.RNode, fcd *SearchReplace) { - dm := rn.GetDataMap() - fcd.ByPath = getValue(dm, "by-path") - fcd.ByValue = getValue(dm, "by-value") - fcd.ByValueRegex = getValue(dm, "by-value-regex") - fcd.PutValue = getValue(dm, "put-value") - fcd.PutComment = getValue(dm, "put-comment") -} - -// getValue returns the value for 'key' in map 'm' -// returns empty string if 'key' doesn't exist in 'm' -func getValue(m map[string]string, key string) string { - if val, ok := m[key]; ok { - return val + if err := searchreplace.Decode(rn, &fcd); err != nil { + return fcd, err } - return "" + return fcd, nil } // searchResultsToItems converts the Search and Replace results to // equivalent items([]framework.Item) -func searchResultsToItems(sr SearchReplace) []framework.Item { +func searchResultsToItems(sr searchreplace.SearchReplace) []framework.Item { var items []framework.Item for _, res := range sr.Results { diff --git a/functions/go/search-replace/pathparser.go b/functions/go/search-replace/searchreplace/pathparser.go similarity index 99% rename from functions/go/search-replace/pathparser.go rename to functions/go/search-replace/searchreplace/pathparser.go index 64f4f9968..f7f571d6c 100644 --- a/functions/go/search-replace/pathparser.go +++ b/functions/go/search-replace/searchreplace/pathparser.go @@ -1,4 +1,4 @@ -package main +package searchreplace import ( "strings" diff --git a/functions/go/search-replace/pathparser_test.go b/functions/go/search-replace/searchreplace/pathparser_test.go similarity index 98% rename from functions/go/search-replace/pathparser_test.go rename to functions/go/search-replace/searchreplace/pathparser_test.go index 414fe315d..3f77edaeb 100644 --- a/functions/go/search-replace/pathparser_test.go +++ b/functions/go/search-replace/searchreplace/pathparser_test.go @@ -1,4 +1,4 @@ -package main +package searchreplace import ( "testing" diff --git a/functions/go/search-replace/putcommentcases_test.go b/functions/go/search-replace/searchreplace/putcommentcases_test.go similarity index 99% rename from functions/go/search-replace/putcommentcases_test.go rename to functions/go/search-replace/searchreplace/putcommentcases_test.go index e1327e573..0d243a9d8 100644 --- a/functions/go/search-replace/putcommentcases_test.go +++ b/functions/go/search-replace/searchreplace/putcommentcases_test.go @@ -1,4 +1,4 @@ -package main +package searchreplace var putPatternCases = []test{ { diff --git a/functions/go/search-replace/search_replace.go b/functions/go/search-replace/searchreplace/search_replace.go similarity index 84% rename from functions/go/search-replace/search_replace.go rename to functions/go/search-replace/searchreplace/search_replace.go index 507e1e1e8..97acd1901 100644 --- a/functions/go/search-replace/search_replace.go +++ b/functions/go/search-replace/searchreplace/search_replace.go @@ -1,4 +1,4 @@ -package main +package searchreplace import ( "fmt" @@ -7,10 +7,23 @@ import ( "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" + "sigs.k8s.io/kustomize/kyaml/sets" "sigs.k8s.io/kustomize/kyaml/yaml" ) -const PathDelimiter = "." +const ( + ByValue = "by-value" + ByValueRegex = "by-value-regex" + ByPath = "by-path" + PutValue = "put-value" + PutComment = "put-comment" + PathDelimiter = "." +) + +// matchers returns the list of supported matchers +func matchers() []string { + return []string{ByValue, ByValueRegex, ByPath, PutValue, PutComment} +} // SearchReplace struct holds the input parameters and results for // Search and Replace operations on resource configs @@ -46,20 +59,20 @@ type SearchReplace struct { // SearchResult holds result of search and replace operation type SearchResult struct { - // file path of the matching field + // FilePath is the file path of the matching field FilePath string - // field path of the matching field + // FieldPath is field path of the matching field FieldPath string - // value of the matching field + // Value of the matching field Value string } // Filter performs the search and replace operation on all input nodes func (sr *SearchReplace) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { - if sr.ByValue != "" && sr.ByValueRegex != "" { - return nodes, errors.Errorf(`only one of ["by-value", "by-value-regex"] can be provided`) + if err := sr.validateMatchers(); err != nil { + return nodes, err } // compile regex once so that it can be used everywhere @@ -89,6 +102,9 @@ func (sr *SearchReplace) Perform(object *yaml.RNode) (*yaml.RNode, error) { return object, err } sr.filePath = filePath + if err != nil { + return object, err + } // check if value should be put by path and process it directly without needing // to traverse all elements of the node @@ -346,3 +362,42 @@ func (sr *SearchReplace) resultsString() string { out += fmt.Sprintf("%s %d field(s)\n", action, sr.Count) return out } + +// Decode decodes the input yaml RNode into SearchReplace struct +// returns error if input yaml RNode contains invalid matcher name inputs +func Decode(rn *yaml.RNode, fcd *SearchReplace) error { + dm := rn.GetDataMap() + if err := validateMatcherNames(dm); err != nil { + return err + } + fcd.ByPath = dm[ByPath] + fcd.ByValue = dm[ByValue] + fcd.ByValueRegex = dm[ByValueRegex] + fcd.PutValue = dm[PutValue] + fcd.PutComment = dm[PutComment] + return nil +} + +// validateMatcherNames validates the input matcher names +func validateMatcherNames(m map[string]string) error { + matcherSet := sets.String{} + matcherSet.Insert(matchers()...) + for key := range m { + if !matcherSet.Has(key) { + return errors.Errorf("invalid matcher %q, must be one of %q", key, matchers()) + } + } + return nil +} + +// validateMatchers validates the input matchers in SearchReplace struct +func (sr *SearchReplace) validateMatchers() error { + if sr.ByValue == "" && sr.ByValueRegex == "" && sr.ByPath == "" { + return errors.Errorf(`at least one of [%q, %q, %q] must be provided`, ByValue, ByValueRegex, ByPath) + } + + if sr.ByValue != "" && sr.ByValueRegex != "" { + return errors.Errorf(`only one of [%q, %q] can be provided`, ByValue, ByValueRegex) + } + return nil +} diff --git a/functions/go/search-replace/search_replace_test.go b/functions/go/search-replace/searchreplace/search_replace_test.go similarity index 79% rename from functions/go/search-replace/search_replace_test.go rename to functions/go/search-replace/searchreplace/search_replace_test.go index 896b36173..b79233f97 100644 --- a/functions/go/search-replace/search_replace_test.go +++ b/functions/go/search-replace/searchreplace/search_replace_test.go @@ -1,4 +1,4 @@ -package main +package searchreplace import ( "io/ioutil" @@ -47,12 +47,15 @@ func TestSearchCommand(t *testing.T) { if !assert.NoError(t, err) { t.FailNow() } - decode(node, s) inout := &kio.LocalPackageReadWriter{ PackagePath: baseDir, NoDeleteFiles: true, PackageFileName: "Kptfile", } + err = Decode(node, s) + if !assert.NoError(t, err) { + t.FailNow() + } err = kio.Pipeline{ Inputs: []kio.Reader{inout}, Filters: []kio.Filter{s}, @@ -90,3 +93,22 @@ func TestSearchCommand(t *testing.T) { } } } + +func TestDecode(t *testing.T) { + rn, err := kyaml.Parse(`data: + by-value: foo + put-values: bar`) + if !assert.NoError(t, err) { + t.FailNow() + } + + err = Decode(rn, &SearchReplace{}) + if !assert.Error(t, err) { + t.FailNow() + } + expected := `invalid matcher "put-values", must be one of ["by-value" "by-value-regex" "by-path" "put-value" "put-comment"]` + if !assert.Equal(t, expected, err.Error()) { + t.FailNow() + } + +} diff --git a/functions/go/search-replace/searchreplacecases_test.go b/functions/go/search-replace/searchreplace/searchreplacecases_test.go similarity index 92% rename from functions/go/search-replace/searchreplacecases_test.go rename to functions/go/search-replace/searchreplace/searchreplacecases_test.go index 10a76ccff..06e67e179 100644 --- a/functions/go/search-replace/searchreplacecases_test.go +++ b/functions/go/search-replace/searchreplace/searchreplacecases_test.go @@ -1,4 +1,4 @@ -package main +package searchreplace var searchReplaceCases = []test{ { @@ -727,4 +727,51 @@ spec: `, errMsg: `only one of ["by-value", "by-value-regex"] can be provided`, }, + { + name: "error when none of the search matchers are provided", + config: ` +data: ~ +`, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + errMsg: `at least one of ["by-value", "by-value-regex", "by-path"] must be provided`, + }, + { + name: "error when none of the required search matchers are provided", + config: ` +data: + put-value: foo +`, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + errMsg: `at least one of ["by-value", "by-value-regex", "by-path"] must be provided`, + }, } diff --git a/functions/go/search-replace/walk.go b/functions/go/search-replace/searchreplace/walk.go similarity index 98% rename from functions/go/search-replace/walk.go rename to functions/go/search-replace/searchreplace/walk.go index 48cee66b6..36ca163f1 100644 --- a/functions/go/search-replace/walk.go +++ b/functions/go/search-replace/searchreplace/walk.go @@ -1,4 +1,4 @@ -package main +package searchreplace import ( "fmt"