Refactor and optimize function to set all setters

kyaml/setters2/set.go

@@ -23,6 +23,9 @@ type Set struct {
 
 	// Count is the number of fields that were updated by calling Filter
 	Count int
+
+	// SetAll if set to true will set all setters regardless of name
+	SetAll bool
 }
 
 // Filter implements Set as a yaml.Filter
@@ -30,13 +33,19 @@ func (s *Set) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	return object, accept(s, object)
 }
 
+// isMatch returns true if the setter with name should have the field
+// value set
+func (s *Set) isMatch(name string) bool {
+	return s.SetAll || s.Name == name
+}
+
 // visitSequence will perform setters for sequences
 func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error {
 	ext, err := getExtFromComment(schema)
 	if err != nil {
 		return err
 	}
-	if ext == nil || ext.Setter == nil || ext.Setter.Name != s.Name ||
+	if ext == nil || ext.Setter == nil || !s.isMatch(ext.Setter.Name) ||
 		len(ext.Setter.ListValues) == 0 {
 		// setter was not invoked for this sequence
 		return nil
@@ -125,7 +134,7 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (
 			p = strings.ReplaceAll(p, v.Marker, subSetter.Setter.Value)
 		}
 
-		if subSetter.Setter.Name == s.Name {
+		if s.isMatch(subSetter.Setter.Name) {
 			// the substitution depends on the specified setter
 			nameMatch = true
 		}
@@ -148,7 +157,7 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (
 // set applies the value from ext to field if its name matches s.Name
 func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) bool {
 	// check full setter
-	if ext.Setter == nil || ext.Setter.Name != s.Name {
+	if ext.Setter == nil || !s.isMatch(ext.Setter.Name) {
 		return false
 	}
 

kyaml/setters2/settersutil/fieldsetter.go

@@ -69,35 +69,31 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) {
 	return s.Count, err
 }
 
-// SetAllSetterDefinitions reads all the Setter Definitions from OpenAPI in source
-// package and sets all setter values in destination packages with out updating
-// destination packages openAPI files
-func SetAllSetterDefinitions(sourcePkgPath, sourcePkgOpenAPIPath string, destDirs ...string) error {
-	// get all the setter definitions from package
-	l := setters2.List{}
-	err := l.List(sourcePkgOpenAPIPath, sourcePkgPath)
-	if err != nil {
+// SetAllSetterDefinitions reads all the Setter Definitions from the OpenAPI
+// file and sets all values in the provided directories.
+func SetAllSetterDefinitions(openAPIPath string, dirs ...string) error {
+	if err := openapi.AddSchemaFromFile(openAPIPath); err != nil {
 		return err
 	}
 
-	// for each setter definition set the setter values in destination packages
-	//TODO(pmarupaka): optimize to perform all the setters in single pass instead of N passes
-	for _, sd := range l.Setters {
-		for _, destDir := range destDirs {
-			fs := FieldSetter{
-				Name:        sd.Name,
-				Value:       sd.Value,
-				ListValues:  sd.ListValues,
-				Description: sd.Description,
-				SetBy:       sd.SetBy,
+	for _, dir := range dirs {
+		rw := &kio.LocalPackageReadWriter{
+			PackagePath: dir,
+			// set output won't include resources from files which
+			//weren't modified.  make sure we don't delete them.
+			NoDeleteFiles: true,
 		}
-			// pass sourcePkgOpenAPIPath remains unchanged due to set but should be passed as
-			// a place holder
-			_, err = fs.Set(sourcePkgOpenAPIPath, destDir)
+
+		// apply all of the setters to the directory
+		err := kio.Pipeline{
+			Inputs: []kio.Reader{rw},
+			// Set all of the setters
+			Filters: []kio.Filter{setters2.SetAll(&setters2.Set{SetAll: true})},
+			Outputs: []kio.Writer{rw},
+		}.Execute()
 		if err != nil {
 			return err
 		}
 	}
-	}
 	return nil
 }

kyaml/setters2/settersutil/fieldsetter_test.go

@@ -52,17 +52,17 @@ spec:
 
 	srcDir, err := ioutil.TempDir("", "")
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
 	destDir1, err := ioutil.TempDir("", "")
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
 	destDir2, err := ioutil.TempDir("", "")
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
 	defer os.RemoveAll(srcDir)
@@ -71,32 +71,36 @@ spec:
 
 	err = ioutil.WriteFile(srcDir+"/OpenAPIFile", []byte(srcOpenAPIFile), 0600)
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 	err = ioutil.WriteFile(destDir1+"/destFile.yaml", []byte(destFile1), 0600)
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
 	err = ioutil.WriteFile(destDir2+"/destFile.yaml", []byte(destFile2), 0600)
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
-	err = SetAllSetterDefinitions(srcDir, srcDir+"/OpenAPIFile", destDir1, destDir2)
+	err = SetAllSetterDefinitions(srcDir+"/OpenAPIFile", destDir1, destDir2)
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
 	}
 
 	actualdestFile1, err := ioutil.ReadFile(destDir1 + "/destFile.yaml")
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
+	}
+	if !assert.Equal(t, strings.Trim(string(actualdestFile1), "\n"), expectedDestFile) {
+		t.FailNow()
 	}
-	assert.Equal(t, strings.Trim(string(actualdestFile1), "\n"), expectedDestFile)
 
 	actualdestFile2, err := ioutil.ReadFile(destDir2 + "/destFile.yaml")
 	if !assert.NoError(t, err) {
-		t.Fail()
+		t.FailNow()
+	}
+	if !assert.Equal(t, strings.Trim(string(actualdestFile2), "\n"), expectedDestFile) {
+		t.FailNow()
 	}
-	assert.Equal(t, strings.Trim(string(actualdestFile2), "\n"), expectedDestFile)
 }