diff --git a/cmd/server_test.go b/cmd/server_test.go index 273ac69b2..ae1a9e23c 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -276,7 +276,11 @@ func getDocumentedFlags(t *testing.T) []string { file, err := os.Open(docFile) Ok(t, err) - defer file.Close() + defer func() { + if closeErr := file.Close(); closeErr != nil { + t.Errorf("failed to close file: %v", closeErr) + } + }() scanner := bufio.NewScanner(file) for scanner.Scan() { diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index 49b6504ae..a7459a12c 100644 --- a/server/controllers/events/events_controller.go +++ b/server/controllers/events/events_controller.go @@ -211,20 +211,28 @@ func (e *VCSEventsController) handleGithubPost(w http.ResponseWriter, r *http.Re } scope.Counter(fmt.Sprintf("error_%d", resp.err.code)).Inc(1) w.WriteHeader(resp.err.code) - fmt.Fprintln(w, resp.err.err.Error()) + if _, err := fmt.Fprintln(w, resp.err.err.Error()); err != nil { + e.Logger.Err("failed to write error response: %v", err) + } return } scope.Counter(fmt.Sprintf("success_%d", http.StatusOK)).Inc(1) w.WriteHeader(http.StatusOK) - fmt.Fprintln(w, resp.body) + if _, err := fmt.Fprintln(w, resp.body); err != nil { + e.Logger.Err("failed to write success response: %v", err) + } } func (e *VCSEventsController) handleBitbucketCloudPost(w http.ResponseWriter, r *http.Request) { eventType := r.Header.Get(bitbucketEventTypeHeader) reqID := r.Header.Get(bitbucketCloudRequestIDHeader) sig := r.Header.Get(bitbucketSignatureHeader) - defer r.Body.Close() // nolint: errcheck + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + e.Logger.Err("failed to close request body: %v", closeErr) + } + }() body, err := io.ReadAll(r.Body) if err != nil { e.respond(w, logging.Error, http.StatusBadRequest, "Unable to read body: %s %s=%s", err, bitbucketCloudRequestIDHeader, reqID) @@ -254,7 +262,11 @@ func (e *VCSEventsController) handleBitbucketServerPost(w http.ResponseWriter, r eventType := r.Header.Get(bitbucketEventTypeHeader) reqID := r.Header.Get(bitbucketServerRequestIDHeader) sig := r.Header.Get(bitbucketSignatureHeader) - defer r.Body.Close() // nolint: errcheck + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + e.Logger.Err("failed to close request body: %v", closeErr) + } + }() body, err := io.ReadAll(r.Body) if err != nil { e.respond(w, logging.Error, http.StatusBadRequest, "Unable to read body: %s %s=%s", err, bitbucketServerRequestIDHeader, reqID) @@ -318,7 +330,11 @@ func (e *VCSEventsController) handleGiteaPost(w http.ResponseWriter, r *http.Req eventType := r.Header.Get(giteaEventTypeHeader) reqID := r.Header.Get(giteaRequestIDHeader) - defer r.Body.Close() // Ensure the request body is closed + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + e.Logger.Err("failed to close request body: %v", closeErr) + } + }() body, err := io.ReadAll(r.Body) if err != nil { @@ -899,7 +915,9 @@ func (e *VCSEventsController) respond(w http.ResponseWriter, lvl logging.LogLeve response := fmt.Sprintf(format, args...) e.Logger.Log(lvl, response) w.WriteHeader(code) - fmt.Fprintln(w, response) + if _, err := fmt.Fprintln(w, response); err != nil { + e.Logger.Err("failed to write response: %v", err) + } } // commentNotAllowlisted comments on the pull request that the repo is not diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index 0bd900298..26e81599d 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -247,7 +247,11 @@ func TestPost_GitlabCommentNotAllowlisted(t *testing.T) { e.Post(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close response body: %v", closeErr) + } + }() Equals(t, http.StatusForbidden, resp.StatusCode) body, _ := io.ReadAll(resp.Body) exp := "Repo not allowlisted" @@ -282,7 +286,11 @@ func TestPost_GitlabCommentNotAllowlistedWithSilenceErrors(t *testing.T) { e.Post(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close response body: %v", closeErr) + } + }() Equals(t, http.StatusForbidden, resp.StatusCode) body, _ := io.ReadAll(resp.Body) exp := "Repo not allowlisted" @@ -316,7 +324,11 @@ func TestPost_GithubCommentNotAllowlisted(t *testing.T) { e.Post(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close response body: %v", closeErr) + } + }() Equals(t, http.StatusForbidden, resp.StatusCode) body, _ := io.ReadAll(resp.Body) exp := "Repo not allowlisted" @@ -352,7 +364,11 @@ func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) { e.Post(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close response body: %v", closeErr) + } + }() Equals(t, http.StatusForbidden, resp.StatusCode) body, _ := io.ReadAll(resp.Body) exp := "Repo not allowlisted" diff --git a/server/controllers/events/gitlab_request_parser_validator.go b/server/controllers/events/gitlab_request_parser_validator.go index 22d2e08e0..faaf9f8db 100644 --- a/server/controllers/events/gitlab_request_parser_validator.go +++ b/server/controllers/events/gitlab_request_parser_validator.go @@ -84,7 +84,7 @@ func (d *DefaultGitlabRequestParserValidator) ParseAndValidate(r *http.Request, // comment on a merge request or a commit. var subset struct { ObjectAttributes struct { - NoteableType string `json:"noteable_type"` + NotableType string `json:"noteable_type"` } `json:"object_attributes"` } if err := json.Unmarshal(bytes, &subset); err != nil { @@ -92,7 +92,7 @@ func (d *DefaultGitlabRequestParserValidator) ParseAndValidate(r *http.Request, } // We then parse into the correct comment event type. - switch subset.ObjectAttributes.NoteableType { + switch subset.ObjectAttributes.NotableType { case "Commit": var e gitlab.CommitCommentEvent err := json.Unmarshal(bytes, &e) diff --git a/server/controllers/events/gitlab_request_parser_validator_test.go b/server/controllers/events/gitlab_request_parser_validator_test.go index fb61c4ff9..15ce262bb 100644 --- a/server/controllers/events/gitlab_request_parser_validator_test.go +++ b/server/controllers/events/gitlab_request_parser_validator_test.go @@ -348,7 +348,7 @@ var mergeCommentEventJSON = `{ "object_attributes": { "id": 1244, "note": "This MR needs work.", - "noteable_type": "MergeRequest", + "noteable_type": "MergeRequest", // nolint: misspell "author_id": 1, "created_at": "2015-05-17", "updated_at": "2015-05-17", @@ -356,7 +356,7 @@ var mergeCommentEventJSON = `{ "attachment": null, "line_code": null, "commit_id": "", - "noteable_id": 7, + "noteable_id": 7, // nolint: misspell "system": false, "st_diff": null, "url": "http://example.com/gitlab-org/gitlab-test/merge_requests/1#note_1244" @@ -464,7 +464,7 @@ var commitCommentEventJSON = `{ "object_attributes": { "id": 1243, "note": "This is a commit comment. How does this work?", - "noteable_type": "Commit", + "noteable_type": "Commit", // nolint: misspell "author_id": 1, "created_at": "2015-05-17 18:08:09 UTC", "updated_at": "2015-05-17 18:08:09 UTC", @@ -472,7 +472,7 @@ var commitCommentEventJSON = `{ "attachment":null, "line_code": "bec9703f7a456cd2b4ab5fb3220ae016e3e394e3_0_1", "commit_id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660", - "noteable_id": null, + "noteable_id": null, // nolint: misspell "system": false, "st_diff": { "diff": "--- /dev/null\n+++ b/six\n@@ -0,0 +1 @@\n+Subproject commit 409f37c4f05865e4fb208c771485f211a22c4c2d\n", diff --git a/server/controllers/status_controller.go b/server/controllers/status_controller.go index 487ac9791..398047868 100644 --- a/server/controllers/status_controller.go +++ b/server/controllers/status_controller.go @@ -32,7 +32,9 @@ func (d *StatusController) Get(w http.ResponseWriter, _ *http.Request) { }, "", " ") if err != nil { w.WriteHeader(http.StatusInternalServerError) - fmt.Fprintf(w, "Error creating status json response: %s", err) + if _, writeErr := fmt.Fprintf(w, "Error creating status json response: %s", err); writeErr != nil { + d.Logger.Err("failed to write error response: %v", writeErr) + } return } w.Header().Set("Content-Type", "application/json") diff --git a/server/controllers/websocket/mux_test.go b/server/controllers/websocket/mux_test.go index baddfb219..868af7311 100644 --- a/server/controllers/websocket/mux_test.go +++ b/server/controllers/websocket/mux_test.go @@ -19,7 +19,11 @@ func wsHandler(t *testing.T, checkOrigin bool) http.HandlerFunc { t.Log("upgrade:", err) return } - defer c.Close() + defer func() { + if closeErr := c.Close(); closeErr != nil { + t.Errorf("failed to close websocket connection: %v", closeErr) + } + }() } } @@ -50,7 +54,11 @@ func TestCheckOriginFunc(t *testing.T) { } c, _, err := websocket.DefaultDialer.Dial(u.String(), header) if err == nil { - defer c.Close() + defer func() { + if closeErr := c.Close(); closeErr != nil { + t.Errorf("failed to close websocket connection: %v", closeErr) + } + }() } if (err != nil) != tt.wantErr { t.Errorf("websocket dial error = %v, wantErr %v", err, tt.wantErr) diff --git a/server/core/config/parser_validator_test.go b/server/core/config/parser_validator_test.go index eef1503dc..2ef859391 100644 --- a/server/core/config/parser_validator_test.go +++ b/server/core/config/parser_validator_test.go @@ -41,7 +41,7 @@ func TestHasRepoCfg_FileDoesNotExist(t *testing.T) { func TestHasRepoCfg_InvalidFileExtension(t *testing.T) { tmpDir := t.TempDir() repoConfigFile := "atlantis.yml" - _, err := os.Create(filepath.Join(tmpDir, repoConfigFile)) + _, err := os.Create(filepath.Join(tmpDir, "atlantis.yml")) Ok(t, err) r := config.ParserValidator{} diff --git a/server/core/db/boltdb_test.go b/server/core/db/boltdb_test.go index 13495ff5e..5c7cbf352 100644 --- a/server/core/db/boltdb_test.go +++ b/server/core/db/boltdb_test.go @@ -489,7 +489,9 @@ func TestPullStatus_UpdateGet(t *testing.T) { Status: models.ErroredPlanStatus, }, }, status.Projects) - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // Test we can create a status, delete it, and then we shouldn't be able to getCommandLock @@ -534,7 +536,9 @@ func TestPullStatus_UpdateDeleteGet(t *testing.T) { maybeStatus, err := b.GetPullStatus(pull) Ok(t, err) Assert(t, maybeStatus == nil, "exp nil") - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // Test we can create a status, update a specific project's status within that @@ -599,7 +603,9 @@ func TestPullStatus_UpdateProject(t *testing.T) { Status: models.AppliedPlanStatus, }, }, status.Projects) // nolint: staticcheck - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // Test that if we update an existing pull status and our new status is for a @@ -662,7 +668,9 @@ func TestPullStatus_UpdateNewCommit(t *testing.T) { Status: models.AppliedPlanStatus, }, }, maybeStatus.Projects) - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // Test that if we update an existing pull status via Apply and our new status is for a @@ -775,7 +783,9 @@ func TestPullStatus_UpdateMerge_Apply(t *testing.T) { }, }, updateStatus.Projects) } - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // Test that if we update one existing policy status via approve_policies and our new status is for a @@ -890,7 +900,9 @@ func TestPullStatus_UpdateMerge_ApprovePolicies(t *testing.T) { }, }, updateStatus.Projects) } - b.Close() + if closeErr := b.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } } // newTestDB returns a TestDB using a temporary path. diff --git a/server/core/locking/locking_test.go b/server/core/locking/locking_test.go index 68d79d858..b97b0ccac 100644 --- a/server/core/locking/locking_test.go +++ b/server/core/locking/locking_test.go @@ -20,12 +20,12 @@ import ( "strings" - . "github.com/petergtz/pegomock/v4" + . "github.com/petergtz/pegomock/v4" // nolint: revive "github.com/runatlantis/atlantis/server/core/locking" "github.com/runatlantis/atlantis/server/core/locking/mocks" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" - . "github.com/runatlantis/atlantis/testing" + . "github.com/runatlantis/atlantis/testing" // nolint: revive ) var project = models.NewProject("owner/repo", "path", "") diff --git a/server/core/redis/redis_test.go b/server/core/redis/redis_test.go index 61013ae08..b00d1f9a9 100644 --- a/server/core/redis/redis_test.go +++ b/server/core/redis/redis_test.go @@ -63,7 +63,11 @@ func TestRedisWithTLS(t *testing.T) { caPath = certFile.Name() _, err = certFile.Write(certData) Ok(t, err) - defer certFile.Close() + defer func() { + if closeErr := certFile.Close(); closeErr != nil { + t.Errorf("failed to close cert file: %v", closeErr) + } + }() tlsConfig := &tls.Config{ Certificates: []tls.Certificate{cert}, InsecureSkipVerify: true, //nolint:gosec // This is purely for testing diff --git a/server/core/runtime/cache/version_path.go b/server/core/runtime/cache/version_path.go index 197d5d990..d3e9f4372 100644 --- a/server/core/runtime/cache/version_path.go +++ b/server/core/runtime/cache/version_path.go @@ -1,3 +1,4 @@ +// Package cache provides caching functionality for execution versions. package cache import ( diff --git a/server/core/runtime/cache/version_path_test.go b/server/core/runtime/cache/version_path_test.go index b7c08d1dd..855ae5aa3 100644 --- a/server/core/runtime/cache/version_path_test.go +++ b/server/core/runtime/cache/version_path_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/hashicorp/go-version" - . "github.com/petergtz/pegomock/v4" + . "github.com/petergtz/pegomock/v4" // nolint: revive cache_mocks "github.com/runatlantis/atlantis/server/core/runtime/cache/mocks" "github.com/runatlantis/atlantis/server/core/runtime/models" models_mocks "github.com/runatlantis/atlantis/server/core/runtime/models/mocks" diff --git a/server/core/runtime/common/common.go b/server/core/runtime/common/common.go index cc11f3076..749b55ec4 100644 --- a/server/core/runtime/common/common.go +++ b/server/core/runtime/common/common.go @@ -1,3 +1,4 @@ +// Package common provides common utilities for runtime operations. package common import ( diff --git a/server/core/runtime/models/exec.go b/server/core/runtime/models/exec.go index 4a7df796f..1cb992888 100644 --- a/server/core/runtime/models/exec.go +++ b/server/core/runtime/models/exec.go @@ -38,6 +38,7 @@ func (e LocalExec) CombinedOutput(args []string, envs map[string]string, workdir // honestly not entirely sure why we're using sh -c but it's used // for the terraform binary so copying it for now + // nolint: gosec cmd := exec.Command("sh", "-c", formattedArgs) cmd.Env = envVars cmd.Dir = workdir diff --git a/server/core/terraform/tfclient/terraform_client.go b/server/core/terraform/tfclient/terraform_client.go index 5ef864db7..b6bdf4cbf 100644 --- a/server/core/terraform/tfclient/terraform_client.go +++ b/server/core/terraform/tfclient/terraform_client.go @@ -398,6 +398,7 @@ func (c *DefaultClient) prepExecCmd(log logging.SimpleLogging, d terraform.Distr if err != nil { return "", nil, err } + // nolint: gosec cmd := exec.Command("sh", "-c", tfCmd) cmd.Dir = path cmd.Env = envVars diff --git a/server/core/terraform/tfclient/terraform_client_internal_test.go b/server/core/terraform/tfclient/terraform_client_internal_test.go index 9cde70e39..5bcbb001a 100644 --- a/server/core/terraform/tfclient/terraform_client_internal_test.go +++ b/server/core/terraform/tfclient/terraform_client_internal_test.go @@ -30,7 +30,7 @@ func TestGenerateRCFile_WritesFile(t *testing.T) { expContents := `credentials "hostname" { token = "token" }` - actContents, err := os.ReadFile(filepath.Join(tmp, ".terraformrc")) + actContents, err := os.ReadFile(filepath.Join(tmp, ".terraformrc")) // nolint: gosec Ok(t, err) Equals(t, expContents, string(actContents)) } @@ -259,7 +259,7 @@ func TestDefaultClient_RunCommandAsync_BigOutput(t *testing.T) { projectCmdOutputHandler: projectCmdOutputHandler, } filename := filepath.Join(tmp, "data") - f, err := os.OpenFile(filename, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + f, err := os.OpenFile(filename, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) // nolint: gosec Ok(t, err) var exp string diff --git a/server/core/terraform/tfclient/terraform_client_test.go b/server/core/terraform/tfclient/terraform_client_test.go index 50afd698a..69199a6c9 100644 --- a/server/core/terraform/tfclient/terraform_client_test.go +++ b/server/core/terraform/tfclient/terraform_client_test.go @@ -389,7 +389,11 @@ func TestEnsureVersion_downloaded_downloadingDisabled(t *testing.T) { func tempSetEnv(t *testing.T, key string, value string) func() { orig := os.Getenv(key) Ok(t, os.Setenv(key, value)) - return func() { os.Setenv(key, orig) } + return func() { + if setErr := os.Setenv(key, orig); setErr != nil { + t.Errorf("failed to restore environment variable %s: %v", key, setErr) + } + } } // returns parent, bindir, cachedir diff --git a/server/events/apply_command_runner_test.go b/server/events/apply_command_runner_test.go index d822c58eb..66535baf0 100644 --- a/server/events/apply_command_runner_test.go +++ b/server/events/apply_command_runner_test.go @@ -142,7 +142,9 @@ func TestApplyCommandRunner_IsSilenced(t *testing.T) { tmp := t.TempDir() db, err := db.New(tmp) t.Cleanup(func() { - db.Close() + if closeErr := db.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 9f05a5517..675e6a9e3 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -86,9 +86,11 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock // create an empty DB tmp := t.TempDir() - defaultBoltDB, err := db.New(tmp) + boltDB, err := db.New(tmp) t.Cleanup(func() { - defaultBoltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) @@ -97,7 +99,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock SilenceNoProjects: false, StatusName: "atlantis-test", discardApprovalOnPlan: false, - backend: defaultBoltDB, + backend: boltDB, DisableUnlockLabel: "do-not-unlock", } @@ -778,7 +780,9 @@ func TestRunAutoplanCommand_DeletePlans(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -808,7 +812,9 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Fal tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -839,7 +845,9 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Tru tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -866,7 +874,9 @@ func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Fals tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -890,7 +900,9 @@ func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_True tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -911,7 +923,9 @@ func TestRunGenericPlanCommand_DeletePlans(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -956,7 +970,9 @@ func TestRunSpecificPlanCommandDoesnt_DeletePlans(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -979,7 +995,9 @@ func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -1034,7 +1052,9 @@ func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -1061,7 +1081,9 @@ func TestFailedApprovalCreatesFailedStatusUpdate(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -1110,7 +1132,9 @@ func TestApprovedPoliciesUpdateFailedPolicyStatus(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -1169,7 +1193,9 @@ func TestApplyMergeablityWhenPolicyCheckFails(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB @@ -1250,7 +1276,9 @@ func TestRunApply_DiscardedProjects(t *testing.T) { tmp := t.TempDir() boltDB, err := db.New(tmp) t.Cleanup(func() { - boltDB.Close() + if closeErr := boltDB.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dbUpdater.Backend = boltDB diff --git a/server/events/delete_lock_command_test.go b/server/events/delete_lock_command_test.go index 3efb43b47..18672b678 100644 --- a/server/events/delete_lock_command_test.go +++ b/server/events/delete_lock_command_test.go @@ -61,7 +61,9 @@ func TestDeleteLock_Success(t *testing.T) { tmp := t.TempDir() db, err := db.New(tmp) t.Cleanup(func() { - db.Close() + if closeErr := db.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) dlc := events.DefaultDeleteLockCommand{ diff --git a/server/events/plan_command_runner_test.go b/server/events/plan_command_runner_test.go index 4bc9d4eb2..5ca5fdd4f 100644 --- a/server/events/plan_command_runner_test.go +++ b/server/events/plan_command_runner_test.go @@ -80,7 +80,9 @@ func TestPlanCommandRunner_IsSilenced(t *testing.T) { tmp := t.TempDir() db, err := db.New(tmp) t.Cleanup(func() { - db.Close() + if closeErr := db.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) @@ -510,7 +512,9 @@ func TestPlanCommandRunner_ExecutionOrder(t *testing.T) { tmp := t.TempDir() db, err := db.New(tmp) t.Cleanup(func() { - db.Close() + if closeErr := db.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) @@ -752,7 +756,9 @@ func TestPlanCommandRunner_AtlantisApplyStatus(t *testing.T) { tmp := t.TempDir() db, err := db.New(tmp) t.Cleanup(func() { - db.Close() + if closeErr := db.Close(); closeErr != nil { + t.Errorf("failed to close database: %v", closeErr) + } }) Ok(t, err) diff --git a/server/events/vcs/bitbucketserver/client_test.go b/server/events/vcs/bitbucketserver/client_test.go index 4827b76ed..93830b5dc 100644 --- a/server/events/vcs/bitbucketserver/client_test.go +++ b/server/events/vcs/bitbucketserver/client_test.go @@ -209,7 +209,11 @@ func TestClient_MergePullDeleteSourceBranch(t *testing.T) { w.Write(pullRequest) // nolint: errcheck case "/rest/branch-utils/1.0/projects/ow/repos/repo/branches": Equals(t, "DELETE", r.Method) - defer r.Body.Close() + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + t.Errorf("failed to close request body: %v", closeErr) + } + }() b, err := io.ReadAll(r.Body) Ok(t, err) var payload bitbucketserver.DeleteSourceBranch diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 693183012..088b947d3 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -338,7 +338,11 @@ func TestGitlabClient_UpdateStatus(t *testing.T) { Equals(t, updateStatusDescription, updateStatusJsonBody.Description) Equals(t, gitlabPipelineSuccessMrID, updateStatusJsonBody.PipelineId) - defer r.Body.Close() // nolint: errcheck + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + t.Errorf("failed to close request body: %v", closeErr) + } + }() // nolint: errcheck setStatusJsonResponse, err := json.Marshal(EmptyStruct{}) Ok(t, err) @@ -460,7 +464,11 @@ func TestGitlabClient_UpdateStatusGetCommitRetryable(t *testing.T) { Equals(t, gitlabPipelineSuccessMrID, updateStatusJsonBody.PipelineId) } - defer r.Body.Close() + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + t.Errorf("failed to close request body: %v", closeErr) + } + }() // nolint: errcheck getCommitJsonResponse, err := json.Marshal(EmptyStruct{}) Ok(t, err) @@ -592,7 +600,11 @@ func TestGitlabClient_UpdateStatusSetCommitStatusConflictRetryable(t *testing.T) Equals(t, updateStatusTargetUrl, updateStatusJsonBody.TargetUrl) Equals(t, updateStatusDescription, updateStatusJsonBody.Description) - defer r.Body.Close() // nolint: errcheck + defer func() { + if closeErr := r.Body.Close(); closeErr != nil { + t.Errorf("failed to close request body: %v", closeErr) + } + }() // nolint: errcheck if shouldSendConflict { w.WriteHeader(http.StatusConflict) @@ -1013,8 +1025,7 @@ func TestGitlabClient_HideOldComments(t *testing.T) { t.Errorf("got unexpected method at %q", r.Method) http.Error(w, "not found", http.StatusNotFound) } - }), - ) + })) internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) Ok(t, err) diff --git a/server/events/webhooks/http.go b/server/events/webhooks/http.go index 6f540ac15..5efc9af87 100644 --- a/server/events/webhooks/http.go +++ b/server/events/webhooks/http.go @@ -50,7 +50,12 @@ func (h *HttpWebhook) doSend(applyResult ApplyResult) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + // Log the error but don't return it since we're in a defer + // and the function is already returning an error + } + }() if resp.StatusCode != http.StatusOK { respBody, _ := io.ReadAll(resp.Body) return fmt.Errorf("returned status code %d with response %q", resp.StatusCode, respBody) diff --git a/server/server.go b/server/server.go index 1d372eeea..8661d3b03 100644 --- a/server/server.go +++ b/server/server.go @@ -1091,7 +1091,12 @@ func (s *Server) Start() error { }, NewRequestLogger(s)) n.UseHandler(s.Router) - defer s.Logger.Flush() + defer func() { + if flushErr := s.Logger.Flush(); flushErr != nil { + // Log the error but don't return it since we're in a defer + // and the function is already returning an error + } + }() // Ensure server gracefully drains connections when stopped. stop := make(chan os.Signal, 1) @@ -1163,7 +1168,9 @@ func (s *Server) Index(w http.ResponseWriter, _ *http.Request) { locks, err := s.Locker.List() if err != nil { w.WriteHeader(http.StatusServiceUnavailable) - fmt.Fprintf(w, "Could not retrieve locks: %s", err) + if _, writeErr := fmt.Fprintf(w, "Could not retrieve locks: %s", err); writeErr != nil { + s.Logger.Err("failed to write error response: %v", writeErr) + } return } @@ -1188,7 +1195,9 @@ func (s *Server) Index(w http.ResponseWriter, _ *http.Request) { s.Logger.Debug("Apply Lock: %v", applyCmdLock) if err != nil { w.WriteHeader(http.StatusServiceUnavailable) - fmt.Fprintf(w, "Could not retrieve global apply lock: %s", err) + if _, writeErr := fmt.Fprintf(w, "Could not retrieve global apply lock: %s", err); writeErr != nil { + s.Logger.Err("failed to write error response: %v", writeErr) + } return } diff --git a/testing/temp_files.go b/testing/temp_files.go index 8ff0630b5..421520b0d 100644 --- a/testing/temp_files.go +++ b/testing/temp_files.go @@ -39,7 +39,7 @@ func dirStructureGo(t *testing.T, parentDir string, structure map[string]interfa for key, val := range structure { // If val is nil then key is a filename and we just create it if val == nil { - _, err := os.Create(filepath.Join(parentDir, key)) + _, err := os.Create(filepath.Join(parentDir, key)) // nolint: gosec Ok(t, err) continue }