Alerting: time interval service to support addressing intervals by Base64 encoded name (#90563)

* rename to getMuteTimingByName

* add UID to api model of MuteTiming

* update GetMuteTiming to search by UID

* update UpdateMuteTiming to support search by UID

* update DeleteMuteTiming to support uid

* make sure UID is populated

* update usages

* use base64 url-safe, no padding encoding for UID
This commit is contained in:
Yuri Tseretyan
2024-07-26 16:43:40 -04:00
committed by GitHub
parent 2ab746ae76
commit 6b0d20c96a
6 changed files with 221 additions and 46 deletions

View File

@@ -2,8 +2,6 @@ package timeinterval
import (
"encoding/json"
"fmt"
"hash/fnv"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
@@ -14,12 +12,6 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
)
func getIntervalUID(t definitions.MuteTimeInterval) string {
sum := fnv.New64()
_, _ = sum.Write([]byte(t.Name))
return fmt.Sprintf("%016x", sum.Sum64())
}
func convertToK8sResources(orgID int64, intervals []definitions.MuteTimeInterval, namespacer request.NamespaceMapper, selector fields.Selector) (*model.TimeIntervalList, error) {
data, err := json.Marshal(intervals)
if err != nil {
@@ -59,12 +51,11 @@ func convertToK8sResource(orgID int64, interval definitions.MuteTimeInterval, na
}
func buildTimeInterval(orgID int64, interval definitions.MuteTimeInterval, spec model.TimeIntervalSpec, namespacer request.NamespaceMapper) model.TimeInterval {
uid := getIntervalUID(interval) // TODO replace to stable UID when we switch to normal storage
i := model.TimeInterval{
TypeMeta: resourceInfo.TypeMeta(),
ObjectMeta: metav1.ObjectMeta{
UID: types.UID(uid), // TODO This is needed to make PATCH work
Name: uid, // TODO replace to stable UID when we switch to normal storage
UID: types.UID(interval.UID), // TODO This is needed to make PATCH work
Name: interval.UID, // TODO replace to stable UID when we switch to normal storage
Namespace: namespacer(orgID),
ResourceVersion: interval.Version,
},
@@ -85,6 +76,7 @@ func convertToDomainModel(interval *model.TimeInterval) (definitions.MuteTimeInt
return definitions.MuteTimeInterval{}, err
}
result.Version = interval.ResourceVersion
result.UID = interval.ObjectMeta.Name
err = result.Validate()
if err != nil {
return definitions.MuteTimeInterval{}, err

View File

@@ -25,10 +25,9 @@ var resourceInfo = notifications.TimeIntervalResourceInfo
type TimeIntervalService interface {
GetMuteTimings(ctx context.Context, orgID int64) ([]definitions.MuteTimeInterval, error)
GetMuteTiming(ctx context.Context, name string, orgID int64) (definitions.MuteTimeInterval, error)
CreateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error)
UpdateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error)
DeleteMuteTiming(ctx context.Context, name string, orgID int64, provenance definitions.Provenance, version string) error
DeleteMuteTiming(ctx context.Context, nameOrUid string, orgID int64, provenance definitions.Provenance, version string) error
}
type legacyStorage struct {
@@ -85,7 +84,7 @@ func (s *legacyStorage) Get(ctx context.Context, uid string, _ *metav1.GetOption
}
for _, mt := range timings {
if getIntervalUID(mt) == uid {
if mt.UID == uid {
return convertToK8sResource(info.OrgID, mt, s.namespacer)
}
}
@@ -159,7 +158,7 @@ func (s *legacyStorage) Update(ctx context.Context,
return old, false, err
}
if p.ObjectMeta.Name != getIntervalUID(interval) {
if p.ObjectMeta.Name != interval.UID {
return nil, false, errors.NewBadRequest("title of cannot be changed. Consider creating a new resource.")
}
@@ -196,8 +195,8 @@ func (s *legacyStorage) Delete(ctx context.Context, uid string, deleteValidation
return nil, false, fmt.Errorf("expected time-interval but got %s", old.GetObjectKind().GroupVersionKind())
}
err = s.service.DeleteMuteTiming(ctx, p.Spec.Name, info.OrgID, definitions.Provenance(models.ProvenanceNone), version) // TODO add support for dry-run option
return old, false, err // false - will be deleted async
err = s.service.DeleteMuteTiming(ctx, p.ObjectMeta.Name, info.OrgID, definitions.Provenance(models.ProvenanceNone), version) // TODO add support for dry-run option
return old, false, err // false - will be deleted async
}
func (s *legacyStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {

View File

@@ -298,7 +298,14 @@ func (srv *ProvisioningSrv) RoutePostMuteTiming(c *contextmodel.ReqContext, mt d
}
func (srv *ProvisioningSrv) RoutePutMuteTiming(c *contextmodel.ReqContext, mt definitions.MuteTimeInterval, name string) response.Response {
mt.Name = name
// if body does not specify name, assume that the path contains the name
if mt.Name == "" {
mt.Name = name
}
// if body contains a name, and it's different from the one in the path, assume the latter to be UID
if mt.Name != name {
mt.UID = name
}
mt.Provenance = determineProvenance(c)
updated, err := srv.muteTimings.UpdateMuteTiming(c.Req.Context(), mt, c.SignedInUser.GetOrgID())
if err != nil {

View File

@@ -117,6 +117,7 @@ type MuteTimingHeaders struct {
// swagger:model
type MuteTimeInterval struct {
UID string `json:"-" yaml:"-"`
config.MuteTimeInterval `json:",inline" yaml:",inline"`
Version string `json:"version,omitempty"`
Provenance Provenance `json:"provenance,omitempty"`

View File

@@ -2,7 +2,9 @@ package provisioning
import (
"context"
"encoding/base64"
"encoding/binary"
"errors"
"fmt"
"hash/fnv"
"slices"
@@ -57,7 +59,11 @@ func (svc *MuteTimingService) GetMuteTimings(ctx context.Context, orgID int64) (
result := make([]definitions.MuteTimeInterval, 0, len(rev.cfg.AlertmanagerConfig.MuteTimeIntervals))
for _, interval := range rev.cfg.AlertmanagerConfig.MuteTimeIntervals {
version := calculateMuteTimeIntervalFingerprint(interval)
def := definitions.MuteTimeInterval{MuteTimeInterval: interval, Version: version}
def := definitions.MuteTimeInterval{
UID: getIntervalUID(interval),
MuteTimeInterval: interval,
Version: version,
}
if prov, ok := provenances[def.ResourceID()]; ok {
def.Provenance = definitions.Provenance(prov)
}
@@ -67,18 +73,25 @@ func (svc *MuteTimingService) GetMuteTimings(ctx context.Context, orgID int64) (
}
// GetMuteTiming returns a mute timing by name
func (svc *MuteTimingService) GetMuteTiming(ctx context.Context, name string, orgID int64) (definitions.MuteTimeInterval, error) {
func (svc *MuteTimingService) GetMuteTiming(ctx context.Context, nameOrUID string, orgID int64) (definitions.MuteTimeInterval, error) {
rev, err := svc.configStore.Get(ctx, orgID)
if err != nil {
return definitions.MuteTimeInterval{}, err
}
mt, idx := getMuteTiming(rev, name)
mt, idx := getMuteTimingByName(rev, nameOrUID)
if idx == -1 {
name, err := uidToName(nameOrUID)
if err == nil {
mt, idx = getMuteTimingByName(rev, name)
}
}
if idx == -1 {
return definitions.MuteTimeInterval{}, ErrTimeIntervalNotFound.Errorf("")
}
result := definitions.MuteTimeInterval{
UID: getIntervalUID(mt),
MuteTimeInterval: mt,
Version: calculateMuteTimeIntervalFingerprint(mt),
}
@@ -102,7 +115,7 @@ func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, err
}
_, idx := getMuteTiming(revision, mt.Name)
_, idx := getMuteTimingByName(revision, mt.Name)
if idx != -1 {
return definitions.MuteTimeInterval{}, ErrTimeIntervalExists.Errorf("")
}
@@ -118,6 +131,7 @@ func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, err
}
return definitions.MuteTimeInterval{
UID: getIntervalUID(mt.MuteTimeInterval),
MuteTimeInterval: mt.MuteTimeInterval,
Version: calculateMuteTimeIntervalFingerprint(mt.MuteTimeInterval),
Provenance: mt.Provenance,
@@ -130,8 +144,33 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, MakeErrTimeIntervalInvalid(err)
}
revision, err := svc.configStore.Get(ctx, orgID)
if err != nil {
return definitions.MuteTimeInterval{}, err
}
var old config.MuteTimeInterval
var idx = -1
if mt.UID != "" {
name, err := uidToName(mt.UID)
if err == nil {
old, idx = getMuteTimingByName(revision, name)
}
} else {
old, idx = getMuteTimingByName(revision, mt.Name)
}
if idx == -1 {
return definitions.MuteTimeInterval{}, ErrTimeIntervalNotFound.Errorf("")
}
// check optimistic concurrency
err = svc.checkOptimisticConcurrency(old, models.Provenance(mt.Provenance), mt.Version, "update")
if err != nil {
return definitions.MuteTimeInterval{}, err
}
// check that provenance is not changed in an invalid way
storedProvenance, err := svc.provenanceStore.GetProvenance(ctx, &mt, orgID)
storedProvenance, err := svc.provenanceStore.GetProvenance(ctx, &definitions.MuteTimeInterval{MuteTimeInterval: old}, orgID)
if err != nil {
return definitions.MuteTimeInterval{}, err
}
@@ -139,23 +178,10 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, err
}
revision, err := svc.configStore.Get(ctx, orgID)
if err != nil {
return definitions.MuteTimeInterval{}, err
}
if revision.cfg.AlertmanagerConfig.MuteTimeIntervals == nil {
return definitions.MuteTimeInterval{}, nil
}
old, idx := getMuteTiming(revision, mt.Name)
if idx == -1 {
return definitions.MuteTimeInterval{}, ErrTimeIntervalNotFound.Errorf("")
}
err = svc.checkOptimisticConcurrency(old, models.Provenance(mt.Provenance), mt.Version, "update")
if err != nil {
return definitions.MuteTimeInterval{}, err
// if the name of the time interval changed
if old.Name != mt.Name {
// TODO implement support for renaming.
return definitions.MuteTimeInterval{}, MakeErrTimeIntervalInvalid(errors.New("name change is not allowed"))
}
revision.cfg.AlertmanagerConfig.MuteTimeIntervals[idx] = mt.MuteTimeInterval
@@ -171,6 +197,7 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, err
}
return definitions.MuteTimeInterval{
UID: getIntervalUID(mt.MuteTimeInterval),
MuteTimeInterval: mt.MuteTimeInterval,
Version: calculateMuteTimeIntervalFingerprint(mt.MuteTimeInterval),
Provenance: mt.Provenance,
@@ -178,15 +205,21 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
}
// DeleteMuteTiming deletes the mute timing with the given name in the given org. If the mute timing does not exist, no error is returned.
func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, name string, orgID int64, provenance definitions.Provenance, version string) error {
func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, nameOrUID string, orgID int64, provenance definitions.Provenance, version string) error {
revision, err := svc.configStore.Get(ctx, orgID)
if err != nil {
return err
}
existing, idx := getMuteTiming(revision, name)
existing, idx := getMuteTimingByName(revision, nameOrUID)
if idx == -1 {
svc.log.FromContext(ctx).Debug("Time interval was not found. Skip deleting", "name", name)
name, err := uidToName(nameOrUID)
if err == nil {
existing, idx = getMuteTimingByName(revision, name)
}
}
if idx == -1 {
svc.log.FromContext(ctx).Debug("Time interval was not found. Skip deleting", "name", nameOrUID)
return nil
}
@@ -200,7 +233,7 @@ func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, name string,
return err
}
if isMuteTimeInUseInRoutes(name, revision.cfg.AlertmanagerConfig.Route) {
if isMuteTimeInUseInRoutes(existing.Name, revision.cfg.AlertmanagerConfig.Route) {
ns, _ := svc.ruleNotificationsStore.ListNotificationSettings(ctx, models.ListNotificationSettingsQuery{OrgID: orgID, TimeIntervalName: existing.Name})
// ignore error here because it's not important
return MakeErrTimeIntervalInUse(true, maps.Keys(ns))
@@ -243,7 +276,7 @@ func isMuteTimeInUseInRoutes(name string, route *definitions.Route) bool {
return false
}
func getMuteTiming(rev *cfgRevision, name string) (config.MuteTimeInterval, int) {
func getMuteTimingByName(rev *cfgRevision, name string) (config.MuteTimeInterval, int) {
idx := slices.IndexFunc(rev.cfg.AlertmanagerConfig.MuteTimeIntervals, func(interval config.MuteTimeInterval) bool {
return interval.Name == name
})
@@ -322,3 +355,15 @@ func (svc *MuteTimingService) checkOptimisticConcurrency(current config.MuteTime
}
return nil
}
func getIntervalUID(t config.MuteTimeInterval) string {
return base64.RawURLEncoding.EncodeToString([]byte(t.Name))
}
func uidToName(uid string) (string, error) {
data, err := base64.RawURLEncoding.DecodeString(uid)
if err != nil {
return uid, err
}
return string(data), nil
}

View File

@@ -63,12 +63,17 @@ func TestGetMuteTimings(t *testing.T) {
require.Equal(t, "Test1", result[0].Name)
require.EqualValues(t, provenances["Test1"], result[0].Provenance)
require.NotEmpty(t, result[0].Version)
require.Equal(t, getIntervalUID(result[0].MuteTimeInterval), result[0].UID)
require.Equal(t, "Test2", result[1].Name)
require.EqualValues(t, provenances["Test2"], result[1].Provenance)
require.NotEmpty(t, result[1].Version)
require.Equal(t, getIntervalUID(result[1].MuteTimeInterval), result[1].UID)
require.Equal(t, "Test3", result[2].Name)
require.EqualValues(t, "", result[2].Provenance)
require.NotEmpty(t, result[2].Version)
require.Equal(t, getIntervalUID(result[2].MuteTimeInterval), result[2].UID)
require.Len(t, store.Calls, 1)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -147,6 +152,7 @@ func TestGetMuteTiming(t *testing.T) {
require.Equal(t, "Test1", result.Name)
require.EqualValues(t, models.ProvenanceAPI, result.Provenance)
require.Equal(t, getIntervalUID(result.MuteTimeInterval), result.UID)
require.NotEmpty(t, result.Version)
require.Len(t, store.Calls, 1)
@@ -154,6 +160,14 @@ func TestGetMuteTiming(t *testing.T) {
require.Equal(t, orgID, store.Calls[0].Args[1])
prov.AssertCalled(t, "GetProvenance", mock.Anything, &result, orgID)
t.Run("and by UID", func(t *testing.T) {
result2, err := sut.GetMuteTiming(context.Background(), result.UID, orgID)
require.NoError(t, err)
require.Equal(t, result, result2)
})
})
t.Run("service returns ErrTimeIntervalNotFound if no mute timings", func(t *testing.T) {
@@ -294,6 +308,7 @@ func TestCreateMuteTimings(t *testing.T) {
require.EqualValues(t, expected, result.MuteTimeInterval)
require.EqualValues(t, expectedProvenance, result.Provenance)
require.Equal(t, getIntervalUID(expected), result.UID)
require.NotEmpty(t, result.Version)
require.Len(t, store.Calls, 2)
@@ -402,6 +417,7 @@ func TestUpdateMuteTimings(t *testing.T) {
}
expectedProvenance := models.ProvenanceAPI
expectedVersion := calculateMuteTimeIntervalFingerprint(expected)
expectedUID := getIntervalUID(expected)
timing := definitions.MuteTimeInterval{
MuteTimeInterval: expected,
Version: originalVersion,
@@ -423,7 +439,51 @@ func TestUpdateMuteTimings(t *testing.T) {
})
t.Run("rejects mute timings if provenance is not right", func(t *testing.T) {
sut, _, prov := createMuteTimingSvcSut()
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
expectedErr := errors.New("test")
sut.validator = func(from, to models.Provenance) error {
return expectedErr
}
timing := definitions.MuteTimeInterval{
MuteTimeInterval: expected,
Provenance: definitions.Provenance(models.ProvenanceFile),
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, expectedErr)
})
t.Run("rejects if mute timing is renamed", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalInvalid)
})
t.Run("rejects mute timings if provenance is not right", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
expectedErr := errors.New("test")
sut.validator = func(from, to models.Provenance) error {
return expectedErr
@@ -477,6 +537,39 @@ func TestUpdateMuteTimings(t *testing.T) {
require.Truef(t, ErrTimeIntervalNotFound.Is(err), "expected ErrTimeIntervalNotFound but got %s", err)
})
t.Run("returns ErrMuteTimingsNotFound if mute timing does not exist", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
t.Run("when UID is specified", func(t *testing.T) {
timing := definitions.MuteTimeInterval{
UID: "not-found",
MuteTimeInterval: expected,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalNotFound)
})
t.Run("when only Name is specified", func(t *testing.T) {
timing := definitions.MuteTimeInterval{
MuteTimeInterval: config.MuteTimeInterval{
Name: "not-found",
},
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalNotFound)
})
})
t.Run("saves mute timing and provenance in a transaction if optimistic concurrency passes", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
@@ -499,6 +592,7 @@ func TestUpdateMuteTimings(t *testing.T) {
require.EqualValues(t, expected, result.MuteTimeInterval)
require.EqualValues(t, expectedProvenance, result.Provenance)
require.EqualValues(t, expectedVersion, result.Version)
require.Equal(t, getIntervalUID(result.MuteTimeInterval), result.UID)
require.Len(t, store.Calls, 2)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -762,6 +856,43 @@ func TestDeleteMuteTimings(t *testing.T) {
})
})
t.Run("deletes mute timing and provenance by UID", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
store.SaveFn = func(ctx context.Context, revision *cfgRevision) error {
assertInTransaction(t, ctx)
return nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceNone, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, _ models.Provisionable, _ int64) error {
assertInTransaction(t, ctx)
return nil
})
uid := getIntervalUID(timingToDelete)
err := sut.DeleteMuteTiming(context.Background(), uid, orgID, "", correctVersion)
require.NoError(t, err)
require.Len(t, store.Calls, 2)
require.Equal(t, "Get", store.Calls[0].Method)
require.Equal(t, orgID, store.Calls[0].Args[1])
require.Equal(t, "Save", store.Calls[1].Method)
require.Equal(t, orgID, store.Calls[1].Args[2])
revision := store.Calls[1].Args[1].(*cfgRevision)
expectedMuteTimings := slices.DeleteFunc(initialConfig().AlertmanagerConfig.MuteTimeIntervals, func(interval config.MuteTimeInterval) bool {
return interval.Name == timingToDelete.Name
})
require.EqualValues(t, expectedMuteTimings, revision.cfg.AlertmanagerConfig.MuteTimeIntervals)
prov.AssertCalled(t, "DeleteProvenance", mock.Anything, &definitions.MuteTimeInterval{MuteTimeInterval: timingToDelete}, orgID)
})
t.Run("propagates errors", func(t *testing.T) {
t.Run("when unable to read config", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()