Fix remaining linter issues

.golangci.yaml

@@ -1,6 +1,26 @@
 run:
   go: "1.22"
 
+linters:
+  enable-all: true
+  disable:
+    - contextcheck
+    - depguard
+    - exhaustive
+    - exhaustruct
+    - gochecknoglobals
+    - gomoddirectives
+    - gomodguard
+    - ireturn
+    - nlreturn
+    - nonamedreturns
+    - paralleltest
+    - testpackage
+    - varnamelen
+    - wsl
+    - execinquery
+    - gomnd
+
 linters-settings:
   cyclop:
     max-complexity: 15
@@ -12,19 +32,6 @@ linters-settings:
     statements: 50
   gocognit:
     min-complexity: 15
-  gocritic:
-    enabled-tags:
-      - diagnostic
-      - experimental
-      - opinionated
-      - performance
-      - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - hugeParam        # No golang generic support. See https://github.com/golangci/golangci-lint/issues/2649
-      - rangeValCopy     # ^
-      - typeDefFirst     # |
-      - paramTypeCombine # |
   interfacebloat:
     max: 12
   lll:
@@ -33,43 +40,16 @@ linters-settings:
     always: true
   misspell:
     locale: US
-  mnd:
-    # don't include "assign"
-    checks:
-    - argument
-    - case
-    - condition
-    - operation
-    - return
   nakedret:
     max-func-lines: 5
 
-linters:
-  enable-all: true
-  disable:
-    - contextcheck
-    - depguard
-    - exhaustive
-    - exhaustruct
-    - gochecknoglobals
-    - gomoddirectives
-    - gomodguard
-    - ireturn
-    - nlreturn
-    - nonamedreturns
-    - paralleltest
-    - testpackage
-    - varnamelen
-    - wsl
-    - execinquery
-    - gomnd
-
 issues:
   exclude-rules:
     - path: _test\.go
       linters:
-        - noctx
         # Always closing the HTTP body unnecessarily complicates the tests
         - bodyclose
+        - funlen
+        - noctx
         # We don't need to wrap errors in tests
-        - wrapcheck
+        - wrapcheck

cmd/poseidon/main.go

@@ -334,7 +334,7 @@ func createManagerHandler(ctx context.Context, handler managerCreator, enabled b
 
 func createNomadManager(ctx context.Context) (runner.Manager, environment.ManagerHandler) {
 	// API initialization
-	nomadAPIClient, err := nomad.NewExecutorAPI(&config.Config.Nomad)
+	nomadAPIClient, err := nomad.NewExecutorAPI(ctx, &config.Config.Nomad)
 	if err != nil {
 		log.WithError(err).WithField("nomad_config", config.Config.Nomad).Fatal("Error creating Nomad API client")
 	}

internal/api/auth/auth_test.go

@@ -32,7 +32,7 @@ func (s *AuthenticationMiddlewareTestSuite) SetupTest() {
 	}
 	s.request = request
 	s.httpAuthenticationMiddleware = HTTPAuthenticationMiddleware(
-		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 			w.WriteHeader(http.StatusOK)
 		}))
 }

internal/api/health.go

@@ -19,7 +19,9 @@ var ErrPrewarmingPoolDepleting = errors.New("the prewarming pool is depleting")
 func Health(manager environment.Manager) http.HandlerFunc {
 	return func(writer http.ResponseWriter, request *http.Request) {
 		if err := checkPrewarmingPool(manager); err != nil {
-			sendJSON(request.Context(), writer, &dto.InternalServerError{Message: err.Error(), ErrorCode: dto.PrewarmingPoolDepleting}, http.StatusServiceUnavailable)
+			sendJSON(request.Context(), writer,
+				&dto.InternalServerError{Message: err.Error(), ErrorCode: dto.PrewarmingPoolDepleting},
+				http.StatusServiceUnavailable)
 			return
 		}
 

internal/api/websocket.go

@@ -44,8 +44,8 @@ func newWebSocketProxy(proxyCtx context.Context, connection ws.Connection) *webS
 
 	proxy := &webSocketProxy{
 		ctx:    wsCtx,
-		Input:  ws.NewCodeOceanToRawReader(connection, wsCtx, proxyCtx),
-		Output: ws.NewCodeOceanOutputWriter(connection, wsCtx, cancelWsCommunication),
+		Input:  ws.NewCodeOceanToRawReader(wsCtx, proxyCtx, connection),
+		Output: ws.NewCodeOceanOutputWriter(wsCtx, connection, cancelWsCommunication),
 	}
 
 	connection.SetCloseHandler(func(code int, text string) error {

internal/api/ws/codeocean_reader.go

@@ -44,7 +44,7 @@ type codeOceanToRawReader struct {
 	priorityBuffer chan byte
 }
 
-func NewCodeOceanToRawReader(connection Connection, wsCtx, executorCtx context.Context) *codeOceanToRawReader {
+func NewCodeOceanToRawReader(wsCtx, executorCtx context.Context, connection Connection) WebSocketReader {
 	return &codeOceanToRawReader{
 		connection:     connection,
 		readCtx:        wsCtx, // This context may be canceled before the executorCtx.
@@ -83,17 +83,17 @@ func (cr *codeOceanToRawReader) readInputLoop(ctx context.Context) {
 		case <-readMessage:
 		}
 
-		if inputContainsError(messageType, err, loopContext) {
+		if inputContainsError(loopContext, messageType, err) {
 			return
 		}
-		if handleInput(reader, cr.buffer, loopContext) {
+		if handleInput(loopContext, reader, cr.buffer) {
 			return
 		}
 	}
 }
 
 // handleInput receives a new message from the client and may forward it to the executor.
-func handleInput(reader io.Reader, buffer chan byte, ctx context.Context) (done bool) {
+func handleInput(ctx context.Context, reader io.Reader, buffer chan byte) (done bool) {
 	message, err := io.ReadAll(reader)
 	if err != nil {
 		log.WithContext(ctx).WithError(err).Warn("error while reading WebSocket message")
@@ -111,7 +111,7 @@ func handleInput(reader io.Reader, buffer chan byte, ctx context.Context) (done
 	return false
 }
 
-func inputContainsError(messageType int, err error, ctx context.Context) (done bool) {
+func inputContainsError(ctx context.Context, messageType int, err error) (done bool) {
 	if err != nil && websocket.IsCloseError(err, websocket.CloseNormalClosure) {
 		log.WithContext(ctx).Debug("ReadInputLoop: The client closed the connection!")
 		// The close handler will do something soon.

internal/api/ws/codeocean_reader_test.go

@@ -24,7 +24,8 @@ func (s *MainTestSuite) TestCodeOceanToRawReaderReturnsOnlyAfterOneByteWasRead()
 	readingCtx, cancel := context.WithCancel(context.Background())
 	forwardingCtx := readingCtx
 	defer cancel()
-	reader := NewCodeOceanToRawReader(nil, readingCtx, forwardingCtx)
+	reader, ok := NewCodeOceanToRawReader(readingCtx, forwardingCtx, nil).(*codeOceanToRawReader)
+	s.Require().True(ok)
 
 	read := make(chan bool)
 	go func() {
@@ -61,7 +62,7 @@ func (s *MainTestSuite) TestCodeOceanToRawReaderReturnsOnlyAfterOneByteWasReadFr
 	readingCtx, cancel := context.WithCancel(context.Background())
 	forwardingCtx := readingCtx
 	defer cancel()
-	reader := NewCodeOceanToRawReader(connection, readingCtx, forwardingCtx)
+	reader := NewCodeOceanToRawReader(readingCtx, forwardingCtx, connection)
 	reader.Start()
 
 	read := make(chan bool)

internal/api/ws/codeocean_writer.go

@@ -63,9 +63,7 @@ type writingLoopMessage struct {
 
 // NewCodeOceanOutputWriter provides an codeOceanOutputWriter for the time the context ctx is active.
 // The codeOceanOutputWriter handles all the messages defined in the websocket.schema.json (start, timeout, stdout, ...).
-func NewCodeOceanOutputWriter(
-	connection Connection, ctx context.Context, done context.CancelFunc,
-) WebSocketWriter {
+func NewCodeOceanOutputWriter(ctx context.Context, connection Connection, done context.CancelFunc) WebSocketWriter {
 	cw := &codeOceanOutputWriter{
 		connection: connection,
 		queue:      make(chan *writingLoopMessage, CodeOceanOutputWriterBufferSize),
@@ -145,7 +143,7 @@ func (cw *codeOceanOutputWriter) startWritingLoop(writingLoopDone context.Cancel
 		message := <-cw.queue
 		done := true
 		if message.data != nil {
-			done = sendMessage(cw.connection, message.data, cw.ctx)
+			done = sendMessage(cw.ctx, cw.connection, message.data)
 		}
 		if done || message.done {
 			log.WithContext(cw.ctx).Trace("Writing loop done")
@@ -156,7 +154,7 @@ func (cw *codeOceanOutputWriter) startWritingLoop(writingLoopDone context.Cancel
 }
 
 // sendMessage is a helper function for the writing loop. It must not be called from somewhere else!
-func sendMessage(connection Connection, message *dto.WebSocketMessage, ctx context.Context) (done bool) {
+func sendMessage(ctx context.Context, connection Connection, message *dto.WebSocketMessage) (done bool) {
 	if message == nil {
 		return false
 	}

internal/api/ws/codeocean_writer_test.go

@@ -15,7 +15,7 @@ func (s *MainTestSuite) TestRawToCodeOceanWriter() {
 	connectionMock, messages := buildConnectionMock(&s.MemoryLeakTestSuite)
 	proxyCtx, cancel := context.WithCancel(context.Background())
 	defer cancel()
-	output := NewCodeOceanOutputWriter(connectionMock, proxyCtx, cancel)
+	output := NewCodeOceanOutputWriter(proxyCtx, connectionMock, cancel)
 	defer output.Close(nil)
 	<-messages // start messages
 
@@ -80,7 +80,7 @@ func (s *MainTestSuite) TestCodeOceanOutputWriter_SendExitInfo() {
 			connectionMock, messages := buildConnectionMock(&s.MemoryLeakTestSuite)
 			proxyCtx, cancel := context.WithCancel(context.Background())
 			defer cancel()
-			output := NewCodeOceanOutputWriter(connectionMock, proxyCtx, cancel)
+			output := NewCodeOceanOutputWriter(proxyCtx, connectionMock, cancel)
 			<-messages // start messages
 
 			output.Close(test.info)

internal/config/config.go

@@ -300,7 +300,7 @@ func loadValue(prefix string, value reflect.Value, logEntry *logrus.Entry) {
 
 func randomFilterToken() string {
 	const tokenLength = 32
-	randomBytes := make([]byte, tokenLength)
+	randomBytes := make([]byte, tokenLength) // nozero // rand.Read requires the slice to be pre-allocated.
 	n, err := rand.Read(randomBytes)
 	if n != tokenLength || err != nil {
 		log.WithError(err).WithField("byteCount", n).Fatal("Failed to generate random token")

internal/environment/nomad_environment.go

@@ -47,8 +47,8 @@ func NewNomadEnvironment(ctx context.Context, environmentID dto.EnvironmentID, a
 
 	ctx, cancel := context.WithCancel(context.WithoutCancel(ctx))
 	e := &NomadEnvironment{apiClient, jobHCL, job, nil, ctx, cancel}
-	e.idleRunners = storage.NewMonitoredLocalStorage[runner.Runner](monitoring.MeasurementIdleRunnerNomad,
-		runner.MonitorEnvironmentID[runner.Runner](environmentID), time.Minute, ctx)
+	e.idleRunners = storage.NewMonitoredLocalStorage[runner.Runner](
+		ctx, monitoring.MeasurementIdleRunnerNomad, runner.MonitorEnvironmentID[runner.Runner](environmentID), time.Minute)
 	return e, nil
 }
 

internal/environment/nomad_manager.go

@@ -178,7 +178,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(
 
 // KeepEnvironmentsSynced loads all environments, runner existing at Nomad, and watches Nomad events to handle further changes.
 func (m *NomadEnvironmentManager) KeepEnvironmentsSynced(ctx context.Context, synchronizeRunners func(ctx context.Context) error) {
-	err := util.RetryConstantAttemptsWithContext(math.MaxInt, ctx, func() error {
+	err := util.RetryConstantAttemptsWithContext(ctx, math.MaxInt, func() error {
 		// Load Environments
 		if err := m.load(ctx); err != nil {
 			log.WithContext(ctx).WithError(err).Warn("Loading Environments failed! Retrying...")
@@ -246,8 +246,8 @@ func newNomadEnvironmentFromJob(ctx context.Context, job *nomadApi.Job, apiClien
 		ctx:       ctx,
 		cancel:    cancel,
 	}
-	e.idleRunners = storage.NewMonitoredLocalStorage[runner.Runner](monitoring.MeasurementIdleRunnerNomad,
-		runner.MonitorEnvironmentID[runner.Runner](e.ID()), time.Minute, ctx)
+	e.idleRunners = storage.NewMonitoredLocalStorage[runner.Runner](
+		ctx, monitoring.MeasurementIdleRunnerNomad, runner.MonitorEnvironmentID[runner.Runner](e.ID()), time.Minute)
 	return e
 }
 

internal/nomad/nomad.go

@@ -127,16 +127,16 @@ type APIClient struct {
 
 // NewExecutorAPI creates a new api client.
 // One client is usually sufficient for the complete runtime of the API.
-func NewExecutorAPI(nomadConfig *config.Nomad) (ExecutorAPI, error) {
+func NewExecutorAPI(ctx context.Context, nomadConfig *config.Nomad) (ExecutorAPI, error) {
 	client := &APIClient{
 		apiQuerier:  &nomadAPIClient{},
 		evaluations: storage.NewLocalStorage[chan error](),
-		allocations: storage.NewMonitoredLocalStorage[*allocationData](monitoring.MeasurementNomadAllocations,
+		allocations: storage.NewMonitoredLocalStorage[*allocationData](ctx, monitoring.MeasurementNomadAllocations,
 			func(p *write.Point, object *allocationData, _ storage.EventType) {
 				p.AddTag(monitoring.InfluxKeyJobID, object.jobID)
 				p.AddTag(monitoring.InfluxKeyClientStatus, object.allocClientStatus)
 				p.AddTag(monitoring.InfluxKeyNomadNode, object.allocNomadNode)
-			}, 0, nil),
+			}, 0),
 	}
 	err := client.init(nomadConfig)
 	return client, err

internal/nomad/nomad_test.go

@@ -170,7 +170,7 @@ func (s *MainTestSuite) TestNewExecutorApiCanBeCreatedWithoutError() {
 	err := expectedClient.init(NomadTestConfig(TestDefaultAddress))
 	s.Require().NoError(err)
 
-	_, err = NewExecutorAPI(NomadTestConfig(TestDefaultAddress))
+	_, err = NewExecutorAPI(s.TestCtx, NomadTestConfig(TestDefaultAddress))
 	s.Require().NoError(err)
 }
 

internal/runner/abstract_manager.go

@@ -27,10 +27,8 @@ type AbstractManager struct {
 // Since this manager is currently directly bound to the lifespan of Poseidon, it does not need a context cancel.
 func NewAbstractManager(ctx context.Context) *AbstractManager {
 	return &AbstractManager{
-		environments: storage.NewMonitoredLocalStorage[ExecutionEnvironment](
-			monitoring.MeasurementEnvironments, monitorEnvironmentData, 0, ctx),
-		usedRunners: storage.NewMonitoredLocalStorage[Runner](
-			monitoring.MeasurementUsedRunner, MonitorRunnersEnvironmentID, time.Hour, ctx),
+		environments: storage.NewMonitoredLocalStorage[ExecutionEnvironment](ctx, monitoring.MeasurementEnvironments, monitorEnvironmentData, 0),
+		usedRunners:  storage.NewMonitoredLocalStorage[Runner](ctx, monitoring.MeasurementUsedRunner, MonitorRunnersEnvironmentID, time.Hour),
 	}
 }
 

internal/runner/aws_runner.go

@@ -62,7 +62,7 @@ func NewAWSFunctionWorkload(
 		cancel:            cancel,
 	}
 	workload.executions = storage.NewMonitoredLocalStorage[*dto.ExecutionRequest](
-		monitoring.MeasurementExecutionsAWS, monitorExecutionsRunnerID(environment.ID(), workload.id), time.Minute, ctx)
+		ctx, monitoring.MeasurementExecutionsAWS, monitorExecutionsRunnerID(environment.ID(), workload.id), time.Minute)
 	workload.InactivityTimer = NewInactivityTimer(workload, func(_ Runner) error {
 		return workload.Destroy(nil)
 	})

internal/runner/nomad_runner.go

@@ -81,7 +81,7 @@ func NewNomadJob(ctx context.Context, jobID string, portMappings []nomadApi.Port
 		cancel:       cancel,
 	}
 	job.executions = storage.NewMonitoredLocalStorage[*dto.ExecutionRequest](
-		monitoring.MeasurementExecutionsNomad, monitorExecutionsRunnerID(job.Environment(), jobID), time.Minute, ctx)
+		ctx, monitoring.MeasurementExecutionsNomad, monitorExecutionsRunnerID(job.Environment(), jobID), time.Minute)
 	job.InactivityTimer = NewInactivityTimer(job, func(r Runner) error {
 		err := r.Destroy(ErrRunnerInactivityTimeout)
 		if err != nil {

pkg/logging/logging.go

@@ -17,16 +17,19 @@ import (
 
 const TimestampFormat = "2006-01-02T15:04:05.000000Z"
 
-var log = &logrus.Logger{
-	Out: os.Stderr,
-	Formatter: &logrus.TextFormatter{
-		TimestampFormat: TimestampFormat,
-		DisableColors:   true,
-		FullTimestamp:   true,
-	},
-	Hooks: make(logrus.LevelHooks),
-	Level: logrus.InfoLevel,
-}
+var (
+	log = &logrus.Logger{
+		Out: os.Stderr,
+		Formatter: &logrus.TextFormatter{
+			TimestampFormat: TimestampFormat,
+			DisableColors:   true,
+			FullTimestamp:   true,
+		},
+		Hooks: make(logrus.LevelHooks),
+		Level: logrus.InfoLevel,
+	}
+	ErrResponseWriterHijack = errors.New("response writer cannot be hijacked")
+)
 
 const GracefulSentryShutdown = 5 * time.Second
 
@@ -73,7 +76,7 @@ func (writer *ResponseWriter) WriteHeader(code int) {
 func (writer *ResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
 	hijacker, ok := writer.ResponseWriter.(http.Hijacker)
 	if !ok {
-		return nil, nil, errors.New("response writer cannot be hijacked")
+		return nil, nil, ErrResponseWriterHijack
 	}
 
 	conn, rw, err := hijacker.Hijack()

pkg/nullio/content_length.go

@@ -26,9 +26,8 @@ type ContentLengthWriter struct {
 func (w *ContentLengthWriter) Write(p []byte) (count int, err error) {
 	if w.contentLengthSet {
 		return w.handleDataForwarding(p)
-	} else {
-		return w.handleContentLengthParsing(p)
 	}
+	return w.handleContentLengthParsing(p)
 }
 
 func (w *ContentLengthWriter) handleDataForwarding(p []byte) (int, error) {

pkg/nullio/nullio.go

@@ -38,7 +38,6 @@ func (rw *ReadWriter) Write(p []byte) (int, error) {
 	n, err := io.Discard.Write(p)
 	if err != nil {
 		return n, fmt.Errorf("error writing to io.Discard: %w", err)
-	} else {
-		return n, nil
 	}
+	return n, nil
 }