From 7d1c6b6bd255fce1af260c5529f0f1d27fa0fc42 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Tue, 7 Oct 2025 11:08:16 -0400 Subject: [PATCH] Alerting: Replace IntegrationConfig with IntegrationSchemaVersion (#112010) * remove unused compat functions * update to alerting module from pr * replace IntegrationConfig with IntegrationSchemaVersion * safely resolve a string into integration type * change usages of integration config --- apps/iam/go.mod | 2 +- apps/iam/go.sum | 4 +- go.mod | 2 +- go.sum | 4 +- .../notifications/receiver/conversions.go | 23 +- pkg/services/ngalert/api/compat/compat.go | 54 ----- pkg/services/ngalert/models/receivers.go | 205 ++---------------- pkg/services/ngalert/models/receivers_test.go | 47 ++-- pkg/services/ngalert/models/testing.go | 8 +- pkg/services/ngalert/notifier/crypto.go | 5 +- .../ngalert/notifier/legacy_storage/compat.go | 11 +- .../notifier/legacy_storage/receivers_test.go | 12 +- .../ngalert/notifier/receiver_svc_test.go | 2 +- pkg/services/ngalert/provisioning/compat.go | 2 +- .../ngalert/provisioning/contactpoints.go | 6 +- .../provisioning/contactpoints_test.go | 4 +- .../notifications/receivers/receiver_test.go | 3 +- 17 files changed, 76 insertions(+), 318 deletions(-) diff --git a/apps/iam/go.mod b/apps/iam/go.mod index b196d540f3f..f271451517f 100644 --- a/apps/iam/go.mod +++ b/apps/iam/go.mod @@ -201,7 +201,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect github.com/googleapis/gax-go/v2 v2.14.2 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165 // indirect + github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d // indirect github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f // indirect github.com/grafana/authlib/types v0.0.0-20250926065801-df98203cff37 // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect diff --git a/apps/iam/go.sum b/apps/iam/go.sum index 5cf6786bea4..548192d3285 100644 --- a/apps/iam/go.sum +++ b/apps/iam/go.sum @@ -721,8 +721,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.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= -github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165 h1:wfehM99Xlpltl9MQx8SITkgFgHmPGqrXoBCVLk/Q6NA= -github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165/go.mod h1:VGjS5gDwWEADPP6pF/drqLxEImgeuHlEW5u8E5EfIrM= +github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d h1:1dj/mcA4zGJpTrfSDBeF4x9/gihn21T8uydC3PnBRmw= +github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d/go.mod h1:VGjS5gDwWEADPP6pF/drqLxEImgeuHlEW5u8E5EfIrM= github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f h1:Cbm6OKkOcJ+7CSZsGsEJzktC/SIa5bxVeYKQLuYK86o= github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f/go.mod h1:axY0cdOg3q0TZHwpHnIz5x16xZ8ZBxJHShsSHHXcHQg= github.com/grafana/authlib/types v0.0.0-20250926065801-df98203cff37 h1:qEwZ+7MbPjzRvTi31iT9w7NBhKIpKwZrFbYmOZLqkwA= diff --git a/go.mod b/go.mod index 1a66ddb70d2..1f0acaa1bb8 100644 --- a/go.mod +++ b/go.mod @@ -86,7 +86,7 @@ require ( github.com/googleapis/gax-go/v2 v2.14.2 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165 // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f // @grafana/identity-access-team github.com/grafana/authlib/types v0.0.0-20250926065801-df98203cff37 // @grafana/identity-access-team github.com/grafana/dataplane/examples v0.0.1 // @grafana/observability-metrics diff --git a/go.sum b/go.sum index 4f36a619014..1a75c27dd1d 100644 --- a/go.sum +++ b/go.sum @@ -1585,8 +1585,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.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= -github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165 h1:wfehM99Xlpltl9MQx8SITkgFgHmPGqrXoBCVLk/Q6NA= -github.com/grafana/alerting v0.0.0-20251002001425-eeed80da0165/go.mod h1:VGjS5gDwWEADPP6pF/drqLxEImgeuHlEW5u8E5EfIrM= +github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d h1:1dj/mcA4zGJpTrfSDBeF4x9/gihn21T8uydC3PnBRmw= +github.com/grafana/alerting v0.0.0-20251006163224-3da3b9a5202d/go.mod h1:VGjS5gDwWEADPP6pF/drqLxEImgeuHlEW5u8E5EfIrM= github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f h1:Cbm6OKkOcJ+7CSZsGsEJzktC/SIa5bxVeYKQLuYK86o= github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f/go.mod h1:axY0cdOg3q0TZHwpHnIz5x16xZ8ZBxJHShsSHHXcHQg= github.com/grafana/authlib/types v0.0.0-20250926065801-df98203cff37 h1:qEwZ+7MbPjzRvTi31iT9w7NBhKIpKwZrFbYmOZLqkwA= diff --git a/pkg/registry/apps/alerting/notifications/receiver/conversions.go b/pkg/registry/apps/alerting/notifications/receiver/conversions.go index a6258eeae58..b0d79121912 100644 --- a/pkg/registry/apps/alerting/notifications/receiver/conversions.go +++ b/pkg/registry/apps/alerting/notifications/receiver/conversions.go @@ -4,6 +4,8 @@ import ( "fmt" "maps" + alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/alerting/receivers/schema" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" @@ -64,11 +66,11 @@ func convertToK8sResource( for _, integration := range receiver.Integrations { spec.Integrations = append(spec.Integrations, model.ReceiverIntegration{ Uid: &integration.UID, - Type: integration.Config.Type, + Type: string(integration.Config.Type()), + Version: string(integration.Config.Version), DisableResolveMessage: &integration.DisableResolveMessage, Settings: maps.Clone(integration.Settings), SecureFields: integration.SecureFields(), - Version: integration.Config.Version, }) } @@ -125,14 +127,21 @@ func convertToDomainModel(receiver *model.Receiver) (*ngmodels.Receiver, map[str } storedSecureFields := make(map[string][]string, len(receiver.Spec.Integrations)) for _, integration := range receiver.Spec.Integrations { - version := &integration.Version - if *version == "" { - version = nil - } - config, err := ngmodels.IntegrationConfigFromType(integration.Type, version) + t, err := alertingNotify.IntegrationTypeFromString(integration.Type) if err != nil { return nil, nil, err } + var config schema.IntegrationSchemaVersion + typeSchema, _ := alertingNotify.GetSchemaForIntegration(t) + if integration.Version != "" { + var ok bool + config, ok = typeSchema.GetVersion(schema.Version(integration.Version)) + if !ok { + return nil, nil, fmt.Errorf("invalid version %s for integration type %s", integration.Version, integration.Type) + } + } else { + config = typeSchema.GetCurrentVersion() + } grafanaIntegration := ngmodels.Integration{ Name: receiver.Spec.Title, Config: config, diff --git a/pkg/services/ngalert/api/compat/compat.go b/pkg/services/ngalert/api/compat/compat.go index eadf40217af..5fda13672ba 100644 --- a/pkg/services/ngalert/api/compat/compat.go +++ b/pkg/services/ngalert/api/compat/compat.go @@ -6,7 +6,6 @@ import ( "time" jsoniter "github.com/json-iterator/go" - amConfig "github.com/prometheus/alertmanager/config" "github.com/prometheus/common/model" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -549,56 +548,3 @@ func ApiRecordFromModelRecord(r *models.Record) *definitions.Record { TargetDatasourceUID: r.TargetDatasourceUID, } } - -func GettableGrafanaReceiverFromReceiver(r *models.Integration, provenance models.Provenance) (definitions.GettableGrafanaReceiver, error) { - out := definitions.GettableGrafanaReceiver{ - UID: r.UID, - Name: r.Name, - Type: r.Config.Type, - Provenance: definitions.Provenance(provenance), - DisableResolveMessage: r.DisableResolveMessage, - SecureFields: r.SecureFields(), - } - - if len(r.Settings) > 0 { - jsonBytes, err := json.Marshal(r.Settings) - if err != nil { - return definitions.GettableGrafanaReceiver{}, err - } - out.Settings = jsonBytes - } - - return out, nil -} - -func GettableApiReceiverFromReceiver(r *models.Receiver) (*definitions.GettableApiReceiver, error) { - out := definitions.GettableApiReceiver{ - Receiver: amConfig.Receiver{ - Name: r.Name, - }, - GettableGrafanaReceivers: definitions.GettableGrafanaReceivers{ - GrafanaManagedReceivers: make([]*definitions.GettableGrafanaReceiver, 0, len(r.Integrations)), - }, - } - - for _, integration := range r.Integrations { - gettable, err := GettableGrafanaReceiverFromReceiver(integration, r.Provenance) - if err != nil { - return nil, err - } - out.GrafanaManagedReceivers = append(out.GrafanaManagedReceivers, &gettable) - } - return &out, nil -} - -func GettableApiReceiversFromReceivers(recvs []*models.Receiver) ([]*definitions.GettableApiReceiver, error) { - out := make([]*definitions.GettableApiReceiver, 0, len(recvs)) - for _, r := range recvs { - gettables, err := GettableApiReceiverFromReceiver(r) - if err != nil { - return nil, err - } - out = append(out, gettables) - } - return out, nil -} diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 08734b0f56b..1130711da90 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -10,7 +10,6 @@ import ( "math" "slices" "sort" - "strings" "github.com/grafana/alerting/models" alertingNotify "github.com/grafana/alerting/notify" @@ -142,7 +141,7 @@ func (r *Receiver) Validate(decryptFn DecryptFn) error { func (r *Receiver) GetIntegrationTypes() []string { result := make([]string, 0, len(r.Integrations)) for _, i := range r.Integrations { - result = append(result, i.Config.Type) + result = append(result, string(i.Config.Type())) } return result } @@ -151,7 +150,7 @@ func (r *Receiver) GetIntegrationTypes() []string { type Integration struct { UID string Name string - Config IntegrationConfig + Config schema.IntegrationSchemaVersion DisableResolveMessage bool // Settings can contain both secure and non-secure settings either unencrypted or redacted. Settings map[string]any @@ -167,189 +166,11 @@ func (integration *Integration) ResourceID() string { return integration.UID } -// IntegrationConfig represents the configuration of an integration. It contains the type and information about the fields. -type IntegrationConfig struct { - Type string - Version string - Fields map[string]IntegrationField -} - -// IntegrationField represents a field in an integration configuration. -type IntegrationField struct { - Name string - Fields map[string]IntegrationField - Secure bool -} - -type IntegrationFieldPath []string - -func NewIntegrationFieldPath(path string) IntegrationFieldPath { - return strings.Split(path, ".") -} - -func (f IntegrationFieldPath) Head() string { - if len(f) > 0 { - return f[0] - } - return "" -} - -func (f IntegrationFieldPath) Tail() IntegrationFieldPath { - return f[1:] -} - -func (f IntegrationFieldPath) IsLeaf() bool { - return len(f) == 1 -} - -func (f IntegrationFieldPath) String() string { - return strings.Join(f, ".") -} - -func (f IntegrationFieldPath) With(segment string) IntegrationFieldPath { - // Copy the existing path to avoid modifying the original slice. - newPath := make(IntegrationFieldPath, len(f)+1) - copy(newPath, f) - newPath[len(newPath)-1] = segment - return newPath -} - -// IntegrationConfigFromType returns an integration configuration for a given integration type of a given version. -// If version is nil, the current version of the integration is used. -// Returns an error if the integration type is not found or if the specified version does not exist. -// -// Parameters: -// -// integrationType - The type of integration to get configuration for -// version - Optional specific version to get configuration for, uses latest if nil -// -// Returns: -// -// IntegrationConfig - The integration configuration -// error - Error if integration type not found or invalid version specified -func IntegrationConfigFromType(integrationType string, version *string) (IntegrationConfig, error) { - typeSchema, ok := alertingNotify.GetSchemaForIntegration(schema.IntegrationType(integrationType)) - if !ok { - return IntegrationConfig{}, fmt.Errorf("integration type %s not found", integrationType) - } - if version == nil { - return IntegrationConfigFromSchema(typeSchema, typeSchema.CurrentVersion) - } - return IntegrationConfigFromSchema(typeSchema, schema.Version(*version)) -} - -// IntegrationConfigFromSchema returns an integration configuration for a given version of the integration type schema. -// Returns an error if the schema does not have such version -func IntegrationConfigFromSchema(typeSchema schema.IntegrationTypeSchema, version schema.Version) (IntegrationConfig, error) { - typeVersion, ok := typeSchema.GetVersion(version) - if !ok { - return IntegrationConfig{}, fmt.Errorf("version %s not found in config", version) - } - integrationConfig := IntegrationConfig{ - Type: string(typeSchema.Type), - Version: string(typeVersion.Version), - Fields: make(map[string]IntegrationField, len(typeVersion.Options)), - } - for _, option := range typeVersion.Options { - integrationConfig.Fields[option.PropertyName] = notifierOptionToIntegrationField(option) - } - return integrationConfig, nil -} - -func notifierOptionToIntegrationField(option schema.Field) IntegrationField { - f := IntegrationField{ - Name: option.PropertyName, - Secure: option.Secure, - Fields: make(map[string]IntegrationField, len(option.SubformOptions)), - } - for _, subformOption := range option.SubformOptions { - f.Fields[subformOption.PropertyName] = notifierOptionToIntegrationField(subformOption) - } - return f -} - -// IsSecureField returns true if the field is both known and marked as secure in the integration configuration. -func (config *IntegrationConfig) IsSecureField(path IntegrationFieldPath) bool { - f, ok := config.GetField(path) - return ok && f.Secure -} - -func (config *IntegrationConfig) GetField(path IntegrationFieldPath) (IntegrationField, bool) { - for _, integrationField := range config.Fields { - if strings.EqualFold(integrationField.Name, path.Head()) { - if path.IsLeaf() { - return integrationField, true - } - return integrationField.GetField(path.Tail()) - } - } - return IntegrationField{}, false -} - -func (config *IntegrationConfig) GetSecretFields() []IntegrationFieldPath { - return traverseFields(config.Fields, nil, func(i IntegrationField) bool { - return i.Secure - }) -} - -func traverseFields(flds map[string]IntegrationField, parentPath IntegrationFieldPath, predicate func(i IntegrationField) bool) []IntegrationFieldPath { - var result []IntegrationFieldPath - for key, field := range flds { - path := parentPath.With(key) - if predicate(field) { - result = append(result, path) - } - if len(field.Fields) > 0 { - result = append(result, traverseFields(field.Fields, path, predicate)...) - } - } - return result -} - -func (config *IntegrationConfig) Clone() IntegrationConfig { - clone := IntegrationConfig{ - Type: config.Type, - Version: config.Version, - } - - if len(config.Fields) > 0 { - clone.Fields = make(map[string]IntegrationField, len(config.Fields)) - for key, field := range config.Fields { - clone.Fields[key] = field.Clone() - } - } - return clone -} - -func (field *IntegrationField) GetField(path IntegrationFieldPath) (IntegrationField, bool) { - for _, integrationField := range field.Fields { - if strings.EqualFold(integrationField.Name, path.Head()) { - if path.IsLeaf() { - return integrationField, true - } - return integrationField.GetField(path.Tail()) - } - } - return IntegrationField{}, false -} - -func (field *IntegrationField) Clone() IntegrationField { - f := IntegrationField{ - Name: field.Name, - Secure: field.Secure, - Fields: make(map[string]IntegrationField, len(field.Fields)), - } - for subName, sub := range field.Fields { - f.Fields[subName] = sub.Clone() - } - return f -} - func (integration *Integration) Clone() Integration { return Integration{ UID: integration.UID, Name: integration.Name, - Config: integration.Config.Clone(), + Config: integration.Config, DisableResolveMessage: integration.DisableResolveMessage, Settings: cloneIntegrationSettings(integration.Settings), SecureSettings: maps.Clone(integration.SecureSettings), @@ -394,7 +215,7 @@ func cloneIntegrationSettingsSlice(src []any) []any { // are stored in SecureSettings and the original values are removed from Settings. // If a field is already in SecureSettings it is not encrypted again. func (integration *Integration) Encrypt(encryptFn EncryptFn) error { - secretFieldPaths := integration.Config.GetSecretFields() + secretFieldPaths := integration.Config.GetSecretFieldsPaths() if len(secretFieldPaths) == 0 { return nil } @@ -419,7 +240,7 @@ func (integration *Integration) Encrypt(encryptFn EncryptFn) error { return errors.Join(errs...) } -func extractField(settings map[string]any, path IntegrationFieldPath) (string, bool, error) { +func extractField(settings map[string]any, path schema.IntegrationFieldPath) (string, bool, error) { val, ok := settings[path.Head()] if !ok { return "", false, nil @@ -439,7 +260,7 @@ func extractField(settings map[string]any, path IntegrationFieldPath) (string, b return extractField(sub, path.Tail()) } -func getFieldValue(settings map[string]any, path IntegrationFieldPath) (any, bool) { +func getFieldValue(settings map[string]any, path schema.IntegrationFieldPath) (any, bool) { val, ok := settings[path.Head()] if !ok { return nil, false @@ -454,7 +275,7 @@ func getFieldValue(settings map[string]any, path IntegrationFieldPath) (any, boo return getFieldValue(sub, path.Tail()) } -func setField(settings map[string]any, path IntegrationFieldPath, valueFn func(current any) any, skipIfNotExist bool) error { +func setField(settings map[string]any, path schema.IntegrationFieldPath, valueFn func(current any) any, skipIfNotExist bool) error { if path.IsLeaf() { current, ok := settings[path.Head()] if skipIfNotExist && !ok { @@ -489,7 +310,7 @@ func (integration *Integration) Decrypt(decryptFn DecryptFn) error { } delete(integration.SecureSettings, key) - path := NewIntegrationFieldPath(key) + path := schema.ParseIntegrationPath(key) err = setField(integration.Settings, path, func(current any) any { return decrypted }, false) @@ -503,7 +324,7 @@ func (integration *Integration) Decrypt(decryptFn DecryptFn) error { // Redact redacts all fields in SecureSettings and moves them to Settings. // The original values are removed from SecureSettings. func (integration *Integration) Redact(redactFn RedactFn) { - for _, path := range integration.Config.GetSecretFields() { + for _, path := range integration.Config.GetSecretFieldsPaths() { _ = setField(integration.Settings, path, func(current any) any { if s, ok := current.(string); ok && s != "" { return redactFn(s) @@ -513,7 +334,7 @@ func (integration *Integration) Redact(redactFn RedactFn) { } for key, secureVal := range integration.SecureSettings { // TODO: Should we trust that the receiver is stored correctly or use known secure settings? - _ = setField(integration.Settings, NewIntegrationFieldPath(key), func(any) any { + _ = setField(integration.Settings, schema.ParseIntegrationPath(key), func(any) any { return redactFn(secureVal) }, false) delete(integration.SecureSettings, key) @@ -546,7 +367,7 @@ func (integration *Integration) SecureFields() map[string]bool { } } // We mark secure fields in the settings as well. This is to ensure legacy behaviour for redacted secure settings. - for _, path := range integration.Config.GetSecretFields() { + for _, path := range integration.Config.GetSecretFieldsPaths() { if secureFields[path.String()] { continue } @@ -576,7 +397,7 @@ func (integration *Integration) Validate(decryptFn DecryptFn) error { return ValidateIntegration(context.Background(), models.IntegrationConfig{ UID: decrypted.UID, Name: decrypted.Name, - Type: decrypted.Config.Type, + Type: string(decrypted.Config.Type()), DisableResolveMessage: decrypted.DisableResolveMessage, Settings: jsonBytes, SecureSettings: decrypted.SecureSettings, @@ -627,7 +448,7 @@ func (r *Receiver) Fingerprint() string { sum.writeString(in.Name) // Do not include fields in fingerprint as these are not part of the receiver definition. - sum.writeString(in.Config.Type) + sum.writeString(string(in.Config.Type())) sum.writeBool(in.DisableResolveMessage) diff --git a/pkg/services/ngalert/models/receivers_test.go b/pkg/services/ngalert/models/receivers_test.go index 785fffd49be..3dcd5a2507b 100644 --- a/pkg/services/ngalert/models/receivers_test.go +++ b/pkg/services/ngalert/models/receivers_test.go @@ -1,7 +1,6 @@ package models import ( - "maps" "reflect" "testing" @@ -20,7 +19,7 @@ func TestReceiver_Clone(t *testing.T) { receiver Receiver }{ {name: "empty receiver", receiver: Receiver{}}, - {name: "empty integration", receiver: Receiver{Integrations: []*Integration{{Config: IntegrationConfig{}}}}}, + {name: "empty integration", receiver: Receiver{Integrations: []*Integration{{Config: schema.IntegrationSchemaVersion{}}}}}, {name: "random receiver", receiver: ReceiverGen()()}, } @@ -48,12 +47,12 @@ func TestReceiver_EncryptDecrypt(t *testing.T) { typeVersion, ok := alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) require.True(t, ok) for _, key := range typeVersion.GetSecretFieldsPaths() { - val, ok, err := extractField(encrypted.Settings, NewIntegrationFieldPath(key)) + val, ok, err := extractField(encrypted.Settings, key) assert.NoError(t, err) if ok { encryptedVal, err := encryptFn(val) assert.NoError(t, err) - encrypted.SecureSettings[key] = encryptedVal + encrypted.SecureSettings[key.String()] = encryptedVal } } @@ -84,9 +83,9 @@ func TestIntegration_Redact(t *testing.T) { version, ok := alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) require.True(t, ok) for _, key := range version.GetSecretFieldsPaths() { - err := setField(expected.Settings, NewIntegrationFieldPath(key), func(current any) any { + err := setField(expected.Settings, key, func(current any) any { if s, isString := current.(string); isString && s != "" { - delete(expected.SecureSettings, key) + delete(expected.SecureSettings, key.String()) return redactFn(s) } return current @@ -242,53 +241,37 @@ func TestSecretsIntegrationConfig(t *testing.T) { // Test that all known integration types have a config and correctly mark their secrets as secure. for integrationType := range notifytest.AllKnownV1ConfigsForTesting { t.Run(string(integrationType), func(t *testing.T) { - schemaType, ok := alertingNotify.GetSchemaForIntegration(integrationType) + config, ok := alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) require.True(t, ok) - config, err := IntegrationConfigFromSchema(schemaType, schema.V1) - assert.NoError(t, err) - - version, ok := schemaType.GetVersion(schema.V1) - require.True(t, ok) - - secrets := version.GetSecretFieldsPaths() + secrets := config.GetSecretFieldsPaths() allSecrets := make(map[string]struct{}, len(secrets)) for _, key := range secrets { - allSecrets[key] = struct{}{} + allSecrets[key.String()] = struct{}{} } - secretFields := config.GetSecretFields() + secretFields := config.GetSecretFieldsPaths() for _, path := range secretFields { _, isSecret := allSecrets[path.String()] assert.Equalf(t, isSecret, config.IsSecureField(path), "field '%s' is expected to be secret", path) delete(allSecrets, path.String()) } - assert.False(t, config.IsSecureField(IntegrationFieldPath{"__--**unknown_field**--__"})) + assert.False(t, config.IsSecureField(schema.ParseIntegrationPath("__--**unknown_field**--__"))) assert.Empty(t, allSecrets, "mismatched secret fields for integration type %s: %v", integrationType, allSecrets) }) } - - t.Run("Unknown version returns error", func(t *testing.T) { - for s := range maps.Keys(notifytest.AllKnownV1ConfigsForTesting) { - schemaType, _ := alertingNotify.GetSchemaForIntegration(s) - _, err := IntegrationConfigFromSchema(schemaType, "unknown") - require.Error(t, err) - return - } - }) } func TestIntegration_SecureFields(t *testing.T) { testutil.SkipIntegrationTestInShortMode(t) // Test that all known integration types have a config and correctly mark their secrets as secure. - for it := range notifytest.AllKnownV1ConfigsForTesting { - integrationType := it + for integrationType := range notifytest.AllKnownV1ConfigsForTesting { t.Run(string(integrationType), func(t *testing.T) { t.Run("contains SecureSettings", func(t *testing.T) { validIntegration := IntegrationGen(IntegrationMuts.WithValidConfig(integrationType))() expected := make(map[string]bool, len(validIntegration.SecureSettings)) - for _, path := range validIntegration.Config.GetSecretFields() { + for _, path := range validIntegration.Config.GetSecretFieldsPaths() { if validIntegration.Config.IsSecureField(path) { expected[path.String()] = true validIntegration.SecureSettings[path.String()] = "test" @@ -303,7 +286,7 @@ func TestIntegration_SecureFields(t *testing.T) { t.Run("contains secret Settings not in SecureSettings", func(t *testing.T) { validIntegration := IntegrationGen(IntegrationMuts.WithValidConfig(integrationType))() expected := make(map[string]bool, len(validIntegration.SecureSettings)) - for _, path := range validIntegration.Config.GetSecretFields() { + for _, path := range validIntegration.Config.GetSecretFieldsPaths() { if validIntegration.Config.IsSecureField(path) { expected[path.String()] = true assert.NoError(t, setField(validIntegration.Settings, path, func(current any) any { @@ -341,8 +324,7 @@ func TestReceiver_Fingerprint(t *testing.T) { "setting": "value", "something": 123, "data": []string{"test"}, - } // Add a broken type to ensure it is stable in the fingerprint. - baseReceiver.Integrations[0].Config = IntegrationConfig{Type: baseReceiver.Integrations[0].Config.Type} // Remove all fields except Type. + } completelyDifferentReceiver := ReceiverGen(ReceiverMuts.WithName("test receiver2"), ReceiverMuts.WithIntegrations( IntegrationGen(im.WithName("test receiver2"), im.WithValidConfig("discord"))(), @@ -351,7 +333,6 @@ func TestReceiver_Fingerprint(t *testing.T) { completelyDifferentReceiver.Integrations[0].DisableResolveMessage = false completelyDifferentReceiver.Integrations[0].SecureSettings = map[string]string{"test": "test"} completelyDifferentReceiver.Provenance = ProvenanceAPI - 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 := "c0c82936be34b183" // If this is a valid fingerprint generation change, update the expected value. diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 0fd681b693c..321f13199d7 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -1323,9 +1323,7 @@ func (n IntegrationMutators) WithValidConfig(integrationType schema.IntegrationT panic(fmt.Sprintf("unknown integration type: %s", integrationType)) } config := ncfg.GetRawNotifierConfig(c.Name) - typeSchema, _ := alertingNotify.GetSchemaForIntegration(integrationType) - integrationConfig, _ := IntegrationConfigFromSchema(typeSchema, schema.V1) - c.Config = integrationConfig + c.Config, _ = alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) var settings map[string]any _ = json.Unmarshal(config.Settings, &settings) @@ -1342,11 +1340,11 @@ func (n IntegrationMutators) WithValidConfig(integrationType schema.IntegrationT func (n IntegrationMutators) WithInvalidConfig(integrationType schema.IntegrationType) Mutator[Integration] { return func(c *Integration) { - typeSchema, ok := alertingNotify.GetSchemaForIntegration(integrationType) + var ok bool + c.Config, ok = alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) if !ok { panic(fmt.Sprintf("unknown integration type: %s", integrationType)) } - c.Config, _ = IntegrationConfigFromSchema(typeSchema, schema.V1) c.Settings = map[string]interface{}{} c.SecureSettings = map[string]string{} if integrationType == webex.Type { diff --git a/pkg/services/ngalert/notifier/crypto.go b/pkg/services/ngalert/notifier/crypto.go index 51aa05cbc8b..ab5d5f03096 100644 --- a/pkg/services/ngalert/notifier/crypto.go +++ b/pkg/services/ngalert/notifier/crypto.go @@ -113,7 +113,7 @@ func encryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt defini if !ok { return fmt.Errorf("failed to get secret keys for contact point type %s", gr.Type) } - secretKeys := typeSchema.GetSecretFieldsPaths() + secretPaths := typeSchema.GetSecretFieldsPaths() secureSettings := gr.SecureSettings if secureSettings == nil { secureSettings = make(map[string]string) @@ -121,7 +121,8 @@ func encryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt defini settingsChanged := false secureSettingsChanged := false - for _, secretKey := range secretKeys { + for _, secretPath := range secretPaths { + secretKey := secretPath.String() settingsValue, ok := settings[secretKey] if !ok { continue diff --git a/pkg/services/ngalert/notifier/legacy_storage/compat.go b/pkg/services/ngalert/notifier/legacy_storage/compat.go index 0db0aa6e384..ea93830d5fb 100644 --- a/pkg/services/ngalert/notifier/legacy_storage/compat.go +++ b/pkg/services/ngalert/notifier/legacy_storage/compat.go @@ -7,6 +7,7 @@ import ( "maps" alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/alerting/receivers/schema" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -26,7 +27,7 @@ func IntegrationToPostableGrafanaReceiver(integration *models.Integration) (*api postable := &apimodels.PostableGrafanaReceiver{ UID: integration.UID, Name: integration.Name, - Type: integration.Config.Type, + Type: string(integration.Config.Type()), DisableResolveMessage: integration.DisableResolveMessage, SecureSettings: maps.Clone(integration.SecureSettings), } @@ -117,10 +118,14 @@ func PostableGrafanaReceiversToIntegrations(postables []*apimodels.PostableGrafa } func PostableGrafanaReceiverToIntegration(p *apimodels.PostableGrafanaReceiver) (*models.Integration, error) { - config, err := models.IntegrationConfigFromType(p.Type, nil) + integrationType, err := alertingNotify.IntegrationTypeFromString(p.Type) if err != nil { return nil, err } + config, ok := alertingNotify.GetSchemaVersionForIntegration(integrationType, schema.V1) + if !ok { + return nil, fmt.Errorf("integration type [%s] does not have schema of version %s", integrationType, schema.V1) + } integration := &models.Integration{ UID: p.UID, Name: p.Name, @@ -132,7 +137,7 @@ func PostableGrafanaReceiverToIntegration(p *apimodels.PostableGrafanaReceiver) if p.Settings != nil { if err := json.Unmarshal(p.Settings, &integration.Settings); err != nil { - return nil, fmt.Errorf("integration '%s' of receiver '%s' has settings that cannot be parsed as JSON: %w", integration.Config.Type, p.Name, err) + return nil, fmt.Errorf("integration '%s' of receiver '%s' has settings that cannot be parsed as JSON: %w", integration.Config.Type(), p.Name, err) } } diff --git a/pkg/services/ngalert/notifier/legacy_storage/receivers_test.go b/pkg/services/ngalert/notifier/legacy_storage/receivers_test.go index a020b833de8..528f5c6d518 100644 --- a/pkg/services/ngalert/notifier/legacy_storage/receivers_test.go +++ b/pkg/services/ngalert/notifier/legacy_storage/receivers_test.go @@ -94,9 +94,7 @@ func TestDeleteReceiver(t *testing.T) { func TestCreateReceiver(t *testing.T) { rawCfg := notifytest.AllKnownV1ConfigsForTesting[webhook.Type] - typeSchema, _ := notify.GetSchemaForIntegration(webhook.Type) - cfgSchema, err := models.IntegrationConfigFromSchema(typeSchema, schema.V1) - require.NoError(t, err) + cfgSchema, _ := notify.GetSchemaVersionForIntegration(webhook.Type, schema.V1) settings := map[string]any{} require.NoError(t, json.Unmarshal([]byte(rawCfg.Config), &settings)) @@ -201,9 +199,7 @@ func TestCreateReceiver(t *testing.T) { func TestUpdateReceiver(t *testing.T) { rawCfg := notifytest.AllKnownV1ConfigsForTesting[webhook.Type] - typeSchema, _ := notify.GetSchemaForIntegration(webhook.Type) - cfgSchema, err := models.IntegrationConfigFromSchema(typeSchema, schema.V1) - require.NoError(t, err) + cfgSchema, _ := notify.GetSchemaVersionForIntegration(webhook.Type, schema.V1) settings := map[string]any{} require.NoError(t, json.Unmarshal([]byte(rawCfg.Config), &settings)) @@ -302,9 +298,7 @@ func TestUpdateReceiver(t *testing.T) { func TestGetReceiver(t *testing.T) { rawCfg := notifytest.AllKnownV1ConfigsForTesting[webhook.Type] - typeSchema, _ := notify.GetSchemaForIntegration(webhook.Type) - cfgSchema, err := models.IntegrationConfigFromSchema(typeSchema, schema.V1) - require.NoError(t, err) + cfgSchema, _ := notify.GetSchemaVersionForIntegration(webhook.Type, schema.V1) settings := map[string]any{} require.NoError(t, json.Unmarshal([]byte(rawCfg.Config), &settings)) diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index 8816124334d..ad968282ed1 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -465,7 +465,7 @@ func TestReceiverService_Create(t *testing.T) { { UID: lineIntegration.UID, Name: lineIntegration.Name, - Type: lineIntegration.Config.Type, + Type: string(lineIntegration.Config.Type()), DisableResolveMessage: lineIntegration.DisableResolveMessage, Settings: definitions.RawMessage(`{}`), // Empty settings, not nil. SecureSettings: map[string]string{ diff --git a/pkg/services/ngalert/provisioning/compat.go b/pkg/services/ngalert/provisioning/compat.go index a4b3ea50922..29c38ec4007 100644 --- a/pkg/services/ngalert/provisioning/compat.go +++ b/pkg/services/ngalert/provisioning/compat.go @@ -61,7 +61,7 @@ func GrafanaIntegrationConfigToEmbeddedContactPoint(r *models.Integration, prove return definitions.EmbeddedContactPoint{ UID: r.UID, Name: r.Name, - Type: r.Config.Type, + Type: string(r.Config.Type()), DisableResolveMessage: r.DisableResolveMessage, Settings: settingJson, Provenance: string(provenance), diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index d349acfa92c..f7d49da1c79 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -251,7 +251,8 @@ func (ecp *ContactPointService) UpdateContactPoint(ctx context.Context, orgID in if !ok { return fmt.Errorf("%w: failed to get secret keys for contact point type %s", ErrValidation, contactPoint.Type) } - for _, secretKey := range typeSchema.GetSecretFieldsPaths() { + for _, secretPath := range typeSchema.GetSecretFieldsPaths() { + secretKey := secretPath.String() secretValue := contactPoint.Settings.Get(secretKey).MustString() if secretValue == apimodels.RedactedValue { contactPoint.Settings.Set(secretKey, rawContactPoint.Settings.Get(secretKey).MustString()) @@ -526,7 +527,8 @@ func RemoveSecretsForContactPoint(e *apimodels.EmbeddedContactPoint) (map[string if !ok { return nil, fmt.Errorf("failed to get secret keys for contact point type %s", e.Type) } - for _, secretKey := range typeSchema.GetSecretFieldsPaths() { + for _, secretPath := range typeSchema.GetSecretFieldsPaths() { + secretKey := secretPath.String() secretValue, err := extractCaseInsensitive(e.Settings, secretKey) if err != nil { return nil, err diff --git a/pkg/services/ngalert/provisioning/contactpoints_test.go b/pkg/services/ngalert/provisioning/contactpoints_test.go index 1a281f912b6..5d5a3b44cba 100644 --- a/pkg/services/ngalert/provisioning/contactpoints_test.go +++ b/pkg/services/ngalert/provisioning/contactpoints_test.go @@ -461,9 +461,9 @@ func TestRemoveSecretsForContactPoint(t *testing.T) { require.NoError(t, err) FIELDS_ASSERT: - for _, field := range expectedFields { + for _, path := range expectedFields { + field := path.String() assert.Contains(t, secureFields, field) - path := strings.Split(field, ".") var expectedValue any = integration.Settings for _, segment := range path { v, ok := expectedValue.(map[string]any) diff --git a/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go b/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go index da834b690e8..ced6afdb670 100644 --- a/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go +++ b/pkg/tests/apis/alerting/notifications/receivers/receiver_test.go @@ -1320,7 +1320,8 @@ func TestIntegrationCRUD(t *testing.T) { typeSchema, ok := notify.GetSchemaVersionForIntegration(integrationType, schema.V1) require.True(t, ok) secretFields := typeSchema.GetSecretFieldsPaths() - for _, field := range secretFields { + for _, fieldPath := range secretFields { + field := fieldPath.String() if _, ok := fields[field]; !ok { // skip field that is not in the original setting continue }