From db8d7ffbf67bc2782411e149e14a05c85e47c31f Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Tue, 30 Jun 2020 22:46:41 +0200 Subject: [PATCH] Linting fixes for the provisioning package (#25690) --- .circleci/config.yml | 2 + Makefile | 2 + .../notifiers/alert_notifications.go | 44 +++++++------- .../provisioning/notifiers/config_reader.go | 22 +++---- .../notifiers/config_reader_test.go | 60 +++++++++---------- pkg/services/provisioning/notifiers/types.go | 26 ++++---- pkg/services/provisioning/values/values.go | 24 ++++++++ 7 files changed, 102 insertions(+), 78 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3a2aeb887c3..094af179966 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -835,6 +835,8 @@ jobs: ./pkg/services/alerting/... \ ./pkg/services/provisioning/datasources/... \ ./pkg/services/provisioning/dashboards/... \ + ./pkg/services/provisioning/notifiers/... \ + ./pkg/services/provisioning/values/... \ ./pkg/plugins/backendplugin/... test-frontend: diff --git a/Makefile b/Makefile index 1e8ca599b2e..d1f005e7d5c 100644 --- a/Makefile +++ b/Makefile @@ -90,6 +90,8 @@ revive-strict: scripts/go/bin/revive ./pkg/services/alerting/... \ ./pkg/services/provisioning/datasources/... \ ./pkg/services/provisioning/dashboards/... \ + ./pkg/services/provisioning/notifiers/... \ + ./pkg/services/provisioning/values/... \ ./pkg/plugins/backendplugin/... scripts/go/bin/golangci-lint: scripts/go/go.mod diff --git a/pkg/services/provisioning/notifiers/alert_notifications.go b/pkg/services/provisioning/notifiers/alert_notifications.go index 865e215e38b..e032d8e80dc 100644 --- a/pkg/services/provisioning/notifiers/alert_notifications.go +++ b/pkg/services/provisioning/notifiers/alert_notifications.go @@ -1,22 +1,18 @@ package notifiers import ( - "errors" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" ) -var ( - ErrInvalidConfigTooManyDefault = errors.New("Alert notification provisioning config is invalid. Only one alert notification can be marked as default") -) - +// Provision alert notifiers func Provision(configDirectory string) error { dc := newNotificationProvisioner(log.New("provisioning.notifiers")) return dc.applyChanges(configDirectory) } +// NotificationProvisioner is responsible for provsioning alert notifiers type NotificationProvisioner struct { log log.Logger cfgProvider *configReader @@ -43,19 +39,19 @@ func (dc *NotificationProvisioner) apply(cfg *notificationsAsConfig) error { func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*deleteNotificationConfig) error { for _, notification := range notificationToDelete { - dc.log.Info("Deleting alert notification", "name", notification.Name, "uid", notification.Uid) + dc.log.Info("Deleting alert notification", "name", notification.Name, "uid", notification.UID) - if notification.OrgId == 0 && notification.OrgName != "" { + if notification.OrgID == 0 && notification.OrgName != "" { getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} if err := bus.Dispatch(getOrg); err != nil { return err } - notification.OrgId = getOrg.Result.Id - } else if notification.OrgId < 0 { - notification.OrgId = 1 + notification.OrgID = getOrg.Result.Id + } else if notification.OrgID < 0 { + notification.OrgID = 1 } - getNotification := &models.GetAlertNotificationsWithUidQuery{Uid: notification.Uid, OrgId: notification.OrgId} + getNotification := &models.GetAlertNotificationsWithUidQuery{Uid: notification.UID, OrgId: notification.OrgID} if err := bus.Dispatch(getNotification); err != nil { return err @@ -75,31 +71,31 @@ func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*d func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*notificationFromConfig) error { for _, notification := range notificationToMerge { - if notification.OrgId == 0 && notification.OrgName != "" { + if notification.OrgID == 0 && notification.OrgName != "" { getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} if err := bus.Dispatch(getOrg); err != nil { return err } - notification.OrgId = getOrg.Result.Id - } else if notification.OrgId < 0 { - notification.OrgId = 1 + notification.OrgID = getOrg.Result.Id + } else if notification.OrgID < 0 { + notification.OrgID = 1 } - cmd := &models.GetAlertNotificationsWithUidQuery{OrgId: notification.OrgId, Uid: notification.Uid} + cmd := &models.GetAlertNotificationsWithUidQuery{OrgId: notification.OrgID, Uid: notification.UID} err := bus.Dispatch(cmd) if err != nil { return err } if cmd.Result == nil { - dc.log.Debug("inserting alert notification from configuration", "name", notification.Name, "uid", notification.Uid) + dc.log.Debug("inserting alert notification from configuration", "name", notification.Name, "uid", notification.UID) insertCmd := &models.CreateAlertNotificationCommand{ - Uid: notification.Uid, + Uid: notification.UID, Name: notification.Name, Type: notification.Type, IsDefault: notification.IsDefault, - Settings: notification.SettingsToJson(), - OrgId: notification.OrgId, + Settings: notification.SettingsToJSON(), + OrgId: notification.OrgID, DisableResolveMessage: notification.DisableResolveMessage, Frequency: notification.Frequency, SendReminder: notification.SendReminder, @@ -111,12 +107,12 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not } else { dc.log.Debug("updating alert notification from configuration", "name", notification.Name) updateCmd := &models.UpdateAlertNotificationWithUidCommand{ - Uid: notification.Uid, + Uid: notification.UID, Name: notification.Name, Type: notification.Type, IsDefault: notification.IsDefault, - Settings: notification.SettingsToJson(), - OrgId: notification.OrgId, + Settings: notification.SettingsToJSON(), + OrgId: notification.OrgID, DisableResolveMessage: notification.DisableResolveMessage, Frequency: notification.Frequency, SendReminder: notification.SendReminder, diff --git a/pkg/services/provisioning/notifiers/config_reader.go b/pkg/services/provisioning/notifiers/config_reader.go index e975dbb77d4..338728df9f2 100644 --- a/pkg/services/provisioning/notifiers/config_reader.go +++ b/pkg/services/provisioning/notifiers/config_reader.go @@ -46,7 +46,7 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error return nil, err } - checkOrgIdAndOrgName(notifications) + checkOrgIDAndOrgName(notifications) err = validateNotifications(notifications) if err != nil { @@ -72,24 +72,24 @@ func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) ( return cfg.mapToNotificationFromConfig(), nil } -func checkOrgIdAndOrgName(notifications []*notificationsAsConfig) { +func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) { for i := range notifications { for _, notification := range notifications[i].Notifications { - if notification.OrgId < 1 { + if notification.OrgID < 1 { if notification.OrgName == "" { - notification.OrgId = 1 + notification.OrgID = 1 } else { - notification.OrgId = 0 + notification.OrgID = 0 } } } for _, notification := range notifications[i].DeleteNotifications { - if notification.OrgId < 1 { + if notification.OrgID < 1 { if notification.OrgName == "" { - notification.OrgId = 1 + notification.OrgID = 1 } else { - notification.OrgId = 0 + notification.OrgID = 0 } } } @@ -107,7 +107,7 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { ) } - if notification.Uid == "" { + if notification.UID == "" { errStrings = append( errStrings, fmt.Sprintf("Added alert notification item %d in configuration doesn't contain required field uid", index+1), @@ -123,7 +123,7 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { ) } - if notification.Uid == "" { + if notification.UID == "" { errStrings = append( errStrings, fmt.Sprintf("Deleted alert notification item %d in configuration doesn't contain required field uid", index+1), @@ -149,7 +149,7 @@ func validateNotifications(notifications []*notificationsAsConfig) error { for _, notification := range notifications[i].Notifications { _, err := alerting.InitNotifier(&models.AlertNotification{ Name: notification.Name, - Settings: notification.SettingsToJson(), + Settings: notification.SettingsToJSON(), Type: notification.Type, }) diff --git a/pkg/services/provisioning/notifiers/config_reader_test.go b/pkg/services/provisioning/notifiers/config_reader_test.go index f7a654bf758..e59da75d955 100644 --- a/pkg/services/provisioning/notifiers/config_reader_test.go +++ b/pkg/services/provisioning/notifiers/config_reader_test.go @@ -13,16 +13,16 @@ import ( ) var ( - correct_properties = "./testdata/test-configs/correct-properties" - incorrect_settings = "./testdata/test-configs/incorrect-settings" - no_required_fields = "./testdata/test-configs/no-required-fields" - correct_properties_with_orgName = "./testdata/test-configs/correct-properties-with-orgName" - brokenYaml = "./testdata/test-configs/broken-yaml" - doubleNotificationsConfig = "./testdata/test-configs/double-default" - emptyFolder = "./testdata/test-configs/empty_folder" - emptyFile = "./testdata/test-configs/empty" - twoNotificationsConfig = "./testdata/test-configs/two-notifications" - unknownNotifier = "./testdata/test-configs/unknown-notifier" + correctProperties = "./testdata/test-configs/correct-properties" + incorrectSettings = "./testdata/test-configs/incorrect-settings" + noRequiredFields = "./testdata/test-configs/no-required-fields" + correctPropertiesWithOrgName = "./testdata/test-configs/correct-properties-with-orgName" + brokenYaml = "./testdata/test-configs/broken-yaml" + doubleNotificationsConfig = "./testdata/test-configs/double-default" + emptyFolder = "./testdata/test-configs/empty_folder" + emptyFile = "./testdata/test-configs/empty" + twoNotificationsConfig = "./testdata/test-configs/two-notifications" + unknownNotifier = "./testdata/test-configs/unknown-notifier" ) func TestNotificationAsConfig(t *testing.T) { @@ -46,7 +46,7 @@ func TestNotificationAsConfig(t *testing.T) { Convey("Can read correct properties", func() { _ = os.Setenv("TEST_VAR", "default") cfgProvider := &configReader{log: log.New("test logger")} - cfg, err := cfgProvider.readConfig(correct_properties) + cfg, err := cfgProvider.readConfig(correctProperties) _ = os.Unsetenv("TEST_VAR") if err != nil { t.Fatalf("readConfig return an error %v", err) @@ -60,8 +60,8 @@ func TestNotificationAsConfig(t *testing.T) { nt := nts[0] So(nt.Name, ShouldEqual, "default-slack-notification") So(nt.Type, ShouldEqual, "slack") - So(nt.OrgId, ShouldEqual, 2) - So(nt.Uid, ShouldEqual, "notifier1") + So(nt.OrgID, ShouldEqual, 2) + So(nt.UID, ShouldEqual, "notifier1") So(nt.IsDefault, ShouldBeTrue) So(nt.Settings, ShouldResemble, map[string]interface{}{ "recipient": "XXX", "token": "xoxb", "uploadImage": true, "url": "https://slack.com", @@ -72,45 +72,45 @@ func TestNotificationAsConfig(t *testing.T) { nt = nts[1] So(nt.Name, ShouldEqual, "another-not-default-notification") So(nt.Type, ShouldEqual, "email") - So(nt.OrgId, ShouldEqual, 3) - So(nt.Uid, ShouldEqual, "notifier2") + So(nt.OrgID, ShouldEqual, 3) + So(nt.UID, ShouldEqual, "notifier2") So(nt.IsDefault, ShouldBeFalse) nt = nts[2] So(nt.Name, ShouldEqual, "check-unset-is_default-is-false") So(nt.Type, ShouldEqual, "slack") - So(nt.OrgId, ShouldEqual, 3) - So(nt.Uid, ShouldEqual, "notifier3") + So(nt.OrgID, ShouldEqual, 3) + So(nt.UID, ShouldEqual, "notifier3") So(nt.IsDefault, ShouldBeFalse) nt = nts[3] So(nt.Name, ShouldEqual, "Added notification with whitespaces in name") So(nt.Type, ShouldEqual, "email") - So(nt.Uid, ShouldEqual, "notifier4") - So(nt.OrgId, ShouldEqual, 3) + So(nt.UID, ShouldEqual, "notifier4") + So(nt.OrgID, ShouldEqual, 3) deleteNts := ntCfg.DeleteNotifications So(len(deleteNts), ShouldEqual, 4) deleteNt := deleteNts[0] So(deleteNt.Name, ShouldEqual, "default-slack-notification") - So(deleteNt.Uid, ShouldEqual, "notifier1") - So(deleteNt.OrgId, ShouldEqual, 2) + So(deleteNt.UID, ShouldEqual, "notifier1") + So(deleteNt.OrgID, ShouldEqual, 2) deleteNt = deleteNts[1] So(deleteNt.Name, ShouldEqual, "deleted-notification-without-orgId") - So(deleteNt.OrgId, ShouldEqual, 1) - So(deleteNt.Uid, ShouldEqual, "notifier2") + So(deleteNt.OrgID, ShouldEqual, 1) + So(deleteNt.UID, ShouldEqual, "notifier2") deleteNt = deleteNts[2] So(deleteNt.Name, ShouldEqual, "deleted-notification-with-0-orgId") - So(deleteNt.OrgId, ShouldEqual, 1) - So(deleteNt.Uid, ShouldEqual, "notifier3") + So(deleteNt.OrgID, ShouldEqual, 1) + So(deleteNt.UID, ShouldEqual, "notifier3") deleteNt = deleteNts[3] So(deleteNt.Name, ShouldEqual, "Deleted notification with whitespaces in name") - So(deleteNt.OrgId, ShouldEqual, 1) - So(deleteNt.Uid, ShouldEqual, "notifier4") + So(deleteNt.OrgID, ShouldEqual, 1) + So(deleteNt.UID, ShouldEqual, "notifier4") }) Convey("One configured notification", func() { @@ -243,7 +243,7 @@ func TestNotificationAsConfig(t *testing.T) { So(err, ShouldBeNil) dc := newNotificationProvisioner(logger) - err = dc.applyChanges(correct_properties_with_orgName) + err = dc.applyChanges(correctPropertiesWithOrgName) if err != nil { t.Fatalf("applyChanges return an error %v", err) } @@ -262,7 +262,7 @@ func TestNotificationAsConfig(t *testing.T) { Convey("Config doesn't contain required field", func() { dc := newNotificationProvisioner(logger) - err := dc.applyChanges(no_required_fields) + err := dc.applyChanges(noRequiredFields) So(err, ShouldNotBeNil) errString := err.Error() @@ -310,7 +310,7 @@ func TestNotificationAsConfig(t *testing.T) { Convey("Read incorrect properties", func() { cfgProvider := &configReader{log: log.New("test logger")} - _, err := cfgProvider.readConfig(incorrect_settings) + _, err := cfgProvider.readConfig(incorrectSettings) So(err, ShouldNotBeNil) So(err.Error(), ShouldEqual, "Alert validation error: Could not find url property in settings") }) diff --git a/pkg/services/provisioning/notifiers/types.go b/pkg/services/provisioning/notifiers/types.go index 48ff9a6ce1d..640106a8750 100644 --- a/pkg/services/provisioning/notifiers/types.go +++ b/pkg/services/provisioning/notifiers/types.go @@ -13,15 +13,15 @@ type notificationsAsConfig struct { } type deleteNotificationConfig struct { - Uid string + UID string Name string - OrgId int64 + OrgID int64 OrgName string } type notificationFromConfig struct { - Uid string - OrgId int64 + UID string + OrgID int64 OrgName string Name string Type string @@ -39,15 +39,15 @@ type notificationsAsConfigV0 struct { } type deleteNotificationConfigV0 struct { - Uid values.StringValue `json:"uid" yaml:"uid"` + UID values.StringValue `json:"uid" yaml:"uid"` Name values.StringValue `json:"name" yaml:"name"` - OrgId values.Int64Value `json:"org_id" yaml:"org_id"` + OrgID values.Int64Value `json:"org_id" yaml:"org_id"` OrgName values.StringValue `json:"org_name" yaml:"org_name"` } type notificationFromConfigV0 struct { - Uid values.StringValue `json:"uid" yaml:"uid"` - OrgId values.Int64Value `json:"org_id" yaml:"org_id"` + UID values.StringValue `json:"uid" yaml:"uid"` + OrgID values.Int64Value `json:"org_id" yaml:"org_id"` OrgName values.StringValue `json:"org_name" yaml:"org_name"` Name values.StringValue `json:"name" yaml:"name"` Type values.StringValue `json:"type" yaml:"type"` @@ -58,7 +58,7 @@ type notificationFromConfigV0 struct { Settings values.JSONValue `json:"settings" yaml:"settings"` } -func (notification notificationFromConfig) SettingsToJson() *simplejson.Json { +func (notification notificationFromConfig) SettingsToJSON() *simplejson.Json { settings := simplejson.New() if len(notification.Settings) > 0 { for k, v := range notification.Settings { @@ -78,8 +78,8 @@ func (cfg *notificationsAsConfigV0) mapToNotificationFromConfig() *notifications for _, notification := range cfg.Notifications { r.Notifications = append(r.Notifications, ¬ificationFromConfig{ - Uid: notification.Uid.Value(), - OrgId: notification.OrgId.Value(), + UID: notification.UID.Value(), + OrgID: notification.OrgID.Value(), OrgName: notification.OrgName.Value(), Name: notification.Name.Value(), Type: notification.Type.Value(), @@ -93,8 +93,8 @@ func (cfg *notificationsAsConfigV0) mapToNotificationFromConfig() *notifications for _, notification := range cfg.DeleteNotifications { r.DeleteNotifications = append(r.DeleteNotifications, &deleteNotificationConfig{ - Uid: notification.Uid.Value(), - OrgId: notification.OrgId.Value(), + UID: notification.UID.Value(), + OrgID: notification.OrgID.Value(), OrgName: notification.OrgName.Value(), Name: notification.Name.Value(), }) diff --git a/pkg/services/provisioning/values/values.go b/pkg/services/provisioning/values/values.go index d2f890894e4..deb6e7c8146 100644 --- a/pkg/services/provisioning/values/values.go +++ b/pkg/services/provisioning/values/values.go @@ -22,11 +22,14 @@ import ( "github.com/grafana/grafana/pkg/util/errutil" ) +// IntValue represents a string value in a YAML +// config that can be overridden by environment variables type IntValue struct { value int Raw string } +// UnmarshalYAML converts YAML into an *IntValue func (val *IntValue) UnmarshalYAML(unmarshal func(interface{}) error) error { interpolated, err := getInterpolated(unmarshal) if err != nil { @@ -41,15 +44,19 @@ func (val *IntValue) UnmarshalYAML(unmarshal func(interface{}) error) error { return errutil.Wrap("cannot convert value int", err) } +// Value returns the wrapped int value func (val *IntValue) Value() int { return val.value } +// Int64Value represents a string value in a YAML +// config that can be overridden by environment variables type Int64Value struct { value int64 Raw string } +// UnmarshalYAML converts YAML into an *Int64Value func (val *Int64Value) UnmarshalYAML(unmarshal func(interface{}) error) error { interpolated, err := getInterpolated(unmarshal) if err != nil { @@ -64,15 +71,19 @@ func (val *Int64Value) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } +// Value returns the wrapped int64 value func (val *Int64Value) Value() int64 { return val.value } +// StringValue represents a string value in a YAML +// config that can be overridden by environment variables type StringValue struct { value string Raw string } +// UnmarshalYAML converts YAML into an *StringValue func (val *StringValue) UnmarshalYAML(unmarshal func(interface{}) error) error { interpolated, err := getInterpolated(unmarshal) if err != nil { @@ -83,15 +94,19 @@ func (val *StringValue) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } +// Value returns the wrapped string value func (val *StringValue) Value() string { return val.value } +// BoolValue represents a string value in a YAML +// config that can be overridden by environment variables type BoolValue struct { value bool Raw string } +// UnmarshalYAML converts YAML into an *BoolValue func (val *BoolValue) UnmarshalYAML(unmarshal func(interface{}) error) error { interpolated, err := getInterpolated(unmarshal) if err != nil { @@ -102,15 +117,19 @@ func (val *BoolValue) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } +// Value returns the wrapped bool value func (val *BoolValue) Value() bool { return val.value } +// JSONValue represents a string value in a YAML +// config that can be overridden by environment variables type JSONValue struct { value map[string]interface{} Raw map[string]interface{} } +// UnmarshalYAML converts YAML into an *JSONValue func (val *JSONValue) UnmarshalYAML(unmarshal func(interface{}) error) error { unmarshaled := make(map[string]interface{}) err := unmarshal(unmarshaled) @@ -131,15 +150,19 @@ func (val *JSONValue) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } +// Value returns the wrapped JSON value as map[string]interface{} func (val *JSONValue) Value() map[string]interface{} { return val.value } +// StringMapValue represents a string value in a YAML +// config that can be overridden by environment variables type StringMapValue struct { value map[string]string Raw map[string]string } +// UnmarshalYAML converts YAML into an *StringMapValue func (val *StringMapValue) UnmarshalYAML(unmarshal func(interface{}) error) error { unmarshaled := make(map[string]string) err := unmarshal(unmarshaled) @@ -159,6 +182,7 @@ func (val *StringMapValue) UnmarshalYAML(unmarshal func(interface{}) error) erro return err } +// Value returns the wrapped map[string]string value func (val *StringMapValue) Value() map[string]string { return val.value }