Dashboards: Add Dashboard API Validation tests and fix underlying issues (#103502)

This commit is contained in:
Marco de Abreu
2025-04-07 05:44:28 +03:00
committed by GitHub
parent 427715b070
commit b2f80d4dca
16 changed files with 1521 additions and 240 deletions
@@ -608,6 +608,29 @@ func (dr *DashboardServiceImpl) GetProvisionedDashboardDataByDashboardUID(ctx co
return dr.dashboardStore.GetProvisionedDataByDashboardUID(ctx, orgID, dashboardUID)
}
func (dr *DashboardServiceImpl) ValidateBasicDashboardProperties(title string, uid string, message string) error {
if title == "" {
return dashboards.ErrDashboardTitleEmpty
}
if len(title) > 5000 {
return dashboards.ErrDashboardTitleTooLong
}
// Validate message
if message != "" && len(message) > 500 {
return dashboards.ErrDashboardMessageTooLong
}
if !util.IsValidShortUID(uid) {
return dashboards.ErrDashboardInvalidUid
} else if util.IsShortUIDTooLong(uid) {
return dashboards.ErrDashboardUidTooLong
}
return nil
}
//nolint:gocyclo
func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, dto *dashboards.SaveDashboardDTO,
validateProvisionedDashboard bool) (*dashboards.SaveDashboardCommand, error) {
@@ -621,16 +644,8 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
dash.Data.Set("title", dash.Title)
dash.SetUID(strings.TrimSpace(dash.UID))
if dash.Title == "" {
return nil, dashboards.ErrDashboardTitleEmpty
}
if len(dash.Title) > 5000 {
return nil, dashboards.ErrDashboardTitleTooLong
}
if len(dto.Message) > 500 {
return nil, dashboards.ErrDashboardMessageTooLong
if err := dr.ValidateBasicDashboardProperties(dash.Title, dash.UID, dto.Message); err != nil {
return nil, err
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
@@ -643,13 +658,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
return nil, dashboards.ErrDashboardFolderNameExists
}
if !util.IsValidShortUID(dash.UID) {
return nil, dashboards.ErrDashboardInvalidUid
} else if util.IsShortUIDTooLong(dash.UID) {
return nil, dashboards.ErrDashboardUidTooLong
}
if err := validateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dash); err != nil {
if err := dr.ValidateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dash.Data.Get("refresh").MustString("")); err != nil {
return nil, err
}
@@ -910,13 +919,12 @@ func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context.
return dr.dashboardStore.DeleteOrphanedProvisionedDashboards(ctx, cmd)
}
func validateDashboardRefreshInterval(minRefreshInterval string, dash *dashboards.Dashboard) error {
func (dr *DashboardServiceImpl) ValidateDashboardRefreshInterval(minRefreshInterval string, targetRefreshInterval string) error {
if minRefreshInterval == "" {
return nil
}
refresh := dash.Data.Get("refresh").MustString("")
if refresh == "" || refresh == "auto" {
if targetRefreshInterval == "" || targetRefreshInterval == "auto" {
// since no refresh is set it is a valid refresh rate
return nil
}
@@ -925,9 +933,9 @@ func validateDashboardRefreshInterval(minRefreshInterval string, dash *dashboard
if err != nil {
return fmt.Errorf("parsing min refresh interval %q failed: %w", minRefreshInterval, err)
}
d, err := gtime.ParseDuration(refresh)
d, err := gtime.ParseDuration(targetRefreshInterval)
if err != nil {
return fmt.Errorf("parsing refresh duration %q failed: %w", refresh, err)
return fmt.Errorf("parsing refresh duration %q failed: %w", targetRefreshInterval, err)
}
if d < minRefreshIntervalDur {
@@ -942,7 +950,7 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt
ctx, span := tracer.Start(ctx, "dashboards.service.SaveProvisionedDashboard")
defer span.End()
if err := validateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard); err != nil {
if err := dr.ValidateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard.Data.Get("refresh").MustString("")); err != nil {
dr.log.Warn("Changing refresh interval for provisioned dashboard to minimum refresh interval", "dashboardUid",
dto.Dashboard.UID, "dashboardTitle", dto.Dashboard.Title, "minRefreshInterval", dr.cfg.MinRefreshInterval)
dto.Dashboard.Data.Set("refresh", dr.cfg.MinRefreshInterval)
@@ -971,7 +979,7 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt
}
if dto.Dashboard.ID == 0 {
dr.setDefaultPermissions(ctx, dto, dash, true)
dr.SetDefaultPermissions(ctx, dto, dash, true)
}
return dash, nil
@@ -1002,7 +1010,7 @@ func (dr *DashboardServiceImpl) SaveDashboard(ctx context.Context, dto *dashboar
ctx, span := tracer.Start(ctx, "dashboards.service.SaveDashboard")
defer span.End()
if err := validateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard); err != nil {
if err := dr.ValidateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard.Data.Get("refresh").MustString("")); err != nil {
dr.log.Warn("Changing refresh interval for imported dashboard to minimum refresh interval",
"dashboardUid", dto.Dashboard.UID, "dashboardTitle", dto.Dashboard.Title, "minRefreshInterval",
dr.cfg.MinRefreshInterval)
@@ -1021,7 +1029,7 @@ func (dr *DashboardServiceImpl) SaveDashboard(ctx context.Context, dto *dashboar
// new dashboard created
if dto.Dashboard.ID == 0 {
dr.setDefaultPermissions(ctx, dto, dash, false)
dr.SetDefaultPermissions(ctx, dto, dash, false)
}
return dash, nil
@@ -1095,7 +1103,7 @@ func (dr *DashboardServiceImpl) ImportDashboard(ctx context.Context, dto *dashbo
ctx, span := tracer.Start(ctx, "dashboards.service.ImportDashboard")
defer span.End()
if err := validateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard); err != nil {
if err := dr.ValidateDashboardRefreshInterval(dr.cfg.MinRefreshInterval, dto.Dashboard.Data.Get("refresh").MustString("")); err != nil {
dr.log.Warn("Changing refresh interval for imported dashboard to minimum refresh interval",
"dashboardUid", dto.Dashboard.UID, "dashboardTitle", dto.Dashboard.Title,
"minRefreshInterval", dr.cfg.MinRefreshInterval)
@@ -1112,7 +1120,7 @@ func (dr *DashboardServiceImpl) ImportDashboard(ctx context.Context, dto *dashbo
return nil, err
}
dr.setDefaultPermissions(ctx, dto, dash, false)
dr.SetDefaultPermissions(ctx, dto, dash, false)
return dash, nil
}
@@ -1178,7 +1186,7 @@ func (dr *DashboardServiceImpl) GetDashboardsByPluginID(ctx context.Context, que
return dr.dashboardStore.GetDashboardsByPluginID(ctx, query)
}
func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *dashboards.SaveDashboardDTO, dash *dashboards.Dashboard, provisioned bool) {
func (dr *DashboardServiceImpl) SetDefaultPermissions(ctx context.Context, dto *dashboards.SaveDashboardDTO, dash *dashboards.Dashboard, provisioned bool) {
ctx, span := tracer.Start(ctx, "dashboards.service.setDefaultPermissions")
defer span.End()
@@ -1765,9 +1773,14 @@ func (dr *DashboardServiceImpl) saveProvisionedDashboardThroughK8s(ctx context.C
meta.SetManagerProperties(m)
meta.SetSourceProperties(s)
// Update will create if not exists (upsert!)
out, err := dr.k8sclient.Update(ctx, obj, cmd.OrgID)
if err != nil {
if err != nil && apierrors.IsNotFound(err) {
// Create if it doesn't already exist.
out, err = dr.k8sclient.Create(ctx, obj, cmd.OrgID)
if err != nil {
return nil, err
}
} else if err != nil {
return nil, err
}
@@ -1782,9 +1795,14 @@ func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd
dashboard.SetPluginIDMeta(obj, cmd.PluginID)
// Update will create if not exists (upsert!)
out, err := dr.k8sclient.Update(ctx, obj, orgID)
if err != nil {
if err != nil && apierrors.IsNotFound(err) {
// Create if it doesn't already exist.
out, err = dr.k8sclient.Create(ctx, obj, orgID)
if err != nil {
return nil, err
}
} else if err != nil {
return nil, err
}
@@ -2161,7 +2161,7 @@ func TestCountDashboardsInOrg(t *testing.T) {
t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) {
service.features = featuremgmt.WithFeatures()
fakeStore.On("CountInOrg", mock.Anything, mock.Anything, false).Return(nil, nil).Once()
fakeStore.On("CountInOrg", mock.Anything, mock.Anything, false).Return(int64(1), nil).Once()
_, err := service.CountDashboardsInOrg(context.Background(), 1)
require.NoError(t, err)
fakeStore.AssertExpectations(t)