diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 4279e6c91c..e36107787e 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/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) } } diff --git a/kyaml/kio/pkgio_writer.go b/kyaml/kio/pkgio_writer.go index b671512e33..a31abacb28 100644 --- a/kyaml/kio/pkgio_writer.go +++ b/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) } } diff --git a/kyaml/kio/pkgio_writer_test.go b/kyaml/kio/pkgio_writer_test.go index 497b5d7f9f..506fa8e49d 100644 --- a/kyaml/kio/pkgio_writer_test.go +++ b/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) - - w := LocalPackageWriter{PackagePath: d} - err := w.Write([]*yaml.RNode{node2, node1, node3}) - if !assert.NoError(t, err) { - assert.FailNow(t, err.Error()) - } - - 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 + testWriterOnDiskAndOnMem(t, func(t *testing.T, fs filesys.FileSystem) { + d, node1, node2, node3, cleanup := getWriterInputs(t, fs) + defer cleanup() + + w := LocalPackageWriter{ + PackagePath: d, + FileSystem: filesys.FileSystemOrOnDisk{FileSystem: fs}, + } + err := w.Write([]*yaml.RNode{node2, node1, node3}) + require.NoError(t, err) + + 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) - - w := LocalPackageWriter{PackagePath: d, KeepReaderAnnotations: true} - err := w.Write([]*yaml.RNode{node2, node1, node3}) - if !assert.NoError(t, err) { - return - } - - b, err := ioutil.ReadFile(filepath.Join(d, "a", "b", "a_test.yaml")) - if !assert.NoError(t, err) { - return - } - assert.Equal(t, `a: b #first + 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, + FileSystem: filesys.FileSystemOrOnDisk{FileSystem: fs}, + } + err := w.Write([]*yaml.RNode{node2, node1, node3}) + require.NoError(t, err) + + 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) - - 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()) - } - - 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 + 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"}, + FileSystem: filesys.FileSystemOrOnDisk{FileSystem: fs}, + } + err := w.Write([]*yaml.RNode{node2, node1, node3}) + require.NoError(t, err) + + 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()) - } - - 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") - } + require.NoError(t, err) + + 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()) - } - - 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") - } + require.NoError(t, err) + + 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) - - w := LocalPackageWriter{PackagePath: d} - err = w.Write([]*yaml.RNode{node2, node1, node3, node4}) - testutil.AssertErrorContains(t, err, "package paths may not be absolute paths") + node4, err := yaml.Parse(n4) + testutil.AssertNoError(t, err, n4) + + 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()) - } - - 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() - } + node4, err := yaml.Parse(node4String) + require.NoError(t, err) + + 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, + 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") + }) +} - 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") +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)) } } diff --git a/kyaml/kio/testing.go b/kyaml/kio/testing.go index 67471c6654..b973e9fbb0 100644 --- a/kyaml/kio/testing.go +++ b/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