fix: misleading error for missing version field

cmd/lint.go

@@ -59,7 +59,7 @@ func LintCmd(lg log.Logger, mt *metrics.StatsdMonitor) *cobra.Command {
 				Logger:           lg,
 			})
 
-			recipes, err := recipe.NewReader("").Read(args[0])
+			recipes, err := recipe.NewReader(lg, "").Read(args[0])
 			if err != nil {
 				return err
 			}

cmd/run.go

@@ -81,7 +81,7 @@ func RunCmd(lg log.Logger, mt *metrics.StatsdMonitor, cfg config.Config) *cobra.
 			ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 			defer stop()
 
-			recipes, err := recipe.NewReader(pathToConfig).Read(args[0])
+			recipes, err := recipe.NewReader(lg, pathToConfig).Read(args[0])
 			if err != nil {
 				return err
 			}

recipe/reader.go

@@ -3,30 +3,31 @@ package recipe
 import (
 	"bytes"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"text/template"
 
 	"github.com/odpf/meteor/generator"
+	"github.com/odpf/salt/log"
 	"gopkg.in/yaml.v3"
 )
 
 // Reader is a struct that reads recipe files.
 type Reader struct {
 	data map[string]string
+	log  log.Logger
 }
 
 var (
 	ErrInvalidRecipeVersion = errors.New("recipe version is invalid or not found")
 )
 
 // NewReader returns a new Reader.
-func NewReader(pathToConfig string) *Reader {
+func NewReader(lg log.Logger, pathToConfig string) *Reader {
 	reader := &Reader{}
 	reader.data = populateData(pathToConfig)
-
+	reader.log = lg
 	return reader
 }
 
@@ -38,7 +39,7 @@ func (r *Reader) Read(path string) (recipes []Recipe, err error) {
 	}
 	switch mode := fi.Mode(); {
 	case mode.IsDir():
-		recipes, err = r.readDir(path)
+		recipes, err = r.readDir(r.log, path)
 		if err != nil {
 			return nil, err
 		}
@@ -90,15 +91,17 @@ func (r *Reader) readFile(path string) (recipe Recipe, err error) {
 	return
 }
 
-func (r *Reader) readDir(path string) (recipes []Recipe, err error) {
+func (r *Reader) readDir(lg log.Logger, path string) (recipes []Recipe, err error) {
 	entries, err := os.ReadDir(path)
 	if err != nil {
 		return
 	}
 
 	for _, entry := range entries {
-		recipe, err := r.readFile(filepath.Join(path, entry.Name()))
+		x := filepath.Join(path, entry.Name())
+		recipe, err := r.readFile(x)
 		if err != nil {
+			lg.Warn("skipping file", "path", x, "err", err.Error())
 			continue
 		}
 
@@ -112,5 +115,5 @@ func validateRecipeVersion(receivedVersion, expectedVersion string) (err error)
 	if strings.Compare(receivedVersion, expectedVersion) == 0 {
 		return
 	}
-	return fmt.Errorf("received recipe version %s does not match to the expected version %s", receivedVersion, expectedVersion)
+	return ErrInvalidRecipeVersion
 }

recipe/reader_test.go

@@ -17,22 +17,22 @@ var (
 
 func TestReaderRead(t *testing.T) {
 	t.Run("should return error if file is not found", func(t *testing.T) {
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 
 		_, err := reader.Read("./wrong-path.yaml")
 		assert.NotNil(t, err)
 	})
 
 	t.Run("should return error if recipe is not parsed correctly", func(t *testing.T) {
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 
 		_, err := reader.Read("./testdata/wrong-format.txt")
 		assert.NotNil(t, err)
 	})
 
 	t.Run("should return recipe from a path given in parameter", func(t *testing.T) {
 		t.Run("where recipe has a name", func(t *testing.T) {
-			reader := recipe.NewReader(emptyConfigPath)
+			reader := recipe.NewReader(testLog, emptyConfigPath)
 
 			recipes, err := reader.Read("./testdata/testdir/test-recipe.yaml")
 			if err != nil {
@@ -62,7 +62,7 @@ func TestReaderRead(t *testing.T) {
 		})
 
 		t.Run("where recipe does not have a name", func(t *testing.T) {
-			reader := recipe.NewReader(emptyConfigPath)
+			reader := recipe.NewReader(testLog, emptyConfigPath)
 
 			recipes, err := reader.Read("./testdata/testdir/test-recipe-no-name.yaml")
 			if err != nil {
@@ -101,7 +101,7 @@ func TestReaderRead(t *testing.T) {
 			os.Unsetenv("METEOR_SOURCE_PASSWORD")
 		}()
 
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 		recipes, err := reader.Read("./testdata/testdir/test-recipe-variables.yaml")
 		if err != nil {
 			t.Fatal(err)
@@ -137,13 +137,13 @@ func TestReaderRead(t *testing.T) {
 	})
 
 	t.Run("should return error if directory is not found", func(t *testing.T) {
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 		_, err := reader.Read("./testdata/wrong-dir")
 		assert.NotNil(t, err)
 	})
 
 	t.Run("should return error if path is not a directory", func(t *testing.T) {
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 		_, err := reader.Read("./testdata/wrong-format.txt")
 		assert.NotNil(t, err)
 	})
@@ -156,7 +156,7 @@ func TestReaderRead(t *testing.T) {
 			os.Unsetenv("METEOR_SOURCE_PASSWORD")
 		}()
 
-		reader := recipe.NewReader(emptyConfigPath)
+		reader := recipe.NewReader(testLog, emptyConfigPath)
 		results, err := reader.Read("./testdata/testdir")
 		if err != nil {
 			t.Fatal(err)
@@ -224,7 +224,7 @@ func TestReaderRead(t *testing.T) {
 
 	// Testing populateData() with various environment configs!!
 	t.Run("should read config file in current directory", func(t *testing.T) {
-		reader := recipe.NewReader("sample_config.yaml")
+		reader := recipe.NewReader(testLog, "sample_config.yaml")
 		results, err := reader.Read("./testdata/testdir/test-recipe-variables.yaml")
 		if err != nil {
 			t.Fatal(err)
@@ -255,7 +255,7 @@ func TestReaderRead(t *testing.T) {
 	})
 
 	t.Run("should read config file in other directory", func(t *testing.T) {
-		reader := recipe.NewReader("testdata/config2.yaml")
+		reader := recipe.NewReader(testLog, "testdata/config2.yaml")
 		results, err := reader.Read("./testdata/testdir/test-recipe-variables.yaml")
 		if err != nil {
 			t.Fatal(err)
@@ -286,12 +286,15 @@ func TestReaderRead(t *testing.T) {
 	})
 
 	t.Run("should return error if version is missing/incorrect", func(t *testing.T) {
-		reader := recipe.NewReader("testdata/config2.yaml")
+		reader := recipe.NewReader(testLog, "testdata/config2.yaml")
 		_, err := reader.Read("./testdata/missing-version.yaml")
 		errors.Is(err, recipe.ErrInvalidRecipeVersion)
 
 		_, err = reader.Read("./testdata/incorrect-version.yaml")
 		errors.Is(err, recipe.ErrInvalidRecipeVersion)
+
+		_, err = reader.Read("./testdata/dir_2") // error is logged in case of a directory
+		assert.Nil(t, err)
 	})
 }
 

recipe/recipe_test.go

@@ -5,13 +5,16 @@ import (
 	"testing"
 
 	"github.com/odpf/meteor/recipe"
+	"github.com/odpf/salt/log"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+var testLog = log.NewLogrus(log.LogrusWithLevel("info"))
+
 // TestRecipeGetLine tests recipe by line number
 func TestRecipeGetLine(t *testing.T) {
-	reader := recipe.NewReader("")
+	reader := recipe.NewReader(testLog, "")
 	r, err := reader.Read("./testdata/recipe-read-line.yaml")
 	require.NoError(t, err)
 	require.Len(t, r, 1)
@@ -58,7 +61,7 @@ func TestRecipeGetLine(t *testing.T) {
 
 // TestRecipeGetLineBySrcTypeTag tests recipe source with tag `type` by line number
 func TestRecipeGetLineBySrcTypeTag(t *testing.T) {
-	reader := recipe.NewReader("")
+	reader := recipe.NewReader(testLog, "")
 	r, err := reader.Read("./testdata/src- typeTag-recipe-read-line.yaml")
 	require.NoError(t, err)
 	require.Len(t, r, 1)

recipe/testdata/dir_2/missing-version.yaml

@@ -0,0 +1,7 @@
+name: test-recipe
+source:
+  name: test-source
+  config:
+    foo: bar
+sinks:
+  - name: test-sink