diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index 4b2f8befe21..7ece312daab 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -47,7 +47,7 @@ type ContactPointService interface { type TemplateService interface { GetTemplates(ctx context.Context, orgID int64) ([]definitions.NotificationTemplate, error) SetTemplate(ctx context.Context, orgID int64, tmpl definitions.NotificationTemplate) (definitions.NotificationTemplate, error) - DeleteTemplate(ctx context.Context, orgID int64, name string) error + DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance) error } type NotificationPolicyService interface { @@ -237,7 +237,7 @@ func (srv *ProvisioningSrv) RoutePutTemplate(c *contextmodel.ReqContext, body de } func (srv *ProvisioningSrv) RouteDeleteTemplate(c *contextmodel.ReqContext, name string) response.Response { - err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), name) + err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), name, determineProvenance(c)) if err != nil { return ErrResp(http.StatusInternalServerError, err, "") } diff --git a/pkg/services/ngalert/provisioning/templates.go b/pkg/services/ngalert/provisioning/templates.go index 024fae594d0..71f4632b32b 100644 --- a/pkg/services/ngalert/provisioning/templates.go +++ b/pkg/services/ngalert/provisioning/templates.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/provisioning/validation" ) type TemplateService struct { @@ -14,6 +15,7 @@ type TemplateService struct { provenanceStore ProvisioningStore xact TransactionManager log log.Logger + validator validation.ProvenanceStatusTransitionValidator } func NewTemplateService(config alertmanagerConfigStore, prov ProvisioningStore, xact TransactionManager, log log.Logger) *TemplateService { @@ -21,6 +23,7 @@ func NewTemplateService(config alertmanagerConfigStore, prov ProvisioningStore, configStore: config, provenanceStore: prov, xact: xact, + validator: validation.ValidateProvenanceRelaxed, log: log, } } @@ -64,6 +67,19 @@ func (t *TemplateService) SetTemplate(ctx context.Context, orgID int64, tmpl def if revision.Config.TemplateFiles == nil { revision.Config.TemplateFiles = map[string]string{} } + + _, ok := revision.Config.TemplateFiles[tmpl.Name] + if ok { + // check that provenance is not changed in an invalid way + storedProvenance, err := t.provenanceStore.GetProvenance(ctx, &tmpl, orgID) + if err != nil { + return definitions.NotificationTemplate{}, err + } + if err := t.validator(storedProvenance, models.Provenance(tmpl.Provenance)); err != nil { + return definitions.NotificationTemplate{}, err + } + } + revision.Config.TemplateFiles[tmpl.Name] = tmpl.Template err = t.xact.InTransaction(ctx, func(ctx context.Context) error { @@ -79,12 +95,30 @@ func (t *TemplateService) SetTemplate(ctx context.Context, orgID int64, tmpl def return tmpl, nil } -func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name string) error { +func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance) error { revision, err := t.configStore.Get(ctx, orgID) if err != nil { return err } + if revision.Config.TemplateFiles == nil { + return nil + } + + _, ok := revision.Config.TemplateFiles[name] + if !ok { + return nil + } + + // check that provenance is not changed in an invalid way + storedProvenance, err := t.provenanceStore.GetProvenance(ctx, &definitions.NotificationTemplate{Name: name}, orgID) + if err != nil { + return err + } + if err = t.validator(storedProvenance, models.Provenance(provenance)); err != nil { + return err + } + delete(revision.Config.TemplateFiles, name) return t.xact.InTransaction(ctx, func(ctx context.Context) error { diff --git a/pkg/services/ngalert/provisioning/templates_test.go b/pkg/services/ngalert/provisioning/templates_test.go index e54ab4e68e0..aeb8d813a61 100644 --- a/pkg/services/ngalert/provisioning/templates_test.go +++ b/pkg/services/ngalert/provisioning/templates_test.go @@ -2,9 +2,11 @@ package provisioning import ( "context" + "errors" "fmt" "testing" + "github.com/stretchr/testify/assert" mock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -12,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage" + "github.com/grafana/grafana/pkg/services/ngalert/provisioning/validation" "github.com/grafana/grafana/pkg/setting" ) @@ -98,6 +101,32 @@ func TestTemplateService(t *testing.T) { require.ErrorIs(t, err, ErrValidation) }) + t.Run("rejects existing templates if provenance is not right", func(t *testing.T) { + mockStore := &legacy_storage.MockAMConfigStore{} + sut := createTemplateServiceSut(legacy_storage.NewAlertmanagerConfigStore(mockStore)) + mockStore.EXPECT(). + GetsConfig(models.AlertConfiguration{ + AlertmanagerConfiguration: configWithTemplates, + }) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) + + expectedErr := errors.New("test") + sut.validator = func(from, to models.Provenance) error { + assert.Equal(t, models.ProvenanceAPI, from) + assert.Equal(t, models.ProvenanceNone, to) + return expectedErr + } + template := definitions.NotificationTemplate{ + Name: "a", + Template: "asdf-new", + } + template.Provenance = definitions.Provenance(models.ProvenanceNone) + + _, err := sut.SetTemplate(context.Background(), 1, template) + + require.ErrorIs(t, err, expectedErr) + }) + t.Run("propagates errors", func(t *testing.T) { t.Run("when unable to read config", func(t *testing.T) { mockStore := &legacy_storage.MockAMConfigStore{} @@ -106,6 +135,7 @@ func TestTemplateService(t *testing.T) { mockStore.EXPECT(). GetLatestAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(nil, fmt.Errorf("failed")) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -120,6 +150,7 @@ func TestTemplateService(t *testing.T) { GetsConfig(models.AlertConfiguration{ AlertmanagerConfiguration: brokenConfig, }) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -133,6 +164,7 @@ func TestTemplateService(t *testing.T) { mockStore.EXPECT(). GetLatestAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(nil, nil) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -148,6 +180,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: configWithTemplates, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT(). SetProvenance(mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(fmt.Errorf("failed to save provenance")) @@ -169,6 +202,7 @@ func TestTemplateService(t *testing.T) { UpdateAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(fmt.Errorf("failed to save config")) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -185,6 +219,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: configWithTemplates, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -201,6 +236,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -220,6 +256,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() result, _ := sut.SetTemplate(context.Background(), 1, tmpl) @@ -240,6 +277,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() result, _ := sut.SetTemplate(context.Background(), 1, tmpl) @@ -259,6 +297,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -278,6 +317,7 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() _, err := sut.SetTemplate(context.Background(), 1, tmpl) @@ -294,8 +334,9 @@ func TestTemplateService(t *testing.T) { mockStore.EXPECT(). GetLatestAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(nil, fmt.Errorf("failed")) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) - err := sut.DeleteTemplate(context.Background(), 1, "template") + err := sut.DeleteTemplate(context.Background(), 1, "template", definitions.Provenance(models.ProvenanceAPI)) require.Error(t, err) }) @@ -307,8 +348,9 @@ func TestTemplateService(t *testing.T) { GetsConfig(models.AlertConfiguration{ AlertmanagerConfiguration: brokenConfig, }) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) - err := sut.DeleteTemplate(context.Background(), 1, "template") + err := sut.DeleteTemplate(context.Background(), 1, "template", definitions.Provenance(models.ProvenanceAPI)) require.Truef(t, legacy_storage.ErrBadAlertmanagerConfiguration.Base.Is(err), "expected ErrBadAlertmanagerConfiguration but got %s", err.Error()) }) @@ -319,8 +361,9 @@ func TestTemplateService(t *testing.T) { mockStore.EXPECT(). GetLatestAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(nil, nil) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) - err := sut.DeleteTemplate(context.Background(), 1, "template") + err := sut.DeleteTemplate(context.Background(), 1, "template", definitions.Provenance(models.ProvenanceAPI)) require.Truef(t, legacy_storage.ErrNoAlertmanagerConfiguration.Is(err), "expected ErrNoAlertmanagerConfiguration but got %s", err.Error()) }) @@ -333,11 +376,12 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: configWithTemplates, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT(). DeleteProvenance(mock.Anything, mock.Anything, mock.Anything). Return(fmt.Errorf("failed to save provenance")) - err := sut.DeleteTemplate(context.Background(), 1, "template") + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI)) require.ErrorContains(t, err, "failed to save provenance") }) @@ -352,9 +396,10 @@ func TestTemplateService(t *testing.T) { mockStore.EXPECT(). UpdateAlertmanagerConfiguration(mock.Anything, mock.Anything). Return(fmt.Errorf("failed to save config")) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() - err := sut.DeleteTemplate(context.Background(), 1, "template") + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI)) require.ErrorContains(t, err, "failed to save config") }) @@ -368,9 +413,10 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: configWithTemplates, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() - err := sut.DeleteTemplate(context.Background(), 1, "a") + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI)) require.NoError(t, err) }) @@ -383,9 +429,10 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: configWithTemplates, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() - err := sut.DeleteTemplate(context.Background(), 1, "does not exist") + err := sut.DeleteTemplate(context.Background(), 1, "does not exist", definitions.Provenance(models.ProvenanceAPI)) require.NoError(t, err) }) @@ -398,12 +445,34 @@ func TestTemplateService(t *testing.T) { AlertmanagerConfiguration: defaultConfig, }) mockStore.EXPECT().SaveSucceeds() + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) sut.provenanceStore.(*MockProvisioningStore).EXPECT().SaveSucceeds() - err := sut.DeleteTemplate(context.Background(), 1, "a") + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI)) require.NoError(t, err) }) + + t.Run("errors if provenance is not right", func(t *testing.T) { + mockStore := &legacy_storage.MockAMConfigStore{} + sut := createTemplateServiceSut(legacy_storage.NewAlertmanagerConfigStore(mockStore)) + mockStore.EXPECT(). + GetsConfig(models.AlertConfiguration{ + AlertmanagerConfiguration: configWithTemplates, + }) + sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) + + expectedErr := errors.New("test") + sut.validator = func(from, to models.Provenance) error { + assert.Equal(t, models.ProvenanceAPI, from) + assert.Equal(t, models.ProvenanceNone, to) + return expectedErr + } + + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceNone)) + + require.ErrorIs(t, err, expectedErr) + }) }) } @@ -413,6 +482,7 @@ func createTemplateServiceSut(configStore alertmanagerConfigStore) *TemplateServ provenanceStore: &MockProvisioningStore{}, xact: newNopTransactionManager(), log: log.NewNopLogger(), + validator: validation.ValidateProvenanceRelaxed, } } diff --git a/pkg/services/provisioning/alerting/text_templates.go b/pkg/services/provisioning/alerting/text_templates.go index 4b60b3912e2..0a567f863ca 100644 --- a/pkg/services/provisioning/alerting/text_templates.go +++ b/pkg/services/provisioning/alerting/text_templates.go @@ -45,7 +45,7 @@ func (c *defaultTextTemplateProvisioner) Unprovision(ctx context.Context, files []*AlertingFile) error { for _, file := range files { for _, deleteTemplate := range file.DeleteTemplates { - err := c.templateService.DeleteTemplate(ctx, deleteTemplate.OrgID, deleteTemplate.Name) + err := c.templateService.DeleteTemplate(ctx, deleteTemplate.OrgID, deleteTemplate.Name, definitions.Provenance(models.ProvenanceFile)) if err != nil { return err }