Screen files, and output only those updated

This is intended to address two problems:

 - LocalPackage{Reader,Writer} like to reformat the YAML that passes
   through them; mostly this is harmless, but occasionally it will end
   up fighting format tooling, e.g., prettier.

 - It's possible that things like Helm chart templates are lying
   around in the git repository to which automation is applied. Those
   templates have extensions of ".yaml" but are not usually parseable
   as YAML, so would result in errors from the file reader.

This commit changes how updates are run -- firstly, it screens files
by checking for a token (`"$imagepolicy"`) that will be present in
files that might need updating. This cheaply removes some nodes --
likely including Helm chart templates -- from consideration.

Secondly, it now only writes files that were actually updated by an
imagepolicy setter, rather than writing everything that was an
input. This means it's less likely to reformat something that doesn't
need to be touched at all.

Signed-off-by: Michael Bridgen <[email protected]>

Author: squaremo

pkg/update/filereader.go

@@ -0,0 +1,114 @@
+package update
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"sigs.k8s.io/kustomize/kyaml/kio"
+	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
+	"sigs.k8s.io/kustomize/kyaml/yaml"
+)
+
+// ScreeningReader is a kio.Reader that includes only files that are
+// pertinent to automation. In practice this means looking for a
+// particular token in each file, and ignoring those files without the
+// token. This avoids most problematic cases -- e.g., templates in a
+// Helm chart, which won't parse as YAML -- and cheaply filters for
+// only those files that need processing.
+type ScreeningLocalReader struct {
+	Token string
+	Path  string
+
+	// This records the relative path of each file that passed
+	// screening (i.e., contained the token), but couldn't be parsed.
+	ProblemFiles []string
+}
+
+// Read scans the .Path recursively for files that contain .Token, and
+// parses any that do. It applies the filename annotation used by
+// [`kio.LocalPackageWriter`](https://godoc.org/sigs.k8s.io/kustomize/kyaml/kio#LocalPackageWriter)
+// so that the same will write files back to their original
+// location. The implementation follows that of
+// [LocalPackageReader.Read](https://godoc.org/sigs.k8s.io/kustomize/kyaml/kio#LocalPackageReader.Read),
+// adapting lightly (mainly to leave features out).
+func (r *ScreeningLocalReader) Read() ([]*yaml.RNode, error) {
+	if r.Path == "" {
+		return nil, fmt.Errorf("must supply path to scan for files")
+	}
+
+	root, err := filepath.Abs(r.Path)
+	if err != nil {
+		return nil, fmt.Errorf("path field cannot be made absolute: %w", err)
+	}
+
+	// For the filename annotation, I want a directory for filenames
+	// to be relative to; but I don't know whether path is a directory
+	// or file yetm so this must wait until the body of the filepath.Walk.
+	var relativePath string
+
+	tokenbytes := []byte(r.Token)
+
+	var result []*yaml.RNode
+	err = filepath.Walk(root, func(p string, info os.FileInfo, err error) error {
+		if err != nil {
+			return fmt.Errorf("walking path for files: %w", err)
+		}
+
+		if p == root {
+			if info.IsDir() {
+				relativePath = p
+				return nil // keep walking
+			}
+			relativePath = filepath.Dir(p)
+		}
+
+		if info.IsDir() {
+			return nil
+		}
+
+		if ext := filepath.Ext(p); ext != ".yaml" && ext != ".yml" {
+			return nil
+		}
+
+		// To check for the token, I need the file contents. This
+		// assumes the file is encoded as UTF8.
+		filebytes, err := ioutil.ReadFile(p)
+		if err != nil {
+			return fmt.Errorf("reading YAML file: %w", err)
+		}
+
+		if !bytes.Contains(filebytes, tokenbytes) {
+			return nil
+		}
+
+		path, err := filepath.Rel(relativePath, p)
+		if err != nil {
+			return fmt.Errorf("relativising path: %w", err)
+		}
+		annotations := map[string]string{
+			kioutil.PathAnnotation: path,
+		}
+
+		rdr := &kio.ByteReader{
+			Reader:         bytes.NewBuffer(filebytes),
+			SetAnnotations: annotations,
+		}
+
+		nodes, err := rdr.Read()
+		// Having screened the file and decided it's worth examining,
+		// an error at this point is most unfortunate. However, it
+		// doesn't need to be the end of the matter; we can record
+		// this file as problematic, and continue.
+		if err != nil {
+			r.ProblemFiles = append(r.ProblemFiles, path)
+			return nil
+		}
+		result = append(result, nodes...)
+		return nil
+	})
+
+	return result, err
+}

pkg/update/filereader_test.go

@@ -0,0 +1,34 @@
+package update
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
+)
+
+var _ = Describe("load YAMLs with ScreeningLocalReader", func() {
+	It("loads only the YAMLs containing the token", func() {
+		r := ScreeningLocalReader{
+			Path:  "testdata/setters/original",
+			Token: "$imagepolicy",
+		}
+		nodes, err := r.Read()
+		Expect(err).ToNot(HaveOccurred())
+		// the test fixture has three files that contain the marker:
+		// - otherns.yaml
+		// - marked.yaml
+		// - kustomization.yaml
+		Expect(len(nodes)).To(Equal(3))
+		filesSeen := map[string]struct{}{}
+		for i := range nodes {
+			path, _, err := kioutil.GetFileAnnotations(nodes[i])
+			Expect(err).ToNot(HaveOccurred())
+			filesSeen[path] = struct{}{}
+		}
+		Expect(filesSeen).To(Equal(map[string]struct{}{
+			"marked.yaml":        struct{}{},
+			"kustomization.yaml": struct{}{},
+			"otherns.yaml":       struct{}{},
+		}))
+	})
+})

pkg/update/setters.go

@@ -36,6 +36,9 @@ func resetSchema() {
 	openapi.SuppressBuiltInSchemaUse()
 }
 
+// UpdateWithSetters takes all YAML files from `inpath`, updates any
+// that contain an "in scope" image policy marker, and writes files it
+// updated (and only those files) back to `outpath`.
 func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect.ImagePolicy) error {
 	// the OpenAPI schema is a package variable in kyaml/openapi. In
 	// lieu of being able to isolate invocations (per
@@ -104,9 +107,9 @@ func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect.
 	}
 
 	// get ready with the reader and writer
-	reader := &kio.LocalPackageReader{
-		PackagePath:        inpath,
-		IncludeSubpackages: true,
+	reader := &ScreeningLocalReader{
+		Path:  inpath,
+		Token: fmt.Sprintf("%q", SetterShortHand),
 	}
 	writer := &kio.LocalPackageWriter{
 		PackagePath: outpath,
@@ -115,7 +118,12 @@ func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect.
 	pipeline := kio.Pipeline{
 		Inputs:  []kio.Reader{reader},
 		Outputs: []kio.Writer{writer},
-		Filters: []kio.Filter{kio.FilterAll(&setters2.Set{SetAll: true})},
+		Filters: []kio.Filter{
+			setters2.SetAll( // run the enclosed single-node setters2.Filter on all nodes,
+				// and only include those in files that changed in the output
+				&setters2.Set{SetAll: true}, // set all images that are in the constructed schema
+			),
+		},
 	}
 
 	// go!