From 9887328c583497c3bed0af1d214231827f6cb92d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Wed, 26 Feb 2020 14:49:43 +0100 Subject: [PATCH 1/2] pkg/util/walkers: refactor CopyWalker and add some tests So at least some changes to this function can be easily tested. Signed-off-by: Mateusz Gozdek --- pkg/util/walkers/copying.go | 28 +++++++++++------- pkg/util/walkers/copying_test.go | 50 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 pkg/util/walkers/copying_test.go diff --git a/pkg/util/walkers/copying.go b/pkg/util/walkers/copying.go index e2e9d3807..49f502b0d 100644 --- a/pkg/util/walkers/copying.go +++ b/pkg/util/walkers/copying.go @@ -15,6 +15,7 @@ package walkers import ( + "fmt" "io" "os" "path/filepath" @@ -36,16 +37,23 @@ func CopyingWalker(path string, newDirPerms os.FileMode) assets.WalkFunc { return errors.Wrap(err, "failed to create dir") } - // TODO: If we start packing binaries, make sure they have executable bit set. - targetFile, err := os.OpenFile(fileName, os.O_RDWR|os.O_CREATE, 0644) - if err != nil { - return errors.Wrap(err, "failed to open target file") - } - defer targetFile.Close() + return writeFile(fileName, r) + } +} - if _, err := io.Copy(targetFile, r); err != nil { - return errors.Wrap(err, "failed to write file") - } - return nil +// writeFile writes data from given io.Reader to the file and makes sure, that +// this is the only content stored in the file. +func writeFile(p string, r io.Reader) error { + // TODO: If we start packing binaries, make sure they have executable bit set. + f, err := os.OpenFile(p, os.O_RDWR|os.O_CREATE, 0644) + if err != nil { + return fmt.Errorf("failed to open target file %s: %w", p, err) + } + defer f.Close() + + if _, err := io.Copy(f, r); err != nil { + return fmt.Errorf("failed writing to file %s: %w", p, err) } + + return nil } diff --git a/pkg/util/walkers/copying_test.go b/pkg/util/walkers/copying_test.go new file mode 100644 index 000000000..71773cb0c --- /dev/null +++ b/pkg/util/walkers/copying_test.go @@ -0,0 +1,50 @@ +// Copyright 2020 The Lokomotive Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package walkers + +import ( + "io/ioutil" + "os" + "reflect" + "strings" + "testing" +) + +const ( + tmpPattern = "lokomotive-test" + testData = "222" +) + +func TestWriteFile(t *testing.T) { + f, err := ioutil.TempFile("", tmpPattern) + if err != nil { + t.Fatalf("Creating temp file should succeed, got: %v", err) + } + + defer os.Remove(f.Name()) + + if err := writeFile(f.Name(), strings.NewReader(testData)); err != nil { + t.Fatalf("Writing to file should succeed, got: %v", err) + } + + d, err := ioutil.ReadFile(f.Name()) + if err != nil { + t.Fatalf("Reading tmp file should succeed, got: %v", err) + } + + if !reflect.DeepEqual(testData, string(d)) { + t.Fatalf("Expected: '%s', got '%s'", testData, string(d)) + } +} From 4d934df081eb986861adc6f2b360cf79355f0562 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Wed, 26 Feb 2020 14:50:56 +0100 Subject: [PATCH 2/2] pkg/util/walkers: truncate file when writing To avoid leaving old content untouched when updating files, which may happen if new content is smaller than the old one. Closes #40 Signed-off-by: Mateusz Gozdek --- pkg/util/walkers/copying.go | 2 +- pkg/util/walkers/copying_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/pkg/util/walkers/copying.go b/pkg/util/walkers/copying.go index 49f502b0d..dd82612b9 100644 --- a/pkg/util/walkers/copying.go +++ b/pkg/util/walkers/copying.go @@ -45,7 +45,7 @@ func CopyingWalker(path string, newDirPerms os.FileMode) assets.WalkFunc { // this is the only content stored in the file. func writeFile(p string, r io.Reader) error { // TODO: If we start packing binaries, make sure they have executable bit set. - f, err := os.OpenFile(p, os.O_RDWR|os.O_CREATE, 0644) + f, err := os.OpenFile(p, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) if err != nil { return fmt.Errorf("failed to open target file %s: %w", p, err) } diff --git a/pkg/util/walkers/copying_test.go b/pkg/util/walkers/copying_test.go index 71773cb0c..b4b2555db 100644 --- a/pkg/util/walkers/copying_test.go +++ b/pkg/util/walkers/copying_test.go @@ -48,3 +48,29 @@ func TestWriteFile(t *testing.T) { t.Fatalf("Expected: '%s', got '%s'", testData, string(d)) } } + +func TestWriteFileTruncate(t *testing.T) { + f, err := ioutil.TempFile("", tmpPattern) + if err != nil { + t.Fatalf("Creating temp file should succeed, got: %v", err) + } + + defer os.Remove(f.Name()) + + if err := writeFile(f.Name(), strings.NewReader("111111")); err != nil { + t.Fatalf("Writing to file should succeed, got: %v", err) + } + + if err := writeFile(f.Name(), strings.NewReader(testData)); err != nil { + t.Fatalf("Updating file should succeed, got: %v", err) + } + + d, err := ioutil.ReadFile(f.Name()) + if err != nil { + t.Fatalf("Reading tmp file should succeed, got: %v", err) + } + + if !reflect.DeepEqual(testData, string(d)) { + t.Fatalf("Expected: '%s', got '%s'", testData, string(d)) + } +}