Commit ba2524cd by Maksim Nabokikh Committed by GitHub

Provisioning: Add validation for missing organisations in datasource, dashboard,…

Provisioning: Add validation for missing organisations in datasource, dashboard, and notifier configurations (#26601)

* Provisioning: Check that org. from config exists

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

* Apply suggestions from code review

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

* Fix some code after applying suggestions

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

* Add negative test case

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
parent 3f212836
...@@ -8,7 +8,8 @@ import ( ...@@ -8,7 +8,8 @@ import (
"strings" "strings"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
yaml "gopkg.in/yaml.v2" "github.com/grafana/grafana/pkg/services/provisioning/utils"
"gopkg.in/yaml.v2"
) )
type configReader struct { type configReader struct {
...@@ -88,6 +89,10 @@ func (cr *configReader) readConfig() ([]*config, error) { ...@@ -88,6 +89,10 @@ func (cr *configReader) readConfig() ([]*config, error) {
dashboard.OrgID = 1 dashboard.OrgID = 1
} }
if err := utils.CheckOrgExists(dashboard.OrgID); err != nil {
return nil, fmt.Errorf("failed to provision dashboards with %q reader: %w", dashboard.Name, err)
}
if dashboard.Type == "" { if dashboard.Type == "" {
dashboard.Type = "file" dashboard.Type = "file"
} }
......
package dashboards package dashboards
import ( import (
"errors"
"fmt"
"os" "os"
"testing" "testing"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
) )
var ( var (
...@@ -19,6 +23,19 @@ var ( ...@@ -19,6 +23,19 @@ var (
func TestDashboardsAsConfig(t *testing.T) { func TestDashboardsAsConfig(t *testing.T) {
t.Run("Dashboards as configuration", func(t *testing.T) { t.Run("Dashboards as configuration", func(t *testing.T) {
logger := log.New("test-logger") logger := log.New("test-logger")
sqlstore.InitTestDB(t)
t.Run("Should fail if orgs don't exist in the database", func(t *testing.T) {
cfgProvider := configReader{path: appliedDefaults, log: logger}
_, err := cfgProvider.readConfig()
require.Equal(t, errors.Unwrap(err), models.ErrOrgNotFound)
})
for i := 1; i <= 2; i++ {
orgCommand := models.CreateOrgCommand{Name: fmt.Sprintf("Main Org. %v", i)}
err := sqlstore.CreateOrg(&orgCommand)
require.NoError(t, err)
}
t.Run("default values should be applied", func(t *testing.T) { t.Run("default values should be applied", func(t *testing.T) {
cfgProvider := configReader{path: appliedDefaults, log: logger} cfgProvider := configReader{path: appliedDefaults, log: logger}
......
package datasources package datasources
import ( import (
"fmt"
"io/ioutil" "io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
...@@ -8,6 +9,7 @@ import ( ...@@ -8,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/provisioning/utils"
"gopkg.in/yaml.v2" "gopkg.in/yaml.v2"
) )
...@@ -95,13 +97,8 @@ func (cr *configReader) validateDefaultUniqueness(datasources []*configs) error ...@@ -95,13 +97,8 @@ func (cr *configReader) validateDefaultUniqueness(datasources []*configs) error
ds.OrgID = 1 ds.OrgID = 1
} }
if ds.Access == "" { if err := cr.validateAccessAndOrgID(ds); err != nil {
ds.Access = models.DS_ACCESS_PROXY return fmt.Errorf("failed to provision %q data source: %w", ds.Name, err)
}
if ds.Access != models.DS_ACCESS_DIRECT && ds.Access != models.DS_ACCESS_PROXY {
cr.log.Warn("invalid access value, will use 'proxy' instead", "value", ds.Access)
ds.Access = models.DS_ACCESS_PROXY
} }
if ds.IsDefault { if ds.IsDefault {
...@@ -121,3 +118,19 @@ func (cr *configReader) validateDefaultUniqueness(datasources []*configs) error ...@@ -121,3 +118,19 @@ func (cr *configReader) validateDefaultUniqueness(datasources []*configs) error
return nil return nil
} }
func (cr *configReader) validateAccessAndOrgID(ds *upsertDataSourceFromConfig) error {
if err := utils.CheckOrgExists(ds.OrgID); err != nil {
return err
}
if ds.Access == "" {
ds.Access = models.DS_ACCESS_PROXY
}
if ds.Access != models.DS_ACCESS_DIRECT && ds.Access != models.DS_ACCESS_PROXY {
cr.log.Warn("invalid access value, will use 'proxy' instead", "value", ds.Access)
ds.Access = models.DS_ACCESS_PROXY
}
return nil
}
...@@ -36,6 +36,7 @@ func TestDatasourceAsConfig(t *testing.T) { ...@@ -36,6 +36,7 @@ func TestDatasourceAsConfig(t *testing.T) {
bus.AddHandler("test", mockUpdate) bus.AddHandler("test", mockUpdate)
bus.AddHandler("test", mockGet) bus.AddHandler("test", mockGet)
bus.AddHandler("test", mockGetAll) bus.AddHandler("test", mockGetAll)
bus.AddHandler("test", mockGetOrg)
Convey("apply default values when missing", func() { Convey("apply default values when missing", func() {
dc := newDatasourceProvisioner(logger) dc := newDatasourceProvisioner(logger)
...@@ -296,3 +297,7 @@ func mockGet(cmd *models.GetDataSourceByNameQuery) error { ...@@ -296,3 +297,7 @@ func mockGet(cmd *models.GetDataSourceByNameQuery) error {
return models.ErrDataSourceNotFound return models.ErrDataSourceNotFound
} }
func mockGetOrg(_ *models.GetOrgByIdQuery) error {
return nil
}
...@@ -11,6 +11,7 @@ import ( ...@@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/provisioning/utils"
"gopkg.in/yaml.v2" "gopkg.in/yaml.v2"
) )
...@@ -47,10 +48,11 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error ...@@ -47,10 +48,11 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error
return nil, err return nil, err
} }
checkOrgIDAndOrgName(notifications) if err := checkOrgIDAndOrgName(notifications); err != nil {
return nil, err
}
err = validateNotifications(notifications) if err := validateNotifications(notifications); err != nil {
if err != nil {
return nil, err return nil, err
} }
...@@ -73,7 +75,7 @@ func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) ( ...@@ -73,7 +75,7 @@ func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) (
return cfg.mapToNotificationFromConfig(), nil return cfg.mapToNotificationFromConfig(), nil
} }
func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) { func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) error {
for i := range notifications { for i := range notifications {
for _, notification := range notifications[i].Notifications { for _, notification := range notifications[i].Notifications {
if notification.OrgID < 1 { if notification.OrgID < 1 {
...@@ -82,6 +84,10 @@ func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) { ...@@ -82,6 +84,10 @@ func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) {
} else { } else {
notification.OrgID = 0 notification.OrgID = 0
} }
} else {
if err := utils.CheckOrgExists(notification.OrgID); err != nil {
return fmt.Errorf("failed to provision %q notification: %w", notification.Name, err)
}
} }
} }
...@@ -95,6 +101,7 @@ func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) { ...@@ -95,6 +101,7 @@ func checkOrgIDAndOrgName(notifications []*notificationsAsConfig) {
} }
} }
} }
return nil
} }
func validateRequiredField(notifications []*notificationsAsConfig) error { func validateRequiredField(notifications []*notificationsAsConfig) error {
......
package notifiers package notifiers
import ( import (
"fmt"
"os" "os"
"testing" "testing"
...@@ -31,6 +32,12 @@ func TestNotificationAsConfig(t *testing.T) { ...@@ -31,6 +32,12 @@ func TestNotificationAsConfig(t *testing.T) {
Convey("Testing notification as configuration", t, func() { Convey("Testing notification as configuration", t, func() {
sqlstore.InitTestDB(t) sqlstore.InitTestDB(t)
for i := 1; i < 5; i++ {
orgCommand := models.CreateOrgCommand{Name: fmt.Sprintf("Main Org. %v", i)}
err := sqlstore.CreateOrg(&orgCommand)
So(err, ShouldBeNil)
}
alerting.RegisterNotifier(&alerting.NotifierPlugin{ alerting.RegisterNotifier(&alerting.NotifierPlugin{
Type: "slack", Type: "slack",
Name: "slack", Name: "slack",
...@@ -227,12 +234,12 @@ func TestNotificationAsConfig(t *testing.T) { ...@@ -227,12 +234,12 @@ func TestNotificationAsConfig(t *testing.T) {
}) })
Convey("Can read correct properties with orgName instead of orgId", func() { Convey("Can read correct properties with orgName instead of orgId", func() {
existingOrg1 := models.CreateOrgCommand{Name: "Main Org. 1"} existingOrg1 := models.GetOrgByNameQuery{Name: "Main Org. 1"}
err := sqlstore.CreateOrg(&existingOrg1) err := sqlstore.GetOrgByName(&existingOrg1)
So(err, ShouldBeNil) So(err, ShouldBeNil)
So(existingOrg1.Result, ShouldNotBeNil) So(existingOrg1.Result, ShouldNotBeNil)
existingOrg2 := models.CreateOrgCommand{Name: "Main Org. 2"} existingOrg2 := models.GetOrgByNameQuery{Name: "Main Org. 2"}
err = sqlstore.CreateOrg(&existingOrg2) err = sqlstore.GetOrgByName(&existingOrg2)
So(err, ShouldBeNil) So(err, ShouldBeNil)
So(existingOrg2.Result, ShouldNotBeNil) So(existingOrg2.Result, ShouldNotBeNil)
......
package utils
import (
"errors"
"fmt"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models"
)
func CheckOrgExists(orgID int64) error {
query := models.GetOrgByIdQuery{Id: orgID}
if err := bus.Dispatch(&query); err != nil {
if errors.Is(err, models.ErrOrgNotFound) {
return err
}
return fmt.Errorf("failed to check whether org. with the given ID exists: %w", err)
}
return nil
}
package utils
import (
"testing"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
. "github.com/smartystreets/goconvey/convey"
)
func TestCheckOrgExists(t *testing.T) {
Convey("with default org in database", t, func() {
sqlstore.InitTestDB(t)
defaultOrg := models.CreateOrgCommand{Name: "Main Org."}
err := sqlstore.CreateOrg(&defaultOrg)
So(err, ShouldBeNil)
Convey("default org exists", func() {
err := CheckOrgExists(defaultOrg.Result.Id)
So(err, ShouldBeNil)
})
Convey("other org doesn't exist", func() {
err := CheckOrgExists(defaultOrg.Result.Id + 1)
So(err, ShouldEqual, models.ErrOrgNotFound)
})
})
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment