diff --git a/docs/sources/shared/alerts/alerting_provisioning.md b/docs/sources/shared/alerts/alerting_provisioning.md index b516880b53a..baa017bb57b 100644 --- a/docs/sources/shared/alerts/alerting_provisioning.md +++ b/docs/sources/shared/alerts/alerting_provisioning.md @@ -457,15 +457,17 @@ DELETE /api/v1/provisioning/mute-timings/:name #### Parameters -| Name | Source | Type | Go type | Separator | Required | Default | Description | -| ---- | ------ | ------ | -------- | --------- | :------: | ------- | ---------------- | -| name | `path` | string | `string` | | ✓ | | Mute timing name | +| Name | Source | Type | Go type | Separator | Required | Default | Description | +| ------- | ------- | ------ | -------- | --------- | :------: | ------- | ------------------------------------------------------------------------------------------------------------- | +| name | `path` | string | `string` | | ✓ | | Mute timing name | +| version | `query` | string | `string` | | | | Current version of the resource. Used for optimistic concurrency validation. Keep empty to bypass validation. | #### All responses | Code | Status | Description | Has headers | Schema | | ------------------------------------ | ---------- | ----------------------------------------- | :---------: | ---------------------------------------------- | | [204](#route-delete-mute-timing-204) | No Content | The mute timing was deleted successfully. | | [schema](#route-delete-mute-timing-204-schema) | +| [409](#route-delete-mute-timing-409) | Conflict | GenericPublicError | | [schema](#route-delete-mute-timing-409-schema) | #### Responses @@ -475,6 +477,14 @@ Status: No Content ###### Schema +##### 409 - Conflict + +Status: Conflict + +###### Schema + +[GenericPublicError](#generic-public-error) + ### Delete a template. (_RouteDeleteTemplate_) ``` @@ -483,15 +493,17 @@ DELETE /api/v1/provisioning/templates/:name #### Parameters -| Name | Source | Type | Go type | Separator | Required | Default | Description | -| ---- | ------ | ------ | -------- | --------- | :------: | ------- | ------------- | -| name | `path` | string | `string` | | ✓ | | Template Name | +| Name | Source | Type | Go type | Separator | Required | Default | Description | +| ------- | ------- | ------ | -------- | --------- | :------: | ------- | ------------------------------------------------------------------------------------------------------------- | +| name | `path` | string | `string` | | ✓ | | Template Name | +| version | `query` | string | `string` | | | | Current version of the resource. Used for optimistic concurrency validation. Keep empty to bypass validation. | #### All responses | Code | Status | Description | Has headers | Schema | | --------------------------------- | ---------- | -------------------------------------- | :---------: | ------------------------------------------- | | [204](#route-delete-template-204) | No Content | The template was deleted successfully. | | [schema](#route-delete-template-204-schema) | +| [409](#route-delete-template-409) | Conflict | GenericPublicError | | [schema](#route-delete-template-409-schema) | #### Responses @@ -501,6 +513,14 @@ Status: No Content ###### Schema +##### 409 - Conflict + +Status: Conflict + +###### Schema + +[GenericPublicError](#generic-public-error) + ### Get a specific alert rule by UID. (_RouteGetAlertRule_) ``` @@ -1363,10 +1383,11 @@ PUT /api/v1/provisioning/mute-timings/:name #### All responses -| Code | Status | Description | Has headers | Schema | -| --------------------------------- | ----------- | ---------------- | :---------: | ------------------------------------------- | -| [200](#route-put-mute-timing-200) | OK | MuteTimeInterval | | [schema](#route-put-mute-timing-200-schema) | -| [400](#route-put-mute-timing-400) | Bad Request | ValidationError | | [schema](#route-put-mute-timing-400-schema) | +| Code | Status | Description | Has headers | Schema | +| --------------------------------- | ----------- | ------------------ | :---------: | ------------------------------------------- | +| [200](#route-put-mute-timing-200) | OK | MuteTimeInterval | | [schema](#route-put-mute-timing-200-schema) | +| [400](#route-put-mute-timing-400) | Bad Request | ValidationError | | [schema](#route-put-mute-timing-400-schema) | +| [409](#route-put-mute-timing-409) | Conflict | GenericPublicError | | [schema](#route-put-mute-timing-409-schema) | #### Responses @@ -1386,6 +1407,14 @@ Status: Bad Request [ValidationError](#validation-error) +##### 409 - Conflict + +Status: Conflict + +###### Schema + +[GenericPublicError](#generic-public-error) + ### Sets the notification policy tree. (_RoutePutPolicyTree_) ``` @@ -1452,6 +1481,7 @@ PUT /api/v1/provisioning/templates/:name | ------------------------------ | ----------- | -------------------- | :---------: | ---------------------------------------- | | [202](#route-put-template-202) | Accepted | NotificationTemplate | | [schema](#route-put-template-202-schema) | | [400](#route-put-template-400) | Bad Request | ValidationError | | [schema](#route-put-template-400-schema) | +| [409](#route-put-template-409) | Conflict | GenericPublicError | | [schema](#route-put-template-409-schema) | #### Responses @@ -1471,6 +1501,14 @@ Status: Bad Request [ValidationError](#validation-error) +##### 409 - Conflict + +Status: Conflict + +###### Schema + +[GenericPublicError](#generic-public-error) + ### Clears the notification policy tree. (_RouteResetPolicyTree_) ``` @@ -1685,10 +1723,11 @@ Status: Accepted {{% responsive-table %}} -| Name | Type | Go type | Required | Default | Description | Example | -| -------------- | -------------------------------- | ----------------- | :------: | ------- | ----------- | ------- | -| name | string | `string` | | | | | -| time_intervals | [][TimeInterval](#time-interval) | `[]*TimeInterval` | | | | | +| Name | Type | Go type | Required | Default | Description | Example | +| -------------- | -------------------------------- | ----------------- | :------: | ------- | ------------------- | ------- | +| name | string | `string` | | | | | +| time_intervals | [][TimeInterval](#time-interval) | `[]*TimeInterval` | | | | | +| version | string | `string` | | | Version of resource | | {{% /responsive-table %}} @@ -1723,11 +1762,12 @@ Status: Accepted {{% responsive-table %}} -| Name | Type | Go type | Required | Default | Description | Example | -| ---------- | ------------------------- | ------------ | :------: | ------- | ----------- | ------- | -| name | string | `string` | | | | | -| provenance | [Provenance](#provenance) | `Provenance` | | | | | -| template | string | `string` | | | | | +| Name | Type | Go type | Required | Default | Description | Example | +| ---------- | ------------------------- | ------------ | :------: | ------- | ------------------- | ------- | +| name | string | `string` | | | | | +| provenance | [Provenance](#provenance) | `Provenance` | | | | | +| template | string | `string` | | | | | +| version | string | `string` | | | Version of resource | | {{% /responsive-table %}} @@ -1737,9 +1777,10 @@ Status: Accepted {{% responsive-table %}} -| Name | Type | Go type | Required | Default | Description | Example | -| -------- | ------ | -------- | :------: | ------- | ----------- | ------- | -| template | string | `string` | | | | | +| Name | Type | Go type | Required | Default | Description | Example | +| -------- | ------ | -------- | :------: | ------- | ------------------------------------------------------- | ------- | +| template | string | `string` | | | | | +| version | string | `string` | | | Version of resource. Should be empty for new templates. | | {{% /responsive-table %}} @@ -1927,3 +1968,18 @@ Status: Accepted | msg | string | `string` | | | | `error message` | {{% /responsive-table %}} + +### GenericPublicError + +**Properties** + +{{% responsive-table %}} + +| Name | Type | Go type | Required | Default | Description | Example | +| ---------- | ---------- | ---------------- | :------: | ------- | ------------------------------------------------------------------------ | ------- | +| statusCode | string | `string` | ✓ | | HTTP Status Code | | +| messageId | string | `string` | ✓ | | Unique code of the error | | +| message | string | `string` | | | Error message | | +| extra | map of any | `map[string]any` | | | Extra information about the error. Format is specific to the error code. | | + +{{% /responsive-table %}} diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index d66e6ea361e..399f3f49b29 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -46,7 +46,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, provenance definitions.Provenance) error + DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance, version string) error } type NotificationPolicyService interface { @@ -221,24 +221,26 @@ func (srv *ProvisioningSrv) RouteGetTemplate(c *contextmodel.ReqContext, name st func (srv *ProvisioningSrv) RoutePutTemplate(c *contextmodel.ReqContext, body definitions.NotificationTemplateContent, name string) response.Response { tmpl := definitions.NotificationTemplate{ - Name: name, - Template: body.Template, - Provenance: determineProvenance(c), + Name: name, + Template: body.Template, + Provenance: determineProvenance(c), + ResourceVersion: body.ResourceVersion, } modified, err := srv.templates.SetTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), tmpl) if err != nil { if errors.Is(err, provisioning.ErrValidation) { return ErrResp(http.StatusBadRequest, err, "") } - return ErrResp(http.StatusInternalServerError, err, "") + return response.ErrOrFallback(http.StatusInternalServerError, "", err) } return response.JSON(http.StatusAccepted, modified) } func (srv *ProvisioningSrv) RouteDeleteTemplate(c *contextmodel.ReqContext, name string) response.Response { - err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), name, determineProvenance(c)) + version := c.Query("version") + err := srv.templates.DeleteTemplate(c.Req.Context(), c.SignedInUser.GetOrgID(), name, determineProvenance(c), version) if err != nil { - return ErrResp(http.StatusInternalServerError, err, "") + return response.ErrOrFallback(http.StatusInternalServerError, "", err) } return response.JSON(http.StatusNoContent, nil) } diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index 396f5939bae..267ccb03a16 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -2264,6 +2264,9 @@ }, "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -2272,6 +2275,9 @@ "properties": { "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -6183,16 +6189,28 @@ "operationId": "RouteDeleteTemplate", "parameters": [ { - "description": "Template Name", + "description": "Template name", "in": "path", "name": "name", "required": true, "type": "string" + }, + { + "description": "Version of template to use for optimistic concurrency. Leave empty to disable validation", + "in": "query", + "name": "version", + "type": "string" } ], "responses": { "204": { "description": " The template was deleted successfully." + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Delete a template.", @@ -6265,6 +6283,12 @@ "schema": { "$ref": "#/definitions/ValidationError" } + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Updates an existing notification template.", diff --git a/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go b/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go index b7779e22321..41a1722cf47 100644 --- a/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go +++ b/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go @@ -26,6 +26,7 @@ package definitions // Responses: // 202: NotificationTemplate // 400: ValidationError +// 409: GenericPublicError // swagger:route DELETE /v1/provisioning/templates/{name} provisioning stable RouteDeleteTemplate // @@ -33,6 +34,7 @@ package definitions // // Responses: // 204: description: The template was deleted successfully. +// 409: GenericPublicError // swagger:parameters RouteGetTemplate RoutePutTemplate RouteDeleteTemplate type RouteGetTemplateParam struct { @@ -41,18 +43,31 @@ type RouteGetTemplateParam struct { Name string `json:"name"` } +// swagger:parameters stable RouteDeleteTemplate +type RouteDeleteTemplateParam struct { + // Template name + // in:path + Name string `json:"name"` + + // Version of template to use for optimistic concurrency. Leave empty to disable validation + // in:query + Version string `json:"version"` +} + // swagger:model type NotificationTemplate struct { - Name string `json:"name"` - Template string `json:"template"` - Provenance Provenance `json:"provenance,omitempty"` + Name string `json:"name"` + Template string `json:"template"` + Provenance Provenance `json:"provenance,omitempty"` + ResourceVersion string `json:"version,omitempty"` } // swagger:model type NotificationTemplates []NotificationTemplate type NotificationTemplateContent struct { - Template string `json:"template"` + Template string `json:"template"` + ResourceVersion string `json:"version,omitempty"` } // swagger:parameters RoutePutTemplate diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index 972d39735f0..ca0afdf9372 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -2264,6 +2264,9 @@ }, "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -2272,6 +2275,9 @@ "properties": { "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -8395,16 +8401,28 @@ "operationId": "RouteDeleteTemplate", "parameters": [ { - "description": "Template Name", + "description": "Template name", "in": "path", "name": "name", "required": true, "type": "string" + }, + { + "description": "Version of template to use for optimistic concurrency. Leave empty to disable validation", + "in": "query", + "name": "version", + "type": "string" } ], "responses": { "204": { "description": " The template was deleted successfully." + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Delete a template.", @@ -8477,6 +8495,12 @@ "schema": { "$ref": "#/definitions/ValidationError" } + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Updates an existing notification template.", diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index c185bb9c6d0..f8fd5e31ecb 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -3393,6 +3393,12 @@ "schema": { "$ref": "#/definitions/ValidationError" } + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } }, @@ -3406,15 +3412,27 @@ "parameters": [ { "type": "string", - "description": "Template Name", + "description": "Template name", "name": "name", "in": "path", "required": true + }, + { + "type": "string", + "description": "Version of template to use for optimistic concurrency. Leave empty to disable validation", + "name": "version", + "in": "query" } ], "responses": { "204": { "description": " The template was deleted successfully." + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } } @@ -5880,6 +5898,9 @@ }, "template": { "type": "string" + }, + "version": { + "type": "string" } } }, @@ -5888,6 +5909,9 @@ "properties": { "template": { "type": "string" + }, + "version": { + "type": "string" } } }, diff --git a/pkg/services/ngalert/provisioning/errors.go b/pkg/services/ngalert/provisioning/errors.go index e0e7a6097d1..ae7cd9290e9 100644 --- a/pkg/services/ngalert/provisioning/errors.go +++ b/pkg/services/ngalert/provisioning/errors.go @@ -18,6 +18,8 @@ var ( 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.")) ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used").MustTemplate("Time interval is used") + ErrTemplateNotFound = errutil.NotFound("alerting.notifications.templates.notFound") + ErrContactPointReferenced = errutil.Conflict("alerting.notifications.contact-points.referenced", errutil.WithPublicMessage("Contact point is currently referenced by a notification policy.")) ErrContactPointUsedInRule = errutil.Conflict("alerting.notifications.contact-points.used-by-rule", errutil.WithPublicMessage("Contact point is currently used in the notification settings of one or many alert rules.")) ) diff --git a/pkg/services/ngalert/provisioning/templates.go b/pkg/services/ngalert/provisioning/templates.go index 71f4632b32b..300b20e4c49 100644 --- a/pkg/services/ngalert/provisioning/templates.go +++ b/pkg/services/ngalert/provisioning/templates.go @@ -3,6 +3,8 @@ package provisioning import ( "context" "fmt" + "hash/fnv" + "unsafe" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -37,8 +39,9 @@ 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{ - Name: name, - Template: tmpl, + Name: name, + Template: tmpl, + ResourceVersion: calculateTemplateFingerprint(tmpl), } provenance, err := t.provenanceStore.GetProvenance(ctx, &tmpl, orgID) @@ -80,6 +83,16 @@ func (t *TemplateService) SetTemplate(ctx context.Context, orgID int64, tmpl def } } + existing, ok := revision.Config.TemplateFiles[tmpl.Name] + if ok { + err = t.checkOptimisticConcurrency(tmpl.Name, existing, models.Provenance(tmpl.Provenance), tmpl.ResourceVersion, "update") + if err != nil { + return definitions.NotificationTemplate{}, err + } + } else if tmpl.ResourceVersion != "" { // if version is set then it's an update operation. Fail because resource does not exist anymore + return definitions.NotificationTemplate{}, ErrTemplateNotFound.Errorf("") + } + revision.Config.TemplateFiles[tmpl.Name] = tmpl.Template err = t.xact.InTransaction(ctx, func(ctx context.Context) error { @@ -92,10 +105,15 @@ func (t *TemplateService) SetTemplate(ctx context.Context, orgID int64, tmpl def return definitions.NotificationTemplate{}, err } - return tmpl, nil + return definitions.NotificationTemplate{ + Name: tmpl.Name, + Template: tmpl.Template, + Provenance: tmpl.Provenance, + ResourceVersion: calculateTemplateFingerprint(tmpl.Template), + }, nil } -func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance) error { +func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name string, provenance definitions.Provenance, version string) error { revision, err := t.configStore.Get(ctx, orgID) if err != nil { return err @@ -105,11 +123,16 @@ func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name return nil } - _, ok := revision.Config.TemplateFiles[name] + existing, ok := revision.Config.TemplateFiles[name] if !ok { return nil } + err = t.checkOptimisticConcurrency(name, 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) if err != nil { @@ -131,3 +154,24 @@ func (t *TemplateService) DeleteTemplate(ctx context.Context, orgID int64, name return t.provenanceStore.DeleteProvenance(ctx, &tgt, orgID) }) } + +func (t *TemplateService) checkOptimisticConcurrency(name, currentContent string, 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 + t.log.Debug("ignoring optimistic concurrency check because version was not provided", "template", name, "operation", action) + } + return nil + } + currentVersion := calculateTemplateFingerprint(currentContent) + if currentVersion != desiredVersion { + return ErrVersionConflict.Errorf("provided version %s of template %s does not match current version %s", desiredVersion, name, currentVersion) + } + return nil +} + +func calculateTemplateFingerprint(t string) string { + sum := fnv.New64() + _, _ = sum.Write(unsafe.Slice(unsafe.StringData(t), len(t))) //nolint:gosec + return fmt.Sprintf("%016x", sum.Sum64()) +} diff --git a/pkg/services/ngalert/provisioning/templates_test.go b/pkg/services/ngalert/provisioning/templates_test.go index aeb8d813a61..35682353b23 100644 --- a/pkg/services/ngalert/provisioning/templates_test.go +++ b/pkg/services/ngalert/provisioning/templates_test.go @@ -127,6 +127,45 @@ func TestTemplateService(t *testing.T) { require.ErrorIs(t, err, expectedErr) }) + t.Run("rejects existing templates if version 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.ProvenanceNone, nil) + + template := definitions.NotificationTemplate{ + Name: "a", + Template: "asdf-new", + ResourceVersion: "bad-version", + Provenance: definitions.Provenance(models.ProvenanceNone), + } + + _, err := sut.SetTemplate(context.Background(), 1, template) + + require.ErrorIs(t, err, ErrVersionConflict) + }) + + t.Run("rejects new template if version is set", func(t *testing.T) { + mockStore := &legacy_storage.MockAMConfigStore{} + sut := createTemplateServiceSut(legacy_storage.NewAlertmanagerConfigStore(mockStore)) + tmpl := createNotificationTemplate() + tmpl.ResourceVersion = "test" + mockStore.EXPECT(). + GetsConfig(models.AlertConfiguration{ + 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) + + require.ErrorIs(t, err, ErrTemplateNotFound) + }) + t.Run("propagates errors", func(t *testing.T) { t.Run("when unable to read config", func(t *testing.T) { mockStore := &legacy_storage.MockAMConfigStore{} @@ -336,7 +375,7 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "template", definitions.Provenance(models.ProvenanceAPI), "") require.Error(t, err) }) @@ -350,7 +389,7 @@ func TestTemplateService(t *testing.T) { }) sut.provenanceStore.(*MockProvisioningStore).EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil) - err := sut.DeleteTemplate(context.Background(), 1, "template", definitions.Provenance(models.ProvenanceAPI)) + 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()) }) @@ -363,7 +402,7 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + 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()) }) @@ -381,7 +420,7 @@ func TestTemplateService(t *testing.T) { DeleteProvenance(mock.Anything, mock.Anything, mock.Anything). Return(fmt.Errorf("failed to save provenance")) - err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI), "") require.ErrorContains(t, err, "failed to save provenance") }) @@ -399,7 +438,7 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI), "") require.ErrorContains(t, err, "failed to save config") }) @@ -416,7 +455,23 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI), "") + + require.NoError(t, err) + }) + + t.Run("deletes template from config file on success ignoring optimistic concurrency", func(t *testing.T) { + mockStore := &legacy_storage.MockAMConfigStore{} + sut := createTemplateServiceSut(legacy_storage.NewAlertmanagerConfigStore(mockStore)) + mockStore.EXPECT(). + GetsConfig(models.AlertConfiguration{ + 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", definitions.Provenance(models.ProvenanceAPI), "b26e328af4bb9aaf") require.NoError(t, err) }) @@ -432,7 +487,7 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "does not exist", definitions.Provenance(models.ProvenanceAPI), "") require.NoError(t, err) }) @@ -448,7 +503,7 @@ func TestTemplateService(t *testing.T) { 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", definitions.Provenance(models.ProvenanceAPI)) + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceAPI), "") require.NoError(t, err) }) @@ -469,10 +524,24 @@ func TestTemplateService(t *testing.T) { return expectedErr } - err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceNone)) + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceNone), "") require.ErrorIs(t, err, expectedErr) }) + + t.Run("errors if version 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.ProvenanceNone, nil) + + err := sut.DeleteTemplate(context.Background(), 1, "a", definitions.Provenance(models.ProvenanceNone), "bad-version") + + require.ErrorIs(t, err, ErrVersionConflict) + }) }) } diff --git a/pkg/services/provisioning/alerting/text_templates.go b/pkg/services/provisioning/alerting/text_templates.go index 0a567f863ca..e64ccdd5959 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, definitions.Provenance(models.ProvenanceFile)) + err := c.templateService.DeleteTemplate(ctx, deleteTemplate.OrgID, deleteTemplate.Name, definitions.Provenance(models.ProvenanceFile), "") if err != nil { return err } diff --git a/public/api-merged.json b/public/api-merged.json index 577d6ed5f3e..5ba6d49f4ec 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -11796,6 +11796,12 @@ "schema": { "$ref": "#/definitions/ValidationError" } + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } }, @@ -11808,15 +11814,27 @@ "parameters": [ { "type": "string", - "description": "Template Name", + "description": "Template name", "name": "name", "in": "path", "required": true + }, + { + "type": "string", + "description": "Version of template to use for optimistic concurrency. Leave empty to disable validation", + "name": "version", + "in": "query" } ], "responses": { "204": { "description": " The template was deleted successfully." + }, + "409": { + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } } @@ -17237,6 +17255,9 @@ }, "template": { "type": "string" + }, + "version": { + "type": "string" } } }, @@ -17245,6 +17266,9 @@ "properties": { "template": { "type": "string" + }, + "version": { + "type": "string" } } }, diff --git a/public/openapi3.json b/public/openapi3.json index 927e7fed22a..72c4b53bcd3 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -7314,6 +7314,9 @@ }, "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -7322,6 +7325,9 @@ "properties": { "template": { "type": "string" + }, + "version": { + "type": "string" } }, "type": "object" @@ -25843,18 +25849,36 @@ "operationId": "RouteDeleteTemplate", "parameters": [ { - "description": "Template Name", + "description": "Template name", "in": "path", "name": "name", "required": true, "schema": { "type": "string" } + }, + { + "description": "Version of template to use for optimistic concurrency. Leave empty to disable validation", + "in": "query", + "name": "version", + "schema": { + "type": "string" + } } ], "responses": { "204": { "description": " The template was deleted successfully." + }, + "409": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GenericPublicError" + } + } + }, + "description": "GenericPublicError" } }, "summary": "Delete a template.", @@ -25945,6 +25969,16 @@ } }, "description": "ValidationError" + }, + "409": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GenericPublicError" + } + } + }, + "description": "GenericPublicError" } }, "summary": "Updates an existing notification template.",