Copy files with relative path to active workspace directory of container

api/dto/dto.go

@@ -66,18 +66,14 @@ type File struct {
 	Content []byte   `json:"content"`
 }
 
-// ToAbsolute returns the absolute path of the FilePath with respect to the given basePath. If the FilePath already is absolute, basePath will be ignored.
+// Cleaned returns the cleaned path of the FilePath.
-func (f FilePath) ToAbsolute(basePath string) string {
+func (f FilePath) Cleaned() string {
-	filePathString := string(f)
+	return path.Clean(string(f))
-	if path.IsAbs(filePathString) {
-		return path.Clean(filePathString)
-	}
-	return path.Clean(path.Join(basePath, filePathString))
 }
 
-// AbsolutePath returns the absolute path of the file. See FilePath.ToAbsolute for details.
+// CleanedPath returns the cleaned path of the file.
-func (f File) AbsolutePath(basePath string) string {
+func (f File) CleanedPath() string {
-	return f.Path.ToAbsolute(basePath)
+	return f.Path.Cleaned()
 }
 
 // IsDirectory returns true iff the path of the File ends with a /.

config/config.go

@@ -32,9 +32,6 @@ var (
 			TLS:       false,
 			Namespace: "default",
 		},
-		Runner: runner{
-			WorkspacePath: "/home/python",
-		},
 		Logger: logger{
 			Level: "INFO",
 		},
@@ -68,11 +65,6 @@ type nomad struct {
 	Namespace string
 }
 
-// runner configures the runners on the executor
-type runner struct {
-	WorkspacePath string
-}
-
 // logger configures the used logger.
 type logger struct {
 	Level string
@@ -82,7 +74,6 @@ type logger struct {
 type configuration struct {
 	Server server
 	Nomad  nomad
-	Runner runner
 	Logger logger
 }
 

configuration.example.yaml

@@ -26,11 +26,6 @@ nomad:
   # Nomad namespace to use. If unset, 'default' is used
   namespace: poseidon
 
-# Configuration of the runners
-runner:
-  # Directory where all files with relative paths will be copied into. Must be writable by the default user in the container.
-  workspacepath: /home/python
-
 # Configuration of the logger
 logger:
   # Log level that is used after reading the config (INFO until then)

runner/runner.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
-	"gitlab.hpi.de/codeocean/codemoon/poseidon/config"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/nomad"
 	"io"
 	"strings"
@@ -28,7 +27,6 @@ const (
 
 var (
 	ErrorFileCopyFailed = errors.New("file copy failed")
-	FileCopyBasePath    = config.Config.Runner.WorkspacePath
 )
 
 type Runner interface {
@@ -111,7 +109,7 @@ func (r *NomadAllocation) UpdateFileSystem(copyRequest *dto.UpdateFileSystemRequ
 	}
 
 	fileDeletionCommand := fileDeletionCommand(copyRequest.Delete)
-	copyCommand := "tar --extract --absolute-names --verbose --directory=/ --file=/dev/stdin;"
+	copyCommand := "tar --extract --absolute-names --verbose --file=/dev/stdin;"
 	updateFileCommand := (&dto.ExecutionRequest{Command: fileDeletionCommand + copyCommand}).FullCommand()
 	stdOut := bytes.Buffer{}
 	stdErr := bytes.Buffer{}
@@ -155,15 +153,15 @@ func createTarArchiveForFiles(filesToCopy []dto.File, w io.Writer) error {
 	return tarWriter.Close()
 }
 
-func fileDeletionCommand(filesToDelete []dto.FilePath) string {
+func fileDeletionCommand(pathsToDelete []dto.FilePath) string {
-	if len(filesToDelete) == 0 {
+	if len(pathsToDelete) == 0 {
 		return ""
 	}
 	command := "rm --recursive --force "
-	for _, filePath := range filesToDelete {
+	for _, filePath := range pathsToDelete {
 		// To avoid command injection, filenames need to be quoted.
 		// See https://unix.stackexchange.com/questions/347332/what-characters-need-to-be-escaped-in-files-without-quotes for details.
-		singleQuoteEscapedFileName := strings.ReplaceAll(filePath.ToAbsolute(FileCopyBasePath), "'", "'\\''")
+		singleQuoteEscapedFileName := strings.ReplaceAll(filePath.Cleaned(), "'", "'\\''")
 		command += fmt.Sprintf("'%s' ", singleQuoteEscapedFileName)
 	}
 	command += ";"
@@ -174,13 +172,13 @@ func tarHeader(file dto.File) *tar.Header {
 	if file.IsDirectory() {
 		return &tar.Header{
 			Typeflag: tar.TypeDir,
-			Name:     file.AbsolutePath(FileCopyBasePath),
+			Name:     file.CleanedPath(),
 			Mode:     0755,
 		}
 	} else {
 		return &tar.Header{
 			Typeflag: tar.TypeReg,
-			Name:     file.AbsolutePath(FileCopyBasePath),
+			Name:     file.CleanedPath(),
 			Mode:     0744,
 			Size:     int64(len(file.Content)),
 		}

runner/runner_test.go

@@ -181,16 +181,15 @@ func (s *UpdateFileSystemTestSuite) TestFilesToCopyAreIncludedInTarArchive() {
 	s.Equal(tests.DefaultFileContent, tarFile.Content)
 }
 
-func (s *UpdateFileSystemTestSuite) TestFilesWithRelativePathArePutInDefaultLocation() {
+func (s *UpdateFileSystemTestSuite) TestTarFilesContainCorrectPathForRelativeFilePath() {
 	s.mockedExecuteCommandCall.Return(0, nil)
 	copyRequest := &dto.UpdateFileSystemRequest{Copy: []dto.File{{Path: tests.DefaultFileName, Content: []byte(tests.DefaultFileContent)}}}
 	_ = s.runner.UpdateFileSystem(copyRequest)
 
 	tarFiles := s.readFilesFromTarArchive(s.stdin)
 	s.Len(tarFiles, 1)
-	tarFile := tarFiles[0]
+	// tar is extracted in the active workdir of the container, file will be put relative to that
-	s.True(strings.HasSuffix(tarFile.Name, tests.DefaultFileName))
+	s.Equal(tests.DefaultFileName, tarFiles[0].Name)
-	s.True(strings.HasPrefix(tarFile.Name, FileCopyBasePath))
 }
 
 func (s *UpdateFileSystemTestSuite) TestFilesWithAbsolutePathArePutInAbsoluteLocation() {

tests/e2e/runners_test.go

@@ -7,7 +7,6 @@ import (
 	"github.com/gorilla/websocket"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/api"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
-	"gitlab.hpi.de/codeocean/codemoon/poseidon/runner"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/tests"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/tests/helpers"
 	"io"
@@ -74,13 +73,13 @@ func (s *E2ETestSuite) TestDeleteRunnerRoute() {
 }
 
 func (s *E2ETestSuite) TestCopyFilesRoute() {
-	runnerId, err := ProvideRunner(&dto.RunnerRequest{})
+	runnerID, err := ProvideRunner(&dto.RunnerRequest{})
 	s.NoError(err)
 	copyFilesRequestByteString, _ := json.Marshal(&dto.UpdateFileSystemRequest{
 		Copy: []dto.File{{Path: tests.DefaultFileName, Content: []byte(tests.DefaultFileContent)}},
 	})
 	sendCopyRequest := func(reader io.Reader) (*http.Response, error) {
-		return helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerId, api.UpdateFileSystemPath), "application/json", reader)
+		return helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath), "application/json", reader)
 	}
 
 	s.Run("File copy with valid payload succeeds", func() {
@@ -89,7 +88,33 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 		s.Equal(http.StatusNoContent, resp.StatusCode)
 
 		s.Run("File content can be printed on runner", func() {
-			s.Equal(tests.DefaultFileContent, s.ContentOfFileOnRunner(runnerId, tests.DefaultFileName))
+			s.Equal(tests.DefaultFileContent, s.PrintContentOfFileOnRunner(runnerID, tests.DefaultFileName))
+		})
+	})
+
+	s.Run("Files are put in correct location", func() {
+		relativeFilePath := "relative/file/path.txt"
+		relativeFileContent := "Relative file content"
+		absoluteFilePath := "/tmp/absolute/file/path.txt"
+		absoluteFileContent := "Absolute file content"
+		testFilePathsCopyRequestString, _ := json.Marshal(&dto.UpdateFileSystemRequest{
+			Copy: []dto.File{
+				{Path: dto.FilePath(relativeFilePath), Content: []byte(relativeFileContent)},
+				{Path: dto.FilePath(absoluteFilePath), Content: []byte(absoluteFileContent)},
+			},
+		})
+
+		resp, err := sendCopyRequest(bytes.NewReader(testFilePathsCopyRequestString))
+		s.NoError(err)
+		s.Equal(http.StatusNoContent, resp.StatusCode)
+
+		s.Run("File content of file with relative path can be printed on runner", func() {
+			// the print command is executed in the context of the default working directory of the container
+			s.Equal(relativeFileContent, s.PrintContentOfFileOnRunner(runnerID, relativeFilePath))
+		})
+
+		s.Run("File content of file with absolute path can be printed on runner", func() {
+			s.Equal(absoluteFileContent, s.PrintContentOfFileOnRunner(runnerID, absoluteFilePath))
 		})
 	})
 
@@ -103,7 +128,7 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 		s.Equal(http.StatusNoContent, resp.StatusCode)
 
 		s.Run("File content can no longer be printed", func() {
-			s.Contains(s.ContentOfFileOnRunner(runnerId, tests.DefaultFileName), "No such file or directory")
+			s.Contains(s.PrintContentOfFileOnRunner(runnerID, tests.DefaultFileName), "No such file or directory")
 		})
 	})
 
@@ -116,9 +141,10 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 		resp, err := sendCopyRequest(bytes.NewReader(copyFilesRequestByteString))
 		s.NoError(err)
 		s.Equal(http.StatusNoContent, resp.StatusCode)
+		_ = resp.Body.Close()
 
 		s.Run("File content can be printed on runner", func() {
-			s.Equal(tests.DefaultFileContent, s.ContentOfFileOnRunner(runnerId, tests.DefaultFileName))
+			s.Equal(tests.DefaultFileContent, s.PrintContentOfFileOnRunner(runnerID, tests.DefaultFileName))
 		})
 	})
 
@@ -138,14 +164,15 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 		err = json.NewDecoder(resp.Body).Decode(internalServerError)
 		s.NoError(err)
 		s.Contains(internalServerError.Message, "Cannot open: Permission denied")
+		_ = resp.Body.Close()
 
 		s.Run("File content can be printed on runner", func() {
-			s.Equal(string(newFileContent), s.ContentOfFileOnRunner(runnerId, tests.DefaultFileName))
+			s.Equal(string(newFileContent), s.PrintContentOfFileOnRunner(runnerID, tests.DefaultFileName))
 		})
 	})
 
 	s.Run("File copy with invalid payload returns bad request", func() {
-		resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerId, api.UpdateFileSystemPath), "text/html", strings.NewReader(""))
+		resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath), "text/html", strings.NewReader(""))
 		s.NoError(err)
 		s.Equal(http.StatusBadRequest, resp.StatusCode)
 	})
@@ -157,8 +184,8 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 	})
 }
 
-func (s *E2ETestSuite) ContentOfFileOnRunner(runnerId string, filename string) string {
+func (s *E2ETestSuite) PrintContentOfFileOnRunner(runnerId string, filename string) string {
-	webSocketURL, _ := ProvideWebSocketURL(&s.Suite, runnerId, &dto.ExecutionRequest{Command: fmt.Sprintf("cat %s/%s", runner.FileCopyBasePath, filename)})
+	webSocketURL, _ := ProvideWebSocketURL(&s.Suite, runnerId, &dto.ExecutionRequest{Command: fmt.Sprintf("cat %s", filename)})
 	connection, _ := ConnectToWebSocket(webSocketURL)
 
 	messages, err := helpers.ReceiveAllWebSocketMessages(connection)