From d603afc199c693ca058bab5c60b8d45c829b82ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Mon, 25 Nov 2019 09:00:17 +0100 Subject: [PATCH 1/2] Include positions filename in the error when YAML unmarshal fails. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- pkg/promtail/positions/positions.go | 6 +- pkg/promtail/positions/positions_test.go | 85 ++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 pkg/promtail/positions/positions_test.go diff --git a/pkg/promtail/positions/positions.go b/pkg/promtail/positions/positions.go index b1c2cc55f08d0..1b3201af9d586 100644 --- a/pkg/promtail/positions/positions.go +++ b/pkg/promtail/positions/positions.go @@ -2,6 +2,7 @@ package positions import ( "flag" + "fmt" "io/ioutil" "os" "path/filepath" @@ -181,7 +182,8 @@ func (p *Positions) cleanup() { } func readPositionsFile(filename string) (map[string]string, error) { - buf, err := ioutil.ReadFile(filepath.Clean(filename)) + cleanfn := filepath.Clean(filename) + buf, err := ioutil.ReadFile(cleanfn) if err != nil { if os.IsNotExist(err) { return map[string]string{}, nil @@ -191,7 +193,7 @@ func readPositionsFile(filename string) (map[string]string, error) { var p File if err := yaml.UnmarshalStrict(buf, &p); err != nil { - return nil, err + return nil, fmt.Errorf("%s: %v", cleanfn, err) } return p.Positions, nil diff --git a/pkg/promtail/positions/positions_test.go b/pkg/promtail/positions/positions_test.go new file mode 100644 index 0000000000000..866d624a6f85e --- /dev/null +++ b/pkg/promtail/positions/positions_test.go @@ -0,0 +1,85 @@ +package positions + +import ( + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func tempFilename(t *testing.T) string { + t.Helper() + + temp, err := ioutil.TempFile("", "positions") + if err != nil { + t.Fatal("tempFilename:", err) + } + err = temp.Close() + if err != nil { + t.Fatal("tempFilename:", err) + } + + name := temp.Name() + err = os.Remove(name) + if err != nil { + t.Fatal("tempFilename:", err) + } + + return name +} + +func TestReadPositionsOK(t *testing.T) { + temp := tempFilename(t) + defer func() { + _ = os.Remove(temp) + }() + + yaml := []byte(`positions: + /tmp/random.log: "17623" +`) + err := ioutil.WriteFile(temp, yaml, 0644) + if err != nil { + t.Fatal(err) + } + + pos, err := readPositionsFile(temp) + require.NoError(t, err) + require.Equal(t, "17623", pos["/tmp/random.log"]) +} + +func TestReadPositionsFromDir(t *testing.T) { + temp := tempFilename(t) + err := os.Mkdir(temp, 0644) + if err != nil { + t.Fatal(err) + } + + defer func() { + _ = os.Remove(temp) + }() + + _, err = readPositionsFile(temp) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), temp)) // error must contain filename +} + +func TestReadPositionsFromBadYaml(t *testing.T) { + temp := tempFilename(t) + defer func() { + _ = os.Remove(temp) + }() + + badYaml := []byte(`positions: + /tmp/random.log: "176 +`) + err := ioutil.WriteFile(temp, badYaml, 0644) + if err != nil { + t.Fatal(err) + } + + _, err = readPositionsFile(temp) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), temp)) // error must contain filename +} From 803ab280c7feba656c224f52f7d875551027300b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Mon, 25 Nov 2019 09:31:50 +0100 Subject: [PATCH 2/2] Include filename when parsing of YAML/JSON file fails. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- pkg/cfg/cfg.go | 2 +- pkg/cfg/files.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 789c733a8075b..dd4bd14af913c 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -29,7 +29,7 @@ func Unmarshal(dst interface{}, sources ...Source) error { for _, source := range sources { if err := source(dst); err != nil { - return errors.Wrap(err, "sourcing") + return err } } return nil diff --git a/pkg/cfg/files.go b/pkg/cfg/files.go index 7cedbc81fd529..afd35a9850c22 100644 --- a/pkg/cfg/files.go +++ b/pkg/cfg/files.go @@ -5,7 +5,8 @@ import ( "flag" "io/ioutil" - yaml "gopkg.in/yaml.v2" + "github.com/pkg/errors" + "gopkg.in/yaml.v2" ) // JSON returns a Source that opens the supplied `.json` file and loads it. @@ -20,7 +21,8 @@ func JSON(f *string) Source { return err } - return dJSON(j)(dst) + err = dJSON(j)(dst) + return errors.Wrap(err, *f) } } @@ -43,7 +45,8 @@ func YAML(f *string) Source { return err } - return dYAML(y)(dst) + err = dYAML(y)(dst) + return errors.Wrap(err, *f) } }