diff --git a/.golangci.yaml b/.golangci.yaml index 7d2ee6cd..a6d04bad 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -19,9 +19,7 @@ linters: - varnamelen - wsl # Deprecated - - execinquery - exportloopref - - gomnd linters-settings: cyclop: diff --git a/cmd/poseidon/main.go b/cmd/poseidon/main.go index ffc33e6e..60f9427f 100644 --- a/cmd/poseidon/main.go +++ b/cmd/poseidon/main.go @@ -141,7 +141,7 @@ func watchMemoryAndAlert(options config.Profiling) { log.WithField("interval", options.MemoryInterval).Error("Configured memory interval too big") return } - intervalDuration := time.Duration(options.MemoryInterval) * time.Millisecond //nolint:gosec // We check for an integer overflow right above. + intervalDuration := time.Duration(options.MemoryInterval) * time.Millisecond var exceeded bool for { diff --git a/internal/api/environments_test.go b/internal/api/environments_test.go index 6cbdfef7..36fd8d73 100644 --- a/internal/api/environments_test.go +++ b/internal/api/environments_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/suite" ) -const jobHCLBasicFormat = "job \"%s\" {}" +const jobHCLBasicFormat = `job "%s" {}` type EnvironmentControllerTestSuite struct { tests.MemoryLeakTestSuite diff --git a/internal/api/runners.go b/internal/api/runners.go index 7b7cca1a..e6fcbc77 100644 --- a/internal/api/runners.go +++ b/internal/api/runners.go @@ -169,7 +169,7 @@ func (r *RunnerController) fileContent(writer http.ResponseWriter, request *http privilegedExecution = false } - writer.Header().Set("Content-Disposition", "attachment; filename=\""+path+"\"") + writer.Header().Set("Content-Disposition", `attachment; filename="`+path+`"`) logging.StartSpan(request.Context(), "api.fs.read", "File Content", func(ctx context.Context, _ *sentry.Span) { err = targetRunner.GetFileContent(ctx, path, writer, privilegedExecution) }) diff --git a/internal/api/websocket_test.go b/internal/api/websocket_test.go index cdd3d831..7da29170 100644 --- a/internal/api/websocket_test.go +++ b/internal/api/websocket_test.go @@ -352,7 +352,7 @@ func newRunnerWithNotMockedRunnerManager(s *MainTestSuite, apiMock *nomad.Execut runnerID := tests.DefaultRunnerID runnerJob := runner.NewNomadJob(s.TestCtx, runnerID, nil, apiMock, nil) - nomadEnvironment, err := environment.NewNomadEnvironment(s.TestCtx, 0, apiMock, "job \"template-0\" {}") + nomadEnvironment, err := environment.NewNomadEnvironment(s.TestCtx, 0, apiMock, `job "template-0" {}`) s.Require().NoError(err) eID, err := nomad.EnvironmentIDFromRunnerID(runnerID) s.Require().NoError(err) @@ -487,13 +487,21 @@ func mockAPIExecute(api *nomad.ExecutorAPIMock, request *dto.ExecutionRequest, mock.Anything, mock.Anything) call.Run(func(args mock.Arguments) { - exit, err := run(args.Get(0).(context.Context), - args.Get(1).(string), - args.Get(2).(string), - args.Get(3).(bool), - args.Get(5).(io.Reader), - args.Get(6).(io.Writer), - args.Get(7).(io.Writer)) - call.ReturnArguments = mock.Arguments{exit, err} + mockCtx, errCtx := args.Get(0).(context.Context) + mockRunnerID, errRunnerID := args.Get(1).(string) + mockCommand, errCommand := args.Get(2).(string) + mockTty, errTty := args.Get(3).(bool) + mockStdin, errStdin := args.Get(5).(io.Reader) + mockStdout, errStdout := args.Get(6).(io.Writer) + mockStderr, errStderr := args.Get(7).(io.Writer) + + if !errCtx || !errRunnerID || !errCommand || !errTty || !errStdin || !errStdout || !errStderr { + call.ReturnArguments = mock.Arguments{1, fmt.Errorf( + "%w: errCtx=%v, errRunnerId=%v, errCommand=%v, errTty=%v, errStdin=%v, errStdout=%v, errStderr=%v", + tests.ErrDefault, errCtx, errRunnerID, errCommand, errTty, errStdin, errStdout, errStderr)} + } else { + exit, err := run(mockCtx, mockRunnerID, mockCommand, mockTty, mockStdin, mockStdout, mockStderr) + call.ReturnArguments = mock.Arguments{exit, err} + } }) } diff --git a/internal/api/ws/codeocean_writer_test.go b/internal/api/ws/codeocean_writer_test.go index 3fa5e94e..772035b2 100644 --- a/internal/api/ws/codeocean_writer_test.go +++ b/internal/api/ws/codeocean_writer_test.go @@ -51,27 +51,27 @@ func (s *MainTestSuite) TestRawToCodeOceanWriter() { type sendExitInfoTestCase struct { name string info *runner.ExitInfo - message dto.WebSocketMessage + message *dto.WebSocketMessage } func (s *MainTestSuite) TestCodeOceanOutputWriter_SendExitInfo() { testCases := []sendExitInfoTestCase{ { "Timeout", &runner.ExitInfo{Err: runner.ErrRunnerInactivityTimeout}, - dto.WebSocketMessage{Type: dto.WebSocketMetaTimeout}, + &dto.WebSocketMessage{Type: dto.WebSocketMetaTimeout}, }, { "Error", &runner.ExitInfo{Err: websocket.ErrCloseSent}, - dto.WebSocketMessage{Type: dto.WebSocketOutputError, Data: "Error executing the request"}, + &dto.WebSocketMessage{Type: dto.WebSocketOutputError, Data: "Error executing the request"}, }, // CodeOcean expects this exact string in case of a OOM Killed runner. { "Specific data for OOM Killed runner", &runner.ExitInfo{Err: runner.ErrOOMKilled}, - dto.WebSocketMessage{Type: dto.WebSocketOutputError, Data: "the allocation was OOM Killed"}, + &dto.WebSocketMessage{Type: dto.WebSocketOutputError, Data: "the allocation was OOM Killed"}, }, { "Exit", &runner.ExitInfo{Code: 21}, - dto.WebSocketMessage{Type: dto.WebSocketExit, ExitCode: 21}, + &dto.WebSocketMessage{Type: dto.WebSocketExit, ExitCode: 21}, }, } diff --git a/internal/environment/nomad_manager_test.go b/internal/environment/nomad_manager_test.go index 4f69d7de..4fe3a565 100644 --- a/internal/environment/nomad_manager_test.go +++ b/internal/environment/nomad_manager_test.go @@ -116,7 +116,7 @@ func (s *MainTestSuite) TestNewNomadEnvironmentManager() { }) s.Run("loads template environment job from file", func() { - templateJobHCL := "job \"" + tests.DefaultTemplateJobID + "\" {}" + templateJobHCL := `job "` + tests.DefaultTemplateJobID + `" {}` environment, err := NewNomadEnvironment(s.TestCtx, tests.DefaultEnvironmentIDAsInteger, executorAPIMock, templateJobHCL) s.Require().NoError(err) diff --git a/internal/nomad/command_execution.go b/internal/nomad/command_execution.go index 921079d2..157c99a5 100644 --- a/internal/nomad/command_execution.go +++ b/internal/nomad/command_execution.go @@ -179,7 +179,7 @@ func injectStartDebugMessage(command string, start uint, end int) string { } description := strings.Join(commandFields, " ") - if strings.HasPrefix(description, "\"") && strings.HasSuffix(description, "\"") { + if strings.HasPrefix(description, `"`) && strings.HasSuffix(description, `"`) { description = description[1 : len(description)-1] } if description == "" { diff --git a/internal/nomad/command_execution_test.go b/internal/nomad/command_execution_test.go index 5f15b34c..ee3b4cd5 100644 --- a/internal/nomad/command_execution_test.go +++ b/internal/nomad/command_execution_test.go @@ -37,7 +37,7 @@ func (s *ExecuteCommandTestSuite) SetupTest() { s.MemoryLeakTestSuite.SetupTest() s.allocationID = "test-allocation-id" s.ctx = context.Background() - s.testCommand = "echo \"do nothing\"" + s.testCommand = `echo "do nothing"` s.expectedStdout = "stdout" s.expectedStderr = "stderr" s.apiMock = &apiQuerierMock{} diff --git a/internal/nomad/sentry_debug_writer_test.go b/internal/nomad/sentry_debug_writer_test.go index 8532c7c5..d2d70130 100644 --- a/internal/nomad/sentry_debug_writer_test.go +++ b/internal/nomad/sentry_debug_writer_test.go @@ -35,7 +35,7 @@ func (s *MainTestSuite) TestSentryDebugWriter_regression_593_empty_command() { debugWriter := NewSentryDebugWriter(s.TestCtx, buf) const commandFieldAfterEnv = 4 // instead of "env CODEOCEAN=true /bin/bash -c sleep infinity" just "sleep infinity". - command := injectStartDebugMessage("env CODEOCEAN=true /bin/bash -c \"\"", commandFieldAfterEnv, -1) + command := injectStartDebugMessage(`env CODEOCEAN=true /bin/bash -c ""`, commandFieldAfterEnv, -1) cmd := exec.Command("/bin/bash", "-c", command) stdout, err := cmd.Output() s.Require().NoError(err) diff --git a/internal/runner/aws_runner.go b/internal/runner/aws_runner.go index 7949dd6c..aa7585a0 100644 --- a/internal/runner/aws_runner.go +++ b/internal/runner/aws_runner.go @@ -248,5 +248,5 @@ func hideEnvironmentVariables(request *dto.ExecutionRequest, unsetPrefix string) if request.Environment == nil { request.Environment = make(map[string]string) } - request.Command = "unset \"${!" + unsetPrefix + "@}\" && " + request.Command + request.Command = `unset "${!` + unsetPrefix + `@}" && ` + request.Command } diff --git a/internal/runner/nomad_runner.go b/internal/runner/nomad_runner.go index e70d6cd2..2230534f 100644 --- a/internal/runner/nomad_runner.go +++ b/internal/runner/nomad_runner.go @@ -114,7 +114,7 @@ func (r *NomadJob) MappedPorts() []*dto.MappedPort { log.WithError(util.ErrOverflow).WithField("mapping", portMapping.To).Warn("not a valid port") } ports = append(ports, &dto.MappedPort{ - ExposedPort: uint(portMapping.To), //nolint:gosec // We check for an overflow right above. + ExposedPort: uint(portMapping.To), HostAddress: fmt.Sprintf("%s:%d", portMapping.HostIP, portMapping.Value), }) } @@ -140,7 +140,7 @@ func (r *NomadJob) UpdateMappedPorts(ports []*dto.MappedPort) error { mapping = append(mapping, nomadApi.PortMapping{ Value: port, - To: int(portMapping.ExposedPort), //nolint:gosec // We check for an integer overflow right above. + To: int(portMapping.ExposedPort), HostIP: hostAddress[0], }) } diff --git a/internal/runner/nomad_runner_test.go b/internal/runner/nomad_runner_test.go index c645cbd0..1f639912 100644 --- a/internal/runner/nomad_runner_test.go +++ b/internal/runner/nomad_runner_test.go @@ -66,7 +66,7 @@ func (s *MainTestSuite) TestMarshalRunner() { runner := NewNomadJob(s.TestCtx, tests.DefaultRunnerID, nil, apiMock, func(_ Runner) error { return nil }) marshal, err := json.Marshal(runner) s.Require().NoError(err) - s.Equal("{\"runnerId\":\""+tests.DefaultRunnerID+"\"}", string(marshal)) + s.JSONEq(`{"runnerId":"`+tests.DefaultRunnerID+`"}`, string(marshal)) s.Require().NoError(runner.Destroy(nil)) } diff --git a/pkg/dto/dto.go b/pkg/dto/dto.go index 3c3b5b25..1109b3b4 100644 --- a/pkg/dto/dto.go +++ b/pkg/dto/dto.go @@ -244,7 +244,7 @@ type WebSocketMessage struct { // MarshalJSON implements the json.Marshaler interface. // This converts the WebSocketMessage into the expected schema (see docs/websocket.schema.json). -func (m WebSocketMessage) MarshalJSON() (res []byte, err error) { +func (m *WebSocketMessage) MarshalJSON() (res []byte, err error) { switch m.Type { case WebSocketOutputStdout, WebSocketOutputStderr, WebSocketOutputError: res, err = json.Marshal(struct { diff --git a/pkg/nullio/ls2json.go b/pkg/nullio/ls2json.go index b95c162a..a8f01683 100644 --- a/pkg/nullio/ls2json.go +++ b/pkg/nullio/ls2json.go @@ -90,7 +90,7 @@ func (w *Ls2JsonWriter) Write(lsData []byte) (int, error) { func (w *Ls2JsonWriter) initializeJSONObject() (count int, err error) { if !w.jsonStartSent { - count, err = w.Target.Write([]byte("{\"files\": [")) + count, err = w.Target.Write([]byte(`{"files": [`)) if count == 0 || err != nil { log.WithContext(w.Ctx).WithError(err).Warn("Could not write to target") err = fmt.Errorf("could not write to target: %w", err) diff --git a/pkg/nullio/ls2json_test.go b/pkg/nullio/ls2json_test.go index 09a9d3c1..1da1483b 100644 --- a/pkg/nullio/ls2json_test.go +++ b/pkg/nullio/ls2json_test.go @@ -35,10 +35,10 @@ func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteCreationAndClose() { s.Zero(count) s.Require().NoError(err) - s.Equal("{\"files\": [", s.buf.String()) + s.Equal(`{"files": [`, s.buf.String()) //nolint:testifylint // The expected string is not a valid JSON s.writer.Close() - s.Equal("{\"files\": []}", s.buf.String()) + s.JSONEq(`{"files": []}`, s.buf.String()) } func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteFile() { @@ -48,8 +48,8 @@ func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteFile() { s.Require().NoError(err) s.writer.Close() - s.Equal("{\"files\": [{\"name\":\"flag\",\"entryType\":\"-\",\"size\":0,\"modificationTime\":1660763446"+ - ",\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"}]}", + s.JSONEq(`{"files": [{"name":"flag","entryType":"-","size":0,"modificationTime":1660763446`+ + `,"permissions":"rw-rw-r--","owner":"kali","group":"kali"}]}`, s.buf.String()) } @@ -62,14 +62,14 @@ func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteRecursive() { s.Require().NoError(err) s.writer.Close() - s.Equal("{\"files\": ["+ - "{\"name\":\"./dir\",\"entryType\":\"d\",\"size\":4096,\"modificationTime\":1660764411,"+ - "\"permissions\":\"rwxrwxr-x\",\"owner\":\"kali\",\"group\":\"kali\"},"+ - "{\"name\":\"./flag\",\"entryType\":\"-\",\"size\":0,\"modificationTime\":1660763446,"+ - "\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"},"+ - "{\"name\":\"./dir/another.txt\",\"entryType\":\"-\",\"size\":3,\"modificationTime\":1660764366,"+ - "\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"}"+ - "]}", + s.JSONEq(`{"files": [`+ + `{"name":"./dir","entryType":"d","size":4096,"modificationTime":1660764411,`+ + `"permissions":"rwxrwxr-x","owner":"kali","group":"kali"},`+ + `{"name":"./flag","entryType":"-","size":0,"modificationTime":1660763446,`+ + `"permissions":"rw-rw-r--","owner":"kali","group":"kali"},`+ + `{"name":"./dir/another.txt","entryType":"-","size":3,"modificationTime":1660764366,`+ + `"permissions":"rw-rw-r--","owner":"kali","group":"kali"}`+ + `]}`, s.buf.String()) } @@ -77,17 +77,19 @@ func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteRemaining() { input1 := "total 4\n" + perm664 + "1 " + ownerGroupKali + "3 1660764366 an.txt\n" + perm664 + "1 kal" _, err := s.writer.Write([]byte(input1)) s.Require().NoError(err) - s.Equal("{\"files\": [{\"name\":\"an.txt\",\"entryType\":\"-\",\"size\":3,\"modificationTime\":1660764366,"+ - "\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"}", s.buf.String()) + //nolint:testifylint // The expected string is not a valid JSON + s.Equal(`{"files": [{"name":"an.txt","entryType":"-","size":3,"modificationTime":1660764366,`+ + `"permissions":"rw-rw-r--","owner":"kali","group":"kali"}`, s.buf.String()) input2 := "i kali 0 1660763446 flag\n" _, err = s.writer.Write([]byte(input2)) s.Require().NoError(err) s.writer.Close() - s.Equal("{\"files\": [{\"name\":\"an.txt\",\"entryType\":\"-\",\"size\":3,\"modificationTime\":1660764366,"+ - "\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"},"+ - "{\"name\":\"flag\",\"entryType\":\"-\",\"size\":0,\"modificationTime\":1660763446,"+ - "\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"}]}", s.buf.String()) + //nolint:testifylint // The expected string is not a valid JSON + s.Equal(`{"files": [{"name":"an.txt","entryType":"-","size":3,"modificationTime":1660764366,`+ + `"permissions":"rw-rw-r--","owner":"kali","group":"kali"},`+ + `{"name":"flag","entryType":"-","size":0,"modificationTime":1660763446,`+ + `"permissions":"rw-rw-r--","owner":"kali","group":"kali"}]}`, s.buf.String()) } func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteLink() { @@ -95,7 +97,7 @@ func (s *Ls2JsonTestSuite) TestLs2JsonWriter_WriteLink() { _, err := s.writer.Write([]byte(input1)) s.Require().NoError(err) s.writer.Close() - s.Equal("{\"files\": [{\"name\":\"another.txt\",\"entryType\":\"l\",\"linkTarget\":\"/bin/bash\",\"size\":3,"+ - "\"modificationTime\":1660764366,\"permissions\":\"rw-rw-r--\",\"owner\":\"kali\",\"group\":\"kali\"}]}", + s.JSONEq(`{"files": [{"name":"another.txt","entryType":"l","linkTarget":"/bin/bash","size":3,`+ + `"modificationTime":1660764366,"permissions":"rw-rw-r--","owner":"kali","group":"kali"}]}`, s.buf.String()) } diff --git a/tests/e2e/runners_test.go b/tests/e2e/runners_test.go index e3641e4f..c1faabeb 100644 --- a/tests/e2e/runners_test.go +++ b/tests/e2e/runners_test.go @@ -110,7 +110,7 @@ func (s *E2ETestSuite) TestListFileSystem_Nomad() { s.Equal(http.StatusOK, response.StatusCode) data, err := io.ReadAll(response.Body) s.Require().NoError(err) - s.Equal("{\"files\": []}", string(data)) + s.JSONEq(`{"files": []}`, string(data)) }) s.Run("With file", func() { @@ -458,7 +458,7 @@ func (s *E2ETestSuite) TestGetFileContent_Nomad() { s.Require().NoError(err) s.Equal(http.StatusOK, response.StatusCode) s.Equal(strconv.Itoa(len(newFileContent)), response.Header.Get("Content-Length")) - s.Equal("attachment; filename=\""+tests.DefaultFileName+"\"", response.Header.Get("Content-Disposition")) + s.Equal(`attachment; filename="`+tests.DefaultFileName+`"`, response.Header.Get("Content-Disposition")) content, err := io.ReadAll(response.Body) s.Require().NoError(err) s.Equal(newFileContent, content)