public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

API: Do not validate/save legacy alerts when saving a dashboard if legacy alerting is disabled (#51883) (#52137)

Browse Source 
* API: Do not validate/save legacy alerts if legacy alerting is disabled

Co-authored-by: Ida Furjesova <ida.furjesova@grafana.com>
(cherry picked from commit b3992df988)

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
This commit is contained in:
Grot (@grafanabot)
2022-07-13 05:52:32 -04:00
committed by GitHub
co-authored by Sofia Papagiannaki
parent 7d8fbb83f4
commit f3f66f6325
3 changed files with 56 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/dashboards/service/dashboard_service.go
+32 -23
View File
@@ -221,7 +221,7 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt
},
}
cmd, err := dr.BuildSaveDashboardCommand(ctx, dto, true, false)
cmd, err := dr.BuildSaveDashboardCommand(ctx, dto, setting.IsLegacyAlertingEnabled(), false)
if err != nil {
return nil, err
}
@@ -239,14 +239,17 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt
OrgID: dto.OrgId,
}
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
// extract/save legacy alerts only if legacy alerting is enabled
if setting.IsLegacyAlertingEnabled() {
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
}
}
if dto.Dashboard.Id == 0 {
@@ -280,14 +283,17 @@ func (dr *DashboardServiceImpl) SaveFolderForProvisionedDashboards(ctx context.C
OrgID: dto.OrgId,
}
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
// extract/save legacy alerts only if legacy alerting is enabled
if setting.IsLegacyAlertingEnabled() {
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
}
}
if dto.Dashboard.Id == 0 {
@@ -308,7 +314,7 @@ func (dr *DashboardServiceImpl) SaveDashboard(ctx context.Context, dto *dashboar
dto.Dashboard.Data.Set("refresh", setting.MinRefreshInterval)
}
cmd, err := dr.BuildSaveDashboardCommand(ctx, dto, true, !allowUiUpdate)
cmd, err := dr.BuildSaveDashboardCommand(ctx, dto, setting.IsLegacyAlertingEnabled(), !allowUiUpdate)
if err != nil {
return nil, err
}
@@ -324,14 +330,17 @@ func (dr *DashboardServiceImpl) SaveDashboard(ctx context.Context, dto *dashboar
OrgID: dto.OrgId,
}
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
// extract/save legacy alerts only if legacy alerting is enabled
if setting.IsLegacyAlertingEnabled() {
alerts, err := dr.dashAlertExtractor.GetAlerts(ctx, dashAlertInfo)
if err != nil {
return nil, err
}
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
err = dr.dashboardStore.SaveAlerts(ctx, dash.Id, alerts)
if err != nil {
return nil, err
}
}
// new dashboard created
pkg/services/dashboards/service/dashboard_service_test.go
+18 -3
View File
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/xorcare/pointer"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log"
@@ -23,7 +24,9 @@ func TestIntegrationDashboardService(t *testing.T) {
t.Run("Dashboard service tests", func(t *testing.T) {
fakeStore := m.FakeDashboardStore{}
defer fakeStore.AssertExpectations(t)
service := &DashboardServiceImpl{
cfg: setting.NewCfg(),
log: log.New("test.logger"),
dashboardStore: &fakeStore,
dashAlertExtractor: &dummyDashAlertExtractor{},
@@ -100,7 +103,6 @@ func TestIntegrationDashboardService(t *testing.T) {
t.Run("Should not return validation error if dashboard is provisioned but UI updates allowed", func(t *testing.T) {
fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything).Return(true, nil).Once()
fakeStore.On("SaveDashboard", mock.Anything).Return(&models.Dashboard{Data: simplejson.New()}, nil).Once()
fakeStore.On("SaveAlerts", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
dto.Dashboard = models.NewDashboard("Dash")
dto.Dashboard.SetId(3)
@@ -110,6 +112,20 @@ func TestIntegrationDashboardService(t *testing.T) {
})
t.Run("Should return validation error if alert data is invalid", func(t *testing.T) {
origAlertingEnabledSet := setting.AlertingEnabled != nil
origAlertingEnabledVal := false
if origAlertingEnabledSet {
origAlertingEnabledVal = *setting.AlertingEnabled
}
setting.AlertingEnabled = pointer.Bool(true)
t.Cleanup(func() {
if !origAlertingEnabledSet {
setting.AlertingEnabled = nil
} else {
setting.AlertingEnabled = &origAlertingEnabledVal
}
})
fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything).Return(true, nil).Once()
fakeStore.On("GetProvisionedDataByDashboardID", mock.Anything).Return(nil, nil).Once()
fakeStore.On("SaveDashboard", mock.Anything).Return(&models.Dashboard{Data: simplejson.New()}, nil).Once()
@@ -118,6 +134,7 @@ func TestIntegrationDashboardService(t *testing.T) {
dto.Dashboard = models.NewDashboard("Dash")
dto.User = &models.SignedInUser{UserId: 1}
_, err := service.SaveDashboard(context.Background(), dto, false)
require.Error(t, err)
require.Equal(t, err.Error(), "alert validation error")
})
})
@@ -128,7 +145,6 @@ func TestIntegrationDashboardService(t *testing.T) {
t.Run("Should not return validation error if dashboard is provisioned", func(t *testing.T) {
fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything).Return(true, nil).Once()
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything).Return(&models.Dashboard{Data: simplejson.New()}, nil).Once()
fakeStore.On("SaveAlerts", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
dto.Dashboard = models.NewDashboard("Dash")
dto.Dashboard.SetId(3)
@@ -140,7 +156,6 @@ func TestIntegrationDashboardService(t *testing.T) {
t.Run("Should override invalid refresh interval if dashboard is provisioned", func(t *testing.T) {
fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything).Return(true, nil).Once()
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything).Return(&models.Dashboard{Data: simplejson.New()}, nil).Once()
fakeStore.On("SaveAlerts", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
oldRefreshInterval := setting.MinRefreshInterval
setting.MinRefreshInterval = "5m"
pkg/setting/setting.go
+6
View File
@@ -1436,6 +1436,12 @@ func readAlertingSettings(iniFile *ini.File) error {
return nil
}
// IsLegacyAlertingEnabled returns whether the legacy alerting is enabled or not.
// It's safe to be used only after readAlertingSettings() and ReadUnifiedAlertingSettings() are executed.
func IsLegacyAlertingEnabled() bool {
return AlertingEnabled != nil && *AlertingEnabled
}
func readSnapshotsSettings(cfg *Cfg, iniFile *ini.File) error {
snapshots := iniFile.Section("snapshots")