From 3c78fb1aa4ea4a379762d5d03c8d9656f12a024c Mon Sep 17 00:00:00 2001 From: XZCendence <44044176+XZCendence@users.noreply.github.com> Date: Thu, 12 Dec 2024 01:15:29 -0500 Subject: [PATCH 01/13] Dashboards: Add option to specify explicit percent change text size for stat panels (#96952) Add option to change percent change text size --- packages/grafana-schema/src/common/common.gen.ts | 4 ++++ packages/grafana-schema/src/common/mudball.cue | 2 ++ .../src/components/BigValue/BigValueLayout.tsx | 9 +++++++-- packages/grafana-ui/src/options/builder/text.tsx | 13 +++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/grafana-schema/src/common/common.gen.ts b/packages/grafana-schema/src/common/common.gen.ts index ad1169b3c96..580ff15f40d 100644 --- a/packages/grafana-schema/src/common/common.gen.ts +++ b/packages/grafana-schema/src/common/common.gen.ts @@ -576,6 +576,10 @@ export type TimelineValueAlignment = ('center' | 'left' | 'right'); * TODO docs */ export interface VizTextDisplayOptions { + /** + * Explicit percent text size + */ + percentSize?: number; /** * Explicit title text size */ diff --git a/packages/grafana-schema/src/common/mudball.cue b/packages/grafana-schema/src/common/mudball.cue index 3ef52efbe31..d131935c497 100644 --- a/packages/grafana-schema/src/common/mudball.cue +++ b/packages/grafana-schema/src/common/mudball.cue @@ -206,6 +206,8 @@ VizTextDisplayOptions: { titleSize?: number // Explicit value text size valueSize?: number + // Explicit percent text size + percentSize?: number } @cuetsy(kind="interface") // TODO docs diff --git a/packages/grafana-ui/src/components/BigValue/BigValueLayout.tsx b/packages/grafana-ui/src/components/BigValue/BigValueLayout.tsx index 23b32b90ad1..ef0a5a80f02 100644 --- a/packages/grafana-ui/src/components/BigValue/BigValueLayout.tsx +++ b/packages/grafana-ui/src/components/BigValue/BigValueLayout.tsx @@ -19,6 +19,7 @@ const VALUE_FONT_WEIGHT = 500; export abstract class BigValueLayout { titleFontSize: number; valueFontSize: number; + percentFontSize: number; chartHeight: number; chartWidth: number; valueColor: string; @@ -41,6 +42,7 @@ export abstract class BigValueLayout { this.titleToAlignTo = this.textValues.titleToAlignTo; this.titleFontSize = 0; this.valueFontSize = 0; + this.percentFontSize = 0; this.chartHeight = 0; this.chartWidth = 0; this.maxTextWidth = width - this.panelPadding * 2; @@ -56,6 +58,9 @@ export abstract class BigValueLayout { this.valueFontSize = text.valueSize; this.valueToAlignTo = ''; } + if (text.percentSize) { + this.percentFontSize = text.percentSize; + } } } @@ -111,8 +116,8 @@ export abstract class BigValueLayout { ): PercentChangeStyles { const VALUE_TO_PERCENT_CHANGE_RATIO = 2.5; const valueContainerStyles = this.getValueAndTitleContainerStyles(); - const percentFontSize = Math.max(this.valueFontSize / VALUE_TO_PERCENT_CHANGE_RATIO, 12); - let iconSize = Math.max(this.valueFontSize / 3, 10); + const percentFontSize = this.percentFontSize || Math.max(this.valueFontSize / VALUE_TO_PERCENT_CHANGE_RATIO, 12); + let iconSize = this.percentFontSize ? this.percentFontSize - 3 : Math.max(this.valueFontSize / 3, 10); const themeVisualizationColors = this.props.theme.visualization; const color = getPercentChangeColor(percentChange, percentChangeColorMode, valueStyles, themeVisualizationColors); diff --git a/packages/grafana-ui/src/options/builder/text.tsx b/packages/grafana-ui/src/options/builder/text.tsx index d6ecba45ebc..b2cfd425113 100644 --- a/packages/grafana-ui/src/options/builder/text.tsx +++ b/packages/grafana-ui/src/options/builder/text.tsx @@ -38,4 +38,17 @@ export function addTextSizeOptions( }, defaultValue: undefined, }); + + builder.addNumberInput({ + path: 'text.percentSize', + category: ['Text size'], + name: 'Percent change', + settings: { + placeholder: 'Auto', + integer: false, + min: 1, + max: 200, + }, + defaultValue: undefined, + }); } From ded90fa28d13b18cc9c4a44471754fdcad20a141 Mon Sep 17 00:00:00 2001 From: Ieva Date: Thu, 12 Dec 2024 08:52:14 +0000 Subject: [PATCH 02/13] App platform: Implement perm check with direct db access (#97579) * implement perm check with direct db access * add tests * more tests * Update pkg/services/authz/rbac/service.go Co-authored-by: Gabriel MABILLE * Update pkg/services/authz/rbac/service.go Co-authored-by: Gabriel MABILLE * allow fetching permissions for a user who is not a member of the org * linting * fix typo --------- Co-authored-by: Gabriel MABILLE --- pkg/services/authz/rbac/models.go | 13 ++ pkg/services/authz/rbac/service.go | 194 ++++++++++++++++-- pkg/services/authz/rbac/service_test.go | 151 ++++++++++++++ .../authz/rbac/store/basic_role_query.sql | 4 + pkg/services/authz/rbac/store/models.go | 30 +++ .../authz/rbac/store/permission_query.sql | 13 ++ pkg/services/authz/rbac/store/queries.go | 92 +++++++++ pkg/services/authz/rbac/store/sql_test.go | 97 +++++++++ pkg/services/authz/rbac/store/store.go | 115 ++++++++++- .../mysql--basic_role_query-basic_roles.sql | 4 + .../mysql--permission_query-admin_user.sql | 7 + ...ysql--permission_query-user_with_teams.sql | 8 + .../mysql--permission_query-viewer_user.sql | 6 + ...ql--user_identifier_query-id_specified.sql | 3 + ...l--user_identifier_query-uid_specified.sql | 3 + ...postgres--basic_role_query-basic_roles.sql | 4 + .../postgres--permission_query-admin_user.sql | 7 + ...gres--permission_query-user_with_teams.sql | 8 + ...postgres--permission_query-viewer_user.sql | 6 + ...es--user_identifier_query-id_specified.sql | 3 + ...s--user_identifier_query-uid_specified.sql | 3 + .../sqlite--basic_role_query-basic_roles.sql | 4 + .../sqlite--permission_query-admin_user.sql | 7 + ...lite--permission_query-user_with_teams.sql | 8 + .../sqlite--permission_query-viewer_user.sql | 6 + ...te--user_identifier_query-id_specified.sql | 3 + ...e--user_identifier_query-uid_specified.sql | 3 + .../rbac/store/user_identifier_query.sql | 7 + pkg/services/authz/server.go | 3 +- 29 files changed, 796 insertions(+), 16 deletions(-) create mode 100644 pkg/services/authz/rbac/models.go create mode 100644 pkg/services/authz/rbac/service_test.go create mode 100644 pkg/services/authz/rbac/store/basic_role_query.sql create mode 100644 pkg/services/authz/rbac/store/models.go create mode 100644 pkg/services/authz/rbac/store/permission_query.sql create mode 100644 pkg/services/authz/rbac/store/queries.go create mode 100644 pkg/services/authz/rbac/store/sql_test.go create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--basic_role_query-basic_roles.sql create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-id_specified.sql create mode 100755 pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-uid_specified.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--basic_role_query-basic_roles.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-id_specified.sql create mode 100755 pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-uid_specified.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--basic_role_query-basic_roles.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-id_specified.sql create mode 100755 pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-uid_specified.sql create mode 100644 pkg/services/authz/rbac/store/user_identifier_query.sql diff --git a/pkg/services/authz/rbac/models.go b/pkg/services/authz/rbac/models.go new file mode 100644 index 00000000000..bb5442c1b2c --- /dev/null +++ b/pkg/services/authz/rbac/models.go @@ -0,0 +1,13 @@ +package rbac + +import "github.com/grafana/authlib/claims" + +type CheckRequest struct { + Namespace claims.NamespaceInfo + UserUID string + Action string + Group string + Resource string + Verb string + Name string +} diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index c73fe9965b4..a55ef5f7f0f 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -1,11 +1,23 @@ package rbac import ( + "context" + "fmt" + "strconv" + authzv1 "github.com/grafana/authlib/authz/proto/v1" - authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" + "github.com/grafana/authlib/claims" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/apiserver/pkg/endpoints/request" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/registry/apis/iam/common" + "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/authz/mappers" + authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/rbac/store" "github.com/grafana/grafana/pkg/storage/legacysql" ) @@ -14,24 +26,180 @@ type Service struct { authzv1.UnimplementedAuthzServiceServer authzextv1.UnimplementedAuthzExtentionServiceServer - store *store.Store + store *store.Store + identityStore legacy.LegacyIdentityStore + actionMapper *mappers.K8sRbacMapper + logger log.Logger tracer tracing.Tracer } -func NewService(sql legacysql.LegacyDatabaseProvider, logger log.Logger, tracer tracing.Tracer) *Service { +func NewService(sql legacysql.LegacyDatabaseProvider, identityStore legacy.LegacyIdentityStore, logger log.Logger, tracer tracing.Tracer) *Service { return &Service{ - store: store.NewStore(sql), - logger: logger, - tracer: tracer, + store: store.NewStore(sql), + identityStore: identityStore, + actionMapper: mappers.NewK8sRbacMapper(), + logger: logger, + tracer: tracer, } } -// TODO: Implement Check -// func (s *Service) Check(ctx context.Context, req *authzv1.CheckRequest) (*authzv1.CheckResponse, error) { -// This needs to be done for the database provider -// ns := req.GetNamespace() -// ctx = request.WithNamespace(ctx, ns) +func (s *Service) Check(ctx context.Context, req *authzv1.CheckRequest) (*authzv1.CheckResponse, error) { + ctx, span := s.tracer.Start(ctx, "authz_direct_db.Check") + defer span.End() + ctxLogger := s.logger.FromContext(ctx) -// return nil, nil -// } + deny := &authzv1.CheckResponse{Allowed: false} + + checkReq, err := s.validateRequest(ctx, req) + if err != nil { + ctxLogger.Error("invalid request", "error", err) + return deny, err + } + ctx = request.WithNamespace(ctx, req.GetNamespace()) + + permissions, err := s.getUserPermissions(ctx, checkReq) + if err != nil { + ctxLogger.Error("could not get user permissions", "subject", req.GetSubject(), "error", err) + return deny, err + } + + allowed, err := s.checkPermission(ctx, permissions, checkReq) + if err != nil { + ctxLogger.Error("could not check permission", "error", err) + return deny, err + } + return &authzv1.CheckResponse{Allowed: allowed}, nil +} + +func (s *Service) validateRequest(ctx context.Context, req *authzv1.CheckRequest) (*CheckRequest, error) { + ctxLogger := s.logger.FromContext(ctx) + + if req.GetNamespace() == "" { + return nil, status.Error(codes.InvalidArgument, "namespace is required") + } + namespace := req.GetNamespace() + ns, err := claims.ParseNamespace(namespace) + if err != nil { + ctxLogger.Error("could not parse namespace", "namespace", namespace, "error", err) + return nil, err + } + + if req.GetSubject() == "" { + return nil, status.Error(codes.InvalidArgument, "subject is required") + } + user := req.GetSubject() + identityType, userUID, err := claims.ParseTypeID(user) + if err != nil { + ctxLogger.Error("could not parse subject", "subject", user, "error", err) + return nil, err + } + // Permission check currently only checks user and service account permissions, so might return a false negative for other types + if !(identityType == claims.TypeUser || identityType == claims.TypeServiceAccount) { + ctxLogger.Warn("unsupported identity type", "type", identityType) + } + + if req.GetGroup() == "" || req.GetResource() == "" || req.GetVerb() == "" { + return nil, status.Error(codes.InvalidArgument, "group, resource and verb are required") + } + action, ok := s.actionMapper.Action(req.GetGroup(), req.GetResource(), req.GetVerb()) + if !ok { + ctxLogger.Error("could not find associated rbac action", "group", req.GetGroup(), "resource", req.GetResource(), "verb", req.GetVerb()) + return nil, status.Error(codes.NotFound, "could not find associated rbac action") + } + + checkReq := &CheckRequest{ + Namespace: ns, + UserUID: userUID, + Action: action, + Group: req.GetGroup(), + Resource: req.GetResource(), + Verb: req.GetVerb(), + Name: req.GetName(), + } + return checkReq, nil +} + +func (s *Service) getUserPermissions(ctx context.Context, req *CheckRequest) ([]accesscontrol.Permission, error) { + var userIDQuery store.UserIdentifierQuery + // Assume that numeric UID is user ID + if userID, err := strconv.Atoi(req.UserUID); err == nil { + userIDQuery = store.UserIdentifierQuery{UserID: int64(userID)} + } else { + userIDQuery = store.UserIdentifierQuery{UserUID: req.UserUID} + } + userIdentifiers, err := s.store.GetUserIdentifiers(ctx, userIDQuery) + if err != nil { + return nil, fmt.Errorf("could not get user internal id: %w", err) + } + + basicRoles, err := s.store.GetBasicRoles(ctx, req.Namespace, store.BasicRoleQuery{UserID: userIdentifiers.ID}) + if err != nil { + return nil, fmt.Errorf("could not get basic roles: %w", err) + } + + teamIDs := make([]int64, 0, 50) + teamQuery := legacy.ListUserTeamsQuery{ + UserUID: userIdentifiers.UID, + Pagination: common.Pagination{Limit: 50}, + } + + for { + teams, err := s.identityStore.ListUserTeams(ctx, req.Namespace, teamQuery) + if err != nil { + return nil, fmt.Errorf("could not get user teams: %w", err) + } + for _, team := range teams.Items { + teamIDs = append(teamIDs, team.ID) + } + teamQuery.Pagination.Continue = teams.Continue + if teams.Continue == 0 { + break + } + } + + userPermQuery := store.PermissionsQuery{ + UserID: userIdentifiers.ID, + Action: req.Action, + TeamIDs: teamIDs, + Role: basicRoles.Role, + IsServerAdmin: basicRoles.IsAdmin, + } + + return s.store.GetUserPermissions(ctx, req.Namespace, userPermQuery) +} + +func (s *Service) checkPermission(ctx context.Context, permissions []accesscontrol.Permission, req *CheckRequest) (bool, error) { + ctxLogger := s.logger.FromContext(ctx) + + // Only check action if the request doesn't specify scope + if req.Name == "" { + return len(permissions) > 0, nil + } + + scopeMap := getScopeMap(permissions) + + // Wildcard grant, no further checks needed + if scopeMap["*"] { + return true, nil + } + + scope, has := s.actionMapper.Scope(req.Group, req.Resource, req.Name) + if !has { + ctxLogger.Error("could not get attribute for resource", "resource", req.Resource) + return false, fmt.Errorf("could not get attribute for resource") + } + return scopeMap[scope], nil +} + +func getScopeMap(permissions []accesscontrol.Permission) map[string]bool { + permMap := make(map[string]bool, len(permissions)) + for _, perm := range permissions { + // If has any wildcard, return immediately + if perm.Kind == "*" || perm.Attribute == "*" || perm.Identifier == "*" { + return map[string]bool{"*": true} + } + permMap[perm.Scope] = true + } + return permMap +} diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go new file mode 100644 index 00000000000..f7d865c5d69 --- /dev/null +++ b/pkg/services/authz/rbac/service_test.go @@ -0,0 +1,151 @@ +package rbac + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/authz/mappers" +) + +func TestService_checkPermission(t *testing.T) { + type testCase struct { + name string + permissions []accesscontrol.Permission + check CheckRequest + expected bool + } + + testCases := []testCase{ + { + name: "should return true if user has permission", + permissions: []accesscontrol.Permission{ + { + Action: "dashboards:read", + Scope: "dashboards:uid:some_dashboard", + Kind: "dashboards", + Attribute: "uid", + Identifier: "some_dashboard", + }, + }, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: true, + }, + { + name: "should return false if user has permission on a different resource", + permissions: []accesscontrol.Permission{ + { + Action: "dashboards:read", + Scope: "dashboards:uid:another_dashboard", + Kind: "dashboards", + Attribute: "uid", + Identifier: "another_dashboard", + }, + }, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: false, + }, + { + name: "should return true if user has wildcard permission on identifier", + permissions: []accesscontrol.Permission{ + { + Action: "dashboards:read", + Scope: "dashboards:uid:*", + Kind: "dashboards", + Attribute: "uid", + Identifier: "*", + }, + }, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: true, + }, + { + name: "should return true if user has wildcard permission on attribute", + permissions: []accesscontrol.Permission{ + { + Action: "dashboards:read", + Scope: "dashboards:*", + Kind: "dashboards", + Attribute: "*", + }, + }, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: true, + }, + { + name: "should return true if user has wildcard permission on kind", + permissions: []accesscontrol.Permission{ + { + Action: "dashboards:read", + Scope: "*", + Kind: "*", + }, + }, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: true, + }, + { + name: "should return true if no resource is specified", + permissions: []accesscontrol.Permission{ + { + Action: "folders:create", + }, + }, + check: CheckRequest{ + Action: "folders:create", + Group: "folder.grafana.app", + Resource: "folders", + }, + expected: true, + }, + { + name: "should return false if user has no permissions on resource", + permissions: []accesscontrol.Permission{}, + check: CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "some_dashboard", + }, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := &Service{logger: log.New("test"), actionMapper: mappers.NewK8sRbacMapper()} + got, err := s.checkPermission(context.Background(), tc.permissions, &tc.check) + require.NoError(t, err) + assert.Equal(t, tc.expected, got) + }) + } +} diff --git a/pkg/services/authz/rbac/store/basic_role_query.sql b/pkg/services/authz/rbac/store/basic_role_query.sql new file mode 100644 index 00000000000..a6b1c2e82c3 --- /dev/null +++ b/pkg/services/authz/rbac/store/basic_role_query.sql @@ -0,0 +1,4 @@ +SELECT COALESCE(ou.role, 'None') AS role, u.is_admin +FROM {{ .Ident .UserTable }} as u + LEFT JOIN {{ .Ident .OrgUserTable }} as ou ON ou.user_id = u.id AND ou.org_id = {{ .Arg .Query.OrgID }} +WHERE u.id = {{ .Arg .Query.UserID }} diff --git a/pkg/services/authz/rbac/store/models.go b/pkg/services/authz/rbac/store/models.go new file mode 100644 index 00000000000..3f502294f9f --- /dev/null +++ b/pkg/services/authz/rbac/store/models.go @@ -0,0 +1,30 @@ +package store + +type UserIdentifiers struct { + ID int64 + UID string +} + +type BasicRole struct { + Role string + IsAdmin bool +} + +type PermissionsQuery struct { + OrgID int64 + UserID int64 + Action string + TeamIDs []int64 + Role string + IsServerAdmin bool +} + +type BasicRoleQuery struct { + UserID int64 + OrgID int64 +} + +type UserIdentifierQuery struct { + UserID int64 + UserUID string +} diff --git a/pkg/services/authz/rbac/store/permission_query.sql b/pkg/services/authz/rbac/store/permission_query.sql new file mode 100644 index 00000000000..da222f09fa0 --- /dev/null +++ b/pkg/services/authz/rbac/store/permission_query.sql @@ -0,0 +1,13 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM {{ .Ident .PermissionTable }} as p +WHERE p.action = {{ .Arg .Query.Action }} AND p.role_id IN ( + SELECT role_id FROM {{ .Ident .UserRoleTable }} as ur WHERE ur.user_id = {{ .Arg .Query.UserID }} AND (ur.org_id = {{ .Arg .Query.OrgID }} OR ur.org_id = 0) + {{ if .Query.TeamIDs }} + UNION + SELECT role_id FROM {{ .Ident .TeamRoleTable }} as tr WHERE tr.team_id IN ({{ .ArgList .Query.TeamIDs }}) AND tr.org_id = {{ .Arg .Query.OrgID }} + {{ end }} + UNION ALL + SELECT role_id FROM {{ .Ident .BuiltinRoleTable }} as br WHERE (br.role = {{ .Arg .Query.Role }} AND (br.org_id = {{ .Arg .Query.OrgID }} OR br.org_id = 0)) + {{ if .Query.IsServerAdmin }} + OR (br.role = 'Grafana Admin') + {{ end }} +) diff --git a/pkg/services/authz/rbac/store/queries.go b/pkg/services/authz/rbac/store/queries.go new file mode 100644 index 00000000000..0214a725e73 --- /dev/null +++ b/pkg/services/authz/rbac/store/queries.go @@ -0,0 +1,92 @@ +package store + +import ( + "embed" + "fmt" + "text/template" + + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" +) + +var ( + //go:embed *.sql + sqlTemplatesFS embed.FS + sqlTemplates = template.Must(template.New("sql").ParseFS(sqlTemplatesFS, `*.sql`)) + + sqlUserPerms = mustTemplate("permission_query.sql") + sqlQueryBasicRoles = mustTemplate("basic_role_query.sql") + sqlUserIdentifiers = mustTemplate("user_identifier_query.sql") +) + +func mustTemplate(filename string) *template.Template { + if t := sqlTemplates.Lookup(filename); t != nil { + return t + } + panic(fmt.Sprintf("template file not found: %s", filename)) +} + +type getUserIdentifiers struct { + sqltemplate.SQLTemplate + Query *UserIdentifierQuery + + UserTable string +} + +func (r getUserIdentifiers) Validate() error { + return nil +} + +func newGetUserIdentifiers(sql *legacysql.LegacyDatabaseHelper, q *UserIdentifierQuery) getUserIdentifiers { + return getUserIdentifiers{ + SQLTemplate: sqltemplate.New(sql.DialectForDriver()), + Query: q, + UserTable: sql.Table("user"), + } +} + +type getBasicRolesQuery struct { + sqltemplate.SQLTemplate + Query *BasicRoleQuery + + UserTable string + OrgUserTable string +} + +func (r getBasicRolesQuery) Validate() error { + return nil +} + +func newGetBasicRoles(sql *legacysql.LegacyDatabaseHelper, q *BasicRoleQuery) getBasicRolesQuery { + return getBasicRolesQuery{ + SQLTemplate: sqltemplate.New(sql.DialectForDriver()), + Query: q, + UserTable: sql.Table("user"), + OrgUserTable: sql.Table("org_user"), + } +} + +type getPermissionsQuery struct { + sqltemplate.SQLTemplate + Query *PermissionsQuery + + PermissionTable string + UserRoleTable string + TeamRoleTable string + BuiltinRoleTable string +} + +func (r getPermissionsQuery) Validate() error { + return nil +} + +func newGetPermissions(sql *legacysql.LegacyDatabaseHelper, q *PermissionsQuery) getPermissionsQuery { + return getPermissionsQuery{ + SQLTemplate: sqltemplate.New(sql.DialectForDriver()), + Query: q, + PermissionTable: sql.Table("permission"), + UserRoleTable: sql.Table("user_role"), + TeamRoleTable: sql.Table("team_role"), + BuiltinRoleTable: sql.Table("builtin_role"), + } +} diff --git a/pkg/services/authz/rbac/store/sql_test.go b/pkg/services/authz/rbac/store/sql_test.go new file mode 100644 index 00000000000..7e76ecfc2b0 --- /dev/null +++ b/pkg/services/authz/rbac/store/sql_test.go @@ -0,0 +1,97 @@ +package store + +import ( + "testing" + "text/template" + + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate/mocks" +) + +func TestIdentityQueries(t *testing.T) { + // prefix tables with grafana + nodb := &legacysql.LegacyDatabaseHelper{ + Table: func(n string) string { + return "grafana." + n + }, + } + + getIdentifiers := func(q *UserIdentifierQuery) sqltemplate.SQLTemplate { + v := newGetUserIdentifiers(nodb, q) + v.SQLTemplate = mocks.NewTestingSQLTemplate() + return &v + } + + getBasicRoles := func(q *BasicRoleQuery) sqltemplate.SQLTemplate { + v := newGetBasicRoles(nodb, q) + v.SQLTemplate = mocks.NewTestingSQLTemplate() + return &v + } + + getPermissions := func(q *PermissionsQuery) sqltemplate.SQLTemplate { + v := newGetPermissions(nodb, q) + v.SQLTemplate = mocks.NewTestingSQLTemplate() + return &v + } + + mocks.CheckQuerySnapshots(t, mocks.TemplateTestSetup{ + RootDir: "testdata", + Templates: map[*template.Template][]mocks.TemplateTestCase{ + sqlUserIdentifiers: { + { + Name: "id_specified", + Data: getIdentifiers(&UserIdentifierQuery{ + UserID: 1, + }), + }, + { + Name: "uid_specified", + Data: getIdentifiers(&UserIdentifierQuery{ + UserUID: "some_uid", + }), + }, + }, + sqlQueryBasicRoles: { + { + Name: "basic_roles", + Data: getBasicRoles(&BasicRoleQuery{ + UserID: 1, + OrgID: 1, + }), + }, + }, + sqlUserPerms: { + { + Name: "viewer_user", + Data: getPermissions(&PermissionsQuery{ + UserID: 1, + OrgID: 1, + Action: "folders:read", + Role: "Viewer", + }), + }, + { + Name: "admin_user", + Data: getPermissions(&PermissionsQuery{ + UserID: 1, + OrgID: 1, + Action: "folders:read", + Role: "Admin", + IsServerAdmin: true, + }), + }, + { + Name: "user_with_teams", + Data: getPermissions(&PermissionsQuery{ + UserID: 1, + OrgID: 1, + Action: "folders:read", + Role: "None", + TeamIDs: []int64{1, 2}, + }), + }, + }, + }, + }) +} diff --git a/pkg/services/authz/rbac/store/store.go b/pkg/services/authz/rbac/store/store.go index 3ba0971f60a..db0a1e72b70 100644 --- a/pkg/services/authz/rbac/store/store.go +++ b/pkg/services/authz/rbac/store/store.go @@ -1,8 +1,15 @@ package store -import "github.com/grafana/grafana/pkg/storage/legacysql" +import ( + "fmt" -// TODO (gamab): Implement GetRoles, GetTeams, GetFolders, GetPermissions + "github.com/grafana/authlib/claims" + "golang.org/x/net/context" + + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" +) type Store struct { sql legacysql.LegacyDatabaseProvider @@ -13,3 +20,107 @@ func NewStore(sql legacysql.LegacyDatabaseProvider) *Store { sql: sql, } } + +func (s *Store) GetUserPermissions(ctx context.Context, ns claims.NamespaceInfo, query PermissionsQuery) ([]accesscontrol.Permission, error) { + sql, err := s.sql(ctx) + if err != nil { + return nil, err + } + + query.OrgID = ns.OrgID + req := newGetPermissions(sql, &query) + q, err := sqltemplate.Execute(sqlUserPerms, req) + if err != nil { + return nil, err + } + + res, err := sql.DB.GetSqlxSession().Query(ctx, q, req.GetArgs()...) + if err != nil { + return nil, err + } + defer func() { + if res != nil { + _ = res.Close() + } + }() + + var perms []accesscontrol.Permission + for res.Next() { + var perm accesscontrol.Permission + if err := res.Scan(&perm.Action, &perm.Kind, &perm.Attribute, &perm.Identifier, &perm.Scope); err != nil { + return nil, err + } + perms = append(perms, perm) + } + + return perms, nil +} + +func (s *Store) GetUserIdentifiers(ctx context.Context, query UserIdentifierQuery) (*UserIdentifiers, error) { + sql, err := s.sql(ctx) + if err != nil { + return nil, err + } + + req := newGetUserIdentifiers(sql, &query) + q, err := sqltemplate.Execute(sqlUserIdentifiers, req) + if err != nil { + return nil, err + } + + rows, err := sql.DB.GetSqlxSession().Query(ctx, q, req.GetArgs()...) + defer func() { + if rows != nil { + _ = rows.Close() + } + }() + if err != nil { + return nil, err + } + + if !rows.Next() { + return nil, fmt.Errorf("user could not be found") + } + + var userIDs UserIdentifiers + if err := rows.Scan(&userIDs.ID, &userIDs.UID); err != nil { + return nil, err + } + + return &userIDs, nil +} + +func (s *Store) GetBasicRoles(ctx context.Context, ns claims.NamespaceInfo, query BasicRoleQuery) (*BasicRole, error) { + sql, err := s.sql(ctx) + if err != nil { + return nil, err + } + + query.OrgID = ns.OrgID + req := newGetBasicRoles(sql, &query) + q, err := sqltemplate.Execute(sqlQueryBasicRoles, req) + if err != nil { + return nil, err + } + + rows, err := sql.DB.GetSqlxSession().Query(ctx, q, req.GetArgs()...) + defer func() { + if rows != nil { + _ = rows.Close() + } + }() + if err != nil { + return nil, err + } + + if !rows.Next() { + return nil, fmt.Errorf("no basic roles found for the user") + } + + var role BasicRole + if err := rows.Scan(&role.Role, &role.IsAdmin); err != nil { + return nil, err + } + + return &role, nil +} diff --git a/pkg/services/authz/rbac/store/testdata/mysql--basic_role_query-basic_roles.sql b/pkg/services/authz/rbac/store/testdata/mysql--basic_role_query-basic_roles.sql new file mode 100755 index 00000000000..34867a08bb3 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--basic_role_query-basic_roles.sql @@ -0,0 +1,4 @@ +SELECT COALESCE(ou.role, 'None') AS role, u.is_admin +FROM `grafana`.`user` as u + LEFT JOIN `grafana`.`org_user` as ou ON ou.user_id = u.id AND ou.org_id = 1 +WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql new file mode 100755 index 00000000000..6af62ecd77b --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-admin_user.sql @@ -0,0 +1,7 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) + OR (br.role = 'Grafana Admin') +) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql new file mode 100755 index 00000000000..568cd54599f --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-user_with_teams.sql @@ -0,0 +1,8 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION + SELECT role_id FROM `grafana`.`team_role` as tr WHERE tr.team_id IN (1, 2) AND tr.org_id = 1 + UNION ALL + SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql new file mode 100755 index 00000000000..5f91c914e94 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--permission_query-viewer_user.sql @@ -0,0 +1,6 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM `grafana`.`permission` as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM `grafana`.`user_role` as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM `grafana`.`builtin_role` as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-id_specified.sql b/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-id_specified.sql new file mode 100755 index 00000000000..0cf10931eae --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-id_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM `grafana`.`user` as u + WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-uid_specified.sql b/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-uid_specified.sql new file mode 100755 index 00000000000..51a299d3ee7 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/mysql--user_identifier_query-uid_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM `grafana`.`user` as u + WHERE u.uid = 'some_uid' diff --git a/pkg/services/authz/rbac/store/testdata/postgres--basic_role_query-basic_roles.sql b/pkg/services/authz/rbac/store/testdata/postgres--basic_role_query-basic_roles.sql new file mode 100755 index 00000000000..b85e5a4d961 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--basic_role_query-basic_roles.sql @@ -0,0 +1,4 @@ +SELECT COALESCE(ou.role, 'None') AS role, u.is_admin +FROM "grafana"."user" as u + LEFT JOIN "grafana"."org_user" as ou ON ou.user_id = u.id AND ou.org_id = 1 +WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql new file mode 100755 index 00000000000..e210f12b984 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-admin_user.sql @@ -0,0 +1,7 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) + OR (br.role = 'Grafana Admin') +) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql new file mode 100755 index 00000000000..e53983aabea --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-user_with_teams.sql @@ -0,0 +1,8 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION + SELECT role_id FROM "grafana"."team_role" as tr WHERE tr.team_id IN (1, 2) AND tr.org_id = 1 + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql new file mode 100755 index 00000000000..5b3d5e5c583 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--permission_query-viewer_user.sql @@ -0,0 +1,6 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-id_specified.sql b/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-id_specified.sql new file mode 100755 index 00000000000..316e1fb893d --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-id_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM "grafana"."user" as u + WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-uid_specified.sql b/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-uid_specified.sql new file mode 100755 index 00000000000..ea37dfa9443 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/postgres--user_identifier_query-uid_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM "grafana"."user" as u + WHERE u.uid = 'some_uid' diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--basic_role_query-basic_roles.sql b/pkg/services/authz/rbac/store/testdata/sqlite--basic_role_query-basic_roles.sql new file mode 100755 index 00000000000..b85e5a4d961 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--basic_role_query-basic_roles.sql @@ -0,0 +1,4 @@ +SELECT COALESCE(ou.role, 'None') AS role, u.is_admin +FROM "grafana"."user" as u + LEFT JOIN "grafana"."org_user" as ou ON ou.user_id = u.id AND ou.org_id = 1 +WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql new file mode 100755 index 00000000000..e210f12b984 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-admin_user.sql @@ -0,0 +1,7 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Admin' AND (br.org_id = 1 OR br.org_id = 0)) + OR (br.role = 'Grafana Admin') +) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql new file mode 100755 index 00000000000..e53983aabea --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-user_with_teams.sql @@ -0,0 +1,8 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION + SELECT role_id FROM "grafana"."team_role" as tr WHERE tr.team_id IN (1, 2) AND tr.org_id = 1 + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'None' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql new file mode 100755 index 00000000000..5b3d5e5c583 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--permission_query-viewer_user.sql @@ -0,0 +1,6 @@ +SELECT p.action, p.kind, p.attribute, p.identifier, p.scope FROM "grafana"."permission" as p +WHERE p.action = 'folders:read' AND p.role_id IN ( + SELECT role_id FROM "grafana"."user_role" as ur WHERE ur.user_id = 1 AND (ur.org_id = 1 OR ur.org_id = 0) + UNION ALL + SELECT role_id FROM "grafana"."builtin_role" as br WHERE (br.role = 'Viewer' AND (br.org_id = 1 OR br.org_id = 0)) +) diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-id_specified.sql b/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-id_specified.sql new file mode 100755 index 00000000000..316e1fb893d --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-id_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM "grafana"."user" as u + WHERE u.id = 1 diff --git a/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-uid_specified.sql b/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-uid_specified.sql new file mode 100755 index 00000000000..ea37dfa9443 --- /dev/null +++ b/pkg/services/authz/rbac/store/testdata/sqlite--user_identifier_query-uid_specified.sql @@ -0,0 +1,3 @@ +SELECT u.id, u.uid +FROM "grafana"."user" as u + WHERE u.uid = 'some_uid' diff --git a/pkg/services/authz/rbac/store/user_identifier_query.sql b/pkg/services/authz/rbac/store/user_identifier_query.sql new file mode 100644 index 00000000000..5bfe15c087e --- /dev/null +++ b/pkg/services/authz/rbac/store/user_identifier_query.sql @@ -0,0 +1,7 @@ +SELECT u.id, u.uid +FROM {{ .Ident .UserTable }} as u +{{ if .Query.UserUID }} + WHERE u.uid = {{ .Arg .Query.UserUID }} +{{ else }} + WHERE u.id = {{ .Arg .Query.UserID }} +{{ end }} diff --git a/pkg/services/authz/server.go b/pkg/services/authz/server.go index 8c471b9f8d0..3225248f7f6 100644 --- a/pkg/services/authz/server.go +++ b/pkg/services/authz/server.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authz/mappers" @@ -25,7 +26,7 @@ import ( ) func RegisterRBACAuthZService(handler grpcserver.Provider, db legacysql.LegacyDatabaseProvider, tracer tracing.Tracer) { - server := rbac.NewService(db, log.New("authz-grpc-server"), tracer) + server := rbac.NewService(db, legacy.NewLegacySQLStores(db), log.New("authz-grpc-server"), tracer) srv := handler.GetServer() authzv1.RegisterAuthzServiceServer(srv, server) From 7de65b39b57a8cadcdf2d143c409535b869bcf49 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:12:03 +0100 Subject: [PATCH 03/13] Elasticsearch and Cloudwatch: Replace deprecated experimental/errorsource (#97363) * Update elasticsearch * Update azure monitor * Update test * Revert "Update azure monitor" This reverts commit 21b53845b85c3a43d679d729c33879fdc3e9f027. * Update cloudwatch --- pkg/tsdb/cloudwatch/annotation_query.go | 11 ++++---- .../cloudwatch/get_metric_data_executor.go | 4 +-- pkg/tsdb/cloudwatch/log_actions.go | 17 ++++++------ pkg/tsdb/cloudwatch/log_sync_query.go | 5 ++-- .../cloudwatch/models/cloudwatch_query.go | 3 +-- pkg/tsdb/cloudwatch/time_series_query.go | 7 +++-- pkg/tsdb/elasticsearch/client/client.go | 6 ++--- pkg/tsdb/elasticsearch/data_query.go | 27 ++++++++++++------- pkg/tsdb/elasticsearch/data_query_test.go | 1 - pkg/tsdb/elasticsearch/elasticsearch.go | 9 +++---- pkg/tsdb/elasticsearch/error_handling_test.go | 3 +-- pkg/tsdb/elasticsearch/response_parser.go | 3 +-- 12 files changed, 47 insertions(+), 49 deletions(-) diff --git a/pkg/tsdb/cloudwatch/annotation_query.go b/pkg/tsdb/cloudwatch/annotation_query.go index d2621419852..b99321a240d 100644 --- a/pkg/tsdb/cloudwatch/annotation_query.go +++ b/pkg/tsdb/cloudwatch/annotation_query.go @@ -11,7 +11,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) @@ -36,7 +35,7 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC if model.Period != nil && *model.Period != "" { p, err := strconv.ParseInt(*model.Period, 10, 64) if err != nil { - return nil, errorsource.DownstreamError(fmt.Errorf("query period must be an int"), false) + return nil, backend.DownstreamError(fmt.Errorf("query period must be an int")) } period = p } @@ -82,13 +81,13 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC } resp, err := cli.DescribeAlarms(params) if err != nil { - result = errorsource.AddDownstreamErrorToResponse(query.RefID, result, fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarms", err)) + result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarms", err))) return result, nil } alarmNames = filterAlarms(resp, utils.Depointerizer(model.Namespace), metricName, dimensions, statistic, period) } else { if model.Region == nil || model.Namespace == nil || metricName == "" || statistic == "" { - return result, errorsource.DownstreamError(errors.New("invalid annotations query"), false) + return result, backend.DownstreamError(errors.New("invalid annotations query")) } var qd []*cloudwatch.Dimension @@ -113,7 +112,7 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC } resp, err := cli.DescribeAlarmsForMetric(params) if err != nil { - result = errorsource.AddDownstreamErrorToResponse(query.RefID, result, fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarmsForMetric", err)) + result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarmsForMetric", err))) return result, nil } for _, alarm := range resp.MetricAlarms { @@ -131,7 +130,7 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC } resp, err := cli.DescribeAlarmHistory(params) if err != nil { - result = errorsource.AddDownstreamErrorToResponse(query.RefID, result, fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarmHistory", err)) + result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(fmt.Errorf("%v: %w", "failed to call cloudwatch:DescribeAlarmHistory", err))) return result, nil } for _, history := range resp.AlarmHistoryItems { diff --git a/pkg/tsdb/cloudwatch/get_metric_data_executor.go b/pkg/tsdb/cloudwatch/get_metric_data_executor.go index 002fedd5639..6ad996ab6e1 100644 --- a/pkg/tsdb/cloudwatch/get_metric_data_executor.go +++ b/pkg/tsdb/cloudwatch/get_metric_data_executor.go @@ -7,7 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/aws/aws-sdk-go/service/cloudwatch/cloudwatchiface" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) @@ -28,7 +28,7 @@ func (e *cloudWatchExecutor) executeRequest(ctx context.Context, client cloudwat resp, err := client.GetMetricDataWithContext(ctx, metricDataInput) if err != nil { - return mdo, errorsource.DownstreamError(err, false) + return mdo, backend.DownstreamError(err) } mdo = append(mdo, resp) diff --git a/pkg/tsdb/cloudwatch/log_actions.go b/pkg/tsdb/cloudwatch/log_actions.go index 2aa6d907095..cdf80b977f4 100644 --- a/pkg/tsdb/cloudwatch/log_actions.go +++ b/pkg/tsdb/cloudwatch/log_actions.go @@ -17,7 +17,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "golang.org/x/sync/errgroup" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" @@ -101,7 +100,7 @@ func (e *cloudWatchExecutor) executeLogActions(ctx context.Context, req *backend dataframe, err := e.executeLogAction(ectx, logsQuery, query, req.PluginContext) if err != nil { resultChan <- backend.Responses{ - query.RefID: errorsource.Response(err), + query.RefID: backend.ErrorResponseWithErrorSource(err), } return nil } @@ -182,12 +181,12 @@ func (e *cloudWatchExecutor) handleGetLogEvents(ctx context.Context, logsClient } if logsQuery.LogGroupName == "" { - return nil, errorsource.DownstreamError(fmt.Errorf("Error: Parameter 'logGroupName' is required"), false) + return nil, backend.DownstreamError(fmt.Errorf("Error: Parameter 'logGroupName' is required")) } queryRequest.SetLogGroupName(logsQuery.LogGroupName) if logsQuery.LogStreamName == "" { - return nil, errorsource.DownstreamError(fmt.Errorf("Error: Parameter 'logStreamName' is required"), false) + return nil, backend.DownstreamError(fmt.Errorf("Error: Parameter 'logStreamName' is required")) } queryRequest.SetLogStreamName(logsQuery.LogStreamName) @@ -201,7 +200,7 @@ func (e *cloudWatchExecutor) handleGetLogEvents(ctx context.Context, logsClient logEvents, err := logsClient.GetLogEventsWithContext(ctx, queryRequest) if err != nil { - return nil, errorsource.DownstreamError(err, false) + return nil, backend.DownstreamError(err) } messages := make([]*string, 0) @@ -230,7 +229,7 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c endTime := timeRange.To if !startTime.Before(endTime) { - return nil, errorsource.DownstreamError(fmt.Errorf("invalid time range: start time must be before end time"), false) + return nil, backend.DownstreamError(fmt.Errorf("invalid time range: start time must be before end time")) } if logsQuery.QueryLanguage == nil { cwli := dataquery.LogsQueryLanguageCWLI @@ -290,7 +289,7 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c e.logger.FromContext(ctx).Debug("ExecuteStartQuery rate exceeded", "err", awsErr) err = &AWSError{Code: throttlingException, Message: err.Error()} } - err = errorsource.DownstreamError(err, false) + err = backend.DownstreamError(err) } return resp, err } @@ -335,7 +334,7 @@ func (e *cloudWatchExecutor) executeStopQuery(ctx context.Context, logsClient cl response = &cloudwatchlogs.StopQueryOutput{Success: aws.Bool(false)} err = nil } else { - err = errorsource.DownstreamError(err, false) + err = backend.DownstreamError(err) } } @@ -365,7 +364,7 @@ func (e *cloudWatchExecutor) executeGetQueryResults(ctx context.Context, logsCli if errors.As(err, &awsErr) { err = &AWSError{Code: awsErr.Code(), Message: err.Error()} } - err = errorsource.DownstreamError(err, false) + err = backend.DownstreamError(err) } return getQueryResultsResponse, err } diff --git a/pkg/tsdb/cloudwatch/log_sync_query.go b/pkg/tsdb/cloudwatch/log_sync_query.go index f5193863e45..4377150f858 100644 --- a/pkg/tsdb/cloudwatch/log_sync_query.go +++ b/pkg/tsdb/cloudwatch/log_sync_query.go @@ -11,7 +11,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" @@ -56,9 +55,9 @@ var executeSyncLogQuery = func(ctx context.Context, e *cloudWatchExecutor, req * } getQueryResultsOutput, err := e.syncQuery(ctx, logsClient, q, logsQuery, instance.Settings.LogsTimeout.Duration) - var sourceError errorsource.Error + var sourceError backend.ErrorWithSource if errors.As(err, &sourceError) { - errorsource.AddErrorToResponse(refId, resp, sourceError) + resp.Responses[refId] = backend.ErrorResponseWithErrorSource(sourceError) continue } if err != nil { diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go index 2aff6763bd6..d073fdb76cd 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/log" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) @@ -311,7 +310,7 @@ func (q *CloudWatchQuery) migrateLegacyQuery(query metricsDataQuery) { func (q *CloudWatchQuery) validateAndSetDefaults(refId string, metricsDataQuery metricsDataQuery, startTime, endTime time.Time, defaultRegionValue string, crossAccountQueryingEnabled bool) error { if metricsDataQuery.Statistic == nil && metricsDataQuery.Statistics == nil { - return errorsource.DownstreamError(fmt.Errorf("query must have either statistic or statistics field"), false) + return backend.DownstreamError(fmt.Errorf("query must have either statistic or statistics field")) } var err error diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 4025aeba77c..8119b3b4692 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -8,7 +8,6 @@ import ( "golang.org/x/sync/errgroup" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" @@ -24,13 +23,13 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba resp := backend.NewQueryDataResponse() if len(req.Queries) == 0 { - return nil, errorsource.DownstreamError(fmt.Errorf("request contains no queries"), false) + return nil, backend.DownstreamError(fmt.Errorf("request contains no queries")) } // startTime and endTime are always the same for all queries startTime := req.Queries[0].TimeRange.From endTime := req.Queries[0].TimeRange.To if !startTime.Before(endTime) { - return nil, errorsource.DownstreamError(fmt.Errorf("invalid time range: start time must be before end time"), false) + return nil, backend.DownstreamError(fmt.Errorf("invalid time range: start time must be before end time")) } instance, err := e.getInstance(ctx, req.PluginContext) @@ -129,7 +128,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba } if err := eg.Wait(); err != nil { - dataResponse := errorsource.Response(fmt.Errorf("metric request error: %w", err)) + dataResponse := backend.ErrorResponseWithErrorSource(fmt.Errorf("metric request error: %w", err)) resultChan <- &responseWrapper{ RefId: getQueryRefIdFromErrorString(err.Error(), requestQueries), DataResponse: &dataResponse, diff --git a/pkg/tsdb/elasticsearch/client/client.go b/pkg/tsdb/elasticsearch/client/client.go index a9de26e65c9..9ab71a8bc84 100644 --- a/pkg/tsdb/elasticsearch/client/client.go +++ b/pkg/tsdb/elasticsearch/client/client.go @@ -17,9 +17,9 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/backend/tracing" - exp "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" ) // Used in logging to mark a stage @@ -182,9 +182,9 @@ func (c *baseClientImpl) ExecuteMultisearch(r *MultiSearchRequest) (*MultiSearch status = "cancelled" } lp := []any{"error", err, "status", status, "duration", time.Since(start), "stage", StageDatabaseRequest} - sourceErr := exp.Error{} + sourceErr := backend.ErrorWithSource{} if errors.As(err, &sourceErr) { - lp = append(lp, "statusSource", sourceErr.Source()) + lp = append(lp, "statusSource", sourceErr.ErrorSource()) } if clientRes != nil { lp = append(lp, "statusCode", clientRes.StatusCode) diff --git a/pkg/tsdb/elasticsearch/data_query.go b/pkg/tsdb/elasticsearch/data_query.go index 4fb4eaaa9f1..98353c771c6 100644 --- a/pkg/tsdb/elasticsearch/data_query.go +++ b/pkg/tsdb/elasticsearch/data_query.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/log" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/components/simplejson" es "github.com/grafana/grafana/pkg/tsdb/elasticsearch/client" @@ -52,7 +51,8 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { if err != nil { mq, _ := json.Marshal(e.dataQueries) e.logger.Error("Failed to parse queries", "error", err, "queries", string(mq), "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return errorsource.AddPluginErrorToResponse(e.dataQueries[0].RefID, response, err), nil + response.Responses[e.dataQueries[0].RefID] = backend.ErrorResponseWithErrorSource(err) + return response, nil } ms := e.client.MultiSearch() @@ -63,7 +63,8 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { if err := e.processQuery(q, ms, from, to); err != nil { mq, _ := json.Marshal(q) e.logger.Error("Failed to process query to multisearch request builder", "error", err, "query", string(mq), "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return errorsource.AddPluginErrorToResponse(q.RefID, response, err), nil + response.Responses[q.RefID] = backend.ErrorResponseWithErrorSource(err) + return response, nil } } @@ -71,21 +72,28 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { if err != nil { mqs, _ := json.Marshal(e.dataQueries) e.logger.Error("Failed to build multisearch request", "error", err, "queriesLength", len(queries), "queries", string(mqs), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return errorsource.AddPluginErrorToResponse(e.dataQueries[0].RefID, response, err), nil + response.Responses[e.dataQueries[0].RefID] = backend.ErrorResponseWithErrorSource(err) + return response, nil } e.logger.Info("Prepared request", "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) res, err := e.client.ExecuteMultisearch(req) if err != nil { if backend.IsDownstreamHTTPError(err) { - err = errorsource.DownstreamError(err, false) + err = backend.DownstreamError(err) } - return errorsource.AddErrorToResponse(e.dataQueries[0].RefID, response, err), nil + response.Responses[e.dataQueries[0].RefID] = backend.ErrorResponseWithErrorSource(err) + return response, nil } if res.Status >= 400 { - errWithSource := errorsource.SourceError(backend.ErrorSourceFromHTTPStatus(res.Status), fmt.Errorf("unexpected status code: %d", res.Status), false) - return errorsource.AddErrorToResponse(e.dataQueries[0].RefID, response, errWithSource), nil + statusErr := fmt.Errorf("unexpected status code: %d", res.Status) + if backend.ErrorSourceFromHTTPStatus(res.Status) == backend.ErrorSourceDownstream { + response.Responses[e.dataQueries[0].RefID] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(statusErr)) + } else { + response.Responses[e.dataQueries[0].RefID] = backend.ErrorResponseWithErrorSource(backend.PluginError(statusErr)) + } + return response, nil } return parseResponse(e.ctx, res.Responses, queries, e.client.GetConfiguredFields(), e.keepLabelsInResponse, e.logger) @@ -94,8 +102,7 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { func (e *elasticsearchDataQuery) processQuery(q *Query, ms *es.MultiSearchRequestBuilder, from, to int64) error { err := isQueryWithError(q) if err != nil { - err = errorsource.DownstreamError(fmt.Errorf("received invalid query. %w", err), false) - return err + return backend.DownstreamError(fmt.Errorf("received invalid query. %w", err)) } defaultTimeField := e.client.GetConfiguredFields().TimeField diff --git a/pkg/tsdb/elasticsearch/data_query_test.go b/pkg/tsdb/elasticsearch/data_query_test.go index 6f7db11d9be..412bab8f56a 100644 --- a/pkg/tsdb/elasticsearch/data_query_test.go +++ b/pkg/tsdb/elasticsearch/data_query_test.go @@ -1431,7 +1431,6 @@ func TestExecuteElasticsearchDataQuery(t *testing.T) { "query": "foo", }`, from, to) require.NoError(t, err) - require.Equal(t, res.Responses["A"].ErrorSource, backend.ErrorSourcePlugin) require.Equal(t, res.Responses["A"].Error.Error(), "invalid character '}' looking for beginning of object key string") })) }) diff --git a/pkg/tsdb/elasticsearch/elasticsearch.go b/pkg/tsdb/elasticsearch/elasticsearch.go index f48d1d64889..ddd8671a114 100644 --- a/pkg/tsdb/elasticsearch/elasticsearch.go +++ b/pkg/tsdb/elasticsearch/elasticsearch.go @@ -19,7 +19,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" "github.com/grafana/grafana-plugin-sdk-go/backend/log" - exp "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" es "github.com/grafana/grafana/pkg/tsdb/elasticsearch/client" ) @@ -98,11 +97,11 @@ func newInstanceSettings(httpClientProvider *httpclient.Provider) datasource.Ins timeField, ok := jsonData["timeField"].(string) if !ok { - return nil, exp.DownstreamError(errors.New("timeField cannot be cast to string"), false) + return nil, backend.DownstreamError(errors.New("timeField cannot be cast to string")) } if timeField == "" { - return nil, exp.DownstreamError(errors.New("elasticsearch time field name is required"), false) + return nil, backend.DownstreamError(errors.New("elasticsearch time field name is required")) } logLevelField, ok := jsonData["logLevelField"].(string) @@ -221,9 +220,9 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq status = "cancelled" } lp := []any{"error", err, "status", status, "duration", time.Since(start), "stage", es.StageDatabaseRequest, "resourcePath", req.Path} - sourceErr := exp.Error{} + sourceErr := backend.ErrorWithSource{} if errors.As(err, &sourceErr) { - lp = append(lp, "statusSource", sourceErr.Source()) + lp = append(lp, "statusSource", sourceErr.ErrorSource()) } if response != nil { lp = append(lp, "statusCode", response.StatusCode) diff --git a/pkg/tsdb/elasticsearch/error_handling_test.go b/pkg/tsdb/elasticsearch/error_handling_test.go index c6407a7e35f..2fb5063c7f2 100644 --- a/pkg/tsdb/elasticsearch/error_handling_test.go +++ b/pkg/tsdb/elasticsearch/error_handling_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/stretchr/testify/require" ) @@ -121,7 +120,7 @@ func TestErrorTooManyDateHistogramBuckets(t *testing.T) { require.True(t, ok) require.Len(t, dataResponse.Frames, 0) require.ErrorContains(t, dataResponse.Error, "Trying to create too many buckets. Must be less than or equal to: [65536].") - var sourceErr errorsource.Error + var sourceErr backend.ErrorWithSource ok = errors.As(dataResponse.Error, &sourceErr) require.True(t, ok) require.Equal(t, sourceErr.ErrorSource().String(), "downstream") diff --git a/pkg/tsdb/elasticsearch/response_parser.go b/pkg/tsdb/elasticsearch/response_parser.go index 158a0fca03b..541006fedc2 100644 --- a/pkg/tsdb/elasticsearch/response_parser.go +++ b/pkg/tsdb/elasticsearch/response_parser.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/backend/tracing" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" @@ -75,7 +74,7 @@ func parseResponse(ctx context.Context, responses []*es.SearchResponse, targets resSpan.End() logger.Error("Processing error response from Elasticsearch", "error", string(me), "query", string(mt)) errResult := getErrorFromElasticResponse(res) - result.Responses[target.RefID] = errorsource.Response(errorsource.DownstreamError(errors.New(errResult), false)) + result.Responses[target.RefID] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(errors.New(errResult))) continue } From e3e7086a91c706206217237906e69d76c27aba12 Mon Sep 17 00:00:00 2001 From: Oscar Kilhed Date: Thu, 12 Dec 2024 10:22:49 +0100 Subject: [PATCH 04/13] GrafanaData: Add now parameter to dateMath.parse to subsitute now string (#97151) * Add now parameter to dateMath.parse to subsitute now string * Create new function, deprecate the old * Add docstring * Typo --- .../grafana-data/src/datetime/datemath.ts | 57 ++++++++++++++----- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/packages/grafana-data/src/datetime/datemath.ts b/packages/grafana-data/src/datetime/datemath.ts index e95e4575401..1392e5d3544 100644 --- a/packages/grafana-data/src/datetime/datemath.ts +++ b/packages/grafana-data/src/datetime/datemath.ts @@ -1,12 +1,13 @@ import { isDate } from 'lodash'; -import { TimeZone } from '../types/time'; +import { TimeZone } from '@grafana/schema'; import { DateTime, dateTime, dateTimeAsMoment, dateTimeForTimeZone, + DateTimeInput, DurationUnit, isDateTime, ISO_8601, @@ -35,6 +36,7 @@ export function isMathString(text: string | DateTime | Date): boolean { } /** + * @deprecated use toDateTime instead * Parses different types input to a moment instance. There is a specific formatting language that can be used * if text arg is string. See unit tests for examples. * @param text @@ -50,14 +52,41 @@ export function parse( if (!text) { return undefined; } + return toDateTime(text, { roundUp, timezone, fiscalYearStartMonth }); +} - if (typeof text !== 'string') { - if (isDateTime(text)) { - return text; +export interface ConversionOptions { + /** + * Set the time to endOf time unit, otherwise to startOf time unit. + */ + roundUp?: boolean; + /** + * Only string 'utc' is acceptable here, for anything else, local timezone is used. + */ + timezone?: TimeZone; + /** + * Setting for which month is the first month of the fiscal year. + */ + fiscalYearStartMonth?: number; + /** + * DateTimeInput to use as now. Useful when parsing multiple values and now needs to be the same. Without this, comparing results from subsequent parses is not guaranteed to be deterministic. + */ + now?: DateTimeInput; +} + +/** + * @param dateTimeRep A DateTime object, a Date object or a string representation of a specific time. + * @param options Options for converting to DateTime + * @returns A DateTime object if possible, undefined if not. + */ +export function toDateTime(dateTimeRep: string | DateTime | Date, options: ConversionOptions): DateTime | undefined { + if (typeof dateTimeRep !== 'string') { + if (isDateTime(dateTimeRep)) { + return dateTimeRep; } - if (isDate(text)) { - return dateTime(text); + if (isDate(dateTimeRep)) { + return dateTime(dateTimeRep); } // We got some non string which is not a moment nor Date. TS should be able to check for that but not always. @@ -68,17 +97,17 @@ export function parse( let index = -1; let parseString = ''; - if (text.substring(0, 3) === 'now') { - time = dateTimeForTimeZone(timezone); - mathString = text.substring('now'.length); + if (dateTimeRep.substring(0, 3) === 'now') { + time = dateTimeForTimeZone(options.timezone, options.now); + mathString = dateTimeRep.substring('now'.length); } else { - index = text.indexOf('||'); + index = dateTimeRep.indexOf('||'); if (index === -1) { - parseString = text; + parseString = dateTimeRep; mathString = ''; // nothing else } else { - parseString = text.substring(0, index); - mathString = text.substring(index + 2); + parseString = dateTimeRep.substring(0, index); + mathString = dateTimeRep.substring(index + 2); } // We're going to just require ISO8601 timestamps, k? time = dateTime(parseString, ISO_8601); @@ -88,7 +117,7 @@ export function parse( return time; } - return parseDateMath(mathString, time, roundUp, fiscalYearStartMonth); + return parseDateMath(mathString, time, options.roundUp, options.fiscalYearStartMonth); } } From 7ad004236e97547996a3c155760c437317f5a36a Mon Sep 17 00:00:00 2001 From: Santiago Date: Thu, 12 Dec 2024 06:30:23 -0300 Subject: [PATCH 05/13] Alerting: Change the description for the 'Message' field in webhooks (#96153) --- .../ngalert/notifier/channels_config/available_channels.go | 2 +- public/app/features/alerting/unified/mockGrafanaNotifiers.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index 95352dd6f49..07df8e78251 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -959,7 +959,7 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, { // New in 9.3. Label: "Message", - Description: "Custom message. You can use template variables.", + Description: "Templated message to be used in the payload's \"message\" field.", Element: ElementTypeTextArea, PropertyName: "message", Placeholder: alertingTemplates.DefaultMessageEmbed, diff --git a/public/app/features/alerting/unified/mockGrafanaNotifiers.ts b/public/app/features/alerting/unified/mockGrafanaNotifiers.ts index db5a7d923c5..a15904148ea 100644 --- a/public/app/features/alerting/unified/mockGrafanaNotifiers.ts +++ b/public/app/features/alerting/unified/mockGrafanaNotifiers.ts @@ -1750,7 +1750,7 @@ export const grafanaAlertNotifiers: Record = { element: 'textarea', inputType: '', label: 'Message', - description: 'Custom message. You can use template variables.', + description: 'Templated message to be used in the payload\'s "message" field.', placeholder: '{{ template "default.message" . }}', propertyName: 'message', selectOptions: null, From d85353926dd992438a060dd8ebbff07931f73a38 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Thu, 12 Dec 2024 09:48:27 +0000 Subject: [PATCH 06/13] Metrics: Export multi registry (#97662) * export multi registry * fixup --- pkg/infra/metrics/service.go | 4 ++-- pkg/infra/metrics/service_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/infra/metrics/service.go b/pkg/infra/metrics/service.go index 6b7f1d2354f..3a964dfc58f 100644 --- a/pkg/infra/metrics/service.go +++ b/pkg/infra/metrics/service.go @@ -66,7 +66,7 @@ func ProvideRegisterer() prometheus.Registerer { func ProvideGatherer() prometheus.Gatherer { k8sGatherer := newAddPrefixWrapper(legacyregistry.DefaultGatherer) - return newMultiRegistry(k8sGatherer, prometheus.DefaultGatherer) + return NewMultiRegistry(k8sGatherer, prometheus.DefaultGatherer) } func ProvideRegistererForTest() prometheus.Registerer { @@ -125,7 +125,7 @@ type multiRegistry struct { gatherers []prometheus.Gatherer } -func newMultiRegistry(gatherers ...prometheus.Gatherer) *multiRegistry { +func NewMultiRegistry(gatherers ...prometheus.Gatherer) *multiRegistry { return &multiRegistry{ gatherers: gatherers, } diff --git a/pkg/infra/metrics/service_test.go b/pkg/infra/metrics/service_test.go index af489a23965..6d32dde6480 100644 --- a/pkg/infra/metrics/service_test.go +++ b/pkg/infra/metrics/service_test.go @@ -51,7 +51,7 @@ func TestGathererPrefixWrapper_Gather(t *testing.T) { func TestMultiRegistry_Gather(t *testing.T) { one := &mockGatherer{} two := &mockGatherer{} - g := newMultiRegistry(one, two) + g := NewMultiRegistry(one, two) t.Run("should merge and sort metrics", func(t *testing.T) { one.GatherFunc = func() ([]*dto.MetricFamily, error) { From 5be8a9b3c3b5ee5b7b4e2d5b1a69c37af4ba5716 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Thu, 12 Dec 2024 09:50:36 +0000 Subject: [PATCH 07/13] Alerting/chore: Deprecate MockDataSourceSrv and remove from Alerting tests (#97267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Mark MockDataSourceSrv as deprecated * Add `module` into necessary existing mock datasources * Remove config mock * Update helper method for test datasources to more accurately reflect real behaviour * Update alerting/unified tests to remove mock datasource srv * Lint fix * Remove some unused re-exports * Make mock mimir use correct module Ⓜ️ * Add necessary handlers for mimir query editor responses * Lint fix * Add implementation to mock mimir data source --- .betterer.results | 8 -- .../alerting/unified/AlertGroups.test.tsx | 12 +-- .../alerting/unified/CloneRuleEditor.test.tsx | 4 +- .../unified/PanelAlertTabContent.test.tsx | 78 ++++++++++--------- .../alerting/unified/RuleList.test.tsx | 51 ++---------- .../alerting/unified/Settings.test.tsx | 7 +- .../components/rules/RulesFilter.test.tsx | 5 +- .../settings/AlertmanagerConfig.test.tsx | 3 + .../components/settings/__mocks__/server.ts | 58 ++++++-------- .../unified/hooks/useFilteredRules.test.ts | 5 +- public/app/features/alerting/unified/mocks.ts | 2 +- .../unified/mocks/server/configure.ts | 3 +- .../mocks/server/handlers/datasources.ts | 25 +++++- .../unified/state/AlertingQueryRunner.test.ts | 20 +++-- .../alerting/unified/testSetup/datasources.ts | 14 +++- 15 files changed, 132 insertions(+), 163 deletions(-) diff --git a/.betterer.results b/.betterer.results index 510154a2371..0dcf32bb34a 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1663,14 +1663,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], [0, 0, 0, "No untranslated strings. Wrap text with ", "6"] ], - "public/app/features/alerting/unified/components/settings/__mocks__/server.ts:5381": [ - [0, 0, 0, "Do not re-export imported variable (\`DataSourcesResponse\`)", "0"], - [0, 0, 0, "Do not re-export imported variable (\`AdminConfigResponse\`)", "1"], - [0, 0, 0, "Do not re-export imported variable (\`AlertmanagersResponse\`)", "2"], - [0, 0, 0, "Do not re-export imported variable (\`InternalAlertmanagerConfiguration\`)", "3"], - [0, 0, 0, "Do not re-export imported variable (\`VanillaAlertmanagerConfiguration\`)", "4"], - [0, 0, 0, "Do not re-export imported variable (\`alertmanagerConfigurationHistory\`)", "5"] - ], "public/app/features/alerting/unified/components/silences/MatchersField.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] diff --git a/public/app/features/alerting/unified/AlertGroups.test.tsx b/public/app/features/alerting/unified/AlertGroups.test.tsx index 6b4f3ca1894..a39b0348f15 100644 --- a/public/app/features/alerting/unified/AlertGroups.test.tsx +++ b/public/app/features/alerting/unified/AlertGroups.test.tsx @@ -2,18 +2,12 @@ import { render, waitFor, waitForElementToBeRemoved } from 'test/test-utils'; import { byRole, byTestId, byText } from 'testing-library-selector'; import { selectors } from '@grafana/e2e-selectors'; -import { setDataSourceSrv } from '@grafana/runtime'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { AccessControlAction } from 'app/types'; import AlertGroups from './AlertGroups'; import { fetchAlertGroups } from './api/alertmanager'; -import { - MockDataSourceSrv, - grantUserPermissions, - mockAlertGroup, - mockAlertmanagerAlert, - mockDataSource, -} from './mocks'; +import { grantUserPermissions, mockAlertGroup, mockAlertmanagerAlert, mockDataSource } from './mocks'; import { AlertmanagerProvider } from './state/AlertmanagerContext'; import { DataSourceType } from './utils/datasource'; @@ -63,7 +57,7 @@ describe('AlertGroups', () => { }); beforeEach(() => { - setDataSourceSrv(new MockDataSourceSrv(dataSources)); + setupDataSources(dataSources.am); }); afterEach(() => { diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx index 27e72824154..3eff21b9b26 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx @@ -3,7 +3,6 @@ import { FormProvider, useForm } from 'react-hook-form'; import { getWrapper, render, waitFor, waitForElementToBeRemoved, within } from 'test/test-utils'; import { byRole, byTestId, byText } from 'testing-library-selector'; -import { setDataSourceSrv } from '@grafana/runtime'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { AccessControlAction } from '../../../types'; @@ -18,7 +17,6 @@ import { CloneRuleEditor, cloneRuleDefinition } from './CloneRuleEditor'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { mockFeatureDiscoveryApi, setupMswServer } from './mockApi'; import { - MockDataSourceSrv, grantUserPermissions, mockDataSource, mockRulerAlertingRule, @@ -74,7 +72,7 @@ describe('CloneRuleEditor', function () { describe('Grafana-managed rules', function () { it('should populate form values from the existing alert rule', async function () { - setDataSourceSrv(new MockDataSourceSrv({})); + setupDataSources(); render( , diff --git a/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx b/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx index 4d87a26183b..488719b2e65 100644 --- a/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx +++ b/public/app/features/alerting/unified/PanelAlertTabContent.test.tsx @@ -1,13 +1,11 @@ import { render } from 'test/test-utils'; import { byTestId, byText } from 'testing-library-selector'; -import { DataSourceApi } from '@grafana/data'; -import { PromOptions, PrometheusDatasource } from '@grafana/prometheus'; -import { setDataSourceSrv, setPluginLinksHook } from '@grafana/runtime'; -import * as ruleActionButtons from 'app/features/alerting/unified/components/rules/RuleActionsButtons'; +import { PromOptions } from '@grafana/prometheus'; +import { setPluginLinksHook } from '@grafana/runtime'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; import { PanelModel } from 'app/features/dashboard/state/PanelModel'; -import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { AccessControlAction } from 'app/types'; import { AlertQuery, PromRulesResponse } from 'app/types/unified-alerting-dto'; @@ -16,7 +14,6 @@ import * as apiRuler from './api/ruler'; import * as alertingAbilities from './hooks/useAbilities'; import { mockAlertRuleApi, setupMswServer } from './mockApi'; import { - MockDataSourceSrv, grantUserPermissions, mockDataSource, mockPromAlert, @@ -26,42 +23,51 @@ import { } from './mocks'; import { captureRequests } from './mocks/server/events'; import { RuleFormValues } from './types/rule-form'; -import * as config from './utils/config'; import { Annotation } from './utils/constants'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; -jest.mock('./api/prometheus'); jest.mock('./api/ruler'); -jest.mock('../../../core/hooks/useMediaQueryChange'); - -jest.spyOn(config, 'getAllDataSources'); -jest.spyOn(ruleActionButtons, 'matchesWidth').mockReturnValue(false); -jest.spyOn(apiRuler, 'rulerUrlBuilder'); jest.spyOn(alertingAbilities, 'useAlertRuleAbility'); + +const prometheusModuleSettings = { alerting: true, module: 'core:plugin/prometheus' }; + const dataSources = { - prometheus: mockDataSource({ - name: 'Prometheus', - type: DataSourceType.Prometheus, - isDefault: false, - }), - default: mockDataSource({ - name: 'Default', - type: DataSourceType.Prometheus, - isDefault: true, - }), + prometheus: mockDataSource( + { + name: 'Prometheus', + type: DataSourceType.Prometheus, + isDefault: false, + jsonData: { manageAlerts: true }, + }, + prometheusModuleSettings + ), + default: mockDataSource( + { + name: 'Default', + type: DataSourceType.Prometheus, + isDefault: true, + jsonData: { manageAlerts: true }, + }, + prometheusModuleSettings + ), + prometheusMinInterval: mockDataSource( + { + name: 'Prometheus Min Interval', + type: DataSourceType.Prometheus, + isDefault: false, + jsonData: { manageAlerts: true, timeInterval: '7m' }, + }, + prometheusModuleSettings + ), }; -dataSources.prometheus.meta.alerting = true; -dataSources.default.meta.alerting = true; const mocks = { - getAllDataSources: jest.mocked(config.getAllDataSources), useAlertRuleAbilityMock: jest.mocked(alertingAbilities.useAlertRuleAbility), rulerBuilderMock: jest.mocked(apiRuler.rulerUrlBuilder), }; -const renderAlertTabContent = (dashboard: DashboardModel, panel: PanelModel) => { +const renderAlertTabContent = (dashboard: DashboardModel, panel: PanelModel) => render(); -}; const promResponse: PromRulesResponse = { status: 'success', @@ -190,19 +196,13 @@ describe('PanelAlertTabContent', () => { AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleExternalWrite, ]); + setupDataSources(...Object.values(dataSources)); setPluginLinksHook(() => ({ links: [], isLoading: false, })); - mocks.getAllDataSources.mockReturnValue(Object.values(dataSources)); - const dsService = new MockDataSourceSrv(dataSources); - dsService.datasources[dataSources.prometheus.uid] = new PrometheusDatasource( - dataSources.prometheus - ) as DataSourceApi; - dsService.datasources[dataSources.default.uid] = new PrometheusDatasource(dataSources.default) as DataSourceApi; - setDataSourceSrv(dsService); mocks.rulerBuilderMock.mockReturnValue({ rules: () => ({ path: `api/ruler/${GRAFANA_RULES_SOURCE_NAME}/api/v1/rules` }), namespace: () => ({ path: 'ruler' }), @@ -297,13 +297,15 @@ describe('PanelAlertTabContent', () => { }); it('Will take into account datasource minInterval', async () => { - (getDatasourceSrv() as unknown as MockDataSourceSrv).datasources[dataSources.prometheus.uid].interval = '7m'; - renderAlertTabContent( dashboard, new PanelModel({ ...panel, maxDataPoints: 100, + datasource: { + type: 'prometheus', + uid: dataSources.prometheusMinInterval.uid, + }, }) ); @@ -318,7 +320,7 @@ describe('PanelAlertTabContent', () => { refId: 'A', datasource: { type: 'prometheus', - uid: 'mock-ds-2', + uid: 'mock-ds-4', }, interval: '', intervalMs: 420000, diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index 6fee153615b..8c6caf63e91 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -7,25 +7,23 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { PluginExtensionTypes } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { DataSourceSrv, locationService, setAppEvents, setDataSourceSrv, usePluginLinks } from '@grafana/runtime'; +import { locationService, setAppEvents, usePluginLinks } from '@grafana/runtime'; import appEvents from 'app/core/app_events'; -import * as ruleActionButtons from 'app/features/alerting/unified/components/rules/RuleActionsButtons'; import { mockUserApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; import { setAlertmanagerChoices } from 'app/features/alerting/unified/mocks/server/configure'; import * as actions from 'app/features/alerting/unified/state/actions'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { getMockUser } from 'app/features/users/__mocks__/userMocks'; import { AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types'; import { PromAlertingRuleState, PromApplication } from 'app/types/unified-alerting-dto'; -import * as analytics from './Analytics'; import RuleList from './RuleList'; import { discoverFeaturesByUid } from './api/buildInfo'; import { fetchRules } from './api/prometheus'; import * as apiRuler from './api/ruler'; import { fetchRulerRules } from './api/ruler'; import { - MockDataSourceSrv, getPotentiallyPausedRulerRules, grantUserPermissions, mockDataSource, @@ -39,7 +37,6 @@ import { someRulerRules, } from './mocks'; import { setupPluginsExtensionsHook } from './testSetup/plugins'; -import * as config from './utils/config'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; jest.mock('@grafana/runtime', () => ({ @@ -51,11 +48,7 @@ jest.mock('@grafana/runtime', () => ({ jest.mock('./api/buildInfo'); jest.mock('./api/prometheus'); jest.mock('./api/ruler'); -jest.mock('../../../core/hooks/useMediaQueryChange'); -jest.spyOn(ruleActionButtons, 'matchesWidth').mockReturnValue(false); -jest.spyOn(analytics, 'logInfo'); -jest.spyOn(config, 'getAllDataSources'); jest.spyOn(actions, 'rulesInSameGroupHaveInvalidFor').mockReturnValue([]); jest.spyOn(apiRuler, 'rulerUrlBuilder'); @@ -63,7 +56,6 @@ setAppEvents(appEvents); setupPluginsExtensionsHook(); const mocks = { - getAllDataSourcesMock: jest.mocked(config.getAllDataSources), usePluginLinksMock: jest.mocked(usePluginLinks), rulesInSameGroupHaveInvalidForMock: jest.mocked(actions.rulesInSameGroupHaveInvalidFor), @@ -173,18 +165,14 @@ describe('RuleList', () => { ], isLoading: false, }); + setupDataSources(...Object.values(dataSources)); }); afterEach(() => { jest.resetAllMocks(); - setDataSourceSrv(undefined as unknown as DataSourceSrv); }); it('load & show rule groups from multiple cloud data sources', async () => { - mocks.getAllDataSourcesMock.mockReturnValue(Object.values(dataSources)); - - setDataSourceSrv(new MockDataSourceSrv(dataSources)); - mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Prometheus, features: { @@ -276,9 +264,6 @@ describe('RuleList', () => { }); it('expand rule group, rule and alert details', async () => { - mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]); - - setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom })); mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -287,7 +272,7 @@ describe('RuleList', () => { }); mocks.api.fetchRules.mockImplementation((dataSourceName: string) => { - if (dataSourceName === GRAFANA_RULES_SOURCE_NAME) { + if (dataSourceName !== dataSources.prom.name) { return Promise.resolve([]); } else { return Promise.resolve([ @@ -427,9 +412,6 @@ describe('RuleList', () => { }); it('filters rules and alerts by labels', async () => { - mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]); - setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom })); - mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -439,7 +421,7 @@ describe('RuleList', () => { mocks.api.fetchRulerRules.mockResolvedValue({}); mocks.api.fetchRules.mockImplementation((dataSourceName: string) => { - if (dataSourceName === GRAFANA_RULES_SOURCE_NAME) { + if (dataSourceName !== dataSources.prom.name) { return Promise.resolve([]); } else { return Promise.resolve([ @@ -573,10 +555,6 @@ describe('RuleList', () => { it.skip('uses entire group when reordering after filtering', async () => { const user = userEvent.setup(); - mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]); - - setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom })); - mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -635,8 +613,6 @@ describe('RuleList', () => { AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleExternalWrite, ]); - mocks.getAllDataSourcesMock.mockReturnValue([]); - setDataSourceSrv(new MockDataSourceSrv({})); mocks.api.fetchRulerRules.mockImplementation(() => Promise.resolve(getPotentiallyPausedRulerRules(true))); mocks.api.fetchRules.mockImplementation((sourceName) => Promise.resolve(sourceName === 'grafana' ? pausedPromRules('grafana') : []) @@ -685,9 +661,6 @@ describe('RuleList', () => { function testCase(name: string, fn: () => Promise) { it(name, async () => { - mocks.getAllDataSourcesMock.mockReturnValue(Object.values(testDatasources)); - setDataSourceSrv(new MockDataSourceSrv(testDatasources)); - mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -776,8 +749,6 @@ describe('RuleList', () => { it('Export button should be visible when the user has alert read permissions', async () => { grantUserPermissions([AccessControlAction.AlertingRuleRead]); - mocks.getAllDataSourcesMock.mockReturnValue([]); - setDataSourceSrv(new MockDataSourceSrv({})); mocks.api.fetchRules.mockResolvedValue([ mockPromRuleNamespace({ name: 'foofolder', @@ -813,8 +784,6 @@ describe('RuleList', () => { AccessControlAction.AlertingRuleRead, ]); - mocks.getAllDataSourcesMock.mockReturnValue([]); - setDataSourceSrv(new MockDataSourceSrv({})); mocks.api.fetchRules.mockResolvedValue([]); mocks.api.fetchRulerRules.mockResolvedValue({}); @@ -832,8 +801,6 @@ describe('RuleList', () => { AccessControlAction.AlertingRuleRead, ]); - mocks.getAllDataSourcesMock.mockReturnValue([]); - setDataSourceSrv(new MockDataSourceSrv({})); mocks.api.fetchRules.mockResolvedValue(somePromRules('grafana')); mocks.api.fetchRulerRules.mockResolvedValue(someRulerRules); @@ -853,8 +820,6 @@ describe('RuleList', () => { AccessControlAction.AlertingRuleExternalWrite, ]); - mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]); - setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom })); mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -867,7 +832,7 @@ describe('RuleList', () => { renderRuleList(); - await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalled()); expect(ui.newRuleButton.get()).toBeInTheDocument(); }); @@ -878,8 +843,6 @@ describe('RuleList', () => { AccessControlAction.AlertingRuleExternalWrite, ]); - mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]); - setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom })); mocks.api.discoverFeaturesByUid.mockResolvedValue({ application: PromApplication.Cortex, features: { @@ -892,7 +855,7 @@ describe('RuleList', () => { renderRuleList(); - await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalled()); expect(ui.newRuleButton.get()).toBeInTheDocument(); }); }); diff --git a/public/app/features/alerting/unified/Settings.test.tsx b/public/app/features/alerting/unified/Settings.test.tsx index 0df8f2570f1..401b4b6fee7 100644 --- a/public/app/features/alerting/unified/Settings.test.tsx +++ b/public/app/features/alerting/unified/Settings.test.tsx @@ -4,11 +4,8 @@ import { render } from 'test/test-utils'; import { byRole, byTestId, byText } from 'testing-library-selector'; import SettingsPage from './Settings'; -import { - DataSourcesResponse, - setupGrafanaManagedServer, - withExternalOnlySetting, -} from './components/settings/__mocks__/server'; +import DataSourcesResponse from './components/settings/__mocks__/api/datasources.json'; +import { setupGrafanaManagedServer, withExternalOnlySetting } from './components/settings/__mocks__/server'; import { setupMswServer } from './mockApi'; import { grantUserRole } from './mocks'; diff --git a/public/app/features/alerting/unified/components/rules/RulesFilter.test.tsx b/public/app/features/alerting/unified/components/rules/RulesFilter.test.tsx index f2df2ef5747..1484cfaa4ec 100644 --- a/public/app/features/alerting/unified/components/rules/RulesFilter.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesFilter.test.tsx @@ -1,11 +1,10 @@ import { render, screen } from 'test/test-utils'; import { byLabelText, byRole } from 'testing-library-selector'; -import { locationService, setDataSourceSrv } from '@grafana/runtime'; +import { locationService } from '@grafana/runtime'; import { setupMswServer } from 'app/features/alerting/unified/mockApi'; import * as analytics from '../../Analytics'; -import { MockDataSourceSrv } from '../../mocks'; import { setupPluginsExtensionsHook } from '../../testSetup/plugins'; import RulesFilter from './Filter/RulesFilter'; @@ -21,8 +20,6 @@ jest.mock('./MultipleDataSourcePicker', () => { }; }); -setDataSourceSrv(new MockDataSourceSrv({})); - setupPluginsExtensionsHook(); const ui = { diff --git a/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx b/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx index 5cc98ea0587..c629073d589 100644 --- a/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx +++ b/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx @@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event'; import { render } from 'test/test-utils'; import { byRole } from 'testing-library-selector'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { AccessControlAction } from 'app/types'; import { setupMswServer } from '../../mockApi'; @@ -13,6 +14,7 @@ import AlertmanagerConfig from './AlertmanagerConfig'; import { EXTERNAL_VANILLA_ALERTMANAGER_UID, PROVISIONED_MIMIR_ALERTMANAGER_UID, + mockDataSources, setupVanillaAlertmanagerServer, } from './__mocks__/server'; @@ -75,6 +77,7 @@ describe('vanilla Alertmanager', () => { beforeEach(() => { setupVanillaAlertmanagerServer(server); + setupDataSources(...Object.values(mockDataSources)); grantUserPermissions([AccessControlAction.AlertingNotificationsRead, AccessControlAction.AlertingInstanceRead]); }); diff --git a/public/app/features/alerting/unified/components/settings/__mocks__/server.ts b/public/app/features/alerting/unified/components/settings/__mocks__/server.ts index b1bb83c5f26..387f499bc05 100644 --- a/public/app/features/alerting/unified/components/settings/__mocks__/server.ts +++ b/public/app/features/alerting/unified/components/settings/__mocks__/server.ts @@ -1,7 +1,6 @@ import { HttpResponse, delay, http } from 'msw'; import { SetupServerApi } from 'msw/node'; -import { setDataSourceSrv } from '@grafana/runtime'; import { AlertManagerCortexConfig, AlertManagerDataSourceJsonData, @@ -10,8 +9,7 @@ import { Receiver, } from 'app/plugins/datasource/alertmanager/types'; -import { MockDataSourceSrv, mockDataSource } from '../../../mocks'; -import * as config from '../../../utils/config'; +import { mockDataSource } from '../../../mocks'; import { DataSourceType } from '../../../utils/datasource'; import internalAlertmanagerConfig from './api/alertmanager/grafana/config/api/v1/alerts.json'; @@ -22,41 +20,34 @@ import datasources from './api/datasources.json'; import admin_config from './api/v1/ngalert/admin_config.json'; import alertmanagers from './api/v1/ngalert/alertmanagers.json'; -export { datasources as DataSourcesResponse }; -export { admin_config as AdminConfigResponse }; -export { alertmanagers as AlertmanagersResponse }; -export { internalAlertmanagerConfig as InternalAlertmanagerConfiguration }; -export { vanillaAlertmanagerConfig as VanillaAlertmanagerConfiguration }; -export { history as alertmanagerConfigurationHistory }; - export const EXTERNAL_VANILLA_ALERTMANAGER_UID = 'vanilla-alertmanager'; export const PROVISIONED_MIMIR_ALERTMANAGER_UID = 'provisioned-alertmanager'; -jest.spyOn(config, 'getAllDataSources'); - -const mocks = { - getAllDataSources: jest.mocked(config.getAllDataSources), -}; - export const mockDataSources = { - [EXTERNAL_VANILLA_ALERTMANAGER_UID]: mockDataSource({ - uid: EXTERNAL_VANILLA_ALERTMANAGER_UID, - name: EXTERNAL_VANILLA_ALERTMANAGER_UID, - type: DataSourceType.Alertmanager, - jsonData: { - implementation: AlertManagerImplementation.prometheus, + [EXTERNAL_VANILLA_ALERTMANAGER_UID]: mockDataSource( + { + uid: EXTERNAL_VANILLA_ALERTMANAGER_UID, + name: EXTERNAL_VANILLA_ALERTMANAGER_UID, + type: DataSourceType.Alertmanager, + jsonData: { + implementation: AlertManagerImplementation.prometheus, + }, }, - }), - [PROVISIONED_MIMIR_ALERTMANAGER_UID]: mockDataSource({ - uid: PROVISIONED_MIMIR_ALERTMANAGER_UID, - name: PROVISIONED_MIMIR_ALERTMANAGER_UID, - type: DataSourceType.Alertmanager, - jsonData: { - // this is a mutable data source type but we're making it readOnly - implementation: AlertManagerImplementation.mimir, + { module: 'core:plugin/alertmanager' } + ), + [PROVISIONED_MIMIR_ALERTMANAGER_UID]: mockDataSource( + { + uid: PROVISIONED_MIMIR_ALERTMANAGER_UID, + name: PROVISIONED_MIMIR_ALERTMANAGER_UID, + type: DataSourceType.Alertmanager, + jsonData: { + // this is a mutable data source type but we're making it readOnly + implementation: AlertManagerImplementation.mimir, + }, + readOnly: true, }, - readOnly: true, - }), + { module: 'core:plugin/alertmanager' } + ), }; export function setupGrafanaManagedServer(server: SetupServerApi) { @@ -72,9 +63,6 @@ export function setupGrafanaManagedServer(server: SetupServerApi) { } export function setupVanillaAlertmanagerServer(server: SetupServerApi) { - mocks.getAllDataSources.mockReturnValue(Object.values(mockDataSources)); - setDataSourceSrv(new MockDataSourceSrv(mockDataSources)); - server.use( createVanillaAlertmanagerConfigurationHandler(EXTERNAL_VANILLA_ALERTMANAGER_UID), ...createAlertmanagerConfigurationHandlers() diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts index 01c91c4cc0e..728bbfaf765 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts @@ -1,8 +1,7 @@ -import { setDataSourceSrv } from '@grafana/runtime'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { PromAlertingRuleState } from '../../../../types/unified-alerting-dto'; import { - MockDataSourceSrv, getCloudRule, mockAlertQuery, mockCombinedCloudRuleNamespace, @@ -25,7 +24,7 @@ const dataSources = { loki: mockDataSource({ uid: 'loki-1', name: 'loki' }), }; beforeAll(() => { - setDataSourceSrv(new MockDataSourceSrv(dataSources)); + setupDataSources(...Object.values(dataSources)); }); describe('filterRules', function () { diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index cf716feee59..818dc9b2d41 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -416,7 +416,7 @@ class MockDataSourceApi extends DataSourceApi { } } -// TODO This should be eventually moved to public/app/features/alerting/unified/testSetup/datasources.ts +/** @deprecated use `setupDatasources` instead */ export class MockDataSourceSrv implements DataSourceSrv { datasources: Record = {}; // @ts-ignore diff --git a/public/app/features/alerting/unified/mocks/server/configure.ts b/public/app/features/alerting/unified/mocks/server/configure.ts index 2738946b624..7e2d6c66c71 100644 --- a/public/app/features/alerting/unified/mocks/server/configure.ts +++ b/public/app/features/alerting/unified/mocks/server/configure.ts @@ -102,9 +102,10 @@ export function mimirDataSource() { url: 'https://mimir.local:9000', jsonData: { manageAlerts: true, + implementation: 'mimir', }, }, - { alerting: true } + { alerting: true, module: 'core:plugin/prometheus' } ); setupDataSources(dataSource); diff --git a/public/app/features/alerting/unified/mocks/server/handlers/datasources.ts b/public/app/features/alerting/unified/mocks/server/handlers/datasources.ts index b586367e6bc..3ea92548fb5 100644 --- a/public/app/features/alerting/unified/mocks/server/handlers/datasources.ts +++ b/public/app/features/alerting/unified/mocks/server/handlers/datasources.ts @@ -28,5 +28,28 @@ export const datasourceBuildInfoHandler = () => } ); -const datasourcesHandlers = [datasourceBuildInfoHandler()]; +// TODO: Add more accurate endpoint responses as tests require +const labelValuesHandler = () => + http.get('/api/datasources/uid/:datasourceUid/resources/api/v1/label/__name__/values', ({ params }) => { + return HttpResponse.json({ status: 'sucess', data: [] }); + }); + +// TODO: Add more accurate endpoint responses as tests require +const resourcesLabelsHandler = () => + http.get('/api/datasources/uid/:datasourceUid/resources/api/v1/labels', () => + HttpResponse.json({ status: 'success', data: [] }) + ); + +// TODO: Add more accurate endpoint responses as tests require +const resourcesMetadataHandler = () => + http.get('/api/datasources/uid/:datasourceUid/resources/api/v1/metadata', () => + HttpResponse.json({ status: 'success', data: {} }) + ); + +const datasourcesHandlers = [ + datasourceBuildInfoHandler(), + labelValuesHandler(), + resourcesLabelsHandler(), + resourcesMetadataHandler(), +]; export default datasourcesHandlers; diff --git a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts index e9935f7bc91..a926172c907 100644 --- a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts +++ b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts @@ -16,10 +16,18 @@ import { import { DataSourceSrv, DataSourceWithBackend, FetchResponse } from '@grafana/runtime'; import { DataQuery } from '@grafana/schema'; import { BackendSrv } from 'app/core/services/backend_srv'; +import { + EXTERNAL_VANILLA_ALERTMANAGER_UID, + mockDataSources, +} from 'app/features/alerting/unified/components/settings/__mocks__/server'; +import { setupMswServer } from 'app/features/alerting/unified/mockApi'; +import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; import { AlertingQueryResponse, AlertingQueryRunner } from './AlertingQueryRunner'; +setupMswServer(); + describe('AlertingQueryRunner', () => { it('should successfully map response and return panel data by refId', async () => { const response = createFetchResponse({ @@ -28,12 +36,12 @@ describe('AlertingQueryRunner', () => { B: { frames: [createDataFrameJSON([5, 6])] }, }, }); + setupDataSources(...Object.values(mockDataSources)); const runner = new AlertingQueryRunner( mockBackendSrv({ fetch: () => of(response), - }), - mockDataSourceSrv() + }) ); const data = runner.get(); @@ -121,8 +129,7 @@ describe('AlertingQueryRunner', () => { const runner = new AlertingQueryRunner( mockBackendSrv({ fetch: () => of(response).pipe(delay(210)), - }), - mockDataSourceSrv() + }) ); const data = runner.get(); @@ -176,8 +183,7 @@ describe('AlertingQueryRunner', () => { const runner = new AlertingQueryRunner( mockBackendSrv({ fetch: () => throwError(error), - }), - mockDataSourceSrv() + }) ); const data = runner.get(); @@ -332,7 +338,7 @@ const createQuery = (refId: string, options?: Partial): AlertQuery = return defaultsDeep(options, { refId, queryType: '', - datasourceUid: '', + datasourceUid: EXTERNAL_VANILLA_ALERTMANAGER_UID, model: { refId }, relativeTimeRange: getDefaultRelativeTimeRange(), }); diff --git a/public/app/features/alerting/unified/testSetup/datasources.ts b/public/app/features/alerting/unified/testSetup/datasources.ts index d70e0d8050b..e0afce55201 100644 --- a/public/app/features/alerting/unified/testSetup/datasources.ts +++ b/public/app/features/alerting/unified/testSetup/datasources.ts @@ -2,8 +2,7 @@ import { keyBy } from 'lodash'; import { DataSourceInstanceSettings } from '@grafana/data'; import { config, setDataSourceSrv } from '@grafana/runtime'; - -import { MockDataSourceSrv } from '../mocks'; +import { DatasourceSrv } from 'app/features/plugins/datasource_srv'; /** * Sets up the data sources for the tests. @@ -11,6 +10,13 @@ import { MockDataSourceSrv } from '../mocks'; * @param configs data source instance settings. Use **mockDataSource** to create mock settings */ export function setupDataSources(...configs: DataSourceInstanceSettings[]) { - config.datasources = keyBy(configs, (c) => c.name); - setDataSourceSrv(new MockDataSourceSrv(config.datasources)); + const dataSourceSrv = new DatasourceSrv(); + const datasourceSettings = keyBy(configs, (c) => c.name); + + const defaultDatasource = configs.find((c) => c.isDefault); + config.datasources = datasourceSettings; + dataSourceSrv.init(config.datasources, defaultDatasource?.name || config.defaultDatasource); + setDataSourceSrv(dataSourceSrv); + + return dataSourceSrv; } From 8f628a0695b2823284766cdfdd7cf583bb2a4e24 Mon Sep 17 00:00:00 2001 From: Mariell Hoversholm Date: Thu, 12 Dec 2024 11:37:11 +0100 Subject: [PATCH 08/13] Cloudwatch: Change deprecated errorsource references (#97827) This has been failing to compile, bringing down a _lot_ of tests along with it (notably the entire pkg/tests dir). The `experimental/errorsource` package was deprecated, so the recommended replacement was used. --- pkg/tsdb/cloudwatch/annotation_query.go | 2 +- pkg/tsdb/cloudwatch/log_sync_query.go | 2 +- pkg/tsdb/cloudwatch/time_series_query.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/tsdb/cloudwatch/annotation_query.go b/pkg/tsdb/cloudwatch/annotation_query.go index b99321a240d..fe17f837105 100644 --- a/pkg/tsdb/cloudwatch/annotation_query.go +++ b/pkg/tsdb/cloudwatch/annotation_query.go @@ -58,7 +58,7 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC cli, err := e.getCWClient(ctx, pluginCtx, region) if err != nil { - result = errorsource.AddDownstreamErrorToResponse(query.RefID, result, fmt.Errorf("%v: %w", "failed to get client", err)) + result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(fmt.Errorf("%v: %w", "failed to get client", err)) return result, nil } diff --git a/pkg/tsdb/cloudwatch/log_sync_query.go b/pkg/tsdb/cloudwatch/log_sync_query.go index 4377150f858..c029704effe 100644 --- a/pkg/tsdb/cloudwatch/log_sync_query.go +++ b/pkg/tsdb/cloudwatch/log_sync_query.go @@ -23,7 +23,7 @@ var executeSyncLogQuery = func(ctx context.Context, e *cloudWatchExecutor, req * instance, err := e.getInstance(ctx, req.PluginContext) if err != nil { - errorsource.AddErrorToResponse(req.Queries[0].RefID, resp, err) + resp.Responses[req.Queries[0].RefID] = backend.ErrorResponseWithErrorSource(err) return resp, nil } diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 8119b3b4692..42f9e6da7e5 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -34,7 +34,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba instance, err := e.getInstance(ctx, req.PluginContext) if err != nil { - errorsource.AddErrorToResponse(req.Queries[0].RefID, resp, err) + resp.Responses[req.Queries[0].RefID] = backend.ErrorResponseWithErrorSource(err) return resp, nil } From afd0699c85e3621cfafcc143ea270d4daaf37526 Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:38:12 +0100 Subject: [PATCH 09/13] Alerting: Fix not being able to remove a reducer when using range query (#97757) * Fix not being able to remove a reducer when using range query * use useEffectOnce --- .../QueryAndExpressionsStep.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index 9406c7fe509..7b0f94b9866 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -2,6 +2,7 @@ import { css } from '@emotion/css'; import { cloneDeep } from 'lodash'; import { useCallback, useEffect, useMemo, useReducer, useState } from 'react'; import { Controller, useFormContext } from 'react-hook-form'; +import { useEffectOnce } from 'react-use'; import { GrafanaTheme2, getDefaultRelativeTimeRange } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; @@ -152,14 +153,14 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P return queries.filter((query) => isExpressionQueryInAlert(query)); }, [queries]); - useEffect(() => { + useEffectOnce(() => { // we only remove or add the reducer(optimize reducer) expression when creating a new alert. // When editing an alert, we assume the user wants to manually adjust expressions and queries for more control and customization. if (!editingExistingRule && isOptimizeReducerEnabled) { dispatch(optimizeReduceExpression({ updatedQueries: dataQueries, expressionQueries })); } - }, [dataQueries, expressionQueries, editingExistingRule, isOptimizeReducerEnabled]); + }); const [type, condition, dataSourceName, editorSettings] = watch([ 'type', @@ -289,6 +290,12 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P setValue('queries', [...updatedQueries, ...expressionQueries], { shouldValidate: false }); updateExpressionAndDatasource(updatedQueries); + // we only remove or add the reducer(optimize reducer) expression when creating a new alert. + // When editing an alert, we assume the user wants to manually adjust expressions and queries for more control and customization. + if (!editingExistingRule && isOptimizeReducerEnabled) { + dispatch(optimizeReduceExpression({ updatedQueries, expressionQueries })); + } + dispatch(setDataQueries(updatedQueries)); dispatch(updateExpressionTimeRange()); @@ -298,7 +305,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P dispatch(rewireExpressions({ oldRefId, newRefId })); } }, - [queries, updateExpressionAndDatasource, getValues, setValue] + [queries, updateExpressionAndDatasource, getValues, setValue, editingExistingRule, isOptimizeReducerEnabled] ); const onChangeRecordingRulesQueries = useCallback( From f710573b37e6764ae879f4bbcc145dee485b0ade Mon Sep 17 00:00:00 2001 From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:53:01 +0100 Subject: [PATCH 10/13] Use searcher GetStats to count for folders and dashboards under a folder to be deleted (#97708) * get stats from index * fix logging * fix logging * actually counting * Use searcher to check for resources referencing a folder * Add tests when deleting a folder * Lint * merge/fix test * will delegate to SQL * get stats from index * fix logging * actually counting * merge/fix test * will delegate to SQL * Uncomment test * Add tests for checking stats before deleting a folder * Change base branch to main * Lint * [REVIEW] remove logs * Check for type assertion success --------- Co-authored-by: Ryan McKinley --- pkg/registry/apis/folders/mocks.go | 12 ++++ pkg/registry/apis/folders/register.go | 63 +++++++++++++--- pkg/registry/apis/folders/register_test.go | 83 +++++++++++++++++++++- 3 files changed, 145 insertions(+), 13 deletions(-) diff --git a/pkg/registry/apis/folders/mocks.go b/pkg/registry/apis/folders/mocks.go index 65f322ce251..ee722574aab 100644 --- a/pkg/registry/apis/folders/mocks.go +++ b/pkg/registry/apis/folders/mocks.go @@ -3,7 +3,9 @@ package folders import ( "context" + "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/stretchr/testify/mock" + "google.golang.org/grpc" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -15,6 +17,11 @@ type storageMock struct { rest.Storage } +type searcherMock struct { + *mock.Mock + resource.ResourceIndexClient +} + func (m storageMock) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { args := m.Called(ctx, name, options) return args.Get(0).(runtime.Object), args.Error(1) @@ -67,3 +74,8 @@ func (m storageMock) Update(ctx context.Context, name string, objInfo rest.Updat } return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2) } + +func (s searcherMock) GetStats(ctx context.Context, req *resource.ResourceStatsRequest, opts ...grpc.CallOption) (*resource.ResourceStatsResponse, error) { + args := s.Called(ctx, req) + return args.Get(0).(*resource.ResourceStatsResponse), args.Error(1) +} diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 2e67f7d4195..a79e2c8f32c 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -17,11 +17,13 @@ import ( common "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/spec3" + grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" + "github.com/grafana/grafana/pkg/storage/unified/resource" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" - grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/builder" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" @@ -29,7 +31,6 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/storage/unified/resource" ) var _ builder.APIGroupBuilder = (*FolderAPIBuilder)(nil) @@ -127,9 +128,7 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API storage[resourceInfo.StoragePath()] = legacyStore storage[resourceInfo.StoragePath("parents")] = &subParentsREST{b.folderSvc} storage[resourceInfo.StoragePath("access")] = &subAccessREST{b.folderSvc} - storage[resourceInfo.StoragePath("count")] = &subCountREST{ - searcher: b.searcher, - } + storage[resourceInfo.StoragePath("count")] = &subCountREST{searcher: b.searcher} // enable dual writer if optsGetter != nil && dualWriteBuilder != nil { @@ -241,12 +240,6 @@ var folderValidationRules = struct { func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error { id := a.GetName() - for _, invalidName := range folderValidationRules.invalidNames { - if id == invalidName { - return dashboards.ErrFolderInvalidUID - } - } - obj := a.GetObject() if obj == nil || a.GetOperation() == admission.Connect { return nil // This is normal for sub-resource @@ -256,7 +249,55 @@ func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, if !ok { return fmt.Errorf("obj is not v0alpha1.Folder") } + verb := a.GetOperation() + switch verb { + case admission.Create: + return b.validateOnCreate(ctx, id, obj) + case admission.Delete: + return b.validateOnDelete(ctx, f) + case admission.Update: + return nil + case admission.Connect: + return nil + } + return nil +} + +func (b *FolderAPIBuilder) validateOnDelete(ctx context.Context, f *v0alpha1.Folder) error { + resp, err := b.searcher.GetStats(ctx, &resource.ResourceStatsRequest{Namespace: f.Namespace, Folder: f.Name}) + if err != nil { + return err + } + + if resp != nil && resp.Error != nil { + return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) + } + + if resp.Stats == nil { + return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) + } + + for _, v := range resp.Stats { + if v.Count > 0 { + return folder.ErrFolderNotEmpty + } + } + + return nil +} + +func (b *FolderAPIBuilder) validateOnCreate(ctx context.Context, id string, obj runtime.Object) error { + for _, invalidName := range folderValidationRules.invalidNames { + if id == invalidName { + return dashboards.ErrFolderInvalidUID + } + } + + f, ok := obj.(*v0alpha1.Folder) + if !ok { + return fmt.Errorf("obj is not v0alpha1.Folder") + } if f.Spec.Title == "" { return dashboards.ErrFolderTitleEmpty } diff --git a/pkg/registry/apis/folders/register_test.go b/pkg/registry/apis/folders/register_test.go index b3ff66b2393..1409f3e37ab 100644 --- a/pkg/registry/apis/folders/register_test.go +++ b/pkg/registry/apis/folders/register_test.go @@ -15,8 +15,10 @@ import ( "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" ) @@ -210,7 +212,7 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) { } } -func TestFolderAPIBuilder_Validate(t *testing.T) { +func TestFolderAPIBuilder_Validate_Create(t *testing.T) { type input struct { obj *v0alpha1.Folder annotations map[string]string @@ -316,7 +318,7 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { tt.input.name, v0alpha1.SchemeGroupVersion.WithResource("folders"), "", - "create", + "CREATE", nil, true, &user.SignedInUser{}, @@ -331,3 +333,80 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { }) } } + +func TestFolderAPIBuilder_Validate_Delete(t *testing.T) { + tests := []struct { + name string + statsResponse *resource.ResourceStatsResponse_Stats + wantErr bool + }{ + { + name: "should allow deletion when folder is empty", + statsResponse: &resource.ResourceStatsResponse_Stats{Count: 0}, + }, + { + name: "should return folder not empty when the folder is not empty", + statsResponse: &resource.ResourceStatsResponse_Stats{Count: 2}, + wantErr: true, + }, + } + + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + us := storageMock{m, s} + sm := searcherMock{Mock: m} + + obj := &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "stacks-123", + Name: "valid-name", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var setupFn = func(m *mock.Mock, stats *resource.ResourceStatsResponse_Stats) { + m.On("GetStats", mock.Anything, &resource.ResourceStatsRequest{Namespace: obj.Namespace, Folder: obj.Name}).Return( + &resource.ResourceStatsResponse{Stats: []*resource.ResourceStatsResponse_Stats{stats}}, + nil, + ).Once() + } + + setupFn(m, tt.statsResponse) + + b := &FolderAPIBuilder{ + gv: resourceInfo.GroupVersion(), + features: nil, + namespacer: func(_ int64) string { return "123" }, + folderSvc: foldertest.NewFakeService(), + storage: us, + accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures("nestedFolders"), zanzana.NewNoopClient()), + searcher: sm, + } + + err := b.Validate(context.Background(), admission.NewAttributesRecord( + obj, + nil, + v0alpha1.SchemeGroupVersion.WithKind("folder"), + obj.Namespace, + obj.Name, + v0alpha1.SchemeGroupVersion.WithResource("folders"), + "", + "DELETE", + nil, + true, + &user.SignedInUser{}, + ), + nil) + + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} From 32554c78a829c3648589ad68c698e5183ff83654 Mon Sep 17 00:00:00 2001 From: Ieva Date: Thu, 12 Dec 2024 10:57:16 +0000 Subject: [PATCH 11/13] Direct DB perm checks: check that the namespaces match (#97828) check that the namespace of the caller matches the namespace in the request --- pkg/services/authz/rbac/service.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index a55ef5f7f0f..67862779f62 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -78,10 +78,17 @@ func (s *Service) validateRequest(ctx context.Context, req *authzv1.CheckRequest if req.GetNamespace() == "" { return nil, status.Error(codes.InvalidArgument, "namespace is required") } - namespace := req.GetNamespace() - ns, err := claims.ParseNamespace(namespace) + authInfo, has := claims.From(ctx) + if !has { + return nil, status.Error(codes.Internal, "could not get auth info from context") + } + if !claims.NamespaceMatches(authInfo.GetNamespace(), req.GetNamespace()) { + return nil, status.Error(codes.PermissionDenied, "namespace does not match") + } + + ns, err := claims.ParseNamespace(req.GetNamespace()) if err != nil { - ctxLogger.Error("could not parse namespace", "namespace", namespace, "error", err) + ctxLogger.Error("could not parse namespace", "namespace", req.GetNamespace(), "error", err) return nil, err } From 8009af54bef1f06a4ce29caacb0b1c102c97dd46 Mon Sep 17 00:00:00 2001 From: Sergej-Vlasov <37613182+Sergej-Vlasov@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:57:42 +0000 Subject: [PATCH 12/13] DefaultGridLayoutManager: Add itemHeight to DashboardGridItem when duplicating panel (#97648) * add itemHeight to DashboardGridItem when duplicating panel * adjust test to include itemHeight check --- .../scene/layout-default/DefaultGridLayoutManager.test.tsx | 1 + .../scene/layout-default/DefaultGridLayoutManager.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.test.tsx b/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.test.tsx index f2ba0d14a2b..39a11c6babd 100644 --- a/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.test.tsx +++ b/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.test.tsx @@ -197,6 +197,7 @@ describe('DefaultGridLayoutManager', () => { const newGridItem = grid.state.children[grid.state.children.length - 1] as DashboardGridItem; expect(newGridItem.state.height).toBe(1); + expect(newGridItem.state.itemHeight).toBe(1); }); it('Should duplicate a repeated panel', () => { diff --git a/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.tsx b/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.tsx index 914b852d8e6..f3f318c2c01 100644 --- a/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.tsx +++ b/public/app/features/dashboard-scene/scene/layout-default/DefaultGridLayoutManager.tsx @@ -183,6 +183,7 @@ export class DefaultGridLayoutManager x: gridItem.state.x, y: gridItem.state.y, height: gridItem.state.height, + itemHeight: gridItem.state.height, width: gridItem.state.width, variableName: gridItem.state.variableName, repeatDirection: gridItem.state.repeatDirection, From 27a54e8c73de70263e5a8ebe1ac6fa99d08f5a6b Mon Sep 17 00:00:00 2001 From: "grafana-pr-automation[bot]" <140550294+grafana-pr-automation[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:29:52 +0000 Subject: [PATCH 13/13] I18n: Download translations from Crowdin (#97787) --- public/locales/de-DE/grafana.json | 5 ++--- public/locales/es-ES/grafana.json | 5 ++--- public/locales/fr-FR/grafana.json | 5 ++--- public/locales/pt-BR/grafana.json | 5 ++--- public/locales/zh-Hans/grafana.json | 5 ++--- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/public/locales/de-DE/grafana.json b/public/locales/de-DE/grafana.json index 8870ce3246e..97de1f1933a 100644 --- a/public/locales/de-DE/grafana.json +++ b/public/locales/de-DE/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Tabelle - {{name}}" }, "toolbar": { + "add-to-extensions": "", + "add-to-queryless-extensions": "", "aria-label": "Werkzeugleiste durchsuchen", "copy-link": "URL kopieren", "copy-link-abs-time": "Absolute URL kopieren", @@ -2201,9 +2203,6 @@ "new-folder": { "title": "Neuer Ordner" }, - "observability": { - "title": "Beobachtbarkeit" - }, "oncall": { "title": "OnCall" }, diff --git a/public/locales/es-ES/grafana.json b/public/locales/es-ES/grafana.json index dde7f5aff67..9c2dd653fd2 100644 --- a/public/locales/es-ES/grafana.json +++ b/public/locales/es-ES/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Tabla - {{name}}" }, "toolbar": { + "add-to-extensions": "", + "add-to-queryless-extensions": "", "aria-label": "Barra de herramientas de Explore", "copy-link": "Copiar URL", "copy-link-abs-time": "", @@ -2201,9 +2203,6 @@ "new-folder": { "title": "Nueva carpeta" }, - "observability": { - "title": "Observabilidad" - }, "oncall": { "title": "OnCall" }, diff --git a/public/locales/fr-FR/grafana.json b/public/locales/fr-FR/grafana.json index fd789dec216..9bf42b88871 100644 --- a/public/locales/fr-FR/grafana.json +++ b/public/locales/fr-FR/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Tableau – {{name}}" }, "toolbar": { + "add-to-extensions": "", + "add-to-queryless-extensions": "", "aria-label": "Explorer la barre d'outils", "copy-link": "Copier l'URL", "copy-link-abs-time": "", @@ -2201,9 +2203,6 @@ "new-folder": { "title": "Nouveau dossier" }, - "observability": { - "title": "Observabilité" - }, "oncall": { "title": "OnCall" }, diff --git a/public/locales/pt-BR/grafana.json b/public/locales/pt-BR/grafana.json index 2985b911ce2..88ba779bff8 100644 --- a/public/locales/pt-BR/grafana.json +++ b/public/locales/pt-BR/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Tabela — {{name}}" }, "toolbar": { + "add-to-extensions": "", + "add-to-queryless-extensions": "", "aria-label": "Explorar barra de ferramentas", "copy-link": "Copiar URL", "copy-link-abs-time": "", @@ -2201,9 +2203,6 @@ "new-folder": { "title": "Nova pasta" }, - "observability": { - "title": "Observação" - }, "oncall": { "title": "OnCall" }, diff --git a/public/locales/zh-Hans/grafana.json b/public/locales/zh-Hans/grafana.json index 2c9db3e0ec4..7fa6ccf0c88 100644 --- a/public/locales/zh-Hans/grafana.json +++ b/public/locales/zh-Hans/grafana.json @@ -1252,6 +1252,8 @@ "title-with-name": "表格 - {{name}}" }, "toolbar": { + "add-to-extensions": "", + "add-to-queryless-extensions": "", "aria-label": "浏览 工具栏", "copy-link": "复制网址", "copy-link-abs-time": "", @@ -2191,9 +2193,6 @@ "new-folder": { "title": "新建文件夹" }, - "observability": { - "title": "可观测性" - }, "oncall": { "title": "OnCall" },