Specify http not found exit code

by replacing it with StatusGone (410) for a missing runner and StatusFailedDependency (424) for missing or not accessible files.

api/swagger.yaml

@@ -78,6 +78,16 @@ components:
         - size
         - modificationTime
       additionalProperties: false
+    ClientError:
+      type: object
+      properties:
+        message:
+          description: Explanation on why the request could not be handled
+          type: string
+          example: Nomad server unreachable
+      required:
+        - message
+      additionalProperties: false
 
   responses:
     BadRequest:
@@ -94,26 +104,36 @@ components:
       description: Client could not be authenticated
     NotFound:
       description: The entity with the given identifier does not exist.
+    RunnerGone:
+      description: The runner is not available any longer.
+      content:
+        application/json:
+          schema:
+            $ref: "#/components/schemas/ClientError"
+    FailedFileDependency:
+      description: The file is not available.
+      content:
+        application/json:
+          schema:
+            $ref: "#/components/schemas/ClientError"
     InternalServerError:
       description: Request could not be handled
       content:
         application/json:
           schema:
-            type: object
+            allOf:
-            properties:
+              - $ref: "#/components/schemas/ClientError"
-              message:
+              - type: object
-                description: Explanation on why the request could not be handled
+                properties:
-                type: string
+                  errorCode:
-                example: Nomad server unreachable
+                    description: Machine readable error description
-              errorCode:
+                    type: string
-                description: Machine readable error description
+                    enum:
-                type: string
+                      - NOMAD_UNREACHABLE
-                enum:
+                      - NOMAD_OVERLOAD
-                  - NOMAD_UNREACHABLE
+                      - NOMAD_INTERNAL_SERVER_ERROR
-                  - NOMAD_OVERLOAD
+                      - UNKNOWN
-                  - NOMAD_INTERNAL_SERVER_ERROR
+                    example: NOMAD_UNREACHABLE
-                  - UNKNOWN
-                example: NOMAD_UNREACHABLE
 
 tags:
   - name: runner
@@ -275,8 +295,8 @@ paths:
           description: Success
         "401":
           $ref: "#/components/responses/Unauthorized"
-        "404":
+        "410":
-          $ref: "#/components/responses/NotFound"
+          $ref: "#/components/responses/RunnerGone"
         "500":
           $ref: "#/components/responses/InternalServerError"
 
@@ -307,6 +327,7 @@ paths:
           description: Specify the directory from where the filesystem is listed.
           schema:
             type: string
+            format: pct-encoded # rfc 3986
             default: "./"
           required: false
       responses:
@@ -324,8 +345,10 @@ paths:
                       $ref: "#/components/schemas/FileHeader"
         "401":
           $ref: "#/components/responses/Unauthorized"
-        "404":
+        "410":
-          $ref: "#/components/responses/NotFound"
+          $ref: "#/components/responses/RunnerGone"
+        "424":
+          $ref: "#/components/responses/FailedFileDependency"
         "500":
           $ref: "#/components/responses/InternalServerError"
     patch:
@@ -373,8 +396,8 @@ paths:
           $ref: "#/components/responses/BadRequest"
         "401":
           $ref: "#/components/responses/Unauthorized"
-        "404":
+        "410":
-          $ref: "#/components/responses/NotFound"
+          $ref: "#/components/responses/RunnerGone"
         "500":
           $ref: "#/components/responses/InternalServerError"
 
@@ -397,6 +420,7 @@ paths:
           description: Specify the file that should be returned by its filename including its path and extension.
           schema:
             type: string
+            format: pct-encoded # rfc 3986
             example: "./flag.txt"
           required: true
       responses:
@@ -409,8 +433,10 @@ paths:
                 format: binary
         "401":
           $ref: "#/components/responses/Unauthorized"
-        "404":
+        "410":
-          $ref: "#/components/responses/NotFound"
+          $ref: "#/components/responses/RunnerGone"
+        "424":
+          $ref: "#/components/responses/FailedFileDependency"
 
   /runners/{runnerId}/execute:
     post:
@@ -475,8 +501,8 @@ paths:
           $ref: "#/components/responses/BadRequest"
         "401":
           $ref: "#/components/responses/Unauthorized"
-        "404":
+        "410":
-          $ref: "#/components/responses/NotFound"
+          $ref: "#/components/responses/RunnerGone"
         "500":
           $ref: "#/components/responses/InternalServerError"
 

internal/api/environments.go

@@ -45,7 +45,7 @@ func (e *EnvironmentController) ConfigureRoutes(router *mux.Router) {
 func (e *EnvironmentController) list(writer http.ResponseWriter, request *http.Request) {
 	fetch, err := parseFetchParameter(request)
 	if err != nil {
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 
@@ -63,12 +63,12 @@ func (e *EnvironmentController) get(writer http.ResponseWriter, request *http.Re
 	environmentID, err := parseEnvironmentID(request)
 	if err != nil {
 		// This case is never used as the router validates the id format
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 	fetch, err := parseFetchParameter(request)
 	if err != nil {
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 
@@ -89,7 +89,7 @@ func (e *EnvironmentController) delete(writer http.ResponseWriter, request *http
 	environmentID, err := parseEnvironmentID(request)
 	if err != nil {
 		// This case is never used as the router validates the id format
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 
@@ -109,12 +109,12 @@ func (e *EnvironmentController) delete(writer http.ResponseWriter, request *http
 func (e *EnvironmentController) createOrUpdate(writer http.ResponseWriter, request *http.Request) {
 	req := new(dto.ExecutionEnvironmentRequest)
 	if err := json.NewDecoder(request.Body).Decode(req); err != nil {
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 	environmentID, err := parseEnvironmentID(request)
 	if err != nil {
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return
 	}
 

internal/api/helpers.go

@@ -11,12 +11,8 @@ func writeInternalServerError(writer http.ResponseWriter, err error, errorCode d
 	sendJSON(writer, &dto.InternalServerError{Message: err.Error(), ErrorCode: errorCode}, http.StatusInternalServerError)
 }
 
-func writeBadRequest(writer http.ResponseWriter, err error) {
+func writeClientError(writer http.ResponseWriter, err error, status uint16) {
-	sendJSON(writer, &dto.ClientError{Message: err.Error()}, http.StatusBadRequest)
+	sendJSON(writer, &dto.ClientError{Message: err.Error()}, int(status))
-}
-
-func writeNotFound(writer http.ResponseWriter, err error) {
-	sendJSON(writer, &dto.ClientError{Message: err.Error()}, http.StatusNotFound)
 }
 
 func sendJSON(writer http.ResponseWriter, content interface{}, httpStatusCode int) {
@@ -36,7 +32,7 @@ func sendJSON(writer http.ResponseWriter, content interface{}, httpStatusCode in
 
 func parseJSONRequestBody(writer http.ResponseWriter, request *http.Request, structure interface{}) error {
 	if err := json.NewDecoder(request.Body).Decode(structure); err != nil {
-		writeBadRequest(writer, err)
+		writeClientError(writer, err, http.StatusBadRequest)
 		return fmt.Errorf("error parsing JSON request body: %w", err)
 	}
 	return nil

internal/api/runners.go

@@ -65,7 +65,7 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req
 	if err != nil {
 		switch {
 		case errors.Is(err, runner.ErrUnknownExecutionEnvironment):
-			writeNotFound(writer, err)
+			writeClientError(writer, err, http.StatusNotFound)
 		case errors.Is(err, runner.ErrNoRunnersAvailable):
 			log.WithField("environment", logging.RemoveNewlineSymbol(strconv.Itoa(int(environmentID)))).
 				Warn("No runners available")
@@ -97,7 +97,7 @@ func (r *RunnerController) listFileSystem(writer http.ResponseWriter, request *h
 	writer.Header().Set("Content-Type", "application/json")
 	err = targetRunner.ListFileSystem(path, recursive, writer, request.Context())
 	if errors.Is(err, runner.ErrFileNotFound) {
-		writeNotFound(writer, err)
+		writeClientError(writer, err, http.StatusFailedDependency)
 		return
 	} else if err != nil {
 		log.WithError(err).Error("Could not perform the requested listFileSystem.")
@@ -133,7 +133,7 @@ func (r *RunnerController) fileContent(writer http.ResponseWriter, request *http
 	writer.Header().Set("Content-Type", "application/octet-stream")
 	err := targetRunner.GetFileContent(path, writer, request.Context())
 	if errors.Is(err, runner.ErrFileNotFound) {
-		writeNotFound(writer, err)
+		writeClientError(writer, err, http.StatusFailedDependency)
 		return
 	} else if err != nil {
 		log.WithError(err).Error("Could not retrieve the requested file.")
@@ -197,7 +197,7 @@ func (r *RunnerController) findRunnerMiddleware(next http.Handler) http.Handler
 			if readErr != nil {
 				log.WithError(readErr).Warn("Failed to discard the request body")
 			}
-			writeNotFound(writer, err)
+			writeClientError(writer, err, http.StatusGone)
 			return
 		}
 		ctx := runner.NewContext(request.Context(), targetRunner)
@@ -214,11 +214,7 @@ func (r *RunnerController) delete(writer http.ResponseWriter, request *http.Requ
 
 	err := r.manager.Return(targetRunner)
 	if err != nil {
-		if errors.Is(err, runner.ErrUnknownExecutionEnvironment) {
+		writeInternalServerError(writer, err, dto.ErrorNomadInternalServerError)
-			writeNotFound(writer, err)
-		} else {
-			writeInternalServerError(writer, err, dto.ErrorNomadInternalServerError)
-		}
 		return
 	}
 

internal/api/runners_test.go

@@ -76,7 +76,7 @@ func (s *MiddlewareTestSuite) TestFindRunnerMiddlewareIfRunnerDoesNotExist() {
 	recorder := httptest.NewRecorder()
 	s.router.ServeHTTP(recorder, s.runnerRequest(invalidID))
 
-	s.Equal(http.StatusNotFound, recorder.Code)
+	s.Equal(http.StatusGone, recorder.Code)
 }
 
 func (s *MiddlewareTestSuite) TestFindRunnerMiddlewareDoesNotEarlyRespond() {
@@ -91,7 +91,7 @@ func (s *MiddlewareTestSuite) TestFindRunnerMiddlewareDoesNotEarlyRespond() {
 	recorder := httptest.NewRecorder()
 	s.router.ServeHTTP(recorder, request)
 
-	s.Equal(http.StatusNotFound, recorder.Code)
+	s.Equal(http.StatusGone, recorder.Code)
 	s.Equal(0, body.Len()) // No data should be unread
 }
 
@@ -283,7 +283,7 @@ func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsBadRequestOn
 	s.Equal(http.StatusBadRequest, s.recorder.Code)
 }
 
-func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemToNonExistingRunnerReturnsNotFound() {
+func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemToNonExistingRunnerReturnsGone() {
 	s.runnerManager.On("Get", invalidID).Return(nil, runner.ErrRunnerNotFound)
 	path, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIDKey, invalidID)
 	s.Require().NoError(err)
@@ -294,7 +294,7 @@ func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemToNonExistingRunner
 	s.Require().NoError(err)
 
 	s.router.ServeHTTP(s.recorder, request)
-	s.Equal(http.StatusNotFound, s.recorder.Code)
+	s.Equal(http.StatusGone, s.recorder.Code)
 }
 
 func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsInternalServerErrorWhenCopyFailed() {
@@ -383,7 +383,7 @@ func (s *UpdateFileSystemRouteTestSuite) TestFileContent() {
 		request, err := http.NewRequest(http.MethodGet, routeURL.String(), strings.NewReader(""))
 		s.Require().NoError(err)
 		s.router.ServeHTTP(s.recorder, request)
-		s.Equal(http.StatusNotFound, s.recorder.Code)
+		s.Equal(http.StatusFailedDependency, s.recorder.Code)
 	})
 
 	s.recorder = httptest.NewRecorder()
@@ -449,7 +449,7 @@ func (s *DeleteRunnerRouteTestSuite) TestReturnInternalServerErrorWhenApiCallToN
 	s.Equal(http.StatusInternalServerError, recorder.Code)
 }
 
-func (s *DeleteRunnerRouteTestSuite) TestDeleteInvalidRunnerIdReturnsNotFound() {
+func (s *DeleteRunnerRouteTestSuite) TestDeleteInvalidRunnerIdReturnsGone() {
 	s.runnerManager.On("Get", mock.AnythingOfType("string")).Return(nil, tests.ErrDefault)
 	deleteURL, err := s.router.Get(DeleteRoute).URL(RunnerIDKey, "1nv4l1dID")
 	s.Require().NoError(err)
@@ -461,5 +461,5 @@ func (s *DeleteRunnerRouteTestSuite) TestDeleteInvalidRunnerIdReturnsNotFound()
 
 	s.router.ServeHTTP(recorder, request)
 
-	s.Equal(http.StatusNotFound, recorder.Code)
+	s.Equal(http.StatusGone, recorder.Code)
 }

internal/api/websocket.go

@@ -80,7 +80,7 @@ func (r *RunnerController) connectToRunner(writer http.ResponseWriter, request *
 
 	executionID := request.URL.Query().Get(ExecutionIDKey)
 	if !targetRunner.ExecutionExists(executionID) {
-		writeNotFound(writer, ErrUnknownExecutionID)
+		writeClientError(writer, ErrUnknownExecutionID, http.StatusNotFound)
 		return
 	}
 

internal/runner/nomad_runner.go

@@ -32,7 +32,7 @@ const (
 var (
 	ErrorUnknownExecution = errors.New("unknown execution")
 	ErrorFileCopyFailed   = errors.New("file copy failed")
-	ErrFileNotFound       = errors.New("file not found")
+	ErrFileNotFound       = errors.New("file not found or insufficient permissions")
 )
 
 // NomadJob is an abstraction to communicate with Nomad environments.

tests/e2e/runners_test.go

@@ -104,16 +104,16 @@ func (s *E2ETestSuite) TestDeleteRunnerRoute() {
 				s.Equal(http.StatusNoContent, resp.StatusCode)
 			})
 
-			s.Run("Deleting it again returns NotFound", func() {
+			s.Run("Deleting it again returns Gone", func() {
 				resp, err := helpers.HTTPDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID), nil)
 				s.NoError(err)
-				s.Equal(http.StatusNotFound, resp.StatusCode)
+				s.Equal(http.StatusGone, resp.StatusCode)
 			})
 
-			s.Run("Deleting non-existing runner returns NotFound", func() {
+			s.Run("Deleting non-existing runner returns Gone", func() {
 				resp, err := helpers.HTTPDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingStringID), nil)
 				s.NoError(err)
-				s.Equal(http.StatusNotFound, resp.StatusCode)
+				s.Equal(http.StatusGone, resp.StatusCode)
 			})
 		})
 	}
@@ -249,10 +249,10 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 				s.Equal(http.StatusBadRequest, resp.StatusCode)
 			})
 
-			s.Run("Copying to non-existing runner returns NotFound", func() {
+			s.Run("Copying to non-existing runner returns Gone", func() {
 				resp, err := CopyFiles(tests.NonExistingStringID, request)
 				s.NoError(err)
-				s.Equal(http.StatusNotFound, resp.StatusCode)
+				s.Equal(http.StatusGone, resp.StatusCode)
 			})
 		})
 	}
@@ -333,7 +333,7 @@ func (s *E2ETestSuite) TestGetFileContent_Nomad() {
 		getFileURL.RawQuery = fmt.Sprintf("%s=%s", api.PathKey, tests.DefaultFileName)
 		response, err := http.Get(getFileURL.String())
 		s.Require().NoError(err)
-		s.Equal(http.StatusNotFound, response.StatusCode)
+		s.Equal(http.StatusFailedDependency, response.StatusCode)
 	})
 
 	s.Run("Ok", func() {