From ff550df32f4016100be2f422ed5a4416a2808508 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Mon, 26 Sep 2022 09:53:25 -0400 Subject: [PATCH] Correlations: Only return correlation for which both source and target datasources exist (#55454) (#55744) * Correlation: only return correlation for which both source and targe ds exist * add test (cherry picked from commit d07abdd23c3e6a28913e2391f67867336b11c620) Co-authored-by: Giordano Ricci --- pkg/services/correlations/database.go | 18 +++------------ .../correlations/correlations_read_test.go | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/pkg/services/correlations/database.go b/pkg/services/correlations/database.go index 7d98a59c1a1..81dff8deaca 100644 --- a/pkg/services/correlations/database.go +++ b/pkg/services/correlations/database.go @@ -147,14 +147,10 @@ func (s CorrelationsService) getCorrelation(ctx context.Context, cmd GetCorrelat return ErrSourceDataSourceDoesNotExists } - found, err := session.Where("uid = ? AND source_uid = ?", correlation.UID, correlation.SourceUID).Get(&correlation) + found, err := session.Select("correlation.*").Join("", "data_source AS dss", "correlation.source_uid = dss.uid and dss.org_id = ?", cmd.OrgId).Join("", "data_source AS dst", "correlation.target_uid = dst.uid and dst.org_id = ?", cmd.OrgId).Where("correlation.uid = ? AND correlation.source_uid = ?", correlation.UID, correlation.SourceUID).Get(&correlation) if !found { return ErrCorrelationNotFound } - if err != nil { - return err - } - return err }) @@ -166,9 +162,6 @@ func (s CorrelationsService) getCorrelation(ctx context.Context, cmd GetCorrelat } func (s CorrelationsService) getCorrelationsBySourceUID(ctx context.Context, cmd GetCorrelationsBySourceUIDQuery) ([]Correlation, error) { - correlationsCondiBean := Correlation{ - SourceUID: cmd.SourceUID, - } correlations := make([]Correlation, 0) err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error { @@ -180,12 +173,7 @@ func (s CorrelationsService) getCorrelationsBySourceUID(ctx context.Context, cmd return ErrSourceDataSourceDoesNotExists } - err := session.Find(&correlations, correlationsCondiBean) - if err != nil { - return err - } - - return err + return session.Select("correlation.*").Join("", "data_source AS dss", "correlation.source_uid = dss.uid and dss.org_id = ?", cmd.OrgId).Join("", "data_source AS dst", "correlation.target_uid = dst.uid and dst.org_id = ?", cmd.OrgId).Where("correlation.source_uid = ?", cmd.SourceUID).Find(&correlations) }) if err != nil { @@ -199,7 +187,7 @@ func (s CorrelationsService) getCorrelations(ctx context.Context, cmd GetCorrela correlations := make([]Correlation, 0) err := s.SQLStore.WithDbSession(ctx, func(session *sqlstore.DBSession) error { - return session.Select("correlation.*").Join("", "data_source", "correlation.source_uid = data_source.uid").Where("data_source.org_id = ?", cmd.OrgId).Find(&correlations) + return session.Select("correlation.*").Join("", "data_source AS dss", "correlation.source_uid = dss.uid and dss.org_id = ?", cmd.OrgId).Join("", "data_source AS dst", "correlation.target_uid = dst.uid and dst.org_id = ?", cmd.OrgId).Find(&correlations) }) if err != nil { return []Correlation{}, err diff --git a/pkg/tests/api/correlations/correlations_read_test.go b/pkg/tests/api/correlations/correlations_read_test.go index 15b180412c0..9e425f5afb9 100644 --- a/pkg/tests/api/correlations/correlations_read_test.go +++ b/pkg/tests/api/correlations/correlations_read_test.go @@ -1,6 +1,7 @@ package correlations import ( + "context" "encoding/json" "fmt" "io" @@ -10,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/correlations" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/stretchr/testify/require" ) @@ -83,6 +85,27 @@ func TestIntegrationReadCorrelation(t *testing.T) { ctx.createDs(createDsCommand) dsWithoutCorrelations := createDsCommand.Result + // This creates 2 records in the correlation table that should never be returned by the API. + // Given all tests in this file work on the assumption that only a single correlation exists, + // this covers the case where bad data exists in the database. + err := ctx.env.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + created, err := sess.InsertMulti(&[]correlations.Correlation{ + { + UID: "uid-1", + SourceUID: dsWithoutCorrelations.Uid, + TargetUID: "THIS-DOES-NOT_EXIST", + }, + { + UID: "uid-2", + SourceUID: "THIS-DOES-NOT_EXIST", + TargetUID: dsWithoutCorrelations.Uid, + }, + }) + require.Equal(t, int64(2), created) + return err + }) + require.NoError(t, err) + t.Run("Get all correlations", func(t *testing.T) { t.Run("Unauthenticated users shouldn't be able to read correlations", func(t *testing.T) { res := ctx.Get(GetParams{