Chore: Start harmonizing linting with plugin SDK (#25854)

* Chore: Harmonize linting with plugin SDK

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Chore: Fix linting issues

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

.circleci/config.yml

@@ -825,10 +825,10 @@ jobs:
           name: Lint Go
           command: |
             # To save memory, run in two batches
-            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.yml -E vet -E deadcode -E gofmt \
-              -E gosimple -E ineffassign -E structcheck -E typecheck ./pkg/...
-            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.yml -E unconvert -E unused \
-              -E varcheck -E goconst -E errcheck -E staticcheck ./pkg/...
+            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.toml -E deadcode -E depguard -E dogsled \
+              -E errcheck -E goconst -E golint -E gosec -E gosimple -E govet ./pkg/...
+            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.toml -E ineffassign \
+              -E rowserrcheck -E staticcheck -E structcheck -E typecheck -E unconvert -E unused -E varcheck ./pkg/...
             ./scripts/go/bin/revive -formatter stylish -config ./scripts/go/configs/revive.toml ./pkg/...
             ./scripts/go/bin/revive -formatter stylish -config ./scripts/go/configs/revive-strict.toml \
               -exclude ./pkg/plugins/backendplugin/pluginextensionv2/... \

Makefile

@@ -99,7 +99,7 @@ scripts/go/bin/golangci-lint: scripts/go/go.mod
 golangci-lint: scripts/go/bin/golangci-lint
 	@echo "lint via golangci-lint"
 	@scripts/go/bin/golangci-lint run \
-		--config ./scripts/go/configs/.golangci.yml \
+		--config ./scripts/go/configs/.golangci.toml \
 		$(GO_FILES)
 
 lint-go: golangci-lint revive revive-strict # Run all code checks for backend.

pkg/api/basic_auth.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"crypto/subtle"
+
 	macaron "gopkg.in/macaron.v1"
 )
 

pkg/api/search.go

@@ -1,10 +1,11 @@
 package api
 
 import (
-	"github.com/grafana/grafana/pkg/util"
 	"net/http"
 	"strconv"
 
+	"github.com/grafana/grafana/pkg/util"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/models"

pkg/api/team_test.go

@@ -9,11 +9,12 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 
+	"net/http"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	. "github.com/smartystreets/goconvey/convey"
 	"github.com/stretchr/testify/assert"
 	macaron "gopkg.in/macaron.v1"
-	"net/http"
 )
 
 type testLogger struct {

pkg/cmd/grafana-cli/commands/commandstest/context.go

@@ -2,6 +2,7 @@ package commandstest
 
 import (
 	"flag"
+
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/utils"
 	"github.com/urfave/cli/v2"
 )

pkg/cmd/grafana-cli/commands/install_command.go

@@ -308,7 +308,7 @@ func extractFile(file *zip.File, filePath string) (err error) {
 	}()
 
 	_, err = io.Copy(dst, src)
-	return
+	return err
 }
 
 // isPathSafe checks if the filePath does not resolve outside of destination. This is used to prevent

pkg/cmd/grafana-cli/services/api_client_test.go

@@ -8,43 +8,56 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestHandleResponse(t *testing.T) {
 	t.Run("Returns body if status == 200", func(t *testing.T) {
-		bodyReader, err := handleResponse(makeResponse(200, "test"))
-		assert.NoError(t, err)
+		resp := makeResponse(200, "test")
+		defer resp.Body.Close()
+		bodyReader, err := handleResponse(resp)
+		require.NoError(t, err)
 		body, err := ioutil.ReadAll(bodyReader)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, "test", string(body))
 	})
 
 	t.Run("Returns ErrorNotFound if status == 404", func(t *testing.T) {
-		_, err := handleResponse(makeResponse(404, ""))
+		resp := makeResponse(404, "")
+		defer resp.Body.Close()
+		_, err := handleResponse(resp)
 		assert.Equal(t, ErrNotFoundError, err)
 	})
 
 	t.Run("Returns message from body if status == 400", func(t *testing.T) {
-		_, err := handleResponse(makeResponse(400, "{ \"message\": \"error_message\" }"))
-		assert.Error(t, err)
+		resp := makeResponse(400, "{ \"message\": \"error_message\" }")
+		defer resp.Body.Close()
+		_, 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) {
-		_, err := handleResponse(makeResponse(400, "{ \"test\": \"test_message\"}"))
-		assert.Error(t, err)
+		resp := makeResponse(400, "{ \"test\": \"test_message\"}")
+		defer resp.Body.Close()
+		_, 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) {
-		_, err := handleResponse(makeResponse(400, ""))
-		assert.Error(t, err)
+		resp := makeResponse(400, "")
+		defer resp.Body.Close()
+		_, err := handleResponse(resp)
+		require.Error(t, err)
 		_ = asBadRequestError(t, err)
 	})
 
 	t.Run("Returns error with invalid status if status == 500", func(t *testing.T) {
-		_, err := handleResponse(makeResponse(500, ""))
-		assert.Error(t, err)
+		resp := makeResponse(500, "")
+		defer resp.Body.Close()
+		_, err := handleResponse(resp)
+		require.Error(t, err)
 		assert.Contains(t, err.Error(), "invalid status")
 	})
 }

pkg/cmd/grafana-server/server.go

@@ -157,7 +157,7 @@ func (s *Server) Run() (err error) {
 
 	s.notifySystemd("READY=1")
 
-	return
+	return err
 }
 
 func (s *Server) Shutdown(reason string) {

pkg/components/imguploader/gcsuploader.go

@@ -89,6 +89,7 @@ func (u *GCSUploader) uploadFile(client *http.Client, imageDiskPath, key string)
 	if err != nil {
 		return err
 	}
+	resp.Body.Close()
 
 	if resp.StatusCode != 200 {
 		return fmt.Errorf("GCS response status code %d", resp.StatusCode)

pkg/components/imguploader/webdavuploader.go

@@ -75,6 +75,7 @@ func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error)
 	if err != nil {
 		return "", err
 	}
+	defer res.Body.Close()
 
 	if res.StatusCode != http.StatusCreated {
 		body, _ := ioutil.ReadAll(res.Body)

pkg/infra/fs/exists_test.go

@@ -1,10 +1,11 @@
 package fs
 
 import (
-	"github.com/stretchr/testify/require"
 	"io/ioutil"
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestExists_NonExistent(t *testing.T) {

pkg/infra/tracing/tracing_test.go

@@ -1,10 +1,11 @@
 package tracing
 
 import (
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGroupSplit(t *testing.T) {

pkg/infra/usagestats/usage_stats.go

@@ -181,9 +181,12 @@ func (uss *UsageStatsService) sendUsageStats(oauthProviders map[string]bool) {
 
 	client := http.Client{Timeout: 5 * time.Second}
 	go func() {
-		if _, err := client.Post(usageStatsURL, "application/json", data); err != nil {
+		resp, err := client.Post(usageStatsURL, "application/json", data)
+		if err != nil {
 			metricsLogger.Error("Failed to send usage stats", "err", err)
+			return
 		}
+		resp.Body.Close()
 	}()
 }
 

pkg/middleware/dashboard_redirect_test.go

@@ -34,7 +34,10 @@ func TestMiddlewareDashboardRedirect(t *testing.T) {
 
 			Convey("Should redirect to new dashboard url with a 301 Moved Permanently", func() {
 				So(sc.resp.Code, ShouldEqual, 301)
-				redirectURL, _ := sc.resp.Result().Location()
+				resp := sc.resp.Result()
+				defer resp.Body.Close()
+				redirectURL, err := resp.Location()
+				So(err, ShouldBeNil)
 				So(redirectURL.Path, ShouldEqual, models.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug))
 				So(len(redirectURL.Query()), ShouldEqual, 2)
 			})
@@ -47,7 +50,10 @@ func TestMiddlewareDashboardRedirect(t *testing.T) {
 
 			Convey("Should redirect to new dashboard url with a 301 Moved Permanently", func() {
 				So(sc.resp.Code, ShouldEqual, 301)
-				redirectURL, _ := sc.resp.Result().Location()
+				resp := sc.resp.Result()
+				defer resp.Body.Close()
+				redirectURL, err := resp.Location()
+				So(err, ShouldBeNil)
 				expectedURL := models.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug)
 				expectedURL = strings.Replace(expectedURL, "/d/", "/d-solo/", 1)
 				So(redirectURL.Path, ShouldEqual, expectedURL)
@@ -66,7 +72,10 @@ func TestMiddlewareDashboardRedirect(t *testing.T) {
 
 			Convey("Should redirect to new dashboard edit url with a 301 Moved Permanently", func() {
 				So(sc.resp.Code, ShouldEqual, 301)
-				redirectURL, _ := sc.resp.Result().Location()
+				resp := sc.resp.Result()
+				defer resp.Body.Close()
+				redirectURL, err := resp.Location()
+				So(err, ShouldBeNil)
 				So(redirectURL.String(), ShouldEqual, "/d/asd/d/asd/dash?editPanel=12&orgId=1")
 			})
 		})

pkg/middleware/middleware_test.go

@@ -623,7 +623,9 @@ func TestTokenRotationAtEndOfRequest(t *testing.T) {
 	rotateEndOfRequestFunc(reqContext, uts, token)(reqContext.Resp)
 
 	foundLoginCookie := false
-	for _, c := range rr.Result().Cookies() {
+	resp := rr.Result()
+	defer resp.Body.Close()
+	for _, c := range resp.Cookies() {
 		if c.Name == "login_token" {
 			foundLoginCookie = true
 

pkg/models/datasource_cache_test.go

@@ -224,13 +224,10 @@ func TestDataSourceProxyCache(t *testing.T) {
 			// 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)
-			if err != nil {
-				log.Fatal(err.Error())
-			}
+			So(err, ShouldBeNil)
+			defer res.Body.Close()
 			body, err := ioutil.ReadAll(res.Body)
-			if err != nil {
-				log.Fatal(err.Error())
-			}
+			So(err, ShouldBeNil)
 			bodyStr := string(body)
 			So(bodyStr, ShouldEqual, "Ok")
 		})

pkg/models/quotas.go

@@ -2,8 +2,9 @@ package models
 
 import (
 	"errors"
-	"github.com/grafana/grafana/pkg/setting"
 	"time"
+
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 var ErrInvalidQuotaTarget = errors.New("Invalid quota target")

pkg/services/cleanup/cleanup_test.go

@@ -1,10 +1,11 @@
 package cleanup
 
 import (
-	"github.com/grafana/grafana/pkg/setting"
-	. "github.com/smartystreets/goconvey/convey"
 	"testing"
 	"time"
+
+	"github.com/grafana/grafana/pkg/setting"
+	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestCleanUpTmpFiles(t *testing.T) {

pkg/services/dashboards/acl_service.go

@@ -1,9 +1,10 @@
 package dashboards
 
 import (
+	"time"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
-	"time"
 )
 
 func MakeUserAdmin(bus bus.Bus, orgId int64, userId int64, dashboardId int64, setViewAndEditPermissions bool) error {

pkg/services/dashboards/dashboard_service_test.go

@@ -1,9 +1,10 @@
 package dashboards
 
 import (
+	"testing"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/setting"
-	"testing"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"

pkg/services/sqlstore/datasource.go

@@ -1,10 +1,11 @@
 package sqlstore
 
 import (
-	"github.com/grafana/grafana/pkg/util/errutil"
 	"strings"
 	"time"
 
+	"github.com/grafana/grafana/pkg/util/errutil"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 

pkg/services/sqlstore/migrator/postgres_dialect.go

@@ -2,10 +2,11 @@ package migrator
 
 import (
 	"fmt"
-	"github.com/lib/pq"
 	"strconv"
 	"strings"
 
+	"github.com/lib/pq"
+
 	"github.com/grafana/grafana/pkg/util/errutil"
 	"xorm.io/xorm"
 )

pkg/services/sqlstore/migrator/sqlite_dialect.go

@@ -2,6 +2,7 @@ package migrator
 
 import (
 	"fmt"
+
 	"github.com/mattn/go-sqlite3"
 	"xorm.io/xorm"
 )

pkg/services/sqlstore/permissions/dashboard.go

@@ -1,9 +1,10 @@
 package permissions
 
 import (
+	"strings"
+
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
-	"strings"
 )
 
 type DashboardPermissionFilter struct {

pkg/services/sqlstore/searchstore/filters.go

@@ -2,8 +2,9 @@ package searchstore
 
 import (
 	"fmt"
-	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 	"strings"
+
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 )
 
 // FilterWhere limits the set of dashboard IDs to the dashboards for