diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index 896a0409..a01cc207 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -130,15 +130,16 @@ Options: RequiredFileKeyWorkflow: workflowFile, } - requiredFilesAbsPaths, err := loadfile.ContextAbsFilepaths(dir, requiredFiles) + requiredFilesAbsPaths, err := loadfile.AbsPathsWithContext(dir, requiredFiles) if err != nil { + flag.Usage() tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %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() @@ -151,17 +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") - var requiredFilesAbsSlice = make([]string, len(requiredFiles)) + var requiredFilesAbsSlice = make([]string, len(requiredFilesAbsPaths)) + var j int for _, f := range requiredFilesAbsPaths { - requiredFilesAbsSlice = append(requiredFilesAbsSlice, f) + 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) } diff --git a/loadfile/loadfile.go b/loadfile/loadfile.go index ce4c0912..242b5188 100644 --- a/loadfile/loadfile.go +++ b/loadfile/loadfile.go @@ -7,32 +7,33 @@ import ( "path/filepath" ) -// LoadContext reads the contents at each file into a map where the key -// is the absolute filepath and file contents is the value. +// 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("failed to obtain absolute path of file %s (%w)", filepath.Base(filePath), err) + return nil, fmt.Errorf("error obtaining absolute path of file %s (%w)", + filePath, err) } - fileData, err := os.ReadFile(absPath) //nolint:gosec + fileData, err := os.ReadFile(filepath.Clean(absPath)) if err != nil { - return nil, fmt.Errorf("failed to read file from context directory: %s (%w)", absPath, err) + return nil, fmt.Errorf("error reading file %s (%w)", absPath, err) } result[absPath] = fileData } return result, err } -// ContextAbsFilepaths creates a map of absolute filepaths. If a required +// 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 ContextAbsFilepaths(rootDir string, requiredFiles map[string]string) (map[string]string, error) { +func AbsPathsWithContext(rootDir string, requiredFiles map[string]string) (map[string]string, error) { absDir, err := filepath.Abs(rootDir) if err != nil { - return nil, err + return nil, fmt.Errorf("error determining context directory absolute path %s (%w)", rootDir, err) } requiredFilesAbs := map[string]string{} for key, f := range requiredFiles { diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 7e572457..24954564 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -22,39 +22,86 @@ func TestLoadContext(t *testing.T) { assert.NoError(t, os.MkdirAll(testdir, os.ModePerm)) - // create a directory and a file + // create a directory dirname := "mydir" dirpath := filepath.Join(testdir, dirname) - filename := "myfile" assert.NoError(t, os.MkdirAll(dirpath, os.ModePerm)) - f, err := os.CreateTemp(testdir, filename) + + // create a file + filename := "myfile" + filePath := filepath.Join(testdir, filename) + f, err := os.Create(filepath.Clean(filePath)) assert.NoError(t, err) - tempfilepath := f.Name() assert.NoError(t, f.Close()) - // create symlinks to the above directory and file + // create symlink to the directory symlinkDirname := dirname + "_sym" - symlinkFilepath := tempfilepath + "_sym" symlinkDirpath := filepath.Join(testdir, symlinkDirname) assert.NoError(t, os.Symlink(dirpath, symlinkDirpath)) - assert.NoError(t, os.Symlink(tempfilepath, symlinkFilepath)) + + // create symlink to the file + symlinkFilepath := filePath + "_sym" + assert.NoError(t, os.Symlink(filePath, symlinkFilepath)) neededFiles := []string{ - tempfilepath, + filePath, symlinkFilepath, } filemap, err := loadfile.LoadContext(neededFiles) - filemapExp := map[string][]byte{ - tempfilepath: {}, - symlinkFilepath: {}, - } // assert no error on attempting to read files // that cannot be read assert.NoError(t, err) - // assert only the regular file will be loaded + // 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, + } + _, err = loadfile.LoadContext(neededFiles) + assert.Error(t, err) + + // error on loading a symlink directory + neededFiles = []string{ + symlinkDirpath, + } + _, err = loadfile.LoadContext(neededFiles) + assert.Error(t, err) + + t.Cleanup(func() { + assert.NoError(t, os.RemoveAll(testdir)) + }) +} + +// This tests AbsPathsWithContext 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"]), + // since the 'b' file has an absolute path, it should be unmodified + "b": "/b.toml", + "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)) })