Merge pull request #4086 from campoy/pkgio-writer-filesys

Add FileSystem interface to LocalPackageWriter

kyaml/kio/pkgio_reader.go

@@ -122,13 +122,14 @@ func (r *LocalPackageReadWriter) Write(nodes []*yaml.RNode) error {
 		PackagePath:           r.PackagePath,
 		ClearAnnotations:      clear,
 		KeepReaderAnnotations: r.KeepReaderAnnotations,
+		FileSystem:            r.FileSystem,
 	}.Write(nodes)
 	if err != nil {
 		return errors.Wrap(err)
 	}
 	deleteFiles := r.files.Difference(newFiles)
 	for f := range deleteFiles {
-		if err = os.Remove(filepath.Join(r.PackagePath, f)); err != nil {
+		if err = r.FileSystem.RemoveAll(filepath.Join(r.PackagePath, f)); err != nil {
 			return errors.Wrap(err)
 		}
 	}

kyaml/kio/pkgio_writer.go

@@ -4,12 +4,14 @@
 package kio
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
@@ -26,6 +28,9 @@ type LocalPackageWriter struct {
 
 	// ClearAnnotations will clear annotations before writing the resources
 	ClearAnnotations []string `yaml:"clearAnnotations,omitempty"`
+
+	// FileSystem can be used to mock the disk file system.
+	FileSystem filesys.FileSystemOrOnDisk
 }
 
 var _ Writer = LocalPackageWriter{}
@@ -36,9 +41,10 @@ func (r LocalPackageWriter) Write(nodes []*yaml.RNode) error {
 		return err
 	}
 
-	if s, err := os.Stat(r.PackagePath); err != nil {
-		return err
-	} else if !s.IsDir() {
+	if !r.FileSystem.Exists(r.PackagePath) {
+		return errors.WrapPrefixf(os.ErrNotExist, "could not write to %q", r.PackagePath)
+	}
+	if !r.FileSystem.IsDir(r.PackagePath) {
 		// if the user specified input isn't a directory, the package is the directory of the
 		// target
 		r.PackagePath = filepath.Dir(r.PackagePath)
@@ -65,45 +71,36 @@ func (r LocalPackageWriter) Write(nodes []*yaml.RNode) error {
 	// validate outputs before writing any
 	for path := range outputFiles {
 		outputPath := filepath.Join(r.PackagePath, path)
-		if st, err := os.Stat(outputPath); !os.IsNotExist(err) {
-			if err != nil {
-				return errors.Wrap(err)
-			}
-			if st.IsDir() {
-				return fmt.Errorf("config.kubernetes.io/path cannot be a directory: %s", path)
-			}
+		if r.FileSystem.IsDir(outputPath) {
+			return fmt.Errorf("config.kubernetes.io/path cannot be a directory: %s", path)
 		}
 
-		err = os.MkdirAll(filepath.Dir(outputPath), 0700)
+		err = r.FileSystem.MkdirAll(filepath.Dir(outputPath))
 		if err != nil {
 			return errors.Wrap(err)
 		}
 	}
 
 	// write files
+	buf := bytes.NewBuffer(nil)
 	for path := range outputFiles {
 		outputPath := filepath.Join(r.PackagePath, path)
-		err = os.MkdirAll(filepath.Dir(filepath.Join(r.PackagePath, path)), 0700)
+		err = r.FileSystem.MkdirAll(filepath.Dir(filepath.Join(r.PackagePath, path)))
 		if err != nil {
 			return errors.Wrap(err)
 		}
 
-		f, err := os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(0600))
-		if err != nil {
+		buf.Reset()
+		w := ByteWriter{
+			Writer:                buf,
+			KeepReaderAnnotations: r.KeepReaderAnnotations,
+			ClearAnnotations:      r.ClearAnnotations,
+		}
+		if err = w.Write(outputFiles[path]); err != nil {
 			return errors.Wrap(err)
 		}
-		if err := func() error {
-			defer f.Close()
-			w := ByteWriter{
-				Writer:                f,
-				KeepReaderAnnotations: r.KeepReaderAnnotations,
-				ClearAnnotations:      r.ClearAnnotations,
-			}
-			if err = w.Write(outputFiles[path]); err != nil {
-				return errors.Wrap(err)
-			}
-			return nil
-		}(); err != nil {
+
+		if err := r.FileSystem.WriteFile(outputPath, buf.Bytes()); err != nil {
 			return errors.Wrap(err)
 		}
 	}

kyaml/kio/pkgio_writer_test.go

@@ -5,14 +5,18 @@ package kio_test
 
 import (
 	"fmt"
-	"io/ioutil"
+	"math/rand"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
+	"time"
 
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/testutil"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	. "sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
@@ -20,53 +24,53 @@ import (
 // TestLocalPackageWriter_Write tests:
 // - ReaderAnnotations are cleared when writing the Resources
 func TestLocalPackageWriter_Write(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	w := LocalPackageWriter{PackagePath: d}
-	err := w.Write([]*yaml.RNode{node2, node1, node3})
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err := w.Write([]*yaml.RNode{node2, node1, node3})
+		require.NoError(t, err)
 
-	b, err := ioutil.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	assert.Equal(t, `a: b #first
+		b, err := fs.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `a: b #first
 ---
 c: d # second
 `, string(b))
 
-	b, err = ioutil.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	assert.Equal(t, `e: f
+		b, err = fs.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `e: f
 g:
   h:
   - i # has a list
   - j
 `, string(b))
+	})
 }
 
 // TestLocalPackageWriter_Write_keepReaderAnnotations tests:
 // - ReaderAnnotations are kept when writing the Resources
 func TestLocalPackageWriter_Write_keepReaderAnnotations(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	w := LocalPackageWriter{PackagePath: d, KeepReaderAnnotations: true}
-	err := w.Write([]*yaml.RNode{node2, node1, node3})
-	if !assert.NoError(t, err) {
-		return
-	}
+		w := LocalPackageWriter{
+			PackagePath:           d,
+			KeepReaderAnnotations: true,
+			FileSystem:            filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err := w.Write([]*yaml.RNode{node2, node1, node3})
+		require.NoError(t, err)
 
-	b, err := ioutil.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
-	if !assert.NoError(t, err) {
-		return
-	}
-	assert.Equal(t, `a: b #first
+		b, err := fs.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `a: b #first
 metadata:
   annotations:
     config.kubernetes.io/index: 0
@@ -79,11 +83,9 @@ metadata:
     config.kubernetes.io/path: "a/b/a_test.yaml"
 `, string(b))
 
-	b, err = ioutil.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
-	if !assert.NoError(t, err) {
-		return
-	}
-	assert.Equal(t, `e: f
+		b, err = fs.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `e: f
 g:
   h:
   - i # has a list
@@ -93,48 +95,50 @@ metadata:
     config.kubernetes.io/index: 0
     config.kubernetes.io/path: "a/b/b_test.yaml"
 `, string(b))
+	})
 }
 
 // TestLocalPackageWriter_Write_clearAnnotations tests:
 // - ClearAnnotations are removed from Resources
 func TestLocalPackageWriter_Write_clearAnnotations(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	w := LocalPackageWriter{PackagePath: d, ClearAnnotations: []string{"config.kubernetes.io/mode"}}
-	err := w.Write([]*yaml.RNode{node2, node1, node3})
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		w := LocalPackageWriter{
+			PackagePath:      d,
+			ClearAnnotations: []string{"config.kubernetes.io/mode"},
+			FileSystem:       filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err := w.Write([]*yaml.RNode{node2, node1, node3})
+		require.NoError(t, err)
 
-	b, err := ioutil.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	assert.Equal(t, `a: b #first
+		b, err := fs.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `a: b #first
 ---
 c: d # second
 `, string(b))
 
-	b, err = ioutil.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	assert.Equal(t, `e: f
+		b, err = fs.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, `e: f
 g:
   h:
   - i # has a list
   - j
 `, string(b))
+	})
 }
 
 // TestLocalPackageWriter_Write_failRelativePath tests:
 // - If a relative path above the package is defined, write fails
 func TestLocalPackageWriter_Write_failRelativePath(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	node4, err := yaml.Parse(`e: f
+		node4, err := yaml.Parse(`e: f
 g:
   h:
   - i # has a list
@@ -144,24 +148,27 @@ metadata:
     config.kubernetes.io/index: 0
     config.kubernetes.io/path: "a/b/../../../b_test.yaml"
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		require.NoError(t, err)
 
-	w := LocalPackageWriter{PackagePath: d}
-	err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
-	if assert.Error(t, err) {
-		assert.Contains(t, err.Error(), "resource must be written under package")
-	}
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
+		if assert.Error(t, err) {
+			assert.Contains(t, err.Error(), "resource must be written under package")
+		}
+	})
 }
 
 // TestLocalPackageWriter_Write_invalidIndex tests:
 // - If a non-int index is given, fail
 func TestLocalPackageWriter_Write_invalidIndex(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	node4, err := yaml.Parse(`e: f
+		node4, err := yaml.Parse(`e: f
 g:
   h:
   - i # has a list
@@ -171,26 +178,29 @@ metadata:
     config.kubernetes.io/index: a
     config.kubernetes.io/path: "a/b/b_test.yaml" # use a different path, should still collide
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		require.NoError(t, err)
 
-	w := LocalPackageWriter{PackagePath: d}
-	err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
-	if assert.Error(t, err) {
-		assert.Contains(t, err.Error(), "unable to parse config.kubernetes.io/index")
-	}
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
+		if assert.Error(t, err) {
+			assert.Contains(t, err.Error(), "unable to parse config.kubernetes.io/index")
+		}
+	})
 }
 
 // TestLocalPackageWriter_Write_absPath tests:
 // - If config.kubernetes.io/path is absolute, fail
 func TestLocalPackageWriter_Write_absPath(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	d = filepath.ToSlash(d)
+		d = filepath.ToSlash(d)
 
-	n4 := fmt.Sprintf(`e: f
+		n4 := fmt.Sprintf(`e: f
 g:
   h:
   - i # has a list
@@ -200,21 +210,26 @@ metadata:
     config.kubernetes.io/index: a
     config.kubernetes.io/path: "%s/a/b/b_test.yaml" # use a different path, should still collide
 `, d)
-	node4, err := yaml.Parse(n4)
-	testutil.AssertNoError(t, err, n4)
+		node4, err := yaml.Parse(n4)
+		testutil.AssertNoError(t, err, n4)
 
-	w := LocalPackageWriter{PackagePath: d}
-	err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
-	testutil.AssertErrorContains(t, err, "package paths may not be absolute paths")
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
+		testutil.AssertErrorContains(t, err, "package paths may not be absolute paths")
+	})
 }
 
 // TestLocalPackageWriter_Write_missingPath tests:
 // - If config.kubernetes.io/path or index are missing, then default them
 func TestLocalPackageWriter_Write_missingAnnotations(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	node4String := `e: f
+		node4String := `e: f
 g:
   h:
   - i # has a list
@@ -223,32 +238,29 @@ kind: Foo
 metadata:
   name: bar
 `
-	node4, err := yaml.Parse(node4String)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		node4, err := yaml.Parse(node4String)
+		require.NoError(t, err)
 
-	w := LocalPackageWriter{PackagePath: d}
-	err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
-	b, err := ioutil.ReadFile(filepath.Join(d, "foo_bar.yaml"))
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
-	if !assert.Equal(t, node4String, string(b)) {
-		t.FailNow()
-	}
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
+		require.NoError(t, err)
+		b, err := fs.ReadFile(filepath.Join(d, "foo_bar.yaml"))
+		require.NoError(t, err)
+		require.Equal(t, node4String, string(b))
+	})
 }
 
 // TestLocalPackageWriter_Write_pathIsDir tests:
 // - If  config.kubernetes.io/path is a directory, fail
 func TestLocalPackageWriter_Write_pathIsDir(t *testing.T) {
-	d, node1, node2, node3 := getWriterInputs(t)
-	defer os.RemoveAll(d)
+	testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) {
+		d, node1, node2, node3, cleanup := getWriterInputs(t, fs)
+		defer cleanup()
 
-	node4, err := yaml.Parse(`e: f
+		node4, err := yaml.Parse(`e: f
 g:
   h:
   - i # has a list
@@ -258,36 +270,41 @@ metadata:
     config.kubernetes.io/path: a/
     config.kubernetes.io/index: 0
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+		require.NoError(t, err)
 
-	w := LocalPackageWriter{PackagePath: d}
-	err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
-	if assert.Error(t, err) {
-		assert.Contains(t, err.Error(), "config.kubernetes.io/path cannot be a directory")
+		w := LocalPackageWriter{
+			PackagePath: d,
+			FileSystem:  filesys.FileSystemOrOnDisk{FileSystem: fs},
+		}
+		err = w.Write([]*yaml.RNode{node2, node1, node3, node4})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "config.kubernetes.io/path cannot be a directory")
+	})
+}
+
+func testWriterOnDiskAndOnMem(t *testing.T, f func(t *testing.T, fs filesys.FileSystem)) {
+	t.Run("on_disk", func(t *testing.T) { f(t, filesys.MakeFsOnDisk()) })
+	// TODO: Once fsnode supports Windows, these tests should also be run.
+	if runtime.GOOS != "windows" {
+		t.Run("on_mem", func(t *testing.T) { f(t, filesys.MakeFsInMemory()) })
 	}
 }
 
-func getWriterInputs(t *testing.T) (string, *yaml.RNode, *yaml.RNode, *yaml.RNode) {
+func getWriterInputs(t *testing.T, mockFS filesys.FileSystem) (string, *yaml.RNode, *yaml.RNode, *yaml.RNode, func()) {
 	node1, err := yaml.Parse(`a: b #first
 metadata:
   annotations:
     config.kubernetes.io/index: 0
     config.kubernetes.io/path: "a/b/a_test.yaml"
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 	node2, err := yaml.Parse(`c: d # second
 metadata:
   annotations:
     config.kubernetes.io/index: 1
     config.kubernetes.io/path: "a/b/a_test.yaml"
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 	node3, err := yaml.Parse(`e: f
 g:
   h:
@@ -298,15 +315,11 @@ metadata:
     config.kubernetes.io/index: 0
     config.kubernetes.io/path: "a/b/b_test.yaml"
 `)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	d, err := ioutil.TempDir("", "kyaml-test")
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
-	if !assert.NoError(t, os.MkdirAll(filepath.Join(d, "a"), 0700)) {
-		assert.FailNow(t, "")
-	}
-	return d, node1, node2, node3
+	require.NoError(t, err)
+
+	// These two lines are similar to calling ioutil.TempDir, but we don't actually create any directory.
+	rand.Seed(time.Now().Unix())
+	path := filepath.Join(os.TempDir(), fmt.Sprintf("kyaml-test%d", rand.Int31()))
+	require.NoError(t, mockFS.MkdirAll(filepath.Join(path, "a")))
+	return path, node1, node2, node3, func() { require.NoError(t, mockFS.RemoveAll(path)) }
 }

kyaml/kio/testing.go

@@ -9,7 +9,7 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // Setup creates directories and files for testing
@@ -21,18 +21,12 @@ type Setup struct {
 // setupDirectories creates directories for reading test configuration from
 func SetupDirectories(t *testing.T, dirs ...string) Setup {
 	d, err := ioutil.TempDir("", "kyaml-test")
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 	err = os.Chdir(d)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 	for _, s := range dirs {
 		err = os.MkdirAll(s, 0700)
-		if !assert.NoError(t, err) {
-			assert.FailNow(t, err.Error())
-		}
+		require.NoError(t, err)
 	}
 	return Setup{Root: d}
 }
@@ -40,13 +34,9 @@ func SetupDirectories(t *testing.T, dirs ...string) Setup {
 // writeFile writes a file under the test directory
 func (s Setup) WriteFile(t *testing.T, path string, value []byte) {
 	err := os.MkdirAll(filepath.Dir(filepath.Join(s.Root, path)), 0700)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 	err = ioutil.WriteFile(filepath.Join(s.Root, path), value, 0600)
-	if !assert.NoError(t, err) {
-		assert.FailNow(t, err.Error())
-	}
+	require.NoError(t, err)
 }
 
 // clean deletes the test config