From 02b3f52a11b4bba2b07fd7ffecdb7726cf84e10c Mon Sep 17 00:00:00 2001 From: Jan-Eric Hellenberg Date: Mon, 31 May 2021 16:31:15 +0200 Subject: [PATCH] Add ability to copy files to and delete files from runner --- api/api.go | 10 +- api/dto/dto.go | 49 +++++- api/runners.go | 34 ++++- api/runners_test.go | 68 +++++++++ api/websocket.go | 3 +- api/websocket_test.go | 32 ++-- config/config.go | 9 ++ configuration.example.yaml | 5 + docs/swagger.yaml | 32 ++-- nomad/api_querier.go | 6 +- nomad/api_querier_mock.go | 14 +- nomad/executor_api_mock.go | 14 +- nomad/nomad.go | 5 +- runner/manager.go | 15 +- runner/runner.go | 115 +++++++++++++- runner/runner_mock.go | 48 +++--- runner/runner_test.go | 165 +++++++++++++++++++-- tests/{test_constants.go => constants.go} | 12 ++ tests/e2e/e2e_test.go | 2 +- tests/e2e/health_test.go | 4 +- tests/e2e/runners_test.go | 173 +++++++++++++++++----- tests/e2e/websocket_test.go | 139 ++++++++--------- tests/{e2e => }/helpers/test_helpers.go | 43 ++++-- 23 files changed, 757 insertions(+), 240 deletions(-) rename tests/{test_constants.go => constants.go} (50%) rename tests/{e2e => }/helpers/test_helpers.go (81%) diff --git a/api/api.go b/api/api.go index 8791ef0..7f4ad59 100644 --- a/api/api.go +++ b/api/api.go @@ -12,9 +12,9 @@ import ( var log = logging.GetLogger("api") const ( - RouteBase = "/api/v1" - RouteHealth = "/health" - RouteRunners = "/runners" + BasePath = "/api/v1" + HealthPath = "/health" + RunnersPath = "/runners" ) // NewRouter returns a *mux.Router which can be @@ -33,8 +33,8 @@ func NewRouter(runnerManager runner.Manager, environmentManager environment.Mana // configureV1Router configures a given router with the routes of version 1 of the Poseidon API. func configureV1Router(router *mux.Router, runnerManager runner.Manager, environmentManager environment.Manager) { - v1 := router.PathPrefix(RouteBase).Subrouter() - v1.HandleFunc(RouteHealth, Health).Methods(http.MethodGet) + v1 := router.PathPrefix(BasePath).Subrouter() + v1.HandleFunc(HealthPath, Health).Methods(http.MethodGet) runnerController := &RunnerController{manager: runnerManager} diff --git a/api/dto/dto.go b/api/dto/dto.go index dbd8e90..43d41ad 100644 --- a/api/dto/dto.go +++ b/api/dto/dto.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "fmt" + "path" + "strings" ) // RunnerRequest is the expected json structure of the request body for the ProvideRunner function. @@ -45,15 +47,54 @@ type RunnerResponse struct { Id string `json:"runnerId"` } -// FileCreation is the expected json structure of the request body for the copy files route. -// TODO: specify content of the struct -type FileCreation struct{} - // ExecutionResponse is the expected response when creating an execution for a runner. type ExecutionResponse struct { WebSocketUrl string `json:"websocketUrl"` } +// UpdateFileSystemRequest is the expected json structure of the request body for the update file system route. +type UpdateFileSystemRequest struct { + Delete []FilePath `json:"delete"` + Copy []File `json:"copy"` +} + +// FilePath specifies the path of a file and is part of the UpdateFileSystemRequest. +type FilePath string + +// File is a DTO for transmitting file contents. It is part of the UpdateFileSystemRequest. +type File struct { + Path FilePath `json:"path"` + 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. +func (f FilePath) ToAbsolute(basePath string) string { + filePathString := 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. +func (f File) AbsolutePath(basePath string) string { + return f.Path.ToAbsolute(basePath) +} + +// IsDirectory returns true iff the path of the File ends with a /. +func (f File) IsDirectory() bool { + return strings.HasSuffix(string(f.Path), "/") +} + +// ByteContent returns the content of the File. If the File is a directory, the content will be empty. +func (f File) ByteContent() []byte { + if f.IsDirectory() { + return []byte("") + } else { + return f.Content + } +} + // WebSocketMessageType is the type for the messages from Poseidon to the client. type WebSocketMessageType string diff --git a/api/runners.go b/api/runners.go index 68d94ac..60a4ddd 100644 --- a/api/runners.go +++ b/api/runners.go @@ -12,11 +12,12 @@ import ( ) const ( - ExecutePath = "/execute" - WebsocketPath = "/websocket" - DeleteRoute = "deleteRunner" - RunnerIdKey = "runnerId" - ExecutionIdKey = "executionId" + ExecutePath = "/execute" + WebsocketPath = "/websocket" + UpdateFileSystemPath = "/files" + DeleteRoute = "deleteRunner" + RunnerIdKey = "runnerId" + ExecutionIdKey = "executionId" ) type RunnerController struct { @@ -26,10 +27,11 @@ type RunnerController struct { // ConfigureRoutes configures a given router with the runner routes of our API. func (r *RunnerController) ConfigureRoutes(router *mux.Router) { - runnersRouter := router.PathPrefix(RouteRunners).Subrouter() + runnersRouter := router.PathPrefix(RunnersPath).Subrouter() runnersRouter.HandleFunc("", r.provide).Methods(http.MethodPost) r.runnerRouter = runnersRouter.PathPrefix(fmt.Sprintf("/{%s}", RunnerIdKey)).Subrouter() r.runnerRouter.Use(r.findRunnerMiddleware) + r.runnerRouter.HandleFunc(UpdateFileSystemPath, r.updateFileSystem).Methods(http.MethodPatch).Name(UpdateFileSystemPath) r.runnerRouter.HandleFunc(ExecutePath, r.execute).Methods(http.MethodPost).Name(ExecutePath) r.runnerRouter.HandleFunc(WebsocketPath, r.connectToRunner).Methods(http.MethodGet).Name(WebsocketPath) r.runnerRouter.HandleFunc("", r.delete).Methods(http.MethodDelete).Name(DeleteRoute) @@ -37,7 +39,7 @@ func (r *RunnerController) ConfigureRoutes(router *mux.Router) { // provide handles the provide runners API route. // It tries to respond with the id of a unused runner. -// This runner is then reserved for future use +// This runner is then reserved for future use. func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Request) { runnerRequest := new(dto.RunnerRequest) if err := parseJSONRequestBody(writer, request, runnerRequest); err != nil { @@ -59,6 +61,24 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req sendJson(writer, &dto.RunnerResponse{Id: nextRunner.Id()}, http.StatusOK) } +// updateFileSystem handles the files API route. +// It takes an dto.UpdateFileSystemRequest and sends it to the runner for processing. +func (r *RunnerController) updateFileSystem(writer http.ResponseWriter, request *http.Request) { + fileCopyRequest := new(dto.UpdateFileSystemRequest) + if err := parseJSONRequestBody(writer, request, fileCopyRequest); err != nil { + return + } + + targetRunner, _ := runner.FromContext(request.Context()) + if err := targetRunner.UpdateFileSystem(fileCopyRequest); err != nil { + log.WithError(err).Error("Could not perform the requested updateFileSystem.") + writeInternalServerError(writer, err, dto.ErrorUnknown) + return + } + + writer.WriteHeader(http.StatusNoContent) +} + // execute handles the execute API route. // It takes an ExecutionRequest and stores it for a runner. // It returns a url to connect to for a websocket connection to this execution in the corresponding runner. diff --git a/api/runners_test.go b/api/runners_test.go index 06a1000..c340829 100644 --- a/api/runners_test.go +++ b/api/runners_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/suite" "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" "net/http" "net/http/httptest" "net/url" @@ -159,6 +160,73 @@ func (suite *RunnerRouteTestSuite) TestExecuteRoute() { }) } +func TestUpdateFileSystemRouteTestSuite(t *testing.T) { + suite.Run(t, new(UpdateFileSystemRouteTestSuite)) +} + +type UpdateFileSystemRouteTestSuite struct { + RunnerRouteTestSuite + path string + recorder *httptest.ResponseRecorder + runnerMock *runner.RunnerMock +} + +func (s *UpdateFileSystemRouteTestSuite) SetupTest() { + s.RunnerRouteTestSuite.SetupTest() + routeUrl, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, tests.DefaultMockId) + if err != nil { + s.T().Fatal(err) + } + s.path = routeUrl.String() + s.runnerMock = &runner.RunnerMock{} + s.runnerManager.On("Get", tests.DefaultMockId).Return(s.runnerMock, nil) + s.recorder = httptest.NewRecorder() +} + +func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsNoContentOnValidRequest() { + s.runnerMock.On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest")).Return(nil) + + copyRequest := dto.UpdateFileSystemRequest{} + body, _ := json.Marshal(copyRequest) + request, _ := http.NewRequest(http.MethodPatch, s.path, bytes.NewReader(body)) + + s.router.ServeHTTP(s.recorder, request) + s.Equal(http.StatusNoContent, s.recorder.Code) + s.runnerMock.AssertCalled(s.T(), "UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest")) +} + +func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsBadRequestOnInvalidRequestBody() { + request, _ := http.NewRequest(http.MethodPatch, s.path, strings.NewReader("")) + + s.router.ServeHTTP(s.recorder, request) + s.Equal(http.StatusBadRequest, s.recorder.Code) +} + +func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemToNonExistingRunnerReturnsNotFound() { + invalidID := "some-invalid-runner-id" + s.runnerManager.On("Get", invalidID).Return(nil, runner.ErrRunnerNotFound) + path, _ := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, invalidID) + copyRequest := dto.UpdateFileSystemRequest{} + body, _ := json.Marshal(copyRequest) + request, _ := http.NewRequest(http.MethodPatch, path.String(), bytes.NewReader(body)) + + s.router.ServeHTTP(s.recorder, request) + s.Equal(http.StatusNotFound, s.recorder.Code) +} + +func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsInternalServerErrorWhenCopyFailed() { + s.runnerMock. + On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest")). + Return(runner.ErrorFileCopyFailed) + + copyRequest := dto.UpdateFileSystemRequest{} + body, _ := json.Marshal(copyRequest) + request, _ := http.NewRequest(http.MethodPatch, s.path, bytes.NewReader(body)) + + s.router.ServeHTTP(s.recorder, request) + s.Equal(http.StatusInternalServerError, s.recorder.Code) +} + func TestDeleteRunnerRouteTestSuite(t *testing.T) { suite.Run(t, new(DeleteRunnerRouteTestSuite)) } diff --git a/api/websocket.go b/api/websocket.go index 7aca176..615ffba 100644 --- a/api/websocket.go +++ b/api/websocket.go @@ -168,7 +168,6 @@ func newWebSocketProxy(connection webSocketConnection) (*webSocketProxy, error) // and handles WebSocket exit messages. func (wp *webSocketProxy) waitForExit(exit <-chan runner.ExitInfo, cancelExecution context.CancelFunc) { defer wp.close() - cancelInputLoop := wp.Stdin.readInputLoop() var exitInfo runner.ExitInfo select { @@ -258,7 +257,7 @@ func (r *RunnerController) connectToRunner(writer http.ResponseWriter, request * } log.WithField("runnerId", targetRunner.Id()).WithField("executionId", executionId).Info("Running execution") - exit, cancel := targetRunner.Execute(executionRequest, proxy.Stdin, proxy.Stdout, proxy.Stderr) + exit, cancel := targetRunner.ExecuteInteractively(executionRequest, proxy.Stdin, proxy.Stdout, proxy.Stderr) proxy.waitForExit(exit, cancel) } diff --git a/api/websocket_test.go b/api/websocket_test.go index 83ebe34..6502797 100644 --- a/api/websocket_test.go +++ b/api/websocket_test.go @@ -16,7 +16,7 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/nomad" "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" - "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/e2e/helpers" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/helpers" "io" "net/http" "net/http/httptest" @@ -40,7 +40,7 @@ type WebSocketTestSuite struct { func (suite *WebSocketTestSuite) SetupTest() { runnerId := "runner-id" - suite.runner, suite.apiMock = helpers.NewNomadAllocationWithMockedApiClient(runnerId) + suite.runner, suite.apiMock = newNomadAllocationWithMockedApiClient(runnerId) // default execution suite.executionId = "execution-id" @@ -97,7 +97,7 @@ func (suite *WebSocketTestSuite) TestWebsocketConnection() { suite.Run("Executes the request in the runner", func() { <-time.After(100 * time.Millisecond) suite.apiMock.AssertCalled(suite.T(), "ExecuteCommand", - mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) suite.Run("Can send input", func() { @@ -137,7 +137,7 @@ func (suite *WebSocketTestSuite) TestCancelWebSocketConnection() { select { case <-canceled: - suite.Fail("Execute canceled unexpected") + suite.Fail("ExecuteInteractively canceled unexpected") default: } @@ -147,7 +147,7 @@ func (suite *WebSocketTestSuite) TestCancelWebSocketConnection() { select { case <-canceled: case <-time.After(time.Second): - suite.Fail("Execute not canceled") + suite.Fail("ExecuteInteractively not canceled") } } @@ -169,7 +169,7 @@ func (suite *WebSocketTestSuite) TestWebSocketConnectionTimeout() { select { case <-canceled: - suite.Fail("Execute canceled unexpected") + suite.Fail("ExecuteInteractively canceled unexpected") case <-time.After(time.Duration(limitExecution.TimeLimit-1) * time.Second): <-time.After(time.Second) } @@ -177,7 +177,7 @@ func (suite *WebSocketTestSuite) TestWebSocketConnectionTimeout() { select { case <-canceled: case <-time.After(time.Second): - suite.Fail("Execute not canceled") + suite.Fail("ExecuteInteractively not canceled") } message, err = helpers.ReceiveNextWebSocketMessage(connection) @@ -245,7 +245,7 @@ func (suite *WebSocketTestSuite) TestWebsocketNonZeroExit() { func TestWebsocketTLS(t *testing.T) { runnerId := "runner-id" - r, apiMock := helpers.NewNomadAllocationWithMockedApiClient(runnerId) + r, apiMock := newNomadAllocationWithMockedApiClient(runnerId) executionId := runner.ExecutionId("execution-id") r.Add(executionId, &executionRequestLs) @@ -307,6 +307,12 @@ func TestRawToCodeOceanWriter(t *testing.T) { // --- Test suite specific test helpers --- +func newNomadAllocationWithMockedApiClient(runnerId string) (r runner.Runner, mock *nomad.ExecutorApiMock) { + mock = &nomad.ExecutorApiMock{} + r = runner.NewNomadAllocation(runnerId, mock) + return +} + func webSocketUrl(scheme string, server *httptest.Server, router *mux.Router, runnerId string, executionId runner.ExecutionId) (*url.URL, error) { websocketUrl, err := url.Parse(server.URL) if err != nil { @@ -331,7 +337,7 @@ var executionRequestLs = dto.ExecutionRequest{Command: "ls"} // mockApiExecuteLs mocks the ExecuteCommand of an ExecutorApi to act as if 'ls existing-file non-existing-file' was executed. func mockApiExecuteLs(api *nomad.ExecutorApiMock) { helpers.MockApiExecute(api, &executionRequestLs, - func(_ string, _ context.Context, _ []string, _ io.Reader, stdout, stderr io.Writer) (int, error) { + func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, stdout, stderr io.Writer) (int, error) { _, _ = stdout.Write([]byte("existing-file\n")) _, _ = stderr.Write([]byte("ls: cannot access 'non-existing-file': No such file or directory\n")) return 0, nil @@ -343,7 +349,7 @@ var executionRequestHead = dto.ExecutionRequest{Command: "head -n 1"} // mockApiExecuteHead mocks the ExecuteCommand of an ExecutorApi to act as if 'head -n 1' was executed. func mockApiExecuteHead(api *nomad.ExecutorApiMock) { helpers.MockApiExecute(api, &executionRequestHead, - func(_ string, _ context.Context, _ []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { + func(_ string, _ context.Context, _ []string, _ bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { scanner := bufio.NewScanner(stdin) for !scanner.Scan() { scanner = bufio.NewScanner(stdin) @@ -359,7 +365,7 @@ var executionRequestSleep = dto.ExecutionRequest{Command: "sleep infinity"} func mockApiExecuteSleep(api *nomad.ExecutorApiMock) <-chan bool { canceled := make(chan bool, 1) helpers.MockApiExecute(api, &executionRequestSleep, - func(_ string, ctx context.Context, _ []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { + func(_ string, ctx context.Context, _ []string, _ bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { <-ctx.Done() close(canceled) return 0, ctx.Err() @@ -372,7 +378,7 @@ var executionRequestError = dto.ExecutionRequest{Command: "error"} // mockApiExecuteError mocks the ExecuteCommand method of an ExecutorApi to return an error. func mockApiExecuteError(api *nomad.ExecutorApiMock) { helpers.MockApiExecute(api, &executionRequestError, - func(_ string, _ context.Context, _ []string, _ io.Reader, _, _ io.Writer) (int, error) { + func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, _, _ io.Writer) (int, error) { return 0, errors.New("intended error") }) } @@ -382,7 +388,7 @@ var executionRequestExitNonZero = dto.ExecutionRequest{Command: "exit 42"} // mockApiExecuteExitNonZero mocks the ExecuteCommand method of an ExecutorApi to exit with exit status 42. func mockApiExecuteExitNonZero(api *nomad.ExecutorApiMock) { helpers.MockApiExecute(api, &executionRequestExitNonZero, - func(_ string, _ context.Context, _ []string, _ io.Reader, _, _ io.Writer) (int, error) { + func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, _, _ io.Writer) (int, error) { return 42, nil }) } diff --git a/config/config.go b/config/config.go index 5ffc0f8..9d877e3 100644 --- a/config/config.go +++ b/config/config.go @@ -32,6 +32,9 @@ var ( TLS: false, Namespace: "default", }, + Runner: runner{ + WorkspacePath: "/home/python", + }, Logger: logger{ Level: "INFO", }, @@ -65,6 +68,11 @@ 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 @@ -74,6 +82,7 @@ type logger struct { type configuration struct { Server server Nomad nomad + Runner runner Logger logger } diff --git a/configuration.example.yaml b/configuration.example.yaml index 538ffeb..9ce1e67 100644 --- a/configuration.example.yaml +++ b/configuration.example.yaml @@ -26,6 +26,11 @@ 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) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 90f7a1a..e0333f6 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -3,7 +3,7 @@ info: title: Poseidon API description: | This API is used by CodeOcean to run code in runners. - version: '0.2.1' + version: '0.2.2' components: schemas: @@ -180,7 +180,7 @@ paths: /runners/{runnerId}/files: patch: summary: Manipulate runner file system - description: Copy the enclosed files to the file system of the specified runner. Existing files get overwritten and results of previous file copy operations on the same runner are present when executing multiple requests. + description: Delete the files with the given paths from the file system of the specified runner. Afterwards, copy the enclosed files to the runner. Existing files get overwritten and results of previous file copy operations on the same runner are present when executing multiple requests. tags: - runner parameters: @@ -199,30 +199,30 @@ paths: schema: type: object properties: - files: - description: Array of files that should be placed in the runner. The files are processed in the order in which they are given + delete: + description: Array of filepaths that should be deleted. Each of the given files or directories should be deleted recursively. + type: array + items: + description: Location of the file or directory that should be deleted. Can be absolute (starting with /) or relative to the workspace directory. + type: string + example: /workspace + copy: + description: Array of files that should be placed in the runner. type: array items: type: object properties: - filepath: - description: Location where the file should be placed. Can be absolute (starting with /) or relative to the workspace directory. Missing parent directories get created. If this ends with a /, the path is interpreted as a directory and content is ignored + path: + description: Location where the file should be placed. Can be absolute (starting with /) or relative to the workspace directory. Missing parent directories are created. If this ends with a /, the path is interpreted as a directory and content is ignored type: string example: /etc/passwd content: - description: The content of the file. Binary data is represented as escape sequences. Any c99 s-char-sequence surrounded by quotes is valid (e.g. "\x01\x02") and converted to its byte representation. If this is not given and delete is false, the file is created with no content + description: The content of the file. MUST be base64 encoded. If this is not given, the file is created with no content. type: string - example: root:x:0:0::/root:/bin/bash - delete: - description: Specify that the path should be deleted. If this is true, content is ignored and the file or directory is deleted (recursively) instead - type: boolean - default: false - example: false + example: cm9vdDp4OjA6MDo6L3Jvb3Q6L2Jpbi9iYXNo # root:x:0:0::/root:/bin/bash required: - - filepath + - path additionalProperties: false - required: - - files additionalProperties: false responses: "204": diff --git a/nomad/api_querier.go b/nomad/api_querier.go index 4505068..079a891 100644 --- a/nomad/api_querier.go +++ b/nomad/api_querier.go @@ -25,7 +25,7 @@ type apiQuerier interface { DeleteRunner(runnerId string) (err error) // ExecuteCommand runs a command in the passed allocation. - ExecuteCommand(allocationID string, ctx context.Context, command []string, + ExecuteCommand(allocationID string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout, stderr io.Writer) (int, error) // loadRunners loads all allocations of the specified job. @@ -66,13 +66,13 @@ func (nc *nomadApiClient) DeleteRunner(runnerId string) (err error) { } func (nc *nomadApiClient) ExecuteCommand(allocationID string, - ctx context.Context, command []string, + ctx context.Context, command []string, tty bool, stdin io.Reader, stdout, stderr io.Writer) (int, error) { allocation, _, err := nc.client.Allocations().Info(allocationID, nil) if err != nil { return 1, err } - return nc.client.Allocations().Exec(ctx, allocation, TaskName, true, command, stdin, stdout, stderr, nil, nil) + return nc.client.Allocations().Exec(ctx, allocation, TaskName, tty, command, stdin, stdout, stderr, nil, nil) } func (nc *nomadApiClient) loadRunners(jobId string) (allocationListStub []*nomadApi.AllocationListStub, err error) { diff --git a/nomad/api_querier_mock.go b/nomad/api_querier_mock.go index 86257e7..ead43bb 100644 --- a/nomad/api_querier_mock.go +++ b/nomad/api_querier_mock.go @@ -56,20 +56,20 @@ func (_m *apiQuerierMock) EvaluationStream(evalID string, ctx context.Context) ( return r0, r1 } -// ExecuteCommand provides a mock function with given fields: allocationID, ctx, command, stdin, stdout, stderr -func (_m *apiQuerierMock) ExecuteCommand(allocationID string, ctx context.Context, command []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { - ret := _m.Called(allocationID, ctx, command, stdin, stdout, stderr) +// ExecuteCommand provides a mock function with given fields: allocationID, ctx, command, tty, stdin, stdout, stderr +func (_m *apiQuerierMock) ExecuteCommand(allocationID string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { + ret := _m.Called(allocationID, ctx, command, tty, stdin, stdout, stderr) var r0 int - if rf, ok := ret.Get(0).(func(string, context.Context, []string, io.Reader, io.Writer, io.Writer) int); ok { - r0 = rf(allocationID, ctx, command, stdin, stdout, stderr) + if rf, ok := ret.Get(0).(func(string, context.Context, []string, bool, io.Reader, io.Writer, io.Writer) int); ok { + r0 = rf(allocationID, ctx, command, tty, stdin, stdout, stderr) } else { r0 = ret.Get(0).(int) } var r1 error - if rf, ok := ret.Get(1).(func(string, context.Context, []string, io.Reader, io.Writer, io.Writer) error); ok { - r1 = rf(allocationID, ctx, command, stdin, stdout, stderr) + if rf, ok := ret.Get(1).(func(string, context.Context, []string, bool, io.Reader, io.Writer, io.Writer) error); ok { + r1 = rf(allocationID, ctx, command, tty, stdin, stdout, stderr) } else { r1 = ret.Error(1) } diff --git a/nomad/executor_api_mock.go b/nomad/executor_api_mock.go index 6d6636b..60800aa 100644 --- a/nomad/executor_api_mock.go +++ b/nomad/executor_api_mock.go @@ -56,20 +56,20 @@ func (_m *ExecutorApiMock) EvaluationStream(evalID string, ctx context.Context) return r0, r1 } -// ExecuteCommand provides a mock function with given fields: allocationID, ctx, command, stdin, stdout, stderr -func (_m *ExecutorApiMock) ExecuteCommand(allocationID string, ctx context.Context, command []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { - ret := _m.Called(allocationID, ctx, command, stdin, stdout, stderr) +// ExecuteCommand provides a mock function with given fields: allocationID, ctx, command, tty, stdin, stdout, stderr +func (_m *ExecutorApiMock) ExecuteCommand(allocationID string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { + ret := _m.Called(allocationID, ctx, command, tty, stdin, stdout, stderr) var r0 int - if rf, ok := ret.Get(0).(func(string, context.Context, []string, io.Reader, io.Writer, io.Writer) int); ok { - r0 = rf(allocationID, ctx, command, stdin, stdout, stderr) + if rf, ok := ret.Get(0).(func(string, context.Context, []string, bool, io.Reader, io.Writer, io.Writer) int); ok { + r0 = rf(allocationID, ctx, command, tty, stdin, stdout, stderr) } else { r0 = ret.Get(0).(int) } var r1 error - if rf, ok := ret.Get(1).(func(string, context.Context, []string, io.Reader, io.Writer, io.Writer) error); ok { - r1 = rf(allocationID, ctx, command, stdin, stdout, stderr) + if rf, ok := ret.Get(1).(func(string, context.Context, []string, bool, io.Reader, io.Writer, io.Writer) error); ok { + r1 = rf(allocationID, ctx, command, tty, stdin, stdout, stderr) } else { r1 = ret.Error(1) } diff --git a/nomad/nomad.go b/nomad/nomad.go index a5c88cd..695f15b 100644 --- a/nomad/nomad.go +++ b/nomad/nomad.go @@ -11,7 +11,10 @@ import ( "strings" ) -var log = logging.GetLogger("nomad") +var ( + log = logging.GetLogger("nomad") + ErrorExecutorCommunicationFailed = errors.New("communication with executor failed") +) // ExecutorApi provides access to an container orchestration solution type ExecutorApi interface { diff --git a/runner/manager.go b/runner/manager.go index c946173..3cda11f 100644 --- a/runner/manager.go +++ b/runner/manager.go @@ -130,17 +130,20 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { log.WithError(err).Printf("Failed get allocation count") break } - neededRunners := job.desiredIdleRunnersCount - job.idleRunners.Length() + 1 - runnerCount := jobScale + neededRunners + additionallyNeededRunners := job.desiredIdleRunnersCount - job.idleRunners.Length() + 1 + requiredRunnerCount := jobScale + if additionallyNeededRunners > 0 { + requiredRunnerCount += additionallyNeededRunners + } time.Sleep(50 * time.Millisecond) - if runnerCount != lastJobScaling { - log.Printf("Set job scaling %d", runnerCount) - err = m.apiClient.SetJobScale(string(job.jobId), runnerCount, "Runner Requested") + if requiredRunnerCount != lastJobScaling { + log.Printf("Set job scaling %d", requiredRunnerCount) + err = m.apiClient.SetJobScale(string(job.jobId), requiredRunnerCount, "Runner Requested") if err != nil { log.WithError(err).Printf("Failed set allocation scaling") continue } - lastJobScaling = runnerCount + lastJobScaling = requiredRunnerCount } } } diff --git a/runner/runner.go b/runner/runner.go index c95e5dc..e44b3f6 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -1,11 +1,17 @@ package runner import ( + "archive/tar" + "bytes" "context" "encoding/json" + "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" "time" ) @@ -20,18 +26,30 @@ const ( runnerContextKey ContextKey = "runner" ) +var ( + ErrorFileCopyFailed = errors.New("file copy failed") + FileCopyBasePath = config.Config.Runner.WorkspacePath +) + type Runner interface { // Id returns the id of the runner. Id() string ExecutionStorage - // Execute runs the given execution request and forwards from and to the given reader and writers. + // ExecuteInteractively runs the given execution request and forwards from and to the given reader and writers. // An ExitInfo is sent to the exit channel on command completion. - Execute(request *dto.ExecutionRequest, stdin io.Reader, stdout, stderr io.Writer) (exit <-chan ExitInfo, cancel context.CancelFunc) + // Output from the runner is forwarded immediately. + ExecuteInteractively( + request *dto.ExecutionRequest, + stdin io.Reader, + stdout, + stderr io.Writer, + ) (exit <-chan ExitInfo, cancel context.CancelFunc) - // Copy copies the specified files into the runner. - Copy(dto.FileCreation) + // UpdateFileSystem processes a dto.UpdateFileSystemRequest by first deleting each given dto.FilePath recursively + // and then copying each given dto.File to the runner. + UpdateFileSystem(request *dto.UpdateFileSystemRequest) error } // NomadAllocation is an abstraction to communicate with Nomad allocations. @@ -64,7 +82,11 @@ type ExitInfo struct { Err error } -func (r *NomadAllocation) Execute(request *dto.ExecutionRequest, stdin io.Reader, stdout, stderr io.Writer) (<-chan ExitInfo, context.CancelFunc) { +func (r *NomadAllocation) ExecuteInteractively( + request *dto.ExecutionRequest, + stdin io.Reader, + stdout, stderr io.Writer, +) (<-chan ExitInfo, context.CancelFunc) { command := request.FullCommand() var ctx context.Context var cancel context.CancelFunc @@ -75,15 +97,94 @@ func (r *NomadAllocation) Execute(request *dto.ExecutionRequest, stdin io.Reader } exit := make(chan ExitInfo) go func() { - exitCode, err := r.api.ExecuteCommand(r.Id(), ctx, command, stdin, stdout, stderr) + exitCode, err := r.api.ExecuteCommand(r.Id(), ctx, command, true, stdin, stdout, stderr) exit <- ExitInfo{uint8(exitCode), err} close(exit) }() return exit, cancel } -func (r *NomadAllocation) Copy(files dto.FileCreation) { +func (r *NomadAllocation) UpdateFileSystem(copyRequest *dto.UpdateFileSystemRequest) error { + var tarBuffer bytes.Buffer + if err := createTarArchiveForFiles(copyRequest.Copy, &tarBuffer); err != nil { + return err + } + fileDeletionCommand := fileDeletionCommand(copyRequest.Delete) + copyCommand := "tar --extract --absolute-names --verbose --directory=/ --file=/dev/stdin;" + updateFileCommand := (&dto.ExecutionRequest{Command: fileDeletionCommand + copyCommand}).FullCommand() + stdOut := bytes.Buffer{} + stdErr := bytes.Buffer{} + exitCode, err := r.api.ExecuteCommand(r.Id(), context.Background(), updateFileCommand, false, + &tarBuffer, &stdOut, &stdErr) + + if err != nil { + return fmt.Errorf( + "%w: nomad error during file copy: %v", + nomad.ErrorExecutorCommunicationFailed, + err) + } + if exitCode != 0 { + return fmt.Errorf( + "%w: stderr output '%s' and stdout output '%s'", + ErrorFileCopyFailed, + stdErr.String(), + stdOut.String()) + } + return nil +} + +func createTarArchiveForFiles(filesToCopy []dto.File, w io.Writer) error { + tarWriter := tar.NewWriter(w) + for _, file := range filesToCopy { + if err := tarWriter.WriteHeader(tarHeader(file)); err != nil { + log. + WithError(err). + WithField("file", file). + Error("Error writing tar file header") + return err + } + if _, err := tarWriter.Write(file.ByteContent()); err != nil { + log. + WithError(err). + WithField("file", file). + Error("Error writing tar file content") + return err + } + } + return tarWriter.Close() +} + +func fileDeletionCommand(filesToDelete []dto.FilePath) string { + if len(filesToDelete) == 0 { + return "" + } + command := "rm --recursive --force " + for _, filePath := range filesToDelete { + // 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), "'", "'\\''") + command += fmt.Sprintf("'%s' ", singleQuoteEscapedFileName) + } + command += ";" + return command +} + +func tarHeader(file dto.File) *tar.Header { + if file.IsDirectory() { + return &tar.Header{ + Typeflag: tar.TypeDir, + Name: file.AbsolutePath(FileCopyBasePath), + Mode: 0755, + } + } else { + return &tar.Header{ + Typeflag: tar.TypeReg, + Name: file.AbsolutePath(FileCopyBasePath), + Mode: 0744, + Size: int64(len(file.Content)), + } + } } // MarshalJSON implements json.Marshaler interface. diff --git a/runner/runner_mock.go b/runner/runner_mock.go index 4b3b04b..709fe0f 100644 --- a/runner/runner_mock.go +++ b/runner/runner_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.8.0. DO NOT EDIT. package runner @@ -21,18 +21,8 @@ func (_m *RunnerMock) Add(id ExecutionId, executionRequest *dto.ExecutionRequest _m.Called(id, executionRequest) } -// Copy provides a mock function with given fields: _a0 -func (_m *RunnerMock) Copy(_a0 dto.FileCreation) { - _m.Called(_a0) -} - -// Delete provides a mock function with given fields: id -func (_m *RunnerMock) Delete(id ExecutionId) { - _m.Called(id) -} - -// Execute provides a mock function with given fields: request, stdin, stdout, stderr -func (_m *RunnerMock) Execute(request *dto.ExecutionRequest, stdin io.Reader, stdout io.Writer, stderr io.Writer) (<-chan ExitInfo, context.CancelFunc) { +// ExecuteInteractively provides a mock function with given fields: request, stdin, stdout, stderr +func (_m *RunnerMock) ExecuteInteractively(request *dto.ExecutionRequest, stdin io.Reader, stdout io.Writer, stderr io.Writer) (<-chan ExitInfo, context.CancelFunc) { ret := _m.Called(request, stdin, stdout, stderr) var r0 <-chan ExitInfo @@ -56,8 +46,22 @@ func (_m *RunnerMock) Execute(request *dto.ExecutionRequest, stdin io.Reader, st return r0, r1 } -// Get provides a mock function with given fields: id -func (_m *RunnerMock) Get(id ExecutionId) (*dto.ExecutionRequest, bool) { +// Id provides a mock function with given fields: +func (_m *RunnerMock) Id() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// Pop provides a mock function with given fields: id +func (_m *RunnerMock) Pop(id ExecutionId) (*dto.ExecutionRequest, bool) { ret := _m.Called(id) var r0 *dto.ExecutionRequest @@ -79,15 +83,15 @@ func (_m *RunnerMock) Get(id ExecutionId) (*dto.ExecutionRequest, bool) { return r0, r1 } -// Id provides a mock function with given fields: -func (_m *RunnerMock) Id() string { - ret := _m.Called() +// UpdateFileSystem provides a mock function with given fields: request +func (_m *RunnerMock) UpdateFileSystem(request *dto.UpdateFileSystemRequest) error { + ret := _m.Called(request) - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() + var r0 error + if rf, ok := ret.Get(0).(func(*dto.UpdateFileSystemRequest) error); ok { + r0 = rf(request) } else { - r0 = ret.Get(0).(string) + r0 = ret.Error(0) } return r0 diff --git a/runner/runner_test.go b/runner/runner_test.go index 1c94ab2..f629f22 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -1,12 +1,20 @@ package runner import ( + "archive/tar" + "bytes" "context" "encoding/json" + "fmt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/nomad" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" + "io" + "regexp" + "strings" "testing" "time" ) @@ -66,42 +74,42 @@ func TestFromContextReturnsIsNotOkWhenContextHasNoRunner(t *testing.T) { func TestExecuteCallsAPI(t *testing.T) { apiMock := &nomad.ExecutorApiMock{} - apiMock.On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(0, nil) - runner := NewNomadAllocation("testRunner", apiMock) + apiMock.On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, true, mock.Anything, mock.Anything, mock.Anything).Return(0, nil) + runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock) request := &dto.ExecutionRequest{Command: "echo 'Hello World!'"} - runner.Execute(request, nil, nil, nil) + runner.ExecuteInteractively(request, nil, nil, nil) <-time.After(50 * time.Millisecond) - apiMock.AssertCalled(t, "ExecuteCommand", "testRunner", mock.Anything, request.FullCommand(), mock.Anything, mock.Anything, mock.Anything) + apiMock.AssertCalled(t, "ExecuteCommand", tests.DefaultRunnerId, mock.Anything, request.FullCommand(), true, mock.Anything, mock.Anything, mock.Anything) } func TestExecuteReturnsAfterTimeout(t *testing.T) { apiMock := newApiMockWithTimeLimitHandling() - runner := NewNomadAllocation("testRunner", apiMock) + runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock) timeLimit := 1 execution := &dto.ExecutionRequest{TimeLimit: timeLimit} - exit, _ := runner.Execute(execution, nil, nil, nil) + exit, _ := runner.ExecuteInteractively(execution, nil, nil, nil) select { case <-exit: - assert.FailNow(t, "Execute should not terminate instantly") + assert.FailNow(t, "ExecuteInteractively should not terminate instantly") case <-time.After(50 * time.Millisecond): } select { case <-time.After(time.Duration(timeLimit) * time.Second): - assert.FailNow(t, "Execute should return after the time limit") - case exitCode := <-exit: - assert.Equal(t, uint8(0), exitCode.Code) + assert.FailNow(t, "ExecuteInteractively should return after the time limit") + case exitInfo := <-exit: + assert.Equal(t, uint8(0), exitInfo.Code) } } func newApiMockWithTimeLimitHandling() (apiMock *nomad.ExecutorApiMock) { apiMock = &nomad.ExecutorApiMock{} apiMock. - On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, true, mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { ctx := args.Get(1).(context.Context) <-ctx.Done() @@ -109,3 +117,138 @@ func newApiMockWithTimeLimitHandling() (apiMock *nomad.ExecutorApiMock) { Return(0, nil) return } + +func TestUpdateFileSystemTestSuite(t *testing.T) { + suite.Run(t, new(UpdateFileSystemTestSuite)) +} + +type UpdateFileSystemTestSuite struct { + suite.Suite + runner *NomadAllocation + apiMock *nomad.ExecutorApiMock + mockedExecuteCommandCall *mock.Call + command []string + stdin *bytes.Buffer +} + +func (s *UpdateFileSystemTestSuite) SetupTest() { + s.apiMock = &nomad.ExecutorApiMock{} + s.runner = NewNomadAllocation(tests.DefaultRunnerId, s.apiMock) + s.mockedExecuteCommandCall = s.apiMock.On("ExecuteCommand", tests.DefaultRunnerId, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + s.command = args.Get(2).([]string) + s.stdin = args.Get(4).(*bytes.Buffer) + }) +} + +func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerPerformsTarExtractionWithAbsoluteNamesOnRunner() { + // note: this method tests an implementation detail of the method UpdateFileSystemOfRunner method + // if the implementation changes, delete this test and write a new one + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{} + err := s.runner.UpdateFileSystem(copyRequest) + s.NoError(err) + s.apiMock.AssertCalled(s.T(), "ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything) + s.Regexp("tar --extract --absolute-names", s.command) +} + +func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfExitCodeIsNotZero() { + s.mockedExecuteCommandCall.Return(1, nil) + copyRequest := &dto.UpdateFileSystemRequest{} + err := s.runner.UpdateFileSystem(copyRequest) + s.ErrorIs(err, ErrorFileCopyFailed) +} + +func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfApiCallDid() { + s.mockedExecuteCommandCall.Return(0, tests.DefaultError) + copyRequest := &dto.UpdateFileSystemRequest{} + err := s.runner.UpdateFileSystem(copyRequest) + s.ErrorIs(err, nomad.ErrorExecutorCommunicationFailed) +} + +func (s *UpdateFileSystemTestSuite) TestFilesToCopyAreIncludedInTarArchive() { + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{Copy: []dto.File{{Path: tests.DefaultFileName, Content: []byte(tests.DefaultFileContent)}}} + err := s.runner.UpdateFileSystem(copyRequest) + s.NoError(err) + s.apiMock.AssertCalled(s.T(), "ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything) + + tarFiles := s.readFilesFromTarArchive(s.stdin) + s.Len(tarFiles, 1) + tarFile := tarFiles[0] + s.True(strings.HasSuffix(tarFile.Name, tests.DefaultFileName)) + s.Equal(byte(tar.TypeReg), tarFile.TypeFlag) + s.Equal(tests.DefaultFileContent, tarFile.Content) +} + +func (s *UpdateFileSystemTestSuite) TestFilesWithRelativePathArePutInDefaultLocation() { + 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] + s.True(strings.HasSuffix(tarFile.Name, tests.DefaultFileName)) + s.True(strings.HasPrefix(tarFile.Name, FileCopyBasePath)) +} + +func (s *UpdateFileSystemTestSuite) TestFilesWithAbsolutePathArePutInAbsoluteLocation() { + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{Copy: []dto.File{{Path: tests.FileNameWithAbsolutePath, Content: []byte(tests.DefaultFileContent)}}} + _ = s.runner.UpdateFileSystem(copyRequest) + + tarFiles := s.readFilesFromTarArchive(s.stdin) + s.Len(tarFiles, 1) + s.Equal(tarFiles[0].Name, tests.FileNameWithAbsolutePath) +} + +func (s *UpdateFileSystemTestSuite) TestDirectoriesAreMarkedAsDirectoryInTar() { + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{Copy: []dto.File{{Path: tests.DefaultDirectoryName, Content: []byte{}}}} + _ = s.runner.UpdateFileSystem(copyRequest) + + tarFiles := s.readFilesFromTarArchive(s.stdin) + s.Len(tarFiles, 1) + tarFile := tarFiles[0] + s.True(strings.HasSuffix(tarFile.Name+"/", tests.DefaultDirectoryName)) + s.Equal(byte(tar.TypeDir), tarFile.TypeFlag) + s.Equal("", tarFile.Content) +} + +func (s *UpdateFileSystemTestSuite) TestFilesToRemoveGetRemoved() { + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{Delete: []dto.FilePath{tests.DefaultFileName}} + err := s.runner.UpdateFileSystem(copyRequest) + s.NoError(err) + s.apiMock.AssertCalled(s.T(), "ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything) + s.Regexp(fmt.Sprintf("rm[^;]+%s' *;", regexp.QuoteMeta(tests.DefaultFileName)), s.command) +} + +func (s *UpdateFileSystemTestSuite) TestFilesToRemoveGetEscaped() { + s.mockedExecuteCommandCall.Return(0, nil) + copyRequest := &dto.UpdateFileSystemRequest{Delete: []dto.FilePath{"/some/potentially/harmful'filename"}} + err := s.runner.UpdateFileSystem(copyRequest) + s.NoError(err) + s.apiMock.AssertCalled(s.T(), "ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything) + s.Contains(strings.Join(s.command, " "), "'/some/potentially/harmful'\\''filename'") +} + +type TarFile struct { + Name string + Content string + TypeFlag byte +} + +func (s *UpdateFileSystemTestSuite) readFilesFromTarArchive(tarArchive io.Reader) (files []TarFile) { + reader := tar.NewReader(tarArchive) + for { + hdr, err := reader.Next() + if err != nil { + break + } + bf, _ := io.ReadAll(reader) + files = append(files, TarFile{Name: hdr.Name, Content: string(bf), TypeFlag: hdr.Typeflag}) + } + return files +} diff --git a/tests/test_constants.go b/tests/constants.go similarity index 50% rename from tests/test_constants.go rename to tests/constants.go index 25f4781..f1d5e7d 100644 --- a/tests/test_constants.go +++ b/tests/constants.go @@ -1,6 +1,13 @@ package tests +import "errors" + const ( + NonExistingId = "n0n-3x1st1ng-1d" + DefaultFileName = "test.txt" + DefaultFileContent = "Hello, Codemoon!" + DefaultDirectoryName = "test/" + FileNameWithAbsolutePath = "/test.txt" DefaultEnvironmentIdAsInteger = 0 AnotherEnvironmentIdAsInteger = 42 DefaultJobId = "s0m3-j0b-1d" @@ -8,4 +15,9 @@ const ( DefaultRunnerId = "s0m3-r4nd0m-1d" AnotherRunnerId = "4n0th3r-runn3r-1d" DefaultExecutionId = "s0m3-3x3cu710n-1d" + DefaultMockId = "m0ck-1d" +) + +var ( + DefaultError = errors.New("an error occured") ) diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 24114a4..fe5dbb0 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -21,7 +21,7 @@ type E2ETestSuite struct { suite.Suite } -func (suite *E2ETestSuite) SetupTest() { +func (s *E2ETestSuite) SetupTest() { // Waiting one second before each test allows Nomad to rescale after tests requested runners. <-time.After(time.Second) } diff --git a/tests/e2e/health_test.go b/tests/e2e/health_test.go index 9376ad6..15603f6 100644 --- a/tests/e2e/health_test.go +++ b/tests/e2e/health_test.go @@ -3,13 +3,13 @@ package e2e import ( "github.com/stretchr/testify/assert" "gitlab.hpi.de/codeocean/codemoon/poseidon/api" - "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/e2e/helpers" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/helpers" "net/http" "testing" ) func TestHealthRoute(t *testing.T) { - resp, err := http.Get(helpers.BuildURL(api.RouteBase, api.RouteHealth)) + resp, err := http.Get(helpers.BuildURL(api.BasePath, api.HealthPath)) if assert.NoError(t, err) { assert.Equal(t, http.StatusNoContent, resp.StatusCode, "The response code should be NoContent") } diff --git a/tests/e2e/runners_test.go b/tests/e2e/runners_test.go index cdc01e7..0a027fe 100644 --- a/tests/e2e/runners_test.go +++ b/tests/e2e/runners_test.go @@ -1,67 +1,170 @@ package e2e import ( + "bytes" "encoding/json" - "github.com/stretchr/testify/assert" + "fmt" + "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/tests/e2e/helpers" + "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/helpers" "io" "net/http" "strings" - "testing" ) -func (suite *E2ETestSuite) TestProvideRunnerRoute() { +func (s *E2ETestSuite) TestProvideRunnerRoute() { runnerRequestString, _ := json.Marshal(dto.RunnerRequest{}) reader := strings.NewReader(string(runnerRequestString)) - resp, err := http.Post(helpers.BuildURL(api.RouteBase, api.RouteRunners), "application/json", reader) - suite.NoError(err) - suite.Equal(http.StatusOK, resp.StatusCode, "The response code should be ok") + resp, err := http.Post(helpers.BuildURL(api.BasePath, api.RunnersPath), "application/json", reader) + s.NoError(err) + s.Equal(http.StatusOK, resp.StatusCode, "The response code should be ok") runnerResponse := new(dto.RunnerResponse) err = json.NewDecoder(resp.Body).Decode(runnerResponse) - suite.NoError(err) + s.NoError(err) - suite.True(runnerResponse.Id != "", "The response contains a runner id") + s.True(runnerResponse.Id != "", "The response contains a runner id") } -func newRunnerId(t *testing.T) string { - runnerRequestString, _ := json.Marshal(dto.RunnerRequest{}) +// ProvideRunner creates a runner with the given RunnerRequest via an external request. +// It needs a running Poseidon instance to work. +func ProvideRunner(request *dto.RunnerRequest) (string, error) { + url := helpers.BuildURL(api.BasePath, api.RunnersPath) + runnerRequestString, _ := json.Marshal(request) reader := strings.NewReader(string(runnerRequestString)) - resp, err := http.Post(helpers.BuildURL(api.RouteBase, api.RouteRunners), "application/json", reader) - assert.NoError(t, err) + resp, err := http.Post(url, "application/json", reader) + if err != nil { + return "", err + } + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("expected response code 200 when getting runner, got %v", resp.StatusCode) + } runnerResponse := new(dto.RunnerResponse) - _ = json.NewDecoder(resp.Body).Decode(runnerResponse) - return runnerResponse.Id + err = json.NewDecoder(resp.Body).Decode(runnerResponse) + if err != nil { + return "", err + } + return runnerResponse.Id, nil } -func (suite *E2ETestSuite) TestDeleteRunnerRoute() { - runnerId := newRunnerId(suite.T()) - suite.NotEqual("", runnerId) +func (s *E2ETestSuite) TestDeleteRunnerRoute() { + runnerId, err := ProvideRunner(&dto.RunnerRequest{}) + s.NoError(err) - suite.Run("Deleting the runner returns NoContent", func() { - resp, err := httpDelete(helpers.BuildURL(api.RouteBase, api.RouteRunners, "/", runnerId), nil) - suite.NoError(err) - suite.Equal(http.StatusNoContent, resp.StatusCode) + s.Run("Deleting the runner returns NoContent", func() { + resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerId), nil) + s.NoError(err) + s.Equal(http.StatusNoContent, resp.StatusCode) }) - suite.Run("Deleting it again returns NotFound", func() { - resp, err := httpDelete(helpers.BuildURL(api.RouteBase, api.RouteRunners, "/", runnerId), nil) - suite.NoError(err) - suite.Equal(http.StatusNotFound, resp.StatusCode) + s.Run("Deleting it again returns NotFound", func() { + resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerId), nil) + s.NoError(err) + s.Equal(http.StatusNotFound, resp.StatusCode) }) - suite.Run("Deleting non-existing runner returns NotFound", func() { - resp, err := httpDelete(helpers.BuildURL(api.RouteBase, api.RouteRunners, "/", "n0n-3x1st1ng-1d"), nil) - suite.NoError(err) - suite.Equal(http.StatusNotFound, resp.StatusCode) + s.Run("Deleting non-existing runner returns NotFound", func() { + resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId), nil) + s.NoError(err) + s.Equal(http.StatusNotFound, resp.StatusCode) }) } -// HttpDelete sends a Delete Http Request with body to the passed url. -func httpDelete(url string, body io.Reader) (response *http.Response, err error) { - req, _ := http.NewRequest(http.MethodDelete, url, body) - client := &http.Client{} - return client.Do(req) +func (s *E2ETestSuite) TestCopyFilesRoute() { + 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) + } + + s.Run("File copy with valid payload succeeds", func() { + resp, err := sendCopyRequest(bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + 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.Run("File deletion request deletes file on runner", func() { + copyFilesRequestByteString, _ := json.Marshal(&dto.UpdateFileSystemRequest{ + Delete: []dto.FilePath{tests.DefaultFileName}, + }) + + resp, err := sendCopyRequest(bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + 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.Run("File copy happens after file deletion", func() { + copyFilesRequestByteString, _ := json.Marshal(&dto.UpdateFileSystemRequest{ + Delete: []dto.FilePath{tests.DefaultFileName}, + Copy: []dto.File{{Path: tests.DefaultFileName, Content: []byte(tests.DefaultFileContent)}}, + }) + + resp, err := sendCopyRequest(bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + 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.Run("If one file produces permission denied error, others are still copied", func() { + newFileContent := []byte("New content") + copyFilesRequestByteString, _ := json.Marshal(&dto.UpdateFileSystemRequest{ + Copy: []dto.File{ + {Path: "/dev/sda", Content: []byte(tests.DefaultFileContent)}, + {Path: tests.DefaultFileName, Content: newFileContent}, + }, + }) + + resp, err := sendCopyRequest(bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + s.Equal(http.StatusInternalServerError, resp.StatusCode) + internalServerError := new(dto.InternalServerError) + err = json.NewDecoder(resp.Body).Decode(internalServerError) + s.NoError(err) + s.Contains(internalServerError.Message, "Cannot open: Permission denied") + + s.Run("File content can be printed on runner", func() { + s.Equal(string(newFileContent), s.ContentOfFileOnRunner(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("")) + s.NoError(err) + s.Equal(http.StatusBadRequest, resp.StatusCode) + }) + + s.Run("Copying to non-existing runner returns NotFound", func() { + resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId, api.UpdateFileSystemPath), "application/json", bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + s.Equal(http.StatusNotFound, resp.StatusCode) + }) +} + +func (s *E2ETestSuite) ContentOfFileOnRunner(runnerId string, filename string) string { + webSocketURL, _ := ProvideWebSocketURL(&s.Suite, runnerId, &dto.ExecutionRequest{Command: fmt.Sprintf("cat %s/%s", runner.FileCopyBasePath, filename)}) + connection, _ := ConnectToWebSocket(webSocketURL) + + messages, err := helpers.ReceiveAllWebSocketMessages(connection) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + + stdout, _, _ := helpers.WebSocketOutputMessages(messages) + return stdout } diff --git a/tests/e2e/websocket_test.go b/tests/e2e/websocket_test.go index f90b9d2..3bc529c 100644 --- a/tests/e2e/websocket_test.go +++ b/tests/e2e/websocket_test.go @@ -7,25 +7,25 @@ import ( "github.com/stretchr/testify/suite" "gitlab.hpi.de/codeocean/codemoon/poseidon/api" "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" - "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/e2e/helpers" + "gitlab.hpi.de/codeocean/codemoon/poseidon/tests/helpers" "net/http" "strings" "time" ) -func (suite *E2ETestSuite) TestExecuteCommandRoute() { - runnerId, err := ProvideRunner(&suite.Suite, &dto.RunnerRequest{}) - suite.Require().NoError(err) +func (s *E2ETestSuite) TestExecuteCommandRoute() { + runnerId, err := ProvideRunner(&dto.RunnerRequest{}) + s.Require().NoError(err) - webSocketURL, err := ProvideWebSocketURL(&suite.Suite, runnerId, &dto.ExecutionRequest{Command: "true"}) - suite.Require().NoError(err) - suite.NotEqual("", webSocketURL) + webSocketURL, err := ProvideWebSocketURL(&s.Suite, runnerId, &dto.ExecutionRequest{Command: "true"}) + s.Require().NoError(err) + s.NotEqual("", webSocketURL) var connection *websocket.Conn var connectionClosed bool connection, err = ConnectToWebSocket(webSocketURL) - suite.Require().NoError(err, "websocket connects") + s.Require().NoError(err, "websocket connects") closeHandler := connection.CloseHandler() connection.SetCloseHandler(func(code int, text string) error { connectionClosed = true @@ -33,119 +33,119 @@ func (suite *E2ETestSuite) TestExecuteCommandRoute() { }) startMessage, err := helpers.ReceiveNextWebSocketMessage(connection) - suite.Require().NoError(err) - suite.Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, startMessage) + s.Require().NoError(err) + s.Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, startMessage) exitMessage, err := helpers.ReceiveNextWebSocketMessage(connection) - suite.Require().NoError(err) - suite.Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, exitMessage) + s.Require().NoError(err) + s.Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, exitMessage) _, err = helpers.ReceiveAllWebSocketMessages(connection) - suite.Require().Error(err) - suite.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) _, _, _ = connection.ReadMessage() - suite.True(connectionClosed, "connection should be closed") + s.True(connectionClosed, "connection should be closed") } -func (suite *E2ETestSuite) TestOutputToStdout() { - connection, err := ProvideWebSocketConnection(&suite.Suite, &dto.ExecutionRequest{Command: "echo Hello World"}) - suite.Require().NoError(err) +func (s *E2ETestSuite) TestOutputToStdout() { + connection, err := ProvideWebSocketConnection(&s.Suite, &dto.ExecutionRequest{Command: "echo Hello World"}) + s.Require().NoError(err) messages, err := helpers.ReceiveAllWebSocketMessages(connection) - suite.Require().Error(err) - suite.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) - suite.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, messages[0]) - suite.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketOutputStdout, Data: "Hello World\r\n"}, messages[1]) - suite.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, messages[2]) + s.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, messages[0]) + s.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketOutputStdout, Data: "Hello World\r\n"}, messages[1]) + s.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, messages[2]) } -func (suite *E2ETestSuite) TestOutputToStderr() { - suite.T().Skip("known bug causing all output to be written to stdout (even if it should be written to stderr)") - connection, err := ProvideWebSocketConnection(&suite.Suite, &dto.ExecutionRequest{Command: "cat -invalid"}) - suite.Require().NoError(err) +func (s *E2ETestSuite) TestOutputToStderr() { + s.T().Skip("known bug causing all output to be written to stdout (even if it should be written to stderr)") + connection, err := ProvideWebSocketConnection(&s.Suite, &dto.ExecutionRequest{Command: "cat -invalid"}) + s.Require().NoError(err) messages, err := helpers.ReceiveAllWebSocketMessages(connection) - suite.Require().Error(err) - suite.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) controlMessages := helpers.WebSocketControlMessages(messages) - suite.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, controlMessages[0]) - suite.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, controlMessages[1]) + s.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaStart}, controlMessages[0]) + s.Require().Equal(&dto.WebSocketMessage{Type: dto.WebSocketExit}, controlMessages[1]) stdout, stderr, errors := helpers.WebSocketOutputMessages(messages) - suite.NotContains(stdout, "cat: invalid option", "Stdout should not contain the error") - suite.Contains(stderr, "cat: invalid option", "Stderr should contain the error") - suite.Empty(errors) + s.NotContains(stdout, "cat: invalid option", "Stdout should not contain the error") + s.Contains(stderr, "cat: invalid option", "Stderr should contain the error") + s.Empty(errors) } -func (suite *E2ETestSuite) TestCommandHead() { +func (s *E2ETestSuite) TestCommandHead() { hello := "Hello World!" - connection, err := ProvideWebSocketConnection(&suite.Suite, &dto.ExecutionRequest{Command: "head -n 1"}) - suite.Require().NoError(err) + connection, err := ProvideWebSocketConnection(&s.Suite, &dto.ExecutionRequest{Command: "head -n 1"}) + s.Require().NoError(err) startMessage, err := helpers.ReceiveNextWebSocketMessage(connection) - suite.Require().NoError(err) - suite.Equal(dto.WebSocketMetaStart, startMessage.Type) + s.Require().NoError(err) + s.Equal(dto.WebSocketMetaStart, startMessage.Type) err = connection.WriteMessage(websocket.TextMessage, []byte(fmt.Sprintf("%s\n", hello))) - suite.Require().NoError(err) + s.Require().NoError(err) messages, err := helpers.ReceiveAllWebSocketMessages(connection) - suite.Require().Error(err) - suite.Equal(err, &websocket.CloseError{Code: websocket.CloseNormalClosure}) + s.Require().Error(err) + s.Equal(err, &websocket.CloseError{Code: websocket.CloseNormalClosure}) stdout, _, _ := helpers.WebSocketOutputMessages(messages) - suite.Contains(stdout, fmt.Sprintf("%s\r\n%s\r\n", hello, hello)) + s.Contains(stdout, fmt.Sprintf("%s\r\n%s\r\n", hello, hello)) } -func (suite *E2ETestSuite) TestCommandReturnsAfterTimeout() { - connection, err := ProvideWebSocketConnection(&suite.Suite, &dto.ExecutionRequest{Command: "sleep 4", TimeLimit: 1}) - suite.Require().NoError(err) +func (s *E2ETestSuite) TestCommandReturnsAfterTimeout() { + connection, err := ProvideWebSocketConnection(&s.Suite, &dto.ExecutionRequest{Command: "sleep 4", TimeLimit: 1}) + s.Require().NoError(err) c := make(chan bool) var messages []*dto.WebSocketMessage go func() { messages, err = helpers.ReceiveAllWebSocketMessages(connection) - if !suite.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) { - suite.T().Fail() + if !s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) { + s.T().Fail() } close(c) }() select { case <-time.After(2 * time.Second): - suite.T().Fatal("The execution should have returned by now") + s.T().Fatal("The execution should have returned by now") case <-c: - if suite.Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaTimeout}, messages[len(messages)-1]) { + if s.Equal(&dto.WebSocketMessage{Type: dto.WebSocketMetaTimeout}, messages[len(messages)-1]) { return } } - suite.T().Fail() + s.T().Fail() } -func (suite *E2ETestSuite) TestEchoEnvironment() { - connection, err := ProvideWebSocketConnection(&suite.Suite, &dto.ExecutionRequest{ +func (s *E2ETestSuite) TestEchoEnvironment() { + connection, err := ProvideWebSocketConnection(&s.Suite, &dto.ExecutionRequest{ Command: "echo $hello", Environment: map[string]string{"hello": "world"}, }) - suite.Require().NoError(err) + s.Require().NoError(err) startMessage, err := helpers.ReceiveNextWebSocketMessage(connection) - suite.Require().NoError(err) - suite.Equal(dto.WebSocketMetaStart, startMessage.Type) + s.Require().NoError(err) + s.Equal(dto.WebSocketMetaStart, startMessage.Type) messages, err := helpers.ReceiveAllWebSocketMessages(connection) - suite.Require().Error(err) - suite.Equal(err, &websocket.CloseError{Code: websocket.CloseNormalClosure}) + s.Require().Error(err) + s.Equal(err, &websocket.CloseError{Code: websocket.CloseNormalClosure}) stdout, _, _ := helpers.WebSocketOutputMessages(messages) - suite.Equal("world\r\n", stdout) + s.Equal("world\r\n", stdout) } // ProvideWebSocketConnection establishes a client WebSocket connection to run the passed ExecutionRequest. // It requires a running Poseidon instance. func ProvideWebSocketConnection(suite *suite.Suite, request *dto.ExecutionRequest) (connection *websocket.Conn, err error) { - runnerId, err := ProvideRunner(suite, &dto.RunnerRequest{}) + runnerId, err := ProvideRunner(&dto.RunnerRequest{}) if err != nil { return } @@ -157,27 +157,10 @@ func ProvideWebSocketConnection(suite *suite.Suite, request *dto.ExecutionReques return } -// ProvideRunner creates a runner with the given RunnerRequest via an external request. -// It needs a running Poseidon instance to work. -func ProvideRunner(suite *suite.Suite, request *dto.RunnerRequest) (string, error) { - url := helpers.BuildURL(api.RouteBase, api.RouteRunners) - runnerRequestBytes, _ := json.Marshal(request) - reader := strings.NewReader(string(runnerRequestBytes)) - resp, err := http.Post(url, "application/json", reader) - suite.Require().NoError(err) - suite.Require().Equal(http.StatusOK, resp.StatusCode) - - runnerResponse := new(dto.RunnerResponse) - err = json.NewDecoder(resp.Body).Decode(runnerResponse) - suite.Require().NoError(err) - - return runnerResponse.Id, nil -} - // ProvideWebSocketURL creates a WebSocket endpoint from the ExecutionRequest via an external api request. // It requires a running Poseidon instance. func ProvideWebSocketURL(suite *suite.Suite, runnerId string, request *dto.ExecutionRequest) (string, error) { - url := helpers.BuildURL(api.RouteBase, api.RouteRunners, "/", runnerId, api.ExecutePath) + url := helpers.BuildURL(api.BasePath, api.RunnersPath, runnerId, api.ExecutePath) executionRequestBytes, _ := json.Marshal(request) reader := strings.NewReader(string(executionRequestBytes)) resp, err := http.Post(url, "application/json", reader) diff --git a/tests/e2e/helpers/test_helpers.go b/tests/helpers/test_helpers.go similarity index 81% rename from tests/e2e/helpers/test_helpers.go rename to tests/helpers/test_helpers.go index 25c76bb..1c046de 100644 --- a/tests/e2e/helpers/test_helpers.go +++ b/tests/helpers/test_helpers.go @@ -12,8 +12,8 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/config" "gitlab.hpi.de/codeocean/codemoon/poseidon/nomad" - "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" "io" + "net/http" "net/http/httptest" "os/exec" "path/filepath" @@ -23,8 +23,14 @@ import ( // BuildURL joins multiple route paths. func BuildURL(parts ...string) (url string) { - parts = append([]string{config.Config.PoseidonAPIURL().String()}, parts...) - return strings.Join(parts, "") + url = config.Config.PoseidonAPIURL().String() + for _, part := range parts { + if !strings.HasPrefix(part, "/") { + url += "/" + } + url += part + } + return } // WebSocketOutputMessages extracts all stdout, stderr and error messages from the passed messages. @@ -105,20 +111,15 @@ func StartTLSServer(t *testing.T, router *mux.Router) (server *httptest.Server, return } -func NewNomadAllocationWithMockedApiClient(runnerId string) (r runner.Runner, mock *nomad.ExecutorApiMock) { - mock = &nomad.ExecutorApiMock{} - r = runner.NewNomadAllocation(runnerId, mock) - return -} - // MockApiExecute mocks the ExecuteCommand method of an ExecutorApi to call the given method run when the command // corresponding to the given ExecutionRequest is called. func MockApiExecute(api *nomad.ExecutorApiMock, request *dto.ExecutionRequest, - run func(runnerId string, ctx context.Context, command []string, stdin io.Reader, stdout, stderr io.Writer) (int, error)) { + run func(runnerId string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout, stderr io.Writer) (int, error)) { call := api.On("ExecuteCommand", mock.AnythingOfType("string"), mock.Anything, request.FullCommand(), + mock.AnythingOfType("bool"), mock.Anything, mock.Anything, mock.Anything) @@ -126,9 +127,25 @@ func MockApiExecute(api *nomad.ExecutorApiMock, request *dto.ExecutionRequest, exit, err := run(args.Get(0).(string), args.Get(1).(context.Context), args.Get(2).([]string), - args.Get(3).(io.Reader), - args.Get(4).(io.Writer), - args.Get(5).(io.Writer)) + args.Get(3).(bool), + args.Get(4).(io.Reader), + args.Get(5).(io.Writer), + args.Get(6).(io.Writer)) call.ReturnArguments = mock.Arguments{exit, err} }) } + +// HttpDelete sends a Delete Http Request with body to the passed url. +func HttpDelete(url string, body io.Reader) (response *http.Response, err error) { + req, _ := http.NewRequest(http.MethodDelete, url, body) + client := &http.Client{} + return client.Do(req) +} + +// HttpPatch sends a Patch Http Request with body to the passed url. +func HttpPatch(url string, contentType string, body io.Reader) (response *http.Response, err error) { + req, _ := http.NewRequest(http.MethodPatch, url, body) + req.Header.Set("Content-Type", contentType) + client := &http.Client{} + return client.Do(req) +}