IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Update mute timings provisioning API to support optimistic locking (#88731)

Browse Source 
* add version to time-interval models
* set time interval fingerprint as version
* update to check provided version
* delete to check if version is provided in query parameter 'version'
* update integration tests
* update specs
This commit is contained in:
Yuri Tseretyan
2024-06-06 18:06:37 -04:00
committed by GitHub
parent a2e21d61f8
commit 003e3efce9
13 changed files with 379 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/services/ngalert/api/api_provisioning.go
View File

@@ -60,7 +60,7 @@ type MuteTimingService interface {
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) error
DeleteMuteTiming(ctx context.Context, name string, orgID int64, provenance definitions.Provenance, version string) error
}
type AlertRuleService interface {
@@ -307,7 +307,8 @@ func (srv *ProvisioningSrv) RoutePutMuteTiming(c *contextmodel.ReqContext, mt de
}
func (srv *ProvisioningSrv) RouteDeleteMuteTiming(c *contextmodel.ReqContext, name string) response.Response {
err := srv.muteTimings.DeleteMuteTiming(c.Req.Context(), name, c.SignedInUser.GetOrgID(), determineProvenance(c))
version := c.Query("version")
err := srv.muteTimings.DeleteMuteTiming(c.Req.Context(), name, c.SignedInUser.GetOrgID(), determineProvenance(c), version)
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "failed to delete mute timing", err)
}

23
pkg/services/ngalert/api/tooling/api.json
View File

@@ -1713,6 +1713,9 @@
"$ref": "#/definitions/TimeIntervalItem"
},
"type": "array"
},
"version": {
"type": "string"
}
},
"type": "object"
@@ -2800,6 +2803,9 @@
"$ref": "#/definitions/TimeIntervalItem"
},
"type": "array"
},
"version": {
"type": "string"
}
},
"type": "object"
@@ -5780,6 +5786,17 @@
"name": "name",
"required": true,
"type": "string"
},
{
"description": "Version of mute timing to use for optimistic concurrency. Leave empty to disable validation",
"in": "query",
"name": "version",
"type": "string"
},
{
"in": "header",
"name": "X-Disable-Provenance",
"type": "string"
}
],
"responses": {
@@ -5863,6 +5880,12 @@
"schema": {
"$ref": "#/definitions/ValidationError"
}
},
"409": {
"description": "GenericPublicError",
"schema": {
"$ref": "#/definitions/GenericPublicError"
}
}
},
"summary": "Replace an existing mute timing.",

17
pkg/services/ngalert/api/tooling/definitions/provisioning_mute_timings.go
View File

@@ -70,6 +70,7 @@ import (
// Responses:
// 202: MuteTimeInterval
// 400: ValidationError
// 409: GenericPublicError
// swagger:route DELETE /v1/provisioning/mute-timings/{name} provisioning stable RouteDeleteMuteTiming
//
@@ -84,20 +85,31 @@ import (
// swagger:model
type MuteTimings []MuteTimeInterval
// swagger:parameters RouteGetTemplate RouteGetMuteTiming RoutePutMuteTiming stable RouteDeleteMuteTiming RouteExportMuteTiming
// swagger:parameters RouteGetTemplate RouteGetMuteTiming RoutePutMuteTiming stable RouteExportMuteTiming
type RouteGetMuteTimingParam struct {
// Mute timing name
// in:path
Name string `json:"name"`
}
// swagger:parameters stable RouteDeleteMuteTiming
type RouteDeleteMuteTimingParam struct {
// Mute timing name
// in:path
Name string `json:"name"`
// Version of mute timing to use for optimistic concurrency. Leave empty to disable validation
// in:query
Version string `json:"version"`
}
// swagger:parameters RoutePostMuteTiming RoutePutMuteTiming
type MuteTimingPayload struct {
// in:body
Body MuteTimeInterval
}
// swagger:parameters RoutePostMuteTiming RoutePutMuteTiming
// swagger:parameters RoutePostMuteTiming RoutePutMuteTiming RouteDeleteMuteTiming
type MuteTimingHeaders struct {
// in:header
XDisableProvenance string `json:"X-Disable-Provenance"`
@@ -106,6 +118,7 @@ type MuteTimingHeaders struct {
// swagger:model
type MuteTimeInterval struct {
config.MuteTimeInterval `json:",inline" yaml:",inline"`
Version string `json:"version,omitempty"`
Provenance Provenance `json:"provenance,omitempty"`
}

2
pkg/services/ngalert/api/tooling/definitions/time_intervals.go
View File

@@ -39,6 +39,7 @@ type GetIntervalsByNameResponse struct {
// swagger:model
type PostableTimeIntervals struct {
Name string `json:"name" hcl:"name"`
Version string `json:"version,omitempty"`
TimeIntervals []TimeIntervalItem `json:"time_intervals" hcl:"intervals,block"`
}
@@ -60,5 +61,6 @@ type TimeIntervalTimeRange struct {
type GettableTimeIntervals struct {
Name string `json:"name" hcl:"name"`
TimeIntervals []TimeIntervalItem `json:"time_intervals" hcl:"intervals,block"`
Version string `json:"version,omitempty"`
Provenance Provenance `json:"provenance,omitempty"`
}

20
pkg/services/ngalert/api/tooling/post.json
View File

@@ -2802,6 +2802,9 @@
"$ref": "#/definitions/TimeIntervalItem"
},
"type": "array"
},
"version": {
"type": "string"
}
},
"type": "object"
@@ -7896,6 +7899,17 @@
"name": "name",
"required": true,
"type": "string"
},
{
"description": "Version of mute timing to use for optimistic concurrency. Leave empty to disable validation",
"in": "query",
"name": "version",
"type": "string"
},
{
"in": "header",
"name": "X-Disable-Provenance",
"type": "string"
}
],
"responses": {
@@ -7979,6 +7993,12 @@
"schema": {
"$ref": "#/definitions/ValidationError"
}
},
"409": {
"description": "GenericPublicError",
"schema": {
"$ref": "#/definitions/GenericPublicError"
}
}
},
"summary": "Replace an existing mute timing.",

20
pkg/services/ngalert/api/tooling/spec.json
View File

@@ -3060,6 +3060,12 @@
"schema": {
"$ref": "#/definitions/ValidationError"
}
},
"409": {
"description": "GenericPublicError",
"schema": {
"$ref": "#/definitions/GenericPublicError"
}
}
}
},
@@ -3077,6 +3083,17 @@
"name": "name",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Version of mute timing to use for optimistic concurrency. Leave empty to disable validation",
"name": "version",
"in": "query"
},
{
"type": "string",
"name": "X-Disable-Provenance",
"in": "header"
}
],
"responses": {
@@ -6316,6 +6333,9 @@
"items": {
"$ref": "#/definitions/TimeIntervalItem"
}
},
"version": {
"type": "string"
}
}
},

2
pkg/services/ngalert/provisioning/errors.go
View File

@@ -21,6 +21,8 @@ var (
errutil.WithPublic("Resource with provenance status '{{ .Public.CurrentProvenance }}' cannot be managed via API that handles resources with provenance status '{{ .Public.TargetProvenance }}'. You must use appropriate API to manage this resource"),
)
ErrVersionConflict = errutil.Conflict("alerting.notifications.conflict")
ErrTimeIntervalNotFound = errutil.NotFound("alerting.notifications.time-intervals.notFound")
ErrTimeIntervalExists = errutil.BadRequest("alerting.notifications.time-intervals.nameExists", errutil.WithPublicMessage("Time interval with this name already exists. Use a different name or update existing one."))
ErrTimeIntervalInvalid = errutil.BadRequest("alerting.notifications.time-intervals.invalidFormat").MustTemplate("Invalid format of the submitted time interval", errutil.WithPublic("Time interval is in invalid format. Correct the payload and try again."))

112
pkg/services/ngalert/provisioning/mute_timings.go
View File

@@ -2,8 +2,13 @@ package provisioning
import (
"context"
"encoding/binary"
"fmt"
"hash/fnv"
"unsafe"
"github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/timeinterval"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
@@ -46,7 +51,8 @@ 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 {
def := definitions.MuteTimeInterval{MuteTimeInterval: interval}
version := calculateMuteTimeIntervalFingerprint(interval)
def := definitions.MuteTimeInterval{MuteTimeInterval: interval, Version: version}
if prov, ok := provenances[def.ResourceID()]; ok {
def.Provenance = definitions.Provenance(prov)
}
@@ -69,6 +75,7 @@ func (svc *MuteTimingService) GetMuteTiming(ctx context.Context, name string, or
result := definitions.MuteTimeInterval{
MuteTimeInterval: mt,
Version: calculateMuteTimeIntervalFingerprint(mt),
}
prov, err := svc.provenanceStore.GetProvenance(ctx, &result, orgID)
@@ -109,7 +116,11 @@ func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitio
if err != nil {
return definitions.MuteTimeInterval{}, err
}
return mt, nil
return definitions.MuteTimeInterval{
MuteTimeInterval: mt.MuteTimeInterval,
Version: calculateMuteTimeIntervalFingerprint(mt.MuteTimeInterval),
Provenance: mt.Provenance,
}, nil
}
// UpdateMuteTiming replaces an existing mute timing within the specified org. The replaced mute timing is returned. If the mute timing does not exist, ErrMuteTimingsNotFound is returned.
@@ -136,10 +147,16 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, nil
}
_, idx, err := getMuteTiming(revision, mt.Name)
old, idx, err := getMuteTiming(revision, mt.Name)
if err != nil {
return definitions.MuteTimeInterval{}, err
}
err = svc.checkOptimisticConcurrency(old, models.Provenance(mt.Provenance), mt.Version, "update")
if err != nil {
return definitions.MuteTimeInterval{}, err
}
revision.cfg.AlertmanagerConfig.MuteTimeIntervals[idx] = mt.MuteTimeInterval
// TODO add diff and noop detection
@@ -152,11 +169,15 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
if err != nil {
return definitions.MuteTimeInterval{}, err
}
return mt, err
return definitions.MuteTimeInterval{
MuteTimeInterval: mt.MuteTimeInterval,
Version: calculateMuteTimeIntervalFingerprint(mt.MuteTimeInterval),
Provenance: mt.Provenance,
}, err
}
// 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) error {
func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, name string, orgID int64, provenance definitions.Provenance, version string) error {
target := definitions.MuteTimeInterval{MuteTimeInterval: config.MuteTimeInterval{Name: name}, Provenance: provenance}
// check that provenance is not changed in an invalid way
storedProvenance, err := svc.provenanceStore.GetProvenance(ctx, &target, orgID)
@@ -179,10 +200,15 @@ func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, name string,
return ErrTimeIntervalInUse.Errorf("")
}
for i, existing := range revision.cfg.AlertmanagerConfig.MuteTimeIntervals {
if name == existing.Name {
intervals := revision.cfg.AlertmanagerConfig.MuteTimeIntervals
revision.cfg.AlertmanagerConfig.MuteTimeIntervals = append(intervals[:i], intervals[i+1:]...)
if name != existing.Name {
continue
}
err = svc.checkOptimisticConcurrency(existing, models.Provenance(provenance), version, "delete")
if err != nil {
return err
}
intervals := revision.cfg.AlertmanagerConfig.MuteTimeIntervals
revision.cfg.AlertmanagerConfig.MuteTimeIntervals = append(intervals[:i], intervals[i+1:]...)
}
return svc.xact.InTransaction(ctx, func(ctx context.Context) error {
@@ -221,3 +247,73 @@ func getMuteTiming(rev *cfgRevision, name string) (config.MuteTimeInterval, int,
}
return config.MuteTimeInterval{}, -1, ErrTimeIntervalNotFound.Errorf("")
}
func calculateMuteTimeIntervalFingerprint(interval config.MuteTimeInterval) string {
sum := fnv.New64()
writeBytes := func(b []byte) {
_, _ = sum.Write(b)
// add a byte sequence that cannot happen in UTF-8 strings.
_, _ = sum.Write([]byte{255})
}
writeString := func(s string) {
if len(s) == 0 {
writeBytes(nil)
return
}
// #nosec G103
// avoid allocation when converting string to byte slice
writeBytes(unsafe.Slice(unsafe.StringData(s), len(s)))
}
// this temp slice is used to convert ints to bytes.
tmp := make([]byte, 8)
writeInt := func(u int) {
binary.LittleEndian.PutUint64(tmp, uint64(u))
writeBytes(tmp)
}
writeRange := func(r timeinterval.InclusiveRange) {
writeInt(r.Begin)
writeInt(r.End)
}
// fields that determine the rule state
writeString(interval.Name)
for _, ti := range interval.TimeIntervals {
for _, time := range ti.Times {
writeInt(time.StartMinute)
writeInt(time.EndMinute)
}
for _, itm := range ti.Months {
writeRange(itm.InclusiveRange)
}
for _, itm := range ti.DaysOfMonth {
writeRange(itm.InclusiveRange)
}
for _, itm := range ti.Weekdays {
writeRange(itm.InclusiveRange)
}
for _, itm := range ti.Years {
writeRange(itm.InclusiveRange)
}
if ti.Location != nil {
writeString(ti.Location.String())
}
}
return fmt.Sprintf("%016x", sum.Sum64())
}
func (svc *MuteTimingService) checkOptimisticConcurrency(current config.MuteTimeInterval, provenance models.Provenance, desiredVersion string, action string) error {
if desiredVersion == "" {
if provenance != models.ProvenanceFile {
// if version is not specified and it's not a file provisioning, emit a log message to reflect that optimistic concurrency is disabled for this request
svc.log.Debug("ignoring optimistic concurrency check because version was not provided", "timeInterval", current.Name, "operation", action)
}
return nil
}
currentVersion := calculateMuteTimeIntervalFingerprint(current)
if currentVersion != desiredVersion {
return ErrVersionConflict.Errorf("provided version %s of time interval %s does not match current version %s", desiredVersion, current.Name, currentVersion)
}
return nil
}

117
pkg/services/ngalert/provisioning/mute_timings_test.go
View File

@@ -61,10 +61,13 @@ func TestGetMuteTimings(t *testing.T) {
require.Len(t, result, len(revision.cfg.AlertmanagerConfig.MuteTimeIntervals))
require.Equal(t, "Test1", result[0].Name)
require.EqualValues(t, provenances["Test1"], result[0].Provenance)
require.NotEmpty(t, result[0].Version)
require.Equal(t, "Test2", result[1].Name)
require.EqualValues(t, provenances["Test2"], result[1].Provenance)
require.NotEmpty(t, result[1].Version)
require.Equal(t, "Test3", result[2].Name)
require.EqualValues(t, "", result[2].Provenance)
require.NotEmpty(t, result[2].Version)
require.Len(t, store.Calls, 1)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -143,6 +146,7 @@ func TestGetMuteTiming(t *testing.T) {
require.Equal(t, "Test1", result.Name)
require.EqualValues(t, models.ProvenanceAPI, result.Provenance)
require.NotEmpty(t, result.Version)
require.Len(t, store.Calls, 1)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -289,6 +293,7 @@ func TestCreateMuteTimings(t *testing.T) {
require.EqualValues(t, expected, result.MuteTimeInterval)
require.EqualValues(t, expectedProvenance, result.Provenance)
require.NotEmpty(t, result.Version)
require.Len(t, store.Calls, 2)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -362,6 +367,10 @@ func TestCreateMuteTimings(t *testing.T) {
func TestUpdateMuteTimings(t *testing.T) {
orgID := int64(1)
original := config.MuteTimeInterval{
Name: "Test",
}
originalVersion := calculateMuteTimeIntervalFingerprint(original)
initialConfig := func() *definitions.PostableUserConfig {
return &definitions.PostableUserConfig{
TemplateFiles: nil,
@@ -391,8 +400,10 @@ func TestUpdateMuteTimings(t *testing.T) {
},
}
expectedProvenance := models.ProvenanceAPI
expectedVersion := calculateMuteTimeIntervalFingerprint(expected)
timing := definitions.MuteTimeInterval{
MuteTimeInterval: expected,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
@@ -428,6 +439,25 @@ func TestUpdateMuteTimings(t *testing.T) {
require.ErrorIs(t, err, expectedErr)
})
t.Run("returns ErrVersionConflict if storage version does not match", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
}
timing := definitions.MuteTimeInterval{
MuteTimeInterval: expected,
Version: "some_random_version",
Provenance: definitions.Provenance(expectedProvenance),
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrVersionConflict)
})
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) {
@@ -446,7 +476,7 @@ func TestUpdateMuteTimings(t *testing.T) {
require.Truef(t, ErrTimeIntervalNotFound.Is(err), "expected ErrTimeIntervalNotFound but got %s", err)
})
t.Run("saves mute timing and provenance in a transaction", func(t *testing.T) {
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) {
return &cfgRevision{cfg: initialConfig()}, nil
@@ -467,6 +497,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.Len(t, store.Calls, 2)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -479,6 +510,41 @@ func TestUpdateMuteTimings(t *testing.T) {
require.EqualValues(t, []config.MuteTimeInterval{expected}, revision.cfg.AlertmanagerConfig.MuteTimeIntervals)
prov.AssertCalled(t, "SetProvenance", mock.Anything, &timing, orgID, expectedProvenance)
t.Run("bypass optimistic concurrency check if version is empty", func(t *testing.T) {
store.Calls = nil
timing := definitions.MuteTimeInterval{
MuteTimeInterval: config.MuteTimeInterval{
Name: expected.Name,
TimeIntervals: []timeinterval.TimeInterval{
{Months: []timeinterval.MonthRange{
{
InclusiveRange: timeinterval.InclusiveRange{
Begin: 1,
End: 10,
},
},
}},
},
},
Version: "",
Provenance: definitions.Provenance(expectedProvenance),
}
expectedVersion := calculateMuteTimeIntervalFingerprint(timing.MuteTimeInterval)
result, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.NoError(t, err)
require.EqualValues(t, timing.MuteTimeInterval, result.MuteTimeInterval)
require.Equal(t, expectedVersion, result.Version)
require.EqualValues(t, expectedProvenance, result.Provenance)
require.Equal(t, "Save", store.Calls[1].Method)
require.Equal(t, orgID, store.Calls[1].Args[2])
revision := store.Calls[1].Args[1].(*cfgRevision)
require.EqualValues(t, []config.MuteTimeInterval{timing.MuteTimeInterval}, revision.cfg.AlertmanagerConfig.MuteTimeIntervals)
})
})
t.Run("propagates errors", func(t *testing.T) {
@@ -547,6 +613,7 @@ func TestDeleteMuteTimings(t *testing.T) {
orgID := int64(1)
timingToDelete := config.MuteTimeInterval{Name: "unused-timing"}
correctVersion := calculateMuteTimeIntervalFingerprint(timingToDelete)
usedTiming := "used-timing"
initialConfig := func() *definitions.PostableUserConfig {
return &definitions.PostableUserConfig{
@@ -576,7 +643,7 @@ func TestDeleteMuteTimings(t *testing.T) {
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).Return(nil)
err := sut.DeleteMuteTiming(context.Background(), "no-timing", orgID, definitions.Provenance(models.ProvenanceAPI))
err := sut.DeleteMuteTiming(context.Background(), "no-timing", orgID, definitions.Provenance(models.ProvenanceAPI), "")
require.NoError(t, err)
require.Len(t, store.Calls, 2)
@@ -604,7 +671,7 @@ func TestDeleteMuteTimings(t *testing.T) {
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).Return(nil)
err := sut.DeleteMuteTiming(context.Background(), "no-timing", orgID, definitions.Provenance(models.ProvenanceNone))
err := sut.DeleteMuteTiming(context.Background(), "no-timing", orgID, definitions.Provenance(models.ProvenanceNone), correctVersion)
require.ErrorIs(t, err, expectedErr)
require.Len(t, store.Calls, 0)
@@ -617,7 +684,7 @@ func TestDeleteMuteTimings(t *testing.T) {
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
err := sut.DeleteMuteTiming(context.Background(), usedTiming, orgID, definitions.Provenance(models.ProvenanceAPI))
err := sut.DeleteMuteTiming(context.Background(), usedTiming, orgID, definitions.Provenance(models.ProvenanceAPI), correctVersion)
require.Len(t, store.Calls, 1)
require.Equal(t, "Get", store.Calls[0].Method)
@@ -625,7 +692,22 @@ func TestDeleteMuteTimings(t *testing.T) {
require.Truef(t, ErrTimeIntervalInUse.Is(err), "expected ErrTimeIntervalInUse but got %s", err)
})
t.Run("deletes mute timing and provenance in transaction", func(t *testing.T) {
t.Run("returns ErrVersionConflict if provided version does not match", 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(models.ProvenanceAPI, nil)
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, definitions.Provenance(models.ProvenanceAPI), "test-version")
require.Len(t, store.Calls, 1)
require.Equal(t, "Get", store.Calls[0].Method)
require.Equal(t, orgID, store.Calls[0].Args[1])
require.ErrorIs(t, err, ErrVersionConflict)
})
t.Run("deletes mute timing and provenance in transaction if passes optimistic concurrency check", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return &cfgRevision{cfg: initialConfig()}, nil
@@ -641,7 +723,7 @@ func TestDeleteMuteTimings(t *testing.T) {
return nil
})
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "")
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", correctVersion)
require.NoError(t, err)
require.Len(t, store.Calls, 2)
@@ -658,6 +740,23 @@ func TestDeleteMuteTimings(t *testing.T) {
require.EqualValues(t, expectedMuteTimings, revision.cfg.AlertmanagerConfig.MuteTimeIntervals)
prov.AssertCalled(t, "DeleteProvenance", mock.Anything, &definitions.MuteTimeInterval{MuteTimeInterval: timingToDelete}, orgID)
t.Run("should bypass optimistic concurrency check if version is empty", func(t *testing.T) {
store.Calls = nil
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", "")
require.NoError(t, err)
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) {
@@ -668,7 +767,7 @@ func TestDeleteMuteTimings(t *testing.T) {
store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
return nil, expectedErr
}
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "")
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", "")
require.ErrorIs(t, err, expectedErr)
})
@@ -683,7 +782,7 @@ func TestDeleteMuteTimings(t *testing.T) {
DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).
Return(expectedErr)
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "")
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", "")
require.ErrorIs(t, err, expectedErr)
@@ -705,7 +804,7 @@ func TestDeleteMuteTimings(t *testing.T) {
return expectedErr
}
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "")
err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", "")
require.ErrorIs(t, err, expectedErr)

2
pkg/services/provisioning/alerting/mute_times_provisioner.go
View File

@@ -63,7 +63,7 @@ func (c *defaultMuteTimesProvisioner) Unprovision(ctx context.Context,
files []*AlertingFile) error {
for _, file := range files {
for _, deleteMuteTime := range file.DeleteMuteTimes {
err := c.muteTimingService.DeleteMuteTiming(ctx, deleteMuteTime.Name, deleteMuteTime.OrgID, definitions.Provenance(models.ProvenanceFile))
err := c.muteTimingService.DeleteMuteTiming(ctx, deleteMuteTime.Name, deleteMuteTime.OrgID, definitions.Provenance(models.ProvenanceFile), "")
if err != nil {
return err
}

27
pkg/tests/api/alerting/api_provisioning_test.go
View File

@@ -527,7 +527,33 @@ func TestMuteTimings(t *testing.T) {
}
})
t.Run("should fail to update mute timing if version does not match", func(t *testing.T) {
tm := anotherMuteTiming
tm.Version = "wrong-version"
tm.TimeIntervals = []timeinterval.TimeInterval{
{
Times: []timeinterval.TimeRange{
{
StartMinute: 36,
EndMinute: 49,
},
},
},
}
_, status, body := apiClient.UpdateMuteTimingWithStatus(t, tm)
requireStatusCode(t, http.StatusConflict, status, body)
var validationError errutil.PublicError
assert.NoError(t, json.Unmarshal([]byte(body), &validationError))
assert.NotEmpty(t, validationError, validationError.Message)
assert.Equal(t, "alerting.notifications.conflict", validationError.MessageID)
if t.Failed() {
t.Fatalf("response: %s", body)
}
})
t.Run("should update existing mute timing", func(t *testing.T) {
mt, _, _ := apiClient.GetMuteTimingByNameWithStatus(t, anotherMuteTiming.Name)
anotherMuteTiming.TimeIntervals = []timeinterval.TimeInterval{
{
Times: []timeinterval.TimeRange{
@@ -538,6 +564,7 @@ func TestMuteTimings(t *testing.T) {
},
},
}
anotherMuteTiming.Version = mt.Version
mt, status, body := apiClient.UpdateMuteTimingWithStatus(t, anotherMuteTiming)
requireStatusCode(t, http.StatusAccepted, status, body)

23
public/api-merged.json
View File

@@ -11155,6 +11155,12 @@
"schema": {
"$ref": "#/definitions/ValidationError"
}
},
"409": {
"description": "GenericPublicError",
"schema": {
"$ref": "#/definitions/GenericPublicError"
}
}
}
},
@@ -11171,6 +11177,17 @@
"name": "name",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Version of mute timing to use for optimistic concurrency. Leave empty to disable validation",
"name": "version",
"in": "query"
},
{
"type": "string",
"name": "X-Disable-Provenance",
"in": "header"
}
],
"responses": {
@@ -15625,6 +15642,9 @@
"items": {
"$ref": "#/definitions/TimeIntervalItem"
}
},
"version": {
"type": "string"
}
}
},
@@ -17765,6 +17785,9 @@
"items": {
"$ref": "#/definitions/TimeIntervalItem"
}
},
"version": {
"type": "string"
}
}
},

31
public/openapi3.json
View File

@@ -6020,6 +6020,9 @@
"$ref": "#/components/schemas/TimeIntervalItem"
},
"type": "array"
},
"version": {
"type": "string"
}
},
"type": "object"
@@ -8160,6 +8163,9 @@
"$ref": "#/components/schemas/TimeIntervalItem"
},
"type": "array"
},
"version": {
"type": "string"
}
},
"type": "object"
@@ -24704,6 +24710,21 @@
"schema": {
"type": "string"
}
},
{
"description": "Version of mute timing to use for optimistic concurrency. Leave empty to disable validation",
"in": "query",
"name": "version",
"schema": {
"type": "string"
}
},
{
"in": "header",
"name": "X-Disable-Provenance",
"schema": {
"type": "string"
}
}
],
"responses": {
@@ -24809,6 +24830,16 @@
}
},
"description": "ValidationError"
},
"409": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/GenericPublicError"
}
}
},
"description": "GenericPublicError"
}
},
"summary": "Replace an existing mute timing.",