Chore: Disable default golangci-lint filter (#29751)

* Disable default golangci-lint filter

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

* Chore: Fix linter warnings

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

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)

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
 }

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"

pkg/infra/log/handlers.go

@@ -1,9 +1,9 @@
 package log
 
 type DisposableHandler interface {
-	Close()
+	Close() error
 }
 
 type ReloadableHandler interface {
-	Reload()
+	Reload() error
 }

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()))

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{

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
 }

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())
 }

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",

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

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")
 }

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():

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() {

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) {