Alerting: Support template UID in template service (#92164)

* add uid to template and populate it
* update delete method to support both uid and name
* update UpdateTemplate to support search by UID and fallback to name + support renaming of the template
* update upsert to exit if template not found and uid is specified
* update Get method to address by name or uid

---------

Co-authored-by: Matthew Jacobson <matthew.jacobson@grafana.com>
This commit is contained in:
Yuri Tseretyan 2024-08-26 16:05:38 -04:00 committed by GitHub
parent 354aee951d
commit 4755eb5176
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 359 additions and 123 deletions

View File

@ -45,9 +45,9 @@ type ContactPointService interface {
type TemplateService interface {
GetTemplates(ctx context.Context, orgID int64) ([]definitions.NotificationTemplate, error)
GetTemplate(ctx context.Context, orgID int64, name string) (definitions.NotificationTemplate, error)
GetTemplate(ctx context.Context, orgID int64, nameOrUid string) (definitions.NotificationTemplate, error)
UpsertTemplate(ctx context.Context, orgID int64, tmpl definitions.NotificationTemplate) (definitions.NotificationTemplate, error)
DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance, version string) error
DeleteTemplate(ctx context.Context, orgID int64, nameOrUid string, provenance definitions.Provenance, version string) error
}
type NotificationPolicyService interface {
@ -229,9 +229,9 @@ func (srv *ProvisioningSrv) RoutePutTemplate(c *contextmodel.ReqContext, body de
return response.JSON(http.StatusAccepted, modified)
}
func (srv *ProvisioningSrv) RouteDeleteTemplate(c *contextmodel.ReqContext, name string) response.Response {
func (srv *ProvisioningSrv) RouteDeleteTemplate(c *contextmodel.ReqContext, nameOrUid string) response.Response {
version := c.Query("version")
err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), name, determineProvenance(c), version)
err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), nameOrUid, determineProvenance(c), version)
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "", err)
}

View File

@ -55,6 +55,7 @@ type RouteDeleteTemplateParam struct {
// swagger:model
type NotificationTemplate struct {
UID string `json:"-" yaml:"-"`
Name string `json:"name"`
Template string `json:"template"`
Provenance Provenance `json:"provenance,omitempty"`

View File

@ -50,6 +50,7 @@ func (t *TemplateService) GetTemplates(ctx context.Context, orgID int64) ([]defi
templates := make([]definitions.NotificationTemplate, 0, len(revision.Config.TemplateFiles))
for name, tmpl := range revision.Config.TemplateFiles {
tmpl := definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(name),
Name: name,
Template: tmpl,
ResourceVersion: calculateTemplateFingerprint(tmpl),
@ -65,20 +66,26 @@ func (t *TemplateService) GetTemplates(ctx context.Context, orgID int64) ([]defi
return templates, nil
}
func (t *TemplateService) GetTemplate(ctx context.Context, orgID int64, name string) (definitions.NotificationTemplate, error) {
func (t *TemplateService) GetTemplate(ctx context.Context, orgID int64, nameOrUid string) (definitions.NotificationTemplate, error) {
revision, err := t.configStore.Get(ctx, orgID)
if err != nil {
return definitions.NotificationTemplate{}, err
}
for tmplName, tmpl := range revision.Config.TemplateFiles {
if tmplName != name {
continue
existingName := nameOrUid
existingContent, ok := revision.Config.TemplateFiles[nameOrUid]
if !ok {
existingName, existingContent, ok = getTemplateByUid(revision.Config.TemplateFiles, nameOrUid)
}
if !ok {
return definitions.NotificationTemplate{}, ErrTemplateNotFound.Errorf("")
}
tmpl := definitions.NotificationTemplate{
Name: name,
Template: tmpl,
ResourceVersion: calculateTemplateFingerprint(tmpl),
UID: legacy_storage.NameToUid(existingName),
Name: existingName,
Template: existingContent,
ResourceVersion: calculateTemplateFingerprint(existingContent),
}
provenance, err := t.provenanceStore.GetProvenance(ctx, &tmpl, orgID)
@ -88,8 +95,6 @@ func (t *TemplateService) GetTemplate(ctx context.Context, orgID int64, name str
tmpl.Provenance = definitions.Provenance(provenance)
return tmpl, nil
}
return definitions.NotificationTemplate{}, ErrTemplateNotFound.Errorf("")
}
func (t *TemplateService) UpsertTemplate(ctx context.Context, orgID int64, tmpl definitions.NotificationTemplate) (definitions.NotificationTemplate, error) {
err := tmpl.Validate()
@ -107,7 +112,11 @@ func (t *TemplateService) UpsertTemplate(ctx context.Context, orgID int64, tmpl
if !errors.Is(err, ErrTemplateNotFound) {
return d, err
}
if tmpl.ResourceVersion != "" { // if version is set then it's an update operation. Fail because resource does not exist anymore
// If template was not found, this is assumed to be a create operation except for two cases:
// - If a ResourceVersion is provided: we should assume that this was meant to be a conditional update operation.
// - If UID is provided: custom UID for templates is not currently supported, so this was meant to be an update
// operation without a ResourceVersion.
if tmpl.ResourceVersion != "" || tmpl.UID != "" {
return definitions.NotificationTemplate{}, ErrTemplateNotFound.Errorf("")
}
return t.createTemplate(ctx, revision, orgID, tmpl)
@ -150,6 +159,7 @@ func (t *TemplateService) createTemplate(ctx context.Context, revision *legacy_s
}
return definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -175,12 +185,28 @@ func (t *TemplateService) updateTemplate(ctx context.Context, revision *legacy_s
revision.Config.TemplateFiles = map[string]string{}
}
existingName := tmpl.Name
exisitingContent, found := revision.Config.TemplateFiles[existingName]
var found bool
var existingName, existingContent string
// if UID is specified, look by UID.
if tmpl.UID != "" {
existingName, existingContent, found = getTemplateByUid(revision.Config.TemplateFiles, tmpl.UID)
// do not fall back to name because we address by UID, and resource can be deleted\renamed
} else {
existingName = tmpl.Name
existingContent, found = revision.Config.TemplateFiles[existingName]
}
if !found {
return definitions.NotificationTemplate{}, ErrTemplateNotFound.Errorf("")
}
if existingName != tmpl.Name { // if template is renamed, check if this name is already taken
_, ok := revision.Config.TemplateFiles[tmpl.Name]
if ok {
// return error if template is being renamed to one that already exists
return definitions.NotificationTemplate{}, ErrTemplateExists.Errorf("")
}
}
// check that provenance is not changed in an invalid way
storedProvenance, err := t.provenanceStore.GetProvenance(ctx, &tmpl, orgID)
if err != nil {
@ -190,7 +216,7 @@ func (t *TemplateService) updateTemplate(ctx context.Context, revision *legacy_s
return definitions.NotificationTemplate{}, err
}
err = t.checkOptimisticConcurrency(tmpl.Name, exisitingContent, models.Provenance(tmpl.Provenance), tmpl.ResourceVersion, "update")
err = t.checkOptimisticConcurrency(tmpl.Name, existingContent, models.Provenance(tmpl.Provenance), tmpl.ResourceVersion, "update")
if err != nil {
return definitions.NotificationTemplate{}, err
}
@ -198,6 +224,14 @@ func (t *TemplateService) updateTemplate(ctx context.Context, revision *legacy_s
revision.Config.TemplateFiles[tmpl.Name] = tmpl.Template
err = t.xact.InTransaction(ctx, func(ctx context.Context) error {
if existingName != tmpl.Name { // if template by was found by UID and it's name is different, then this is the rename operation. Delete old resources.
delete(revision.Config.TemplateFiles, existingName)
err := t.provenanceStore.DeleteProvenance(ctx, &definitions.NotificationTemplate{Name: existingName}, orgID)
if err != nil {
return err
}
}
if err := t.configStore.Save(ctx, revision, orgID); err != nil {
return err
}
@ -208,6 +242,7 @@ func (t *TemplateService) updateTemplate(ctx context.Context, revision *legacy_s
}
return definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name), // if name was changed, this UID will not match the incoming one
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -215,7 +250,7 @@ func (t *TemplateService) updateTemplate(ctx context.Context, revision *legacy_s
}, nil
}
func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance, version string) error {
func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, nameOrUid string, provenance definitions.Provenance, version string) error {
revision, err := t.configStore.Get(ctx, orgID)
if err != nil {
return err
@ -225,18 +260,22 @@ func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name
return nil
}
existing, ok := revision.Config.TemplateFiles[name]
existingName := nameOrUid
existing, ok := revision.Config.TemplateFiles[nameOrUid]
if !ok {
existingName, existing, ok = getTemplateByUid(revision.Config.TemplateFiles, nameOrUid)
}
if !ok {
return nil
}
err = t.checkOptimisticConcurrency(name, existing, models.Provenance(provenance), version, "delete")
err = t.checkOptimisticConcurrency(existingName, existing, models.Provenance(provenance), version, "delete")
if err != nil {
return err
}
// check that provenance is not changed in an invalid way
storedProvenance, err := t.provenanceStore.GetProvenance(ctx, &definitions.NotificationTemplate{Name: name}, orgID)
storedProvenance, err := t.provenanceStore.GetProvenance(ctx, &definitions.NotificationTemplate{Name: existingName}, orgID)
if err != nil {
return err
}
@ -244,14 +283,14 @@ func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name
return err
}
delete(revision.Config.TemplateFiles, name)
delete(revision.Config.TemplateFiles, existingName)
return t.xact.InTransaction(ctx, func(ctx context.Context) error {
if err := t.configStore.Save(ctx, revision, orgID); err != nil {
return err
}
tgt := definitions.NotificationTemplate{
Name: name,
Name: existingName,
}
return t.provenanceStore.DeleteProvenance(ctx, &tgt, orgID)
})
@ -277,3 +316,12 @@ func calculateTemplateFingerprint(t string) string {
_, _ = sum.Write(unsafe.Slice(unsafe.StringData(t), len(t))) //nolint:gosec
return fmt.Sprintf("%016x", sum.Sum64())
}
func getTemplateByUid(templates map[string]string, uid string) (string, string, bool) {
for n, tmpl := range templates {
if legacy_storage.NameToUid(n) == uid {
return n, tmpl, true
}
}
return "", "", false
}

View File

@ -46,18 +46,21 @@ func TestGetTemplates(t *testing.T) {
expected := []definitions.NotificationTemplate{
{
UID: legacy_storage.NameToUid("template1"),
Name: "template1",
Template: "test1",
Provenance: definitions.Provenance(models.ProvenanceAPI),
ResourceVersion: calculateTemplateFingerprint("test1"),
},
{
UID: legacy_storage.NameToUid("template2"),
Name: "template2",
Template: "test2",
Provenance: definitions.Provenance(models.ProvenanceFile),
ResourceVersion: calculateTemplateFingerprint("test2"),
},
{
UID: legacy_storage.NameToUid("template3"),
Name: "template3",
Template: "test3",
Provenance: definitions.Provenance(models.ProvenanceNone),
@ -144,6 +147,7 @@ func TestGetTemplate(t *testing.T) {
require.NoError(t, err)
expected := definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(templateName),
Name: templateName,
Template: templateContent,
Provenance: definitions.Provenance(models.ProvenanceAPI),
@ -244,6 +248,7 @@ func TestUpsertTemplate(t *testing.T) {
require.NoError(t, err)
require.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -285,6 +290,7 @@ func TestUpsertTemplate(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -321,6 +327,7 @@ func TestUpsertTemplate(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -355,6 +362,7 @@ func TestUpsertTemplate(t *testing.T) {
expectedContent := fmt.Sprintf("{{ define \"%s\" }}\n content\n{{ end }}", templateName)
require.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: expectedContent,
Provenance: tmpl.Provenance,
@ -465,6 +473,22 @@ func TestUpsertTemplate(t *testing.T) {
_, err := sut.UpsertTemplate(context.Background(), orgID, template)
require.ErrorIs(t, err, ErrTemplateNotFound)
})
t.Run("rejects new template has UID ", func(t *testing.T) {
sut, store, _ := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
return revision(), nil
}
template := definitions.NotificationTemplate{
UID: "new-template",
Name: "template2",
Template: "asdf-new",
Provenance: definitions.Provenance(models.ProvenanceNone),
}
_, err := sut.UpsertTemplate(context.Background(), orgID, template)
require.ErrorIs(t, err, ErrTemplateNotFound)
})
t.Run("propagates errors", func(t *testing.T) {
tmpl := definitions.NotificationTemplate{
Name: templateName,
@ -565,6 +589,7 @@ func TestCreateTemplate(t *testing.T) {
require.NoError(t, err)
require.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -695,7 +720,7 @@ func TestUpdateTemplate(t *testing.T) {
}
}
t.Run("returns ErrTemplateNotFound if template does not exist", func(t *testing.T) {
t.Run("returns ErrTemplateNotFound if template name does not exist", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
assert.Equal(t, orgID, org)
@ -712,7 +737,46 @@ func TestUpdateTemplate(t *testing.T) {
prov.AssertExpectations(t)
})
t.Run("updates current template", func(t *testing.T) {
t.Run("returns ErrTemplateNotFound if template UID does not exist", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
assert.Equal(t, orgID, org)
return &legacy_storage.ConfigRevision{
Config: &definitions.PostableUserConfig{
TemplateFiles: map[string]string{
"not-found": "test", // create a template with name that matches UID to make sure we do not search by name
tmpl.Name: "test",
},
},
ConcurrencyToken: amConfigToken,
}, nil
}
tmpl := tmpl
tmpl.UID = "not-found"
_, err := sut.UpdateTemplate(context.Background(), orgID, tmpl)
require.ErrorIs(t, err, ErrTemplateNotFound)
require.Len(t, store.Calls, 1)
prov.AssertExpectations(t)
})
testcases := []struct {
name string
templateUid string
}{
{
name: "by name",
templateUid: "",
},
{
name: "by uid",
templateUid: legacy_storage.NameToUid(tmpl.UID),
},
}
for _, tt := range testcases {
t.Run(fmt.Sprintf("updates current template %s", tt.name), func(t *testing.T) {
t.Run("when version matches", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
@ -723,10 +787,12 @@ func TestUpdateTemplate(t *testing.T) {
assertInTransaction(t, ctx)
}).Return(nil)
tmpl.UID = tt.templateUid
result, err := sut.UpdateTemplate(context.Background(), orgID, tmpl)
require.NoError(t, err)
assert.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -756,6 +822,7 @@ func TestUpdateTemplate(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
@ -769,6 +836,73 @@ func TestUpdateTemplate(t *testing.T) {
assert.Equal(t, tmpl.Template, saved.Config.TemplateFiles[tmpl.Name])
})
})
}
t.Run("creates a new template and delete old one when template is renamed", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
return revision(), nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).Return(nil).Run(func(ctx context.Context, o models.Provisionable, org int64) {
assertInTransaction(t, ctx)
}).Return(nil)
prov.EXPECT().SetProvenance(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(ctx context.Context, o models.Provisionable, org int64, p models.Provenance) {
assertInTransaction(t, ctx)
}).Return(nil)
oldName := tmpl.Name
tmpl := tmpl
tmpl.UID = legacy_storage.NameToUid(tmpl.Name) // UID matches the current template
tmpl.Name = "new-template-name" // but name is different
result, err := sut.UpdateTemplate(context.Background(), orgID, tmpl)
require.NoError(t, err)
assert.Equal(t, definitions.NotificationTemplate{
UID: legacy_storage.NameToUid(tmpl.Name),
Name: tmpl.Name,
Template: tmpl.Template,
Provenance: tmpl.Provenance,
ResourceVersion: calculateTemplateFingerprint(tmpl.Template),
}, result)
require.Len(t, store.Calls, 2)
require.Equal(t, "Save", store.Calls[1].Method)
saved := store.Calls[1].Args[1].(*legacy_storage.ConfigRevision)
assert.Equal(t, amConfigToken, saved.ConcurrencyToken)
assert.Contains(t, saved.Config.TemplateFiles, tmpl.Name)
assert.Equal(t, tmpl.Template, saved.Config.TemplateFiles[tmpl.Name])
assert.NotContains(t, saved.Config.TemplateFiles, oldName)
prov.AssertCalled(t, "DeleteProvenance", mock.Anything, mock.MatchedBy(func(t *definitions.NotificationTemplate) bool {
return t.Name == oldName
}), mock.Anything)
prov.AssertExpectations(t)
})
t.Run("rejects rename operation if template with the new name exists", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, org int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{
Config: &definitions.PostableUserConfig{
TemplateFiles: map[string]string{
tmpl.Name: currentTemplateContent,
"new-template-name": "test",
},
},
ConcurrencyToken: amConfigToken,
}, nil
}
tmpl := tmpl
tmpl.UID = legacy_storage.NameToUid(tmpl.Name) // UID matches the current template
tmpl.Name = "new-template-name" // but name matches another existing template
_, err := sut.UpdateTemplate(context.Background(), orgID, tmpl)
require.ErrorIs(t, err, ErrTemplateExists)
prov.AssertExpectations(t)
})
t.Run("rejects templates that fail validation", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
@ -918,7 +1052,21 @@ func TestDeleteTemplate(t *testing.T) {
}
}
t.Run("deletes template from config file on success", func(t *testing.T) {
testCase := []struct {
name string
templateNameOrUid string
}{
{
name: "by name",
templateNameOrUid: templateName,
},
{
name: "by uid",
templateNameOrUid: legacy_storage.NameToUid(templateName),
},
}
for _, tt := range testCase {
t.Run(fmt.Sprintf("deletes template from config file %s", tt.name), func(t *testing.T) {
t.Run("when version matches", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
@ -929,7 +1077,7 @@ func TestDeleteTemplate(t *testing.T) {
assertInTransaction(t, ctx)
}).Return(nil)
err := sut.DeleteTemplate(context.Background(), orgID, templateName, definitions.Provenance(models.ProvenanceFile), templateVersion)
err := sut.DeleteTemplate(context.Background(), orgID, tt.templateNameOrUid, definitions.Provenance(models.ProvenanceFile), templateVersion)
require.NoError(t, err)
@ -957,7 +1105,7 @@ func TestDeleteTemplate(t *testing.T) {
assertInTransaction(t, ctx)
}).Return(nil)
err := sut.DeleteTemplate(context.Background(), orgID, templateName, definitions.Provenance(models.ProvenanceFile), "")
err := sut.DeleteTemplate(context.Background(), orgID, tt.templateNameOrUid, definitions.Provenance(models.ProvenanceFile), "")
require.NoError(t, err)
require.Len(t, store.Calls, 2)
@ -974,6 +1122,45 @@ func TestDeleteTemplate(t *testing.T) {
prov.AssertExpectations(t)
})
})
}
t.Run("should look by name before uid", func(t *testing.T) {
expectedToDelete := legacy_storage.NameToUid(templateName)
sut, store, prov := createTemplateServiceSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{
Config: &definitions.PostableUserConfig{
TemplateFiles: map[string]string{
templateName: templateContent,
expectedToDelete: templateContent,
},
},
ConcurrencyToken: amConfigToken,
}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceFile, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).Run(func(ctx context.Context, o models.Provisionable, org int64) {
assertInTransaction(t, ctx)
}).Return(nil)
err := sut.DeleteTemplate(context.Background(), orgID, expectedToDelete, definitions.Provenance(models.ProvenanceFile), templateVersion)
require.NoError(t, err)
require.Len(t, store.Calls, 2)
require.Equal(t, "Save", store.Calls[1].Method)
saved := store.Calls[1].Args[1].(*legacy_storage.ConfigRevision)
assert.Equal(t, amConfigToken, saved.ConcurrencyToken)
assert.NotContains(t, saved.Config.TemplateFiles, expectedToDelete)
assert.Contains(t, saved.Config.TemplateFiles, templateName)
prov.AssertCalled(t, "DeleteProvenance", mock.Anything, mock.MatchedBy(func(t *definitions.NotificationTemplate) bool {
return t.Name == expectedToDelete
}), orgID)
prov.AssertExpectations(t)
})
t.Run("does not error when deleting templates that do not exist", func(t *testing.T) {
sut, store, prov := createTemplateServiceSut()