From db33df504160259c515a421c3b61ae048ffc1ed5 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Tue, 13 Aug 2024 10:59:19 -0400 Subject: [PATCH] Alerting: Template API to return errutil errors (#91821) --- .../shared/alerts/alerting_provisioning.md | 21 ++++++---------- pkg/services/ngalert/api/api_provisioning.go | 9 +++---- pkg/services/ngalert/api/tooling/api.json | 21 ++++++++++------ .../definitions/provisioning_templates.go | 5 ++-- pkg/services/ngalert/api/tooling/post.json | 24 +++++++++++------- pkg/services/ngalert/api/tooling/spec.json | 24 +++++++++++------- pkg/services/ngalert/provisioning/errors.go | 13 ++++++++++ .../ngalert/provisioning/templates.go | 2 +- .../ngalert/provisioning/templates_test.go | 4 +-- public/api-merged.json | 21 ++++++++++------ public/openapi3.json | 25 +++++++++++++------ 11 files changed, 104 insertions(+), 65 deletions(-) diff --git a/docs/sources/shared/alerts/alerting_provisioning.md b/docs/sources/shared/alerts/alerting_provisioning.md index baa017bb57b..4fef1da7e57 100644 --- a/docs/sources/shared/alerts/alerting_provisioning.md +++ b/docs/sources/shared/alerts/alerting_provisioning.md @@ -1060,7 +1060,7 @@ GET /api/v1/provisioning/templates/:name | Code | Status | Description | Has headers | Schema | | ------------------------------ | --------- | -------------------- | :---------: | ---------------------------------------- | | [200](#route-get-template-200) | OK | NotificationTemplate | | [schema](#route-get-template-200-schema) | -| [404](#route-get-template-404) | Not Found | Not found. | | [schema](#route-get-template-404-schema) | +| [404](#route-get-template-404) | Not Found | GenericPublicError | | [schema](#route-get-template-404-schema) | #### Responses @@ -1074,7 +1074,7 @@ Status: OK ##### 404 - Not found. -Status: Not Found +[GenericPublicError](#generic-public-error) ###### Schema @@ -1086,10 +1086,9 @@ GET /api/v1/provisioning/templates #### All responses -| Code | Status | Description | Has headers | Schema | -| ------------------------------- | --------- | --------------------- | :---------: | ----------------------------------------- | -| [200](#route-get-templates-200) | OK | NotificationTemplates | | [schema](#route-get-templates-200-schema) | -| [404](#route-get-templates-404) | Not Found | Not found. | | [schema](#route-get-templates-404-schema) | +| Code | Status | Description | Has headers | Schema | +| ------------------------------- | ------ | --------------------- | :---------: | ----------------------------------------- | +| [200](#route-get-templates-200) | OK | NotificationTemplates | | [schema](#route-get-templates-200-schema) | #### Responses @@ -1101,12 +1100,6 @@ Status: OK [NotificationTemplates](#notification-templates) -##### 404 - Not found. - -Status: Not Found - -###### Schema - ### Create a new alert rule. (_RoutePostAlertRule_) ``` @@ -1480,7 +1473,7 @@ PUT /api/v1/provisioning/templates/:name | Code | Status | Description | Has headers | Schema | | ------------------------------ | ----------- | -------------------- | :---------: | ---------------------------------------- | | [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) | +| [400](#route-put-template-400) | Bad Request | GenericPublicError | | [schema](#route-put-template-400-schema) | | [409](#route-put-template-409) | Conflict | GenericPublicError | | [schema](#route-put-template-409-schema) | #### Responses @@ -1499,7 +1492,7 @@ Status: Bad Request ###### Schema -[ValidationError](#validation-error) +[GenericPublicError](#generic-public-error) ##### 409 - Conflict diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index 399f3f49b29..ed645537d34 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -201,7 +201,7 @@ func (srv *ProvisioningSrv) RouteDeleteContactPoint(c *contextmodel.ReqContext, func (srv *ProvisioningSrv) RouteGetTemplates(c *contextmodel.ReqContext) response.Response { templates, err := srv.templates.GetTemplates(c.Req.Context(), c.SignedInUser.GetOrgID()) if err != nil { - return ErrResp(http.StatusInternalServerError, err, "") + return response.ErrOrFallback(http.StatusInternalServerError, "", err) } return response.JSON(http.StatusOK, templates) } @@ -209,14 +209,14 @@ func (srv *ProvisioningSrv) RouteGetTemplates(c *contextmodel.ReqContext) respon func (srv *ProvisioningSrv) RouteGetTemplate(c *contextmodel.ReqContext, name string) response.Response { templates, err := srv.templates.GetTemplates(c.Req.Context(), c.SignedInUser.GetOrgID()) if err != nil { - return ErrResp(http.StatusInternalServerError, err, "") + return response.ErrOrFallback(http.StatusInternalServerError, "", err) } for _, tmpl := range templates { if tmpl.Name == name { return response.JSON(http.StatusOK, tmpl) } } - return response.Empty(http.StatusNotFound) + return response.Err(provisioning.ErrTemplateNotFound) } func (srv *ProvisioningSrv) RoutePutTemplate(c *contextmodel.ReqContext, body definitions.NotificationTemplateContent, name string) response.Response { @@ -228,9 +228,6 @@ func (srv *ProvisioningSrv) RoutePutTemplate(c *contextmodel.ReqContext, body de } 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 response.ErrOrFallback(http.StatusInternalServerError, "", err) } return response.JSON(http.StatusAccepted, modified) diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index 267ccb03a16..b0f8e6b32fb 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -1906,7 +1906,7 @@ "type": "array" } }, - "title": "Headers represents the configuration for HTTP headers.", + "title": "Header represents the configuration for a single HTTP header.", "type": "object" }, "Headers": { @@ -3403,6 +3403,12 @@ "Route": { "description": "A Route is a node that contains definitions of how to handle alerts. This is modified\nfrom the upstream alertmanager in that it adds the ObjectMatchers property.", "properties": { + "active_time_intervals": { + "items": { + "type": "string" + }, + "type": "array" + }, "continue": { "type": "boolean" }, @@ -4575,6 +4581,7 @@ "type": "object" }, "alertGroups": { + "description": "AlertGroups alert groups", "items": { "$ref": "#/definitions/alertGroup", "type": "object" @@ -6173,9 +6180,6 @@ "schema": { "$ref": "#/definitions/NotificationTemplates" } - }, - "404": { - "description": " Not found." } }, "summary": "Get all notification templates.", @@ -6237,7 +6241,10 @@ } }, "404": { - "description": " Not found." + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Get a notification template.", @@ -6279,9 +6286,9 @@ } }, "400": { - "description": "ValidationError", + "description": "GenericPublicError", "schema": { - "$ref": "#/definitions/ValidationError" + "$ref": "#/definitions/GenericPublicError" } }, "409": { diff --git a/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go b/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go index 41a1722cf47..b854a1132c6 100644 --- a/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go +++ b/pkg/services/ngalert/api/tooling/definitions/provisioning_templates.go @@ -6,7 +6,6 @@ package definitions // // Responses: // 200: NotificationTemplates -// 404: description: Not found. // swagger:route GET /v1/provisioning/templates/{name} provisioning stable RouteGetTemplate // @@ -14,7 +13,7 @@ package definitions // // Responses: // 200: NotificationTemplate -// 404: description: Not found. +// 404: GenericPublicError // swagger:route PUT /v1/provisioning/templates/{name} provisioning stable RoutePutTemplate // @@ -25,7 +24,7 @@ package definitions // // Responses: // 202: NotificationTemplate -// 400: ValidationError +// 400: GenericPublicError // 409: GenericPublicError // swagger:route DELETE /v1/provisioning/templates/{name} provisioning stable RouteDeleteTemplate diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index ca0afdf9372..91d33d799c2 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -1906,7 +1906,7 @@ "type": "array" } }, - "title": "Headers represents the configuration for HTTP headers.", + "title": "Header represents the configuration for a single HTTP header.", "type": "object" }, "Headers": { @@ -3403,6 +3403,12 @@ "Route": { "description": "A Route is a node that contains definitions of how to handle alerts. This is modified\nfrom the upstream alertmanager in that it adds the ObjectMatchers property.", "properties": { + "active_time_intervals": { + "items": { + "type": "string" + }, + "type": "array" + }, "continue": { "type": "boolean" }, @@ -4310,6 +4316,7 @@ "type": "object" }, "URL": { + "description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nThe Host field contains the host and port subcomponents of the URL.\nWhen the port is present, it is separated from the host with a colon.\nWhen the host is an IPv6 address, it must be enclosed in square brackets:\n\"[fe80::1]:80\". The [net.JoinHostPort] function combines a host and port\ninto a string suitable for the Host field, adding square brackets to\nthe host when necessary.\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use the [URL.EscapedPath] method, which preserves\nthe original encoding of Path.\n\nThe RawPath field is an optional field which is only set when the default\nencoding of Path is different from the escaped path. See the EscapedPath method\nfor more details.\n\nURL's String method uses the EscapedPath method to obtain the path.", "properties": { "ForceQuery": { "type": "boolean" @@ -4345,7 +4352,7 @@ "$ref": "#/definitions/Userinfo" } }, - "title": "URL is a custom URL type that allows validation at configuration load time.", + "title": "A URL represents a parsed URL (technically, a URI reference).", "type": "object" }, "UpdateRuleGroupResponse": { @@ -4575,7 +4582,6 @@ "type": "object" }, "alertGroups": { - "description": "AlertGroups alert groups", "items": { "$ref": "#/definitions/alertGroup", "type": "object" @@ -8385,9 +8391,6 @@ "schema": { "$ref": "#/definitions/NotificationTemplates" } - }, - "404": { - "description": " Not found." } }, "summary": "Get all notification templates.", @@ -8449,7 +8452,10 @@ } }, "404": { - "description": " Not found." + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } }, "summary": "Get a notification template.", @@ -8491,9 +8497,9 @@ } }, "400": { - "description": "ValidationError", + "description": "GenericPublicError", "schema": { - "$ref": "#/definitions/ValidationError" + "$ref": "#/definitions/GenericPublicError" } }, "409": { diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index f8fd5e31ecb..45a9fe74bb5 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -3314,9 +3314,6 @@ "schema": { "$ref": "#/definitions/NotificationTemplates" } - }, - "404": { - "description": " Not found." } } } @@ -3346,7 +3343,10 @@ } }, "404": { - "description": " Not found." + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } }, @@ -3389,9 +3389,9 @@ } }, "400": { - "description": "ValidationError", + "description": "GenericPublicError", "schema": { - "$ref": "#/definitions/ValidationError" + "$ref": "#/definitions/GenericPublicError" } }, "409": { @@ -5519,7 +5519,7 @@ }, "Header": { "type": "object", - "title": "Headers represents the configuration for HTTP headers.", + "title": "Header represents the configuration for a single HTTP header.", "properties": { "files": { "type": "array", @@ -7037,6 +7037,12 @@ "description": "A Route is a node that contains definitions of how to handle alerts. This is modified\nfrom the upstream alertmanager in that it adds the ObjectMatchers property.", "type": "object", "properties": { + "active_time_intervals": { + "type": "array", + "items": { + "type": "string" + } + }, "continue": { "type": "boolean" }, @@ -7943,8 +7949,9 @@ } }, "URL": { + "description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nThe Host field contains the host and port subcomponents of the URL.\nWhen the port is present, it is separated from the host with a colon.\nWhen the host is an IPv6 address, it must be enclosed in square brackets:\n\"[fe80::1]:80\". The [net.JoinHostPort] function combines a host and port\ninto a string suitable for the Host field, adding square brackets to\nthe host when necessary.\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use the [URL.EscapedPath] method, which preserves\nthe original encoding of Path.\n\nThe RawPath field is an optional field which is only set when the default\nencoding of Path is different from the escaped path. See the EscapedPath method\nfor more details.\n\nURL's String method uses the EscapedPath method to obtain the path.", "type": "object", - "title": "URL is a custom URL type that allows validation at configuration load time.", + "title": "A URL represents a parsed URL (technically, a URI reference).", "properties": { "ForceQuery": { "type": "boolean" @@ -8208,7 +8215,6 @@ } }, "alertGroups": { - "description": "AlertGroups alert groups", "type": "array", "items": { "type": "object", diff --git a/pkg/services/ngalert/provisioning/errors.go b/pkg/services/ngalert/provisioning/errors.go index ae7cd9290e9..101a11c3555 100644 --- a/pkg/services/ngalert/provisioning/errors.go +++ b/pkg/services/ngalert/provisioning/errors.go @@ -19,6 +19,7 @@ var ( ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used").MustTemplate("Time interval is used") ErrTemplateNotFound = errutil.NotFound("alerting.notifications.templates.notFound") + ErrTemplateInvalid = errutil.BadRequest("alerting.notifications.templates.invalidFormat").MustTemplate("Invalid format of the submitted template", errutil.WithPublic("Template is in invalid format. Correct the payload and try again.")) 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.")) @@ -54,3 +55,15 @@ func MakeErrTimeIntervalInUse(usedByRoutes bool, rules []models.AlertRuleKey) er Error: nil, }) } + +// MakeErrTimeIntervalInvalid creates an error with the ErrTimeIntervalInvalid template +func MakeErrTemplateInvalid(err error) error { + data := errutil.TemplateData{ + Public: map[string]interface{}{ + "Error": err.Error(), + }, + Error: err, + } + + return ErrTemplateInvalid.Build(data) +} diff --git a/pkg/services/ngalert/provisioning/templates.go b/pkg/services/ngalert/provisioning/templates.go index 300b20e4c49..06c878e4742 100644 --- a/pkg/services/ngalert/provisioning/templates.go +++ b/pkg/services/ngalert/provisioning/templates.go @@ -59,7 +59,7 @@ func (t *TemplateService) GetTemplates(ctx context.Context, orgID int64) ([]defi func (t *TemplateService) SetTemplate(ctx context.Context, orgID int64, tmpl definitions.NotificationTemplate) (definitions.NotificationTemplate, error) { err := tmpl.Validate() if err != nil { - return definitions.NotificationTemplate{}, fmt.Errorf("%w: %s", ErrValidation, err.Error()) + return definitions.NotificationTemplate{}, MakeErrTemplateInvalid(err) } revision, err := t.configStore.Get(ctx, orgID) diff --git a/pkg/services/ngalert/provisioning/templates_test.go b/pkg/services/ngalert/provisioning/templates_test.go index 35682353b23..82123fea5b7 100644 --- a/pkg/services/ngalert/provisioning/templates_test.go +++ b/pkg/services/ngalert/provisioning/templates_test.go @@ -98,7 +98,7 @@ func TestTemplateService(t *testing.T) { _, err := sut.SetTemplate(context.Background(), 1, tmpl) - require.ErrorIs(t, err, ErrValidation) + require.ErrorIs(t, err, ErrTemplateInvalid) }) t.Run("rejects existing templates if provenance is not right", func(t *testing.T) { @@ -341,7 +341,7 @@ func TestTemplateService(t *testing.T) { _, err := sut.SetTemplate(context.Background(), 1, tmpl) - require.ErrorIs(t, err, ErrValidation) + require.ErrorIs(t, err, ErrTemplateInvalid) }) t.Run("does not reject template with unknown field", func(t *testing.T) { diff --git a/public/api-merged.json b/public/api-merged.json index 5ba6d49f4ec..bc7800c62b9 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -11719,9 +11719,6 @@ "schema": { "$ref": "#/definitions/NotificationTemplates" } - }, - "404": { - "description": " Not found." } } } @@ -11750,7 +11747,10 @@ } }, "404": { - "description": " Not found." + "description": "GenericPublicError", + "schema": { + "$ref": "#/definitions/GenericPublicError" + } } } }, @@ -11792,9 +11792,9 @@ } }, "400": { - "description": "ValidationError", + "description": "GenericPublicError", "schema": { - "$ref": "#/definitions/ValidationError" + "$ref": "#/definitions/GenericPublicError" } }, "409": { @@ -16225,7 +16225,7 @@ }, "Header": { "type": "object", - "title": "Headers represents the configuration for HTTP headers.", + "title": "Header represents the configuration for a single HTTP header.", "properties": { "files": { "type": "array", @@ -19500,6 +19500,12 @@ "description": "A Route is a node that contains definitions of how to handle alerts. This is modified\nfrom the upstream alertmanager in that it adds the ObjectMatchers property.", "type": "object", "properties": { + "active_time_intervals": { + "type": "array", + "items": { + "type": "string" + } + }, "continue": { "type": "boolean" }, @@ -22163,6 +22169,7 @@ } }, "alertGroups": { + "description": "AlertGroups alert groups", "type": "array", "items": { "type": "object", diff --git a/public/openapi3.json b/public/openapi3.json index 72c4b53bcd3..f703e5e0bbb 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -6304,7 +6304,7 @@ "type": "array" } }, - "title": "Headers represents the configuration for HTTP headers.", + "title": "Header represents the configuration for a single HTTP header.", "type": "object" }, "Headers": { @@ -9559,6 +9559,12 @@ "Route": { "description": "A Route is a node that contains definitions of how to handle alerts. This is modified\nfrom the upstream alertmanager in that it adds the ObjectMatchers property.", "properties": { + "active_time_intervals": { + "items": { + "type": "string" + }, + "type": "array" + }, "continue": { "type": "boolean" }, @@ -12222,6 +12228,7 @@ "type": "object" }, "alertGroups": { + "description": "AlertGroups alert groups", "items": { "$ref": "#/components/schemas/alertGroup" }, @@ -25833,9 +25840,6 @@ } }, "description": "NotificationTemplates" - }, - "404": { - "description": " Not found." } }, "summary": "Get all notification templates.", @@ -25911,7 +25915,14 @@ "description": "NotificationTemplate" }, "404": { - "description": " Not found." + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GenericPublicError" + } + } + }, + "description": "GenericPublicError" } }, "summary": "Get a notification template.", @@ -25964,11 +25975,11 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ValidationError" + "$ref": "#/components/schemas/GenericPublicError" } } }, - "description": "ValidationError" + "description": "GenericPublicError" }, "409": { "content": {