From b820fd6bef3b38a6d8985a9751a6f201d59f152f Mon Sep 17 00:00:00 2001 From: Moustafa Baiou Date: Wed, 29 Jan 2025 16:00:45 -0500 Subject: [PATCH 1/8] Alerting: Fix Alertmanager configuration updates (#99610) * Alerting: Fix Alertmanager configuration updates Alertmanager configuration updates would behave inconsistently when performing no-op updates with `mysql` as the store. In particular this bug manifested as a failure to reload the provisioned alertmanager configuration components with no changes to the configuration itself. This would result in a 500 error with mysql store only. The core issue is that we were relying on the number of rows affected by the update query to determine if the configuration was found in the db or not. While this behavior works for certain sql dialects, mysql does not return the number of rows matched by the update query but rather the number of rows actually updated. Also discovered and fixed the mismatched `xorm` tag for the `CreatedAt` field to match the actual column name in the db. References: https://dev.mysql.com/doc/refman/8.4/en/update.html --- pkg/services/ngalert/models/alertmanager.go | 2 +- pkg/services/ngalert/store/alertmanager.go | 17 +++++++++++- .../ngalert/store/alertmanager_test.go | 26 +++++++++++++++++++ .../api/alerting/api_provisioning_test.go | 14 ++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/models/alertmanager.go b/pkg/services/ngalert/models/alertmanager.go index 0b69f682c8a..98e86587625 100644 --- a/pkg/services/ngalert/models/alertmanager.go +++ b/pkg/services/ngalert/models/alertmanager.go @@ -9,7 +9,7 @@ type AlertConfiguration struct { AlertmanagerConfiguration string ConfigurationHash string ConfigurationVersion string - CreatedAt int64 `xorm:"created"` + CreatedAt int64 `xorm:"created_at"` Default bool OrgID int64 `xorm:"org_id"` } diff --git a/pkg/services/ngalert/store/alertmanager.go b/pkg/services/ngalert/store/alertmanager.go index fc07e7a5f35..33e62f46fb9 100644 --- a/pkg/services/ngalert/store/alertmanager.go +++ b/pkg/services/ngalert/store/alertmanager.go @@ -110,9 +110,24 @@ func (st DBstore) SaveAlertmanagerConfigurationWithCallback(ctx context.Context, // UpdateAlertmanagerConfiguration replaces an alertmanager configuration with optimistic locking. It assumes that an existing revision of the configuration exists in the store, and will return an error otherwise. func (st *DBstore) UpdateAlertmanagerConfiguration(ctx context.Context, cmd *models.SaveAlertmanagerConfigurationCmd) error { return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + newConfigHash := fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))) + // check for no-op update + if newConfigHash == cmd.FetchedConfigurationHash { + // double check that the configuration with this hash is in the db + ok, err := sess.Table("alert_configuration"). + Where("org_id = ? AND configuration_hash = ?", cmd.OrgID, cmd.FetchedConfigurationHash). + Exist() + if err != nil { + return err + } + if !ok { + return ErrVersionLockedObjectNotFound + } + return nil + } config := models.AlertConfiguration{ AlertmanagerConfiguration: cmd.AlertmanagerConfiguration, - ConfigurationHash: fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))), + ConfigurationHash: newConfigHash, ConfigurationVersion: cmd.ConfigurationVersion, Default: cmd.Default, OrgID: cmd.OrgID, diff --git a/pkg/services/ngalert/store/alertmanager_test.go b/pkg/services/ngalert/store/alertmanager_test.go index 4433760c8e1..4f27e2b6e83 100644 --- a/pkg/services/ngalert/store/alertmanager_test.go +++ b/pkg/services/ngalert/store/alertmanager_test.go @@ -114,6 +114,32 @@ func TestIntegrationAlertmanagerStore(t *testing.T) { require.ErrorIs(t, err, ErrVersionLockedObjectNotFound) }) + + t.Run("UpdateAlertmanagerConfiguration doesn't update the db if the update is a no-op", func(t *testing.T) { + _, configMD5 := setupConfig(t, "my-config", store) + + originalConfig, err := store.GetLatestAlertmanagerConfiguration(context.Background(), 1) + require.NoError(t, err) + cmd := buildSaveConfigCmd(t, "my-config", 1) + cmd.FetchedConfigurationHash = configMD5 + err = store.UpdateAlertmanagerConfiguration(context.Background(), &cmd) + require.NoError(t, err) + config, err := store.GetLatestAlertmanagerConfiguration(context.Background(), 1) + require.NoError(t, err) + require.Equal(t, "my-config", config.AlertmanagerConfiguration) + require.Equal(t, configMD5, config.ConfigurationHash) + // CreatedAt should not have changed as we didn't touch the config in the DB + require.Equal(t, originalConfig.CreatedAt, config.CreatedAt) + }) + t.Run("UpdateAlertmanagerConfiguration fails if the config doesn't exist and the hashes in the cmd match", func(t *testing.T) { + configRaw := "my-non-existent-config" + configHash := fmt.Sprintf("%x", md5.Sum([]byte(configRaw))) + cmd := buildSaveConfigCmd(t, configRaw, 1) + cmd.FetchedConfigurationHash = configHash + err := store.UpdateAlertmanagerConfiguration(context.Background(), &cmd) + require.Error(t, err) + require.EqualError(t, err, ErrVersionLockedObjectNotFound.Error()) + }) } func TestIntegrationAlertmanagerHash(t *testing.T) { diff --git a/pkg/tests/api/alerting/api_provisioning_test.go b/pkg/tests/api/alerting/api_provisioning_test.go index ec9cda827b8..f4bc4dab890 100644 --- a/pkg/tests/api/alerting/api_provisioning_test.go +++ b/pkg/tests/api/alerting/api_provisioning_test.go @@ -903,6 +903,9 @@ func TestIntegrationExportFileProvision(t *testing.T) { require.Len(t, export.MuteTimings, 1) require.YAMLEq(t, expectedYaml, exportRaw) }) + t.Run("reloading provisioning should not fail", func(t *testing.T) { + apiClient.ReloadAlertingFileProvisioning(t) + }) }) } @@ -1004,6 +1007,17 @@ func TestIntegrationExportFileProvisionContactPoints(t *testing.T) { var export definitions.AlertingFileExport require.NoError(t, yaml.Unmarshal([]byte(exportRaw), &export)) + // verify the file exported matches the file provisioned thing + require.Len(t, export.ContactPoints, 1) + require.YAMLEq(t, string(expectedYaml), exportRaw) + }) + t.Run("reloading provisioning should not change things", func(t *testing.T) { + apiClient.ReloadAlertingFileProvisioning(t) + + exportRaw := apiClient.ExportReceiver(t, "cp_1_$escaped", "yaml", true) + var export definitions.AlertingFileExport + require.NoError(t, yaml.Unmarshal([]byte(exportRaw), &export)) + // verify the file exported matches the file provisioned thing require.Len(t, export.ContactPoints, 1) require.YAMLEq(t, string(expectedYaml), exportRaw) From cf177776bfcfb22664d15097caa35e690e20aae5 Mon Sep 17 00:00:00 2001 From: Garret Wyman Date: Wed, 29 Jan 2025 17:12:16 -0500 Subject: [PATCH 2/8] Alerting: Adding color option for slack receiver (#99615) --- docs/sources/administration/provisioning/index.md | 1 + .../file-provisioning/index.md | 2 ++ go.mod | 2 +- go.sum | 4 ++-- .../ngalert/api/tooling/definitions/contact_points.go | 1 + pkg/services/ngalert/models/receivers_test.go | 2 +- .../notifier/channels_config/available_channels.go | 8 ++++++++ pkg/storage/unified/apistore/go.mod | 2 +- pkg/storage/unified/apistore/go.sum | 4 ++-- pkg/storage/unified/resource/go.mod | 2 +- pkg/storage/unified/resource/go.sum | 4 ++-- 11 files changed, 22 insertions(+), 10 deletions(-) diff --git a/docs/sources/administration/provisioning/index.md b/docs/sources/administration/provisioning/index.md index 5c164a708aa..b06979b4880 100644 --- a/docs/sources/administration/provisioning/index.md +++ b/docs/sources/administration/provisioning/index.md @@ -479,6 +479,7 @@ The following sections detail the supported settings and secure settings for eac | mentionGroups | | | mentionChannel | | | token | yes | +| color | | #### Alert notification `victorops` diff --git a/docs/sources/alerting/set-up/provision-alerting-resources/file-provisioning/index.md b/docs/sources/alerting/set-up/provision-alerting-resources/file-provisioning/index.md index b7eb4b841fa..850600cc074 100644 --- a/docs/sources/alerting/set-up/provision-alerting-resources/file-provisioning/index.md +++ b/docs/sources/alerting/set-up/provision-alerting-resources/file-provisioning/index.md @@ -511,6 +511,8 @@ settings: # endpointUrl: https://custom_url/api/chat.postMessage # + color: {{ if eq .Status "firing" }}#D63232{{ else }}#36a64f{{ end }} + # title: | {{ template "slack.default.title" . }} text: | diff --git a/go.mod b/go.mod index 9ee6a8c744a..3b872f030f5 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,7 @@ require ( github.com/googleapis/gax-go/v2 v2.14.1 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.3 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 // @grafana/identity-access-team github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c // @grafana/identity-access-team github.com/grafana/dataplane/examples v0.0.1 // @grafana/observability-metrics diff --git a/go.sum b/go.sum index b1f20abba02..57b33a896ee 100644 --- a/go.sum +++ b/go.sum @@ -1498,8 +1498,8 @@ github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7Fsg github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 h1:616HUg7WVyLJLtVvk2pZ845M4Gk0fswgTwEZSuAZbJU= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= diff --git a/pkg/services/ngalert/api/tooling/definitions/contact_points.go b/pkg/services/ngalert/api/tooling/definitions/contact_points.go index bb38195c780..1e523a8cbe6 100644 --- a/pkg/services/ngalert/api/tooling/definitions/contact_points.go +++ b/pkg/services/ngalert/api/tooling/definitions/contact_points.go @@ -227,6 +227,7 @@ type SlackIntegration struct { MentionChannel *string `json:"mentionChannel,omitempty" yaml:"mentionChannel,omitempty" hcl:"mention_channel"` MentionUsers *string `json:"mentionUsers,omitempty" yaml:"mentionUsers,omitempty" hcl:"mention_users"` MentionGroups *string `json:"mentionGroups,omitempty" yaml:"mentionGroups,omitempty" hcl:"mention_groups"` + Color *string `json:"color,omitempty" yaml:"color,omitempty" hcl:"color"` } type TelegramIntegration struct { diff --git a/pkg/services/ngalert/models/receivers_test.go b/pkg/services/ngalert/models/receivers_test.go index a47387c58b5..d83f346f0ae 100644 --- a/pkg/services/ngalert/models/receivers_test.go +++ b/pkg/services/ngalert/models/receivers_test.go @@ -331,7 +331,7 @@ func TestReceiver_Fingerprint(t *testing.T) { completelyDifferentReceiver.Integrations[0].Config = IntegrationConfig{Type: completelyDifferentReceiver.Integrations[0].Config.Type} // Remove all fields except Type. t.Run("stable across code changes", func(t *testing.T) { - expectedFingerprint := "a3402fdaba03030c" // If this is a valid fingerprint generation change, update the expected value. + expectedFingerprint := "c0c82936be34b183" // If this is a valid fingerprint generation change, update the expected value. assert.Equal(t, expectedFingerprint, baseReceiver.Fingerprint()) }) t.Run("stable across clones", func(t *testing.T) { diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index 07df8e78251..595ac33c447 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -691,6 +691,14 @@ func GetAvailableNotifiers() []*NotifierPlugin { Placeholder: "Slack endpoint url", PropertyName: "endpointUrl", }, + { + Label: "Color", + Element: ElementTypeInput, + InputType: InputTypeText, + Description: "Templated color of the slack message", + Placeholder: alertingTemplates.DefaultMessageColor, + PropertyName: "color", + }, { // New in 8.0. Label: "Title", Element: ElementTypeInput, diff --git a/pkg/storage/unified/apistore/go.mod b/pkg/storage/unified/apistore/go.mod index 0a0624c9222..ede76c6bdfb 100644 --- a/pkg/storage/unified/apistore/go.mod +++ b/pkg/storage/unified/apistore/go.mod @@ -170,7 +170,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.1 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 // indirect + github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // indirect github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect github.com/grafana/dskit v0.0.0-20241105154643-a6b453a88040 // indirect diff --git a/pkg/storage/unified/apistore/go.sum b/pkg/storage/unified/apistore/go.sum index f8a3eb6fd01..6a81ac11767 100644 --- a/pkg/storage/unified/apistore/go.sum +++ b/pkg/storage/unified/apistore/go.sum @@ -547,8 +547,8 @@ github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 h1:616HUg7WVyLJLtVvk2pZ845M4Gk0fswgTwEZSuAZbJU= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= diff --git a/pkg/storage/unified/resource/go.mod b/pkg/storage/unified/resource/go.mod index f7e3c9cf4b5..13ee3bd6f3b 100644 --- a/pkg/storage/unified/resource/go.mod +++ b/pkg/storage/unified/resource/go.mod @@ -115,7 +115,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.1 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 // indirect + github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect github.com/grafana/grafana-app-sdk/logging v0.29.0 // indirect github.com/grafana/grafana-aws-sdk v0.31.5 // indirect diff --git a/pkg/storage/unified/resource/go.sum b/pkg/storage/unified/resource/go.sum index c6184792eae..4674123e79e 100644 --- a/pkg/storage/unified/resource/go.sum +++ b/pkg/storage/unified/resource/go.sum @@ -403,8 +403,8 @@ github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2z github.com/gorilla/mux v1.7.1/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4 h1:616HUg7WVyLJLtVvk2pZ845M4Gk0fswgTwEZSuAZbJU= -github.com/grafana/alerting v0.0.0-20250117230852-a5e8136407d4/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= +github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= From 8e9e3b8362a3007a726296a8c1a426cca64ab8dd Mon Sep 17 00:00:00 2001 From: Scott Lepper Date: Wed, 29 Jan 2025 17:31:59 -0500 Subject: [PATCH 3/8] [search] title phrase (#99753) [search] rename title_sort to title_phrase --- .../folderimpl/folder_unifiedstorage.go | 4 +-- .../folderimpl/folder_unifiedstorage_test.go | 2 +- pkg/storage/unified/resource/document.go | 22 +++++++-------- pkg/storage/unified/resource/document_test.go | 2 +- pkg/storage/unified/resource/resource.pb.go | 8 +++--- pkg/storage/unified/resource/resource.proto | 8 +++--- pkg/storage/unified/search/bleve.go | 22 +++++++++++---- pkg/storage/unified/search/bleve_mappings.go | 7 +++-- pkg/storage/unified/search/bleve_test.go | 28 +++++++++---------- .../testdata/doc/dashboard-aaa-out.json | 2 +- .../search/testdata/doc/folder-aaa-out.json | 2 +- .../search/testdata/doc/folder-bbb-out.json | 2 +- .../search/testdata/doc/playlist-aaa-out.json | 2 +- .../search/testdata/doc/report-aaa-out.json | 2 +- 14 files changed, 62 insertions(+), 51 deletions(-) diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index ae1f579efd0..d32641d68f2 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -240,9 +240,9 @@ func (s *Service) getFolderByTitleFromApiServer(ctx context.Context, orgID int64 Key: folderkey, Fields: []*resource.Requirement{ { - Key: resource.SEARCH_FIELD_TITLE_SORT, + Key: resource.SEARCH_FIELD_TITLE_PHRASE, Operator: string(selection.In), - Values: []string{strings.ToLower(title)}, + Values: []string{title}, }, }, Labels: []*resource.Requirement{}, diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index a19c83f209e..7377c18bc2f 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -567,7 +567,7 @@ func (r resourceClientMock) Search(ctx context.Context, in *resource.ResourceSea } if len(in.Options.Fields) > 0 && - in.Options.Fields[0].Key == resource.SEARCH_FIELD_TITLE_SORT && + in.Options.Fields[0].Key == resource.SEARCH_FIELD_TITLE_PHRASE && in.Options.Fields[0].Operator == "in" && len(in.Options.Fields[0].Values) > 0 && in.Options.Fields[0].Values[0] == "foo" { diff --git a/pkg/storage/unified/resource/document.go b/pkg/storage/unified/resource/document.go index ce5ecdf0d16..a868719263d 100644 --- a/pkg/storage/unified/resource/document.go +++ b/pkg/storage/unified/resource/document.go @@ -63,7 +63,7 @@ type IndexableDocument struct { Title string `json:"title,omitempty"` // internal sort field for title ( don't set this directly ) - TitleSort string `json:"title_sort,omitempty"` + TitlePhrase string `json:"title_phrase,omitempty"` // A generic description -- helpful in global search Description string `json:"description,omitempty"` @@ -163,15 +163,15 @@ func NewIndexableDocument(key *ResourceKey, rv int64, obj utils.GrafanaMetaAcces } } doc := &IndexableDocument{ - Key: key, - RV: rv, - Name: key.Name, - Title: title, // We always want *something* to display - TitleSort: strings.ToLower(title), // Lowercase for case-insensitive sorting - Labels: obj.GetLabels(), - Folder: obj.GetFolder(), - CreatedBy: obj.GetCreatedBy(), - UpdatedBy: obj.GetUpdatedBy(), + Key: key, + RV: rv, + Name: key.Name, + Title: title, // We always want *something* to display + TitlePhrase: strings.ToLower(title), // Lowercase for case-insensitive sorting + Labels: obj.GetLabels(), + Folder: obj.GetFolder(), + CreatedBy: obj.GetCreatedBy(), + UpdatedBy: obj.GetUpdatedBy(), } doc.RepoInfo, _ = obj.GetRepositoryInfo() ts := obj.GetCreationTimestamp() @@ -253,7 +253,7 @@ const SEARCH_FIELD_NAMESPACE = "namespace" const SEARCH_FIELD_NAME = "name" const SEARCH_FIELD_RV = "rv" const SEARCH_FIELD_TITLE = "title" -const SEARCH_FIELD_TITLE_SORT = "title_sort" +const SEARCH_FIELD_TITLE_PHRASE = "title_phrase" // filtering/sorting on title by full phrase const SEARCH_FIELD_DESCRIPTION = "description" const SEARCH_FIELD_TAGS = "tags" const SEARCH_FIELD_LABELS = "labels" // All labels, not a specific one diff --git a/pkg/storage/unified/resource/document_test.go b/pkg/storage/unified/resource/document_test.go index 21ef906c6c0..593d1fbffd9 100644 --- a/pkg/storage/unified/resource/document_test.go +++ b/pkg/storage/unified/resource/document_test.go @@ -35,7 +35,7 @@ func TestStandardDocumentBuilder(t *testing.T) { }, "rv": 10, "title": "test playlist unified storage", - "title_sort": "test playlist unified storage", + "title_phrase": "test playlist unified storage", "created": 1717236672000, "createdBy": "user:ABC", "updatedBy": "user:XYZ", diff --git a/pkg/storage/unified/resource/resource.pb.go b/pkg/storage/unified/resource/resource.pb.go index 651f9a5e4ba..9a41f4c4e5a 100644 --- a/pkg/storage/unified/resource/resource.pb.go +++ b/pkg/storage/unified/resource/resource.pb.go @@ -1284,12 +1284,12 @@ type ListOptions struct { // Group+Namespace+Resource (not name) Key *ResourceKey `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"` // (best effort) Match label - // Allowed to send more results than actually match because the filter will be appled - // to the resutls agin in the client. That time with the full field selector + // Allowed to send more results than actually match because the filter will be applied + // to the results again in the client. That time with the full field selector Labels []*Requirement `protobuf:"bytes,2,rep,name=labels,proto3" json:"labels,omitempty"` // (best effort) fields matcher - // Allowed to send more results than actually match because the filter will be appled - // to the resutls agin in the client. That time with the full field selector + // Allowed to send more results than actually match because the filter will be applied + // to the results again in the client. That time with the full field selector Fields []*Requirement `protobuf:"bytes,3,rep,name=fields,proto3" json:"fields,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/pkg/storage/unified/resource/resource.proto b/pkg/storage/unified/resource/resource.proto index 005b7e6ab4e..ae7d067ff5a 100644 --- a/pkg/storage/unified/resource/resource.proto +++ b/pkg/storage/unified/resource/resource.proto @@ -209,13 +209,13 @@ message ListOptions { ResourceKey key = 1; // (best effort) Match label - // Allowed to send more results than actually match because the filter will be appled - // to the resutls agin in the client. That time with the full field selector + // Allowed to send more results than actually match because the filter will be applied + // to the results again in the client. That time with the full field selector repeated Requirement labels = 2; // (best effort) fields matcher - // Allowed to send more results than actually match because the filter will be appled - // to the resutls agin in the client. That time with the full field selector + // Allowed to send more results than actually match because the filter will be applied + // to the results again in the client. That time with the full field selector repeated Requirement fields = 3; } diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 145bd025cd7..bf9748cf5bc 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -684,9 +684,11 @@ func getSortFields(req *resource.ResourceSearchRequest) []string { // fields that we went to sort by the full text var textSortFields = map[string]string{ - resource.SEARCH_FIELD_TITLE: resource.SEARCH_FIELD_TITLE + "_sort", + resource.SEARCH_FIELD_TITLE: resource.SEARCH_FIELD_TITLE_PHRASE, } +const lowerCase = "phrase" + // Convert a "requirement" into a bleve query func requirementQuery(req *resource.Requirement, prefix string) (query.Query, *resource.ErrorResult) { switch selection.Operator(req.Operator) { @@ -696,14 +698,14 @@ func requirementQuery(req *resource.Requirement, prefix string) (query.Query, *r } if len(req.Values[0]) == 1 { - q := query.NewMatchQuery(req.Values[0]) + q := query.NewMatchQuery(filterValue(req.Key, req.Values[0])) q.FieldVal = prefix + req.Key return q, nil } conjuncts := []query.Query{} for _, v := range req.Values { - q := query.NewMatchQuery(v) + q := query.NewMatchQuery(filterValue(req.Key, v)) q.FieldVal = prefix + req.Key conjuncts = append(conjuncts, q) } @@ -720,14 +722,14 @@ func requirementQuery(req *resource.Requirement, prefix string) (query.Query, *r return query.NewMatchAllQuery(), nil } if len(req.Values) == 1 { - q := query.NewMatchQuery(req.Values[0]) + q := query.NewMatchQuery(filterValue(req.Key, req.Values[0])) q.FieldVal = prefix + req.Key return q, nil } disjuncts := []query.Query{} for _, v := range req.Values { - q := query.NewMatchQuery(v) + q := query.NewMatchQuery(filterValue(req.Key, v)) q.FieldVal = prefix + req.Key disjuncts = append(disjuncts, q) } @@ -739,7 +741,7 @@ func requirementQuery(req *resource.Requirement, prefix string) (query.Query, *r var mustNotQueries []query.Query for _, value := range req.Values { - mustNotQueries = append(mustNotQueries, bleve.NewMatchQuery(value)) + mustNotQueries = append(mustNotQueries, bleve.NewMatchQuery(filterValue(req.Key, value))) } boolQuery.AddMustNot(mustNotQueries...) @@ -754,6 +756,14 @@ func requirementQuery(req *resource.Requirement, prefix string) (query.Query, *r ) } +// filterValue will convert the value to lower case if the field is a phrase field +func filterValue(field string, v string) string { + if strings.HasSuffix(field, lowerCase) { + return strings.ToLower(v) + } + return v +} + func (b *bleveIndex) hitsToTable(ctx context.Context, selectFields []string, hits search.DocumentMatchCollection, explain bool) (*resource.ResourceTable, error) { _, span := b.tracing.Start(ctx, tracingPrexfixBleve+"hitsToTable") defer span.End() diff --git a/pkg/storage/unified/search/bleve_mappings.go b/pkg/storage/unified/search/bleve_mappings.go index a1ec7d9fd6b..027fd87b947 100644 --- a/pkg/storage/unified/search/bleve_mappings.go +++ b/pkg/storage/unified/search/bleve_mappings.go @@ -24,11 +24,12 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM } mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_NAME, nameMapping) - // for sorting by title - titleSortMapping := bleve.NewKeywordFieldMapping() - mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE_SORT, titleSortMapping) + // for filtering/sorting by title full phrase + titlePhraseMapping := bleve.NewKeywordFieldMapping() + mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE_PHRASE, titlePhraseMapping) // for searching by title + // TODO: do we still need this since we have SEARCH_FIELD_TITLE_PHRASE? titleSearchMapping := bleve.NewTextFieldMapping() mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE, titleSearchMapping) diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index 4535b276c0a..66acc8d9a25 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -78,9 +78,9 @@ func TestBleveBackend(t *testing.T) { Group: "dashboard.grafana.app", Resource: "dashboards", }, - Title: "aaa (dash)", - TitleSort: "aaa (dash)", - Folder: "xxx", + Title: "aaa (dash)", + TitlePhrase: "aaa (dash)", + Folder: "xxx", Fields: map[string]any{ DASHBOARD_PANEL_TYPES: []string{"timeseries", "table"}, DASHBOARD_ERRORS_TODAY: 25, @@ -106,9 +106,9 @@ func TestBleveBackend(t *testing.T) { Group: "dashboard.grafana.app", Resource: "dashboards", }, - Title: "bbb (dash)", - TitleSort: "bbb (dash)", - Folder: "xxx", + Title: "bbb (dash)", + TitlePhrase: "bbb (dash)", + Folder: "xxx", Fields: map[string]any{ DASHBOARD_PANEL_TYPES: []string{"timeseries"}, DASHBOARD_ERRORS_TODAY: 40, @@ -134,10 +134,10 @@ func TestBleveBackend(t *testing.T) { Group: "dashboard.grafana.app", Resource: "dashboards", }, - Name: "ccc", - Title: "ccc (dash)", - TitleSort: "ccc (dash)", - Folder: "zzz", + Name: "ccc", + Title: "ccc (dash)", + TitlePhrase: "ccc (dash)", + Folder: "zzz", RepoInfo: &utils.ResourceRepositoryInfo{ Name: "repo2", Path: "path/in/repo2.yaml", @@ -332,8 +332,8 @@ func TestBleveBackend(t *testing.T) { Group: "folder.grafana.app", Resource: "folders", }, - Title: "zzz (folder)", - TitleSort: "zzz (folder)", + Title: "zzz (folder)", + TitlePhrase: "zzz (folder)", RepoInfo: &utils.ResourceRepositoryInfo{ Name: "repo-1", Path: "path/to/folder.json", @@ -349,8 +349,8 @@ func TestBleveBackend(t *testing.T) { Group: "folder.grafana.app", Resource: "folders", }, - Title: "yyy (folder)", - TitleSort: "yyy (folder)", + Title: "yyy (folder)", + TitlePhrase: "yyy (folder)", Labels: map[string]string{ "region": "west", }, diff --git a/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json b/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json index 04278a03fde..9fa0a53b2c1 100644 --- a/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json @@ -8,7 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test title", - "title_sort": "test title", + "title_phrase": "test title", "description": "test description", "tags": [ "a", diff --git a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json index 85809ce618f..86cb4f5df16 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json @@ -8,7 +8,7 @@ "name": "aaa", "rv": 1234, "title": "test-aaa", - "title_sort": "test-aaa", + "title_phrase": "test-aaa", "created": 1730490142000, "createdBy": "user:1", "repo": { diff --git a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json index dec666e5d42..5d268564f5b 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json @@ -8,7 +8,7 @@ "name": "bbb", "rv": 1234, "title": "test-bbb", - "title_sort": "test-bbb", + "title_phrase": "test-bbb", "created": 1730490142000, "createdBy": "user:1", "repo": { diff --git a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json index a4fc4117a9a..c0f12814338 100644 --- a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json @@ -8,7 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test AAA", - "title_sort": "test aaa", + "title_phrase": "test aaa", "created": 1731336353000, "createdBy": "user:t000000001", "repo": { diff --git a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json index ad38a85e3c8..6b193267072 100644 --- a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json @@ -8,7 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test AAA", - "title_sort": "test aaa", + "title_phrase": "test aaa", "labels": { "grafana.app/deprecatedInternalID": "123" }, From 70073427041e15c353e0d467b714527584765aea Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Wed, 29 Jan 2025 17:48:26 -0500 Subject: [PATCH 4/8] Alerting: k8s receivers api encrypt existing unencrypted secureFields on update (#99784) * apply security patch: v11.5.x/305-202501232115.patch commit 874ce8d12caad3742857ca86d2da7d5f81f3f825 Author: Matt Jacobson Date: Thu Jan 23 16:14:28 2025 -0500 linting commit c4b6d9194cc8b79e252e562a27a2d09a42d7a5e8 Author: Matt Jacobson Date: Thu Jan 23 14:56:35 2025 -0500 CVE-2024-11741 - victorops url --- go.mod | 2 +- go.sum | 4 +- go.work.sum | 1 + .../api/tooling/definitions/contact_points.go | 2 +- .../channels_config/available_channels.go | 1 + .../available_channels_test.go | 2 +- pkg/services/ngalert/notifier/receiver_svc.go | 8 +++ .../ngalert/notifier/receiver_svc_test.go | 54 ++++++++++++++++++- pkg/storage/unified/apistore/go.mod | 2 +- pkg/storage/unified/apistore/go.sum | 4 +- pkg/storage/unified/resource/go.mod | 2 +- pkg/storage/unified/resource/go.sum | 4 +- 12 files changed, 74 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 3b872f030f5..d8bde533370 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,7 @@ require ( github.com/googleapis/gax-go/v2 v2.14.1 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.3 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 // @grafana/identity-access-team github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c // @grafana/identity-access-team github.com/grafana/dataplane/examples v0.0.1 // @grafana/observability-metrics diff --git a/go.sum b/go.sum index 57b33a896ee..42231876a94 100644 --- a/go.sum +++ b/go.sum @@ -1498,8 +1498,8 @@ github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7Fsg github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a h1:44E+I3EPdh/W02Uyfyig86EJKPjvzcF3y0A+FEi1fBk= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= diff --git a/go.work.sum b/go.work.sum index cad88eb884c..c335966892c 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1502,6 +1502,7 @@ github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB7 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/grafana/alerting v0.0.0-20250115195200-209e052dba64/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250120144156-d6737a7dc8f5/go.mod h1:V63rh3udd7sqXJeaG+nGUmViwVnM/bY6t8U9Tols2GU= github.com/grafana/authlib v0.0.0-20250120145936-5f0e28e7a87c/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120144156-d6737a7dc8f5/go.mod h1:qYjSd1tmJiuVoSICp7Py9/zD54O9uQQA3wuM6Gg4DFM= diff --git a/pkg/services/ngalert/api/tooling/definitions/contact_points.go b/pkg/services/ngalert/api/tooling/definitions/contact_points.go index 1e523a8cbe6..9df9b8ee444 100644 --- a/pkg/services/ngalert/api/tooling/definitions/contact_points.go +++ b/pkg/services/ngalert/api/tooling/definitions/contact_points.go @@ -268,7 +268,7 @@ type ThreemaIntegration struct { type VictoropsIntegration struct { DisableResolveMessage *bool `json:"-" yaml:"-" hcl:"disable_resolve_message"` - URL string `json:"url" yaml:"url" hcl:"url"` + URL Secret `json:"url" yaml:"url" hcl:"url"` MessageType *string `json:"messageType,omitempty" yaml:"messageType,omitempty" hcl:"message_type"` Title *string `json:"title,omitempty" yaml:"title,omitempty" hcl:"title"` diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index 595ac33c447..e1748a7550b 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -388,6 +388,7 @@ func GetAvailableNotifiers() []*NotifierPlugin { Placeholder: "VictorOps url", PropertyName: "url", Required: true, + Secure: true, }, { // New in 8.0. Label: "Message Type", diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go index 5240c177873..5e305cb1224 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go @@ -15,7 +15,7 @@ func TestGetSecretKeysForContactPointType(t *testing.T) { {receiverType: "kafka", expectedSecretFields: []string{"password"}}, {receiverType: "email", expectedSecretFields: []string{}}, {receiverType: "pagerduty", expectedSecretFields: []string{"integrationKey"}}, - {receiverType: "victorops", expectedSecretFields: []string{}}, + {receiverType: "victorops", expectedSecretFields: []string{"url"}}, {receiverType: "oncall", expectedSecretFields: []string{"password", "authorization_credentials"}}, {receiverType: "pushover", expectedSecretFields: []string{"apiToken", "userKey"}}, {receiverType: "slack", expectedSecretFields: []string{"token", "url"}}, diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index bd4852660aa..a4ba285d311 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -485,6 +485,14 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive return nil, err } + // We re-encrypt the existing receiver to ensure any unencrypted secure fields that are correctly encrypted, note this should NOT re-encrypt secure fields that are already encrypted. + // This is rare, but can happen if a receiver is created with unencrypted secure fields and then the secure option is added later. + // Preferably, this would be handled by receiver config versions and migrations but for now this is a good safety net. + err = existing.Encrypt(rs.encryptor(ctx)) + if err != nil { + return nil, err + } + span.AddEvent("Loaded current receiver", trace.WithAttributes( attribute.String("concurrency_token", revision.ConcurrencyToken), attribute.String("receiver", existing.Name), diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index d8c27a5d0c7..d8b5d019237 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -583,6 +583,44 @@ func TestReceiverService_Update(t *testing.T) { ), rm.Encrypted(models.Base64Enrypt)), expectedProvenances: map[string]models.Provenance{slackIntegration.UID: models.ProvenanceNone}, }, + { + name: "encrypts previously unencrypted secure fields", + user: writer, + receiver: models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, im.AddSetting("token", "unencryptedValue"))), + ), + existing: util.Pointer(models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, + im.AddSetting("token", "unencryptedValue"), // This will get encrypted. + ), + ))), + expectedUpdate: models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, + im.AddSecureSetting("token", "dW5lbmNyeXB0ZWRWYWx1ZQ==")), + ), rm.Encrypted(models.Base64Enrypt)), + expectedProvenances: map[string]models.Provenance{slackIntegration.UID: models.ProvenanceNone}, + }, + { + // This test is important for covering the rare case when an existing field is marked as secure. + // The UI will receive the field as secure and, if unchanged, will pass it back on update as a secureField instead of a Setting. + name: "encrypts previously unencrypted secure fields when passed in as secureFields", + user: writer, + receiver: models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, im.AddSetting("newField", "newValue"))), + ), + secureFields: map[string][]string{slackIntegration.UID: {"token"}}, + existing: util.Pointer(models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, + im.AddSetting("token", "unencryptedValue"), // This will get encrypted. + ), + ))), + expectedUpdate: models.CopyReceiverWith(baseReceiver, rm.WithIntegrations( + models.CopyIntegrationWith(slackIntegration, + im.AddSetting("newField", "newValue"), + im.AddSecureSetting("token", "dW5lbmNyeXB0ZWRWYWx1ZQ==")), + ), rm.Encrypted(models.Base64Enrypt)), + expectedProvenances: map[string]models.Provenance{slackIntegration.UID: models.ProvenanceNone}, + }, { name: "doesn't copy existing unsecure fields", user: writer, @@ -684,8 +722,22 @@ func TestReceiverService_Update(t *testing.T) { sut := createReceiverServiceSut(t, &secretsService) if tc.existing != nil { - created, err := sut.CreateReceiver(context.Background(), tc.existing, tc.user.GetOrgID(), tc.user) + // Create route after receivers as they will be referenced. + revision, err := sut.cfgStore.Get(context.Background(), tc.user.GetOrgID()) require.NoError(t, err) + result, err := revision.CreateReceiver(tc.existing) + require.NoError(t, err) + + created, err := PostableApiReceiverToReceiver(result, tc.existing.Provenance) + require.NoError(t, err) + err = sut.cfgStore.Save(context.Background(), revision, tc.user.GetOrgID()) + require.NoError(t, err) + + for _, integration := range created.Integrations { + target := definitions.EmbeddedContactPoint{UID: integration.UID} + err = sut.provisioningStore.SetProvenance(context.Background(), &target, tc.user.GetOrgID(), created.Provenance) + require.NoError(t, err) + } if tc.version == "" { tc.version = created.Version diff --git a/pkg/storage/unified/apistore/go.mod b/pkg/storage/unified/apistore/go.mod index ede76c6bdfb..32b82f51bdc 100644 --- a/pkg/storage/unified/apistore/go.mod +++ b/pkg/storage/unified/apistore/go.mod @@ -170,7 +170,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.1 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // indirect + github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a // indirect github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect github.com/grafana/dskit v0.0.0-20241105154643-a6b453a88040 // indirect diff --git a/pkg/storage/unified/apistore/go.sum b/pkg/storage/unified/apistore/go.sum index 6a81ac11767..d25df5fb16f 100644 --- a/pkg/storage/unified/apistore/go.sum +++ b/pkg/storage/unified/apistore/go.sum @@ -547,8 +547,8 @@ github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a h1:44E+I3EPdh/W02Uyfyig86EJKPjvzcF3y0A+FEi1fBk= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= diff --git a/pkg/storage/unified/resource/go.mod b/pkg/storage/unified/resource/go.mod index 13ee3bd6f3b..1f87f8bc70e 100644 --- a/pkg/storage/unified/resource/go.mod +++ b/pkg/storage/unified/resource/go.mod @@ -115,7 +115,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.1 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce // indirect + github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect github.com/grafana/grafana-app-sdk/logging v0.29.0 // indirect github.com/grafana/grafana-aws-sdk v0.31.5 // indirect diff --git a/pkg/storage/unified/resource/go.sum b/pkg/storage/unified/resource/go.sum index 4674123e79e..0e30c13985c 100644 --- a/pkg/storage/unified/resource/go.sum +++ b/pkg/storage/unified/resource/go.sum @@ -403,8 +403,8 @@ github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2z github.com/gorilla/mux v1.7.1/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce h1:lilqLsOGzo+0SuyXjaN5XRVJbnkJRB0bXMoIlYHTIPE= -github.com/grafana/alerting v0.0.0-20250128163937-4446935bbcce/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a h1:44E+I3EPdh/W02Uyfyig86EJKPjvzcF3y0A+FEi1fBk= +github.com/grafana/alerting v0.0.0-20250129195454-3e5b80036b7a/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901 h1:nqV1YrtX+ZG+EYB5dcmFMWhg2Y038OMaAHAADbOC9RA= github.com/grafana/authlib v0.0.0-20250123104008-e99947858901/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= From 2d491a9367f1867cc1d21d0da7dd72520c71ab35 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 29 Jan 2025 16:44:42 -0700 Subject: [PATCH 5/8] K8s: Implement folder search (#99781) --- pkg/api/folder_bench_test.go | 2 +- .../dashboards/service/search/search.go | 6 +- pkg/services/folder/folderimpl/folder.go | 12 + .../folderimpl/folder_unifiedstorage.go | 99 +++++++++ .../folderimpl/folder_unifiedstorage_test.go | 206 ++++++++++++++++++ pkg/services/folder/foldertest/foldertest.go | 7 + pkg/services/folder/model.go | 9 + pkg/services/folder/service.go | 5 + pkg/services/search/service.go | 23 +- pkg/services/search/service_test.go | 3 + 10 files changed, 368 insertions(+), 4 deletions(-) diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index 0448eaddde1..c1114a3a48d 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -490,7 +490,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog SQLStore: sc.db, Features: features, QuotaService: quotaSrv, - SearchService: search.ProvideService(sc.cfg, sc.db, starSvc, dashboardSvc), + SearchService: search.ProvideService(sc.cfg, sc.db, starSvc, dashboardSvc, folderServiceWithFlagOn, features), folderService: folderServiceWithFlagOn, DashboardService: dashboardSvc, } diff --git a/pkg/services/dashboards/service/search/search.go b/pkg/services/dashboards/service/search/search.go index bb8a6dddb5e..a10f08954f2 100644 --- a/pkg/services/dashboards/service/search/search.go +++ b/pkg/services/dashboards/service/search/search.go @@ -31,7 +31,7 @@ func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alp } titleIDX := 0 - folderIDX := 1 + folderIDX := -1 tagsIDX := -1 scoreIDX := 0 explainIDX := 0 @@ -80,9 +80,11 @@ func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alp Resource: row.Key.Resource, // folders | dashboards Name: row.Key.Name, // The Grafana UID Title: string(row.Cells[titleIDX]), - Folder: string(row.Cells[folderIDX]), Field: fields, } + if folderIDX > 0 && row.Cells[folderIDX] != nil { + hit.Folder = string(row.Cells[folderIDX]) + } if tagsIDX > 0 && row.Cells[tagsIDX] != nil { _ = json.Unmarshal(row.Cells[tagsIDX], &hit.Tags) } diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index cafa0e38c91..b97ca614cc5 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -34,6 +34,7 @@ import ( "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/publicdashboards" + "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/store/entity" @@ -167,6 +168,17 @@ func (s *Service) DBMigration(db db.DB) { s.log.Debug("syncing dashboard and folder tables finished") } +func (s *Service) SearchFolders(ctx context.Context, q folder.SearchFoldersQuery) (model.HitList, error) { + if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) { + // TODO: + // - implement filtering by alerting folders and k6 folders (see the dashboards store `FindDashboards` method for reference) + // - implement fallback on search client in unistore to go to legacy store (will need to read from dashboard store) + return s.searchFoldersFromApiServer(ctx, q) + } + + return nil, fmt.Errorf("cannot be called on the legacy folder service") +} + func (s *Service) GetFolders(ctx context.Context, q folder.GetFoldersQuery) ([]*folder.Folder, error) { if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) { return s.getFoldersFromApiServer(ctx, q) diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index d32641d68f2..fed599819a1 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -3,6 +3,7 @@ package folderimpl import ( "context" "fmt" + "strconv" "strings" "time" @@ -20,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" "github.com/grafana/grafana/pkg/events" "github.com/grafana/grafana/pkg/infra/metrics" + "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/dashboards" @@ -28,9 +30,11 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/guardian" + "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/store/entity" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" + "github.com/grafana/grafana/pkg/storage/unified/search" "github.com/grafana/grafana/pkg/util" ) @@ -169,6 +173,101 @@ func (s *Service) getFromApiServer(ctx context.Context, q *folder.GetFolderQuery return f, err } +// searchFoldesFromApiServer uses the search grpc connection to search folders and returns the hit list +func (s *Service) searchFoldersFromApiServer(ctx context.Context, query folder.SearchFoldersQuery) (model.HitList, error) { + if query.OrgID == 0 { + requester, err := identity.GetRequester(ctx) + if err != nil { + return nil, err + } + query.OrgID = requester.GetOrgID() + } + + request := &resource.ResourceSearchRequest{ + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Namespace: s.k8sclient.getNamespace(query.OrgID), + Group: v0alpha1.FolderResourceInfo.GroupVersionResource().Group, + Resource: v0alpha1.FolderResourceInfo.GroupVersionResource().Resource, + }, + Fields: []*resource.Requirement{}, + Labels: []*resource.Requirement{}, + }, + Limit: 100000} + + if len(query.UIDs) > 0 { + request.Options.Fields = []*resource.Requirement{{ + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.In), + Values: query.UIDs, + }} + } else if len(query.IDs) > 0 { + values := make([]string, len(query.IDs)) + for i, id := range query.IDs { + values[i] = strconv.FormatInt(id, 10) + } + + request.Options.Labels = append(request.Options.Labels, &resource.Requirement{ + Key: utils.LabelKeyDeprecatedInternalID, // nolint:staticcheck + Operator: string(selection.In), + Values: values, + }) + } + + if query.Title != "" { + // allow wildcard search + request.Query = "*" + strings.ToLower(query.Title) + "*" + } + + if query.Limit > 0 { + request.Limit = query.Limit + } + + client := s.k8sclient.getSearcher(ctx) + + res, err := client.Search(ctx, request) + if err != nil { + return nil, err + } + + parsedResults, err := dashboardsearch.ParseResults(res, 0) + if err != nil { + return nil, err + } + + hitList := make([]*model.Hit, len(parsedResults.Hits)) + foldersMap := map[string]*folder.Folder{} + for i, item := range parsedResults.Hits { + f, ok := foldersMap[item.Folder] + if !ok { + f, err = s.Get(ctx, &folder.GetFolderQuery{ + UID: &item.Folder, + OrgID: query.OrgID, + SignedInUser: query.SignedInUser, + }) + if err != nil { + return nil, err + } + foldersMap[item.Folder] = f + } + slug := slugify.Slugify(item.Title) + hitList[i] = &model.Hit{ + ID: item.Field.GetNestedInt64(search.DASHBOARD_LEGACY_ID), + UID: item.Name, + OrgID: query.OrgID, + Title: item.Title, + URI: "db/" + slug, + URL: dashboards.GetFolderURL(item.Name, slug), + Type: model.DashHitFolder, + FolderUID: item.Folder, + FolderTitle: f.Title, + FolderID: f.ID, // nolint:staticcheck + } + } + + return hitList, nil +} + func (s *Service) getFolderByIDFromApiServer(ctx context.Context, id int64, orgID int64) (*folder.Folder, error) { if id == 0 { return &folder.GeneralFolder, nil diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index 7377c18bc2f..e2d50566662 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "k8s.io/client-go/dynamic" clientrest "k8s.io/client-go/rest" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -33,6 +34,7 @@ import ( "github.com/grafana/grafana/pkg/services/guardian" ngstore "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/publicdashboards" + "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" @@ -605,6 +607,81 @@ func (r resourceClientMock) Search(ctx context.Context, in *resource.ResourceSea }, nil } + if in.Query == "*test*" { + return &resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "_id", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{ + Name: "uid", + Resource: "folders", + }, + Cells: [][]byte{ + []byte("123"), + []byte("testing-123"), + []byte("parent-uid"), + }, + }, + }, + }, + TotalHits: 1, + }, nil + } + + if len(in.Options.Fields) > 0 && + in.Options.Fields[0].Key == resource.SEARCH_FIELD_NAME && + in.Options.Fields[0].Operator == "in" && + len(in.Options.Fields[0].Values) > 0 { + rows := []*resource.ResourceTableRow{} + for i, row := range in.Options.Fields[0].Values { + rows = append(rows, &resource.ResourceTableRow{ + Key: &resource.ResourceKey{ + Name: row, + Resource: "folders", + }, + Cells: [][]byte{ + []byte(fmt.Sprintf("%d", i)), // set legacy id as the row id + []byte(fmt.Sprintf("folder%d", i)), // set title as folder + row id + []byte(""), + }, + }) + } + return &resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "_id", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: rows, + }, + TotalHits: int64(len(rows)), + }, nil + } + // not found return &resource.ResourceSearchResponse{ Results: &resource.ResourceTable{}, @@ -628,3 +705,132 @@ func (r resourceClientMock) GetBlob(ctx context.Context, in *resource.GetBlobReq func (r resourceClientMock) IsHealthy(ctx context.Context, in *resource.HealthCheckRequest, opts ...grpc.CallOption) (*resource.HealthCheckResponse, error) { return nil, nil } + +type mockFoldersK8sCli struct { + mock.Mock + searcher resourceClientMock +} + +func (m *mockFoldersK8sCli) getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) { + args := m.Called(ctx, orgID) + return args.Get(0).(dynamic.ResourceInterface), args.Bool(1) +} + +func (m *mockFoldersK8sCli) getNamespace(orgID int64) string { + if orgID == 1 { + return "default" + } + return fmt.Sprintf("orgs-%d", orgID) +} + +func (m *mockFoldersK8sCli) getSearcher(ctx context.Context) resource.ResourceClient { + return m.searcher +} + +func TestSearchFoldersFromApiServer(t *testing.T) { + fakeK8sClient := new(mockFoldersK8sCli) + service := Service{ + k8sclient: fakeK8sClient, + features: featuremgmt.WithFeatures(featuremgmt.FlagKubernetesFoldersServiceV2), + } + fakeK8sClient.On("getSearcher", mock.Anything).Return(fakeK8sClient) + user := &user.SignedInUser{OrgID: 1} + ctx := identity.WithRequester(context.Background(), user) + + t.Run("Should search by uids if provided", func(t *testing.T) { + query := folder.SearchFoldersQuery{ + UIDs: []string{"uid1", "uid2"}, + IDs: []int64{1, 2}, // will ignore these because uid is passed in + SignedInUser: user, + } + result, err := service.searchFoldersFromApiServer(ctx, query) + require.NoError(t, err) + + expectedResult := model.HitList{ + { + UID: "uid1", + // no parent folder is returned, so the general folder should be set + FolderID: 0, + FolderTitle: "General", + // orgID should be taken from signed in user + OrgID: 1, + // the rest should be automatically set when parsing the hit results from search + Type: model.DashHitFolder, + URI: "db/folder0", + Title: "folder0", + URL: "/dashboards/f/uid1/folder0", + }, + { + UID: "uid2", + FolderID: 0, + FolderTitle: "General", + OrgID: 1, + Type: model.DashHitFolder, + URI: "db/folder1", + Title: "folder1", + URL: "/dashboards/f/uid2/folder1", + }, + } + require.Equal(t, expectedResult, result) + }) + + t.Run("Search by ID if uids are not provided", func(t *testing.T) { + query := folder.SearchFoldersQuery{ + IDs: []int64{123}, + SignedInUser: user, + } + result, err := service.searchFoldersFromApiServer(ctx, query) + require.NoError(t, err) + + expectedResult := model.HitList{ + { + UID: "foo", + FolderID: 0, + FolderTitle: "General", + OrgID: 1, + Type: model.DashHitFolder, + URI: "db/folder1", + Title: "folder1", + URL: "/dashboards/f/foo/folder1", + }, + } + require.Equal(t, expectedResult, result) + }) + + t.Run("Search by title, wildcard should be added to search request (won't match in search mock if not)", func(t *testing.T) { + // the search here will return a parent, this will be the parent folder returned when we query for it to add to the hit info + fakeFolderStore := folder.NewFakeStore() + fakeFolderStore.ExpectedFolder = &folder.Folder{ + UID: "parent-uid", + ID: 2, + Title: "parent title", + } + service.unifiedStore = fakeFolderStore + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ + CanSaveValue: true, + CanViewValue: true, + }) + + query := folder.SearchFoldersQuery{ + Title: "test", + SignedInUser: user, + } + result, err := service.searchFoldersFromApiServer(ctx, query) + require.NoError(t, err) + + expectedResult := model.HitList{ + { + UID: "uid", + FolderID: 2, + FolderTitle: "parent title", + FolderUID: "parent-uid", + OrgID: 1, + Type: model.DashHitFolder, + URI: "db/testing-123", + Title: "testing-123", + URL: "/dashboards/f/uid/testing-123", + }, + } + require.Equal(t, expectedResult, result) + }) +} diff --git a/pkg/services/folder/foldertest/foldertest.go b/pkg/services/folder/foldertest/foldertest.go index 205d984d72b..7a5edf096e6 100644 --- a/pkg/services/folder/foldertest/foldertest.go +++ b/pkg/services/folder/foldertest/foldertest.go @@ -4,11 +4,13 @@ import ( "context" "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/search/model" ) type FakeService struct { ExpectedFolders []*folder.Folder ExpectedFolder *folder.Folder + ExpectedHitList model.HitList ExpectedError error ExpectedDescendantCounts map[string]int64 } @@ -82,6 +84,11 @@ func (s *FakeService) GetDescendantCountsLegacy(ctx context.Context, q *folder.G func (s *FakeService) GetFolders(ctx context.Context, q folder.GetFoldersQuery) ([]*folder.Folder, error) { return s.ExpectedFolders, s.ExpectedError } + +func (s *FakeService) SearchFolders(ctx context.Context, q folder.SearchFoldersQuery) (model.HitList, error) { + return s.ExpectedHitList, s.ExpectedError +} + func (s *FakeService) GetFoldersLegacy(ctx context.Context, q folder.GetFoldersQuery) ([]*folder.Folder, error) { return s.ExpectedFolders, s.ExpectedError } diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index dae38765773..769ee259f3f 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -179,6 +179,15 @@ type GetFoldersQuery struct { SignedInUser identity.Requester `json:"-"` } +type SearchFoldersQuery struct { + OrgID int64 + UIDs []string + IDs []int64 + Title string + Limit int64 + SignedInUser identity.Requester `json:"-"` +} + // GetParentsQuery captures the information required by the folder service to // return a list of all parent folders of a given folder. type GetParentsQuery struct { diff --git a/pkg/services/folder/service.go b/pkg/services/folder/service.go index 656bdeeb866..89a123b9b45 100644 --- a/pkg/services/folder/service.go +++ b/pkg/services/folder/service.go @@ -2,6 +2,8 @@ package folder import ( "context" + + "github.com/grafana/grafana/pkg/services/search/model" ) type Service interface { @@ -40,6 +42,9 @@ type Service interface { GetFolders(ctx context.Context, q GetFoldersQuery) ([]*Folder, error) GetFoldersLegacy(ctx context.Context, q GetFoldersQuery) ([]*Folder, error) + // SearchFolders returns a list of folders that match the query. + SearchFolders(ctx context.Context, q SearchFoldersQuery) (model.HitList, error) + // GetChildren returns an array containing all child folders. GetChildren(ctx context.Context, q *GetChildrenQuery) ([]*Folder, error) GetChildrenLegacy(ctx context.Context, q *GetChildrenQuery) ([]*Folder, error) diff --git a/pkg/services/search/service.go b/pkg/services/search/service.go index 2eea2f28864..1b48756ea07 100644 --- a/pkg/services/search/service.go +++ b/pkg/services/search/service.go @@ -8,7 +8,10 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/search/model" + "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" "github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -17,7 +20,7 @@ import ( var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/search") -func ProvideService(cfg *setting.Cfg, sqlstore db.DB, starService star.Service, dashboardService dashboards.DashboardService) *SearchService { +func ProvideService(cfg *setting.Cfg, sqlstore db.DB, starService star.Service, dashboardService dashboards.DashboardService, folderService folder.Service, features featuremgmt.FeatureToggles) *SearchService { s := &SearchService{ Cfg: cfg, sortOptions: map[string]model.SortOption{ @@ -26,6 +29,8 @@ func ProvideService(cfg *setting.Cfg, sqlstore db.DB, starService star.Service, }, sqlstore: sqlstore, starService: starService, + folderService: folderService, + features: features, dashboardService: dashboardService, } return s @@ -61,6 +66,8 @@ type SearchService struct { sqlstore db.DB starService star.Service dashboardService dashboards.DashboardService + folderService folder.Service + features featuremgmt.FeatureToggles } func (s *SearchService) SearchHandler(ctx context.Context, query *Query) (model.HitList, error) { @@ -107,6 +114,20 @@ func (s *SearchService) SearchHandler(ctx context.Context, query *Query) (model. dashboardQuery.Sort = sortOpt } + // if folders are stored in unified storage, we need to use the folder service to query for folders + if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) && (query.Type == searchstore.TypeFolder || query.Type == searchstore.TypeAlertFolder) { + hits, err := s.folderService.SearchFolders(ctx, folder.SearchFoldersQuery{ + OrgID: query.OrgId, + UIDs: query.FolderUIDs, + IDs: query.FolderIds, + Title: query.Title, + Limit: query.Limit, + SignedInUser: query.SignedInUser, + }) + + return sortedHits(hits), err + } + hits, err := s.dashboardService.SearchDashboards(ctx, &dashboardQuery) if err != nil { return nil, err diff --git a/pkg/services/search/service_test.go b/pkg/services/search/service_test.go index 297884efbec..01bb5052598 100644 --- a/pkg/services/search/service_test.go +++ b/pkg/services/search/service_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/star/startest" @@ -35,6 +36,7 @@ func TestSearch_SortedResults(t *testing.T) { sqlstore: db, starService: ss, dashboardService: ds, + features: &featuremgmt.FeatureManager{}, } query := &Query{ @@ -76,6 +78,7 @@ func TestSearch_StarredResults(t *testing.T) { sqlstore: db, starService: ss, dashboardService: ds, + features: &featuremgmt.FeatureManager{}, } query := &Query{ From 4b0f8d8363090e109b2ed7d5f88444713158d870 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 29 Jan 2025 18:42:06 -0700 Subject: [PATCH 6/8] K8s: Fix search when query is set (#99787) --- .../dashboards/service/dashboard_service.go | 2 ++ pkg/services/dashboards/service/search/search.go | 15 +++++++++++++++ .../folder/folderimpl/folder_unifiedstorage.go | 2 ++ 3 files changed, 19 insertions(+) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index bc193b82b18..d85fb8ed692 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -1702,6 +1702,8 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Contex if query.Title != "" { // allow wildcard search request.Query = "*" + strings.ToLower(query.Title) + "*" + // if using query, you need to specify the fields you want + request.Fields = dashboardsearch.IncludeFields } if len(query.Tags) > 0 { diff --git a/pkg/services/dashboards/service/search/search.go b/pkg/services/dashboards/service/search/search.go index a10f08954f2..6165dfe0237 100644 --- a/pkg/services/dashboards/service/search/search.go +++ b/pkg/services/dashboards/service/search/search.go @@ -19,6 +19,21 @@ var ( resource.SEARCH_FIELD_FOLDER: "", resource.SEARCH_FIELD_TAGS: "", } + + IncludeFields = []string{ + resource.SEARCH_FIELD_TITLE, + resource.SEARCH_FIELD_TAGS, + resource.SEARCH_FIELD_LABELS, + resource.SEARCH_FIELD_FOLDER, + resource.SEARCH_FIELD_CREATED, + resource.SEARCH_FIELD_CREATED_BY, + resource.SEARCH_FIELD_UPDATED, + resource.SEARCH_FIELD_UPDATED_BY, + resource.SEARCH_FIELD_REPOSITORY_NAME, + resource.SEARCH_FIELD_REPOSITORY_PATH, + resource.SEARCH_FIELD_REPOSITORY_HASH, + resource.SEARCH_FIELD_REPOSITORY_TIME, + } ) func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alpha1.SearchResults, error) { diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index fed599819a1..a1473e7558d 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -217,6 +217,8 @@ func (s *Service) searchFoldersFromApiServer(ctx context.Context, query folder.S if query.Title != "" { // allow wildcard search request.Query = "*" + strings.ToLower(query.Title) + "*" + // if using query, you need to specify the fields you want + request.Fields = dashboardsearch.IncludeFields } if query.Limit > 0 { From 3c0383f0d58e47a9f7f519681ba4faf47ccb314f Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 30 Jan 2025 09:59:00 +0300 Subject: [PATCH 7/8] K8s/Test: Allow setting license path in test helper (#99786) --- pkg/tests/apis/client.go | 95 ++++++++++++++++++++++++ pkg/tests/apis/helper.go | 120 +++++++++++-------------------- pkg/tests/apis/openapi_test.go | 56 +-------------- pkg/tests/testinfra/testinfra.go | 8 +++ 4 files changed, 147 insertions(+), 132 deletions(-) create mode 100644 pkg/tests/apis/client.go diff --git a/pkg/tests/apis/client.go b/pkg/tests/apis/client.go new file mode 100644 index 00000000000..f3acfa72a6d --- /dev/null +++ b/pkg/tests/apis/client.go @@ -0,0 +1,95 @@ +package apis + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/dynamic" +) + +// TypedClient is the struct that implements a typed interface for resource operations +type TypedClient[T any, L any] struct { + Client dynamic.ResourceInterface +} + +func (c *TypedClient[T, L]) Create(ctx context.Context, resource *T, opts metav1.CreateOptions) (*T, error) { + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource) + if err != nil { + return nil, err + } + u := &unstructured.Unstructured{Object: unstructuredObj} + result, err := c.Client.Create(ctx, u, opts) + if err != nil { + return nil, err + } + createdObj := new(T) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, createdObj) + if err != nil { + return nil, err + } + return createdObj, nil +} + +func (c *TypedClient[T, L]) Update(ctx context.Context, resource *T, opts metav1.UpdateOptions) (*T, error) { + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource) + if err != nil { + return nil, err + } + u := &unstructured.Unstructured{Object: unstructuredObj} + result, err := c.Client.Update(ctx, u, opts) + if err != nil { + return nil, err + } + updatedObj := new(T) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, updatedObj) + if err != nil { + return nil, err + } + return updatedObj, nil +} + +func (c *TypedClient[T, L]) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { + return c.Client.Delete(ctx, name, opts) +} + +func (c *TypedClient[T, L]) Get(ctx context.Context, name string, opts metav1.GetOptions) (*T, error) { + result, err := c.Client.Get(ctx, name, opts) + if err != nil { + return nil, err + } + retrievedObj := new(T) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, retrievedObj) + if err != nil { + return nil, err + } + return retrievedObj, nil +} + +func (c *TypedClient[T, L]) List(ctx context.Context, opts metav1.ListOptions) (*L, error) { + result, err := c.Client.List(ctx, opts) + if err != nil { + return nil, err + } + listObj := new(L) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.UnstructuredContent(), listObj) + if err != nil { + return nil, err + } + return listObj, nil +} + +func (c *TypedClient[T, L]) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (*T, error) { + result, err := c.Client.Patch(ctx, name, pt, data, opts, subresources...) + if err != nil { + return nil, err + } + patchedObj := new(T) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, patchedObj) + if err != nil { + return nil, err + } + return patchedObj, nil +} diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index efcd10c9a98..39cc4d8651e 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -8,10 +8,12 @@ import ( "io" "net/http" "os" + "path/filepath" "strconv" "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -19,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer/yaml" - "k8s.io/apimachinery/pkg/types" yamlutil "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" @@ -661,86 +662,49 @@ func (c *K8sTestHelper) CreateTeam(name, email string, orgID int64) team.Team { return team } -// TypedClient is the struct that implements a typed interface for resource operations -type TypedClient[T any, L any] struct { - Client dynamic.ResourceInterface -} +// Compare the OpenAPI schema from one api against a cached snapshot +func VerifyOpenAPISnapshots(t *testing.T, dir string, gv schema.GroupVersion, h *K8sTestHelper) { + if gv.Group == "" { + return // skip invalid groups + } + path := fmt.Sprintf("/openapi/v3/apis/%s/%s", gv.Group, gv.Version) + t.Run(path, func(t *testing.T) { + rsp := DoRequest(h, RequestParams{ + Method: http.MethodGet, + Path: path, + User: h.Org1.Admin, + }, &AnyResource{}) -func (c *TypedClient[T, L]) Create(ctx context.Context, resource *T, opts metav1.CreateOptions) (*T, error) { - unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource) - if err != nil { - return nil, err - } - u := &unstructured.Unstructured{Object: unstructuredObj} - result, err := c.Client.Create(ctx, u, opts) - if err != nil { - return nil, err - } - createdObj := new(T) - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, createdObj) - if err != nil { - return nil, err - } - return createdObj, nil -} + require.NotNil(t, rsp.Response) + require.Equal(t, 200, rsp.Response.StatusCode, path) -func (c *TypedClient[T, L]) Update(ctx context.Context, resource *T, opts metav1.UpdateOptions) (*T, error) { - unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource) - if err != nil { - return nil, err - } - u := &unstructured.Unstructured{Object: unstructuredObj} - result, err := c.Client.Update(ctx, u, opts) - if err != nil { - return nil, err - } - updatedObj := new(T) - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, updatedObj) - if err != nil { - return nil, err - } - return updatedObj, nil -} + var prettyJSON bytes.Buffer + err := json.Indent(&prettyJSON, rsp.Body, "", " ") + require.NoError(t, err) + pretty := prettyJSON.String() -func (c *TypedClient[T, L]) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { - return c.Client.Delete(ctx, name, opts) -} + write := false + fpath := filepath.Join(dir, fmt.Sprintf("%s-%s.json", gv.Group, gv.Version)) -func (c *TypedClient[T, L]) Get(ctx context.Context, name string, opts metav1.GetOptions) (*T, error) { - result, err := c.Client.Get(ctx, name, opts) - if err != nil { - return nil, err - } - retrievedObj := new(T) - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, retrievedObj) - if err != nil { - return nil, err - } - return retrievedObj, nil -} + // nolint:gosec + // We can ignore the gosec G304 warning since this is a test and the function is only called with explicit paths + body, err := os.ReadFile(fpath) + if err == nil { + if !assert.JSONEq(t, string(body), pretty) { + t.Logf("openapi spec has changed: %s", path) + t.Fail() + write = true + } + } else { + t.Errorf("missing openapi spec for: %s", path) + write = true + } -func (c *TypedClient[T, L]) List(ctx context.Context, opts metav1.ListOptions) (*L, error) { - result, err := c.Client.List(ctx, opts) - if err != nil { - return nil, err - } - listObj := new(L) - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.UnstructuredContent(), listObj) - if err != nil { - return nil, err - } - return listObj, nil -} - -func (c *TypedClient[T, L]) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (*T, error) { - result, err := c.Client.Patch(ctx, name, pt, data, opts, subresources...) - if err != nil { - return nil, err - } - patchedObj := new(T) - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, patchedObj) - if err != nil { - return nil, err - } - return patchedObj, nil + if write { + e2 := os.WriteFile(fpath, []byte(pretty), 0644) + if e2 != nil { + t.Errorf("error writing file: %s", e2.Error()) + } + } + }) } diff --git a/pkg/tests/apis/openapi_test.go b/pkg/tests/apis/openapi_test.go index a4a7164fa0d..403a543d544 100644 --- a/pkg/tests/apis/openapi_test.go +++ b/pkg/tests/apis/openapi_test.go @@ -1,22 +1,16 @@ package apis import ( - "bytes" "context" "encoding/json" "fmt" - "net/http" - "os" - "path/filepath" "testing" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/version" apimachineryversion "k8s.io/apimachinery/pkg/version" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tests/testinfra" "github.com/grafana/grafana/pkg/tests/testsuite" @@ -36,6 +30,7 @@ func TestIntegrationOpenAPIs(t *testing.T) { EnableFeatureToggles: []string{ featuremgmt.FlagKubernetesFoldersServiceV2, // Will be default on by G12 featuremgmt.FlagQueryService, // Query Library + featuremgmt.FlagProvisioning, }, }) @@ -77,50 +72,3 @@ func TestIntegrationOpenAPIs(t *testing.T) { VerifyOpenAPISnapshots(t, dir, gv, h) } } - -// This function should be moved to oss (it is now a duplicate) -func VerifyOpenAPISnapshots(t *testing.T, dir string, gv schema.GroupVersion, h *K8sTestHelper) { - if gv.Group == "" { - return // skip invalid groups - } - path := fmt.Sprintf("/openapi/v3/apis/%s/%s", gv.Group, gv.Version) - t.Run(path, func(t *testing.T) { - rsp := DoRequest(h, RequestParams{ - Method: http.MethodGet, - Path: path, - User: h.Org1.Admin, - }, &AnyResource{}) - - require.NotNil(t, rsp.Response) - require.Equal(t, 200, rsp.Response.StatusCode, path) - - var prettyJSON bytes.Buffer - err := json.Indent(&prettyJSON, rsp.Body, "", " ") - require.NoError(t, err) - pretty := prettyJSON.String() - - write := false - fpath := filepath.Join(dir, fmt.Sprintf("%s-%s.json", gv.Group, gv.Version)) - - // nolint:gosec - // We can ignore the gosec G304 warning since this is a test and the function is only called with explicit paths - body, err := os.ReadFile(fpath) - if err == nil { - if !assert.JSONEq(t, string(body), pretty) { - t.Logf("openapi spec has changed: %s", path) - t.Fail() - write = true - } - } else { - t.Errorf("missing openapi spec for: %s", path) - write = true - } - - if write { - e2 := os.WriteFile(fpath, []byte(pretty), 0644) - if e2 != nil { - t.Errorf("error writing file: %s", e2.Error()) - } - } - }) -} diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 6010c712356..eaeb4db521d 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -295,6 +295,13 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { _, err = alertingSect.NewKey("max_attempts", "3") require.NoError(t, err) + if opts.LicensePath != "" { + section, err := cfg.NewSection("enterprise") + require.NoError(t, err) + _, err = section.NewKey("license_path", opts.LicensePath) + require.NoError(t, err) + } + rbacSect, err := cfg.NewSection("rbac") require.NoError(t, err) _, err = rbacSect.NewKey("permission_cache", "false") @@ -529,6 +536,7 @@ type GrafanaOpts struct { GrafanaComAPIURL string UnifiedStorageConfig map[string]setting.UnifiedStorageConfig GrafanaComSSOAPIToken string + LicensePath string // When "unified-grpc" is selected it will also start the grpc server APIServerStorageType options.StorageType From 0c120db84d6a6019e2eb9ce3456225afd7830c6e Mon Sep 17 00:00:00 2001 From: Mariell Hoversholm Date: Thu, 30 Jan 2025 08:11:50 +0100 Subject: [PATCH 8/8] Provisioning: Setup server and storage (#99757) feat: setup server and storage This simply sets up the API server and its storage for `provisioning.grafana.app`. Features will be added eventually. --- pkg/apiserver/registry/generic/storage.go | 6 + pkg/apiserver/registry/generic/store.go | 5 +- pkg/registry/apis/provisioning/register.go | 160 +++++++++++++++++++++ pkg/registry/apis/wireset.go | 2 + 4 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 pkg/registry/apis/provisioning/register.go diff --git a/pkg/apiserver/registry/generic/storage.go b/pkg/apiserver/registry/generic/storage.go index 945d51fbd58..67b0efecb8d 100644 --- a/pkg/apiserver/registry/generic/storage.go +++ b/pkg/apiserver/registry/generic/storage.go @@ -29,3 +29,9 @@ func NewRegistryStore(scheme *runtime.Scheme, resourceInfo utils.ResourceInfo, o } return store, nil } + +func NewRegistryStatusStore(scheme *runtime.Scheme, specStore *registry.Store) *StatusREST { + gv := specStore.New().GetObjectKind().GroupVersionKind().GroupVersion() + strategy := NewStatusStrategy(scheme, gv) + return NewStatusREST(specStore, strategy) +} diff --git a/pkg/apiserver/registry/generic/store.go b/pkg/apiserver/registry/generic/store.go index 3cc50bd5445..d8eb09d8cf7 100644 --- a/pkg/apiserver/registry/generic/store.go +++ b/pkg/apiserver/registry/generic/store.go @@ -26,7 +26,10 @@ type StatusREST struct { store *genericregistry.Store } -var _ = rest.Patcher(&StatusREST{}) +var ( + _ rest.Patcher = (*StatusREST)(nil) + _ rest.Storage = (*StatusREST)(nil) +) // New creates a new DataPlaneService object. func (r *StatusREST) New() runtime.Object { diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go new file mode 100644 index 00000000000..3171f55c66a --- /dev/null +++ b/pkg/registry/apis/provisioning/register.go @@ -0,0 +1,160 @@ +package provisioning + +import ( + "context" + "fmt" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" + "github.com/grafana/grafana/pkg/services/apiserver/builder" + "github.com/grafana/grafana/pkg/services/featuremgmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authorization/authorizer" + "k8s.io/apiserver/pkg/registry/rest" + genericapiserver "k8s.io/apiserver/pkg/server" + "k8s.io/kube-openapi/pkg/common" + "k8s.io/kube-openapi/pkg/spec3" +) + +var ( + _ builder.APIGroupBuilder = (*APIBuilder)(nil) +) + +type APIBuilder struct{} + +// NewAPIBuilder creates an API builder. +// It avoids anything that is core to Grafana, such that it can be used in a multi-tenant service down the line. +// This means there are no hidden dependencies, and no use of e.g. *settings.Cfg. +func NewAPIBuilder() *APIBuilder { + return &APIBuilder{} +} + +// RegisterAPIService returns an API builder, from [NewAPIBuilder]. It is called by Wire. +// This function happily uses services core to Grafana, and does not need to be multi-tenancy-compatible. +func RegisterAPIService( + features featuremgmt.FeatureToggles, + apiregistration builder.APIRegistrar, +) (*APIBuilder, error) { + if !features.IsEnabledGlobally(featuremgmt.FlagProvisioning) && + !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) { + return nil, nil // skip registration unless opting into experimental apis OR the feature specifically + } + + builder := NewAPIBuilder() + apiregistration.RegisterAPI(builder) + return builder, nil +} + +func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { + return authorizer.AuthorizerFunc( + func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + // TODO: Implement a webhook authoriser somehow. + + // fallback to the standard authorizer + return authorizer.DecisionNoOpinion, "", nil + }) +} + +func (b *APIBuilder) GetGroupVersion() schema.GroupVersion { + return provisioning.SchemeGroupVersion +} + +func (b *APIBuilder) InstallSchema(scheme *runtime.Scheme) error { + err := provisioning.AddToScheme(scheme) + if err != nil { + return err + } + + // This is required for --server-side apply + err = provisioning.AddKnownTypes(provisioning.InternalGroupVersion, scheme) + if err != nil { + return err + } + + metav1.AddToGroupVersion(scheme, provisioning.SchemeGroupVersion) + // Only 1 version (for now?) + return scheme.SetVersionPriority(provisioning.SchemeGroupVersion) +} + +func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error { + repositoryStorage, err := grafanaregistry.NewRegistryStore(opts.Scheme, provisioning.RepositoryResourceInfo, opts.OptsGetter) + if err != nil { + return fmt.Errorf("failed to create repository storage: %w", err) + } + + repositoryStatusStorage := grafanaregistry.NewRegistryStatusStore(opts.Scheme, repositoryStorage) + + storage := map[string]rest.Storage{} + storage[provisioning.RepositoryResourceInfo.StoragePath()] = repositoryStorage + storage[provisioning.RepositoryResourceInfo.StoragePath("status")] = repositoryStatusStorage + apiGroupInfo.VersionedResourcesStorageMap[provisioning.VERSION] = storage + return nil +} + +func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { + obj := a.GetObject() + + if obj == nil || a.GetOperation() == admission.Connect { + return nil // This is normal for sub-resource + } + + r, ok := obj.(*provisioning.Repository) + if !ok { + return fmt.Errorf("expected repository configuration") + } + + // TODO: Do something based on the resource we got. + _ = r + + return nil +} + +func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { + obj := a.GetObject() + if obj == nil || a.GetOperation() == admission.Connect { + return nil // This is normal for sub-resource + } + + var list field.ErrorList + // TODO: Fill the list with validation errors. + + if len(list) > 0 { + return apierrors.NewInvalid( + provisioning.RepositoryResourceInfo.GroupVersionKind().GroupKind(), + a.GetName(), list) + } + return nil +} + +func (b *APIBuilder) GetOpenAPIDefinitions() common.GetOpenAPIDefinitions { + return provisioning.GetOpenAPIDefinitions +} + +func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartHookFunc, error) { + postStartHooks := map[string]genericapiserver.PostStartHookFunc{ + "grafana-provisioning": func(postStartHookCtx genericapiserver.PostStartHookContext) error { + // TODO: Set up a shared informer for a controller and a watcher with workers. + return nil + }, + } + return postStartHooks, nil +} + +func (b *APIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.OpenAPI, error) { + oas.Info.Description = "Provisioning" + + root := "/apis/" + b.GetGroupVersion().String() + "/" + + // The root API discovery list + sub := oas.Paths.Paths[root] + if sub != nil && sub.Get != nil { + sub.Get.Tags = []string{"API Discovery"} // sorts first in the list + } + + return oas, nil +} diff --git a/pkg/registry/apis/wireset.go b/pkg/registry/apis/wireset.go index 7cedee7d208..8c4ec829b3b 100644 --- a/pkg/registry/apis/wireset.go +++ b/pkg/registry/apis/wireset.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/folders" "github.com/grafana/grafana/pkg/registry/apis/iam" "github.com/grafana/grafana/pkg/registry/apis/peakq" + "github.com/grafana/grafana/pkg/registry/apis/provisioning" "github.com/grafana/grafana/pkg/registry/apis/query" "github.com/grafana/grafana/pkg/registry/apis/scope" "github.com/grafana/grafana/pkg/registry/apis/service" @@ -40,6 +41,7 @@ var WireSet = wire.NewSet( folders.RegisterAPIService, iam.RegisterAPIService, peakq.RegisterAPIService, + provisioning.RegisterAPIService, service.RegisterAPIService, query.RegisterAPIService, scope.RegisterAPIService,