From c2cad26ca980dcca932dc8308dd545d321eb6246 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 15 Dec 2020 09:32:06 +0100 Subject: [PATCH] Chore: Disable default golangci-lint filter (#29751) * Disable default golangci-lint filter Signed-off-by: Arve Knudsen * Chore: Fix linter warnings Signed-off-by: Arve Knudsen --- pkg/api/api.go | 4 ++ pkg/api/avatar/avatar.go | 18 +++++-- pkg/api/dashboard_snapshot.go | 10 +++- pkg/api/ldap_debug.go | 17 ++----- pkg/api/login_oauth.go | 4 +- pkg/api/pluginproxy/access_token_provider.go | 11 ++++- pkg/api/static/static.go | 14 ++++-- .../grafana-cli/commands/install_command.go | 14 +++++- .../commands/install_command_test.go | 2 +- pkg/cmd/grafana-cli/services/api_client.go | 22 +++++++-- .../grafana-cli/services/api_client_test.go | 47 ++++++++++++------- pkg/cmd/grafana-server/main.go | 10 +++- .../imguploader/azureblobuploader.go | 11 ++++- pkg/components/imguploader/webdavuploader.go | 11 ++++- pkg/infra/fs/exists_test.go | 6 ++- pkg/infra/log/file.go | 35 ++++++++++---- pkg/infra/log/file_test.go | 7 +-- pkg/infra/log/handlers.go | 4 +- pkg/infra/log/log.go | 30 +++++++++--- pkg/infra/log/syslog.go | 4 +- pkg/infra/log/syslog_windows.go | 3 +- pkg/infra/metrics/graphitebridge/graphite.go | 7 ++- .../metrics/graphitebridge/graphite_test.go | 37 ++++++--------- pkg/infra/tracing/tracing.go | 2 +- pkg/infra/tracing/tracing_test.go | 20 +++++--- pkg/infra/usagestats/service.go | 4 +- pkg/infra/usagestats/usage_stats.go | 17 +++++-- pkg/infra/usagestats/usage_stats_test.go | 10 ++-- pkg/login/social/azuread_oauth.go | 2 +- pkg/login/social/common.go | 14 ++++-- pkg/login/social/generic_oauth.go | 14 ++++-- pkg/login/social/github_oauth.go | 18 +++---- pkg/login/social/gitlab_oauth.go | 6 +-- pkg/login/social/google_oauth.go | 2 +- pkg/login/social/grafana_com_oauth.go | 2 +- pkg/login/social/okta_oauth.go | 4 +- pkg/login/social/social.go | 2 +- pkg/middleware/dashboard_redirect_test.go | 18 +++++-- pkg/middleware/middleware_test.go | 6 ++- pkg/models/datasource_cache_test.go | 10 ++-- .../backendplugin/grpcplugin/client.go | 9 ++-- .../backendplugin/grpcplugin/grpc_plugin.go | 4 +- pkg/plugins/backendplugin/manager.go | 16 +++++-- pkg/plugins/dashboards.go | 2 +- pkg/plugins/plugins.go | 6 +-- pkg/plugins/update_checker.go | 15 ++++-- pkg/server/server.go | 6 ++- pkg/services/alerting/alerting_usage_test.go | 5 +- pkg/services/alerting/notifiers/pushover.go | 4 +- pkg/services/alerting/notifiers/telegram.go | 9 +++- .../contexthandler/auth_proxy_test.go | 4 +- .../contexthandler/authproxy/authproxy.go | 40 ++++++++++------ .../authproxy/authproxy_test.go | 18 ++++--- .../contexthandler/contexthandler_test.go | 6 ++- pkg/services/ldap/settings_test.go | 6 ++- pkg/services/notifications/webhook.go | 7 ++- .../provisioning/values/values_test.go | 24 ++++++---- pkg/services/rendering/http_mode.go | 6 ++- pkg/setting/dynamic_settings_test.go | 8 +++- pkg/setting/expanders_test.go | 9 ++-- pkg/setting/setting_test.go | 35 +++++++++----- .../applicationinsights-datasource.go | 6 ++- .../applicationinsights-metrics_test.go | 24 ++++++---- .../azure-log-analytics-datasource.go | 7 ++- .../azure-log-analytics-table-frame_test.go | 25 ++++++---- .../azuremonitor/azuremonitor-datasource.go | 6 ++- .../azuremonitor-datasource_test.go | 20 ++++---- .../insights-analytics-datasource.go | 6 ++- pkg/tsdb/cloudmonitoring/cloudmonitoring.go | 6 ++- .../cloudmonitoring/cloudmonitoring_test.go | 2 + pkg/tsdb/elasticsearch/client/client.go | 6 ++- pkg/tsdb/graphite/graphite.go | 6 ++- pkg/tsdb/influxdb/influxdb.go | 9 ++-- pkg/tsdb/opentsdb/opentsdb.go | 6 ++- pkg/tsdb/sqleng/sql_engine.go | 7 ++- scripts/go/configs/.golangci.toml | 18 +++++++ 76 files changed, 598 insertions(+), 274 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index cb8a85af58b..9049f70cc6d 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -1,3 +1,4 @@ +// Package api contains API logic. package api import ( @@ -7,10 +8,13 @@ import ( "github.com/grafana/grafana/pkg/api/avatar" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/models" ) +var plog = log.New("api") + // registerRoutes registers all API HTTP routes. func (hs *HTTPServer) registerRoutes() { reqSignedIn := middleware.ReqSignedIn diff --git a/pkg/api/avatar/avatar.go b/pkg/api/avatar/avatar.go index 616ac79abfd..b633a2f9100 100644 --- a/pkg/api/avatar/avatar.go +++ b/pkg/api/avatar/avatar.go @@ -136,7 +136,9 @@ func newNotFound() *Avatar { // variable. // nolint:gosec path := filepath.Join(setting.StaticRootPath, "img", "user_profile.png") - + // It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration + // variable. + // nolint:gosec if data, err := ioutil.ReadFile(path); err != nil { log.Errorf(3, "Failed to read user_profile.png, %v", path) } else { @@ -212,7 +214,10 @@ func (a *thunderTask) fetch() error { a.Avatar.timestamp = time.Now() log.Debugf("avatar.fetch(fetch new avatar): %s", a.Url) - req, _ := http.NewRequest("GET", a.Url, nil) + req, err := http.NewRequest("GET", a.Url, nil) + if err != nil { + return err + } req.Header.Set("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/jpeg,image/png,*/*;q=0.8") req.Header.Set("Accept-Encoding", "deflate,sdch") req.Header.Set("Accept-Language", "zh-CN,zh;q=0.8") @@ -221,10 +226,13 @@ func (a *thunderTask) fetch() error { resp, err := client.Do(req) if err != nil { a.Avatar.notFound = true - return fmt.Errorf("gravatar unreachable, %v", err) + return fmt.Errorf("gravatar unreachable: %w", err) } - - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + log.Warn("Failed to close response body", "err", err) + } + }() if resp.StatusCode != 200 { a.Avatar.notFound = true diff --git a/pkg/api/dashboard_snapshot.go b/pkg/api/dashboard_snapshot.go index 2aa18b44c25..2af175a1f0b 100644 --- a/pkg/api/dashboard_snapshot.go +++ b/pkg/api/dashboard_snapshot.go @@ -54,7 +54,11 @@ func createExternalDashboardSnapshot(cmd models.CreateDashboardSnapshotCommand) if err != nil { return nil, err } - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + plog.Warn("Failed to close response body", "err", err) + } + }() if response.StatusCode != 200 { return nil, fmt.Errorf("create external snapshot response status code %d", response.StatusCode) @@ -178,7 +182,9 @@ func deleteExternalDashboardSnapshot(externalUrl string) error { if err != nil { return err } - defer response.Body.Close() + if err := response.Body.Close(); err != nil { + plog.Warn("Failed closing response body", "err", err) + } if response.StatusCode == 200 { return nil diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index e9d6f5bfcfa..361f6d8ce53 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -19,7 +19,7 @@ var ( getLDAPConfig = multildap.GetConfig newLDAP = multildap.New - logger = log.New("LDAP.debug") + ldapLogger = log.New("LDAP.debug") errOrganizationNotFound = func(orgId int64) error { return fmt.Errorf("unable to find organization with ID '%d'", orgId) @@ -117,7 +117,6 @@ func (hs *HTTPServer) GetLDAPStatus(c *models.ReqContext) Response { } ldapConfig, err := getLDAPConfig(hs.Cfg) - if err != nil { return Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again", err) } @@ -129,7 +128,6 @@ func (hs *HTTPServer) GetLDAPStatus(c *models.ReqContext) Response { } statuses, err := ldap.Ping() - if err != nil { return Error(http.StatusBadRequest, "Failed to connect to the LDAP server(s)", err) } @@ -187,12 +185,11 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) Response { ldapServer := newLDAP(ldapConfig.Servers) user, _, err := ldapServer.User(query.Result.Login) - if err != nil { if errors.Is(err, multildap.ErrDidNotFindUser) { // User was not in the LDAP server - we need to take action: if setting.AdminUser == query.Result.Login { // User is *the* Grafana Admin. We cannot disable it. errMsg := fmt.Sprintf(`Refusing to sync grafana super admin "%s" - it would be disabled`, query.Result.Login) - logger.Error(errMsg) + ldapLogger.Error(errMsg) return Error(http.StatusBadRequest, errMsg, err) } @@ -210,7 +207,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) Response { return Error(http.StatusBadRequest, "User not found in LDAP. Disabled the user without updating information", nil) // should this be a success? } - logger.Debug("Failed to sync the user with LDAP", "err", err) + ldapLogger.Debug("Failed to sync the user with LDAP", "err", err) return Error(http.StatusBadRequest, "Something went wrong while finding the user in LDAP", err) } @@ -221,7 +218,6 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) Response { } err = bus.Dispatch(upsertCmd) - if err != nil { return Error(http.StatusInternalServerError, "Failed to update the user", err) } @@ -236,7 +232,6 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { } ldapConfig, err := getLDAPConfig(hs.Cfg) - if err != nil { return Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration", err) } @@ -255,7 +250,7 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { return Error(http.StatusNotFound, "No user was found in the LDAP server(s) with that username", err) } - logger.Debug("user found", "user", user) + ldapLogger.Debug("user found", "user", user) name, surname := splitName(user.Name) @@ -304,16 +299,14 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { u.OrgRoles = orgRoles - logger.Debug("mapping org roles", "orgsRoles", u.OrgRoles) + ldapLogger.Debug("mapping org roles", "orgsRoles", u.OrgRoles) err = u.FetchOrgs() - if err != nil { return Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err) } cmd := &models.GetTeamsForLDAPGroupCommand{Groups: user.Groups} err = bus.Dispatch(cmd) - if err != nil && !errors.Is(err, bus.ErrHandlerNotFound) { return Error(http.StatusBadRequest, "Unable to find the teams for this user", err) } diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 64a16c2c386..fe4b425dc5a 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -224,11 +224,11 @@ func buildExternalUserInfo(token *oauth2.Token, userInfo *social.BasicUserInfo, var orgID int64 if setting.AutoAssignOrg && setting.AutoAssignOrgId > 0 { orgID = int64(setting.AutoAssignOrgId) - logger.Debug("The user has a role assignment and organization membership is auto-assigned", + plog.Debug("The user has a role assignment and organization membership is auto-assigned", "role", userInfo.Role, "orgId", orgID) } else { orgID = int64(1) - logger.Debug("The user has a role assignment and organization membership is not auto-assigned", + plog.Debug("The user has a role assignment and organization membership is not auto-assigned", "role", userInfo.Role, "orgId", orgID) } extUser.OrgRoles[orgID] = rt diff --git a/pkg/api/pluginproxy/access_token_provider.go b/pkg/api/pluginproxy/access_token_provider.go index e91d291f598..1c12c4520a8 100644 --- a/pkg/api/pluginproxy/access_token_provider.go +++ b/pkg/api/pluginproxy/access_token_provider.go @@ -113,7 +113,10 @@ func (provider *accessTokenProvider) getAccessToken(data templateData) (string, params.Add(key, interpolatedParam) } - getTokenReq, _ := http.NewRequest("POST", urlInterpolated, bytes.NewBufferString(params.Encode())) + getTokenReq, err := http.NewRequest("POST", urlInterpolated, bytes.NewBufferString(params.Encode())) + if err != nil { + return "", err + } getTokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") getTokenReq.Header.Set("Content-Length", strconv.Itoa(len(params.Encode()))) @@ -122,7 +125,11 @@ func (provider *accessTokenProvider) getAccessToken(data templateData) (string, return "", err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + logger.Warn("Failed to close response body", "err", err) + } + }() var token jwtToken if err := json.NewDecoder(resp.Body).Decode(&token); err != nil { diff --git a/pkg/api/static/static.go b/pkg/api/static/static.go index 0971a93c5a8..96c4e23aacf 100644 --- a/pkg/api/static/static.go +++ b/pkg/api/static/static.go @@ -134,7 +134,11 @@ func staticHandler(ctx *macaron.Context, log *log.Logger, opt StaticOptions) boo if err != nil { return false } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("Failed to close file: %s\n", err) + } + }() fi, err := f.Stat() if err != nil { @@ -154,7 +158,11 @@ func staticHandler(ctx *macaron.Context, log *log.Logger, opt StaticOptions) boo if err != nil { return false // Discard error. } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("Failed to close file: %s", err) + } + }() fi, err = f.Stat() if err != nil || fi.IsDir() { @@ -163,7 +171,7 @@ func staticHandler(ctx *macaron.Context, log *log.Logger, opt StaticOptions) boo } if !opt.SkipLogging { - log.Println("[Static] Serving " + file) + log.Printf("[Static] Serving %s\n", file) } // Add an Expires header to the static content diff --git a/pkg/cmd/grafana-cli/commands/install_command.go b/pkg/cmd/grafana-cli/commands/install_command.go index 311b686165e..f859b12e606 100644 --- a/pkg/cmd/grafana-cli/commands/install_command.go +++ b/pkg/cmd/grafana-cli/commands/install_command.go @@ -111,11 +111,17 @@ func InstallPlugin(pluginName, version string, c utils.CommandLine, client utils if err != nil { return errutil.Wrap("failed to create temporary file", err) } - defer os.Remove(tmpFile.Name()) + defer func() { + if err := os.Remove(tmpFile.Name()); err != nil { + logger.Warn("Failed to remove temporary file", "file", tmpFile.Name(), "err", err) + } + }() err = client.DownloadFile(pluginName, tmpFile, downloadURL, checksum) if err != nil { - tmpFile.Close() + if err := tmpFile.Close(); err != nil { + logger.Warn("Failed to close file", "err", err) + } return errutil.Wrap("failed to download plugin archive", err) } err = tmpFile.Close() @@ -228,6 +234,8 @@ func extractFiles(archiveFile string, pluginName string, filePath string, allowS newFile := filepath.Join(filePath, newFileName) if zf.FileInfo().IsDir() { + // We can ignore gosec G304 here since it makes sense to give all users read access + // nolint:gosec if err := os.MkdirAll(newFile, 0755); err != nil { if os.IsPermission(err) { return fmt.Errorf(permissionsDeniedMessage, newFile) @@ -240,6 +248,8 @@ func extractFiles(archiveFile string, pluginName string, filePath string, allowS } // Create needed directories to extract file + // We can ignore gosec G304 here since it makes sense to give all users read access + // nolint:gosec if err := os.MkdirAll(filepath.Dir(newFile), 0755); err != nil { return errutil.Wrap("failed to create directory to extract plugin files", err) } diff --git a/pkg/cmd/grafana-cli/commands/install_command_test.go b/pkg/cmd/grafana-cli/commands/install_command_test.go index aa9a12e0cb0..41751847194 100644 --- a/pkg/cmd/grafana-cli/commands/install_command_test.go +++ b/pkg/cmd/grafana-cli/commands/install_command_test.go @@ -215,7 +215,7 @@ func setupFakePluginsDir(t *testing.T) (string, func()) { err := os.RemoveAll(dirname) require.Nil(t, err) - err = os.MkdirAll(dirname, 0774) + err = os.MkdirAll(dirname, 0750) require.Nil(t, err) return dirname, func() { diff --git a/pkg/cmd/grafana-cli/services/api_client.go b/pkg/cmd/grafana-cli/services/api_client.go index 1ac8cbf6458..beb2e7fe8bf 100644 --- a/pkg/cmd/grafana-cli/services/api_client.go +++ b/pkg/cmd/grafana-cli/services/api_client.go @@ -94,14 +94,20 @@ func (client *GrafanaComClient) DownloadFile(pluginName string, tmpFile *os.File if err != nil { return errutil.Wrap("Failed to send request", err) } - defer bodyReader.Close() + defer func() { + if err := bodyReader.Close(); err != nil { + logger.Warn("Failed to close body", "err", err) + } + }() w := bufio.NewWriter(tmpFile) h := md5.New() if _, err = io.Copy(w, io.TeeReader(bodyReader, h)); err != nil { return errutil.Wrap("Failed to compute MD5 checksum", err) } - w.Flush() + if err := w.Flush(); err != nil { + return fmt.Errorf("failed to write to %q: %w", tmpFile.Name(), err) + } if len(checksum) > 0 && checksum != fmt.Sprintf("%x", h.Sum(nil)) { return fmt.Errorf("expected MD5 checksum does not match the downloaded archive - please contact security@grafana.com") } @@ -131,7 +137,11 @@ func sendRequestGetBytes(client http.Client, repoUrl string, subPaths ...string) if err != nil { return []byte{}, err } - defer bodyReader.Close() + defer func() { + if err := bodyReader.Close(); err != nil { + logger.Warn("Failed to close stream", "err", err) + } + }() return ioutil.ReadAll(bodyReader) } @@ -182,7 +192,11 @@ func handleResponse(res *http.Response) (io.ReadCloser, error) { if res.StatusCode/100 == 4 { body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + logger.Warn("Failed to close response body", "err", err) + } + }() if err != nil || len(body) == 0 { return nil, &BadRequestError{Status: res.Status} } diff --git a/pkg/cmd/grafana-cli/services/api_client_test.go b/pkg/cmd/grafana-cli/services/api_client_test.go index 3ea9c584e09..5825e670799 100644 --- a/pkg/cmd/grafana-cli/services/api_client_test.go +++ b/pkg/cmd/grafana-cli/services/api_client_test.go @@ -14,8 +14,9 @@ import ( func TestHandleResponse(t *testing.T) { t.Run("Returns body if status == 200", func(t *testing.T) { - resp := makeResponse(200, "test") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 200, "test") bodyReader, err := handleResponse(resp) require.NoError(t, err) body, err := ioutil.ReadAll(bodyReader) @@ -24,54 +25,68 @@ func TestHandleResponse(t *testing.T) { }) t.Run("Returns ErrorNotFound if status == 404", func(t *testing.T) { - resp := makeResponse(404, "") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 404, "") _, err := handleResponse(resp) assert.Equal(t, ErrNotFoundError, err) }) t.Run("Returns message from body if status == 400", func(t *testing.T) { - resp := makeResponse(400, "{ \"message\": \"error_message\" }") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 400, "{ \"message\": \"error_message\" }") _, err := handleResponse(resp) require.Error(t, err) assert.Equal(t, "error_message", asBadRequestError(t, err).Message) }) t.Run("Returns body if status == 400 and no message key", func(t *testing.T) { - resp := makeResponse(400, "{ \"test\": \"test_message\"}") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 400, "{ \"test\": \"test_message\"}") _, err := handleResponse(resp) require.Error(t, err) assert.Equal(t, "{ \"test\": \"test_message\"}", asBadRequestError(t, err).Message) }) t.Run("Returns Bad request error if status == 400 and no body", func(t *testing.T) { - resp := makeResponse(400, "") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 400, "") _, err := handleResponse(resp) require.Error(t, err) _ = asBadRequestError(t, err) }) t.Run("Returns error with invalid status if status == 500", func(t *testing.T) { - resp := makeResponse(500, "") - defer resp.Body.Close() + // The body gets closed within makeResponse + // nolint:bodyclose + resp := makeResponse(t, 500, "") _, err := handleResponse(resp) require.Error(t, err) assert.Contains(t, err.Error(), "invalid status") }) } -func makeResponse(status int, body string) *http.Response { +func makeResponse(t *testing.T, status int, body string) *http.Response { + t.Helper() + return &http.Response{ StatusCode: status, - Body: makeBody(body), + Body: makeBody(t, body), } } -func makeBody(body string) io.ReadCloser { - return ioutil.NopCloser(bytes.NewReader([]byte(body))) +func makeBody(t *testing.T, body string) io.ReadCloser { + t.Helper() + + reader := ioutil.NopCloser(bytes.NewReader([]byte(body))) + t.Cleanup(func() { + err := reader.Close() + assert.NoError(t, err) + }) + return reader } func asBadRequestError(t *testing.T, err error) *BadRequestError { diff --git a/pkg/cmd/grafana-server/main.go b/pkg/cmd/grafana-server/main.go index 0ab0a6a7186..c2335516d8f 100644 --- a/pkg/cmd/grafana-server/main.go +++ b/pkg/cmd/grafana-server/main.go @@ -108,7 +108,11 @@ func main() { } func executeServer(configFile, homePath, pidFile, packaging string, traceDiagnostics *tracingDiagnostics) error { - defer log.Close() + defer func() { + if err := log.Close(); err != nil { + fmt.Fprintf(os.Stderr, "Failed to close log: %s\n", err) + } + }() if traceDiagnostics.enabled { fmt.Println("diagnostics: tracing enabled", "file", traceDiagnostics.file) @@ -183,7 +187,9 @@ func listenToSystemSignals(s *server.Server) { for { select { case <-sighupChan: - log.Reload() + if err := log.Reload(); err != nil { + fmt.Fprintf(os.Stderr, "Failed to reload loggers: %s\n", err) + } case sig := <-signalChan: s.Shutdown(fmt.Sprintf("System signal: %s", sig)) } diff --git a/pkg/components/imguploader/azureblobuploader.go b/pkg/components/imguploader/azureblobuploader.go index 80023b8c30c..736de276bfa 100644 --- a/pkg/components/imguploader/azureblobuploader.go +++ b/pkg/components/imguploader/azureblobuploader.go @@ -70,9 +70,17 @@ func (az *AzureBlobUploader) Upload(ctx context.Context, imageDiskPath string) ( if err != nil { return "", err } + defer func() { + if err := resp.Body.Close(); err != nil { + logger.Warn("Failed to close response body", "err", err) + } + }() if resp.StatusCode > 400 && resp.StatusCode < 600 { - body, _ := ioutil.ReadAll(io.LimitReader(resp.Body, 1<<20)) + body, err := ioutil.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return "", err + } aerr := &Error{ Code: resp.StatusCode, Status: resp.Status, @@ -80,7 +88,6 @@ func (az *AzureBlobUploader) Upload(ctx context.Context, imageDiskPath string) ( Header: resp.Header, } aerr.parseXML() - resp.Body.Close() return "", aerr } diff --git a/pkg/components/imguploader/webdavuploader.go b/pkg/components/imguploader/webdavuploader.go index 3fd9894a8cf..7e6207f95d1 100644 --- a/pkg/components/imguploader/webdavuploader.go +++ b/pkg/components/imguploader/webdavuploader.go @@ -78,10 +78,17 @@ func (u *WebdavUploader) Upload(ctx context.Context, imgToUpload string) (string if err != nil { return "", err } - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + logger.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode != http.StatusCreated { - body, _ := ioutil.ReadAll(res.Body) + body, err := ioutil.ReadAll(res.Body) + if err != nil { + return "", fmt.Errorf("failed to read response body: %w", err) + } return "", fmt.Errorf("failed to upload image, statuscode: %d, body: %s", res.StatusCode, body) } diff --git a/pkg/infra/fs/exists_test.go b/pkg/infra/fs/exists_test.go index a9996dd42dd..28b10431e5e 100644 --- a/pkg/infra/fs/exists_test.go +++ b/pkg/infra/fs/exists_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +19,10 @@ func TestExists_NonExistent(t *testing.T) { func TestExists_Existent(t *testing.T) { f, err := ioutil.TempFile("", "") require.NoError(t, err) - defer os.Remove(f.Name()) + t.Cleanup(func() { + err := os.Remove(f.Name()) + assert.NoError(t, err) + }) exists, err := Exists(f.Name()) require.NoError(t, err) diff --git a/pkg/infra/log/file.go b/pkg/infra/log/file.go index 59ccf52f063..745e737c961 100644 --- a/pkg/infra/log/file.go +++ b/pkg/infra/log/file.go @@ -55,11 +55,15 @@ func (l *MuxWriter) Write(b []byte) (int, error) { } // set os.File in writer. -func (l *MuxWriter) SetFd(fd *os.File) { +func (l *MuxWriter) setFD(fd *os.File) error { if l.fd != nil { - l.fd.Close() + if err := l.fd.Close(); err != nil { + return err + } } + l.fd = fd + return nil } // create a FileLogWriter returning as LoggerInterface. @@ -98,13 +102,17 @@ func (w *FileLogWriter) StartLogger() error { if err != nil { return err } - w.mw.SetFd(fd) + if err := w.mw.setFD(fd); err != nil { + return err + } + return w.initFd() } func (w *FileLogWriter) docheck(size int) { w.startLock.Lock() defer w.startLock.Unlock() + if w.Rotate && ((w.Maxlines > 0 && w.maxlinesCurlines >= w.Maxlines) || (w.Maxsize > 0 && w.maxsizeCursize >= w.Maxsize) || (w.Daily && time.Now().Day() != w.dailyOpendate)) { @@ -119,6 +127,9 @@ func (w *FileLogWriter) docheck(size int) { func (w *FileLogWriter) createLogFile() (*os.File, error) { // Open the log file + // We can ignore G304 here since we can't unconditionally lock these log files down to be readable only + // by the owner + // nolint:gosec return os.OpenFile(w.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) } @@ -187,7 +198,9 @@ func (w *FileLogWriter) DoRotate() error { defer w.mw.Unlock() fd := w.mw.fd - fd.Close() + if err := fd.Close(); err != nil { + return err + } // close fd before rename // Rename the file to its newfound home @@ -228,8 +241,8 @@ func (w *FileLogWriter) deleteOldLog() { } // destroy file logger, close file writer. -func (w *FileLogWriter) Close() { - w.mw.fd.Close() +func (w *FileLogWriter) Close() error { + return w.mw.fd.Close() } // flush file logger. @@ -242,18 +255,22 @@ func (w *FileLogWriter) Flush() { } // Reload file logger -func (w *FileLogWriter) Reload() { +func (w *FileLogWriter) Reload() error { // block Logger's io.Writer w.mw.Lock() defer w.mw.Unlock() // Close fd := w.mw.fd - fd.Close() + if err := fd.Close(); err != nil { + return err + } // Open again err := w.StartLogger() if err != nil { - fmt.Fprintf(os.Stderr, "Reload StartLogger: %s\n", err) + return err } + + return nil } diff --git a/pkg/infra/log/file_test.go b/pkg/infra/log/file_test.go index f3aeb2eb410..d8092c8f29b 100644 --- a/pkg/infra/log/file_test.go +++ b/pkg/infra/log/file_test.go @@ -23,9 +23,10 @@ func TestLogFile(t *testing.T) { require.NotNil(t, fileLogWrite) t.Cleanup(func() { - fileLogWrite.Close() - err := os.Remove(fileLogWrite.Filename) - require.NoError(t, err) + err := fileLogWrite.Close() + assert.NoError(t, err) + err = os.Remove(fileLogWrite.Filename) + assert.NoError(t, err) }) fileLogWrite.Filename = "grafana_test.log" diff --git a/pkg/infra/log/handlers.go b/pkg/infra/log/handlers.go index 804d8fcbd70..fb5b9ce3088 100644 --- a/pkg/infra/log/handlers.go +++ b/pkg/infra/log/handlers.go @@ -1,9 +1,9 @@ package log type DisposableHandler interface { - Close() + Close() error } type ReloadableHandler interface { - Reload() + Reload() error } diff --git a/pkg/infra/log/log.go b/pkg/infra/log/log.go index ccec1058e9f..188baf71a40 100644 --- a/pkg/infra/log/log.go +++ b/pkg/infra/log/log.go @@ -69,6 +69,10 @@ func Infof(format string, v ...interface{}) { Root.Info(message) } +func Warn(msg string, v ...interface{}) { + Root.Warn(msg, v...) +} + func Warnf(format string, v ...interface{}) { var message string if len(v) > 0 { @@ -90,21 +94,33 @@ func Errorf(skip int, format string, v ...interface{}) { func Fatalf(skip int, format string, v ...interface{}) { Root.Crit(fmt.Sprintf(format, v...)) - Close() + if err := Close(); err != nil { + fmt.Fprintf(os.Stderr, "Failed to close log: %s\n", err) + } os.Exit(1) } -func Close() { +func Close() error { + var err error for _, logger := range loggersToClose { - logger.Close() + if e := logger.Close(); e != nil && err == nil { + err = e + } } loggersToClose = make([]DisposableHandler, 0) + + return err } -func Reload() { +// Reload reloads all loggers. +func Reload() error { for _, logger := range loggersToReload { - logger.Reload() + if err := logger.Reload(); err != nil { + return err + } } + + return nil } var logLevels = map[string]log15.Lvl{ @@ -164,7 +180,9 @@ func getLogFormat(format string) log15.Format { } func ReadLoggingConfig(modes []string, logsPath string, cfg *ini.File) error { - Close() + if err := Close(); err != nil { + return err + } defaultLevelName, _ := getLogLevelFromConfig("log", "info", cfg) defaultFilters := getFilters(util.SplitString(cfg.Section("log").Key("filters").String())) diff --git a/pkg/infra/log/syslog.go b/pkg/infra/log/syslog.go index 2132690cae6..d5394b4c0cf 100644 --- a/pkg/infra/log/syslog.go +++ b/pkg/infra/log/syslog.go @@ -77,8 +77,8 @@ func (sw *SysLogHandler) Log(r *log15.Record) error { return err } -func (sw *SysLogHandler) Close() { - sw.syslog.Close() +func (sw *SysLogHandler) Close() error { + return sw.syslog.Close() } var facilities = map[string]syslog.Priority{ diff --git a/pkg/infra/log/syslog_windows.go b/pkg/infra/log/syslog_windows.go index 9361d6c5fa5..e795c6b1447 100644 --- a/pkg/infra/log/syslog_windows.go +++ b/pkg/infra/log/syslog_windows.go @@ -18,5 +18,6 @@ func (sw *SysLogHandler) Log(r *log15.Record) error { return nil } -func (sw *SysLogHandler) Close() { +func (sw *SysLogHandler) Close() error { + return nil } diff --git a/pkg/infra/metrics/graphitebridge/graphite.go b/pkg/infra/metrics/graphitebridge/graphite.go index 1504ef3e146..df63f3cfb11 100644 --- a/pkg/infra/metrics/graphitebridge/graphite.go +++ b/pkg/infra/metrics/graphitebridge/graphite.go @@ -27,6 +27,7 @@ import ( "strings" "time" + "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" @@ -201,7 +202,11 @@ func (b *Bridge) Push() error { if err != nil { return err } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + logger.Warn("Failed to close connection", "err", err) + } + }() return b.writeMetrics(conn, mfs, b.prefix, model.Now()) } diff --git a/pkg/infra/metrics/graphitebridge/graphite_test.go b/pkg/infra/metrics/graphitebridge/graphite_test.go index dd4af83ef6c..2553f25af16 100644 --- a/pkg/infra/metrics/graphitebridge/graphite_test.go +++ b/pkg/infra/metrics/graphitebridge/graphite_test.go @@ -12,14 +12,16 @@ import ( "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/model" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestCountersAsDelta(t *testing.T) { - b, _ := NewBridge(&Config{ + b, err := NewBridge(&Config{ URL: "localhost:12345", CountersAsDelta: true, }) + require.NoError(t, err) ty := dto.MetricType(0) mf := &dto.MetricFamily{ Type: &ty, @@ -442,29 +444,21 @@ func TestSkipNanValues(t *testing.T) { Gatherer: reg, CountersAsDelta: true, }) - if err != nil { - t.Fatalf("error creating bridge: %v", err) - } + require.NoError(t, err) // first collect mfs, err := reg.Gather() - if err != nil { - t.Fatalf("error: %v", err) - } + require.NoError(t, err) var buf bytes.Buffer err = b.writeMetrics(&buf, mfs, "prefix.", model.Time(1477043083)) - if err != nil { - t.Fatalf("error: %v", err) - } + require.NoError(t, err) want := `prefix.http_request_total_sum.constname.constvalue 0 1477043 prefix.http_request_total_count.constname.constvalue.count 0 1477043 ` - if got := buf.String(); want != got { - t.Fatalf("wanted \n%s\n, got \n%s\n", want, got) - } + assert.Equal(t, want, buf.String()) } func TestPush(t *testing.T) { @@ -489,20 +483,17 @@ func TestPush(t *testing.T) { Gatherer: reg, Prefix: "prefix.", }) - if err != nil { - t.Fatalf("error creating bridge: %v", err) - } + require.NoError(t, err) nmg, err := newMockGraphite(port) - if err != nil { - t.Fatalf("error creating mock graphite: %v", err) - } - defer nmg.Close() + require.NoError(t, err) + t.Cleanup(func() { + err := nmg.Close() + require.NoError(t, err) + }) err = b.Push() - if err != nil { - t.Fatalf("error pushing: %v", err) - } + require.NoError(t, err) wants := []string{ "prefix.name.constname.constvalue.labelname.val1.count 1", diff --git a/pkg/infra/tracing/tracing.go b/pkg/infra/tracing/tracing.go index 249234d17d2..538a96d9580 100644 --- a/pkg/infra/tracing/tracing.go +++ b/pkg/infra/tracing/tracing.go @@ -130,7 +130,7 @@ func (ts *TracingService) Run(ctx context.Context) error { if ts.closer != nil { ts.log.Info("Closing tracing") - ts.closer.Close() + return ts.closer.Close() } return nil diff --git a/pkg/infra/tracing/tracing_test.go b/pkg/infra/tracing/tracing_test.go index ab9d2901f38..2c073c13cae 100644 --- a/pkg/infra/tracing/tracing_test.go +++ b/pkg/infra/tracing/tracing_test.go @@ -58,9 +58,11 @@ func TestInitJaegerCfg_Enabled(t *testing.T) { } func TestInitJaegerCfg_DisabledViaEnv(t *testing.T) { - os.Setenv("JAEGER_DISABLED", "true") + err := os.Setenv("JAEGER_DISABLED", "true") + require.NoError(t, err) defer func() { - os.Unsetenv("JAEGER_DISABLED") + err := os.Unsetenv("JAEGER_DISABLED") + require.NoError(t, err) }() ts := &TracingService{enabled: true} @@ -71,9 +73,11 @@ func TestInitJaegerCfg_DisabledViaEnv(t *testing.T) { } func TestInitJaegerCfg_EnabledViaEnv(t *testing.T) { - os.Setenv("JAEGER_DISABLED", "false") + err := os.Setenv("JAEGER_DISABLED", "false") + require.NoError(t, err) defer func() { - os.Unsetenv("JAEGER_DISABLED") + err := os.Unsetenv("JAEGER_DISABLED") + require.NoError(t, err) }() ts := &TracingService{enabled: false} @@ -84,12 +88,14 @@ func TestInitJaegerCfg_EnabledViaEnv(t *testing.T) { } func TestInitJaegerCfg_InvalidEnvVar(t *testing.T) { - os.Setenv("JAEGER_DISABLED", "totallybogus") + err := os.Setenv("JAEGER_DISABLED", "totallybogus") + require.NoError(t, err) defer func() { - os.Unsetenv("JAEGER_DISABLED") + err := os.Unsetenv("JAEGER_DISABLED") + require.NoError(t, err) }() ts := &TracingService{} - _, err := ts.initJaegerCfg() + _, err = ts.initJaegerCfg() require.EqualError(t, err, "cannot parse env var JAEGER_DISABLED=totallybogus: strconv.ParseBool: parsing \"totallybogus\": invalid syntax") } diff --git a/pkg/infra/usagestats/service.go b/pkg/infra/usagestats/service.go index 95898d5c81b..f1f1b012f17 100644 --- a/pkg/infra/usagestats/service.go +++ b/pkg/infra/usagestats/service.go @@ -50,7 +50,9 @@ func (uss *UsageStatsService) Run(ctx context.Context) error { for { select { case <-onceEveryDayTick.C: - uss.sendUsageStats() + if err := uss.sendUsageStats(); err != nil { + metricsLogger.Warn("Failed to send usage stats", "err", err) + } case <-everyMinuteTicker.C: uss.updateTotalStats() case <-ctx.Done(): diff --git a/pkg/infra/usagestats/usage_stats.go b/pkg/infra/usagestats/usage_stats.go index 62ba481a768..4cb64946ee2 100644 --- a/pkg/infra/usagestats/usage_stats.go +++ b/pkg/infra/usagestats/usage_stats.go @@ -186,19 +186,22 @@ func (uss *UsageStatsService) GetUsageReport() (UsageReport, error) { return report, nil } -func (uss *UsageStatsService) sendUsageStats() { +func (uss *UsageStatsService) sendUsageStats() error { if !setting.ReportingEnabled { - return + return nil } metricsLogger.Debug(fmt.Sprintf("Sending anonymous usage stats to %s", usageStatsURL)) report, err := uss.GetUsageReport() if err != nil { - return + return err } - out, _ := json.MarshalIndent(report, "", " ") + out, err := json.MarshalIndent(report, "", " ") + if err != nil { + return err + } data := bytes.NewBuffer(out) client := http.Client{Timeout: 5 * time.Second} @@ -208,8 +211,12 @@ func (uss *UsageStatsService) sendUsageStats() { metricsLogger.Error("Failed to send usage stats", "err", err) return } - resp.Body.Close() + if err := resp.Body.Close(); err != nil { + metricsLogger.Warn("Failed to close response body", "err", err) + } }() + + return nil } func (uss *UsageStatsService) updateTotalStats() { diff --git a/pkg/infra/usagestats/usage_stats_test.go b/pkg/infra/usagestats/usage_stats_test.go index f1e92e62ef7..9575e55fe3c 100644 --- a/pkg/infra/usagestats/usage_stats_test.go +++ b/pkg/infra/usagestats/usage_stats_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/licensing" + "github.com/stretchr/testify/require" "net/http" "net/http/httptest" @@ -171,11 +172,13 @@ func TestMetrics(t *testing.T) { "grafana_com": true, } - uss.sendUsageStats() + err := uss.sendUsageStats() + require.NoError(t, err) Convey("Given reporting not enabled and sending usage stats", func() { setting.ReportingEnabled = false - uss.sendUsageStats() + err := uss.sendUsageStats() + require.NoError(t, err) Convey("Should not gather stats or call http endpoint", func() { So(getSystemStatsQuery, ShouldBeNil) @@ -195,7 +198,8 @@ func TestMetrics(t *testing.T) { setting.Packaging = "deb" wg.Add(1) - uss.sendUsageStats() + err := uss.sendUsageStats() + require.NoError(t, err) Convey("Should gather stats and call http endpoint", func() { if waitTimeout(&wg, 2*time.Second) { diff --git a/pkg/login/social/azuread_oauth.go b/pkg/login/social/azuread_oauth.go index 3e40d4ce831..f2f31e87111 100644 --- a/pkg/login/social/azuread_oauth.go +++ b/pkg/login/social/azuread_oauth.go @@ -56,7 +56,7 @@ func (s *SocialAzureAD) UserInfo(_ *http.Client, token *oauth2.Token) (*BasicUse groups := extractGroups(claims) if !s.IsGroupMember(groups) { - return nil, ErrMissingGroupMembership + return nil, errMissingGroupMembership } return &BasicUserInfo{ diff --git a/pkg/login/social/common.go b/pkg/login/social/common.go index ef3eccdec7a..ac3fd5bf45d 100644 --- a/pkg/login/social/common.go +++ b/pkg/login/social/common.go @@ -14,10 +14,10 @@ import ( ) var ( - ErrMissingGroupMembership = &Error{"User not a member of one of the required groups"} + errMissingGroupMembership = Error{"user not a member of one of the required groups"} ) -type HttpGetResponse struct { +type httpGetResponse struct { Body []byte Headers http.Header } @@ -44,20 +44,24 @@ func isEmailAllowed(email string, allowedDomains []string) bool { return valid } -func HttpGet(client *http.Client, url string) (response HttpGetResponse, err error) { +func (s *SocialBase) httpGet(client *http.Client, url string) (response httpGetResponse, err error) { r, err := client.Get(url) if err != nil { return } - defer r.Body.Close() + defer func() { + if err := r.Body.Close(); err != nil { + s.log.Warn("Failed to close response body", "err", err) + } + }() body, err := ioutil.ReadAll(r.Body) if err != nil { return } - response = HttpGetResponse{body, r.Header} + response = httpGetResponse{body, r.Header} if r.StatusCode >= 300 { err = fmt.Errorf(string(response.Body)) diff --git a/pkg/login/social/generic_oauth.go b/pkg/login/social/generic_oauth.go index dc9af92790f..e7c0efa1992 100644 --- a/pkg/login/social/generic_oauth.go +++ b/pkg/login/social/generic_oauth.go @@ -229,7 +229,11 @@ func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson s.log.Error("Error creating zlib reader", "error", err) return nil } - defer fr.Close() + defer func() { + if err := fr.Close(); err != nil { + s.log.Warn("Failed closing zlib reader", "error", err) + } + }() rawJSON, err = ioutil.ReadAll(fr) if err != nil { s.log.Error("Error decompressing payload", "error", err) @@ -251,7 +255,7 @@ func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson func (s *SocialGenericOAuth) extractFromAPI(client *http.Client) *UserInfoJson { s.log.Debug("Getting user info from API") - rawUserInfoResponse, err := HttpGet(client, s.apiUrl) + rawUserInfoResponse, err := s.httpGet(client, s.apiUrl) if err != nil { s.log.Debug("Error getting user info from API", "url", s.apiUrl, "error", err) return nil @@ -347,7 +351,7 @@ func (s *SocialGenericOAuth) FetchPrivateEmail(client *http.Client) (string, err IsConfirmed bool `json:"is_confirmed"` } - response, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/emails")) + response, err := s.httpGet(client, fmt.Sprintf(s.apiUrl+"/emails")) if err != nil { s.log.Error("Error getting email address", "url", s.apiUrl+"/emails", "error", err) return "", errutil.Wrap("Error getting email address", err) @@ -390,7 +394,7 @@ func (s *SocialGenericOAuth) FetchTeamMemberships(client *http.Client) ([]int, b Id int `json:"id"` } - response, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/teams")) + response, err := s.httpGet(client, fmt.Sprintf(s.apiUrl+"/teams")) if err != nil { s.log.Error("Error getting team memberships", "url", s.apiUrl+"/teams", "error", err) return nil, false @@ -419,7 +423,7 @@ func (s *SocialGenericOAuth) FetchOrganizations(client *http.Client) ([]string, Login string `json:"login"` } - response, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/orgs")) + response, err := s.httpGet(client, fmt.Sprintf(s.apiUrl+"/orgs")) if err != nil { s.log.Error("Error getting organizations", "url", s.apiUrl+"/orgs", "error", err) return nil, false diff --git a/pkg/login/social/github_oauth.go b/pkg/login/social/github_oauth.go index 70a5492e804..af04f158328 100644 --- a/pkg/login/social/github_oauth.go +++ b/pkg/login/social/github_oauth.go @@ -29,8 +29,8 @@ type GithubTeam struct { } var ( - ErrMissingTeamMembership = &Error{"User not a member of one of the required teams"} - ErrMissingOrganizationMembership = &Error{"User not a member of one of the required organizations"} + ErrMissingTeamMembership = Error{"user not a member of one of the required teams"} + ErrMissingOrganizationMembership = Error{"user not a member of one of the required organizations"} ) func (s *SocialGithub) Type() int { @@ -86,7 +86,7 @@ func (s *SocialGithub) FetchPrivateEmail(client *http.Client) (string, error) { Verified bool `json:"verified"` } - response, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/emails")) + response, err := s.httpGet(client, fmt.Sprintf(s.apiUrl+"/emails")) if err != nil { return "", fmt.Errorf("Error getting email address: %s", err) } @@ -114,7 +114,7 @@ func (s *SocialGithub) FetchTeamMemberships(client *http.Client) ([]GithubTeam, teams := make([]GithubTeam, 0) for hasMore { - response, err := HttpGet(client, url) + response, err := s.httpGet(client, url) if err != nil { return nil, fmt.Errorf("Error getting team memberships: %s", err) } @@ -157,16 +157,16 @@ func (s *SocialGithub) FetchOrganizations(client *http.Client, organizationsUrl Login string `json:"login"` } - response, err := HttpGet(client, organizationsUrl) + response, err := s.httpGet(client, organizationsUrl) if err != nil { - return nil, fmt.Errorf("Error getting organizations: %s", err) + return nil, fmt.Errorf("error getting organizations: %s", err) } var records []Record err = json.Unmarshal(response.Body, &records) if err != nil { - return nil, fmt.Errorf("Error getting organizations: %s", err) + return nil, fmt.Errorf("error getting organizations: %s", err) } var logins = make([]string, len(records)) @@ -184,9 +184,9 @@ func (s *SocialGithub) UserInfo(client *http.Client, token *oauth2.Token) (*Basi Email string `json:"email"` } - response, err := HttpGet(client, s.apiUrl) + response, err := s.httpGet(client, s.apiUrl) if err != nil { - return nil, fmt.Errorf("Error getting user info: %s", err) + return nil, fmt.Errorf("error getting user info: %s", err) } err = json.Unmarshal(response.Body, &data) diff --git a/pkg/login/social/gitlab_oauth.go b/pkg/login/social/gitlab_oauth.go index d6be068c2e5..afb8731c629 100644 --- a/pkg/login/social/gitlab_oauth.go +++ b/pkg/login/social/gitlab_oauth.go @@ -62,7 +62,7 @@ func (s *SocialGitlab) GetGroupsPage(client *http.Client, url string) ([]string, return nil, next } - response, err := HttpGet(client, url) + response, err := s.httpGet(client, url) if err != nil { s.log.Error("Error getting groups from GitLab API", "err", err) return nil, next @@ -98,7 +98,7 @@ func (s *SocialGitlab) UserInfo(client *http.Client, token *oauth2.Token) (*Basi State string } - response, err := HttpGet(client, s.apiUrl+"/user") + response, err := s.httpGet(client, s.apiUrl+"/user") if err != nil { return nil, fmt.Errorf("Error getting user info: %s", err) } @@ -123,7 +123,7 @@ func (s *SocialGitlab) UserInfo(client *http.Client, token *oauth2.Token) (*Basi } if !s.IsGroupMember(groups) { - return nil, ErrMissingGroupMembership + return nil, errMissingGroupMembership } return userInfo, nil diff --git a/pkg/login/social/google_oauth.go b/pkg/login/social/google_oauth.go index 60e1e4568bd..e15834a45fb 100644 --- a/pkg/login/social/google_oauth.go +++ b/pkg/login/social/google_oauth.go @@ -27,7 +27,7 @@ func (s *SocialGoogle) UserInfo(client *http.Client, token *oauth2.Token) (*Basi Email string `json:"email"` } - response, err := HttpGet(client, s.apiUrl) + response, err := s.httpGet(client, s.apiUrl) if err != nil { return nil, fmt.Errorf("Error getting user info: %s", err) } diff --git a/pkg/login/social/grafana_com_oauth.go b/pkg/login/social/grafana_com_oauth.go index 4049aa9b8bd..2757bc6aca0 100644 --- a/pkg/login/social/grafana_com_oauth.go +++ b/pkg/login/social/grafana_com_oauth.go @@ -54,7 +54,7 @@ func (s *SocialGrafanaCom) UserInfo(client *http.Client, token *oauth2.Token) (* Orgs []OrgRecord `json:"orgs"` } - response, err := HttpGet(client, s.url+"/api/oauth2/user") + response, err := s.httpGet(client, s.url+"/api/oauth2/user") if err != nil { return nil, fmt.Errorf("Error getting user info: %s", err) } diff --git a/pkg/login/social/okta_oauth.go b/pkg/login/social/okta_oauth.go index 4fb30c4bea5..800f22d3f26 100644 --- a/pkg/login/social/okta_oauth.go +++ b/pkg/login/social/okta_oauth.go @@ -84,7 +84,7 @@ func (s *SocialOkta) UserInfo(client *http.Client, token *oauth2.Token) (*BasicU groups := s.GetGroups(&data) if !s.IsGroupMember(groups) { - return nil, ErrMissingGroupMembership + return nil, errMissingGroupMembership } return &BasicUserInfo{ @@ -98,7 +98,7 @@ func (s *SocialOkta) UserInfo(client *http.Client, token *oauth2.Token) (*BasicU } func (s *SocialOkta) extractAPI(data *OktaUserInfoJson, client *http.Client) error { - rawUserInfoResponse, err := HttpGet(client, s.apiUrl) + rawUserInfoResponse, err := s.httpGet(client, s.apiUrl) if err != nil { s.log.Debug("Error getting user info response", "url", s.apiUrl, "error", err) return errutil.Wrapf(err, "error getting user info response") diff --git a/pkg/login/social/social.go b/pkg/login/social/social.go index 528920ae0f1..ca0bec2598a 100644 --- a/pkg/login/social/social.go +++ b/pkg/login/social/social.go @@ -54,7 +54,7 @@ type Error struct { s string } -func (e *Error) Error() string { +func (e Error) Error() string { return e.s } diff --git a/pkg/middleware/dashboard_redirect_test.go b/pkg/middleware/dashboard_redirect_test.go index 30a9d27fe4c..3e4f9d5e915 100644 --- a/pkg/middleware/dashboard_redirect_test.go +++ b/pkg/middleware/dashboard_redirect_test.go @@ -35,8 +35,12 @@ func TestMiddlewareDashboardRedirect(t *testing.T) { sc.fakeReqWithParams("GET", "/dashboard/db/dash?orgId=1&panelId=2", map[string]string{}).exec() assert.Equal(t, 301, sc.resp.Code) + // nolint:bodyclose resp := sc.resp.Result() - resp.Body.Close() + t.Cleanup(func() { + err := resp.Body.Close() + assert.NoError(t, err) + }) redirectURL, err := resp.Location() require.NoError(t, err) assert.Equal(t, models.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug), redirectURL.Path) @@ -54,8 +58,12 @@ func TestMiddlewareDashboardRedirect(t *testing.T) { sc.fakeReqWithParams("GET", "/dashboard-solo/db/dash?orgId=1&panelId=2", map[string]string{}).exec() assert.Equal(t, 301, sc.resp.Code) + // nolint:bodyclose resp := sc.resp.Result() - resp.Body.Close() + t.Cleanup(func() { + err := resp.Body.Close() + assert.NoError(t, err) + }) redirectURL, err := resp.Location() require.NoError(t, err) expectedURL := models.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug) @@ -71,8 +79,12 @@ func TestMiddlewareDashboardRedirect(t *testing.T) { sc.fakeReqWithParams("GET", "/d/asd/dash?orgId=1&panelId=12&fullscreen&edit", map[string]string{}).exec() assert.Equal(t, 301, sc.resp.Code) + // nolint:bodyclose resp := sc.resp.Result() - resp.Body.Close() + t.Cleanup(func() { + err := resp.Body.Close() + assert.NoError(t, err) + }) redirectURL, err := resp.Location() require.NoError(t, err) assert.Equal(t, "/d/asd/d/asd/dash?editPanel=12&orgId=1", redirectURL.String()) diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index ff930813cbc..df286edc82a 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -378,8 +378,10 @@ func TestMiddlewareContext(t *testing.T) { return nil }) - key := fmt.Sprintf(authproxy.CachePrefix, authproxy.HashCacheKey(hdrName+"-"+group)) - err := sc.remoteCacheService.Set(key, userID, 0) + h, err := authproxy.HashCacheKey(hdrName + "-" + group) + require.NoError(t, err) + key := fmt.Sprintf(authproxy.CachePrefix, h) + err = sc.remoteCacheService.Set(key, userID, 0) require.NoError(t, err) sc.fakeReq("GET", "/") diff --git a/pkg/models/datasource_cache_test.go b/pkg/models/datasource_cache_test.go index 15779de553e..0a28f06d934 100644 --- a/pkg/models/datasource_cache_test.go +++ b/pkg/models/datasource_cache_test.go @@ -9,6 +9,7 @@ import ( "time" . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/components/securejsondata" @@ -223,15 +224,16 @@ func TestDataSourceProxyCache(t *testing.T) { // 2. Get HTTP transport from datasource which uses the test server as backend ds.Url = backend.URL transport, err := ds.GetHttpTransport() - if err != nil { - log.Fatal(err.Error()) - } + So(err, ShouldBeNil) // 3. Send test request which should have the Authorization header set req := httptest.NewRequest("GET", backend.URL+"/test-headers", nil) res, err := transport.RoundTrip(req) So(err, ShouldBeNil) - defer res.Body.Close() + t.Cleanup(func() { + err := res.Body.Close() + assert.NoError(t, err) + }) body, err := ioutil.ReadAll(res.Body) So(err, ShouldBeNil) bodyStr := string(body) diff --git a/pkg/plugins/backendplugin/grpcplugin/client.go b/pkg/plugins/backendplugin/grpcplugin/client.go index a32c9bb7bea..2b495ac8624 100644 --- a/pkg/plugins/backendplugin/grpcplugin/client.go +++ b/pkg/plugins/backendplugin/grpcplugin/client.go @@ -31,7 +31,10 @@ var handshake = goplugin.HandshakeConfig{ MagicCookieValue: grpcplugin.MagicCookieValue, } -func newClientConfig(executablePath string, env []string, logger log.Logger, versionedPlugins map[int]goplugin.PluginSet) *goplugin.ClientConfig { +func newClientConfig(executablePath string, env []string, logger log.Logger, + versionedPlugins map[int]goplugin.PluginSet) *goplugin.ClientConfig { + // We can ignore gosec G201 here, since the dynamic part of executablePath comes from the plugin definition + // nolint:gosec cmd := exec.Command(executablePath) cmd.Env = env @@ -77,7 +80,7 @@ func getV2PluginSet() goplugin.PluginSet { // NewBackendPlugin creates a new backend plugin factory used for registering a backend plugin. func NewBackendPlugin(pluginID, executablePath string, startFns PluginStartFuncs) backendplugin.PluginFactoryFunc { - return New(PluginDescriptor{ + return newPlugin(PluginDescriptor{ pluginID: pluginID, executablePath: executablePath, managed: true, @@ -93,7 +96,7 @@ func NewBackendPlugin(pluginID, executablePath string, startFns PluginStartFuncs // NewRendererPlugin creates a new renderer plugin factory used for registering a backend renderer plugin. func NewRendererPlugin(pluginID, executablePath string, startFns PluginStartFuncs) backendplugin.PluginFactoryFunc { - return New(PluginDescriptor{ + return newPlugin(PluginDescriptor{ pluginID: pluginID, executablePath: executablePath, managed: false, diff --git a/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go b/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go index 54277e8adf4..5e3cc790287 100644 --- a/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go +++ b/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go @@ -26,8 +26,8 @@ type grpcPlugin struct { mutex sync.RWMutex } -// New allocates and returns a new gRPC (external) backendplugin.Plugin. -func New(descriptor PluginDescriptor) backendplugin.PluginFactoryFunc { +// newPlugin allocates and returns a new gRPC (external) backendplugin.Plugin. +func newPlugin(descriptor PluginDescriptor) backendplugin.PluginFactoryFunc { return backendplugin.PluginFactoryFunc(func(pluginID string, logger log.Logger, env []string) (backendplugin.Plugin, error) { return &grpcPlugin{ descriptor: descriptor, diff --git a/pkg/plugins/backendplugin/manager.go b/pkg/plugins/backendplugin/manager.go index 51804d89277..5913b202708 100644 --- a/pkg/plugins/backendplugin/manager.go +++ b/pkg/plugins/backendplugin/manager.go @@ -257,6 +257,11 @@ func (m *manager) callResourceInternal(w http.ResponseWriter, req *http.Request, childCtx, cancel := context.WithCancel(req.Context()) defer cancel() stream := newCallResourceResponseStream(childCtx) + defer func() { + if err := stream.Close(); err != nil { + m.logger.Warn("Failed to close stream", "err", err) + } + }() var wg sync.WaitGroup wg.Add(1) var flushStreamErr error @@ -265,12 +270,15 @@ func (m *manager) callResourceInternal(w http.ResponseWriter, req *http.Request, wg.Done() }() - innerErr := p.CallResource(req.Context(), crReq, stream) - stream.Close() - if innerErr != nil { - return innerErr + if err := p.CallResource(req.Context(), crReq, stream); err != nil { + return err } + if err := stream.Close(); err != nil { + return err + } + wg.Wait() + return flushStreamErr }) } diff --git a/pkg/plugins/dashboards.go b/pkg/plugins/dashboards.go index 6cd9f827449..431f69e74e7 100644 --- a/pkg/plugins/dashboards.go +++ b/pkg/plugins/dashboards.go @@ -95,11 +95,11 @@ func loadPluginDashboard(pluginId, path string) (*models.Dashboard, error) { return nil, PluginNotFoundError{pluginId} } + dashboardFilePath := filepath.Join(plugin.PluginDir, path) // nolint:gosec // We can ignore the gosec G304 warning on this one because `plugin.PluginDir` is based // on plugin folder structure on disk and not user input. `path` comes from the // `plugin.json` configuration file for the loaded plugin - dashboardFilePath := filepath.Join(plugin.PluginDir, path) reader, err := os.Open(dashboardFilePath) if err != nil { return nil, err diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 730627c66a8..20977c4ed60 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -335,9 +335,6 @@ func (s *PluginScanner) walker(currentPath string, f os.FileInfo, err error) err return nil } - // nolint:gosec - // We can ignore the gosec G304 warning on this one because `currentPath` is based - // on plugin the folder structure on disk and not user input. if err := s.loadPlugin(currentPath); err != nil { s.log.Error("Failed to load plugin", "error", err, "pluginPath", filepath.Dir(currentPath)) s.errors = append(s.errors, err) @@ -349,6 +346,9 @@ func (s *PluginScanner) walker(currentPath string, f os.FileInfo, err error) err func (s *PluginScanner) loadPlugin(pluginJSONFilePath string) error { s.log.Debug("Loading plugin", "path", pluginJSONFilePath) currentDir := filepath.Dir(pluginJSONFilePath) + // nolint:gosec + // We can ignore the gosec G304 warning on this one because `currentPath` is based + // on plugin the folder structure on disk and not user input. reader, err := os.Open(pluginJSONFilePath) if err != nil { return err diff --git a/pkg/plugins/update_checker.go b/pkg/plugins/update_checker.go index 864dbf5c2d8..d1e60e05208 100644 --- a/pkg/plugins/update_checker.go +++ b/pkg/plugins/update_checker.go @@ -48,13 +48,15 @@ func (pm *PluginManager) checkForUpdates() { pluginSlugs := getAllExternalPluginSlugs() resp, err := httpClient.Get("https://grafana.com/api/plugins/versioncheck?slugIn=" + pluginSlugs + "&grafanaVersion=" + setting.BuildVersion) - if err != nil { log.Tracef("Failed to get plugins repo from grafana.com, %v", err.Error()) return } - - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + log.Warn("Failed to close response body", "err", err) + } + }() body, err := ioutil.ReadAll(resp.Body) if err != nil { @@ -91,8 +93,11 @@ func (pm *PluginManager) checkForUpdates() { log.Tracef("Failed to get latest.json repo from github.com: %v", err.Error()) return } - - defer resp2.Body.Close() + defer func() { + if err := resp2.Body.Close(); err != nil { + pm.log.Warn("Failed to close response body", "err", err) + } + }() body, err = ioutil.ReadAll(resp2.Body) if err != nil { log.Tracef("Update check failed, reading response from github.com, %v", err.Error()) diff --git a/pkg/server/server.go b/pkg/server/server.go index 6fd28aa63e9..d4ed3e594c4 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -341,7 +341,11 @@ func (s *Server) notifySystemd(state string) { s.log.Warn("Failed to connect to systemd", "err", err, "socket", notifySocket) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.log.Warn("Failed to close connection", "err", err) + } + }() _, err = conn.Write([]byte(state)) if err != nil { diff --git a/pkg/services/alerting/alerting_usage_test.go b/pkg/services/alerting/alerting_usage_test.go index 3c554fef7ea..c66887fc625 100644 --- a/pkg/services/alerting/alerting_usage_test.go +++ b/pkg/services/alerting/alerting_usage_test.go @@ -19,10 +19,13 @@ func TestAlertingUsageStats(t *testing.T) { ae.Bus.AddHandler(func(query *models.GetAllAlertsQuery) error { var createFake = func(file string) *simplejson.Json { + // Ignore gosec warning G304 since it's a test + // nolint:gosec content, err := ioutil.ReadFile(file) require.NoError(t, err, "expected to be able to read file") - j, _ := simplejson.NewJson(content) + j, err := simplejson.NewJson(content) + require.NoError(t, err) return j } diff --git a/pkg/services/alerting/notifiers/pushover.go b/pkg/services/alerting/notifiers/pushover.go index c13270205bc..a58c5a8b000 100644 --- a/pkg/services/alerting/notifiers/pushover.go +++ b/pkg/services/alerting/notifiers/pushover.go @@ -387,8 +387,10 @@ func (pn *PushoverNotifier) genPushoverBody(evalContext *alerting.EvalContext, m if err != nil { return nil, b, err } + if err := w.Close(); err != nil { + return nil, b, err + } - w.Close() headers := map[string]string{ "Content-Type": w.FormDataContentType(), } diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index ede1ccb1487..eef7a6c5530 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/pkg/services/alerting/notifiers/telegram.go @@ -170,6 +170,11 @@ func (tn *TelegramNotifier) buildMessageInlineImage(evalContext *alerting.EvalCo func (tn *TelegramNotifier) generateTelegramCmd(message string, messageField string, apiAction string, extraConf func(writer *multipart.Writer)) (*models.SendWebhookSync, error) { var body bytes.Buffer w := multipart.NewWriter(&body) + defer func() { + if err := w.Close(); err != nil { + tn.log.Warn("Failed to close writer", "err", err) + } + }() fw, err := w.CreateFormField("chat_id") if err != nil { @@ -189,7 +194,9 @@ func (tn *TelegramNotifier) generateTelegramCmd(message string, messageField str extraConf(w) - w.Close() + if err := w.Close(); err != nil { + return nil, err + } tn.log.Info("Sending telegram notification", "chat_id", tn.ChatID, "bot_token", tn.BotToken, "apiAction", apiAction) url := fmt.Sprintf(telegramAPIURL, tn.BotToken, apiAction) diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index 790f7959a22..b7a86d50eea 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -65,7 +65,9 @@ func TestInitContextWithAuthProxy_CachedInvalidUserID(t *testing.T) { Logger: log.New("Test"), } req.Header.Set(svc.Cfg.AuthProxyHeaderName, name) - key := fmt.Sprintf(authproxy.CachePrefix, authproxy.HashCacheKey(name)) + h, err := authproxy.HashCacheKey(name) + require.NoError(t, err) + key := fmt.Sprintf(authproxy.CachePrefix, h) t.Logf("Injecting stale user ID in cache with key %q", key) err = svc.RemoteCache.Set(key, int64(33), 0) diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index df3d52f3564..e77f2de78ff 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -141,25 +141,29 @@ func (auth *AuthProxy) IsAllowedIP() error { )) } -func HashCacheKey(key string) string { +func HashCacheKey(key string) (string, error) { hasher := fnv.New128a() - // according to the documentation, Hash.Write cannot error, but linter is complaining - hasher.Write([]byte(key)) // nolint: errcheck - return hex.EncodeToString(hasher.Sum(nil)) + if _, err := hasher.Write([]byte(key)); err != nil { + return "", err + } + return hex.EncodeToString(hasher.Sum(nil)), nil } // getKey forms a key for the cache based on the headers received as part of the authentication flow. // Our configuration supports multiple headers. The main header contains the email or username. // And the additional ones that allow us to specify extra attributes: Name, Email or Groups. -func (auth *AuthProxy) getKey() string { +func (auth *AuthProxy) getKey() (string, error) { key := strings.TrimSpace(auth.header) // start the key with the main header auth.headersIterator(func(_, header string) { key = strings.Join([]string{key, header}, "-") // compose the key with any additional headers }) - hashedKey := HashCacheKey(key) - return fmt.Sprintf(CachePrefix, hashedKey) + hashedKey, err := HashCacheKey(key) + if err != nil { + return "", err + } + return fmt.Sprintf(CachePrefix, hashedKey), nil } // Login logs in user ID by whatever means possible. @@ -194,7 +198,10 @@ func (auth *AuthProxy) Login(logger log.Logger, ignoreCache bool) (int64, error) // GetUserViaCache gets user ID from cache. func (auth *AuthProxy) GetUserViaCache(logger log.Logger) (int64, error) { - cacheKey := auth.getKey() + cacheKey, err := auth.getKey() + if err != nil { + return 0, err + } logger.Debug("Getting user ID via auth cache", "cacheKey", cacheKey) userID, err := auth.remoteCache.Get(cacheKey) if err != nil { @@ -208,7 +215,10 @@ func (auth *AuthProxy) GetUserViaCache(logger log.Logger) (int64, error) { // RemoveUserFromCache removes user from cache. func (auth *AuthProxy) RemoveUserFromCache(logger log.Logger) error { - cacheKey := auth.getKey() + cacheKey, err := auth.getKey() + if err != nil { + return err + } logger.Debug("Removing user from auth cache", "cacheKey", cacheKey) if err := auth.remoteCache.Delete(cacheKey); err != nil { return err @@ -318,18 +328,20 @@ func (auth *AuthProxy) GetSignedInUser(userID int64) (*models.SignedInUser, erro // Remember user in cache func (auth *AuthProxy) Remember(id int64) error { - key := auth.getKey() + key, err := auth.getKey() + if err != nil { + return err + } // Check if user already in cache - userID, _ := auth.remoteCache.Get(key) - if userID != nil { + userID, err := auth.remoteCache.Get(key) + if err == nil && userID != nil { return nil } expiration := time.Duration(auth.cfg.AuthProxySyncTTL) * time.Minute - err := auth.remoteCache.Set(key, id, expiration) - if err != nil { + if err := auth.remoteCache.Set(key, id, expiration); err != nil { return err } diff --git a/pkg/services/contexthandler/authproxy/authproxy_test.go b/pkg/services/contexthandler/authproxy/authproxy_test.go index 7c5130b0b90..cb396760057 100644 --- a/pkg/services/contexthandler/authproxy/authproxy_test.go +++ b/pkg/services/contexthandler/authproxy/authproxy_test.go @@ -87,12 +87,16 @@ func TestMiddlewareContext(t *testing.T) { t.Run("When the cache only contains the main header with a simple cache key", func(t *testing.T) { const id int64 = 33 // Set cache key - key := fmt.Sprintf(CachePrefix, HashCacheKey(hdrName)) - err := cache.Set(key, id, 0) + h, err := HashCacheKey(hdrName) + require.NoError(t, err) + key := fmt.Sprintf(CachePrefix, h) + err = cache.Set(key, id, 0) require.NoError(t, err) // Set up the middleware auth := prepareMiddleware(t, cache, nil) - assert.Equal(t, key, auth.getKey()) + gotKey, err := auth.getKey() + require.NoError(t, err) + assert.Equal(t, key, gotKey) gotID, err := auth.Login(logger, false) require.NoError(t, err) @@ -104,15 +108,17 @@ func TestMiddlewareContext(t *testing.T) { const id int64 = 33 const group = "grafana-core-team" - key := fmt.Sprintf(CachePrefix, HashCacheKey(hdrName+"-"+group)) - err := cache.Set(key, id, 0) + h, err := HashCacheKey(hdrName + "-" + group) + require.NoError(t, err) + key := fmt.Sprintf(CachePrefix, h) + err = cache.Set(key, id, 0) require.NoError(t, err) auth := prepareMiddleware(t, cache, func(req *http.Request, cfg *setting.Cfg) { req.Header.Set("X-WEBAUTH-GROUPS", group) cfg.AuthProxyHeaders = map[string]string{"Groups": "X-WEBAUTH-GROUPS"} }) - assert.Equal(t, "auth-proxy-sync-ttl:14f69b7023baa0ac98c96b31cec07bc0", auth.getKey()) + assert.Equal(t, "auth-proxy-sync-ttl:14f69b7023baa0ac98c96b31cec07bc0", key) gotID, err := auth.Login(logger, false) require.NoError(t, err) diff --git a/pkg/services/contexthandler/contexthandler_test.go b/pkg/services/contexthandler/contexthandler_test.go index d01c04b37dc..a87235cbae6 100644 --- a/pkg/services/contexthandler/contexthandler_test.go +++ b/pkg/services/contexthandler/contexthandler_test.go @@ -64,8 +64,12 @@ func TestTokenRotationAtEndOfRequest(t *testing.T) { ctxHdlr.rotateEndOfRequestFunc(reqContext, uts, token)(reqContext.Resp) foundLoginCookie := false + // nolint:bodyclose resp := rr.Result() - defer resp.Body.Close() + t.Cleanup(func() { + err := resp.Body.Close() + assert.NoError(t, err) + }) for _, c := range resp.Cookies() { if c.Name == "login_token" { foundLoginCookie = true diff --git a/pkg/services/ldap/settings_test.go b/pkg/services/ldap/settings_test.go index 6cb27b48c6f..0c542798de0 100644 --- a/pkg/services/ldap/settings_test.go +++ b/pkg/services/ldap/settings_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestReadingLDAPSettings(t *testing.T) { @@ -14,9 +15,10 @@ func TestReadingLDAPSettings(t *testing.T) { } func TestReadingLDAPSettingsWithEnvVariable(t *testing.T) { - os.Setenv("ENV_PASSWORD", "MySecret") + err := os.Setenv("ENV_PASSWORD", "MySecret") + require.NoError(t, err) config, err := readConfig("testdata/ldap.toml") - assert.Nil(t, err, "No error when reading ldap config") + require.NoError(t, err) assert.EqualValues(t, "MySecret", config.Servers[0].BindPassword) } diff --git a/pkg/services/notifications/webhook.go b/pkg/services/notifications/webhook.go index 028814dc3ec..2ae7f5f4fcc 100644 --- a/pkg/services/notifications/webhook.go +++ b/pkg/services/notifications/webhook.go @@ -76,8 +76,11 @@ func (ns *NotificationService) sendWebRequestSync(ctx context.Context, webhook * if err != nil { return err } - - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + ns.log.Warn("Failed to close response body", "err", err) + } + }() if resp.StatusCode/100 == 2 { ns.log.Debug("Webhook succeeded", "url", webhook.Url, "statuscode", resp.Status) diff --git a/pkg/services/provisioning/values/values_test.go b/pkg/services/provisioning/values/values_test.go index 0da5174c5ea..89e791c4d10 100644 --- a/pkg/services/provisioning/values/values_test.go +++ b/pkg/services/provisioning/values/values_test.go @@ -19,10 +19,14 @@ import ( func TestValues(t *testing.T) { Convey("Values", t, func() { - os.Setenv("INT", "1") - os.Setenv("STRING", "test") - os.Setenv("EMPTYSTRING", "") - os.Setenv("BOOL", "true") + err := os.Setenv("INT", "1") + So(err, ShouldBeNil) + err = os.Setenv("STRING", "test") + So(err, ShouldBeNil) + err = os.Setenv("EMPTYSTRING", "") + So(err, ShouldBeNil) + err = os.Setenv("BOOL", "true") + So(err, ShouldBeNil) Convey("IntValue", func() { type Data struct { @@ -240,10 +244,14 @@ func TestValues(t *testing.T) { }) Reset(func() { - os.Unsetenv("INT") - os.Unsetenv("STRING") - os.Unsetenv("EMPTYSTRING") - os.Unsetenv("BOOL") + err := os.Unsetenv("INT") + So(err, ShouldBeNil) + err = os.Unsetenv("STRING") + So(err, ShouldBeNil) + err = os.Unsetenv("EMPTYSTRING") + So(err, ShouldBeNil) + err = os.Unsetenv("BOOL") + So(err, ShouldBeNil) }) }) } diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 7cc30c891a6..96d1ead5465 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -77,7 +77,11 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, renderKey string, } // save response to file - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + rs.log.Warn("Failed to close response body", "err", err) + } + }() // check for timeout first if errors.Is(reqContext.Err(), context.DeadlineExceeded) { diff --git a/pkg/setting/dynamic_settings_test.go b/pkg/setting/dynamic_settings_test.go index b580f5914df..978ace89694 100644 --- a/pkg/setting/dynamic_settings_test.go +++ b/pkg/setting/dynamic_settings_test.go @@ -15,8 +15,12 @@ func TestDynamicSettingsSupport_Override(t *testing.T) { keyName := "bar" expected := "dynamic value" - os.Setenv(envKey, expected) - defer func() { os.Unsetenv(envKey) }() + err := os.Setenv(envKey, expected) + require.NoError(t, err) + defer func() { + err := os.Unsetenv(envKey) + require.NoError(t, err) + }() value := cfg.SectionWithEnvOverrides(sectionName).Key(keyName).MustString("default value") require.Equal(t, expected, value) diff --git a/pkg/setting/expanders_test.go b/pkg/setting/expanders_test.go index 645a0b02919..061e9a7e950 100644 --- a/pkg/setting/expanders_test.go +++ b/pkg/setting/expanders_test.go @@ -16,7 +16,8 @@ import ( func TestExpandVar_EnvSuccessful(t *testing.T) { const key = "GF_TEST_SETTING_EXPANDER_ENV" const expected = "aurora borealis" - os.Setenv(key, expected) + err := os.Setenv(key, expected) + require.NoError(t, err) // expanded format { @@ -39,12 +40,14 @@ func TestExpandVar_FileSuccessful(t *testing.T) { file := f.Name() defer func() { - os.Remove(file) + err := os.Remove(file) + require.NoError(t, err) }() _, err = f.WriteString("hello, world") require.NoError(t, err) - f.Close() + err = f.Close() + require.NoError(t, err) got, err := ExpandVar(fmt.Sprintf("$__file{%s}", file)) assert.NoError(t, err) diff --git a/pkg/setting/setting_test.go b/pkg/setting/setting_test.go index ea1c23cae81..f975a87211b 100644 --- a/pkg/setting/setting_test.go +++ b/pkg/setting/setting_test.go @@ -39,7 +39,10 @@ func TestLoadingSettings(t *testing.T) { if err != nil { t.Errorf("failed to load defaults.ini file: %v", err) } - defer file.Close() + defer func() { + err := file.Close() + So(err, ShouldBeNil) + }() scanner := bufio.NewScanner(file) for scanner.Scan() { @@ -51,10 +54,11 @@ func TestLoadingSettings(t *testing.T) { }) Convey("Should be able to override via environment variables", func() { - os.Setenv("GF_SECURITY_ADMIN_USER", "superduper") + err := os.Setenv("GF_SECURITY_ADMIN_USER", "superduper") + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{HomePath: "../../"}) + err = cfg.Load(&CommandLineArgs{HomePath: "../../"}) So(err, ShouldBeNil) So(AdminUser, ShouldEqual, "superduper") @@ -63,29 +67,32 @@ func TestLoadingSettings(t *testing.T) { }) Convey("Should replace password when defined in environment", func() { - os.Setenv("GF_SECURITY_ADMIN_PASSWORD", "supersecret") + err := os.Setenv("GF_SECURITY_ADMIN_PASSWORD", "supersecret") + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{HomePath: "../../"}) + err = cfg.Load(&CommandLineArgs{HomePath: "../../"}) So(err, ShouldBeNil) So(appliedEnvOverrides, ShouldContain, "GF_SECURITY_ADMIN_PASSWORD=*********") }) Convey("Should return an error when url is invalid", func() { - os.Setenv("GF_DATABASE_URL", "postgres.%31://grafana:secret@postgres:5432/grafana") + err := os.Setenv("GF_DATABASE_URL", "postgres.%31://grafana:secret@postgres:5432/grafana") + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{HomePath: "../../"}) + err = cfg.Load(&CommandLineArgs{HomePath: "../../"}) So(err, ShouldNotBeNil) }) Convey("Should replace password in URL when url environment is defined", func() { - os.Setenv("GF_DATABASE_URL", "mysql://user:secret@localhost:3306/database") + err := os.Setenv("GF_DATABASE_URL", "mysql://user:secret@localhost:3306/database") + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{HomePath: "../../"}) + err = cfg.Load(&CommandLineArgs{HomePath: "../../"}) So(err, ShouldBeNil) So(appliedEnvOverrides, ShouldContain, "GF_DATABASE_URL=mysql://user:-redacted-@localhost:3306/database") @@ -186,9 +193,10 @@ func TestLoadingSettings(t *testing.T) { Convey("Can use environment variables in config values", func() { if runtime.GOOS == windows { - os.Setenv("GF_DATA_PATH", `c:\tmp\env_override`) + err := os.Setenv("GF_DATA_PATH", `c:\tmp\env_override`) + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{ + err = cfg.Load(&CommandLineArgs{ HomePath: "../../", Args: []string{"cfg:paths.data=${GF_DATA_PATH}"}, }) @@ -196,9 +204,10 @@ func TestLoadingSettings(t *testing.T) { So(cfg.DataPath, ShouldEqual, `c:\tmp\env_override`) } else { - os.Setenv("GF_DATA_PATH", "/tmp/env_override") + err := os.Setenv("GF_DATA_PATH", "/tmp/env_override") + require.NoError(t, err) cfg := NewCfg() - err := cfg.Load(&CommandLineArgs{ + err = cfg.Load(&CommandLineArgs{ HomePath: "../../", Args: []string{"cfg:paths.data=${GF_DATA_PATH}"}, }) diff --git a/pkg/tsdb/azuremonitor/applicationinsights-datasource.go b/pkg/tsdb/azuremonitor/applicationinsights-datasource.go index b35705d2588..44033230ad0 100644 --- a/pkg/tsdb/azuremonitor/applicationinsights-datasource.go +++ b/pkg/tsdb/azuremonitor/applicationinsights-datasource.go @@ -172,7 +172,11 @@ func (e *ApplicationInsightsDatasource) executeQuery(ctx context.Context, query } body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + azlog.Warn("Failed to close response body", "err", err) + } + }() if err != nil { return nil, err } diff --git a/pkg/tsdb/azuremonitor/applicationinsights-metrics_test.go b/pkg/tsdb/azuremonitor/applicationinsights-metrics_test.go index 20c205a5182..9b9bb3b6718 100644 --- a/pkg/tsdb/azuremonitor/applicationinsights-metrics_test.go +++ b/pkg/tsdb/azuremonitor/applicationinsights-metrics_test.go @@ -156,8 +156,7 @@ func TestInsightsMetricsResultToFrame(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - res, err := loadInsightsMetricsResponse(tt.testFile) - require.NoError(t, err) + res := loadInsightsMetricsResponse(t, tt.testFile) frame, err := InsightsMetricsResultToFrame(res, tt.metric, tt.agg, tt.dimensions) require.NoError(t, err) @@ -171,16 +170,23 @@ func TestInsightsMetricsResultToFrame(t *testing.T) { } } -func loadInsightsMetricsResponse(name string) (MetricsResult, error) { - var mr MetricsResult +func loadInsightsMetricsResponse(t *testing.T, name string) MetricsResult { + t.Helper() path := filepath.Join("testdata", name) + // Ignore gosec warning G304 since it's a test + // nolint:gosec f, err := os.Open(path) - if err != nil { - return mr, err - } - defer f.Close() + require.NoError(t, err) + defer func() { + err := f.Close() + require.NoError(t, err) + }() + d := json.NewDecoder(f) + var mr MetricsResult err = d.Decode(&mr) - return mr, err + require.NoError(t, err) + + return mr } diff --git a/pkg/tsdb/azuremonitor/azure-log-analytics-datasource.go b/pkg/tsdb/azuremonitor/azure-log-analytics-datasource.go index 47f305d49d5..911466f5635 100644 --- a/pkg/tsdb/azuremonitor/azure-log-analytics-datasource.go +++ b/pkg/tsdb/azuremonitor/azure-log-analytics-datasource.go @@ -264,11 +264,14 @@ func (ar *AzureLogAnalyticsResponse) GetPrimaryResultTable() (*AzureLogAnalytics func (e *AzureLogAnalyticsDatasource) unmarshalResponse(res *http.Response) (AzureLogAnalyticsResponse, error) { body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() - if err != nil { return AzureLogAnalyticsResponse{}, err } + defer func() { + if err := res.Body.Close(); err != nil { + azlog.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode/100 != 2 { azlog.Debug("Request failed", "status", res.Status, "body", string(body)) diff --git a/pkg/tsdb/azuremonitor/azure-log-analytics-table-frame_test.go b/pkg/tsdb/azuremonitor/azure-log-analytics-table-frame_test.go index ab4bbf2a057..eb5c3995972 100644 --- a/pkg/tsdb/azuremonitor/azure-log-analytics-table-frame_test.go +++ b/pkg/tsdb/azuremonitor/azure-log-analytics-table-frame_test.go @@ -10,6 +10,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/xorcare/pointer" ) @@ -140,8 +141,7 @@ func TestLogTableToFrame(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - res, err := loadLogAnalyticsTestFileWithNumber(tt.testFile) - require.NoError(t, err) + res := loadLogAnalyticsTestFileWithNumber(t, tt.testFile) frame, err := LogTableToFrame(&res.Tables[0]) require.NoError(t, err) @@ -152,17 +152,22 @@ func TestLogTableToFrame(t *testing.T) { } } -func loadLogAnalyticsTestFileWithNumber(name string) (AzureLogAnalyticsResponse, error) { - var data AzureLogAnalyticsResponse - +func loadLogAnalyticsTestFileWithNumber(t *testing.T, name string) AzureLogAnalyticsResponse { + t.Helper() path := filepath.Join("testdata", name) + // Ignore gosec warning G304 since it's a test + // nolint:gosec f, err := os.Open(path) - if err != nil { - return data, err - } - defer f.Close() + require.NoError(t, err) + defer func() { + err := f.Close() + assert.NoError(t, err) + }() + d := json.NewDecoder(f) d.UseNumber() + var data AzureLogAnalyticsResponse err = d.Decode(&data) - return data, err + require.NoError(t, err) + return data } diff --git a/pkg/tsdb/azuremonitor/azuremonitor-datasource.go b/pkg/tsdb/azuremonitor/azuremonitor-datasource.go index 9b0ef6ba8bc..679611e6e27 100644 --- a/pkg/tsdb/azuremonitor/azuremonitor-datasource.go +++ b/pkg/tsdb/azuremonitor/azuremonitor-datasource.go @@ -206,6 +206,11 @@ func (e *AzureMonitorDatasource) executeQuery(ctx context.Context, query *AzureM queryResult.Error = err return queryResult, AzureMonitorResponse{}, nil } + defer func() { + if err := res.Body.Close(); err != nil { + azlog.Warn("Failed to close response body", "err", err) + } + }() data, err := e.unmarshalResponse(res) if err != nil { @@ -256,7 +261,6 @@ func (e *AzureMonitorDatasource) createRequest(ctx context.Context, dsInfo *mode func (e *AzureMonitorDatasource) unmarshalResponse(res *http.Response) (AzureMonitorResponse, error) { body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() if err != nil { return AzureMonitorResponse{}, err } diff --git a/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go b/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go index 3598c22c568..d5ec77c053c 100644 --- a/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go +++ b/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go @@ -433,10 +433,9 @@ func TestAzureMonitorParseResponse(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - azData, err := loadTestFile("azuremonitor/" + tt.responseFile) - require.NoError(t, err) + azData := loadTestFile(t, "azuremonitor/"+tt.responseFile) res := &tsdb.QueryResult{Meta: simplejson.New(), RefId: "A"} - err = datasource.parseResponse(res, azData, tt.mockQuery) + err := datasource.parseResponse(res, azData, tt.mockQuery) require.NoError(t, err) frames, err := res.Dataframes.Decoded() @@ -496,14 +495,17 @@ func TestFindClosestAllowIntervalMS(t *testing.T) { } } -func loadTestFile(name string) (AzureMonitorResponse, error) { - var azData AzureMonitorResponse +func loadTestFile(t *testing.T, name string) AzureMonitorResponse { + t.Helper() path := filepath.Join("testdata", name) + // Ignore gosec warning G304 since it's a test + // nolint:gosec jsonBody, err := ioutil.ReadFile(path) - if err != nil { - return azData, err - } + require.NoError(t, err) + + var azData AzureMonitorResponse err = json.Unmarshal(jsonBody, &azData) - return azData, err + require.NoError(t, err) + return azData } diff --git a/pkg/tsdb/azuremonitor/insights-analytics-datasource.go b/pkg/tsdb/azuremonitor/insights-analytics-datasource.go index 829d75c323a..4070c6dc381 100644 --- a/pkg/tsdb/azuremonitor/insights-analytics-datasource.go +++ b/pkg/tsdb/azuremonitor/insights-analytics-datasource.go @@ -132,10 +132,14 @@ func (e *InsightsAnalyticsDatasource) executeQuery(ctx context.Context, query *I } body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() if err != nil { return queryResultError(err) } + defer func() { + if err := res.Body.Close(); err != nil { + azlog.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode/100 != 2 { azlog.Debug("Request failed", "status", res.Status, "body", string(body)) diff --git a/pkg/tsdb/cloudmonitoring/cloudmonitoring.go b/pkg/tsdb/cloudmonitoring/cloudmonitoring.go index 42ca088dfb5..a8386fc882a 100644 --- a/pkg/tsdb/cloudmonitoring/cloudmonitoring.go +++ b/pkg/tsdb/cloudmonitoring/cloudmonitoring.go @@ -482,10 +482,14 @@ func (e *CloudMonitoringExecutor) executeQuery(ctx context.Context, query *cloud func (e *CloudMonitoringExecutor) unmarshalResponse(res *http.Response) (cloudMonitoringResponse, error) { body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() if err != nil { return cloudMonitoringResponse{}, err } + defer func() { + if err := res.Body.Close(); err != nil { + slog.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode/100 != 2 { slog.Error("Request failed", "status", res.Status, "body", string(body)) diff --git a/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go b/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go index 4983e17964e..ce2f3da3e4f 100644 --- a/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go +++ b/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go @@ -989,6 +989,8 @@ func TestCloudMonitoring(t *testing.T) { func loadTestFile(path string) (cloudMonitoringResponse, error) { var data cloudMonitoringResponse + // Can ignore gosec warning G304 here since it's a test path + // nolint:gosec jsonBody, err := ioutil.ReadFile(path) if err != nil { return data, err diff --git a/pkg/tsdb/elasticsearch/client/client.go b/pkg/tsdb/elasticsearch/client/client.go index d0ebe7130e1..ee746cf1a41 100644 --- a/pkg/tsdb/elasticsearch/client/client.go +++ b/pkg/tsdb/elasticsearch/client/client.go @@ -226,7 +226,11 @@ func (c *baseClientImpl) ExecuteMultisearch(r *MultiSearchRequest) (*MultiSearch return nil, err } res := clientRes.httpResponse - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + clientLog.Warn("Failed to close response body", "err", err) + } + }() clientLog.Debug("Received multisearch response", "code", res.StatusCode, "status", res.Status, "content-length", res.ContentLength) diff --git a/pkg/tsdb/graphite/graphite.go b/pkg/tsdb/graphite/graphite.go index 2416889c726..61946aad038 100644 --- a/pkg/tsdb/graphite/graphite.go +++ b/pkg/tsdb/graphite/graphite.go @@ -133,10 +133,14 @@ func (e *GraphiteExecutor) Query(ctx context.Context, dsInfo *models.DataSource, func (e *GraphiteExecutor) parseResponse(res *http.Response) ([]TargetResponseDTO, error) { body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() if err != nil { return nil, err } + defer func() { + if err := res.Body.Close(); err != nil { + glog.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode/100 != 2 { glog.Info("Request failed", "status", res.Status, "body", string(body)) diff --git a/pkg/tsdb/influxdb/influxdb.go b/pkg/tsdb/influxdb/influxdb.go index b29cf2b0d70..685c0c0a495 100644 --- a/pkg/tsdb/influxdb/influxdb.go +++ b/pkg/tsdb/influxdb/influxdb.go @@ -82,10 +82,13 @@ func (e *InfluxDBExecutor) Query(ctx context.Context, dsInfo *models.DataSource, if err != nil { return nil, err } - - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + glog.Warn("Failed to close response body", "err", err) + } + }() if resp.StatusCode/100 != 2 { - return nil, fmt.Errorf("InfluxDB returned statuscode invalid status code: %s", resp.Status) + return nil, fmt.Errorf("InfluxDB returned error status: %s", resp.Status) } var response Response diff --git a/pkg/tsdb/opentsdb/opentsdb.go b/pkg/tsdb/opentsdb/opentsdb.go index 28b70029459..2edd0084ce9 100644 --- a/pkg/tsdb/opentsdb/opentsdb.go +++ b/pkg/tsdb/opentsdb/opentsdb.go @@ -110,10 +110,14 @@ func (e *OpenTsdbExecutor) parseResponse(query OpenTsdbQuery, res *http.Response queryRes := tsdb.NewQueryResult() body, err := ioutil.ReadAll(res.Body) - defer res.Body.Close() if err != nil { return nil, err } + defer func() { + if err := res.Body.Close(); err != nil { + plog.Warn("Failed to close response body", "err", err) + } + }() if res.StatusCode/100 != 2 { plog.Info("Request failed", "status", res.Status, "body", string(body)) diff --git a/pkg/tsdb/sqleng/sql_engine.go b/pkg/tsdb/sqleng/sql_engine.go index cbcbd95348f..7141e6f88a6 100644 --- a/pkg/tsdb/sqleng/sql_engine.go +++ b/pkg/tsdb/sqleng/sql_engine.go @@ -173,8 +173,11 @@ func (e *sqlQueryEndpoint) Query(ctx context.Context, dsInfo *models.DataSource, queryResult.Error = e.queryResultTransformer.TransformQueryError(err) return } - - defer rows.Close() + defer func() { + if err := rows.Close(); err != nil { + e.log.Warn("Failed to close rows", "err", err) + } + }() format := query.Model.Get("format").MustString("time_series") diff --git a/scripts/go/configs/.golangci.toml b/scripts/go/configs/.golangci.toml index 22270b25135..a914a147557 100644 --- a/scripts/go/configs/.golangci.toml +++ b/scripts/go/configs/.golangci.toml @@ -57,6 +57,9 @@ enable = [ # Disabled linters (might want them later) # "unparam" +[issues] +exclude-use-default = false + # Enable when appropriate # Poorly chosen identifier [[issues.exclude-rules]] @@ -112,3 +115,18 @@ text = "Unknwon` is a misspelling of `Unknown" [[issues.exclude-rules]] linters = ["errorlint"] text = "non-wrapping format verb for fmt.Errorf" + +# TODO: Enable +[[issues.exclude-rules]] +linters = ["stylecheck"] +text = "ST1000" + +# TODO: Enable +[[issues.exclude-rules]] +linters = ["stylecheck"] +text = "ST1020" + +# TODO: Enable +[[issues.exclude-rules]] +linters = ["stylecheck"] +text = "ST1021"