diff --git a/.circleci/config.yml b/.circleci/config.yml index 30146576e63..3e95583ecae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -27,12 +27,13 @@ jobs: steps: - checkout - run: 'go get -u gopkg.in/alecthomas/gometalinter.v2' + - run: 'go get -u github.com/gordonklaus/ineffassign' - run: 'go get -u github.com/opennota/check/cmd/structcheck' - run: 'go get -u github.com/mdempsky/unconvert' - run: 'go get -u github.com/opennota/check/cmd/varcheck' - run: name: run linters - command: 'gometalinter.v2 --enable-gc --vendor --deadline 10m --disable-all --enable=structcheck --enable=unconvert --enable=varcheck ./...' + command: 'gometalinter.v2 --enable-gc --vendor --deadline 10m --disable-all --enable=ineffassign --enable=structcheck --enable=unconvert --enable=varcheck ./...' test-frontend: docker: diff --git a/pkg/cmd/grafana-server/main.go b/pkg/cmd/grafana-server/main.go index 777176f27da..da99bc9ba40 100644 --- a/pkg/cmd/grafana-server/main.go +++ b/pkg/cmd/grafana-server/main.go @@ -100,9 +100,9 @@ func main() { } func listenToSystemSignals(server *GrafanaServerImpl, shutdownCompleted chan int) { + var code int signalChan := make(chan os.Signal, 1) ignoreChan := make(chan os.Signal, 1) - code := 0 signal.Notify(ignoreChan, syscall.SIGHUP) signal.Notify(signalChan, os.Interrupt, os.Kill, syscall.SIGTERM) diff --git a/pkg/components/dashdiffs/compare.go b/pkg/components/dashdiffs/compare.go index d51011fbead..ae940091ed1 100644 --- a/pkg/components/dashdiffs/compare.go +++ b/pkg/components/dashdiffs/compare.go @@ -141,5 +141,9 @@ func getDiff(baseData, newData *simplejson.Json) (interface{}, diff.Diff, error) left := make(map[string]interface{}) err = json.Unmarshal(leftBytes, &left) + if err != nil { + return nil, nil, err + } + return left, jsonDiff, nil } diff --git a/pkg/components/dynmap/dynmap_test.go b/pkg/components/dynmap/dynmap_test.go index fa5f73c3719..62d356bd67d 100644 --- a/pkg/components/dynmap/dynmap_test.go +++ b/pkg/components/dynmap/dynmap_test.go @@ -60,6 +60,7 @@ func TestFirst(t *testing.T) { }` j, err := NewObjectFromBytes([]byte(testJSON)) + assert.True(err == nil, "failed to create new object from bytes") a, err := j.GetObject("address") assert.True(a != nil && err == nil, "failed to create json from string") @@ -108,6 +109,7 @@ func TestFirst(t *testing.T) { //log.Println("address: ", address) s, err = address.GetString("street") + assert.True(s == "Street 42" && err == nil, "street mismatching") addressAsString, err := j.GetString("address") assert.True(addressAsString == "" && err != nil, "address should not be an string") @@ -148,6 +150,7 @@ func TestFirst(t *testing.T) { //assert.True(element.IsObject() == true, "first fail") element, err := elementValue.Object() + assert.True(err == nil, "create element fail") s, err = element.GetString("street") assert.True(s == "Street 42" && err == nil, "second fail") @@ -232,6 +235,7 @@ func TestSecond(t *testing.T) { assert.True(fromName == "Tom Brady" && err == nil, "fromName mismatch") actions, err := dataItem.GetObjectArray("actions") + assert.True(err == nil, "get object from array failed") for index, action := range actions { diff --git a/pkg/components/imguploader/azureblobuploader_test.go b/pkg/components/imguploader/azureblobuploader_test.go index 570e105b321..ca978f70e3d 100644 --- a/pkg/components/imguploader/azureblobuploader_test.go +++ b/pkg/components/imguploader/azureblobuploader_test.go @@ -13,6 +13,7 @@ func TestUploadToAzureBlob(t *testing.T) { err := setting.NewConfigContext(&setting.CommandLineArgs{ HomePath: "../../../", }) + So(err, ShouldBeNil) uploader, _ := NewImageUploader() diff --git a/pkg/components/imguploader/imguploader_test.go b/pkg/components/imguploader/imguploader_test.go index b0311dac975..b272a45e7a5 100644 --- a/pkg/components/imguploader/imguploader_test.go +++ b/pkg/components/imguploader/imguploader_test.go @@ -19,6 +19,7 @@ func TestImageUploaderFactory(t *testing.T) { Convey("with bucket url https://foo.bar.baz.s3-us-east-2.amazonaws.com", func() { s3sec, err := setting.Cfg.GetSection("external_image_storage.s3") + So(err, ShouldBeNil) s3sec.NewKey("bucket_url", "https://foo.bar.baz.s3-us-east-2.amazonaws.com") s3sec.NewKey("access_key", "access_key") s3sec.NewKey("secret_key", "secret_key") @@ -37,6 +38,7 @@ func TestImageUploaderFactory(t *testing.T) { Convey("with bucket url https://s3.amazonaws.com/mybucket", func() { s3sec, err := setting.Cfg.GetSection("external_image_storage.s3") + So(err, ShouldBeNil) s3sec.NewKey("bucket_url", "https://s3.amazonaws.com/my.bucket.com") s3sec.NewKey("access_key", "access_key") s3sec.NewKey("secret_key", "secret_key") @@ -55,15 +57,15 @@ func TestImageUploaderFactory(t *testing.T) { Convey("with bucket url https://s3-us-west-2.amazonaws.com/mybucket", func() { s3sec, err := setting.Cfg.GetSection("external_image_storage.s3") + So(err, ShouldBeNil) s3sec.NewKey("bucket_url", "https://s3-us-west-2.amazonaws.com/my.bucket.com") s3sec.NewKey("access_key", "access_key") s3sec.NewKey("secret_key", "secret_key") uploader, err := NewImageUploader() - So(err, ShouldBeNil) - original, ok := uploader.(*S3Uploader) + original, ok := uploader.(*S3Uploader) So(ok, ShouldBeTrue) So(original.region, ShouldEqual, "us-west-2") So(original.bucket, ShouldEqual, "my.bucket.com") @@ -82,6 +84,7 @@ func TestImageUploaderFactory(t *testing.T) { setting.ImageUploadProvider = "webdav" webdavSec, err := setting.Cfg.GetSection("external_image_storage.webdav") + So(err, ShouldBeNil) webdavSec.NewKey("url", "webdavUrl") webdavSec.NewKey("username", "username") webdavSec.NewKey("password", "password") @@ -107,14 +110,14 @@ func TestImageUploaderFactory(t *testing.T) { setting.ImageUploadProvider = "gcs" gcpSec, err := setting.Cfg.GetSection("external_image_storage.gcs") + So(err, ShouldBeNil) gcpSec.NewKey("key_file", "/etc/secrets/project-79a52befa3f6.json") gcpSec.NewKey("bucket", "project-grafana-east") uploader, err := NewImageUploader() - So(err, ShouldBeNil) - original, ok := uploader.(*GCSUploader) + original, ok := uploader.(*GCSUploader) So(ok, ShouldBeTrue) So(original.keyFile, ShouldEqual, "/etc/secrets/project-79a52befa3f6.json") So(original.bucket, ShouldEqual, "project-grafana-east") @@ -128,15 +131,15 @@ func TestImageUploaderFactory(t *testing.T) { Convey("with container name", func() { azureBlobSec, err := setting.Cfg.GetSection("external_image_storage.azure_blob") + So(err, ShouldBeNil) azureBlobSec.NewKey("account_name", "account_name") azureBlobSec.NewKey("account_key", "account_key") azureBlobSec.NewKey("container_name", "container_name") uploader, err := NewImageUploader() - So(err, ShouldBeNil) - original, ok := uploader.(*AzureBlobUploader) + original, ok := uploader.(*AzureBlobUploader) So(ok, ShouldBeTrue) So(original.account_name, ShouldEqual, "account_name") So(original.account_key, ShouldEqual, "account_key") diff --git a/pkg/components/imguploader/webdavuploader.go b/pkg/components/imguploader/webdavuploader.go index 53d75247c76..f5478ea8a2f 100644 --- a/pkg/components/imguploader/webdavuploader.go +++ b/pkg/components/imguploader/webdavuploader.go @@ -41,14 +41,20 @@ func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error) url.Path = path.Join(url.Path, filename) imgData, err := ioutil.ReadFile(pa) + if err != nil { + return "", err + } + req, err := http.NewRequest("PUT", url.String(), bytes.NewReader(imgData)) + if err != nil { + return "", err + } if u.username != "" { req.SetBasicAuth(u.username, u.password) } res, err := netClient.Do(req) - if err != nil { return "", err } diff --git a/pkg/log/file_test.go b/pkg/log/file_test.go index 3e98e0786cc..97a3b8fe82f 100644 --- a/pkg/log/file_test.go +++ b/pkg/log/file_test.go @@ -32,7 +32,9 @@ func TestLogFile(t *testing.T) { Convey("Logging should add lines", func() { err := fileLogWrite.WriteLine("test1\n") + So(err, ShouldBeNil) err = fileLogWrite.WriteLine("test2\n") + So(err, ShouldBeNil) err = fileLogWrite.WriteLine("test3\n") So(err, ShouldBeNil) So(fileLogWrite.maxlines_curlines, ShouldEqual, 3) diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index 0b8efe808d5..1e62c68d7eb 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/pkg/services/alerting/notifiers/telegram.go @@ -3,13 +3,14 @@ package notifiers import ( "bytes" "fmt" + "io" + "mime/multipart" + "os" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" - "io" - "mime/multipart" - "os" ) const ( @@ -133,6 +134,9 @@ func (this *TelegramNotifier) buildMessageInlineImage(evalContext *alerting.Eval } ruleUrl, err := evalContext.GetRuleUrl() + if err != nil { + return nil, err + } metrics := generateMetricsMessage(evalContext) message := generateImageCaption(evalContext, ruleUrl, metrics) diff --git a/pkg/services/alerting/notifiers/victorops.go b/pkg/services/alerting/notifiers/victorops.go index 4b4db553cde..a753ca3cbf6 100644 --- a/pkg/services/alerting/notifiers/victorops.go +++ b/pkg/services/alerting/notifiers/victorops.go @@ -83,27 +83,6 @@ func (this *VictoropsNotifier) Notify(evalContext *alerting.EvalContext) error { return nil } - fields := make([]map[string]interface{}, 0) - fieldLimitCount := 4 - for index, evt := range evalContext.EvalMatches { - fields = append(fields, map[string]interface{}{ - "title": evt.Metric, - "value": evt.Value, - "short": true, - }) - if index > fieldLimitCount { - break - } - } - - if evalContext.Error != nil { - fields = append(fields, map[string]interface{}{ - "title": "Error message", - "value": evalContext.Error.Error(), - "short": false, - }) - } - messageType := evalContext.Rule.State if evalContext.Rule.State == models.AlertStateAlerting { // translate 'Alerting' to 'CRITICAL' (Victorops analog) messageType = AlertStateCritical diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index abf92ae0555..5e56b1d88c3 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -649,6 +649,9 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou } _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + if err != nil { + sc.reportFailure(tc, nil, err) + } sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index de0a49d34d9..7d4231deeae 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -235,7 +235,6 @@ func getOrCreateFolderId(cfg *DashboardsAsConfig, service dashboards.DashboardPr func resolveSymlink(fileinfo os.FileInfo, path string) (os.FileInfo, error) { checkFilepath, err := filepath.EvalSymlinks(path) if path != checkFilepath { - path = checkFilepath fi, err := os.Lstat(checkFilepath) if err != nil { return nil, err diff --git a/pkg/services/sqlstore/annotation_test.go b/pkg/services/sqlstore/annotation_test.go index 5af5f271993..949ed8135ba 100644 --- a/pkg/services/sqlstore/annotation_test.go +++ b/pkg/services/sqlstore/annotation_test.go @@ -256,6 +256,7 @@ func TestAnnotations(t *testing.T) { annotationId := items[0].Id err = repo.Delete(&annotations.DeleteParams{Id: annotationId}) + So(err, ShouldBeNil) items, err = repo.Find(query) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index c0848f08863..4238967417f 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -77,7 +77,7 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error { } parentVersion := dash.Version - affectedRows := int64(0) + var affectedRows int64 var err error if dash.Id == 0 { diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index c68ffd08cd7..a034a0565a3 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -154,6 +154,7 @@ func TestDashboardAclDataAccess(t *testing.T) { DashboardId: savedFolder.Id, Permission: m.PERMISSION_EDIT, }) + So(err, ShouldBeNil) q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} err = GetDashboardAclInfoList(q1) diff --git a/pkg/services/sqlstore/migrations/migrations_test.go b/pkg/services/sqlstore/migrations/migrations_test.go index 8e8e4af824f..53b398124af 100644 --- a/pkg/services/sqlstore/migrations/migrations_test.go +++ b/pkg/services/sqlstore/migrations/migrations_test.go @@ -27,7 +27,7 @@ func TestMigrations(t *testing.T) { sqlutil.CleanDB(x) - has, err := x.SQL(sql).Get(&r) + _, err = x.SQL(sql).Get(&r) So(err, ShouldNotBeNil) mg := NewMigrator(x) @@ -36,7 +36,7 @@ func TestMigrations(t *testing.T) { err = mg.Start() So(err, ShouldBeNil) - has, err = x.SQL(sql).Get(&r) + has, err := x.SQL(sql).Get(&r) So(err, ShouldBeNil) So(has, ShouldBeTrue) expectedMigrations := mg.MigrationsCount() - 2 //we currently skip to migrations. We should rewrite skipped migrations to write in the log as well. until then we have to keep this diff --git a/pkg/services/sqlstore/playlist.go b/pkg/services/sqlstore/playlist.go index 67720cbadb8..7b726880b9e 100644 --- a/pkg/services/sqlstore/playlist.go +++ b/pkg/services/sqlstore/playlist.go @@ -22,6 +22,9 @@ func CreatePlaylist(cmd *m.CreatePlaylistCommand) error { } _, err := x.Insert(&playlist) + if err != nil { + return err + } playlistItems := make([]m.PlaylistItem, 0) for _, item := range cmd.Items { diff --git a/pkg/services/sqlstore/plugin_setting.go b/pkg/services/sqlstore/plugin_setting.go index 312a3bda523..f694fbbd5f0 100644 --- a/pkg/services/sqlstore/plugin_setting.go +++ b/pkg/services/sqlstore/plugin_setting.go @@ -48,6 +48,9 @@ func UpdatePluginSetting(cmd *m.UpdatePluginSettingCmd) error { var pluginSetting m.PluginSetting exists, err := sess.Where("org_id=? and plugin_id=?", cmd.OrgId, cmd.PluginId).Get(&pluginSetting) + if err != nil { + return err + } sess.UseBool("enabled") sess.UseBool("pinned") if !exists { diff --git a/pkg/services/sqlstore/preferences.go b/pkg/services/sqlstore/preferences.go index 399b23f3ffa..a070fa621b5 100644 --- a/pkg/services/sqlstore/preferences.go +++ b/pkg/services/sqlstore/preferences.go @@ -72,6 +72,9 @@ func SavePreferences(cmd *m.SavePreferencesCommand) error { var prefs m.Preferences exists, err := sess.Where("org_id=? AND user_id=?", cmd.OrgId, cmd.UserId).Get(&prefs) + if err != nil { + return err + } if !exists { prefs = m.Preferences{ diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index f136411eeba..f4b022906da 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -74,6 +74,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { Convey("Should be able to return all teams a user is member of", func() { groupId := group2.Result.Id err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[0]}) + So(err, ShouldBeNil) query := &m.GetTeamsByUserQuery{OrgId: testOrgId, UserId: userIds[0]} err = GetTeamsByUser(query) @@ -103,7 +104,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[2]}) So(err, ShouldBeNil) err = testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId}) - + So(err, ShouldBeNil) err = DeleteTeam(&m.DeleteTeamCommand{OrgId: testOrgId, Id: groupId}) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 1546fb83b21..5e2efbd7fde 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -168,11 +168,9 @@ func GetUserByLogin(query *m.GetUserByLoginQuery) error { return m.ErrUserNotFound } - user := new(m.User) - // Try and find the user by login first. // It's not sufficient to assume that a LoginOrEmail with an "@" is an email. - user = &m.User{Login: query.LoginOrEmail} + user := &m.User{Login: query.LoginOrEmail} has, err := x.Get(user) if err != nil { @@ -202,9 +200,7 @@ func GetUserByEmail(query *m.GetUserByEmailQuery) error { return m.ErrUserNotFound } - user := new(m.User) - - user = &m.User{Email: query.Email} + user := &m.User{Email: query.Email} has, err := x.Get(user) if err != nil {