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

@@ -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 {
 

functions/go/search-replace/pathparser.go → functions/go/search-replace/searchreplace/pathparser.go

@@ -1,4 +1,4 @@
-package main
+package searchreplace
 
 import (
 	"strings"

functions/go/search-replace/pathparser_test.go → functions/go/search-replace/searchreplace/pathparser_test.go

@@ -1,4 +1,4 @@
-package main
+package searchreplace
 
 import (
 	"testing"

functions/go/search-replace/putcommentcases_test.go → functions/go/search-replace/searchreplace/putcommentcases_test.go

@@ -1,4 +1,4 @@
-package main
+package searchreplace
 
 var putPatternCases = []test{
 	{

functions/go/search-replace/search_replace.go → 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
+}

functions/go/search-replace/search_replace_test.go → 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()
+	}
+
+}

functions/go/search-replace/searchreplacecases_test.go → 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`,
+	},
 }

functions/go/search-replace/walk.go → functions/go/search-replace/searchreplace/walk.go

@@ -1,4 +1,4 @@
-package main
+package searchreplace
 
 import (
 	"fmt"