Fix load context bug

cmd/arcaflow/main.go

@@ -5,16 +5,13 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"os"
-	"os/signal"
-	"path/filepath"
-	"strings"
-
 	"go.arcalot.io/log/v2"
-
 	"go.flow.arcalot.io/engine"
 	"go.flow.arcalot.io/engine/config"
+	"go.flow.arcalot.io/engine/loadfile"
 	"gopkg.in/yaml.v3"
+	"os"
+	"os/signal"
 )
 
 // These variables are filled using ldflags during the build process with Goreleaser.
@@ -38,6 +35,15 @@ const ExitCodeWorkflowErrorOutput = 2
 // ExitCodeWorkflowFailed indicates that the workflow execution failed.
 const ExitCodeWorkflowFailed = 3
 
+// RequiredFileKeyWorkflow is the key for the workflow file in hash map of files required for execution.
+const RequiredFileKeyWorkflow = "workflow"
+
+// RequiredFileKeyConfig is the key for the config file in hash map of files required for execution.
+const RequiredFileKeyConfig = "config"
+
+// RequiredFileKeyInput is the key for the input file in hash map of files required for execution.
+const RequiredFileKeyInput = "input"
+
 func main() {
 	tempLogger := log.New(log.Config{
 		Level:       log.LevelInfo,
@@ -117,9 +123,23 @@ Options:
 	}
 
 	var err error
+
+	requiredFiles := map[string]string{
+		RequiredFileKeyConfig:   configFile,
+		RequiredFileKeyInput:    input,
+		RequiredFileKeyWorkflow: workflowFile,
+	}
+
+	requiredFilesAbsPaths, err := loadfile.AbsPathsWithContext(dir, requiredFiles)
+	if err != nil {
+		flag.Usage()
+		tempLogger.Errorf("context path resolution failed %s (%v)", dir, err)
+		os.Exit(ExitCodeInvalidData)
+	}
+
 	var configData any = map[any]any{}
 	if configFile != "" {
-		configData, err = loadYamlFile(configFile)
+		configData, err = loadYamlFile(requiredFilesAbsPaths[RequiredFileKeyConfig])
 		if err != nil {
 			tempLogger.Errorf("Failed to load configuration file %s (%v)", configFile, err)
 			flag.Usage()
@@ -132,13 +152,20 @@ Options:
 		flag.Usage()
 		os.Exit(ExitCodeInvalidData)
 	}
-	cfg.Log.Stdout = os.Stderr
 
+	// now we are ready to instantiate our main logger
+	cfg.Log.Stdout = os.Stderr
 	logger := log.New(cfg.Log).WithLabel("source", "main")
 
-	dirContext, err := loadContext(dir)
+	var requiredFilesAbsSlice = make([]string, len(requiredFilesAbsPaths))
+	var j int
+	for _, f := range requiredFilesAbsPaths {
+		requiredFilesAbsSlice[j] = f
+		j++
+	}
+	dirContext, err := loadfile.LoadContext(requiredFilesAbsSlice)
 	if err != nil {
-		logger.Errorf("Failed to load configuration file %s (%v)", configFile, err)
+		logger.Errorf("Failed to load required files into context (%v)", err)
 		flag.Usage()
 		os.Exit(ExitCodeInvalidData)
 	}
@@ -160,7 +187,7 @@ Options:
 		}
 	}
 
-	os.Exit(runWorkflow(flow, dirContext, workflowFile, logger, inputData))
+	os.Exit(runWorkflow(flow, dirContext, requiredFilesAbsPaths[RequiredFileKeyWorkflow], logger, inputData))
 }
 
 func runWorkflow(flow engine.WorkflowEngine, dirContext map[string][]byte, workflowFile string, logger log.Logger, inputData []byte) int {
@@ -224,32 +251,6 @@ func handleOSInterrupt(ctrlC chan os.Signal, cancel context.CancelFunc, logger l
 	os.Exit(1)
 }
 
-func loadContext(dir string) (map[string][]byte, error) {
-	absDir, err := filepath.Abs(dir)
-	if err != nil {
-		return nil, fmt.Errorf("failed to obtain absolute path of context directory %s (%w)", dir, err)
-	}
-	result := map[string][]byte{}
-	err = filepath.Walk(absDir,
-		func(path string, i os.FileInfo, err error) error {
-			if err != nil {
-				return err
-			}
-
-			if !i.IsDir() {
-				fileData, err := os.ReadFile(path) //nolint:gosec
-				if err != nil {
-					return fmt.Errorf("failed to read file from context directory: %s (%w)", path, err)
-				}
-				path = strings.TrimPrefix(path, absDir)
-				path = strings.TrimPrefix(path, string([]byte{os.PathSeparator}))
-				result[path] = fileData
-			}
-			return nil
-		})
-	return result, err
-}
-
 func loadYamlFile(configFile string) (any, error) {
 	fileContents, err := os.ReadFile(configFile) //nolint:gosec
 	if err != nil {

loadfile/loadfile.go

@@ -0,0 +1,47 @@
+// Package loadfile provides functions to load files from a directory.
+package loadfile
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// LoadContext reads the content of each file into a map where the key
+// is the absolute filepath and the file content is the value.
+func LoadContext(neededFilepaths []string) (map[string][]byte, error) {
+	result := map[string][]byte{}
+	var err error
+	for _, filePath := range neededFilepaths {
+		absPath, err := filepath.Abs(filePath)
+		if err != nil {
+			return nil, fmt.Errorf("error obtaining absolute path of file %s (%w)",
+				filePath, err)
+		}
+		fileData, err := os.ReadFile(filepath.Clean(absPath))
+		if err != nil {
+			return nil, fmt.Errorf("error reading file %s (%w)", absPath, err)
+		}
+		result[absPath] = fileData
+	}
+	return result, err
+}
+
+// AbsPathsWithContext creates a map of absolute filepaths. If a required
+// file is not provided with an absolute path, then it is joined with the
+// root directory.
+func AbsPathsWithContext(rootDir string, requiredFiles map[string]string) (map[string]string, error) {
+	absDir, err := filepath.Abs(rootDir)
+	if err != nil {
+		return nil, fmt.Errorf("error determining context directory absolute path %s (%w)", rootDir, err)
+	}
+	requiredFilesAbs := map[string]string{}
+	for key, f := range requiredFiles {
+		abspath := f
+		if !filepath.IsAbs(f) {
+			abspath = filepath.Join(absDir, f)
+		}
+		requiredFilesAbs[key] = abspath
+	}
+	return requiredFilesAbs, nil
+}

loadfile/loadfile_test.go

@@ -0,0 +1,113 @@
+package loadfile_test
+
+import (
+	"go.arcalot.io/assert"
+	"go.flow.arcalot.io/engine/loadfile"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// This tests the functional behavior of LoadContext when it is given
+// a directory of files that cover more than the directory file node
+// type 'd' (as seen in *nix systems). Specifically, this test adds
+// symbolic link files for a regular file and a directory file. The
+// engine should only attempt to read files of a type that the os can
+// read (i.e. not throw an error on a call to os.ReadFile()), and
+// disregard (not throw an error) files with a type it cannot read.
+func TestLoadContext(t *testing.T) {
+	testdir := "/tmp/loadfile-test"
+	// cleanup directory even if it's there
+	_ = os.RemoveAll(testdir)
+
+	assert.NoError(t, os.MkdirAll(testdir, os.ModePerm))
+
+	// create a directory
+	dirname := "mydir"
+	dirpath := filepath.Join(testdir, dirname)
+	assert.NoError(t, os.MkdirAll(dirpath, os.ModePerm))
+
+	// create a file
+	filename := "myfile"
+	filePath := filepath.Join(testdir, filename)
+	f, err := os.Create(filepath.Clean(filePath))
+	assert.NoError(t, err)
+	assert.NoError(t, f.Close())
+
+	// create symlink to the directory
+	symlinkDirname := dirname + "_sym"
+	symlinkDirpath := filepath.Join(testdir, symlinkDirname)
+	assert.NoError(t, os.Symlink(dirpath, symlinkDirpath))
+
+	// create symlink to the file
+	symlinkFilepath := filePath + "_sym"
+	assert.NoError(t, os.Symlink(filePath, symlinkFilepath))
+
+	neededFiles := []string{
+		filePath,
+		symlinkFilepath,
+	}
+	filemap, err := loadfile.LoadContext(neededFiles)
+	// assert no error on attempting to read files
+	// that cannot be read
+	assert.NoError(t, err)
+
+	// assert only the regular and symlinked file are loaded
+	filemapExp := map[string][]byte{
+		filePath:        {},
+		symlinkFilepath: {},
+	}
+	assert.Equals(t, filemap, filemapExp)
+
+	// error on loading a directory
+	neededFiles = []string{
+		dirpath,
+	}
+
+	errFileRead := "reading file"
+	ctxFiles, err := loadfile.LoadContext(neededFiles)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), errFileRead)
+	assert.Equals(t, ctxFiles, nil)
+
+	// error on loading a symlink directory
+	neededFiles = []string{
+		symlinkDirpath,
+	}
+	ctxFiles, err = loadfile.LoadContext(neededFiles)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), errFileRead)
+	assert.Equals(t, ctxFiles, nil)
+
+	t.Cleanup(func() {
+		assert.NoError(t, os.RemoveAll(testdir))
+	})
+}
+
+// This tests AbsPathsWithContext which joins relative paths
+// with the context (root) directory, and passes through
+// absolute paths unmodified.
+func TestContextAbsFilepaths(t *testing.T) {
+	testdir, err := os.MkdirTemp(os.TempDir(), "")
+	assert.NoError(t, err)
+
+	testFilepaths := map[string]string{
+		"a": "a.yaml",
+		"b": "/b.toml",
+		"c": "rel/../subdir/c.txt",
+	}
+
+	absPathsExp := map[string]string{
+		"a": filepath.Join(testdir, testFilepaths["a"]),
+		"b": testFilepaths["b"],
+		"c": filepath.Join(testdir, testFilepaths["c"]),
+	}
+
+	absPathsGot, err := loadfile.AbsPathsWithContext(testdir, testFilepaths)
+	assert.NoError(t, err)
+	assert.Equals(t, absPathsExp, absPathsGot)
+
+	t.Cleanup(func() {
+		assert.NoError(t, os.RemoveAll(testdir))
+	})
+}