apitech
/
grafana
mirror of https://github.com/grafana/grafana
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Alerting: recognize Cortex datasources correctly in the frontend (#44316)

Browse Source 
* Alerting: always use msg field for user facing errors

* fix: revert front-end Cortex detection

Co-authored-by: gillesdemey <gilles.de.mey@gmail.com>
pull/44217/head^2
Jean-Philippe Quéméner 3 years ago committed by GitHub
parent e1af78954f
commit 8ee3f59cd4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 38 additions and 34 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      pkg/services/ngalert/api/util.go
  2. 2
      pkg/tests/api/alerting/api_admin_configuration_test.go
  3. 4
      pkg/tests/api/alerting/api_alertmanager_configuration_test.go
  4. 38
      pkg/tests/api/alerting/api_alertmanager_test.go
  5. 6
      pkg/tests/api/alerting/api_prometheus_test.go
  6. 6
      pkg/tests/api/alerting/api_ruler_test.go
  7. 14
      public/app/features/alerting/unified/api/ruler.ts

2
pkg/services/ngalert/api/util.go
Unescape View File

@ -265,7 +265,7 @@ func ErrResp(status int, err error, msg string, args ...interface{}) *response.N
if msg != "" {
err = errors.WithMessagef(err, msg, args...)
}
return response.Error(status, "API error", err)
return response.Error(status, err.Error(), err)
}
// accessForbiddenResp creates a response of forbidden access.

2
pkg/tests/api/alerting/api_admin_configuration_test.go
Unescape View File

@ -67,7 +67,7 @@ func TestAdminConfiguration_SendingToExternalAlertmanagers(t *testing.T) {
resp := getRequest(t, alertsURL, http.StatusNotFound) // nolint
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.JSONEq(t, string(b), "{\"message\": \"API error\",\"error\": \"no admin configuration available\"}")
require.JSONEq(t, string(b), "{\"message\": \"no admin configuration available\",\"error\": \"no admin configuration available\"}")
}
// Now, lets re-set external Alertmanagers for main organisation.

4
pkg/tests/api/alerting/api_alertmanager_configuration_test.go
Unescape View File

@ -92,7 +92,7 @@ func TestAlertmanagerConfigurationIsTransactional(t *testing.T) {
}
`
resp := postRequest(t, alertConfigURL, payload, http.StatusBadRequest) // nolint
require.JSONEq(t, `{"message": "API error","error":"failed to save and apply Alertmanager configuration: failed to build integration map: the receiver is invalid: failed to validate receiver \"slack.receiver\" of type \"slack\": token must be specified when using the Slack chat API"}`, getBody(t, resp.Body))
require.JSONEq(t, `{"message": "failed to save and apply Alertmanager configuration: failed to build integration map: the receiver is invalid: failed to validate receiver \"slack.receiver\" of type \"slack\": token must be specified when using the Slack chat API","error":"failed to save and apply Alertmanager configuration: failed to build integration map: the receiver is invalid: failed to validate receiver \"slack.receiver\" of type \"slack\": token must be specified when using the Slack chat API"}`, getBody(t, resp.Body))
resp = getRequest(t, alertConfigURL, http.StatusOK) // nolint
require.JSONEq(t, defaultAlertmanagerConfigJSON, getBody(t, resp.Body))
@ -215,7 +215,7 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
`
resp := postRequest(t, alertConfigURL, payload, http.StatusBadRequest) // nolint
require.JSONEq(t, `{"message": "API error","error": "unknown receiver: invalid"}`, getBody(t, resp.Body))
require.JSONEq(t, `{"message": "unknown receiver: invalid","error": "unknown receiver: invalid"}`, getBody(t, resp.Body))
}
// The secure settings must be present

38
pkg/tests/api/alerting/api_alertmanager_test.go
Unescape View File

@ -100,7 +100,7 @@ func TestAMConfigAccess(t *testing.T) {
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/alertmanager/grafana/config/api/v1/alerts",
expStatus: http.StatusForbidden,
expBody: `{"message": "API error","error": "permission denied"}`,
expBody: `{"message": "permission denied","error": "permission denied"}`,
},
{
desc: "editor request should succeed",
@ -170,7 +170,7 @@ func TestAMConfigAccess(t *testing.T) {
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/alertmanager/grafana/config/api/v1/alerts",
expStatus: http.StatusForbidden,
expBody: `{"message": "API error","error": "permission denied"}`,
expBody: `{"message": "permission denied","error": "permission denied"}`,
},
{
desc: "editor request should succeed",
@ -233,7 +233,7 @@ func TestAMConfigAccess(t *testing.T) {
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/alertmanager/grafana/api/v2/silences",
expStatus: http.StatusForbidden,
expBody: `{"message": "API error","error": "permission denied"}`,
expBody: `{"message": "permission denied","error": "permission denied"}`,
},
{
desc: "editor request should succeed",
@ -339,7 +339,7 @@ func TestAMConfigAccess(t *testing.T) {
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/alertmanager/grafana/api/v2/silence/%s",
expStatus: http.StatusForbidden,
expBody: `{"message": "API error","error": "permission denied"}`,
expBody: `{"message": "permission denied","error": "permission denied"}`,
},
{
desc: "editor request should succeed",
@ -612,7 +612,7 @@ func TestRulerAccess(t *testing.T) {
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/ruler/grafana/api/v1/rules/default",
expStatus: http.StatusForbidden,
expectedResponse: `{"message": "API error","error":"user does not have permissions to edit the namespace: user does not have permissions to edit the namespace"}`,
expectedResponse: `{"message": "user does not have permissions to edit the namespace: user does not have permissions to edit the namespace","error":"user does not have permissions to edit the namespace: user does not have permissions to edit the namespace"}`,
},
{
desc: "editor request should succeed",
@ -903,7 +903,7 @@ func TestAlertRuleCRUD(t *testing.T) {
Data: []ngmodels.AlertQuery{},
},
},
expectedResponse: `{"message": "API error","error":"failed to update rule group: invalid alert rule: no queries or expressions are found"}`,
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: no queries or expressions are found","error":"failed to update rule group: invalid alert rule: no queries or expressions are found"}`,
},
{
desc: "alert rule with empty title",
@ -933,7 +933,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to update rule group: invalid alert rule: title is empty"}`,
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: title is empty","error":"failed to update rule group: invalid alert rule: title is empty"}`,
},
{
desc: "alert rule with too long name",
@ -963,7 +963,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to update rule group: invalid alert rule: name length should not be greater than 190"}`,
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: name length should not be greater than 190","error":"failed to update rule group: invalid alert rule: name length should not be greater than 190"}`,
},
{
desc: "alert rule with too long rulegroup",
@ -993,7 +993,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to update rule group: invalid alert rule: rule group name length should not be greater than 190"}`,
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: rule group name length should not be greater than 190","error":"failed to update rule group: invalid alert rule: rule group name length should not be greater than 190"}`,
},
{
desc: "alert rule with invalid interval",
@ -1024,7 +1024,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to update rule group: invalid alert rule: interval (1s) should be non-zero and divided exactly by scheduler interval: 10s"}`,
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: interval (1s) should be non-zero and divided exactly by scheduler interval: 10s","error":"failed to update rule group: invalid alert rule: interval (1s) should be non-zero and divided exactly by scheduler interval: 10s"}`,
},
{
desc: "alert rule with unknown datasource",
@ -1054,7 +1054,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to validate alert rule \"AlwaysFiring\": invalid query A: data source not found: unknown"}`,
expectedResponse: `{"message": "failed to validate alert rule \"AlwaysFiring\": invalid query A: data source not found: unknown","error":"failed to validate alert rule \"AlwaysFiring\": invalid query A: data source not found: unknown"}`,
},
{
desc: "alert rule with invalid condition",
@ -1084,7 +1084,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "API error","error":"failed to validate alert rule \"AlwaysFiring\": condition B not found in any query or expression: it should be one of: [A]"}`,
expectedResponse: `{"message": "failed to validate alert rule \"AlwaysFiring\": condition B not found in any query or expression: it should be one of: [A]","error":"failed to validate alert rule \"AlwaysFiring\": condition B not found in any query or expression: it should be one of: [A]"}`,
},
}
@ -1374,7 +1374,7 @@ func TestAlertRuleCRUD(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
require.JSONEq(t, `{"message": "API error","error":"failed to update rule group: failed to get alert rule unknown: could not find alert rule"}`, string(b))
require.JSONEq(t, `{"message": "failed to update rule group: failed to get alert rule unknown: could not find alert rule","error":"failed to update rule group: failed to get alert rule unknown: could not find alert rule"}`, string(b))
// let's make sure that rule definitions are not affected by the failed POST request.
u = fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
@ -1493,7 +1493,7 @@ func TestAlertRuleCRUD(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.JSONEq(t, fmt.Sprintf(`{"message": "API error","error":"failed to validate alert rule \"AlwaysAlerting\": conflicting UID \"%s\" found"}`, ruleUID), string(b))
require.JSONEq(t, fmt.Sprintf(`{"message": "failed to validate alert rule \"AlwaysAlerting\": conflicting UID \"%s\" found","error":"failed to validate alert rule \"AlwaysAlerting\": conflicting UID \"%s\" found"}`, ruleUID, ruleUID), string(b))
// let's make sure that rule definitions are not affected by the failed POST request.
u = fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
@ -1898,7 +1898,7 @@ func TestAlertRuleCRUD(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusNotFound, resp.StatusCode)
require.JSONEq(t, `{"message": "API error","error":"failed to delete rule group: rule group not found under this namespace"}`, string(b))
require.JSONEq(t, `{"message": "failed to delete rule group: rule group not found under this namespace","error":"failed to delete rule group: rule group not found under this namespace"}`, string(b))
})
t.Run("succeed if the rule group name does exist", func(t *testing.T) {
@ -2108,7 +2108,7 @@ func TestQuota(t *testing.T) {
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
assert.Equal(t, http.StatusForbidden, resp.StatusCode)
require.JSONEq(t, `{"message": "API error","error":"quota reached"}`, string(b))
require.JSONEq(t, `{"message": "quota reached","error":"quota reached"}`, string(b))
})
t.Run("when quota limit exceed updating existing rule should succeed", func(t *testing.T) {
@ -2405,7 +2405,7 @@ func TestEval(t *testing.T) {
}
`,
expectedStatusCode: http.StatusBadRequest,
expectedResponse: `{"message": "API error","error":"invalid condition: condition B not found in any query or expression: it should be one of: [A]"}`,
expectedResponse: `{"message": "invalid condition: condition B not found in any query or expression: it should be one of: [A]","error":"invalid condition: condition B not found in any query or expression: it should be one of: [A]"}`,
},
{
desc: "unknown query datasource",
@ -2430,7 +2430,7 @@ func TestEval(t *testing.T) {
}
`,
expectedStatusCode: http.StatusBadRequest,
expectedResponse: `{"message": "API error","error":"invalid condition: invalid query A: data source not found: unknown"}`,
expectedResponse: `{"message": "invalid condition: invalid query A: data source not found: unknown","error":"invalid condition: invalid query A: data source not found: unknown"}`,
},
}
@ -2586,7 +2586,7 @@ func TestEval(t *testing.T) {
}
`,
expectedStatusCode: http.StatusBadRequest,
expectedResponse: `{"message": "API error","error":"invalid queries or expressions: invalid query A: data source not found: unknown"}`,
expectedResponse: `{"message": "invalid queries or expressions: invalid query A: data source not found: unknown","error":"invalid queries or expressions: invalid query A: data source not found: unknown"}`,
},
}

6
pkg/tests/api/alerting/api_prometheus_test.go
Unescape View File

@ -206,7 +206,7 @@ func TestPrometheusRules(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, 400, resp.StatusCode)
require.JSONEq(t, `{"message": "API error","error":"failed to update rule group: invalid alert rule: cannot have Panel ID without a Dashboard UID"}`, string(b))
require.JSONEq(t, `{"message": "failed to update rule group: invalid alert rule: cannot have Panel ID without a Dashboard UID","error":"failed to update rule group: invalid alert rule: cannot have Panel ID without a Dashboard UID"}`, string(b))
}
// Now, let's see how this looks like.
@ -600,7 +600,7 @@ func TestPrometheusRulesFilterByDashboard(t *testing.T) {
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.JSONEq(t, `{"message": "API error","error":"invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax"}`, string(b))
require.JSONEq(t, `{"message": "invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax","error":"invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax"}`, string(b))
}
// Now, let's check a panel_id without dashboard_uid returns a 400 Bad Request response
@ -616,7 +616,7 @@ func TestPrometheusRulesFilterByDashboard(t *testing.T) {
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.JSONEq(t, `{"message": "API error","error":"panel_id must be set with dashboard_uid"}`, string(b))
require.JSONEq(t, `{"message": "panel_id must be set with dashboard_uid","error":"panel_id must be set with dashboard_uid"}`, string(b))
}
}

6
pkg/tests/api/alerting/api_ruler_test.go
Unescape View File

@ -428,7 +428,7 @@ func TestAlertRuleConflictingTitle(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
require.JSONEq(t, `{"message": "API error","error":"failed to update rule group: a conflicting alert rule is found: rule title under the same organisation and folder should be unique"}`, string(b))
require.JSONEq(t, `{"message": "failed to update rule group: a conflicting alert rule is found: rule title under the same organisation and folder should be unique","error":"failed to update rule group: a conflicting alert rule is found: rule title under the same organisation and folder should be unique"}`, string(b))
})
t.Run("trying to create alert with same title under another folder should succeed", func(t *testing.T) {
@ -782,7 +782,7 @@ func TestRulerRulesFilterByDashboard(t *testing.T) {
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.JSONEq(t, `{"message": "API error","error":"invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax"}`, string(b))
require.JSONEq(t, `{"message": "invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax","error":"invalid panel_id: strconv.ParseInt: parsing \"invalid\": invalid syntax"}`, string(b))
}
// Now, let's check a panel_id without dashboard_uid returns a 400 Bad Request response
@ -798,6 +798,6 @@ func TestRulerRulesFilterByDashboard(t *testing.T) {
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.JSONEq(t, `{"message": "API error","error":"panel_id must be set with dashboard_uid"}`, string(b))
require.JSONEq(t, `{"message": "panel_id must be set with dashboard_uid","error":"panel_id must be set with dashboard_uid"}`, string(b))
}
}

14
public/app/features/alerting/unified/api/ruler.ts
Unescape View File

@ -108,9 +108,6 @@ async function rulerGetRequest<T>(url: string, empty: T, params?: Record<string,
}
const notFoundError = error.status === 404;
const rulerNotSupported =
error.status === 500 &&
error.data.error?.includes('unexpected content type from upstream. expected YAML, got text/html');
if (notFoundError) {
// the endpoint will return 404 but confirm that it's a Cortex endpoint
@ -119,7 +116,7 @@ async function rulerGetRequest<T>(url: string, empty: T, params?: Record<string,
}
// any other 404 should throw an exception
throw new Error('404 from rules config endpoint. Perhaps ruler API is not enabled?');
} else if (rulerNotSupported) {
} else if (isRulerNotSupported(error)) {
// assert if the endoint is not supported at all
throw {
...error,
@ -139,8 +136,15 @@ function isResponseError(error: unknown): error is FetchResponse<ErrorResponseMe
return hasErrorCode && hasErrorMessage;
}
function isRulerNotSupported(error: FetchResponse<ErrorResponseMessage>) {
return (
error.status === 500 &&
error.data.message?.includes('unexpected content type from upstream. expected YAML, got text/html')
);
}
function isCortexErrorResponse(error: FetchResponse<ErrorResponseMessage>) {
return error.data.error?.includes('group does not exist') || error.data.error?.includes('no rule groups found');
return error.data.message?.includes('group does not exist') || error.data.message?.includes('no rule groups found');
}
export async function deleteNamespace(dataSourceName: string, namespace: string): Promise<void> {

Write Preview
Loading…
Cancel
Save